Opened 12 years ago
Last modified 5 weeks ago
#20313 assigned New feature
AnonymousUser should follow custom User implementation
Reported by: | Owned by: | thinkingpotato | |
---|---|---|---|
Component: | contrib.auth | Version: | |
Severity: | Normal | Keywords: | |
Cc: | julenx@…, Sergey Fedoseev, Tobias Wiese, Evstifeev Roman | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Introducing custom User classes opened a few new options for handling authorization logic, e.g.:
self.request.user.has_purchased(object)
or as @akaariai mentioned:
request.user.has_role_in_org(some_org)
Without being able to define custom AnonymousUser class that follows User implementation this will not work.
There are some ideas on how to solve that, and the ones discussed are:
- defining
anonymous_user_class
onUserClass
(@akaariai) - merging
User
andAnonymousUser
(@apollo13)
The current dirty patch uses the same approach as with get_user_model()
:
- django.contrib.auth.get_anonymous_model
- django.conf.global_settings.AUTH_ANONYMOUS_MODEL
and changes in:
- django.contrib.auth.context_processors
- django.db.models.sql.where.WhereNode
Attachments (1)
Change History (13)
comment:1 by , 12 years ago
comment:2 by , 12 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
follow-up: 6 comment:4 by , 12 years ago
The obvious workaround in current code is to just check if user is authenticated first, as documented here:
https://docs.djangoproject.com/en/dev/topics/auth/default/#authentication-in-web-requests
If the custom user method you are calling returns a value that you are checking truthiness on, then you can and
it with is_authenticated using python's logic shortcutting.
Adding one line, and one level of indent seems reasonable amount of effort given the alternative of having to define an anonymous mirror class for your custom user.
This has the advantage of keeping it even more explicit in you code when you are doing something with an authenticated user.
Is there something more than code conciseness that would be enabled by setting up a custom anon user? Perhaps I'm missing some of the argument in favor of doing this.
comment:5 by , 11 years ago
Triage Stage: | Unreviewed → Accepted |
---|
Accepting on the basis that there is something that needs to be addressed here. @ptone makes a good point about checking is_authenticated first, but I can see how being able to "just use it" could be useful under some circumstances, and we've been encouraging that behaviour in the days of non-swappable User models.
Personally, I'm not completely sold that get_anonymous_model() or ANONYMOUS_USER_MODEL is needed here - that feels a bit like overkill. However, I'm not sure I've got a substantially better option, other than some sort of implicit contract about has_* methods returning True (or something similar)
comment:6 by , 11 years ago
Replying to ptone:
The obvious workaround in current code is to just check if user is authenticated first, as documented here:
Is there something more than code conciseness that would be enabled by setting up a custom anon user? Perhaps I'm missing some of the argument in favor of doing this.
Let's say you ask an outsourced programmer to write a module that will enable some kind of model-level authorization mechanism or whatever requiring user based on current request. And that programmer has no idea about Django internals. All you define is the interface you both will be using, which is quite straigthforward: passing MyUser instance as the first argument. But suddenly it's not MyUser instance anymore, request.user
becomes a AnonymousUser that is substantially different class, implementing different methods and behaviour. Moreover, you can't even change that. It just makes no sense. Code conciseness, even if was the only reason for solving this issue, in my opinion is the most important one to let people do their stuff easily and logically. Speaking of my personal preferences, I would rather put one line in my custom User class, rather than repeat
is_authenticated
~80 times (after looking at my recent code).
Replying to russellm:
Accepting on the basis that there is something that needs to be addressed here. @ptone makes a good point about checking is_authenticated first, but I can see how being able to "just use it" could be useful under some circumstances, and we've been encouraging that behaviour in the days of non-swappable User models.
Personally, I'm not completely sold that get_anonymous_model() or ANONYMOUS_USER_MODEL is needed here - that feels a bit like overkill. However, I'm not sure I've got a substantially better option, other than some sort of implicit contract about has_* methods returning True (or something similar)
It feels a bit like overkill to me as well, but on the other hand seems like a price you have to pay for tight integration between different Django services.
If I understand you correctly, developers would not be able to use methods starting with a keyword reserved by Django. Personally, I would be scared if framework interfered my design decisions so much. On the other hand, I bet that the first day after introducing it #django IRC would be flooded with questions on how to use it. Defining a class is clean and simple. One sentence in the docs. Everyone who did object programming will understand it.
comment:7 by , 11 years ago
Cc: | added |
---|
We basically have the same issue but would need to have a different solution, since we're using a special user under a reserved username which is always considered to be anonymous.
Perhaps an ANONYMOUS_USER
attribute in the AUTH_USER_MODEL
model could define what an anonymous user is: a string that matches a value for USERNAME_FIELD
, a specific instance of a class etc.
comment:8 by , 11 years ago
This is actually a very neat idea: to create a common interface for all use cases. Assigning a function that returns whatever is needed would be a nice addition! +1
comment:9 by , 9 years ago
I've hit this problem as well. I'm trying to add a property to each user and reference it in the template, with a default for an anonymous user, and it's much, much harder since I can't add the same method to the anonymous user as the authenticated user.
However, I'm not really sure why Django even has two classes for this. Wouldn't an anonymous user be a regular User
object for which is_anonymous()
would return True
? is_anonymous()
could decide this any way it liked (lack of id
, lack of some specific property, or even just _anonymous
set to True).
That way, all methods would distinguish between authenticated and unauthenticated users through the standard interface, i.e. the is_anonymous()
method (and its converse, is_authenticated()
). Does that make sense?
by , 7 years ago
Attachment: | patch-20313-20170608.diff added |
---|
A solution which works for me (using branch 1.11rc1) and maybe works for everybody but needs more work to be complete
comment:10 by , 6 years ago
Cc: | added |
---|
comment:11 by , 5 years ago
Cc: | added |
---|
comment:12 by , 5 weeks ago
Cc: | added |
---|
Forget the change in
django.db.models.sql.where.WhereNode
- completely out of topic.