feat: add more authentication information to swagger #35674
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
this is work in progress FIXES: APER-3554
* updates the `docs-settings` to make the generated swagger `securityDefinitions` include both JWT and CSRF methods, as well as basic. A few linter fixes happened as a side effect. * Put in wordier descriptions for all three, since we don't have great shared documentation about authn/authz. * Added CSRF to `login_session`, which also serves as a proof of concept for other endpoits * Also regenerated the swagger doc, which picked up some extra changes. Generated swagger now has help and allows extra auth methods so some preveiously unusable endpoints can be hit. FIXES: APER-3554
Fixed capitalization error, and ran the linter which reformatted a lot of the file, because github actions really didn't like some lint errors.
deborahgu
commented
Oct 18, 2024
| @@ -62,17 +65,17 @@ | |||
| log = logging.getLogger("edx.student") | |||
| AUDIT_LOG = logging.getLogger("audit") | |||
| USER_MODEL = get_user_model() | |||
| PASSWORD_RESET_INITIATED = 'edx.user.passwordreset.initiated' | |||
There was a problem hiding this comment.
I apologize for mixing reformatter auto fixes with actual changes in a single PR. Assume everything in this file except the blocks that I am flagging with particular comments are just automatic fixes to make the linter happier.
deborahgu
commented
Oct 18, 2024
Comment on lines
+26
to
+33
| from drf_yasg import openapi | ||
| from drf_yasg.utils import swagger_auto_schema | ||
| from edx_django_utils.monitoring import set_custom_attribute | ||
| from eventtracking import tracker | ||
| from openedx_events.learning.data import UserData, UserPersonalData | ||
| from openedx_events.learning.signals import SESSION_LOGIN_COMPLETED | ||
| from openedx_filters.learning.filters import StudentLoginRequested | ||
| from rest_framework import status |
There was a problem hiding this comment.
these lines are a real change, not just formatting.
deborahgu
commented
Oct 18, 2024
Comment on lines
+752
to
+764
| @swagger_auto_schema( | ||
| request_body=login_user_schema, | ||
| responses=login_user_responses, | ||
| security=[ | ||
| {"csrf": []}, | ||
| ], | ||
| ) | ||
| @method_decorator(csrf_protect) | ||
| def post(self, request, api_version): | ||
| """Log in a user. | ||
|
|
||
| See `login_user` for details. | ||
|
|
||
| Example Usage: | ||
|
|
||
| POST /api/user/v1/login_session | ||
| with POST params `email`, `password`. | ||
|
|
||
| 200 {'success': true} | ||
| """ | ||
| POST /user/{api_version}/account/login_session/ | ||
|
|
||
| Returns 200 on success, and a detailed error message otherwise. |
There was a problem hiding this comment.
these changes are real, not just formatter.
deborahgu
commented
Oct 18, 2024
Comment on lines
+715
to
+730
| login_user_schema = openapi.Schema( | ||
| type=openapi.TYPE_OBJECT, | ||
| properties={ | ||
| "email": openapi.Schema(type=openapi.TYPE_STRING), | ||
| "password": openapi.Schema(type=openapi.TYPE_STRING), | ||
| }, | ||
| ) | ||
|
|
||
| login_user_return_schema = openapi.Schema( | ||
| type=openapi.TYPE_OBJECT, | ||
| properties={ | ||
| "success": openapi.Schema(type=openapi.TYPE_BOOLEAN), | ||
| "value": openapi.Schema(type=openapi.TYPE_STRING), | ||
| "error_code": openapi.Schema(type=openapi.TYPE_STRING), | ||
| }, | ||
| ) |
There was a problem hiding this comment.
these changes are real, not just formatter.
deborahgu
commented
Oct 18, 2024
Comment on lines
+740
to
746
| login_user_responses = { | ||
| status.HTTP_200_OK: login_user_return_schema, | ||
| status.HTTP_400_BAD_REQUEST: login_user_return_schema, | ||
| status.HTTP_403_FORBIDDEN: login_user_return_schema, | ||
| } | ||
|
|
||
| @method_decorator(ensure_csrf_cookie) |
There was a problem hiding this comment.
these changes are real, not just formatter.
deborahgu
commented
Oct 18, 2024
| @@ -6788,6 +6826,59 @@ paths: | |||
| in: path | |||
| required: true | |||
| type: string | |||
| /mobile/{api_version}/notifications/create-token/: | |||
There was a problem hiding this comment.
these are unrelated changes, clearly the documents haven't been regenerated since these endpoints were created.
jsnwesson
reviewed
Oct 28, 2024
| 'token_type': 'jwt' | ||
| ``` | ||
|
|
||
| Your JWT will be returned in the response as `access_token`. Prefix with `JWT ` in your header. |
There was a problem hiding this comment.
is the space in JWT intentional? My assumption by reading this would be to add this to my header:
JWT {access token string}
jsnwesson
approved these changes
Oct 28, 2024
|
Everything looks good to me! |
…user_api-api-docs
the automatic reformatting broke some of the lint amnesty, so I undid the automatic reformatting on those lines in particular.
deborahgu
deleted the
dkaplan1/APER-3554_add-sample-payloads-to-user_api-api-docs
branch
|
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
|
2U Release Notice: This PR has been deployed to the edX production environment. |
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
docs-settingsto make the generated swaggersecurityDefinitionsinclude both JWT and CSRF methods, as well as basic. A few linter fixes happened as a side effect.login_session, which also serves as a proof of concept for other endpointsGenerated swagger now has help and allows extra auth methods so some preveiously unusable endpoints can be hit.
Testing instructions
make swaggerworkslms-openapi.ymlinto any OpenAPI validator or preview tool. Online tools are available at https://swagger.io/, and most IDEs have validating plugins.login_sessionImages
general authentication modal
login_sessionauthentication modalFIXES: APER-3554