Fix issue 636, pass request object to authenticate function. #643

W0126 · 2018-09-09T10:15:39Z

Pass request object to authenticate function, request may used in authenticate function.

coveralls · 2018-09-09T10:20:05Z

Pull Request Test Coverage Report for Build 1163

1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 94.697%

Totals
Change from base Build 1157:	0.0%
Covered Lines:	1250
Relevant Lines:	1320

💛 - Coveralls

cleder · 2018-09-20T10:52:34Z

oauth2_provider/oauth2_validators.py

@@ -572,7 +572,7 @@ def validate_user(self, username, password, client, request, *args, **kwargs):
        """
        Check username and password correspond to a valid and active User
        """
-        u = authenticate(username=username, password=password)
+        u = authenticate(request, username=username, password=password)
        if u is not None and u.is_active:


is_active is already checked in django (>=1.11 I think) do we need to check this here again?

This fix make sure request as a parameter to authenticate function, so authentication module can get request information.

@cleder I agree regarding is_active, in fact Django also skips the check if user model doesn't implement is_active. Let's remove this part.

It could be a separate PR though. Approving as the fix is correct.

adamchainz · 2019-07-08T11:28:03Z

This would be useful for me. I just discovered django-axes, requires request to be passed, and throws an exception if it's not: https://django-axes.readthedocs.io/en/latest/3_usage.html#authenticating-users

What's the blocker on this getting merged? I'm happy to take over.

The [usage documentation](https://django-axes.readthedocs.io/en/latest/3_usage.html) advises to create subclass of `AxesBackend` to ignore the lack of `request` if necessary. I've done this in a project using `django-oauth-toolkit`, which doesn't pass `request` (though it should as per [this PR](django-oauth/django-oauth-toolkit#643)). This meant that the axes.W003 check was being triggered, so I've fixed it to check for subclasses of `AxesBackend` as well as the class itself.

W0126 · 2019-08-14T10:00:58Z

@adamchainz we may need at least 1 reviewers with write access to approve this.

adamchainz · 2019-08-14T13:00:38Z

@cleder ? Also I volunteer myself to be given write access if that helps :)

IvanAnishchuk

is_active check could be improved but otherwise valid fix.

IvanAnishchuk · 2019-10-19T23:50:59Z

oauth2_provider/oauth2_validators.py

@@ -572,7 +572,7 @@ def validate_user(self, username, password, client, request, *args, **kwargs):
        """
        Check username and password correspond to a valid and active User
        """
-        u = authenticate(username=username, password=password)
+        u = authenticate(request, username=username, password=password)
        if u is not None and u.is_active:


@cleder I agree regarding is_active, in fact Django also skips the check if user model doesn't implement is_active. Let's remove this part.

IvanAnishchuk · 2019-10-19T23:52:32Z

oauth2_provider/oauth2_validators.py

@@ -572,7 +572,7 @@ def validate_user(self, username, password, client, request, *args, **kwargs):
        """
        Check username and password correspond to a valid and active User
        """
-        u = authenticate(username=username, password=password)
+        u = authenticate(request, username=username, password=password)
        if u is not None and u.is_active:


It could be a separate PR though. Approving as the fix is correct.

adamchainz · 2019-11-19T17:03:15Z

Please revert this, it's passing a non django request to authenticate() !

blu14x · 2020-03-12T08:41:30Z

Please revert this, it's passing a non django request to authenticate() !

Exactly.
I hope this will be reverted sometime. Until then I use a PatchedTokenView for requests to /o/token/:

from django.contrib.auth import authenticate
from oauth2_provider.oauth2_validators import OAuth2Validator
from oauth2_provider.views import TokenView


class PatchedOAuth2Validator(OAuth2Validator):
    def validate_user(self, username, password, client, request, *args, **kwargs):
        u = authenticate(username=username, password=password)
        if u is not None and u.is_active:
            request.user = u
            return True
        return False


class PatchedTokenView(TokenView):
    validator_class = PatchedOAuth2Validator

n2ygk · 2020-03-13T12:15:04Z

I will try to look at this over the weekend. A PR with a failing test that shows the error and fixes it by reverting would be appreciated. Updated contributing guidelines hopefully reinforce why good test coverage is needed: https://django-oauth-toolkit.readthedocs.io/en/latest/contributing.html

…

On Thu, Mar 12, 2020 at 4:41 AM Nocturn ***@***.***> wrote: Please revert this, it's passing a non django request to authenticate() ! Exactly — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#643 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABBHS5ZWFXRMQANG656YCSDRHCN4TANCNFSM4FUA46GQ> .

…#643)" This reverts commit d5e6645.

indigodomo · 2020-03-16T21:56:20Z

