Skip to content

Commit 47962c5

Browse files
sberyozkinsberyozkin
authored
Merge pull request quarkusio#50397 from sberyozkin/oidc_log_and_revoke_request_filter_fix
Fix OIDC TokenRevocation filter binding and improve request logging
2 parents 461391b + e082f0e commit 47962c5

File tree

4 files changed

+89
-61
lines changed

4 files changed

+89
-61
lines changed

‎extensions/oidc-client/runtime/src/main/java/io/quarkus/oidc/client/runtime/OidcClientImpl.java‎

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,19 @@
4141
public class OidcClientImpl implements OidcClient {
4242

4343
private enum Operation {
44-
Get,
45-
Refresh,
46-
Revoke
44+
GET("Get"),
45+
REFRESH("Refresh"),
46+
REVOKE("Revoke");
47+
48+
String op;
49+
50+
Operation(String op) {
51+
this.op = op;
52+
}
53+
54+
String operation() {
55+
return op;
56+
}
4757
}
4858

4959
private static final Logger LOG = Logger.getLogger(OidcClientImpl.class);
@@ -101,7 +111,7 @@ public Uni<Tokens> getTokens(Map<String, String> additionalGrantParameters) {
101111
throw new OidcClientException(
102112
"Only 'refresh_token' grant is supported, please call OidcClient#refreshTokens method instead");
103113
}
104-
return getJsonResponse(OidcEndpoint.Type.TOKEN, tokenGrantParams, additionalGrantParameters, Operation.Get);
114+
return getJsonResponse(OidcEndpoint.Type.TOKEN, tokenGrantParams, additionalGrantParameters, Operation.GET);
105115
}
106116

107117
@Override
@@ -112,7 +122,7 @@ public Uni<Tokens> refreshTokens(String refreshToken, Map<String, String> additi
112122
}
113123
MultiMap refreshGrantParams = copyMultiMap(commonRefreshGrantParams);
114124
refreshGrantParams.add(OidcConstants.REFRESH_TOKEN_VALUE, refreshToken);
115-
return getJsonResponse(OidcEndpoint.Type.TOKEN, refreshGrantParams, additionalGrantParameters, Operation.Refresh);
125+
return getJsonResponse(OidcEndpoint.Type.TOKEN, refreshGrantParams, additionalGrantParameters, Operation.REFRESH);
116126
}
117127

118128
@Override
@@ -128,7 +138,7 @@ public Uni<Boolean> revokeAccessToken(String accessToken, Map<String, String> ad
128138
tokenRevokeParams.set(OidcConstants.REVOCATION_TOKEN, accessToken);
129139
return postRequest(requestProps, OidcEndpoint.Type.TOKEN_REVOCATION, client.postAbs(tokenRevokeUri),
130140
tokenRevokeParams,
131-
additionalParameters, Operation.Revoke)
141+
additionalParameters, Operation.REVOKE)
132142
.transform(resp -> toRevokeResponse(requestProps, resp));
133143
} else {
134144
LOG.debugf("%s OidcClient can not revoke the access token because the revocation endpoint URL is not set");
@@ -247,7 +257,7 @@ private UniOnItem<HttpResponse<Buffer>> postRequest(
247257
}
248258
}
249259
if (LOG.isDebugEnabled()) {
250-
LOG.debugf("%s token: url : %s, headers: %s, request params: %s", op.name(), request.uri(), request.headers(),
260+
LOG.debugf("%s token: url : %s, headers: %s, request params: %s", op.operation(), request.uri(), request.headers(),
251261
body);
252262
}
253263
// Retry up to three times with a one-second delay between the retries if the connection is closed
@@ -381,6 +391,6 @@ OidcClientConfig getConfig() {
381391
}
382392

383393
static boolean isRefresh(Operation op) {
384-
return op == Operation.Refresh;
394+
return op == Operation.REFRESH;
385395
}
386396
}

‎extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcProviderClientImpl.java‎

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,24 @@
4242
import io.vertx.mutiny.ext.web.client.WebClient;
4343

