You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shiro.apache.org by Kalle Korhonen <ka...@gmail.com> on 2009/08/23 08:22:14 UTC

Changes in r806735 cause "No ServletRequest found in ThreadContext"

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

Re: Changes in r806735 cause "No ServletRequest found in ThreadContext"

Posted by Les Hazlewood <lh...@apache.org>.
p.s. don't forget to call threadState.clearAllThreadState() after
you're done to perform proper cleanup.

But note that this API will change a bit - that method will probably
be renamed to just clear() and I'll also probably introduce some
interfaces so you don't have to call the implementations directly.

On Mon, Aug 24, 2009 at 2:11 PM, Les Hazlewood<lh...@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<ka...@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<lh...@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<ka...@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<ka...@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
>>>>>
>>>>
>>>
>>
>

Re: Changes in r806735 cause "No ServletRequest found in ThreadContext"

Posted by Kalle Korhonen <ka...@gmail.com>.
On Tue, Aug 25, 2009 at 6:38 AM, Les Hazlewood<lh...@apache.org> wrote:
> 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.

Based on that and without knowing all the details, I'd scrap it
immediately. You can always resurrect it later, but where ever
possible it makes sense to limit the user facing surface area of the
API as much as possible since it's more difficult to remove than add.

I see that Subject.login() now works again, but RememberMe is broken,
because the new DefaultSecurityManager.createSubject(Map context)
doesn't call getRememberedIdentity at any point. Compare to the old
createSubject() that does:
        if (session != null) {
            if (session.getAttribute(SessionSubjectBinder.PRINCIPALS_SESSION_KEY)
== null) {
                remembered = getRememberedIdentity();
            }
        }

Should I open an issue or are you still working on it to complete this
refactoring?

Kalle


> On Tue, Aug 25, 2009 at 12:51 AM, Kalle
> Korhonen<ka...@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<ka...@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<lh...@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<ka...@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<lh...@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<ka...@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<lh...@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<ka...@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<ka...@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
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

Re: Changes in r806735 cause "No ServletRequest found in ThreadContext"

Posted by Les Hazlewood <lh...@apache.org>.
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<ka...@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<ka...@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<lh...@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<ka...@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<lh...@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<ka...@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<lh...@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<ka...@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<ka...@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
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

Re: Changes in r806735 cause "No ServletRequest found in ThreadContext"

Posted by Kalle Korhonen <ka...@gmail.com>.
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<ka...@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<lh...@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<ka...@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<lh...@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<ka...@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<lh...@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<ka...@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<ka...@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
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

Re: Changes in r806735 cause "No ServletRequest found in ThreadContext"

Posted by Kalle Korhonen <ka...@gmail.com>.
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<lh...@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<ka...@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<lh...@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<ka...@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<lh...@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<ka...@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<ka...@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
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

Re: Changes in r806735 cause "No ServletRequest found in ThreadContext"

Posted by Les Hazlewood <lh...@apache.org>.
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<ka...@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<lh...@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<ka...@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<lh...@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<ka...@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<ka...@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
>>>>>>
>>>>>
>>>>
>>>
>>
>

Re: Changes in r806735 cause "No ServletRequest found in ThreadContext"

Posted by Kalle Korhonen <ka...@gmail.com>.
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<lh...@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<ka...@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<lh...@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<ka...@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<ka...@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
>>>>>
>>>>
>>>
>>
>

Re: Changes in r806735 cause "No ServletRequest found in ThreadContext"

Posted by Les Hazlewood <lh...@apache.org>.
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<ka...@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<lh...@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<ka...@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<ka...@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
>>>>
>>>
>>
>

Re: Changes in r806735 cause "No ServletRequest found in ThreadContext"

Posted by Kalle Korhonen <ka...@gmail.com>.
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<lh...@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<ka...@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<ka...@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
>>>
>>
>

Re: Changes in r806735 cause "No ServletRequest found in ThreadContext"

Posted by Les Hazlewood <lh...@apache.org>.
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<ka...@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<ka...@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
>>
>

Re: Changes in r806735 cause "No ServletRequest found in ThreadContext"

Posted by Les Hazlewood <lh...@apache.org>.
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<ka...@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<ka...@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
>>
>

Re: Changes in r806735 cause "No ServletRequest found in ThreadContext"

Posted by Kalle Korhonen <ka...@gmail.com>.
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<ka...@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
>