Op 04-01-13 22:03, Maurits van Rees schreef:
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?
Any reaction to this?
While working on adding some tests, I noticed that it may be better to
not introduce this new IUpdateLoginNamePlugin plugin interface, but
instead add the extra methods (updateUser and updateAllLoginNames) to
the IUserEnumerationPlugin. That would mean changing an existing
interface, which is why I initially did not do this, but I have already
added the implementation to the ZODBUserManager class. The updateUser
method has been there for a long time, it just was not part of any
interface yet.
In updateLoginName(user_id, login_name) PAS first calls
self.getUserById(user_id) in my implementation and does not even call
updateUser(user_id, login_name) on an IUpdateLoginName plugin if the
user is not found. So any plugin that implements IUpdateLoginName is
probably also going to implement IUserEnumeration.
Would you agree it is better to merge those two plugin interfaces? Added
bonus would be that this voids the need for migration code, because no
extra plugin interface needs to be registered and activated.
Again, my branch is here:
http://svn.zope.org/repos/main/Products.PluggableAuthService/branches/maurits-login-transform
--
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