4444
public class OidcProviderClientImpl implements OidcProviderClient, Closeable {
45+
46+
private enum TokenOperation {
47+
GET("Get"),
48+
REFRESH("Refresh"),
49+
INTROSPECT("Introspect"),
50+
REVOKE("Revoke");
51+
52+
String op;
53+
54+
TokenOperation(String op) {
55+
this.op = op;
56+
}
57+
58+
String operation() {
59+
return op;
60+
}
61+
}
62+
4563
private static final Logger LOG = Logger.getLogger(OidcProviderClientImpl.class);
4664

4765
private static final String AUTHORIZATION_HEADER = String.valueOf(HttpHeaders.AUTHORIZATION);
@@ -214,7 +232,8 @@ public Uni<TokenIntrospection> introspectAccessToken(final String token) {
214232
introspectionParams.add(OidcConstants.INTROSPECTION_TOKEN, token);
215233
introspectionParams.add(OidcConstants.INTROSPECTION_TOKEN_TYPE_HINT, OidcConstants.ACCESS_TOKEN_VALUE);
216234
final OidcRequestContextProperties requestProps = getRequestProps(null, null);
217-
return getHttpResponse(requestProps, metadata.getIntrospectionUri(), introspectionParams, true)
235+
return getHttpResponse(requestProps, metadata.getIntrospectionUri(), introspectionParams, TokenOperation.INTROSPECT,
236+
OidcEndpoint.Type.INTROSPECTION)
218237
.transform(resp -> getTokenIntrospection(requestProps, resp));
219238
}
220239

@@ -234,7 +253,8 @@ Uni<AuthorizationCodeTokens> getAuthorizationCodeTokens(String code, String redi
234253
codeGrantParams.addAll(oidcConfig.codeGrant().extraParams());
235254
}
236255
final OidcRequestContextProperties requestProps = getRequestProps(OidcConstants.AUTHORIZATION_CODE);
237-
return getHttpResponse(requestProps, metadata.getTokenUri(), codeGrantParams, false)
256+
return getHttpResponse(requestProps, metadata.getTokenUri(), codeGrantParams, TokenOperation.GET,
257+
OidcEndpoint.Type.TOKEN)
238258
.transform(resp -> getAuthorizationCodeTokens(requestProps, resp));
239259
}
240260

@@ -243,7 +263,8 @@ Uni<AuthorizationCodeTokens> refreshAuthorizationCodeTokens(String refreshToken)
243263
refreshGrantParams.add(OidcConstants.GRANT_TYPE, OidcConstants.REFRESH_TOKEN_GRANT);
244264
refreshGrantParams.add(OidcConstants.REFRESH_TOKEN_VALUE, refreshToken);
245265
final OidcRequestContextProperties requestProps = getRequestProps(OidcConstants.REFRESH_TOKEN_GRANT);
246-
return getHttpResponse(requestProps, metadata.getTokenUri(), refreshGrantParams, false)
266+
return getHttpResponse(requestProps, metadata.getTokenUri(), refreshGrantParams, TokenOperation.REFRESH,
267+
OidcEndpoint.Type.TOKEN)
247268
.transform(resp -> getAuthorizationCodeTokens(requestProps, resp));
248269
}
249270

