Fix client authentication for DeviceCodeGrant when getting a token #920
Conversation
…AuthorizationCodeGrant, DeviceCodeGrant, RefreshTokenGrant, and ResourceOwnerPasswordCredentialsGrant. Add some missing imports to the oauth2 __init__.py
| if self.request_validator.client_authentication_required( | ||
| request | ||
| ) and not self.request_validator.authenticate_client(request): | ||
| raise rfc6749_errors.InvalidClientError(request=request) |
There was a problem hiding this comment.
This already happens inside self.validate_token_request(request) below.
There was a problem hiding this comment.
Pull Request Overview
This PR centralizes client authentication logic by introducing a validate_client_authentication helper in the base grant type and replaces duplicated authentication checks in several grant flows. It also adds missing imports for device‐code endpoints and errors in the top‐level oauth2 package and includes new tests for DeviceCodeGrant authentication.
- Introduce
validate_client_authenticationinGrantTypeBaseand apply it in Authorization Code, Device Code, Refresh Token, and Resource Owner Password flows. - Replace inline client authentication blocks in grant type implementations with the new helper.
- Add tests for public vs. confidential client authentication errors in
DeviceCodeGrant. - Add missing imports for device‐code endpoints and errors in
oauthlib/oauth2/__init__.py.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/oauth2/rfc8628/grant_types/test_device_code.py | Add tests for public and confidential client authentication errors |
| oauthlib/oauth2/rfc8628/grant_types/device_code.py | Replace inline auth logic with validate_client_authentication |
| oauthlib/oauth2/rfc6749/grant_types/resource_owner_password_credentials.py | Use validate_client_authentication instead of inline checks |
| oauthlib/oauth2/rfc6749/grant_types/refresh_token.py | Replace inline auth checks with validate_client_authentication |
| oauthlib/oauth2/rfc6749/grant_types/base.py | Introduce validate_client_authentication helper |
| oauthlib/oauth2/rfc6749/grant_types/authorization_code.py | Invoke validate_client_authentication in token request validation |
| oauthlib/oauth2/init.py | Add missing relative imports for device‐code endpoints and errors |
Comments suppressed due to low confidence (2)
oauthlib/oauth2/rfc6749/grant_types/base.py:178
- [nitpick] The docstring only notes failure of client authentication; consider expanding it to describe behavior for both confidential and public clients and reference relevant RFC sections.
def validate_client_authentication(self, request):
oauthlib/oauth2/rfc6749/grant_types/authorization_code.py:455
- There are no existing tests verifying the client authentication logic for the Authorization Code grant; consider adding tests covering both confidential and public client scenarios.
self.validate_client_authentication(request)
| raise errors.InvalidClientError(request=request) | ||
|
|
||
| # Handles public clients | ||
| elif not self.request_validator.authenticate_client_id(request.client_id, request): |
There was a problem hiding this comment.
validate_client_authentication invokes authenticate_client_id without checking if request.client_id is set; if client_id is None, this may raise an unexpected exception. Consider adding an explicit guard or raising an InvalidClientError when request.client_id is missing before calling authenticate_client_id.
Copilot uses AI. Check for mistakes.
| @@ -66,5 +66,6 @@ | |||
| from .rfc6749.tokens import BearerToken, OAuth2Token | |||
| from .rfc6749.utils import is_secure_transport | |||
| from .rfc8628.clients import DeviceClient | |||
| from oauthlib.oauth2.rfc8628.endpoints import DeviceAuthorizationEndpoint, DeviceApplicationServer | |||
| from oauthlib.oauth2.rfc8628.grant_types import DeviceCodeGrant | |||
| from .rfc8628.endpoints import DeviceAuthorizationEndpoint, DeviceApplicationServer | |||
There was a problem hiding this comment.
[nitpick] Imports from the rfc8628 subpackage are not grouped alphabetically with the existing imports; consider organizing import order to match project style guidelines for readability.
Copilot uses AI. Check for mistakes.
Adds a helper method for validating client authentication and use it in
AuthorizationCodeGrant,DeviceCodeGrant,RefreshTokenGrant, andResourceOwnerPasswordCredentialsGrant.Putting the helper method in
DeviceCodeGrantalso fixes an issue where the client being public/confidential was not being taken into consideration.Add some missing imports to the oauth2 init.py that I noticed weren't there when implementing changes in my own code.
Resolves #919