Op 28-12-12 17:57, Wichert Akkerman schreef:
On Dec 28, 2012, at 16:51 , Maurits van Rees <m.van.r...@zestsoftware.nl> wrote:

I have started a branch for this:
http://svn.zope.org/repos/main/Products.PluggableAuthService/branches/maurits-login-transform/

The only commit message I did so far should be pretty clear:

================================================
Add possibility to transform the login name.

The BasePlugin now has a property 'login_transform' and a method 
'applyTransform'.
In proper places, plugins can call 'login_name = 
self.applyTransform(login_name)'.
When 'login_transform' is 'lower', this method will return 
'self.lower(login_name)'.
Care is taken to not fail when the method does not exist.  The original 
login_name
is then returned.
A use case is to transform all login names to lowercase if you want to use the 
email
address as login name in Plone.
The ZODBUserManager and CookieAuthHelper plugins use this now,
though it is not strictly needed for the last one.
More may follow.
================================================

It has extra tests and they pass. I will have a look at which other plugins may 
need this.

Wichert, you seem to suggest adding this in the main acl_users object. With my 
current implementation it is probably best to keep this an option in each 
plugin, with the base code (property and method) available in the BasePlugin 
class.
There is a risk here in that multiple plugin types use the login name: All of 
IExtractionPlugin, IAuthenticationPlugin, ICredentialsUpdatePlugin, 
IUserAdderPlugin, IUserFactoryPlugin and IUserEnumerationPlugin all use the 
login name. If you only do the transform in one of them you are likely to get 
strange results: for example a user may show up multiple times in a user 
listings of he has logged in with different casing for his email address and 
multiple property sheets were created, or the authentication cookie might not 
match the login name user by the user, etc.  For that reason I am reasonably 
sure this needs to be done either in PAS itself at every API point where a 
login name is accepted.

I have basically reverted my earlier changes on that branch and have now done this in PAS itself instead of the plugins. PAS now has a login_transform property, a helper method _get_login_transform_method and an applyTransform(login_name) method. PAS calls that method in all spots that I could discover that need it.

One tricky thing is that the ZODBUserManager plugin has an updateUser(user_id, login_name) method that needs to call applyTransform itself, because there is no code in PAS where ZODBUserManager.updateUser is called.

That makes me think we could use a new plugin type for updateUser. It is currently not in any of the interfaces. So on my branch I have added an IUpdateLoginNamePlugin plugin interface:

class IUpdateLoginNamePlugin( Interface ):
    """ Update the login name of a user.
    """

    def updateUser( user_id, login_name ):
        """ Update the login name of the user with id user_id.
        """

    def updateAllLoginNames(quit_on_first_error=True):
        """Update login names of all users to their canonical value.

        This should be done after changing the login_transform
        property of PAS.

        You can set quit_on_first_error to False to report all errors
        before quitting with an error.  This can be useful if you want
        to know how many problems there are, if any.
        """

In a standard Plone context, the source_users object would be the only plugin that has this plugin interface activated.

I have added three related methods in IPluggableAuthService:

    def updateLoginName(user_id, login_name):
        """Update login name of user.
        """

    def updateOwnLoginName(login_name):
        """Update own login name of authenticated user.
        """

    def updateAllLoginNames(quit_on_first_error=True):
        """Update login names of all users to their canonical value.

        This should be done after changing the login_transform
        property of PAS.

        You can set quit_on_first_error to False to report all errors
        before quitting with an error.  This can be useful if you want
        to know how many problems there are, if any.
        """

Those methods call the related methods of any activated IUpdateLoginNamePlugin plugins. Note that I have overridden the _setPropValue method in PAS to make sure updateAllLoginNames is called when the login_transform method is changed and is not empty.

I must add more tests, but other than that: does this look good?

If I create a fresh Plone 4.2 site with these changes all seems to work fine. But for current installations some migration is needed, because the new plugin type is not known yet, so you run into tracebacks like this:

Traceback (innermost last):
  Module ZPublisher.Publish, line 126, in publish
  Module ZPublisher.mapply, line 77, in mapply
  Module ZPublisher.Publish, line 46, in call_object
  Module OFS.PropertyManager, line 305, in manage_editProperties
  Module OFS.PropertyManager, line 210, in _updateProperty
Module Products.PluggableAuthService.PluggableAuthService, line 1113, in _setPropValue Module Products.PluggableAuthService.PluggableAuthService, line 1321, in updateAllLoginNames
  Module Products.PluginRegistry.PluginRegistry, line 106, in listPlugins
  Module Products.PluginRegistry.PluginRegistry, line 385, in _getPlugins
KeyError: <InterfaceClass Products.PluggableAuthService.interfaces.plugins.IUpdateLoginNamePlugin>

If you never change the login_transform property you should never encounter this error, but then you cannot use the new feature.

In Plone I can probably create a GenericSetup upgrade step in PlonePAS and/or plone.app.upgrade. But is there a way to write migration code for PAS without Plone? Is that needed? There are two GS profiles in PAS, which are used when adding a 'Configured PAS' in the ZMI. On Plone 4.2 this fails for several reasons. Is there any way to automatically update an existing manually added PAS outside of Plone?

This would be the xml that you could add in the Export/Import tab of the plugins page, where the line with plugin id source_users is optional, but when I try it in Plone 4.2 it has no effect:

 <plugin-type id="IUpdateLoginNamePlugin" title="update_login_name"
description="Login name updater plugins allow to set a new login name for a user." interface="Products.PluggableAuthService.interfaces.plugins.IUpdateLoginNamePlugin" >
  <plugin id="source_users" />
 </plugin-type>

Anyway, here is the branch url again, please have a look:

http://svn.zope.org/repos/main/Products.PluggableAuthService/branches/maurits-login-transform/


For example, if you have an LDAP plugin, you probably do not want to transform 
the logins there, though I am not sure if LDAP/AD servers are normally case 
sensitive.  In that case it would also be best to not transform the login in 
the extractCredentials plugin, otherwise someone with an upper or mixed case 
login name in LDAP would not be able to login.  For the ZODBUserManager it does 
not really matter if the passed credentials have already been transformed or 
not.
And perhaps you might want to transform them by doing a case insensitive search 
in LDAP/AD to find the canonical spelling for the login name and use that. 
Which makes you wonder if it makes sense to define a new plugin type to handle 
this.

Come to think of it, if you have an LDAP/AD setup that expects login names in mixed or upper case and a different plugin for which you want it all lowercase, you should probably simply not do that in one Plone/Zope site. So never mind this argument from me for the original implementation.

--
Maurits van Rees: http://maurits.vanrees.org/
Zest Software: http://zestsoftware.nl

_______________________________________________
Zope-PAS mailing list
Zope-PAS@zope.org
https://mail.zope.org/mailman/listinfo/zope-pas

Reply via email to