Make OAuth2AuthorizationConsent customizable

[fwollsch]

Expected Behavior
When sending additional parameters from a consent-form, these additional parameters should by accessable by User-Code.
The OAuth2AuthorizationConsent should be customizable by a user-definable component before being passed to the OAuth2AuthorizationService.

Current Behavior
When sending additional parameters from a consent-form, these additional parameters are present inside the OAuth2AuthorizationCodeRequestAuthenticationToken.
The OAuth2AuthorizationConsent is created only using the authorized scopes.

Context
In my OAuth2-Server, a user can create multiple entities.
On the consent-page, the user can review all requested scopes and select which of its entities the client should be given access to.

My consent-page HTML looks like this:

<form method="post" th:action="@{/oauth2/authorize}">
    <input type="hidden" name="client_id" th:value="${clientId}" />
    <input type="hidden" name="state" th:value="${state}" />

    <div th:each="entity : ${entities}">
        <p>
            <input type="checkbox" th:name="${'entity:' + entity.id}" th:text="${entity.displayName}" checked />
        </p>
    </div>

    <hr />

    <div th:each="scope : ${scopes}">
        <input type="hidden" name="scope" th:value="${scope}" />
        <p th:text="${scope}"></p>
    </div>

    <button type="submit">Submit</button>
    <!--<button type="reset">Cancel</button>-->
</form>

I need some way to know which entities have been selected by the user on the consent-page.
In my case, all selected entity-ids are present in the OAuth2AuthorizationCodeRequestAuthenticationToken.getAdditionalParameters(), but these are never passed to any User-Code.

Ideally, I'd like to customize the OAuth2AuthorizationConsent before it's being passed to the OAuth2AuthorizationService with access to the additional parameters.

As a workaround, I currently use a custom OAuth2AuthorizationCodeRequestAuthenticationProvider:

    private static final ThreadLocal<Map<String, Object>> ADDITIONAL_PARAMETERS = new ThreadLocal<>();

    public CustomOAuth2AuthorizationCodeRequestAuthenticationProvider(RegisteredClientRepository registeredClientRepository, OAuth2AuthorizationService authorizationService, OAuth2AuthorizationConsentService authorizationConsentService) {
        super(registeredClientRepository, authorizationService, authorizationConsentService);
    }

    @Override
    public Authentication authenticate(Authentication authentication) throws AuthenticationException {
        final OAuth2AuthorizationCodeRequestAuthenticationToken authorizationCodeRequestAuthentication = (OAuth2AuthorizationCodeRequestAuthenticationToken) authentication;
        final Map<String, Object> additionalParameters = authorizationCodeRequestAuthentication.getAdditionalParameters();

        ADDITIONAL_PARAMETERS.set(additionalParameters);
        try {
            return super.authenticate(authorizationCodeRequestAuthentication);
        } finally {
            ADDITIONAL_PARAMETERS.remove();
        }
    }

    public static <T> T getAdditionalParameter(String key) {
        final Map<String, Object> additionalParameters = ADDITIONAL_PARAMETERS.get();
        if (additionalParameters == null) {
            throw new IllegalStateException();
        }

        return (T) additionalParameters.get(key);
    }

Side Nodes
The additionalParameters are currently created by passing all Request-Parameters that are not explicitly known (such as scope, client_id and state), but only the first value is actually added to the additionalParameters.

This requires my consent-page to send each selected entity using a different name (like in the example HTML above).
Ideally, I would like to use one name (for example "entity") and send the selected entities with their id as a value (like it works with scope at the moment).

[jgrandja]

Thanks for the report @fwollsch.

I understand that you're looking to customize the OAuth2AuthorizationConsent (before it's OAuth2AuthorizationConsentService.save()) using input from OAuth2AuthorizationCodeRequestAuthenticationToken. Is my understanding correct?

[fwollsch]

Hello @jgrandja ,

yes, exactly.

In my case it's the OAuth2AuthorizationCodeRequestAuthenticationToken#getAdditionalParameters() which contains all not explicitly mapped Parameters sent to POST /oauth2/authorize.

It doesn't have to be the OAuth2AuthorizationConsentService which receives these parameters. It can be a entirely new type of Component, if that fits the design better. It's just that currently the OAuth2AuthorizationCodeRequestAuthenticationToken#getAdditionalParameters() are never passed down to any plugin-able component (and because of that I had to implement it the "hacky" way I showed above).

I attached a video of how my consent page looks like:
https://user-images.githubusercontent.com/72974593/134470267-ba3df5f2-4d16-4e0a-8c6a-2ddc609f332d.mov

