Skip to content

OpenID Connect: Make OAuth2AuthorizationRequest for IDToken validation #8342

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
furti opened this issue Apr 7, 2020 · 9 comments
Closed
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: enhancement A general enhancement

Comments

@furti
Copy link
Contributor

furti commented Apr 7, 2020

Summary

I can't find an easy way, to validate that the acr value in the ID Token is what the client expects. The spec says, when requesting an acr claims value, the Client SHOULD check, if the acr value is appropriate.

For now, the only way I see is registering a custom AuthenticationProvider that enhances the logic of the default Authentication Provider and performs the validation afterwards.

Requesting acr claims is not supported by spring security at the moment, but it is pretty straight forward to create a custom OAuth2AuthorizationRequestResolver that adds the relevant data to the OAuth2AuthorizationRequest. So the information, that a acr value was requested is stored inside the OAuth2AuthorizationRequest.

Actual Behavior

Currently the OAuth2AuthorizationRequest is not passed to the ID Token generation and validation logic here:

private OidcIdToken createOidcToken(ClientRegistration clientRegistration, OAuth2AccessTokenResponse accessTokenResponse) {

Only the ClientRegistration is passed in.

The only place where data from the request is used to validate the ID Token ist in

There is no way to do additional validation of the ID Token based on data of the OAuth2AuthorizationRequest.

Expected Behavior

It should be possible to add custom validation logic of the ID Token based on data of the OAuth2AuthorizationRequest.

I see two possible solutions

Solution 1

Create a ID Token Validator interface and call it on line

And move the noce validation into a default validator implementation.

Solution 2

Enhance the JwtDecoderFactory and pass the OAuth2AuthorizationRequest to the factory.

I would prefer solution 2, because a lot of ID Token Validation is already done in the decoder factory. The only thing done outside is Nonce validation. When the request is available to the factory, the nonce validation could be moved inside.
The factory is already pluggable. So its easy to customize and add additional validations when needed.

Version

5.2.3 -> Still in latest master

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Apr 7, 2020
@jgrandja
Copy link
Contributor

jgrandja commented Apr 7, 2020

@furti The recommended way is to plug in a custom OAuth2TokenValidator via OidcIdTokenDecoderFactory.setJwtValidatorFactory() - Option 2.

Can you provide me more details on your custom validation logic. For example, what are the acr values being requested in the Authorization Request? Are the values static or dynamic? And are they different per client?

@jgrandja jgrandja added in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged labels Apr 7, 2020
@furti
Copy link
Contributor Author

furti commented Apr 8, 2020

@jgrandja

Lets assume a Client has different protected resources. Some resources are permitted for username and password authentication, and others are only permitted with Multifactor Authentication.
This means the Client wants to dynamically create a Authorization Request, that requests certain acr values by adding the claims Paramter to the Authorization Request.

"acr": {"essential": true,
          "values": ["WhateverValueClientNeeds"]}

For this I already created a custom OAuth2AuthorizationRequestResolver that adds the Parameter depending on some HttpServletRequest Parameter.
It also stores the requestes ACR Values in the attributes of the OAuth2AuthorizationRequest for further usage.

private OAuth2AuthorizationRequest resolve(HttpServletRequest request,
        OAuth2AuthorizationRequest authorizationRequest)
    {
        if (authorizationRequest == null)
        {
            return null;
        }

        Map<String, Object> additionalParameters = new LinkedHashMap<>(authorizationRequest.getAdditionalParameters());
        Map<String, Object> attributes = new LinkedHashMap<>(authorizationRequest.getAttributes());

        addRequestedAcrParameter(request, additionalParameters, attributes);

        return OAuth2AuthorizationRequest
            .from(authorizationRequest) //
            .additionalParameters(additionalParameters)
            .attributes(attributes)
            .build();
    }

    private void addRequestedAcrParameter(HttpServletRequest request, Map<String, Object> additionalParameters,
        Map<String, Object> attributes)
    {
        String[] acr = request.getParameterValues(ACR_PARAM);

        if (acr == null)
        {
            return;
        }

        attributes.put(ACR_PARAM, Arrays.asList(acr));
        additionalParameters.put("claims", buildAcrRequest(acr));
    }

    private String buildAcrRequest(String[] acrs)
    {
        String acr = Arrays.stream(acrs).collect(Collectors.joining(","));

        return String.format("{\"id_token\":{\"acr\": {\"values\": [\"%s\"], \"essential\": true}}}", acr);
    }

Now the OpenID Connect spec says https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation

...
12. If the acr Claim was requested, the Client SHOULD check that the asserted Claim Value is appropriate. The meaning and processing of acr Claim Values is out of scope for this specification.
...

For me that means, check that the Server responded with an acr value in the list of the requested acr values.

But the OAuth2TokenValidator only gets the token passed in, and has no knowledge of the OAuth2AuthorizationRequest. So it can not access the requested acr values.

For now I register a custom OidcAuthorizationCodeAuthenticationProvider that handles validation of acr values after the default authentication is performed

@Override
public Authentication authenticate(Authentication authentication) throws AuthenticationException
{
    Collection<String> requestedAcrValues = getRequestedAcrValues(authentication);

    OAuth2LoginAuthenticationToken openIdAuthentication =
        (OAuth2LoginAuthenticationToken) super.authenticate(authentication);

    validateAcrValues(requestedAcrValues, openIdAuthentication);

    return openIdAuthentication;
}

I think this is not the most clean approach to extend validation. It also has the disadvantage of fetching the userinfo, even when the acr value is invalid, as fetching happens in the super call.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Apr 8, 2020
@jgrandja
Copy link
Contributor

jgrandja commented Apr 13, 2020

@furti I see your dilemma. You need access to the OAuth2AuthorizationRequest to perform the validation, however, it's not provided (or accessible) to the OAuth2TokenValidator.

It doesn't make sense to directly provide the OAuth2AuthorizationRequest to the OAuth2TokenValidator given that an Authorization Request is only applicable to authorization_code - an OAuth2TokenValidator may be implemented for a client_credentials client.

The first thought that came to mind is providing a ThreadLocal mechanism for accessing Authorization Request/Response attributes, similar to RequestContextHolder and SecurityContextHolder. But I'm not sure if this is the right approach. Let me think about this one for a bit and see what we can do to address your use case.

@jgrandja jgrandja added type: enhancement A general enhancement and removed status: feedback-provided Feedback has been provided labels Apr 17, 2020
@jgrandja
Copy link
Contributor

@furti Have you been able to figure out an alternative way to validate the acr claim in the ID Token?

@jgrandja jgrandja added the status: waiting-for-feedback We need additional information before we can continue label May 18, 2021
@spring-projects-issues
Copy link

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label May 25, 2021
@spring-projects-issues
Copy link

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.

@spring-projects-issues spring-projects-issues removed status: waiting-for-feedback We need additional information before we can continue status: feedback-reminder We've sent a reminder that we need additional information before we can continue labels Jun 1, 2021
@furti
Copy link
Contributor Author

furti commented Jul 9, 2021

Sorry for my late reply. For now I used the approach mentioned above with a custom AuthenticationProvider as it works. I also have no idea on how to solve this in a clean way.

@titiviking
Copy link

I'm having the same dilemma.... Several resources that require different authentication levels...
I'm surprised this is not yet provided in an easy manner by Spring boot... basic principles as Step-up authentication cannot be supported without such feature...

@Nordiii
Copy link

Nordiii commented Mar 24, 2023

Yeap same Problem, want some Admin resources behind higher LoA and the only hint I can find regarding this is this closed GitHub issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

5 participants