- Notifications
You must be signed in to change notification settings - Fork 824
Topic/1112#1113
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
Topic/1112 #1113
Uh oh!
There was an error while loading. Please reload this page.
Conversation
n2ygk commented Feb 9, 2022
Thanks for this first PR. Hopefully one of the review team will take a look shortly. |
Andrew-Chen-Wang left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
codecovbot commented Feb 9, 2022 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Codecov Report
@@ Coverage Diff @@## master #1113 +/- ## ======================================= Coverage 96.62% 96.62% ======================================= Files 32 32 Lines 1806 1806 ======================================= Hits 1745 1745 Misses 61 61 Continue to review full report at Codecov.
|
daffyd commented Feb 9, 2022 via email
Adding in the suggested changes. And yes - copied the wrong import and of course my eyes saw the wrong thing when I reviewed the result. sigh. As far as overriding the default behavior of permission_classes - I am up to alternate suggestions, but that was the only thing that worked for me. I am using application_c.GRANT_CLIENT_CREDENTIALS to create the application. (maybe improperly.... as I don't set a user as this is m2m) Certainly could suggest for more complex operations overriding get_permission_classes() would make more sense ? There is a lot of chatter on this I found while fighting the issue, and this was the only suggestion that handled the case. But django/rest/oauth2 newbie here. …On Wed, Feb 9, 2022 at 1:29 PM Andrew Chen Wang ***@***.***> wrote: ***@***.**** requested changes on this pull request. Thanks for the PR! ------------------------------ In docs/tutorial/tutorial_03.rst <#1113 (comment)> : > @@ -78,3 +78,29 @@ Now supposing your access token value is `123456` you can try to access your aut :: curl -H "Authorization: Bearer 123456" -X GET http://localhost:8000/secret + +Working with Rest_framework generic class based views +----------------------------------------------------- + +If you have completed the `Django REST framework tutorial +<https://www.django-rest-framework.org/tutorial/3-class-based-views/#using-generic-class-based-views>`_, +you will be familiar with the 'Snippet' example, in particular the SnippetList and SnippetDetail classes. + +It would be really nice to reuse those views, but also support token handling. Instead of reworking +those classes to be ProtectedResourceView based, the solution is much simpler than that. + +Assuming you have already modified the settings as was already shown. ⬇️ Suggested change -Assuming you have already modified the settings as was already shown. +Assume you have already modified the settings as was already shown. It might be better to combine this sentence with the next paragraph. ------------------------------ In docs/tutorial/tutorial_03.rst <#1113 (comment)> : > @@ -78,3 +78,29 @@ Now supposing your access token value is `123456` you can try to access your aut :: curl -H "Authorization: Bearer 123456" -X GET http://localhost:8000/secret + +Working with Rest_framework generic class based views +----------------------------------------------------- + +If you have completed the `Django REST framework tutorial +<https://www.django-rest-framework.org/tutorial/3-class-based-views/#using-generic-class-based-views>`_, +you will be familiar with the 'Snippet' example, in particular the SnippetList and SnippetDetail classes. + +It would be really nice to reuse those views, but also support token handling. Instead of reworking +those classes to be ProtectedResourceView based, the solution is much simpler than that. + +Assuming you have already modified the settings as was already shown. + +The key is setting a class variable to override the default *permissions_classes* with something that will use our :term:`Access Token` properly. ⬇️ Suggested change -The key is setting a class variable to override the default *permissions_classes* with something that will use our :term:`Access Token` properly. +The key is setting a class attribute to override the default *permissions_classes* with something that will use our :term:`Access Token` properly. ------------------------------ In docs/tutorial/tutorial_03.rst <#1113 (comment)> : > +you will be familiar with the 'Snippet' example, in particular the SnippetList and SnippetDetail classes. + +It would be really nice to reuse those views, but also support token handling. Instead of reworking +those classes to be ProtectedResourceView based, the solution is much simpler than that. + +Assuming you have already modified the settings as was already shown. + +The key is setting a class variable to override the default *permissions_classes* with something that will use our :term:`Access Token` properly. + +.. code-block:: python + + from django.contrib.auth.decorators import login_required + + class SnippetList(generics.ListCreateAPIView): + ... + permission_classes = [TokenHasReadWriteScope] I believe this will override other default permission classes. Perhaps you can override the get_permission_classes method ------------------------------ In docs/tutorial/tutorial_03.rst <#1113 (comment)> : > +----------------------------------------------------- + +If you have completed the `Django REST framework tutorial +<https://www.django-rest-framework.org/tutorial/3-class-based-views/#using-generic-class-based-views>`_, +you will be familiar with the 'Snippet' example, in particular the SnippetList and SnippetDetail classes. + +It would be really nice to reuse those views, but also support token handling. Instead of reworking +those classes to be ProtectedResourceView based, the solution is much simpler than that. + +Assuming you have already modified the settings as was already shown. + +The key is setting a class variable to override the default *permissions_classes* with something that will use our :term:`Access Token` properly. + +.. code-block:: python + + from django.contrib.auth.decorators import login_required Random import? Also if login is required, then it is necessary not to override the class attribute and instead override the method. ------------------------------ In docs/tutorial/tutorial_03.rst <#1113 (comment)> : > @@ -78,3 +78,29 @@ Now supposing your access token value is `123456` you can try to access your aut :: curl -H "Authorization: Bearer 123456" -X GET http://localhost:8000/secret + +Working with Rest_framework generic class based views +----------------------------------------------------- + +If you have completed the `Django REST framework tutorial +<https://www.django-rest-framework.org/tutorial/3-class-based-views/#using-generic-class-based-views>`_, +you will be familiar with the 'Snippet' example, in particular the SnippetList and SnippetDetail classes. + +It would be really nice to reuse those views, but also support token handling. Instead of reworking ⬇️ Suggested change -It would be really nice to reuse those views, but also support token handling. Instead of reworking +It would be nice to reuse those views **and** support token handling. Instead of reworking — Reply to this email directly, view it on GitHub <#1113 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACN7V4OLMEPYDXOUAK46VZDU2KXB7ANCNFSM5N6IDDBQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you authored the thread.Message ID: ***@***.***> |
Andrew-Chen-Wang commented Feb 9, 2022 via email • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
On my phone so not sure if this is the right suggestion, but to override get permission classes, you can just call super() and extend with the remaining permission classes to be added. Then return. |
Andrew-Chen-Wang left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM thanks!
Fixes #
1112
Description of the Change
Modifying the tutorial to suggest how to extend the rest_framework class based views tutorial to use oauth-toolkit.
Checklist
CHANGELOG.mdupdated (only for user relevant changes)AUTHORS