Opened 3 years ago

Closed 3 years ago

#19780 closed Bug (fixed)

Discrepancy in modwsgi auth handler

Reported by: aaugustin Owned by: nobody
Component: contrib.auth Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

check_password does:

        if not user.is_active:
            return None

while groups_for_user does:

        try:
            if not user.is_active:
                return []
        except AttributeError as e:
            # a custom user may not support is_active
            return []

Shouldn't these be rewritten to:

        if not getattr(user, 'is_active', True):
            return None / return []

Change History (4)

comment:1 Changed 3 years ago by claudep

See #19057, [5f8b97f9fb058e5e02f1f99423fc3b0020ecdeb0] and [2b5f848207b1dab35afd6f63d0107629c76d4d9a].

If I understand well the note added in https://docs.djangoproject.com/en/dev/howto/deployment/wsgi/apache-auth/ the auth handler doesn't support a custom user without is_active. Then I think it'd better to let the AttributeError be raised up the stack, it will be easier to debug. Preston?

comment:2 Changed 3 years ago by Preston Holmes <preston@…>

In 0e18fb04bad99de237b5eb8ea4f9ff2f3cd147d3:

Made modwsgi groups_for_user consistent with check_password

2b5f848207b1dab35afd6f63d0107629c76d4d9a based its changes on #19061
that made the is_active attribute mandatory for user models.
The try/except was not removed for the groups_for_user function.

refs #19780

comment:3 Changed 3 years ago by Preston Holmes <preston@…>

In bb12ea2cf1cb0e093bd35886398e0f27d49fcde8:

[1.5.x] Made modwsgi groups_for_user consistent with check_password

2b5f848207b1dab35afd6f63d0107629c76d4d9a based its changes on #19061
that made the is_active attribute mandatory for user models.
The try/except was not removed for the groups_for_user function.

refs #19780

comment:4 Changed 3 years ago by ptone

  • Resolution set to fixed
  • Status changed from new to closed

#19061 actually stipulated that is_active is mandatory - but the attribute error catch was not removed from the groups method as it should have been in 2b5f848207b1dab35afd6f63d0107629c76d4d9a.

Thanks Aymeric for the catch.

Note: See TracTickets for help on using tickets.
Back to Top