When is this going to get updated in pypi? The broken version is in there now. I really don't want to have to deploy with my own copy...

n2ygk · 2020-03-16T22:06:46Z

You can always use a git reference in your requirements.txt. For example:

git+https://github.com/jazzband/django-oauth-toolkit.git@9bd4c1bfcf26eecf8fde793baa48c386885c19be

adamchainz · 2020-03-16T22:50:13Z

Or install from a commit has on github: https://adamj.eu/tech/2019/03/11/pip-install-from-a-git-repository/

The [usage documentation](https://django-axes.readthedocs.io/en/latest/3_usage.html) advises to create subclass of `AxesBackend` to ignore the lack of `request` if necessary. I've done this in a project using `django-oauth-toolkit`, which doesn't pass `request` (though it should as per [this PR](django-oauth/django-oauth-toolkit#643)). This meant that the axes.W003 check was being triggered, so I've fixed it to check for subclasses of `AxesBackend` as well as the class itself.

Fix issue 636, pass request object to authenticate function.

a68e7c7

cleder reviewed Sep 20, 2018

View reviewed changes

Merge branch 'master' into fix-issue-636

79012ec

adamchainz mentioned this pull request Jul 8, 2019

Don't trigger axes.W003 for subclasses of AxesBackend jazzband/django-axes#461

Merged

Merge branch 'master' into fix-issue-636

22bd0d8

IvanAnishchuk approved these changes Oct 19, 2019

View reviewed changes

IvanAnishchuk and others added 2 commits October 20, 2019 02:53

Merge branch 'master' into fix-issue-636

ff27e22

Merge branch 'master' into fix-issue-636

f6f9bb7

IvanAnishchuk merged commit d5e6645 into django-oauth:master Oct 20, 2019

This was referenced Nov 13, 2019

Request object missing from Django authentication #636

Closed

django-axes compatibility #712

Closed

Do you plan to release a new version soon? #697

Closed

W0126 deleted the fix-issue-636 branch November 27, 2019 16:55

blu14x mentioned this pull request Mar 12, 2020

AttributeError: get_full_path when running any rest_framework.test.APITestCase #808

Closed

2 tasks

n2ygk added a commit that referenced this pull request Mar 14, 2020

Revert "Fix issue #636, pass request object to authenticate function. (…

4f3a296

…#643)" This reverts commit d5e6645.

n2ygk mentioned this pull request Mar 14, 2020

Revert "Fix issue 636, pass request object to authenticate function." #812

Merged

n2ygk added a commit that referenced this pull request Mar 14, 2020

document revert of #643

10a05d0

auvipy pushed a commit that referenced this pull request Mar 16, 2020

Revert "Fix issue #636, pass request object to authenticate function. (…

bca1f91

…#643)" This reverts commit d5e6645.

auvipy pushed a commit that referenced this pull request Mar 16, 2020

document revert of #643

9bd4c1b

obulat mentioned this pull request Apr 21, 2021

Bump django-oauth-toolkit from 1.1.2 to 1.4.1 in /cccatalog-api (original #706) WordPress/openverse-api#53

Closed

Fix issue 636, pass request object to authenticate function. #643

Fix issue 636, pass request object to authenticate function. #643

Uh oh!

Conversation

W0126 commented Sep 9, 2018

Uh oh!

coveralls commented Sep 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 1163

💛 - Coveralls

Uh oh!

cleder Sep 20, 2018

Choose a reason for hiding this comment

Uh oh!

W0126 Sep 27, 2018

Choose a reason for hiding this comment

Uh oh!

IvanAnishchuk Oct 19, 2019

Choose a reason for hiding this comment

Uh oh!

IvanAnishchuk Oct 19, 2019

Choose a reason for hiding this comment

Uh oh!

adamchainz commented Jul 8, 2019

Uh oh!

W0126 commented Aug 14, 2019

Uh oh!

adamchainz commented Aug 14, 2019

Uh oh!

IvanAnishchuk left a comment

Choose a reason for hiding this comment

Uh oh!

IvanAnishchuk Oct 19, 2019

Choose a reason for hiding this comment

Uh oh!

IvanAnishchuk Oct 19, 2019

Choose a reason for hiding this comment

Uh oh!

adamchainz commented Nov 19, 2019

Uh oh!

blu14x commented Mar 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

n2ygk commented Mar 13, 2020 via email • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

indigodomo commented Mar 16, 2020

Uh oh!

n2ygk commented Mar 16, 2020

Uh oh!

adamchainz commented Mar 16, 2020

Uh oh!

Uh oh!

coveralls commented Sep 9, 2018 •

edited

Loading

blu14x commented Mar 12, 2020 •

edited

Loading

n2ygk commented Mar 13, 2020 via email •

edited

Loading