-
-
Notifications
You must be signed in to change notification settings - Fork 34
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
Client secret removal #234
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The client_secret field has been removed from the serializers in the drf-social-oauth2 package. This change is made to enhance the security of our system, as the client_secret usually kept confidential and should not be serialized.
A new method, get_application, has been created. It takes the request's validated data and retrieves an Application object based on the provided client_id. If a client_id is not provided or is invalid, it returns a Response object with an appropriate error message and status code. This function is then used in the post method to further modify the request.
A logger was set up in drf_social_oauth2/views.py to produce a warning whenever a 'client_secret' is detected in the validated request data. This measure was taken to alert about potential security risks associated with unnecessarily exposing client secrets.
The `client_secret` is now no longer accepted as part of the request data in the `RevokeTokenSerializer` class, increasing overall security. A warning is also logged if `client_secret` is detected in the request data. To compensate for this change, the `client_secret` is now retrieved from the application itself in the `RevokeTokenSerializer` view post method.
Updated the drf-social-oauth2 test fixtures to include client_id in the app and modified the save function to get the first Application object for a user. The added client_id enhances test coverage and getting the first Application object handles scenarios where a User may associate with more than one Application.
The token field, which previously had a character limit of 5000, was removed from the RevokeTokenSerializer class in drf_social_oauth2/serializers.py. This simplifies the serializer as this field is no longer needed.
This commit updates the test cases in OAuth2 endpoints to use the first Application object associated with a user rather than getting it directly. It also extends tests for Application retrieval in the views to cover various scenarios including correct, incorrect, and missing client_id. Changes in test data have also been reflected, streamlining correct client_ids. Some redundant token tests in views have been removed.
Our application now extracts the token from the authorization header in the RevokeToken endpoint. This change has been made to facilitate a more secure token revoke in the drf-social-oauth2 authentication, where the token in use is pulled directly from the header instead of being passed through request data.
This change updates the method of retrieving a user's application in the test fixtures of the drf-social-oauth2 project.
Removed 'client_secret' from OAuth2 tests to simplify token requests. Updated 'get_application' function to 'get_application_or_400' across the test_views.py file. Also, added a test case for token revocation upon receiving an invalid token endpoint for enhanced security checks.
The method get_application has been renamed to get_application_or_400 to more accurately reflect its behavior. This change reflects across the entire module, impacting both the ConvertTokenSerializer and RevokeTokenSerializer methods, which now call the renamed method. The new name indicates the method returns an HTTP 400 error if the application is not retrieved properly.
The hardcoded database parameters in the test_settings.py file have been replaced with environmental variables. This improves security and enables easier configuration in various deployment environments.
The get_application method has been refactored to return None when the application doesn't exist or the client_id is missing. The check to determine if the application is not found has been moved to ConvertTokenSerializer and RevokeTokenSerializer methods. Corresponding test cases have also been updated to reflect the changes.
The version number of drf-social-oauth2 has been updated to 3.0.0 from 2.3.0 in the __init__.py, the conf.py in the docs, and the installation instructions in installation.rst. This change reflects the new additional features and improvements in the new version, which are yet to be documented.
The 'client_secret' field has been removed from the API definitions in the 'api.yaml' file. This change affects both the token request and revoke token request definitions. The removal of this field simplifies the API usage and increases security by no longer requiring the client secret for these operations.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Please include changes' summary that your pull request entails.
Fixes # (issue)
Checklist