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.

Sign up for GitHub

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

Do not make OIDC state cookie name unique if multiple code flows are not allowed #34784

Merged
merged 1 commit into from
Jul 17, 2023
Merged

Do not make OIDC state cookie name unique if multiple code flows are not allowed #34784

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 5 additions & 2 deletions ...sions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationMechanism.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,10 @@ private Uni<SecurityIdentity> processRedirectFromOidc(RoutingContext context, Oi
return stateParamIsMissing(oidcTenantConfig, context, cookies, stateQueryParam.size() > 1);
}

String stateCookieNameSuffix = oidcTenantConfig.authentication.allowMultipleCodeFlows ? "_" + stateQueryParam.get(0)
: "";
final Cookie stateCookie = context.request().getCookie(
getStateCookieName(oidcTenantConfig) + "_" + stateQueryParam.get(0));
getStateCookieName(oidcTenantConfig) + stateCookieNameSuffix);

if (stateCookie == null) {
return stateCookieIsMissing(oidcTenantConfig, context, cookies);
Expand Down Expand Up @@ -971,8 +973,9 @@ private String generateCodeFlowState(RoutingContext context, TenantConfigContext
extraStateValue.setRestorePath("?" + context.request().query());
cookieValue += (COOKIE_DELIM + encodeExtraStateValue(extraStateValue, configContext));
}
String stateCookieNameSuffix = configContext.oidcConfig.authentication.allowMultipleCodeFlows ? "_" + uuid : "";
createCookie(context, configContext.oidcConfig,
getStateCookieName(configContext.oidcConfig) + "_" + uuid, cookieValue, 60 * 30);
getStateCookieName(configContext.oidcConfig) + stateCookieNameSuffix, cookieValue, 60 * 30);
return uuid;
}

Expand Down
1 change: 1 addition & 0 deletions integration-tests/oidc-code-flow/src/main/resources/application.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ quarkus.oidc.tenant-jwt.client-id=quarkus-app-jwt
quarkus.oidc.tenant-jwt.credentials.jwt.secret=AyM1SysPpbyDfgZld3umj1qzKObwVMkoqQ-EstJQLr_T-1qS0gZH75aKtMN3Yj0iPS4hcgUuTwjAzZr1Z9CAow
quarkus.oidc.tenant-jwt.token.issuer=${quarkus.oidc.auth-server-url}
quarkus.oidc.tenant-jwt.authentication.redirect-path=/web-app/callback-jwt-after-redirect
quarkus.oidc.tenant-jwt.authentication.allow-multiple-code-flows=false
quarkus.oidc.tenant-jwt.application-type=web-app

# Tenant with client which needs to use client_secret_jwt but uses client_secret_post
Expand Down
22 changes: 19 additions & 3 deletions integration-tests/oidc-code-flow/src/test/java/io/quarkus/it/keycloak/CodeFlowTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ public void testCodeFlowNoConsent() throws IOException {

Cookie stateCookie = getStateCookie(webClient, null);
assertNotNull(stateCookie);
assertEquals(stateCookie.getName(), "q_auth_Default_test_" + getStateCookieStateParam(stateCookie));
assertNull(stateCookie.getSameSite());

webClient.getCookieManager().clearCookies();
Expand Down Expand Up @@ -673,9 +674,10 @@ public void testIdTokenInjectionJwtMethod() throws IOException, InterruptedExcep
WebResponse webResponse = webClient
.loadWebResponse(
new WebRequest(URI.create("http://localhost:8081/web-app/callback-jwt-before-redirect").toURL()));
assertNotNull(getStateCookie(webClient, "tenant-jwt"));
assertNotNull(getStateCookieStateParam(webClient, "tenant-jwt"));
assertNull(getStateCookieSavedPath(webClient, "tenant-jwt"));
Cookie stateCookie = getNonUniqueStateCookie(webClient, "tenant-jwt");
assertEquals(stateCookie.getName(), "q_auth_tenant-jwt");
assertNotNull(getStateCookieStateParam(stateCookie));
assertNull(getStateCookieSavedPath(stateCookie));

HtmlPage page = webClient.getPage(webResponse.getResponseHeaderValue("location"));
assertEquals("Sign in to quarkus", page.getTitleText());
Expand Down Expand Up @@ -1265,15 +1267,29 @@ private Cookie getStateCookie(WebClient webClient, String tenantId) {
return null;
}

private Cookie getNonUniqueStateCookie(WebClient webClient, String tenantId) {
String cookieName = "q_auth" + (tenantId == null ? "_Default_test" : "_" + tenantId);
return webClient.getCookieManager().getCookie(cookieName);
}

private String getStateCookieStateParam(WebClient webClient, String tenantId) {
return getStateCookie(webClient, tenantId).getValue().split("\\|")[0];
}

private String getStateCookieStateParam(Cookie stateCookie) {
return stateCookie.getValue().split("\\|")[0];
}

private String getStateCookieSavedPath(WebClient webClient, String tenantId) {
String[] parts = getStateCookie(webClient, tenantId).getValue().split("\\|");
return parts.length == 2 ? parts[1] : null;
}

private String getStateCookieSavedPath(Cookie stateCookie) {
String[] parts = stateCookie.getValue().split("\\|");
return parts.length == 2 ? parts[1] : null;
}

private Cookie getSessionCookie(WebClient webClient, String tenantId) {
return webClient.getCookieManager().getCookie("q_session" + (tenantId == null ? "_Default_test" : "_" + tenantId));
}
Expand Down