Changes between Version 2 and Version 3 of ContribAuthImprovements


Ignore:
Timestamp:
Mar 22, 2012, 6:59:44 PM (12 years ago)
Author:
Russell Keith-Magee
Comment:

Filled in some more blanks, and clarifies some of the options with examples

Legend:

Unmodified
Added
Removed
Modified

  • ContribAuthImprovements

    v2 v3  
    99 * There's no ability to add additional fields to the base user model.
    1010
    11 The last point requires some elaboration. The overwhelming stated use case for adding additional fields is to avoid the need for a UserProfile-style 1-1 model on aesthetic or performance grounds. The Django core team has historically counselled users away from this technique.
    12 
    13 However, there is also a use case for adding non-email, non-username identification marks to the base User object (e.g., an authentication identifier from another login system). This requires additional fields, and needing to perform a join to simply validate a user is wasteful.
     11The last point requires some elaboration. The overwhelming stated use case for adding additional fields is to avoid the need for a !UserProfile-style 1-1 model on aesthetic or performance grounds. The Django core team has historically counselled users away from this technique.
     12
     13However, there is also a use case for adding non-email, non-username identification marks to the base User object e.g., an authentication identifier from another login system. This requires additional fields, and needing to perform a join in order to perform the core function of a user object is wasteful.
    1414
    1515These use cases have driven a long-lived request (#3011) to "fix" auth.User, either by removing the base limitations of the model, or making the User model user-configurable in some way.
    … …  
    3232
    3333 * Introduce a new `USE_NEW_USER_SETTINGS` setting. This would be set as False by default in global_settings.py, and True by default in the project template. This means existing projects get a value of False, but can opt-in to True at their convenience; new projects get a value of True.
    34  * Modify the existing User model to use this setting to determine the length/uniqueness
    35  * Introduce a "MigrationWarning" that will be raised if USE_NEW_USER_SETTINGS is False. Assuming this is introduced in 1.5, Django 1.5 users would get the warning if they have USE_NEW_USER_SETTINGS set to False. Django 1.6 would raise an error if USE_NEW_USER_SETTINGS is False.
     34 * Modify the existing User model to use this setting to determine the length/uniqueness of email and username fields
     35 * Introduce a "MigrationWarning" that will be raised if `USE_NEW_USER_SETTINGS` is False. Assuming this is introduced in 1.5, Django 1.5 users would get the warning if they have `USE_NEW_USER_SETTINGS` set to False. Django 1.6 would raise an error if `USE_NEW_USER_SETTINGS` is False. Django 1.7 would remove all references to the setting.
     36
     37Optionally:
     38
     39 * We could introduce 2 settings -- one for username length, and one for email length, to separate the two issues (and allow for people who want to introduce the email length fix, but not the username length fix.
    3640
    3741=== Advantages ===
    … …  
    4549 * Introduces a setting that immediately becomes deprecated (since it won't be needed once the migration cycle is complete)
    4650 * Doesn't address the problem with any other usage of EmailField having a max_length of 75.
    47  * Introduces a circular dependency between settings and models. When settings are loaded, INSTALLED_APPS is inspected, and each models file is loaded. If a models file contains a reference to settings, hilarity can ensue. This isn't a problem *most* of the time, but it can lead to some interesting side effects.
     51 * Introduces a circular dependency between settings and models. When settings are loaded, `INSTALLED_APPS` is inspected, and each models file is loaded. If a models file contains a reference to settings, hilarity can ensue. This isn't a problem *most* of the time, but it can lead to some interesting side effects.
    4852
    4953== Solution 1a: Superminimal with forced migration ==
    … …  
    6266 * Poor failure modes. If a users fail to read and act on the instructions in the release notes, their projects won't fail until the first user enters an email that is longer than 75 characters, or a username longer than 30 characters, at which point DatabaseErrors will be raised.
    6367 
    64 == Solution 2: AUTH_USER_MODEL setting ==
     68== Solution 2: `AUTH_USER_MODEL` setting ==
    6569
    6670Allow users to specify a User model via a setting. This is essentially the original #3011 proposal.
    … …  
    6872=== Implementation ===
    6973
    70 Introduce a AUTH_USER_MODEL setting; instead of assuming that auth.User is the user model, the auth app dynamically swaps in the model described in AUTH_USER_MODEL, and calls it User.
     74Introduce an `AUTH_USER_MODEL` setting; instead of assuming that auth.User is the user model, the model found at contrib.auth.models.User is determined at runtime by reading settings. This can be facilitated through the use of a 'get_user_model()' or similar helper function.
     75
     76There are several examples of implementations implementing this approach:
     77
     78 * https://github.com/aino/django-primate Uses a monkeypatch to override Django's code at the moment, but this patch wouldn't be required if it were merged to trunk.
     79 * https://github.com/claymation/django/tree/pluggable-auth-apps This puts the confuguration at the app level (i.e., the setting is `AUTH_APP`, not `AUTH_USER_MODEL`), but uses the same principle
    7180
    7281=== Advantages ===
    … …  
    7887Optionally:
    7988
    80  * Introduce an AbstractUser base class that implements the core of the User contract, and encourage people developing User models to extend this base class.
    81  * Modify auth.User to extend the new AbstractUser. Care must be taken to ensure that the database expression of the new User model is identical to the old User model, to ensure backwards compatibility.
     89 * Introduce an !AbstractUser base class that implements the core of the User contract, and encourage people developing User models to extend this base class.
     90 * Modify auth.User to extend the new !AbstractUser. Care must be taken to ensure that the database expression of the new User model is identical to the old User model, to ensure backwards compatibility.
    8291 * Introduce other concrete User models implementing common login patterns (e.g., login by email)
    8392 
    84 In order to support contrib.admin, there will be a need to include some permissions API in the User model -- this is required by the admin app, but doesn't necessarily have to be part of the AbstractUser. Care should be taken to ensure that the AbstractUser really does represent the minimal requirements for user authentication, and that authorisation concerns are kept separate.
    85 
    86 All three points here are optional. We can introduce an AbstractUser without changing the base User. We also don't have to introduce other concrete models -- we could leave this up to the community at large to develop an ecosystem of User models.
     93In order to support contrib.admin, there will be a need to include some permissions API in the User model -- this is required by the admin app, but doesn't necessarily have to be part of the !AbstractUser. Care should be taken to ensure that the !AbstractUser really does represent the minimal requirements for user authentication, and that authorisation concerns are kept separate.
     94
     95All three points here are optional. We can introduce an !AbstractUser without changing the base User. We also don't have to introduce other concrete models -- we could leave this up to the community at large to develop an ecosystem of User models.
    8796
    8897=== Problems ===
    8998
    9099 * Has the same settings-models circular dependency problem as Solution 1.
    91  * Doesn't address the EmailField length problem for existing users. We could address this by having a User model (reflecting current field lengths) and a new SimpleUser (that reflects better defaults); then use global_settings and project template settings to define which User is the default for new vs existing projects.
     100 * Doesn't address the !EmailField length problem for existing users. We could address this by having a User model (reflecting current field lengths) and a new SimpleUser (that reflects better defaults); then use global_settings and project template settings to define which User is the default for new vs existing projects.
    92101 * Doesn't solve the analogous problem for any other project. E.g., contrib.comments already has pluggable Comments models, and has invented a bespoke solution. Other projects will have similar needs; this solution doesn't address the duplication of code.
    93102 * Has unpredictable failure modes if a third-party app assumes that User has a certain attribute or property which the project-provided User model doesn't support (or supports in a way different to the core auth.User model).
    94103
    95 === Solution 3: Leverage App Refactor ===
     104== Solution 3: Leverage App Refactor ==
    96105
    97106Use Arthur Koziel's App Refactor patch from GSoC 2010 as a way to define a configurable auth app.
    … …  
    101110 * Land the App Refactor patch. This introduces a number of benefits -- reliable hooks for app startup, configurable app labels, predictable module loading, amongst others -- but the one that matters for the purposes of auth.User is that it allows Apps to be treated as items that need to be configured as a runtime activity. In this case, we need to be able to specify, at a project level, which model is your "User" model in the auth app.
    102111
    103  * Introduce the concept of a LazyForeignKey. LazyForeignKey is a normal foreign key, with all the usual foreign key behaviors; the only difference is that the model it links to isn't specified in the model -- it's a configuration item drawn from an application configuration. So, ForeignKey('auth.User') creates a foreign key to django.contrib.auth.User; LazyForeignKey('auth.User') asks the auth app for the model that is being used as the 'User' model, and creates a foreign key to that. This should just be a matter of slotting into the existing model reference resolution code, which is something that the app refactor cleans up.
    104 
    105  * Add a Meta option to models -- `plugable` -- which controls whether the model can be replaced at runtime. If it can be, then the model may not be synchronised (so we don't get empty auth_user tables).
     112 * Introduce the concept of a !LazyForeignKey. !LazyForeignKey is a normal foreign key, with all the usual foreign key behaviors; the only difference is that the model it links to isn't specified in the model -- it's a configuration item drawn from an application configuration. So, !ForeignKey('auth.User') creates a foreign key to django.contrib.auth.User; !LazyForeignKey('auth.User') asks the auth app for the model that is being used as the 'User' model, and creates a foreign key to that. This can be done by slotting into the existing model reference resolution code, which is something that the app refactor cleans up.
     113
     114 * Add a Meta option to models -- `pluggable` -- which controls whether the model can be replaced at runtime. If it can be, then the model may not be synchronised (so we don't get empty auth_user tables).
    106115
    107116Optionally:
    108117
    109  * Introduce an AbstractUser, and other concrete User models, same as for Solution 2
     118 * Introduce an !AbstractUser, and other concrete User models, same as for Solution 2
    110119
    111120=== Advantages ===
    112121
    113122 * Doesn't have the circular dependency between settings and User
    114  * Solves the generic problem. contrib.comments could be retrofitted to use this approach, and any other application could do the same.
    115 
    116 === Problems ===
    117 
    118  * No transparent update path -- requires that third party apps be updated to be "pluggable auth compatible". This means app authors need to convert all ForeignKey(User) into LazyForeignKey('auth.User'), and modify any usage of forms/fields etc. This could be considered a benefit, however; Migrating User models is a nontrivial step, and it should probably involve some opt-in engineering.
    119 
    120  * Doesn't address the immediate problem for EmailField. We could do the same User/SimpleUser conversion here; with the added benefit that we are also introducing apprefactor, so we can use the distinction between an "unconfigured" auth app and a "Django 1.5 AppRefactor Configured" auth app as the point for identifying whether User or SimpleUser is in use.
    121 
    122 == Solution 3a: Transparent LazyForeignKeys ==
    123 
    124 As for Solution 3, but don't include an explicit LazyForeignKey class. Instead, use the "pluggable" marker in the Meta class; If you define a ForeignKey to a 'pluggable' model, you're indicating that this foreign key reference might be changed later on.
     123 * Solves the generic problem, not a specific auth problem. contrib.comments could be retrofitted to use this approach, and any other application could do the same.
     124
     125=== Problems ===
     126
     127 * No transparent update path -- requires that third party apps be updated to be "pluggable auth compatible". This means app authors need to convert all ForeignKey(User) into LazyForeignKey('auth.User'), and modify any usage of forms etc. This could be considered a benefit, however; Migrating User models is a nontrivial step, and it should probably involve some opt-in engineering.
     128
     129 * Doesn't address the immediate problem for EmailField. We could do the same User/!SimpleUser conversion here; with the added benefit that we are also introducing App Refactor, so we can use the distinction between an "unconfigured" auth app and a "Django 1.5 App Refactor Configured" auth app as the point for identifying whether User or SimpleUser is in use.
     130
     131== Solution 3a: Transparent !LazyForeignKeys ==
     132
     133As for Solution 3, but don't include an explicit !LazyForeignKey class. Instead, use the "pluggable" marker in the Meta class; If you define a !ForeignKey to a 'pluggable' model, you're indicating that this foreign key reference might be changed later on.
    125134
    126135=== Advantages ===
    127136
    128  * As for Solution 3, but provides a transparent migration path for apps -- no need to manually change ForeignKey(User). However, depending on your perspective, this might not actually be an advantage at all, because it removes the opt-in.
     137 * As for Solution 3, but provides a transparent migration path for apps -- no need to manually change ForeignKey(User). However, depending on your perspective, this might not actually be an advantage at all, because it removes the opt-in migration path.
    129138
    130139=== Problems ===
    … …  
    140149 * it doesn't yet handle permissions, so it can't be used for contrib.admin
    141150 * It exhibits the settings-models dependency problem
    142  * It introduces a new feature -- the ability to have *multiple* user models -- that isn't an obvious improvement. Additional discussion is required before being adopted.
     151 * It introduces a new feature -- the ability to have *multiple* user models -- that isn't an obviously required improvement. Additional discussion is required before being adopted.
    143152
    144153=== Advantages ===
    … …  
    152161== Universal problems ==
    153162
    154 Regardless of the final
     163Regardless of the final solution that is chosen, the decision to move to a pluggable User model will introduce some challenges.
    155164
    156165=== The User Contract ===
    … …  
    160169Consensus seems to be that the 'minimal contract' for User needs to be little more than that required basic identification -- you need to be able to authenticate using arbitrary credentials, and be able to service a request to print "Hello <user>"
    161170
    162 This 'minimal'
     171Beyond this base contract, we're in true duck-typing territory. If you have an app that expects to find an 'is_admin' attribute, and the provided User model doesn't have one, then your app won't work with that User model (and the failure mode won't be predictable).
     172
     173This means that the onus will be on the developer using pluggable User model to test that their User model will work as expected. This can be helped by app developers:
     174
     175 * Clearly documenting the app's User model requirements.
     176 * Including test suites in their app that explicitly test their User model requirements.
    163177
    164178=== Separation of authentication from authorisation ===
    165179
    166 contrib.admin requires
     180contrib.admin requires a permissions API on User. However, this API isn't an obvious thing to require for *all* User objects. There has been very little discussion about the possibility of an abstract API for permissions.
     181
     182The best approach here may be implement contrib.admin's permissions API as a mixin, separate to the AbstractUser base class. This would allow someone who doesn't want admin's permission model to avoid them, while making it easy to include all Django's permission requirements if the developer wants to use contrib.admin.
    167183
    168184=== Forms ===
    169185
    170 Any form interaction with User model needs to be able to adapt as the User model changes.
    171 
    172  * Does ModelForm need to automatically resolve model references for models marked as pluggable?
    173  * Should Forms be configurable items for apps?
     186If an app creates a form (or ModelForm) on User, the exact contents of that form will be unpredictable. It will be extremely easy to define a form in an app that has clean methods or widget overrides that reference fields that don't exist on the User model in use.
     187
     188It may be necessary to prevent ModelForm from being used on User (or any other pluggable model in the App Refactor case), and encourage app developers to make any Forms that they have configurable. This is analogous to Django's existing configurability for LoginForm et al.
    174189
    175190=== Inheritance ===
    … …  
    181196== Parallel concerns ==
    182197
    183 Most of these solutions don't fully address the limitations with EmailField. As a separate concern, we could address this problem in the same way that Solution 1 proposes to fix the User model, but focussed on the EmailField's max_length argument specifically:
     198None of these fully address the limitations with EmailField -- that the default max_length of 75 is too short to hold all email addresses. As a separate concern, we could address this problem in the same way that Solution 1 proposes to fix the User model, but focussed on the EmailField's max_length argument specifically:
    184199
    185200 * Introduce a new `ALLOW_RFC_COMPLIANT_EMAIL_ADDRESSES` setting. This would be set as False by default in global_settings.py, and True by default in the project template. This means existing projects get a value of False, but can opt-in to True at their convenience; new projects get a value of True.
    186  * Modify the EmailField definition to use this setting to determine the length/uniqueness
    187  * Introduce a "MigrationWarning" that will be raised if `ALLOW_RFC_COMPLIANT_EMAIL_ADDRESSES` is False (or max_length isn't manually specified). Assuming this is introduced in 1.5, Django 1.5 users would get the warning if they have ALLOW_RFC_COMPLIANT_EMAIL_ADDRESSES set to False. Django 1.6 would raise an error if ALLOW_RFC_COMPLIANT_EMAIL_ADDRESSES is False.
    188 
    189 This would address *all* EmailFields, not just the one in auth.User. This fix could be used in conjunction with any of the solutions proposed here; in fact, it would make some of them simpler (since there wouldn't be a need for a SimpleUser to migrate to an improved email field length).
     201 * Modify the EmailField definition to use this setting to determine the max_length
     202 * Introduce a "MigrationWarning" that will be raised if `ALLOW_RFC_COMPLIANT_EMAIL_ADDRESSES` is False (or max_length isn't manually specified). Assuming this is introduced in 1.5, Django 1.5 users would get the warning if they have `ALLOW_RFC_COMPLIANT_EMAIL_ADDRESSES` set to False. Django 1.6 would raise an error if `ALLOW_RFC_COMPLIANT_EMAIL_ADDRESSES` is False. Django 1.7 would remove all reference to the setting.
     203
     204This would address the problem for ''all'' EmailFields, not just the one in auth.User. This fix could be used in conjunction with any of the solutions proposed here; in fact, it would make some of them simpler (since there wouldn't be a need for a SimpleUser to migrate to an improved email field length).
    190205
    191206== Recommendations ==
Back to Top