This may help to understand what I'm referring to.

[jgrandja]

This makes sense @fwollsch.

I was expecting to introduce a Customizer component for OAuth2AuthorizationConsent at some point. It's very common to have different types of authority information presented to the user during consent, other than scope.

I will get to this shortly and propose a design that you can review and validate.

[sjohnr]

@fwollsch, that's awesome!

I attached a video of how my consent page looks like: https://user-images.githubusercontent.com/72974593/134470267-ba3df5f2-4d16-4e0a-8c6a-2ddc609f332d.mov

This may help to understand what I'm referring to.

GW2 FTW! I don't play much any more but I love that game! I used to play all the time. I'd love to hear more about your use case for the authorization server! (@sjohnr on twitter)

So here's what I'm thinking. We can add a Customizer<OAuth2AuthorizationContext> authorizationConsentCustomizer, and populate it with the consent builder, authorization, and request:

Map<Object, Object> context = new HashMap<>();
context.put(OAuth2AuthorizationConsent.Builder.class, authorizationConsentBuilder);
context.put(OAuth2Authorization.class, authorization);
context.put(OAuth2AuthorizationRequest.class, authorizationRequest);
OAuth2AuthenticationContext authenticationContext = new OAuth2AuthenticationContext(
		authorizationCodeRequestAuthentication, context);
this.authorizationConsentCustomizer.customize(authenticationContext);

We could possibly put other things into the context if needed. We could also look at creating a new strongly-typed context just for OAuth2AuthorizationConsent.Builder and associated objects needed for customization. We only need to do this if it gets hard to manage all the objects needed for customization.

Either way, this should allow you access to all additional parameters and ability to customize the consent prior to saving.

The additionalParameters are currently created by passing all Request-Parameters that are not explicitly known (such as scope, client_id and state), but only the first value is actually added to the additionalParameters.

Note: I'm not currently sure about handling multi-valued parameters. That would probably need to be a separate issue that we can evaluate the need to solve for, since additional parameters don't currently accomodate multiple values.

Does this sound like a suitable solution? Any thoughts or concerns?

[fwollsch]

Perfect! This sounds exactly how I imagined it.
The multi-valued parameters would be nice to have, but definitely not required.

[fwollsch]

@sjohnr I couldn't reach you on Twitter, but maybe I just didnt find the DM button.

My use case for Gw2 is to provide a unified and easier to use way to authorize applications to use your API-Tokens.

The Gw2-API is extremely powerful (only read access, but still very powerful). Sadly, you can only create API-Tokens without a TTL or similar using the official website. So people end up (including me) having one API-Token with all permissions created on the official website and using that for every gw2 website/tool out there.

Luckily, the Gw2-API provides one endpoint to create so-called "subtokens". Given another API-Token, you can request a subtoken with reduced permissions and an expiration time. The authorization server I'm building uses this Endpoint to provide authorized clients with subtokens for the gw2-accounts a user consented.

You can check out the sourcecode if you like: https://github.com/gw2auth/oauth2-server
Or the website of course: https://gw2auth.com

[markhobson]

Hi, I've just absorbed this change whilst testing 0.2.1 for another issue. I'm not using this customisation but my deny consent flow now breaks. This is due to the change in OAuth2AuthorizationCodeRequestAuthenticationProvider that now requires no additional parameters to be sent in order to return access denied.

In my case, I'm happening to also submit a CSRF token _csrf and the deny button's name deny. I could look to remove these but I was wondering what the rationale is for this strict check?

[sjohnr]

Hi @markhobson. Thanks for bringing it up.

This is a bit tricky, because this feature of adding support for custom authorization consent is essentially making authorized scopes not required, which per the spec, they are not. Therefore, we are looking for either scopes or some additional parameters to be present to determine whether to process the request as a consent. I don't know of any other signal we can look for to continue to support the deny consent flow. I could be wrong, but I think for consistency with your use case, the deny consent flow would have to submit the form without any parameters. If the CSRF token is also present as a form parameter, that might present a real problem with this approach, as I would think a CSRF token should be required when submitting authorization consent.

We'll have to discuss this a bit more and see what options there are for this.

[jgrandja]

Thanks for the report @markhobson. We'll address this before we release 0.2.1 next week.

[jgrandja]

Thanks again @markhobson for catching this regression before the release next week.

This should now be resolved via c418306.

Can you please confirm the fix has restored original behaviour for your deny consent flow?

[markhobson]

Hi @sjohnr @jgrandja, thanks for the replies and the quick fix. I've tried main and the deny consent flow now works correctly. Thank you!

[jgrandja]

Excellent ! Thanks for checking!