@@ -263,7 +284,8 @@ private Uni<Boolean> revokeToken(String token, String tokenTypeHint) {
263284
tokenRevokeParams.set(OidcConstants.REVOCATION_TOKEN, token);
264285
tokenRevokeParams.set(OidcConstants.REVOCATION_TOKEN_TYPE_HINT, tokenTypeHint);
265286

266-
return getHttpResponse(requestProps, metadata.getRevocationUri(), tokenRevokeParams, false)
287+
return getHttpResponse(requestProps, metadata.getRevocationUri(), tokenRevokeParams, TokenOperation.REVOKE,
288+
OidcEndpoint.Type.TOKEN_REVOCATION)
267289
.transform(resp -> toRevokeResponse(requestProps, resp));
268290
} else {
269291
LOG.debugf("The %s token can not be revoked because the revocation endpoint URL is not set", tokenTypeHint);
@@ -282,7 +304,7 @@ private Boolean toRevokeResponse(OidcRequestContextProperties requestProps, Http
282304
}
283305

284306
private UniOnItem<HttpResponse<Buffer>> getHttpResponse(OidcRequestContextProperties requestProps, String uri,
285-
MultiMap formBody, boolean introspect) {
307+
MultiMap formBody, TokenOperation op, OidcEndpoint.Type endpointType) {
286308
HttpRequest<Buffer> request = client.postAbs(uri);
287309

288310
Buffer buffer = null;
@@ -291,7 +313,7 @@ private UniOnItem<HttpResponse<Buffer>> getHttpResponse(OidcRequestContextProper
291313
request.putHeader(CONTENT_TYPE_HEADER, APPLICATION_X_WWW_FORM_URLENCODED);
292314
request.putHeader(ACCEPT_HEADER, APPLICATION_JSON);
293315

294-
if (introspect && introspectionBasicAuthScheme != null) {
316+
if (isIntrospection(op) && introspectionBasicAuthScheme != null) {
295317
request.putHeader(AUTHORIZATION_HEADER, introspectionBasicAuthScheme);
296318
if (oidcConfig.clientId().isPresent() && oidcConfig.introspectionCredentials().includeClientId()) {
297319
formBody.set(OidcConstants.CLIENT_ID, oidcConfig.clientId().get());
@@ -339,14 +361,12 @@ private UniOnItem<HttpResponse<Buffer>> getHttpResponse(OidcRequestContextProper
339361
}
340362
}
341363
if (LOG.isDebugEnabled()) {
342-
LOG.debugf("%s token: %s params: %s headers: %s", (introspect ? "Introspect" : "Get"), metadata.getTokenUri(),
343-
formBody,
344-
request.headers());
364+
LOG.debugf("%s token: url : %s, headers: %s, request params: %s", op.operation(), request.uri(), request.headers(),
365+
formBody);
345366
}
346367
// Retry up to three times with a one-second delay between the retries if the connection is closed.
347368

348-
OidcEndpoint.Type endpoint = introspect ? OidcEndpoint.Type.INTROSPECTION : OidcEndpoint.Type.TOKEN;
349-
Uni<HttpResponse<Buffer>> response = filterHttpRequest(requestProps, endpoint, request, buffer)
369+
Uni<HttpResponse<Buffer>> response = filterHttpRequest(requestProps, endpointType, request, buffer)
350370
.sendBuffer(OidcCommonUtils.getRequestBuffer(requestProps, buffer))
351371
.onFailure(SocketException.class)
352372
.retry()
@@ -475,4 +495,7 @@ public WebClient getWebClient() {
475495
record UserInfoResponse(String contentType, String data) {
476496
}
477497

498+
static boolean isIntrospection(TokenOperation op) {
499+
return op == TokenOperation.INTROSPECT;
500+
}
478501
}

‎integration-tests/oidc-wiremock-logout/src/main/java/io/quarkus/it/keycloak/OidcTokenRevocationRequestFilter.java‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
package io.quarkus.it.keycloak;
2+
3+
import jakarta.enterprise.context.ApplicationScoped;
4+
5+
import io.quarkus.arc.Unremovable;
6+
import io.quarkus.oidc.common.OidcEndpoint;
7+
import io.quarkus.oidc.common.OidcEndpoint.Type;
8+
import io.quarkus.oidc.common.OidcRequestFilter;
9+
10+
@ApplicationScoped
11+
@Unremovable
12+
@OidcEndpoint(value = { Type.TOKEN_REVOCATION })
13+
public class OidcTokenRevocationRequestFilter implements OidcRequestFilter {
14+
15+
@Override
16+
public void filter(OidcRequestContext rc) {
17+
String body = rc.requestBody().toString();
18+
if (body.contains("token_type_hint=access_token")) {
19+
rc.request().putHeader("token-revocation-filter", "access-token");
20+
} else if (body.contains("token_type_hint=refresh_token")) {
21+
rc.request().putHeader("token-revocation-filter", "refresh-token");
22+
}
23+
24+
}
25+
}

‎integration-tests/oidc-wiremock-logout/src/test/java/io/quarkus/it/keycloak/CodeFlowAuthorizationTest.java‎

Lines changed: 12 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,14 @@
11
package io.quarkus.it.keycloak;
22

3-
import static com.github.tomakehurst.wiremock.client.WireMock.containing;
3+
import static com.github.tomakehurst.wiremock.client.WireMock.equalTo;
44
import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor;
55
import static com.github.tomakehurst.wiremock.client.WireMock.urlPathMatching;
6-
import static org.awaitility.Awaitility.await;
76
import static org.junit.jupiter.api.Assertions.assertEquals;
87
import static org.junit.jupiter.api.Assertions.assertNotNull;
98
import static org.junit.jupiter.api.Assertions.assertNull;
109
import static org.junit.jupiter.api.Assertions.assertTrue;
1110

12-
import java.io.IOException;
1311
import java.net.URI;
14-
import java.time.Duration;
15-
import java.util.concurrent.Callable;
16-
import java.util.concurrent.TimeUnit;
1712

1813
import org.htmlunit.SilentCssErrorHandler;
1914
import org.htmlunit.TextPage;
@@ -26,7 +21,6 @@
2621
import org.junit.jupiter.api.Test;
2722

2823
import com.github.tomakehurst.wiremock.WireMockServer;
29-
import com.github.tomakehurst.wiremock.client.WireMock;
3024

3125
import io.quarkus.test.common.QuarkusTestResource;
3226
import io.quarkus.test.junit.QuarkusTest;
@@ -48,13 +42,12 @@ public void testCodeFlowFormPostAndBackChannelLogout() throws Exception {
4842
}
4943

5044
@Test
51-
public void testBackChannelLogoutForDynamicTenants() throws IOException {
45+
public void testBackChannelLogoutForDynamicTenants() throws Exception {
5246
testCodeFlowFormPostAndBackChannelLogout("dynamic-tenant-one", "back-channel-logout/dynamic-tenant-one");
5347
testCodeFlowFormPostAndBackChannelLogout("dynamic-tenant-two", "back-channel-logout/dynamic-tenant-two");
5448
}
5549

56-
private void testCodeFlowFormPostAndBackChannelLogout(String tenantId, String backChannelPath) throws IOException {
57-
defineRevokeTokenStubs();
50+
private void testCodeFlowFormPostAndBackChannelLogout(String tenantId, String backChannelPath) throws Exception {
5851
try (final WebClient webClient = createWebClient()) {
5952
webClient.getOptions().setRedirectEnabled(true);
6053
HtmlPage page = webClient.getPage("http://localhost:8081/service/" + tenantId);
@@ -111,44 +104,21 @@ private void testCodeFlowFormPostAndBackChannelLogout(String tenantId, String ba
111104

112105
assertNull(getSessionCookie(webClient, tenantId));
113106

114-
await().atMost(10, TimeUnit.SECONDS)
115-
.pollInterval(Duration.ofSeconds(3))
116-
.until(new Callable<Boolean>() {
117-
@Override
118-
public Boolean call() throws Exception {
119-
try {
120-
wireMockServer.verify(2,
121-
postRequestedFor(urlPathMatching("/auth/realms/quarkus/revoke")));
122-
return true;
123-
} catch (Throwable t) {
124-
return false;
125-
}
126-
}
127-
});
128-
129-
wireMockServer.verify(2,
130-
postRequestedFor(urlPathMatching("/auth/realms/quarkus/revoke")));
107+
Thread.sleep(3000);
108+
109+
wireMockServer.verify(1,
110+
postRequestedFor(urlPathMatching("/auth/realms/quarkus/revoke"))
111+
.withHeader("token-revocation-filter", equalTo("access-token")));
112+
wireMockServer.verify(1,
113+
postRequestedFor(urlPathMatching("/auth/realms/quarkus/revoke"))
114+
.withHeader("token-revocation-filter", equalTo("refresh-token")));
115+
131116
wireMockServer.resetRequests();
132117

133118
webClient.getCookieManager().clearCookies();
134119
}
135120
}
136121

137-
private void defineRevokeTokenStubs() {
138-
wireMockServer
139-
.stubFor(WireMock.post("/auth/realms/quarkus/revoke")
140-
.withRequestBody(containing("token"))
141-
.withRequestBody(containing("token_type_hint=access_token"))
142-
.willReturn(WireMock.aResponse()
143-
.withStatus(200)));
144-
wireMockServer
145-
.stubFor(WireMock.post("/auth/realms/quarkus/revoke")
146-
.withRequestBody(containing("token"))
147-
.withRequestBody(containing("token_type_hint=refresh_token"))
148-
.willReturn(WireMock.aResponse()
149-
.withStatus(200)));
150-
}
151-
152122
private WebClient createWebClient() {
153123
WebClient webClient = new WebClient();
154124
webClient.setCssErrorHandler(new SilentCssErrorHandler());

0 commit comments

Comments
 (0)