Skip to content
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

JCL-444: Improve support for RFC 9207 #1246

Merged
merged 4 commits into from
May 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions openid/src/main/java/com/inrupt/client/openid/Metadata.java
Original file line number Diff line number Diff line change
Expand Up @@ -112,4 +112,9 @@ public class Metadata {
* A list of DPoP signing algorithm values supported by the given OpenID Connect provider.
*/
public List<String> dpopSigningAlgValuesSupported;

/**
* Indication of whether the OpenID Connect provider supports RFC-9207.
*/
public boolean authorizationResponseIssParameterSupported;
jholleran marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,15 @@ ErrorResponse tryParseError(final InputStream input) {
}

private Request tokenRequest(final Metadata metadata, final TokenRequest request) {
// RFC 9207 describes this behavior as a SHOULD but recognizes use cases that vary;
// this would be good to consider when adding broader configuration support to the libraries.
if (metadata.authorizationResponseIssParameterSupported) {
if (!Objects.equals(request.getIssuer(), metadata.issuer)) {
throw new OpenIdException("Issuer mismatch. " +
"Please verify that the designated OpenID issuer is correct");
}
}

if (!metadata.grantTypesSupported.contains(request.getGrantType())) {
throw new OpenIdException("Grant type [" + request.getGrantType() + "] is not supported by this provider.");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ public static Session ofClientCredentials(final URI issuer, final String clientI
return new OpenIdSession(id, dpop, () -> provider.token(TokenRequest.newBuilder()
.clientSecret(clientSecret)
.authMethod(authMethod)
.issuer(issuer)
.scopes(config.getScopes().toArray(new String[0]))
.build("client_credentials", clientId))
.thenApply(response -> {
Expand All @@ -166,11 +167,13 @@ public static Session ofClientCredentials(final OpenIdProvider provider,
final OpenIdConfig config) {
final String id = UUID.randomUUID().toString();
final DPoP dpop = DPoP.of(config.getProofKeyPairs());
return new OpenIdSession(id, dpop, () -> provider.token(TokenRequest.newBuilder()
return new OpenIdSession(id, dpop, () -> provider.metadata()
.thenCompose(metadata -> provider.token(TokenRequest.newBuilder()
.clientSecret(clientSecret)
.authMethod(authMethod)
.scopes(config.getScopes().toArray(new String[0]))
.build("client_credentials", clientId))
.issuer(metadata.issuer)
.build("client_credentials", clientId)))
.thenApply(response -> {
final JwtClaims claims = parseIdToken(response.idToken, config);
return new Credential(response.tokenType, getIssuer(claims), response.idToken,
Expand Down
28 changes: 26 additions & 2 deletions openid/src/main/java/com/inrupt/client/openid/TokenRequest.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public final class TokenRequest {
private final String clientSecret;
private final String authMethod;
private final URI redirectUri;
private final URI issuer;
private final List<String> scopes;

/**
Expand All @@ -58,6 +59,15 @@ public List<String> getScopes() {
return scopes;
}

/**
* Get the issuer.
*
* @return the issuer, may be {@code null}
*/
public URI getIssuer() {
return issuer;
}

/**
* Get the authentication method.
*
Expand Down Expand Up @@ -123,7 +133,8 @@ public static Builder newBuilder() {

/* package-private */
TokenRequest(final String clientId, final String clientSecret, final URI redirectUri, final String grantType,
final String authMethod, final String code, final String codeVerifier, final List<String> scopes) {
final String authMethod, final String code, final String codeVerifier, final URI issuer,
final List<String> scopes) {
this.clientId = clientId;
this.clientSecret = clientSecret;
this.redirectUri = redirectUri;
Expand All @@ -132,6 +143,7 @@ public static Builder newBuilder() {
this.code = code;
this.codeVerifier = codeVerifier;
this.scopes = scopes;
this.issuer = issuer;
}

/**
Expand All @@ -146,6 +158,7 @@ public static class Builder {
private String builderCode;
private String builderCodeVerifier;
private URI builderRedirectUri;
private URI builderIssuer;
private List<String> builderScopes = new ArrayList<>();

/**
Expand Down Expand Up @@ -181,6 +194,17 @@ public Builder scopes(final String... scopes) {
return this;
}

/**
* Set the issuer URI.
*
* @param issuer the issuer value
* @return this builder
*/
public Builder issuer(final URI issuer) {
builderIssuer = issuer;
return this;
}

/**
* Set the authentication method for the token endpoint.
*
Expand Down Expand Up @@ -240,7 +264,7 @@ public TokenRequest build(final String grantType, final String clientId) {
}

return new TokenRequest(clientId, builderClientSecret, builderRedirectUri, grant, builderAuthMethod,
builderCode, builderCodeVerifier, builderScopes);
builderCode, builderCodeVerifier, builderIssuer, builderScopes);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,19 +75,16 @@ private void setupMocks() {
.willReturn(aResponse()
.withStatus(200)
.withHeader("Content-Type", "application/json")
.withBody(getMetadataJSON()
.replace(
"/oauth/oauth20/token",
wireMockServer.baseUrl() + "/oauth/oauth20/token"))));
.withBody(getMetadataJSON())));

wireMockServer.stubFor(post(urlPathMatching("/oauth/oauth20/token"))
wireMockServer.stubFor(post(urlPathMatching("/token"))
.withHeader("Content-Type", containing("application/x-www-form-urlencoded"))
.withRequestBody(containing("myCodeverifier"))
.willReturn(aResponse()
.withStatus(200)
.withHeader("Content-Type", "application/json")
.withBody(getTokenResponseJSON())));
wireMockServer.stubFor(post(urlPathMatching("/oauth/oauth20/token"))
wireMockServer.stubFor(post(urlPathMatching("/token"))
.withHeader("Content-Type", containing("application/x-www-form-urlencoded"))
.atPriority(1)
.withRequestBody(containing("code=none"))
Expand All @@ -96,7 +93,7 @@ private void setupMocks() {
.withHeader("Content-Type", "application/json")
.withBodyFile("token-error.json")));

wireMockServer.stubFor(post(urlPathMatching("/oauth/oauth20/token"))
wireMockServer.stubFor(post(urlPathMatching("/token"))
.withHeader("Content-Type", containing("application/x-www-form-urlencoded"))
.atPriority(2)
.withRequestBody(containing("grant_type=client_credentials"))
Expand All @@ -108,7 +105,8 @@ private void setupMocks() {

private String getMetadataJSON() {
try (final InputStream res = OpenIdMockHttpService.class.getResourceAsStream("/metadata.json")) {
return new String(IOUtils.toByteArray(res), UTF_8);
return new String(IOUtils.toByteArray(res), UTF_8)
.replace("http://example.test", wireMockServer.baseUrl());
} catch (final IOException ex) {
throw new UncheckedIOException("Could not read class resource", ex);
}
Expand All @@ -130,9 +128,6 @@ private String getTokenResponseJSON() {
"}";
}




public String start() {
wireMockServer.start();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

import com.inrupt.client.auth.DPoP;
import com.inrupt.client.openid.TokenRequest.Builder;
import com.inrupt.client.util.URIBuilder;

import java.io.ByteArrayInputStream;
import java.io.IOException;
Expand All @@ -33,8 +34,6 @@
import java.security.NoSuchAlgorithmException;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.OptionalInt;
import java.util.UUID;
import java.util.concurrent.CompletableFuture;
Expand All @@ -49,14 +48,14 @@ class OpenIdProviderTest {

private static OpenIdProvider openIdProvider;
private static final OpenIdMockHttpService mockHttpService = new OpenIdMockHttpService();
private static final Map<String, String> config = new HashMap<>();
private static final DPoP dpop = DPoP.of();

private static URI issuer;

@BeforeAll
static void setup() throws NoSuchAlgorithmException {
config.put("openid_uri", mockHttpService.start());
openIdProvider = new OpenIdProvider(URI.create(config.get("openid_uri")), dpop);
issuer = URI.create(mockHttpService.start());
openIdProvider = new OpenIdProvider(issuer, dpop);
}

@AfterAll
Expand All @@ -66,15 +65,16 @@ static void teardown() {

@Test
void metadataAsyncTest() {
assertEquals("http://example.test",
openIdProvider.metadata().toCompletableFuture().join().issuer.toString());
assertEquals("http://example.test/oauth/jwks",
openIdProvider.metadata().toCompletableFuture().join().jwksUri.toString());
assertEquals(issuer,
openIdProvider.metadata().toCompletableFuture().join().issuer);
assertEquals(URIBuilder.newBuilder(issuer).path("jwks").build(),
openIdProvider.metadata().toCompletableFuture().join().jwksUri);
}

@Test
void unknownMetadata() {
final OpenIdProvider provider = new OpenIdProvider(URI.create(config.get("openid_uri") + "/not-found"), dpop);
final OpenIdProvider provider = new OpenIdProvider(URIBuilder.newBuilder(issuer).path("not-found").build(),
dpop);
final CompletionException err = assertThrows(CompletionException.class,
provider.metadata().toCompletableFuture()::join);
assertTrue(err.getCause() instanceof OpenIdException);
Expand All @@ -90,7 +90,7 @@ void authorizeAsyncTest() {
URI.create("myRedirectUri")
);
assertEquals(
"http://example.test/auth?client_id=myClientId&redirect_uri=myRedirectUri&" +
issuer + "/auth?client_id=myClientId&redirect_uri=myRedirectUri&" +
"response_type=code&code_challenge=myCodeChallenge&code_challenge_method=method",
openIdProvider.authorize(authReq).toCompletableFuture().join().toString()
);
Expand All @@ -114,11 +114,67 @@ void tokenRequestIllegalArgumentsTest() {
() -> builder.build("myGrantType", null));
}

@Test
void tokenIssuerMismatch() {
final TokenRequest tokenReq = TokenRequest.newBuilder()
.code("someCode")
.codeVerifier("myCodeverifier")
.issuer(URI.create("https://not.an.issuer.test"))
.redirectUri(URI.create("https://example.test/redirectUri"))
.build(
"authorization_code",
"myClientId"
);

final CompletionException ex = assertThrows(CompletionException.class, openIdProvider.token(tokenReq)
.toCompletableFuture()::join);
assertTrue(ex.getCause() instanceof OpenIdException);
final OpenIdException cause = (OpenIdException) ex.getCause();
assertTrue(cause.getMessage().contains("Issuer mismatch"));
}

@Test
void tokenIssuerMissing() {
final TokenRequest tokenReq = TokenRequest.newBuilder()
.code("someCode")
.codeVerifier("myCodeverifier")
.redirectUri(URI.create("https://example.test/redirectUri"))
.build(
"authorization_code",
"myClientId"
);

final CompletionException ex = assertThrows(CompletionException.class, openIdProvider.token(tokenReq)
.toCompletableFuture()::join);
assertTrue(ex.getCause() instanceof OpenIdException);
final OpenIdException cause = (OpenIdException) ex.getCause();
assertTrue(cause.getMessage().contains("Issuer mismatch"));
}

@Test
void tokenIssuerMatch() {
final TokenRequest tokenReq = TokenRequest.newBuilder()
.code("someCode")
.codeVerifier("myCodeverifier")
.issuer(issuer)
.redirectUri(URI.create("https://example.test/redirectUri"))
.build(
"authorization_code",
"myClientId"
);
final TokenResponse token = openIdProvider.token(tokenReq)
.toCompletableFuture().join();
assertEquals("123456", token.accessToken);
assertNotNull(token.idToken);
assertEquals("Bearer", token.tokenType);
}

@Test
void tokenNoClientSecretTest() {
final TokenRequest tokenReq = TokenRequest.newBuilder()
.code("someCode")
.codeVerifier("myCodeverifier")
.issuer(issuer)
.redirectUri(URI.create("https://example.test/redirectUri"))
.build(
"authorization_code",
Expand All @@ -137,6 +193,7 @@ void tokenWithClientSecretBasicTest() {
.code("someCode")
.codeVerifier("myCodeverifier")
.clientSecret("myClientSecret")
.issuer(issuer)
.authMethod("client_secret_basic")
.redirectUri(URI.create("https://example.test/redirectUri"))
.build(
Expand All @@ -156,6 +213,7 @@ void tokenWithClientSecretePostTest() {
.code("someCode")
.codeVerifier("myCodeverifier")
.clientSecret("myClientSecret")
.issuer(issuer)
.authMethod("client_secret_post")
.redirectUri(URI.create("https://example.test/redirectUri"))
.build(
Expand All @@ -174,6 +232,7 @@ void tokenAsyncTest() {
final TokenRequest tokenReq = TokenRequest.newBuilder()
.code("someCode")
.codeVerifier("myCodeverifier")
.issuer(issuer)
.redirectUri(URI.create("https://example.test/redirectUri"))
.build("authorization_code", "myClientId");
final TokenResponse token = openIdProvider.token(tokenReq).toCompletableFuture().join();
Expand All @@ -187,6 +246,7 @@ void tokenAsyncStatusCodesTest() {
final TokenRequest tokenReq = TokenRequest.newBuilder()
.code("none")
.codeVerifier("none")
.issuer(issuer)
.redirectUri(URI.create("none"))
.build("authorization_code", "none");

Expand Down Expand Up @@ -217,7 +277,7 @@ void endSessionAsyncTest() {
.build();
final URI uri = openIdProvider.endSession(endReq).toCompletableFuture().join();
assertEquals(
"http://example.test/endSession?" +
issuer + "/endSession?" +
"client_id=myClientId&post_logout_redirect_uri=https://example.test/redirectUri&id_token_hint=&state=solid",
uri.toString()
);
Expand Down
Loading