Spring Security OAuth
  1. Spring Security OAuth
  2. SECOAUTH-346

scope attribute not working correctly on <oauth:resource/> in 1.0.0RC3

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Complete
    • Affects Version/s: 1.0.0
    • Fix Version/s: None
    • Component/s: OAuth 2
    • Labels:
      None

      Description

      I am working on updating from 1.0.0M6d to 1.0.0RC3. In my client I have the following oauth:resource defined:

       
      <oauth:resource id="service" type="authorization_code" client-id="client" client-secret="secret"
      		access-token-uri="${accessTokenUri}" user-authorization-uri="${userAuthorizationUri}"  scope="read,write"/>
      

      On the resource I have the following:

       
      <http pattern="/service/**" create-session="never" entry-point-ref="oauthAuthenticationEntryPoint"
      		access-decision-manager-ref="accessDecisionManager" xmlns="http://www.springframework.org/schema/security">
      		<anonymous enabled="false" />
      		<intercept-url pattern="/service" access="ROLE_USER,SCOPE_READ" />
      		<intercept-url pattern="/service/**" access="ROLE_USER,SCOPE_READ" method="GET"/>
      		<custom-filter ref="resourceServerFilter" before="PRE_AUTH_FILTER" />
      		<access-denied-handler ref="oauthAccessDeniedHandler" />
      	</http>
      

      and for the AccessDecisionManager:

      <bean id="accessDecisionManager" class="org.springframework.security.access.vote.UnanimousBased" xmlns="http://www.springframework.org/schema/beans">
      		<constructor-arg>
      			<list>
      				<bean class="org.springframework.security.oauth2.provider.vote.ScopeVoter" />
      				<bean class="org.springframework.security.access.vote.RoleVoter" />
      				<bean class="org.springframework.security.access.vote.AuthenticatedVoter" />
      			</list>
      		</constructor-arg>
      	</bean>
      

      When I replace ScopeVoter with my own implementation, I can see that the AuthorizationRequest associated with the Authentication passed to the vote() method on ScopeVoter has only a single scope associated with it: 'read write'. It looks like somewhere the scopes are getting parsed incorrectly in the request. If I remove the scope attribute from oauth:resource, it seems to fall back to the scopes registered with the client and the request succeeds.

      I did some further investigation to narrow down the issue. I am using the JdbcTokenStore. If I load the access token, the scope field is ['read write'], instead of ['read', 'write']. If load the AuthorizationRequest (readAuthentication) for the access token, the scope is ['read', 'write'] which is correct. Based on this the scope seems to be getting mangled either when the access token is initially created or when it is read on the resource server.

        Activity

        Hide
        Dave Syer added a comment -

        I had a go and I can't reproduce this. Can you provide steps to reproduce in the sparklr/tonr sample? Or a test case?

        Show
        Dave Syer added a comment - I had a go and I can't reproduce this. Can you provide steps to reproduce in the sparklr/tonr sample? Or a test case?
        Hide
        Kelly Davis added a comment -

        I think I might need some help figuring out where the issue might be for reproducing it. I modified the resource in tonr as follows:

        <oauth:resource id="sparklr" type="authorization_code" client-id="tonr" client-secret="secret"
          access-token-uri="${accessTokenUri}" user-authorization-uri="${userAuthorizationUri}" scope="read,write" />
        

        This doesn't seem to trigger the error. I tried replacing the tokenstore and clientdetailsservice with the jdbc implementations, which is what I am using, and that doesn't trigger it either. Any ideas on other things that I can try that might tickle this? My project and tonr/sparklr are sufficiently different that trying to track down what is different is non-trivial.

        Show
        Kelly Davis added a comment - I think I might need some help figuring out where the issue might be for reproducing it. I modified the resource in tonr as follows: <oauth:resource id= "sparklr" type= "authorization_code" client-id= "tonr" client-secret= "secret" access-token-uri= "${accessTokenUri}" user-authorization-uri= "${userAuthorizationUri}" scope= "read,write" /> This doesn't seem to trigger the error. I tried replacing the tokenstore and clientdetailsservice with the jdbc implementations, which is what I am using, and that doesn't trigger it either. Any ideas on other things that I can try that might tickle this? My project and tonr/sparklr are sufficiently different that trying to track down what is different is non-trivial.
        Hide
        Kelly Davis added a comment -

        I've done some more work on this trying to narrow down where it is occurring. It looks like setScope is getting called on DefaultAuthorizationRequest, changing it from ['read','write'] to ['read write']. I inserted a printstacktrace and got the following:

        at org.springframework.security.oauth2.provider.DefaultAuthorizationRequest.setScope(DefaultAuthorizationRequest.java:142)
                at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
                at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
                at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
                at java.lang.reflect.Method.invoke(Method.java:597)
                at org.springframework.beans.BeanWrapperImpl.setPropertyValue(BeanWrapperImpl.java:1154)
                at org.springframework.beans.BeanWrapperImpl.setPropertyValue(BeanWrapperImpl.java:924)
                at org.springframework.beans.AbstractPropertyAccessor.setPropertyValues(AbstractPropertyAccessor.java:76)
                at org.springframework.validation.DataBinder.applyPropertyValues(DataBinder.java:692)
                at org.springframework.validation.DataBinder.doBind(DataBinder.java:588)
                at org.springframework.web.bind.WebDataBinder.doBind(WebDataBinder.java:191)
                at org.springframework.web.bind.ServletRequestDataBinder.bind(ServletRequestDataBinder.java:112)
                at org.springframework.web.servlet.mvc.method.annotation.ServletModelAttributeMethodProcessor.bindRequestParameters(ServletModelAttributeMethodProcessor.java:153)
                at org.springframework.web.method.annotation.ModelAttributeMethodProcessor.resolveArgument(ModelAttributeMethodProcessor.java:107)
                at org.springframework.web.method.support.HandlerMethodArgumentResolverComposite.resolveArgument(HandlerMethodArgumentResolverComposite.java:75)
                at org.springframework.web.method.support.InvocableHandlerMethod.getMethodArgumentValues(InvocableHandlerMethod.java:156)
                at org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest(InvocableHandlerMethod.java:117)
                at org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:96)
                at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.invokeHandlerMethod(RequestMappingHandlerAdapter.java:617)
                at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.handleInternal(RequestMappingHandlerAdapter.java:578)
                at org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter.handle(AbstractHandlerMethodAdapter.java:80)
                at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:923)
                at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:852)
                at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:882)
                at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:778)
        

        This is called when /oauth/authorize is called and the allow / deny scopes are displayed to the user. I have verified this does not occur in the sparklr / tonr example.

        Show
        Kelly Davis added a comment - I've done some more work on this trying to narrow down where it is occurring. It looks like setScope is getting called on DefaultAuthorizationRequest, changing it from ['read','write'] to ['read write'] . I inserted a printstacktrace and got the following: at org.springframework.security.oauth2.provider.DefaultAuthorizationRequest.setScope(DefaultAuthorizationRequest.java:142) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at java.lang.reflect.Method.invoke(Method.java:597) at org.springframework.beans.BeanWrapperImpl.setPropertyValue(BeanWrapperImpl.java:1154) at org.springframework.beans.BeanWrapperImpl.setPropertyValue(BeanWrapperImpl.java:924) at org.springframework.beans.AbstractPropertyAccessor.setPropertyValues(AbstractPropertyAccessor.java:76) at org.springframework.validation.DataBinder.applyPropertyValues(DataBinder.java:692) at org.springframework.validation.DataBinder.doBind(DataBinder.java:588) at org.springframework.web.bind.WebDataBinder.doBind(WebDataBinder.java:191) at org.springframework.web.bind.ServletRequestDataBinder.bind(ServletRequestDataBinder.java:112) at org.springframework.web.servlet.mvc.method.annotation.ServletModelAttributeMethodProcessor.bindRequestParameters(ServletModelAttributeMethodProcessor.java:153) at org.springframework.web.method.annotation.ModelAttributeMethodProcessor.resolveArgument(ModelAttributeMethodProcessor.java:107) at org.springframework.web.method.support.HandlerMethodArgumentResolverComposite.resolveArgument(HandlerMethodArgumentResolverComposite.java:75) at org.springframework.web.method.support.InvocableHandlerMethod.getMethodArgumentValues(InvocableHandlerMethod.java:156) at org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest(InvocableHandlerMethod.java:117) at org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:96) at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.invokeHandlerMethod(RequestMappingHandlerAdapter.java:617) at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.handleInternal(RequestMappingHandlerAdapter.java:578) at org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter.handle(AbstractHandlerMethodAdapter.java:80) at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:923) at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:852) at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:882) at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:778) This is called when /oauth/authorize is called and the allow / deny scopes are displayed to the user. I have verified this does not occur in the sparklr / tonr example.
        Hide
        Dave Syer added a comment -

        OK, so you must have a custom handler for /oauth/confirm_access, and it looks like it is trying to bind a DefaultAuthorizationRequest to the HttpRequest. The framework can't stop you from doing that I suppose, but it's a mistake, so we can try and ensure that mistake is hard to make, and maybe fix the DefaultAuthorizationRequest so it can cope with what's happening to it.

        You can fix your app by changing the signature of your custom handler (see the one in sparklr2 for an example that works).

        Show
        Dave Syer added a comment - OK, so you must have a custom handler for /oauth/confirm_access, and it looks like it is trying to bind a DefaultAuthorizationRequest to the HttpRequest. The framework can't stop you from doing that I suppose, but it's a mistake, so we can try and ensure that mistake is hard to make, and maybe fix the DefaultAuthorizationRequest so it can cope with what's happening to it. You can fix your app by changing the signature of your custom handler (see the one in sparklr2 for an example that works).
        Hide
        Dave Syer added a comment -

        I think all you might need to do is remove a @ModelAttribute annotation. Can you share the custom handler code?

        Show
        Dave Syer added a comment - I think all you might need to do is remove a @ModelAttribute annotation. Can you share the custom handler code?
        Hide
        Dave Syer added a comment -

        Fixed by adding special case to DefaultAuthorizationRequest.setScope() which extracts the actual scope from a badly constructed one. It's debatable whether we should just throw an exception with a message that tells the caller what they did wrong. I'm open to that if anyone has a strong opinion.

        Show
        Dave Syer added a comment - Fixed by adding special case to DefaultAuthorizationRequest.setScope() which extracts the actual scope from a badly constructed one. It's debatable whether we should just throw an exception with a message that tells the caller what they did wrong. I'm open to that if anyone has a strong opinion.
        Hide
        Dave Syer added a comment -

        I found another source of this error (maybe it applies to your case or maybe not), if your access confirmation page has a form on it to submit back the approval, it should specify the action explicitly, otherwise you will be submitting the authorization request parameters back on approval. The way the AuthorizationEnpoint was implemented (I've changed it) that would attempt to rebind them to the request (unneeded and unhelpful) and set an invalid scope because of the way the binder works.

        Show
        Dave Syer added a comment - I found another source of this error (maybe it applies to your case or maybe not), if your access confirmation page has a form on it to submit back the approval, it should specify the action explicitly, otherwise you will be submitting the authorization request parameters back on approval. The way the AuthorizationEnpoint was implemented (I've changed it) that would attempt to rebind them to the request (unneeded and unhelpful) and set an invalid scope because of the way the binder works.
        Hide
        Kelly Davis added a comment -

        Based on your comments (and some sleep) I was able to track down the cause. For my AccessConfirmationController, I was using the version from 1.0.0.M6d (https://github.com/SpringSource/spring-security-oauth/blob/1.0.0.M6d/samples/oauth2/sparklr/src/main/java/org/springframework/security/oauth/examples/sparklr/mvc/AccessConfirmationController.java). I am guessing by not removing the AuthorizationRequest from the model, it updates it on form submit?

        Show
        Kelly Davis added a comment - Based on your comments (and some sleep) I was able to track down the cause. For my AccessConfirmationController, I was using the version from 1.0.0.M6d ( https://github.com/SpringSource/spring-security-oauth/blob/1.0.0.M6d/samples/oauth2/sparklr/src/main/java/org/springframework/security/oauth/examples/sparklr/mvc/AccessConfirmationController.java ). I am guessing by not removing the AuthorizationRequest from the model, it updates it on form submit?
        Hide
        Dave Syer added a comment -

        Correct. I modified the AuthorizationEndpoint to do the same thing (much safer).

        Show
        Dave Syer added a comment - Correct. I modified the AuthorizationEndpoint to do the same thing (much safer).

          People

          • Assignee:
            Dave Syer
            Reporter:
            Kelly Davis
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: