Skip to content

Commit f00802e

Browse files
acoburnacoburn
committed
Code review
1 parent 37ccf0b commit f00802e

File tree

5 files changed

+57
-67
lines changed

5 files changed

+57
-67
lines changed

‎openid/src/main/java/com/inrupt/client/openid/OpenIdProvider.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -211,8 +211,10 @@ ErrorResponse tryParseError(final InputStream input) {
211211
}
212212

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

‎openid/src/main/java/com/inrupt/client/openid/OpenIdSession.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ public static Session ofClientCredentials(final URI issuer, final String clientI
142142
return new OpenIdSession(id, dpop, () -> provider.token(TokenRequest.newBuilder()
143143
.clientSecret(clientSecret)
144144
.authMethod(authMethod)
145+
.issuer(issuer)
145146
.scopes(config.getScopes().toArray(new String[0]))
146147
.build("client_credentials", clientId))
147148
.thenApply(response -> {
@@ -166,11 +167,13 @@ public static Session ofClientCredentials(final OpenIdProvider provider,
166167
final OpenIdConfig config) {
167168
final String id = UUID.randomUUID().toString();
168169
final DPoP dpop = DPoP.of(config.getProofKeyPairs());
169-
return new OpenIdSession(id, dpop, () -> provider.token(TokenRequest.newBuilder()
170+
return new OpenIdSession(id, dpop, () -> provider.metadata()
171+
.thenCompose(metadata -> provider.token(TokenRequest.newBuilder()
170172
.clientSecret(clientSecret)
171173
.authMethod(authMethod)
172174
.scopes(config.getScopes().toArray(new String[0]))
173-
.build("client_credentials", clientId))
175+
.issuer(metadata.issuer)
176+
.build("client_credentials", clientId)))
174177
.thenApply(response -> {
175178
final JwtClaims claims = parseIdToken(response.idToken, config);
176179
return new Credential(response.tokenType, getIssuer(claims), response.idToken,

‎openid/src/test/java/com/inrupt/client/openid/OpenIdMockHttpService.java‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -80,14 +80,14 @@ private void setupMocks() {
8080
"/oauth/oauth20/token",
8181
wireMockServer.baseUrl() + "/oauth/oauth20/token"))));
8282

83-
wireMockServer.stubFor(post(urlPathMatching("/oauth/oauth20/token"))
83+
wireMockServer.stubFor(post(urlPathMatching("/token"))
8484
.withHeader("Content-Type", containing("application/x-www-form-urlencoded"))
8585
.withRequestBody(containing("myCodeverifier"))
8686
.willReturn(aResponse()
8787
.withStatus(200)
8888
.withHeader("Content-Type", "application/json")
8989
.withBody(getTokenResponseJSON())));
90-
wireMockServer.stubFor(post(urlPathMatching("/oauth/oauth20/token"))
90+
wireMockServer.stubFor(post(urlPathMatching("/token"))
9191
.withHeader("Content-Type", containing("application/x-www-form-urlencoded"))
9292
.atPriority(1)
9393
.withRequestBody(containing("code=none"))
@@ -96,7 +96,7 @@ private void setupMocks() {
9696
.withHeader("Content-Type", "application/json")
9797
.withBodyFile("token-error.json")));
9898

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

109109
private String getMetadataJSON() {
110110
try (final InputStream res = OpenIdMockHttpService.class.getResourceAsStream("/metadata.json")) {
111-
return new String(IOUtils.toByteArray(res), UTF_8);
111+
return new String(IOUtils.toByteArray(res), UTF_8)
112+
.replace("http://example.test", wireMockServer.baseUrl());
112113
} catch (final IOException ex) {
113114
throw new UncheckedIOException("Could not read class resource", ex);
114115
}
@@ -130,9 +131,6 @@ private String getTokenResponseJSON() {
130131
"}";
131132
}
132133

133-
134-
135-
136134
public String start() {
137135
wireMockServer.start();
138136

‎openid/src/test/java/com/inrupt/client/openid/OpenIdProviderTest.java‎

Lines changed: 37 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
import com.inrupt.client.auth.DPoP;
2727
import com.inrupt.client.openid.TokenRequest.Builder;
28+
import com.inrupt.client.util.URIBuilder;
2829

2930
import java.io.ByteArrayInputStream;
3031
import java.io.IOException;
@@ -33,8 +34,6 @@
3334
import java.security.NoSuchAlgorithmException;
3435
import java.util.Arrays;
3536
import java.util.Collections;
36-
import java.util.HashMap;
37-
import java.util.Map;
3837
import java.util.OptionalInt;
3938
import java.util.UUID;
4039
import java.util.concurrent.CompletableFuture;
@@ -49,14 +48,14 @@ class OpenIdProviderTest {
4948

5049
private static OpenIdProvider openIdProvider;
5150
private static final OpenIdMockHttpService mockHttpService = new OpenIdMockHttpService();
52-
private static final Map<String, String> config = new HashMap<>();
5351
private static final DPoP dpop = DPoP.of();
5452

53+
private static URI issuer;
5554

5655
@BeforeAll
5756
static void setup() throws NoSuchAlgorithmException {
58-
config.put("openid_uri", mockHttpService.start());
59-
openIdProvider = new OpenIdProvider(URI.create(config.get("openid_uri")), dpop);
57+
issuer = URI.create(mockHttpService.start());
58+
openIdProvider = new OpenIdProvider(issuer, dpop);
6059
}
6160

6261
@AfterAll
@@ -66,15 +65,16 @@ static void teardown() {
6665

6766
@Test
6867
void metadataAsyncTest() {
69-
assertEquals("http://example.test",
70-
openIdProvider.metadata().toCompletableFuture().join().issuer.toString());
71-
assertEquals("http://example.test/oauth/jwks",
72-
openIdProvider.metadata().toCompletableFuture().join().jwksUri.toString());
68+
assertEquals(issuer,
69+
openIdProvider.metadata().toCompletableFuture().join().issuer);
70+
assertEquals(URIBuilder.newBuilder(issuer).path("jwks").build(),
71+
openIdProvider.metadata().toCompletableFuture().join().jwksUri);
7372
}
7473

7574
@Test
7675
void unknownMetadata() {
77-
final OpenIdProvider provider = new OpenIdProvider(URI.create(config.get("openid_uri") + "/not-found"), dpop);
76+
final OpenIdProvider provider = new OpenIdProvider(URIBuilder.newBuilder(issuer).path("not-found").build(),
77+
dpop);
7878
final CompletionException err = assertThrows(CompletionException.class,
7979
provider.metadata().toCompletableFuture()::join);
8080
assertTrue(err.getCause() instanceof OpenIdException);
@@ -90,7 +90,7 @@ void authorizeAsyncTest() {
9090
URI.create("myRedirectUri")
9191
);
9292
assertEquals(
93-
"http://example.test/auth?client_id=myClientId&redirect_uri=myRedirectUri&" +
93+
issuer + "/auth?client_id=myClientId&redirect_uri=myRedirectUri&" +
9494
"response_type=code&code_challenge=myCodeChallenge&code_challenge_method=method",
9595
openIdProvider.authorize(authReq).toCompletableFuture().join().toString()
9696
);
@@ -119,7 +119,25 @@ void tokenIssuerMismatch() {
119119
final TokenRequest tokenReq = TokenRequest.newBuilder()
120120
.code("someCode")
121121
.codeVerifier("myCodeverifier")
122-
.issuer(URI.create("https://issuer.test"))
122+
.issuer(URI.create("https://not.an.issuer.test"))
123+
.redirectUri(URI.create("https://example.test/redirectUri"))
124+
.build(
125+
"authorization_code",
126+
"myClientId"
127+
);
128+
129+
final CompletionException ex = assertThrows(CompletionException.class, openIdProvider.token(tokenReq)
130+
.toCompletableFuture()::join);
131+
assertTrue(ex.getCause() instanceof OpenIdException);
132+
final OpenIdException cause = (OpenIdException) ex.getCause();
133+
assertTrue(cause.getMessage().contains("Issuer mismatch"));
134+
}
135+
136+
@Test
137+
void tokenIssuerMissing() {
138+
final TokenRequest tokenReq = TokenRequest.newBuilder()
139+
.code("someCode")
140+
.codeVerifier("myCodeverifier")
123141
.redirectUri(URI.create("https://example.test/redirectUri"))
124142
.build(
125143
"authorization_code",
@@ -138,7 +156,7 @@ void tokenIssuerMatch() {
138156
final TokenRequest tokenReq = TokenRequest.newBuilder()
139157
.code("someCode")
140158
.codeVerifier("myCodeverifier")
141-
.issuer(URI.create("http://example.test"))
159+
.issuer(issuer)
142160
.redirectUri(URI.create("https://example.test/redirectUri"))
143161
.build(
144162
"authorization_code",
@@ -156,6 +174,7 @@ void tokenNoClientSecretTest() {
156174
final TokenRequest tokenReq = TokenRequest.newBuilder()
157175
.code("someCode")
158176
.codeVerifier("myCodeverifier")
177+
.issuer(issuer)
159178
.redirectUri(URI.create("https://example.test/redirectUri"))
160179
.build(
161180
"authorization_code",
@@ -174,6 +193,7 @@ void tokenWithClientSecretBasicTest() {
174193
.code("someCode")
175194
.codeVerifier("myCodeverifier")
176195
.clientSecret("myClientSecret")
196+
.issuer(issuer)
177197
.authMethod("client_secret_basic")
178198
.redirectUri(URI.create("https://example.test/redirectUri"))
179199
.build(
@@ -193,6 +213,7 @@ void tokenWithClientSecretePostTest() {
193213
.code("someCode")
194214
.codeVerifier("myCodeverifier")
195215
.clientSecret("myClientSecret")
216+
.issuer(issuer)
196217
.authMethod("client_secret_post")
197218
.redirectUri(URI.create("https://example.test/redirectUri"))
198219
.build(
@@ -211,6 +232,7 @@ void tokenAsyncTest() {
211232
final TokenRequest tokenReq = TokenRequest.newBuilder()
212233
.code("someCode")
213234
.codeVerifier("myCodeverifier")
235+
.issuer(issuer)
214236
.redirectUri(URI.create("https://example.test/redirectUri"))
215237
.build("authorization_code", "myClientId");
216238
final TokenResponse token = openIdProvider.token(tokenReq).toCompletableFuture().join();
@@ -224,6 +246,7 @@ void tokenAsyncStatusCodesTest() {
224246
final TokenRequest tokenReq = TokenRequest.newBuilder()
225247
.code("none")
226248
.codeVerifier("none")
249+
.issuer(issuer)
227250
.redirectUri(URI.create("none"))
228251
.build("authorization_code", "none");
229252

@@ -254,7 +277,7 @@ void endSessionAsyncTest() {
254277
.build();
255278
final URI uri = openIdProvider.endSession(endReq).toCompletableFuture().join();
256279
assertEquals(
257-
"http://example.test/endSession?" +
280+
issuer + "/endSession?" +
258281
"client_id=myClientId&post_logout_redirect_uri=https://example.test/redirectUri&id_token_hint=&state=solid",
259282
uri.toString()
260283
);

‎openid/src/test/resources/metadata.json‎

Lines changed: 6 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,65 +1,29 @@
11
{
22
"issuer":"http://example.test",
3-
"token_endpoint":"/oauth/oauth20/token",
3+
"token_endpoint":"http://example.test/token",
44
"end_session_endpoint":"http://example.test/endSession",
5-
"userinfo_endpoint":"http://example.test/oauth/userinfo",
6-
"jwks_uri":"http://example.test/oauth/jwks",
5+
"userinfo_endpoint":"http://example.test/userinfo",
6+
"jwks_uri":"http://example.test/jwks",
77
"authorization_endpoint":"http://example.test/auth",
88
"scopes_supported":[
9-
"READ",
10-
"WRITE",
11-
"DELETE",
129
"openid",
13-
"scope",
1410
"profile",
15-
"email",
16-
"address",
17-
"phone"
11+
"email"
1812
],
1913
"response_types_supported":[
20-
"code",
21-
"code id_token",
22-
"code token",
23-
"code id_token token",
24-
"token",
25-
"id_token",
26-
"id_token token"
14+
"code"
2715
],
2816
"grant_types_supported":[
2917
"authorization_code",
30-
"implicit",
31-
"password",
3218
"client_credentials",
3319
"urn:ietf:params:oauth:grant-type:jwt-bearer"
3420
],
3521
"subject_types_supported":[
3622
"public"
3723
],
3824
"id_token_signing_alg_values_supported":[
39-
"HS256",
40-
"HS384",
41-
"HS512",
4225
"RS256",
43-
"RS384",
44-
"RS512",
45-
"ES256",
46-
"ES384",
47-
"ES512",
48-
"PS256",
49-
"PS384",
50-
"PS512"
51-
],
52-
"id_token_encryption_alg_values_supported":[
53-
"RSA1_5",
54-
"RSA-OAEP",
55-
"RSA-OAEP-256",
56-
"A128KW",
57-
"A192KW",
58-
"A256KW",
59-
"A128GCMKW",
60-
"A192GCMKW",
61-
"A256GCMKW",
62-
"dir"
26+
"ES256"
6327
],
6428
"token_endpoint_auth_methods_supported":[
6529
"client_secret_post",

0 commit comments

Comments
 (0)