On Fri, Mar 26, 2010 at 3:22 AM, d3f3nd3r <d3f3n...@bytegeist.org> wrote: > > On Mar 25, 4:19 pm, Russell Keith-Magee <freakboy3...@gmail.com> > wrote: >> >> Firstly, Backwards compatibility. contrib.auth is a major component in >> many, if not most existing Django installations. If you're proposing >> *any* change, the existing code needs to be 100% backwards compatible. >> >> For example, I'm not entirely certain I follow why the changes to >> login() et al are necessary. You're proposing to change login(), etc, >> but you're a little light on details on how backwards compatibility >> will be maintained. >> > > If you say, the auth framework should only work with site based login > procedures (displaying the login form, calling a login() method and > redirecting to another page all inside the Django app) there is no > need the refractor the base get_user() method. But when the login > process is handled completely outside the Django app (for example > using the Google Accounts Python API; the api forces you to redirect > the user to a Google login page, Google checks username and password > and redirects the user back to the app) I see no way of calling a > login() method inside the Django app and setting > request.session[SESSION_KEY], but a custom authentication middleware > fixes the problem. > But I think changing the stuff in contrib.auth.__init__ won't work > without breaking a lot of custom user backends.
And that's my concern. Breaking existing code *is not an option*. That said, I'm still not clear why this can't be accommodated without breaking backwards incompatibility. The current auth middleware uses a call to get_user() to do the authentication, but I don't see why that call can't be abstracted in some meaningful (and, more importantly, backwards compatible way). >> Also, if you're proposing that the auth.User model be refactored, >> you're going to need to demonstrate why existing auth.User installs >> aren't going to be affected. >> >> Secondly, as far as I can make out, your proposal doesn't address the >> biggest problem I can see -- Foreign Keys/ManyToMany to User. It isn't >> much good proposing a way to use a different User model if you don't >> provide a way for other applications that currently ship with a >> ForeignKey to contrib.auth.User to change that into a ForeignKey on >> your new User model. The approach used by contrib.Comments with >> COMMENTS_APP may be worth investigating here. >> >> A related problem here: what happens if a developer changes their User >> model during the development process? How do I migrate my existing >> contrib.auth users to use mycustom.auth users? Is this even possible? >> Is there any way to protect against this class of error? >> > > My idea is to add an extra abstraction layer for the User class in > form of a BaseUser class. The BaseUser class consists of : > > * the whole permission stuff (user_permissions, get_group_permissions, > get_all_permissions,....), > > * the the authentication stuff (is_anonymous and is_authenticated, > is_superuser, is_staff, last_login, date_joined, groups) > > * a single ident field > > Other specific user classes extends the BaseUser class via Multi-table > inheritance. > > The "old" User class will contain the same fields/methods as before, > but it will inherit some fields from BaseUser and add the missing ones > (username, password, set_password...), so it won't break existing > auth.User installs. > > To add a custom user model you have to extend BaseUser (if you don't > have database based authentication, but Facebook or Twitter) or the > "new" User class (if you want to add custom information to the > standard, databased user model, like extra fields or modify existing > fields via overriding). > I suggest linking to BaseUser instead of User if possible (to become > independent from the Django database User class). Everything that has > to do with permissions, authentication information or if you want to > link an entry to an user (for example a blog page to a specific user) > should link to BaseUser. So is BaseUser going to be a concrete class or not? If it's a concrete class, then you've just violated backwards compatibility, because every single auth.User instance will need to be migrated to be a class inheriting from BaseUser. If BaseUser is an abstract class, you have a different problem - you can't create a foreign key to an abstract class. >> Another related problem - even if you make User pluggable, there will >> still be a need for some common interface for user-like objects. What >> should be on this interface? >> > > For a pluggable User interface I suggest splitting into Permission and > Authentication stuff. It the interface will look something like this: > > class BaseUserPermission: > - user_permissions > - groups > - is_staff > - is_superuser > - get_group_permissions() > - get_all_permissions() > - has_perm() > - has_module_perms() > - ….. > > class BaseUserAuthentication: > - last_login > - date_joined > - is_anonymous() > - is_authenticated() > - ….. > > class BaseUser(BaseUserAuthentication, BaseUserPermissions, > models.Model): > - ident > - ….. Ok, sure. To pick some holes: - why are "staff' and "superuser" forced attributes of the base class? What if I have different a permission model that I want to use? - What if I don't want to track the date joined and last login? What if I want to track different login data, like the server/location of last login? >> Lastly, what is the impact on other areas of Django code? Backwards >> compatibility means that all existing code must continue to work >> as-is, but if we're shipping a new capability, all the built-in Django >> code should be updated to use that new capability. When we rolled out >> the new messages model, we had to modify admin and auth to use the new >> messages. What aspects of Django currently use the User model and will >> need to be updated? >> > > Examples where BaseUser can replace the “old” User be used inside > Django: > > * contrib.auth.* > * contrib.admin: > ** .models.LogEntry > ** .models.LogEntryManager.log_action > * contrib.comment.model.Comment > * contrib.auth.models.Message <sarcasm>Wow... that's some penetrating insight you've given me there. </sarcasm> Care to give me some details that I couldn't generate myself using grep? >> So - if I haven't scared you off yet, I'm certainly interested in >> hearing more details. This is one of the really big project >> flexibility issues in Django, and I for one am certainly interested in >> seeing it fixed. However, the devil is in the details. >> > > I hope I answered some of your questions and please answer if you want/ > need more detailed information The problem is that you haven't yet convinced me that you even understand the scope of the problem. You've been very light on detail when it matters, and more importantly, several key components of proposal appear to be predicated on breaking backwards incompatibility. I just can't reinforce strongly enough exactly how much this is *not* an option. You're not going to get selected for the SoC unless you can convince us that you have a firm grasp on the problem that needs to be solved, and that you have at least a basic understanding of how to achieve an acceptable solution. So far, you haven't convinced me that you do, on either count. Yours, Russ Magee %-) -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
