Architectural Review of early Jan 14 commits

[Les Hazlewood <lh...@apache.org>]

Hi team,

I was working on some additions last night to 1) clean up our
DefaultSecurityManager (and by inheritance, our DefaultWebSecurityManager)
to be more pluggable and 2) begin to prepare for 'assumed identity'/'runAs'
support.  Please do an SVN update and review.

I added things like a SubjectFactory and SubjectBinder that we've discussed
in previous threads so an end-user does not have to subclass the
SecurityManager directly - you can instead inject either component if your
creation logic or binding logic are custom to your application.  I
essentially just moved the existing SecurityManager logic into default
implementations for both new interfaces, so everything still works the same
as before (logic is nearly identical).

What I'm not so sure on is should the methods in these two interfaces be
exactly as they are defined, or should they be altered.  Currently the
interfaces reflect exactly what is needed by the current SecurityManager
implementations.  Some points:

1.  It is also not entirely clear to me how the implementations of these two
interfaces should interact with the RemeberMeManager, if at all.  Currently
both SubjectFactory and SubjectBinder implementations don't 'know' about
RememberMe at all, and instead the SecurityManager implementation
coordinates the interaction between these 3 components (RembmerMe,
SubjectFactory, and SubjectBinder).

But should it be that way?  I mean, wouldn't a SubjectFactory _want_ to have
a RememberMeManager as a fallback in case it can't acquire the principals
out of the session?  This is the way the DefaultWebSecurityManager works -
it needs to the RememberMeManager, but only for Subject creation.  If that's
the case, shouldn't the web-aware SubjectFactory have a reference to it? So
that work doesn't have to exist in the SecurityManager?  OO delegation and
all that...

Similarly for binding the Subject to the application.  When binding for the
first time, should the SubjectBinder be responsible for adding information
to RememberMe as well as binding it to the thread and/or session?  Currently
the SecurityManager adds to RememberMeManager directly before calling
bind(subject);

2.  Currently the SecurityManager is needed by the default SubjectFactory
implementation because it generates DelegatingSubject instances, which needs
to reference a SecurityManager.  But the SecurityManager has an internal
reference to the SubjectFactory instance - a little circular dependency
going on.  Not as nice as I would like, but it feels like a necessary thing
given how DelegatingSubject works.

Similarly the web SessionFactory needs an instance to the WebSessionManager
so it can acquire a Session based on an incoming request and then the
Subject is constructed based on the information stored in the Session.

3.  Should the logic that the SecurityManager currently implements that
coordinates between the 3 components (RememberMeManager, SubjectFactory,
SubjectBinder) be bundled up and implemented by a SubjectManager or
SubjectAccessor implementation?  And that implementation does the current
logic?

This would make the SecurityManager implementation a pure wrapper object
that delegates _all_ logic to wrapped components.  This is very flexible at
the cost of a layer of indirection (loose coupling, high cohesion, etc).

It seems like this might be a good idea if we still enable passthrough
customization methods on the SecurityManager implementation like we do
everywhere else:

defaultSecurityManager.setSubjectFactory( SubjectFactory subjectFactory) {
    this.subjectAccessor.setSubjectFactory( subjectFactory);
}

---

Any thoughts?

Les