shiro-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Les Hazlewood <lhazlew...@apache.org>
Subject Re: Changes in r806735 cause "No ServletRequest found in ThreadContext"
Date Tue, 25 Aug 2009 13:38:43 GMT
Yep you're right - it still is not fixed just yet.  I didn't have the
time late yesterday to make that fix, but hopefully I can focus on
that first thing today.

On the SecurityManager comment - it is an interesting question.  I
think it might be a good idea to pass in the Subject instance to the
login method in addition to the authentication token - that way the
SecurityManager could execute logic based on that particular subject.

For example, during login, if a RememberMeManager is available, it
will be invoked to remember that subject.  Currently that logic is
based on the AuthenticationInfo only, but in the future, it could be
valuable to have a handle to the Subject in case other things need to
be done for that Subject - things we can't foresee now, but certainly
might appear in the future.  Such is the benefit of coarse-grained
method calls - we could easily react to change without changing the
public-facing API.  The logout method already takes this approach, and
I'm thinking it should be the same for login as well.  Thoughts?

Your thoughts also make me wonder about the need for the
org.apache.shiro.mgt.SubjectBinder interface.  It was a new addition
after 0.9, so it can be scrapped if we no longer need it since 1.0
hasn't been released yet.  It was added to allow custom binding and
retrieval of Subject instances after they had been created - to allow
someone to save that Subject state somewhere if necessary so it can be
retrieved later on.  But its method signatures imply ThreadLocal-based
behavior, which isn't very clean.

