Hi Kalle,
Yep, I'd be happy to - and what I committed on Friday was a first pass
preparing for RunAs functionality, and I'm more than open to feedback
from anyone that may wish to contribute it. Technically the
refactoring is still in-progress and there are still things left to
do, so you probably caught something that wasn't caught by the test
cases (we should add another one ;)).
Here goes:
Constructing a Subject in an ad-hoc manner, such as what would be
required for RunAs or for any type of framework work, was previously
very complicated - it required deep knowledge about Shiro's use of the
ThreadContext as well as what permutations of things in the
ThreadContext could create.
In trying to tackle the RunAs problem, specifically as it related to
supporting Callable and Runnable instances, it became evident (yet
again) that the thread-binding mechanisms were less than ideal and
rather incestuous - they cropped up in lots of places in many points
of code and was rather unruly.
The thread-binding issue is important specifically for Callable and
Runnable since those instances regularly will execute on other
threads, and the thread-binding problem became a bigger one to solve.
It is easy to solve this problem for a web Request/Response pair
because you're guaranteed by the Servlet specification to have a
single thread which handles the request. We don't have this guarantee
with Callable/Runnable or in any other environment.
So, I looked at the ShiroFilter and realized it was solving the same
problem as the Runnable/Callable case - before execution, some stuff
needed to bound to the current thread, and then after execution, some
stuff needed to be cleaned off the thread. That's where I realized
this before/after logic could easily be abstracted away and reused in
many places and is why you now see the (still work-in-progress)
ThreadStateManager (which could be renamed). This would abstract away
all the ThreadContext usages so it wouldn't be so pervasive across the
source code and the Runnable/Callable support could use the same
mechanism.
As for the things bound to the thread context - the Request/Response
pair, an InetAddress, SecurityManager, etc - these all really were
bound to the thread for one purpose and one only - so that they would
be readily available for constructing Subject instances as necessary.
The thread-bound instances were part of Shiro's implementation details
and never really intended to be consumed by Shiro's end users
directly.
But now with the latest SecurityManager method that takes in a context
Map, those things can be passed to the security manager instead of
being lazily picked up off the thread. This is a much cleaner
programming model in terms of managing state - it works like any other
method call and does not rely on the presence of hard-to-trace
thread-bound values.
The map mechanism also allows a lot flexibility in creating ad-hoc
Subject instances - a custom SubjectFactory can be plugged in at any
time and end-users can direct exactly how Subject instances get
created. But probably a very small % of end-users would ever need to
do that since most can now use the new SubjectBinder and
WebSubjectBinder classes.
These two classes allow Shiro-end users to construct Subject instances
in an easy-to-understand way whenever they want depending on their
needs, without knowledge of how the context Map is constructed. This
could be used in many ways, but one simple example is to create a
Subject for running a daemon thread - no user request is received by
the thread, but it still needs to execute with an identity:
PrincipalCollection daemonUserPrincipals = new
SimplePrincipalCollection("daemon", "myRealm");
Subject daemonUser = new
SubjectBuilder(getSecurityManager()).setPrincipals(daemonUserPrincipals).build();
Until this mechanism, there was no easy way to create a Subject
instance without very deep knowledge of Shiro's internals. The beauty
of this solution is that the runAs support would probably use the
exact same classes.
As for the exception you're seeing, you should be using a
WebSubjectBuilder in a web environment, not the SubjectBuilder. That
will guarantee that the Request/Response pair will be available for
Subject construction.
Sorry for the long email - but at least it is documented in some form now!
Best,
Les
On Mon, Aug 24, 2009 at 1:56 AM, Kalle
Korhonen<[email protected]> wrote:
> No.. this can't be right - for example calling
> SecurityUtils.getSubject().login(authenticationToken) results in:
> java.lang.IllegalStateException: Subject context map must contain a
> javax.servlet.ServletRequest instance to support Web Subject
> construction. DefaultSecurityManager's createSubject operations create
> Subject context map from scratch, and obviously it won't have the
> required objects in the context. Les, care to clarify your refactoring
> plan and how this is supposed to work?
>
> Kalle
>
>
> On Sat, Aug 22, 2009 at 11:22 PM, Kalle
> Korhonen<[email protected]> wrote:
>> I see the internals of Shiro have been changed quite a bit in r806735.
>> ShiroFilter.bind() now does:
>> Subject subject = new WebSubjectBuilder(getSecurityManager(),
>> request, response).build();
>> WebThreadStateManager threadState = new
>> WebThreadStateManager(subject, request, response);
>> threadState.bindThreadState();
>>
>> which for Tapestry integration I'm working on results in:
>>
>> java.lang.IllegalStateException: No ServletRequest found in
>> ThreadContext. Make sure WebUtils.bind() is being called. (typically
>> called by ShiroFilter) This could also happen when running
>> integration tests that don't properly call WebUtils.bind().
>> at
>> org.apache.shiro.web.WebUtils.getRequiredServletRequest(WebUtils.java:351)
>> at
>> org.apache.shiro.web.session.ServletContainerSessionManager.doGetSession(ServletContainerSessionManager.java:69)
>> at
>> org.apache.shiro.session.mgt.AbstractSessionManager.getSession(AbstractSessionManager.java:246)
>> at
>> org.apache.shiro.session.mgt.AbstractSessionManager.checkValid(AbstractSessionManager.java:265)
>> at
>> org.apache.shiro.mgt.SessionsSecurityManager.checkValid(SessionsSecurityManager.java:294)
>> at
>> org.apache.shiro.mgt.DefaultSecurityManager.getSession(DefaultSecurityManager.java:196)
>> at
>> org.apache.shiro.mgt.DefaultSecurityManager.resolveSessionIfNecessary(DefaultSecurityManager.java:437)
>> at
>> org.apache.shiro.mgt.DefaultSecurityManager.getSubject(DefaultSecurityManager.java:403)
>> at
>> org.apache.shiro.subject.SubjectBuilder.build(SubjectBuilder.java:95)
>> at
>> org.trailsframework.security.services.SecurityConfiguration.service(SecurityConfiguration.java:87)
>>
>> I.e. WebSubject requires the request is already bound to thread
>> context, but WebThreadStateManager (that's supposed to bind it)
>> requires a subject to exist. If I call WebUtils.bind(request)
>> before instantiating a WebSubjectBuilder, everything works. Les, is it
>> expected I still need to bind the request/response separately or
>> perhaps this is a defect/refactoring still in progress?
>>
>> Kalle
>>
>
