newforms-admin and django.contrib.auth
James Bennett
ever-so-close to being able to handle one cool use case which came up
during the design discussion at PyCon: running django.contrib.admin
without django.contrib.auth.
The implemention of has_permission() on AdminSite, and
has_(add|change|delete)_permission() on ModelAdmin, gets it almost all
the way there, but leaves two stumbling blocks:
1. Creating instances of LogEntry, which FK to
django.contrib.auth.models.User, and
2. Creating instances of Message, which also FK to
django.contrib.auth.models.User.
Right now, you *can* work around this by overriding/duplicating a lot
of code in ModelAdmin, but factoring out log entries and messages into
methods -- similar to what's already been done with permission checks
-- would make that unnecessary.
I'm not particularly attached to these method names, but adding
methods on ModelAdmin, say, log_action() and send_user_message(), and
having the object-saving code call those methods instead of directly
handling logging and messages, would solve this pretty cleanly (and
also add a little bit more useful functionality for someone who wants
to roll their own implementation of these features).
Anyone have strong feelings one way or another?
--
"Bureaucrat Conrad, you are technically correct -- the best kind of correct."
Malcolm Tredinnick
On Sun, 2007-12-16 at 22:04 -0600, James Bennett wrote:
> While playing around a bit with newforms-admin, I noticed that it's
> ever-so-close to being able to handle one cool use case which came up
> during the design discussion at PyCon: running django.contrib.admin
> without django.contrib.auth.
I'm either confused or scared. Presumably the admin application still
relies on authentication, right? Without making me rustle through all
the code, can you explain how it is enforcing access restrictions
without the authentication app?
Malcolm
--
A conclusion is the place where you got tired of thinking.
http://www.pointy-stick.com/blog/
James Bennett
> I'm either confused or scared. Presumably the admin application still
> relies on authentication, right? Without making me rustle through all
> the code, can you explain how it is enforcing access restrictions
> without the authentication app?
It still relies on knowing whether someone has permission to do
something, just not on the specific implementation in
django.contrib.auth. At the moment, for example, access to the admin
interface is determined by AdminSite.has_permission(), which receives
the HttpRequest and returns a boolean. Similarly, ModelAdmin has
methods -- has_add_permission(), has_change_permission() and
has_delete_permission() -- which receive the HttpRequest (and the
model object, in the change/delete cases) and return booleans.
This means the default implementation can happily use
django.contrib.auth and the existing is_staff flag and permission
system, but that using something else is as easy as subclassing and
overriding the right methods. And with a little refactoring to remove
the direct reliance on the LogEntry and Message models, it would be
easy to run the Django admin without django.contrib.auth, simply by
plugging in whatever auth/permissions system you like in the
appropriate places.
Simon Willison
On 17 Dec 2007, at 08:08, James Bennett wrote:
> This means the default implementation can happily use
> django.contrib.auth and the existing is_staff flag and permission
> system, but that using something else is as easy as subclassing and
> overriding the right methods. And with a little refactoring to remove
> the direct reliance on the LogEntry and Message models, it would be
> easy to run the Django admin without django.contrib.auth, simply by
> plugging in whatever auth/permissions system you like in the
> appropriate places.
I love it. This would make it much cleaner to integrate the Django
admin with existing authentication schemes (LDAP, Active Directory,
whatever custom SSO mechanism Oracle sell to big companies) without
having to create a new Django user for every existing SSO user and
keep the two in sync somehow. Sounds like it wouldn't be an enormous
change either.
Cheers,
Simon
flo...@gmail.com
upon emission of some signal? That way, other apps could hook in and
log their own actions as well, along with removing the admin's
dependency on auth. It could go into django.contrib.logging or
something.
What do you guys think of that?
James Bennett
> Tell me if I'm crazy here, but what if LogEntry objects were created
> upon emission of some signal? That way, other apps could hook in and
> log their own actions as well, along with removing the admin's
> dependency on auth. It could go into django.contrib.logging or
> something.
>
> What do you guys think of that?
I think it's a bad idea. Signal dispatch involves much, much, much,
much, *much* more overhead than a simple method call, so using a
signal where there's no clearly-defined need for it always gets a -1
from me (despite the popularity of "define a signal for X" requests on
this list).
Also, I don't think that the admin's LogEntry model needs to be, or
should be, exposed in this fashion. It's logically part of the
administrative interface, and trying to force it into a "generic
logging" hole is probably a bad idea.
flo...@gmail.com
more going on there than I previously thought (possibly due to the
frequency of signal requests, like you mentioned).
Ivan Sagalaev
> This means the default implementation can happily use
> django.contrib.auth and the existing is_staff flag and permission
> system, but that using something else is as easy as subclassing and
> overriding the right methods. And with a little refactoring to remove
> the direct reliance on the LogEntry and Message models, it would be
> easy to run the Django admin without django.contrib.auth
ditch User model but still use django.contrib.auth to call custom
AUTHENTICATION_BACKENDs, use it's context processor to populate {{ user }}?
Joseph Kocherhans
I don't want to speak for James, but that's how I see it.
Joseph
Joseph Kocherhans
+1 on the concept. There are details to figure out, for instance,
which method is in charge of generating the actual content of the
message, but it shouldn't be that difficult. I've been wanting this
for well, nearly as long as Django has existed. I'd lost sight of that
and didn't realize we were so close.
Joseph
Brian Rosner
<ubern...@gmail.com> said:
+1. There are a few other places in ModelAdmin that should be split out
into its own method, but unrelated to this. This would definitely solve
this problem cleanly for those looking for this feature.
--
Brian Rosner
http://oebfare.com
Ivan Sagalaev
> I don't want to speak for James, but that's how I see it.
Then this is great!
Russell Keith-Magee
>
> While playing around a bit with newforms-admin, I noticed that it's
> ever-so-close to being able to handle one cool use case which came up
> during the design discussion at PyCon: running django.contrib.admin
> without django.contrib.auth.
> Anyone have strong feelings one way or another?
+1 from me. I don't have a particular use case for this myself, but I
agree that being able to swap out contrib.auth with an alternate admin
scheme is a cool use case that we should support if it is possible to
do so.
Yours,
Russ Magee %-)
David Danier
> methods on ModelAdmin, say, log_action() and send_user_message(), and
> having the object-saving code call those methods instead of directly
> handling logging and messages, would solve this pretty cleanly (and
> also add a little bit more useful functionality for someone who wants
> to roll their own implementation of these features).
>
> Anyone have strong feelings one way or another?
Correct me if I'm wrong, but doesn't this miss the big parts when
writing your own auth-application. After adding this two methods the
tables for LogEntry and Message still exist in the database, but are
unusable. If you want to use a similar functionality you have to replace
all parts of the code using Message and/or LogEntry-models as
User.message_set/logentry_set are not valid any more.
For the re-usability of applications this does not help either. If you
need your own auth-system in one application there is no way to get
third-party-applications to use your auth-system without really ugly hacks.
Currently it looks like parts of generic-auth are going into
newforms-admin (permission-check-functions). I think this is not ideal,
because now you have a nice designed system to authorize users, but it
comes bundled to the admin.
So I think having independent application for authorization would be
better, newforms-admin could still use this. (Additionally I would split
"auth" into authorization (has the permission?) and authentication (is
login valid?))
As I did write my own auth-system I know that beeing able to fix the
ForeignKeys to User in _one_ application is mostly not enough (this
change does not fis django.contrib.comments for example). Anyway, it
looks like this does not get fixed here at all, as the ForeignKeys _do_
still exist, but are not usable any more. So you need to skip
message/log-creation at all or replace the code by using your own copy
of LogEntry/Message with replaces ForeignKeys (To get this clear: 100%
copy with only the import of User-model is enough). Thats sad, even if
only because you have to duplicate code (no DRY) and can't use the
things provided by django any more (without creating a copy).
I once tried to get a solution by being able to _really_ replace the
auth-application (meaning you can replace
django.contrib.auth.models.User and similar), but this did not get much
attention.
Proof of concept: https://svn.webmasterpro.de/django-auth-rewrite/
...works fine in my own application, but I did not patch django, instead
I put it into my own application and replaced the imports in the current
admin to get it working.
I really think things could get a little less coupled to the auth-system
django provides, as I needed this myself. So strictly speaking I'm for
every change getting closer to a solution.
But I don't think "fixing" (or working around) things in one application
is enough, there needs to be a solution that provides this functionality
everywhere or it needs to get fixed everywhere.
See
http://code.djangoproject.com/browser/django/trunk/django/views/generic/create_update.py#L42
for an example of coupling to django.contrib.auth: The generic view
assumes request.user is available (which might not be the case when not
using django.contrib.auth or not using an auth-system at all) and that
it has a is_authenticated/message_set attribute (which might be ok, as
this could be called "API" you need to reproduce).
Greetings, David Danier
David Danier
> I'm not particularly attached to these method names, but adding
> methods on ModelAdmin, say, log_action() and send_user_message(), and
> having the object-saving code call those methods instead of directly
> handling logging and messages, [...]
Everybody who replaces the auth-system could simply provide
User.message_set/logentry_set. Thats how I did it, this "fixes" things
in more places than only the admin (e.g. generiv views).
Example:
class DjangoMessageUserSet(object): # dummy object
def create(self, *args, **kwargs):
pass
class User(models.Model):
# [...]
message_set = DjangoMessageUserSet()
So having log_action() and send_user_message() would mainly be nice to
clean the code, but does not really provide any additional flexibility
when replacing the auth-system. As said before: It only fixes one
application out of many.
Instead replacing code like LogEntry.objects.log_action(...) with
request.user.logentry_set.create(...) would help, not only in the admin.
Greetings, David Danier
James Bennett
> Correct me if I'm wrong, but doesn't this miss the big parts when
> writing your own auth-application. After adding this two methods the
> tables for LogEntry and Message still exist in the database, but are
> unusable. If you want to use a similar functionality you have to replace
> all parts of the code using Message and/or LogEntry-models as
> User.message_set/logentry_set are not valid any more.
If it is apocalyptically destructive to your application for there to
be tables it doesn't use, then this is a problem.
If you don't mind the fact that you won't be using the defaults, then
this is not a problem.
Your complaints about code duplication are somewhat strange to me,
because I would assume that if you're supplying your own
authentication/permission scheme you would expect that things which
use the Django auth scheme simply would not work, and would have
supplied your own implementations of anything your system actually
requires.
Meanwhile I'm not really sure why you'd fall back to emulating things
like message_set -- just remove the auth context processor and (once
this change to newforms-admin gets sorted out) there shouldn't be
anything which needs to access that. If there is anything which needs
to access that, bring it up and we'll see if it can't be changed.
David Danier
> be tables it doesn't use, then this is a problem.
Thats not really the problem, even if I don't really like it and don't
think it's what perfectionists like either.
> If you don't mind the fact that you won't be using the defaults, then
> this is not a problem.
True, but having to duplicate every app in django.contrib I want to use
(and which uses django.contrib.auth.models.User) only because the
ForeignKey is wrong, that's bad in my opinion. And fixing the admin will
not really help with this issue. So I think it would be nicer to really
fix the problem in django.contrib.auth, not django.contrib.admin.
If for example django.contrib.auth.models would do something like "from
the.place.where.i.put.my.auth.models import User" every application
importing User from django.contrib.auth.models would just work...if you
provide some basic API the application needs of course.
Now putting the creation of Message and LogEntry-objects into it's own
methods in ModelAdmin is not even half the fix you need:
* Having to override this methods is not really much cleaner than the
other ways to accomplish this
* Every third-party-application will need this change, as they will not
use your ModelAdmin-derivate by default
* Some parts of django do not use these admin-methods and should not do
so (because this would bundle them to the admin), but they do use things
like User.message_set
Additionally this change will only help users in a simple way that don't
care if they have Message or LogEntry available. Everybody else needs so
put their own code into log_action() and send_user_message() to get a
functionality that is already inside the admin, but only not working
because some key references the wrong model (everything else would work
perfectly).
> Your complaints about code duplication are somewhat strange to me,
> because I would assume that if you're supplying your own
> authentication/permission scheme you would expect that things which
> use the Django auth scheme simply would not work, and would have
> supplied your own implementations of anything your system actually
> requires.
I did replace the django.contrib.auth mainly because I wanted (read:
needed) to put a guest-user into the database instead of using
AnonymousUser (I changed things of the User-model later, but thats what
got me started). So my model and everything else was just like the
application django provides. Sadly all of the other things django
provides (which use django.contrib.auth) did not work afterwards.
A simple fix would have been to replace the django.contrib.auth-module
itself (I love that Python allows this from other modules), but it
seemed hackish to me. Patching django is an option, but only if you
know, that your patch might probably go into django. Otherwise you will
need to maintain this patch forever.
So forking django.contrib.auth was my way to go. And it way certainly
easy, as I really only needed to fix some imports and remove the
following line to get the admin working again:
http://code.djangoproject.com/browser/django/trunk/django/contrib/admin/views/main.py#L29
> Meanwhile I'm not really sure why you'd fall back to emulating things
> like message_set -- just remove the auth context processor and (once
> this change to newforms-admin gets sorted out) there shouldn't be
> anything which needs to access that. If there is anything which needs
> to access that, bring it up and we'll see if it can't be changed.
One example might be the generic-views, as stated in my previous email:
http://code.djangoproject.com/browser/django/trunk/django/views/generic/create_update.py#L42
Some user in IRC told me that putting your own "user"-variable into
request forces you to rebuild _all_ of the users API, but I think
renaming your is even more hackish...and of course the code does not
even check if request.user exists, so a user-variable needs to be put
there anyway.
Strictly speaking putting send_user_message() into ModelAdmin does not
help at all, because some apps might use User.message_set. LogEntry is
declared inside django.contrib.admin, this is different, but Message is
something django.contrib.auth provides. So wrapping the creation of
Message-objects inside the admin does help to fix the admin, but there
may be other parts of the code or other applications that use Message.
Perhaps instead adding User.send_message() could help, but removing the
Message-functionality certainly does need someone to do similar things
like what I've done by replacing User.message_set.
As said before: I did replace django.contrib.auth, so I know what
trouble you run into doing so. So trust me, only adding these two
methods will not be enough to _really_ support users that want to
replace django.contrib.auth.
Of course these methods help people running the admin without
django.contrib.auth, but that's all. No django.contrib.comments or other
stuff that needs User-objects (and no re-usablility of third-party-apps
without code-changes). But after all I think only unbundling the admin
could be done with some if-statements inside the admin-code without
needing the user to do anything at all. Doing so you might even skip
having LogEntry (if is_installed("django.contrib.auth"): class
LogEntry()...). As for log_action() this could be done this way:
-------8<-------------------------------------------------------
class ModelAdmin(...):
[...]
if is_installed("django.contrib.auth"):
def log_action(self, user, action, ...):
user.logentry_set.create(action, ...)
else:
def log_action(self, user, action, ...):
pass
------------------------------------------------------->8-------
(Note: is_installed() is some imaginary function I used, there might be
something in django to do this, but I think it's clear what I try to say
even if the function name is wrong)
As stated some weeks/month ago in an email about my
"auth-rewrite"-branch: I'm willing to help. Being able to replace
django.contrib.auth gives some nice advantages, even over the current
backends-system.
For example you do not need to sync your db with LDAP when using LDAP as
the backend. A User-model only containing the DN would be enough here.
Greetings, David Danier
James Bennett
> True, but having to duplicate every app in django.contrib I want to use
> (and which uses django.contrib.auth.models.User) only because the
> ForeignKey is wrong, that's bad in my opinion. And fixing the admin will
> not really help with this issue. So I think it would be nicer to really
> fix the problem in django.contrib.auth, not django.contrib.admin.
OK, so what you want is very very different from what I'm getting at here.
What I want == newforms-admin can be run without django.contrib.auth,
and people who do so understand that in doing so they are jettisoning
anything which relies on django.contrib.auth.
What you want == django.contrib.auth becomes a magical wrapper which
transparently switches out the entire auth codebase on demand so that
'from django.contrib.auth.models import User' might do practically
anything depending on the configuration.
These are so very much unlike each other that I'm not even sure why
one came up in a discussion of the other. Perhaps I'm missing
something?
David Danier
Yes and no, see below.
> What I want == newforms-admin can be run without django.contrib.auth,
> and people who do so understand that in doing so they are jettisoning
> anything which relies on django.contrib.auth.
True, but I think it would be nice being able to keep the log if users
have their own User-model.
> What you want == django.contrib.auth becomes a magical wrapper which
> transparently switches out the entire auth codebase on demand so that
> 'from django.contrib.auth.models import User' might do practically
> anything depending on the configuration.
Half true. Actually I'm trying to tell you that even if you change the
admin the way you propose users will still run into serious trouble when
trying to replace django.contrib.auth. This begins with not being able
to use all generic views and probably ends with other apps using
django.contrib.auth (django.contrib.* or third-party).
> These are so very much unlike each other that I'm not even sure why
> one came up in a discussion of the other. Perhaps I'm missing
> something?
As you try to improve the situation for users who want to replace
django.contrib.auth I think it is the right place. First because there
are some things that remain unresolved even with your proposed change,
second because I don't think fixing every application on it's own is the
right way to get the situation resolved in the long term. At least
third-party apps will remain problematic even if everything inside
django.contrib is fixed. But perhaps I'm to focused on the idea of
reusable apps like the documentation states:
"A project can contain multiple apps. An app can be in multiple projects."
(http://www.djangoproject.com/documentation/tutorial01/#creating-models)
If you are only interested in fixing the admin it might be displaced,
but please consider the things that will remain broken even with your
changes. One example still is:
Being able to override/disable send_user_message() inside the admin does
not do the job, as Message comes from django.contrib.auth, so apps
outside of the admin will probably count on having user.message_set. And
reading the code even generic views (which are outside of _any_ app!) do
rely on Message. (see previous mails)
Additionally I think putting solutions for common problems into the
admin is the wrong approach. This was done when putting
has_FOO_permission() into the admin instead of focusing on something
like the "generic-auth" branch and using the permission checks
introduced there (why should I need to add the admin-app only to have a
nice and clean permission-framework?). It looks to me like the
"unreplaceable django.contrib.auth"-problem will be fixed (or worked
around) only in the admin now, without considering adding this
functionality to the entire django-framework.
Greetings, David Danier
David Cramer
briefly on the initial thread).
For messages, why not switch these to using sessions? We have
implemented anonymous (on-demand) sessions locally, it could be very
useful to do this in general.