Scope handling

[mozg]

Hi Ryan,

I tried to work with scope, but no success so far.
By default, scope is not sent from auth server to client, even if it's changed. I managed to fix it by creating a custom redirect strategy (see https://jira.springsource.org/browse/SECOAUTH-29, setting redirection strategy in <oauth:verification-code> does not work, but setting it in <oauthrovider> works as if I did it in <oauth:verification-code>).
Now, the client do not send the scope back to the auth server (though it is possible by specification). I wonder if I missed something and there is a way of customizing this.
Anyway, my solution looks very unnatural and the correct scope handling is not provided by the framework. I believe the framework must provide the handling which works out of the box and is much easier to customize.

Unfortunately, OAuth2 draft 10 is very unclear about how an empty scope should be treated in each case. The absent scope in an initial authorization request seems obvious, but does empty scope returned by the auth server mean that it did not approve any scope elements or just that scope is not changed? The same can be applied to the auth token request.
What do you think?

[stoicflame]

To be honest, I haven't tested the scoping facilities very thoroughly. I agree that the framework needs to provide full scope handling capabilities out-of-the-box. What things do you suggest need to be changed? Can you provide some tests that expose the problems?

[mozg]

Since specs are not clear, IMHO, the only approach which will work for every configuration is to send the scope always. I suggest to make it default behavior.

I implemented the authorization page where a user can select scope elements he wants to authorize. In this case some extension able to handle request and shrink the scope would be useful (something like UserApprovalHandler).
Here is my implementation:

public class ScopeAwareApprovalFilter extends BasicUserApprovalFilter {
    public static final String SCOPE_KEY = ScopeAwareApprovalFilter.class.getCanonicalName() + "#SCOPE";

    @Override
    public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
            throws IOException, ServletException {
        if (requiresProcessing(request)) {
            final HttpServletRequest req = (HttpServletRequest) request;
            final HttpServletResponse resp = (HttpServletResponse) response;
            VerificationCodeAuthenticationToken authToken = getAuthenticationCache().getAuthentication(req, resp);
            final Set<String> approvedScope = new HashSet<String>(asList(request.getParameterValues("scope")));
            if (!authToken.getScope().containsAll(approvedScope)) {
                throw new InvalidScopeException("Invalid scope: " + approvedScope);
            }
            getAuthenticationCache().updateAuthentication(
                    new VerificationCodeAuthenticationToken(authToken.getClientId(), approvedScope, authToken.getState(),
                            authToken.getRequestedRedirect()),
                    req, resp);
            request.setAttribute(SCOPE_KEY, approvedScope);
        }
        super.doFilter(request, response, chain);
    }
}

I'd prefer to move the scope validation and token update code outside of this extension.

VerificationCodeFilter.successfulAuthentication() does not append the scope to redirection uri, so I needed to create a custom redirection strategy. I suppose that it should be fixed, so I won't need this strategy.

Also, the client code does not send the scope back to the auth server upon token acquisition. I could not figure out how to fix it without any changes to the library and could not find a place to fix. But I suggest that it should be extensible. The extension must be able to shrink the scope (probably, just based on the scope granted). Though I can't find any use-cases for shrinking scope at this stage except for paranoiac security settings, e.g. since scope "a" was not granted, the client considers scope "b" unneeded, so it do not request a token for it just in case if it gets stolen.

[stoicflame]

These changes seem pretty reasonable. Could you attach a patch (or patches) implementing your suggestions to a JIRA issue?

[vtsao]

What was the JIRA ticket for this, was this fixed?

[stoicflame]

I don't think an issue has been opened yet.