-
-
Notifications
You must be signed in to change notification settings - Fork 797
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
Topic/1112 #1113
Conversation
Thanks for this first PR. Hopefully one of the review team will take a look shortly. |
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!
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.
|
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:
***@***.***>
|
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.
|
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.md
updated (only for user relevant changes)AUTHORS