On Tue, Aug 25, 2009 at 12:51 AM, Kalle
Korhonen<kalle.o.korhonen@gmail.com> wrote:
> Ok, I see that after your latest changes (in r807476) the login itself
> in DefaultSecurityManager succeeds, but after that login does:
>        Subject subject = createSubject(token, info);
>        bind(subject);
>        return subject;
>
> which will cause the same:
> Caused by: java.lang.IllegalStateException: Subject context map must
> contain a javax.servlet.ServletRequest instance to support Web Subject
> construction.
>        at org.apache.shiro.web.mgt.DefaultWebSubjectFactory.getServletRequest(DefaultWebSubjectFactory.java:40)
>        at org.apache.shiro.web.mgt.DefaultWebSubjectFactory.createSubject(DefaultWebSubjectFactory.java:70)
>        at org.apache.shiro.mgt.DefaultSecurityManager.getSubject(DefaultSecurityManager.java:403)
>        at org.apache.shiro.mgt.DefaultSecurityManager.createSubject(DefaultSecurityManager.java:274)
>        at org.apache.shiro.mgt.DefaultSecurityManager.login(DefaultSecurityManager.java:370)
>        at org.apache.shiro.subject.DelegatingSubject.login(DelegatingSubject.java:245)
>
> Should the SecurityManager return Subject at all anymore if the
> creation of it is managed elsewhere in the builders?
>
> Kalle
>
>
> On Mon, Aug 24, 2009 at 12:07 PM, Kalle
> Korhonen<kalle.o.korhonen@gmail.com> wrote:
>> I'm ahead of you - I already added WebUtils.bind.. calls in my
>> implementation corresponding to ShiroFilter (that's the first issue I
>> reported and worked around). The login issue still stands.
>>
>> Kalle
>>
>>
>> On Mon, Aug 24, 2009 at 11:51 AM, Les Hazlewood<lhazlewood@apache.org> wrote:
>>> Ah yes, you're right - sorry about that.  I'll see if I can get that
>>> working now.  It is definitely a high priority to have this working
>>> asap.
>>>
>>> On Mon, Aug 24, 2009 at 2:27 PM, Kalle
>>> Korhonen<kalle.o.korhonen@gmail.com> wrote:
>>>> Naturally I already did what the ShiroFilter does to get any Subject
>>>> checks going. However Subject.login() doesn't work because of the
>>>> reasons mentioned in my previous post (see the stacktrace - it enters
>>>> DefaultSecurityManager.createSubject where the context is (re-)created
>>>> and it won't have the request/response objects in it).
>>>>
>>>> Kalle
>>>>
>>>>
>>>> On Mon, Aug 24, 2009 at 11:11 AM, Les Hazlewood<lhazlewood@apache.org>
wrote:
>>>>> Aha - you've encountered something that wasn't fixed this weekend yet
>>>>> - the SecurityManager.login method signature would need to change to
>>>>> take in a Subject argument in addition to the AuthenticationToken.
>>>>> Naturally this would be hidden from Subject API end-users, but
>>>>> represents the final change for this type of state-management:
>>>>>
>>>>> Currently the SecurityManager implementations assume that an existing
>>>>> Subject is bound to the currently executing thread and can rely on the
>>>>> ThreadContext.  It would then acquire the existing Subject from the
>>>>> thread, and call login on that instance.  In your example, you use the
>>>>> subject immediately after building it - without binding it to the
>>>>> thread first.
>>>>>
>>>>> A SecurityManager API change that accepts the Subject as a parameter
>>>>> would eliminate these types of problems since it wouldn't have to
>>>>> acquire the Subject from some other location anymore.  There are other
>>>>> benefits too to keeping that method signature coarse-grained with the
>>>>> Subject, but I digress...
>>>>>
>>>>> In the meantime, you should be able to do what the ShiroFilter does
>>>>> and ensure that a Subject is available immediately on the thread after
>>>>> construction.  Then you could use SecurityUtils everywhere in your
>>>>> code after that:
>>>>>
>>>>> Subject subject = new WebSubjectBuilder(securityManager, request,
>>>>> response).build();
>>>>>
>>>>> //this API is actively changing and will undergo another change today
>>>>> or tomorrow to not accept a request/response since the Subject will
>>>>> already have those.
>>>>> ThreadStateManager threadState = new WebThreadStateManager(subject,
>>>>> request, response);
>>>>> threadState.bindThreadState();
>>>>>
>>>>> subject.login(authcToken);
>>>>>
>>>>> If that doesn't work, feel free to reply and I'll get it straightened
out.
>>>>>
>>>>> - Les
>>>>>
>>>>> On Mon, Aug 24, 2009 at 1:28 PM, Kalle
>>>>> Korhonen<kalle.o.korhonen@gmail.com> wrote:
>>>>>> Les, thanks I really appreciate you taking time to talk through the
>>>>>> changes - mostly it verifies what I was being able to deduce from
the
>>>>>> dffs. Yea, sounds like more tests are needed. In the meantime, can
you
>>>>>> comment on where this is going wrong?
>>>>>>
>>>>>> I'm trying to call Subject.login. Even if I change it to using the
new
>>>>>> builder from SecurityUtils:
>>>>>>                        // SecurityUtils.getSubject().login(authenticationToken);
>>>>>>                        (new WebSubjectBuilder(securityManager,
request,
>>>>>> response).build()).login(authenticationToken);
>>>>>>
>>>>>> And I get a stack trace:
>>>>>> Caused by: java.lang.IllegalStateException: Subject context map must
>>>>>> contain a javax.servlet.ServletRequest instance to support Web Subject
>>>>>> construction.
>>>>>>        at org.apache.shiro.web.mgt.DefaultWebSubjectFactory.getServletRequest(DefaultWebSubjectFactory.java:40)
>>>>>>        at org.apache.shiro.web.mgt.DefaultWebSubjectFactory.createSubject(DefaultWebSubjectFactory.java:70)
>>>>>>        at org.apache.shiro.mgt.DefaultSecurityManager.getSubject(DefaultSecurityManager.java:405)
>>>>>>        at org.apache.shiro.mgt.DefaultSecurityManager.createSubject(DefaultSecurityManager.java:275)
>>>>>>        at org.apache.shiro.mgt.DefaultSecurityManager.login(DefaultSecurityManager.java:372)
>>>>>>        at org.apache.shiro.subject.DelegatingSubject.login(DelegatingSubject.java:245)
>>>>>>        at xxx.web.pages.SignIn.onValidateForm(SignIn.java:88)
>>>>>>
>>>>>> If you look at DefaultSecurityManager.createSubject() operations,
they
>>>>>> just create the context from scratch (see my previous email). So
where
>>>>>> is the problem - should it use the builder or should the
>>>>>> DefaultWebSecurityManager override these operations or something
else?
>>>>>>
>>>>>> Kalle
>>>>>>
>>>>>>
>>>>>> On Mon, Aug 24, 2009 at 7:32 AM, Les Hazlewood<lhazlewood@apache.org>
wrote:
>>>>>>> Sounds like we need some test cases for this - it appears that
the
>>>>>>> exception you're seeing is if there is no Subject accessible
to the
>>>>>>> thread.  That shouldn't be the case since the ShiroFilter should
have
>>>>>>> bound the subject to the thread via the WebSubjectBuilder and
>>>>>>> WebThreadStateManager usage.
>>>>>>>
>>>>>>> On Mon, Aug 24, 2009 at 1:56 AM, Kalle
>>>>>>> Korhonen<kalle.o.korhonen@gmail.com> 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<kalle.o.korhonen@gmail.com> 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
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

Mime
View raw message