Code

#19057 closed Bug (fixed)

mod_wsgi authentication handlers fail with custom user model

Reported by: russellm Owned by: nobody
Component: contrib.auth Version: master
Severity: Release blocker Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The mod_wsgi authentication handler introduced in r373932fa don't work if you have a custom User model.

I can't see any fundamental reason that they shouldn't be able to be made to work for the general case. At the very least, the tests should be marked @skipIfCustom

Marking release blocker because it's a bug in a new feature.

Attachments (0)

Change History (10)

comment:1 Changed 19 months ago by ptone

  • Has patch set
  • Triage Stage changed from Accepted to Ready for checkin

comment:2 Changed 19 months ago by Preston Holmes <preston@…>

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

In 5f8b97f9fb058e5e02f1f99423fc3b0020ecdeb0:

Fixed #19057 -- support custom user models in mod_wsgi auth handler

thanks @freakboy3742 for the catch and review

comment:3 Changed 19 months ago by russellm

  • Resolution fixed deleted
  • Status changed from closed to reopened

Work is still required; the committed test:

1) doesn't check the behavior for custom users, and
2) assumes that User is always available.

comment:4 Changed 19 months ago by ptone

  • Triage Stage changed from Ready for checkin to Accepted

This should hopefully address point 1:

https://github.com/ptone/django/commit/a1da92acb9fb0fa095ad850a3e575071804cdffc

But I'm unclear as to point 2

There are lots of places in the test suite that still refer to a hard coded django.contrib.auth.models.User

We still need to have our coverage for the default user model, and it doesn't harmful to leave those concrete references in tests as long as the custom user option is also tested.

Are you suggesting we adopt the same UserModel = auth.get_user_model pattern for all test code that test user behavior? That would seem to leave the default user model untested if run in a project with a custom auth user.

comment:5 Changed 19 months ago by ptone

ref #19050 - I think that actually clarifies the issue for point 2 from comment 3 above

will investigate with test project I have on hand that sets auth user model setting

comment:6 Changed 19 months ago by ptone

seeing the light now on point 2 after running it with a sample project where user model swapped out - this should be fixed with:
https://github.com/ptone/django/commit/fc2c4be763088687765abfdee5563e58ee279c86

I'm wondering if we should have something like this set up on the CI server - not too many tickets will bump against this issue - but until you get into the mindset that User isn't always available in the test suite (though User._meta.installed == True), its hard to remember that it is not available in tests the way it always has been.

also I'm going to look at if we can improve the error, which currently is AttributeError: type object 'User' has no attribute 'objects' vs DatabaseError: no such table: <...> for non-installed models.

OK so the issue the manager not being there is due to a check of 'swapped' in setting up the manager:

Though I haven't run the whole suite I can take out those, checks, restoring the standard manager behavior and no tests fail that I might expect to fail.

Obviously these tests on this issue will change depending on the outcome of #19061 - currently I do NOT assume is_valid is true - since there might be some other way a user is invalid, and I want to default prevent access. Of course raising a NotImplemented would be viable here as well.

comment:7 Changed 19 months ago by russellm

Ok - I put *in* the swapped checks by request during the review process because the feedback I got was that "table does not exist" wasn't a meaningful error. Admittedly, the error messages that are now generated aren't really much better.

Instead of removing the checks, a better approach would be to install a dummy manager object that returns an error when it's used.

comment:8 Changed 19 months ago by ptone

  • Triage Stage changed from Accepted to Ready for checkin

updated and hopefully final patch:

https://github.com/ptone/django/compare/modwsgi-customuser

pending final resolution on #19061

comment:9 Changed 18 months ago by Russell Keith-Magee <russell@…>

In 81f5d4a1a7b955afe530d5292726b3a8a93d7038:

Added some test guards for some recently added auth tests.

Refs #19061, #19057.

comment:10 Changed 18 months ago by ptone

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

This feature and it's tests should now be custom user compatible:

github-trac didn't seem to auto-reclose this ticket.

https://github.com/django/django/commit/2b5f848207b1dab35afd6f63d0107629c76d4d9a

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.