-
Building on Wichert's recent good work on trunk, I refactored membrane trunk such that the only Archetypes imports are in the at sub-package, the tests or in the examples sub-package. I also removed any code that imports the examples package by default. So in theory, *using* membrane no longer requires Archetypes though I haven't tested this. While doing this work I noticed that something in the at sub-package and I'd like to solicit input. The Products.membrane.plugins.usermanager.MembraneUserManager.authenticateCredentials method looks up an IMembraneUserAuth adapter to delegate to: https://dev.plone.org/collective/browser/Products.membrane/trunk/Products/membrane/plugins/usermanager.py#L84 The IMembraneUserAuth implementation in Products.membrane.at.authentication.Authentication.authenticateCredentials method in turn looks up an adapter to Products.membrane.at.interfaces.IUserAuthentication: https://dev.plone.org/collective/browser/Products.membrane/trunk/Products/membrane/at/authentication.py#L33 This seems like one layer of indirection too many. Is there a reason for this indirection? Should we remove/unify these? It also seems like the authentication, roles, userdeleter, userchanger modules in the at package could be moved out of the at package since they have no AT dependencies. Thoughts? Ross
- Thread Outline:
-
Ross Patterson wrote: > Building on Wichert's recent good work on trunk, I refactored membrane > trunk such that the only Archetypes imports are in the at sub-package, > the tests or in the examples sub-package. I also removed any code that > imports the examples package by default. So in theory, *using* membrane > no longer requires Archetypes though I haven't tested this. > > While doing this work I noticed that something in the at sub-package and > I'd like to solicit input. > > The > Products.membrane.plugins.usermanager.MembraneUserManager.authenticateCredentials > method looks up an IMembraneUserAuth adapter to delegate to: > > https://dev.plone.org/collective/browser/Products.membrane/trunk/Products/membrane/plugins/usermanager.py#L84 > > The IMembraneUserAuth implementation in > Products.membrane.at.authentication.Authentication.authenticateCredentials > method in turn looks up an adapter to > Products.membrane.at.interfaces.IUserAuthentication: > > https://dev.plone.org/collective/browser/Products.membrane/trunk/Products/membrane/at/authentication.py#L33 > > This seems like one layer of indirection too many. Is there a reason > for this indirection? Should we remove/unify these? indeed there is. the original implementation only had the first layer, but optilude wanted to be able to use content objects for authentication (and other membership-related functionality) w/o the content objects directly containing any membership-specific code: https://dev.plone.org/collective/changeset/26675/membrane/trunk/factories/authentication.py or, for the larger changeset: https://dev.plone.org/collective/changeset/26675 personally, i've never had a need to be so rigorous in keeping the member-related functionality out of my content objects, so i don't feel strongly about keeping or losing this, but i have no idea who may be using this functionality. there's one other thing i noticed from browsing through the code in your links above, though... i r87732 (https://dev.plone.org/collective/changeset/87732/Products.membrane/trunk/Products/membrane/tools/membrane.py) it looks like wichert changed the name of the membrane tool's "getUserAuthProvider" method to "getUserObject". while i understand the motivation there, the new name makes me a bit uneasy. it is an explicit goal of membrane to NOT restrict people to a single content object representing a user, and it is possible to have several different content objects all contributing to the user object that PAS creates. 'getUserObject' obscures this, IMO, which is why the method was called getUserAuthProvider, since that describes exactly what is being returned w/o making any claims that the returned object represents more than that. not a huge deal, but i think it's important enough to mention. -r
-
On 7/13/09 8:21 PM, Rob Miller wrote: > there's one other thing i noticed from browsing through the code in your > links above, though... i r87732 > (https://dev.plone.org/collective/changeset/87732/Products.membrane/trunk/Products/membrane/tools/membrane.py) > it looks like wichert changed the name of the membrane tool's > "getUserAuthProvider" method to "getUserObject". while i understand the > motivation there, the new name makes me a bit uneasy. it is an explicit > goal of membrane to NOT restrict people to a single content object > representing a user, and it is possible to have several different > content objects all contributing to the user object that PAS creates. > 'getUserObject' obscures this, IMO, which is why the method was called > getUserAuthProvider, since that describes exactly what is being returned > w/o making any claims that the returned object represents more than that. > > not a huge deal, but i think it's important enough to mention. My renaming was on purpose: I wanted to be able to have user objects that do not support authentication. This can be useful when you use other methods than workflow to allow users to authenticate, for example by using a marker interface that controls adaptability to the authentication interface. For that reason I refactored things so you always start with a very minimal user object which exposes the very minimum required to work with PAS: a user id and a login name. All other possible facets of a user (properties, authentication, etc.) are split out into separate adapters. That allows for maximum flexibility in building your users. Wichert.
-