Skip to content

Open ID Connect issuer URI "cleanup" before discovery breaks issuer URI matching #6377

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
denisw opened this issue Jan 9, 2019 · 2 comments
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose)

Comments

@denisw
Copy link
Contributor

denisw commented Jan 9, 2019

Summary

When doing OpenID Connect issuer discovery, Spring Security removes any trailing slash from the issuer URI before appending /.well-known/openid-configuration, as mandated by the spec. However, after having fetched the configuration, the the returned configuration's issuer URI matched against the "cleaned up" version of the issue URI, rather than the the originally provided one.

This is problematic if the canonical issuer URI has a trailing space (e.g., https://auth.example.com/).

Actual Behavior

java.lang.IllegalStateException: The Issuer "https://auth.example.com/" provided in the OpenID Configuration did not match the requested issuer "https://auth.example.com"

Note that the "requested issuer" always lacks the trailing slash, even if it was explicitly specified with trailing slash in the configuration.

Expected Behavior

No error, as the issuer URI returned by the configuration endpoint and the one provided in the application configuration are actually the same.

Details

OAuth2ClientPropertiesRegistrationAdapter.java has the following code:

String issuer = provider.getIssuerUri();
if (issuer != null) {
    String cleanedIssuer = cleanIssuerPath(issuer);
    Builder builder = ClientRegistrations
            .fromOidcIssuerLocation(cleanedIssuer)
            .registrationId(registrationId);
    return getBuilder(builder, provider);
}

And in fromOidcIssuerLocation():

String openidConfiguration = getOpenidConfiguration(issuer);
OIDCProviderMetadata metadata = parse(openidConfiguration);
String metadataIssuer = metadata.getIssuer().getValue();
if (!issuer.equals(metadataIssuer)) {
    throw new IllegalStateException("The Issuer \"" + metadataIssuer + "\" provided in the OpenID Configuration did not match the requested issuer \"" + issuer + "\"");
}

Because fromOidcIssuerLocation() doesn't know about the originally provided issuer URL, it matches against the cleanedIssuer, which causes the described problem. The easiest way to solve that is to do the cleanup within fromOidcIssuerLocation(), which then still has the original version available for matching.

@denisw denisw changed the title Open ID Connect issuer URI "cleanup" breaks issuer URI comparison Open ID Connect issuer URI "cleanup" before discovery breaks issuer URI matching Jan 9, 2019
@jgrandja
Copy link
Contributor

jgrandja commented Jan 9, 2019

Thanks for the report @denisw! This should be resolved via spring-boot#15324.

Are you able to test it with Boot 2.1.2 snapshot? Or you can wait until this Friday when Boot 2.1.2 will be released.

I'll leave this open until you confirm it's been fixed.

@jgrandja jgrandja added status: waiting-for-feedback We need additional information before we can continue in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) OIDC labels Jan 9, 2019
@denisw
Copy link
Contributor Author

denisw commented Jan 9, 2019

@jgrandja Thanks for the quick reply! Things work perfectly with the 2.1.2 snapshot. 😃 I will close the issue now.

@denisw denisw closed this as completed Jan 9, 2019
@rwinch rwinch removed the status: waiting-for-feedback We need additional information before we can continue label Jan 9, 2019
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)
Projects
None yet
Development

No branches or pull requests

3 participants