On Sat, Feb 20, 2010 at 4:47 AM, Kalle Korhonen <[email protected]> wrote: > I think we need to do three things: > 1) Decide what is the contract for the Subject.Builder - is it allowed > to fail or should it always return a subject
The initial idea at least was that it should always return a valid subject assuming it has access to a SecurityManager. The Subject.Builder is just an abstraction so people don't have to know how to populate the Subject context Map themselves. This implies that a Subject.Builder should always have a reference to a valid SecurityManager instance - either via SecurityUtils or one that is provided explicitly (Subject.Builder(securityManager).blah().blah()...) > 2) Break the circular dependency between SecurityUtils.getSubject -> > Subject.Builder.buildSubject() -> SecurityUtils.getSecurityManager(). > I think SecurityUtils is nothing more than a convenience and > internally, we need to be careful about *not* using it. The difficulty here is that the client tier could have a reference to a SecurityManager remoting proxy - I've done this in more than a few client-tier standalone apps. In this case, the 2nd 'if block' in the SecureRemoteInvocationFactory at line 80 would utilize the Subject.Builder which would in turn use the client-tier SecurityManager proxy. Maybe if a client-side SecurityManager proxy existed, it could be injected into the Factory, and if so, use it with the Builder in the line 80 code block? And if there isn't one provided, try SecurityUtils.getSecurityManager, but if it throws an exception, ignore it assuming that the client tier doesn't have one? Here is a little back story on how/why the SecureRemoteInvocationFactory works the way it does: It primarily existed to support remote client applications that were started/launched after a server-side session had been created first (think login HTTPs page --> success --> launch/show app). Then the client tier app would have in its start-up parameters that sessionId (Flex app, Java Web Start, etc) which would be attached to the remoting payload for every remote call to ensure that the server would always know who that user was. This implied that the client tier might not use Shiro's APIs directly - Subject/SecurityManager/etc, which is a real possibility for clients based on languages not yet supported by Shiro (PHP, C#, etc) or even simpler Java applications that don't want to (or can't for reasons out of our control) utilize Shiro's API. They could just send the sessionId only. In these no-Shiro-API clients, they would perform permission checks to show buttons/views or not based on a call to say, a SubjectService, implying remote invocations to check the user's access control. > 3) Add unit tests to SecureRemoteInvocationFactory to cover all cases > for heuristics I agree - we have a SecureRemoteInvocationFactoryTest class already, but it does need to updated after we decide exactly how this should operate moving forward. My opinion is that we allow a SecurityManager instance to be injected and we can use that if it exists. If not, we try SecurityUtils.getSecurityManager (it can be the case in a single-user standalone application to make a client-side SecurityManager proxy accessible via a static reference). But if that method throws an exception (which should be a changed to a type-safe exception, like UnavailableSecurityManagerException), we just ignore it for the remoting factory and assume there isn't one. In that case, the sessionId is checked as a last resort, and if it doesn't exist, then the server invocation will be anonymous, at which point the server-side Shiro mechanisms might prevent the invocation completely. > Note that the current sample that's > committed isn't runnable at the moment (the webstart libs are not > copied to the right location and the jnlp lib entries are wrong, > etc.). Yep, that is the reason for the issue initially - it wouldn't start because it doesn't have a valid classpath. What do you think of my potential solution after #3 above? It seems fairly robust. Cheers, Les
