Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

typing issues #856

Open
jvanasco opened this issue Aug 24, 2023 · 2 comments
Open

typing issues #856

jvanasco opened this issue Aug 24, 2023 · 2 comments

Comments

@jvanasco
Copy link
Contributor

I've run into some issues adding typing to a project that uses oauthlib.

The first issue is regarding oauthlib.common.Request.

The package is managing many object attributes through a dict, which creates issues with typing. See https://github.com/oauthlib/oauthlib/blob/master/oauthlib/common.py#L360-L401

I initially opened a PR against the stubs here (python/typeshed#10613), however typeshed noticed this section of code appears to be designed to handle parsed http(s) request data and should be strints.

This pattern is not shared across the library.

For example, Reference the docs:

# oauthlib_request.client = the client associated with the token

# oauthlib_request has a few convenient attributes set such as
# oauthlib_request.client = the client associated with the token
# oauthlib_request.user = the user associated with the token
# oauthlib_request.scopes = the scopes bound to this token

And then again here, we see that request.client should be an object:

if not self.request_validator.authenticate_client(request):
log.debug('Client authentication failed, %r.', request)
raise errors.InvalidClientError(request=request)
elif not hasattr(request.client, 'client_id'):
raise NotImplementedError('Authenticate client must set the '
'request.client.client_id attribute '
'in authenticate_client.')

This is repeated often throughout the code.

It's been a few years since I've worked much with this library, but I believe the following are supposed to be objects:

  • client
  • user
  • refresh_token

And several others should be Lists of strings.

IMHO, the Request class should have declared attributes for the objects, lists and strings that are not simply decoded from the http(s) query.

@jvanasco
Copy link
Contributor Author

Another issue with typing can only be solved with a breaking change:

This library often uses the same name for an object of different types. This happens a lot with "token"s – which can either be a string OR an object/dict. I think users of the library would benefit if tokens that should be objects/dicts were named appropriately.

@auvipy
Copy link
Contributor

auvipy commented Sep 26, 2023

I am open to any relevant improvement proposal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants