[Zope-PAS] [Zope-CMF] PAS: authenticateCredentials: check lowercase too?
wichert at wiggy.net
Fri Dec 28 16:57:23 UTC 2012
On Dec 28, 2012, at 16:51 , Maurits van Rees <m.van.rees at zestsoftware.nl> wrote:
> Op 28-12-12 10:56, Wichert Akkerman schreef:
>> On Dec 27, 2012, at 20:52 , Tres Seaver <tseaver at palladion.com> wrote:
>>> -----BEGIN PGP SIGNED MESSAGE-----
>>> Hash: SHA1
>>> (PAS stuff is OT for the CMF list. Please follow up on zope-pas at zope.org
>>> or the equivalent Gmane newsgroup).
> Thanks, will do.
>>> I would prefer to have the case insensitivity be a configurable option of
>>> the plugin (in which case it would always lowercase the login when the
>>> user was created or updates, as well as before comparing).
>> A bit more specifically you need to define a canonical spelling of a login name and a way to convert
> > a login name to the canonical version which was applied at every point a login name is passed in via
> > the PAS API. lower() would then be a possible transformation to get such a canonical spelling.
> > It might make sense to do that in PAS so you don't have to duplicate that in all PAS plugins.
> I have started a branch for this:
> 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.
> 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.
More information about the Zope-PAS