Refactoring UserManagerImpl et al. was Re: Dumb question about LoginModulePlugins

[Ian Boston <ie...@tfd.co.uk>]

I have had a further look at this, and making the UserManager pluggable is possible, however it will require refactoring the Group storage mechanism to replace the weak reference membership property (in JR2 version) with String a multivalue string property (probably principals) that can be interpreted either as node UUID references or probably principals. Thinking about it a set of child nodes with references, weak or otherwise might work better.

IMHO this would unbind the whole implementation from JCR node storage and make it possible to plugin other implementation.

In reality, how feasible is this going to be for Sling/Jackrabbit ? Or is it going to have to remain a custom UserManager Implementation ?

Ian

On 27 Feb 2010, at 12:24, Ian Boston wrote:

> I have been checking some of the code base here and discovered there are a number of places where there are class cast bindings to implementations rather than the API.
> 
> eg
> 
> In AuthorizableImpl
> 
>    public boolean isMember(Authorizable authorizable) throws RepositoryException {
>        if (authorizable == null || !(authorizable instanceof AuthorizableImpl)) {
>            return false;
>        } else {
>            String thisID = getID();
>            AuthorizableImpl impl = (AuthorizableImpl) authorizable;
>            for (Iterator it = impl.memberOf(); it.hasNext();) {
>                if (thisID.equals(((GroupImpl) it.next()).getID())) {
>                    return true;
>                }
>            }
>            return false;
>        }
>    }
> 
> Although this doesn't look to bad, and it wont cause an exception if the authorizable parameter is not an AuthorizableImpl it does mean that group membership is strictly limited to users and groups defined within the same UserManager.
> 
> ie 
> All users and groups have to be represented as JCR nodes in the Jackrabbit UserManager.
> Its not possible to implement external user managers that produce User and Group objects without re-implementing the Jackrabbit UserManager classes.
> 
> The Principal Manager Plugin  might still be possible, but I am not certain.
> 
> Ian
> 
> 
> On 26 Feb 2010, at 15:25, Ian Boston wrote:
> 
>> Felix,
>> Finally got round to doing something about this.
>> 
>> I have done a patch at [1] that implements the diagram at the bottom of [2], the Green boxes. At this stage its only in outline, with no test coverage.
>> 
>> I also cleaned up the Service trackers and made then a little bit thread safer.
>> 
>> Is this what you were thinking of ? I think it covers my needs, I will probably implement in our custom server bundle.
>> 
>> The patch is against the [3]
>> 
>> Ian
>> 
>> 1 http://codereview.appspot.com/224073
>> 2 http://confluence.sakaiproject.org/display/KERNDOC/KERN-504+Groups+Pull+Integration
>> 3 http://svn.apache.org/repos/asf/jackrabbit/sandbox/jackrabbit2-bundle
>> 
>> On 30 Jan 2010, at 10:49, Felix Meschberger wrote:
>> 
>>> Hi,
>>> 
>>> Yes, the current LoginModulePlugin interface we have is conceptually
>>> incomplete. For my prototype of migrating that stuff to Jackrabbit, I
>>> have added a LoginModulePluginFactory service interface, which creates
>>> LoginModule instances on demand, which are then used for a single login
>>> process only.
>>> 
>>> See also http://wiki.apache.org/jackrabbit/JackrabbitOsgi
>>> 
>>> Regards
>>> Felix
>>> 
>>> On 29.01.2010 18:50, Ian Boston wrote:
>>>> Hi,
>>>> 
>>>> IIRC, LoginModules are bound to a session, hence one is created with the session to service its needs, and then destroyed when the session is destroyed. On creation the doInit(CallbackHandler callbackHandler, Session session, Map options) with the session that the LoginModule is bound to.
>>>> 
>>>> With the Sling PluggableLoginModule this happens, but then there is an Activator.getLoginModules() (a static), which goes to the activator and grabs the list of LoginModulePlugins from the service tracker. AFAICT, these are singleton service implementations.
>>>> 
>>>> Next doInit() is called on each module. 
>>>> 
>>>> later setPrincipal(Set ) is called on each module.
>>>> 
>>>> IIUC that means that all LoginModulePlugin implementations must bind their state to the thread making the method call, since they are a service and not created new with each session. They must also be thread safe, and we are assuming (probably safely) that only a single thread will ever use the session in question. AFAICT, there is no other way of sharing state on calls like setPrincipal(Set).
>>>> 
>>>> Did I read the code correctly ?
>>>> Was this the intention or were LoginModulePlugins intended to be created with the session and bound to the session ?
>>>> 
>>>> Ian
>> 
>