Skip to content

[ELY-2903] skip nonce check on oidc client refresh. Check nonce on oi… #2277

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

Closed
rsearls wants to merge 1 commit into from
Closed

[ELY-2903] skip nonce check on oidc client refresh. Check nonce on oi… #2277

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
3 changes: 1 addition & 2 deletions http/oidc/src/main/java/org/wildfly/security/http/oidc/OidcCookieTokenStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import static org.wildfly.security.http.oidc.ElytronMessages.log;
import static org.wildfly.security.http.oidc.Oidc.OIDC_STATE_COOKIE;
import static org.wildfly.security.http.oidc.Oidc.checkCachedAccountMatchesRequest;
import static org.wildfly.security.http.oidc.Oidc.SESSION_RANDOM_VALUE;

import java.net.URISyntaxException;
import java.util.List;
Expand Down Expand Up @@ -228,7 +227,7 @@ public static OidcPrincipal<RefreshableOidcSecurityContext> getPrincipalFromCook
idToken = new IDToken(new JwtConsumerBuilder().setSkipSignatureVerification().setSkipAllValidators().build().processToClaims(idTokenString));
}
log.debug("Token obtained from cookie");
RefreshableOidcSecurityContext secContext = new RefreshableOidcSecurityContext(deployment, facade.getRequest().getCookie(SESSION_RANDOM_VALUE), tokenStore, accessTokenString, accessToken, idTokenString, idToken, refreshTokenString);
RefreshableOidcSecurityContext secContext = new RefreshableOidcSecurityContext(deployment, tokenStore, accessTokenString, accessToken, idTokenString, idToken, refreshTokenString);
return new OidcPrincipal<>(idToken.getPrincipalName(deployment), secContext);
} catch (InvalidJwtException e) {
log.failedToParseTokenFromCookie(e);
Expand Down
2 changes: 1 addition & 1 deletion http/oidc/src/main/java/org/wildfly/security/http/oidc/OidcRequestAuthenticator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ protected AuthChallenge resolveCode(String code) {
TokenValidator tokenValidator = TokenValidator.builder(deployment).build();

TokenValidator.VerifiedTokens verifiedTokens = tokenValidator.parseAndVerifyToken(idTokenString, tokenString,
facade.getRequest().getCookie(SESSION_RANDOM_VALUE));
facade.getRequest().getCookie(SESSION_RANDOM_VALUE), true);
idToken = verifiedTokens.getIdToken();
token = verifiedTokens.getAccessToken();

Expand Down
6 changes: 2 additions & 4 deletions http/oidc/src/main/java/org/wildfly/security/http/oidc/RefreshableOidcSecurityContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,16 @@ public class RefreshableOidcSecurityContext extends OidcSecurityContext {
protected transient OidcClientConfiguration clientConfiguration;
protected transient OidcTokenStore tokenStore;
protected String refreshToken;
protected transient OidcHttpFacade.Cookie cookie;

public RefreshableOidcSecurityContext() {
}

public RefreshableOidcSecurityContext(OidcClientConfiguration clientConfiguration, OidcHttpFacade.Cookie cookie, OidcTokenStore tokenStore, String tokenString,
public RefreshableOidcSecurityContext(OidcClientConfiguration clientConfiguration, OidcTokenStore tokenStore, String tokenString,
Copy link
Copy Markdown
Contributor

@fjuma fjuma Apr 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just an FYI for other reviewers, this isn't public API.

AccessToken token, String idTokenString, IDToken idToken, String refreshToken) {
super(tokenString, token, idTokenString, idToken);
this.clientConfiguration = clientConfiguration;
this.tokenStore = tokenStore;
this.refreshToken = refreshToken;
this.cookie = cookie;
}

@Override
Expand Down Expand Up @@ -151,7 +149,7 @@ public boolean refreshToken(boolean checkActive) {
IDToken idToken;
try {
TokenValidator tokenValidator = TokenValidator.builder(clientConfiguration).build();
TokenValidator.VerifiedTokens verifiedTokens = tokenValidator.parseAndVerifyToken(idTokenString, accessTokenString, cookie);
TokenValidator.VerifiedTokens verifiedTokens = tokenValidator.parseAndVerifyToken(idTokenString, accessTokenString);
idToken = verifiedTokens.getIdToken();
accessToken = verifiedTokens.getAccessToken();
log.debug("Token Verification succeeded!");
Expand Down
5 changes: 2 additions & 3 deletions http/oidc/src/main/java/org/wildfly/security/http/oidc/RequestAuthenticator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import static org.wildfly.security.http.oidc.Oidc.FACES_REQUEST;
import static org.wildfly.security.http.oidc.Oidc.HTML_CONTENT_TYPE;
import static org.wildfly.security.http.oidc.Oidc.PARTIAL;
import static org.wildfly.security.http.oidc.Oidc.SESSION_RANDOM_VALUE;
import static org.wildfly.security.http.oidc.Oidc.SOAP_ACTION;
import static org.wildfly.security.http.oidc.Oidc.TEXT_CONTENT_TYPE;
import static org.wildfly.security.http.oidc.Oidc.WILDCARD_CONTENT_TYPE;
Expand Down Expand Up @@ -201,14 +200,14 @@ protected boolean verifySSL() {
}

protected void completeAuthentication(OidcRequestAuthenticator oidc) {
RefreshableOidcSecurityContext session = new RefreshableOidcSecurityContext(deployment, facade.getRequest().getCookie(SESSION_RANDOM_VALUE), facade.getTokenStore(), oidc.getTokenString(), oidc.getToken(), oidc.getIDTokenString(), oidc.getIDToken(), oidc.getRefreshToken());
RefreshableOidcSecurityContext session = new RefreshableOidcSecurityContext(deployment, facade.getTokenStore(), oidc.getTokenString(), oidc.getToken(), oidc.getIDTokenString(), oidc.getIDToken(), oidc.getRefreshToken());
final OidcPrincipal<RefreshableOidcSecurityContext> principal = new OidcPrincipal<>(oidc.getIDToken().getPrincipalName(deployment), session);
completeOidcAuthentication(principal);
log.debugv("User ''{0}'' invoking ''{1}'' on client ''{2}''", principal.getName(), facade.getRequest().getURI(), deployment.getResourceName());
}

protected void completeAuthentication(BearerTokenRequestAuthenticator bearer) {
RefreshableOidcSecurityContext session = new RefreshableOidcSecurityContext(deployment, facade.getRequest().getCookie(SESSION_RANDOM_VALUE), null, bearer.getTokenString(), bearer.getToken(), null, null, null);
RefreshableOidcSecurityContext session = new RefreshableOidcSecurityContext(deployment, null, bearer.getTokenString(), bearer.getToken(), null, null, null);
final OidcPrincipal<RefreshableOidcSecurityContext> principal = new OidcPrincipal<>(bearer.getToken().getPrincipalName(deployment), session);
completeBearerAuthentication(principal);
log.debugv("User ''{0}'' invoking ''{1}'' on client ''{2}''", principal.getName(), facade.getRequest().getURI(), deployment.getResourceName());
Expand Down
11 changes: 9 additions & 2 deletions http/oidc/src/main/java/org/wildfly/security/http/oidc/TokenValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,20 +76,27 @@ private TokenValidator(Builder builder) {
this.clientConfiguration = builder.clientConfiguration;
}

public VerifiedTokens parseAndVerifyToken(final String idToken, final String accessToken) throws OidcException {
return parseAndVerifyToken(idToken, accessToken, null, false);
}

/**
* Parse and verify the given ID token.
*
* @param idToken the ID token
* @return the {@code VerifiedTokens} if the ID token was valid
* @throws OidcException if the ID token is invalid
*/
public VerifiedTokens parseAndVerifyToken(final String idToken, final String accessToken, OidcHttpFacade.Cookie cookie) throws OidcException {
public VerifiedTokens parseAndVerifyToken(final String idToken, final String accessToken,
Comment thread
fjuma marked this conversation as resolved.
OidcHttpFacade.Cookie cookie, boolean isValidateNonce) throws OidcException {
try {
JwtContext idJwtContext = setVerificationKey(idToken, jwtConsumerBuilder);
jwtConsumerBuilder.setExpectedAudience(clientConfiguration.getResourceName());
jwtConsumerBuilder.registerValidator(new AzpValidator(clientConfiguration.getResourceName()));
jwtConsumerBuilder.registerValidator(new AtHashValidator(accessToken, clientConfiguration.getTokenSignatureAlgorithm()));
jwtConsumerBuilder.registerValidator(new NonceValidator(cookie));
if (isValidateNonce) {
jwtConsumerBuilder.registerValidator(new NonceValidator(cookie));
}

// second pass to validate
jwtConsumerBuilder.build().processContext(idJwtContext);
Expand Down
17 changes: 17 additions & 0 deletions http/oidc/src/test/java/org/wildfly/security/http/oidc/OidcBaseTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
import org.wildfly.security.credential.BearerTokenCredential;
import org.wildfly.security.credential.Credential;
import org.wildfly.security.evidence.Evidence;
import org.wildfly.security.http.HttpScope;
import org.wildfly.security.http.HttpServerAuthenticationMechanism;
import org.wildfly.security.http.HttpServerAuthenticationMechanismFactory;
import org.wildfly.security.http.HttpServerCookie;
Expand Down Expand Up @@ -189,6 +190,11 @@ protected CallbackHandler getCallbackHandler(boolean checkScope, String expected
}

protected static Dispatcher createAppResponse(HttpServerAuthenticationMechanism mechanism, int expectedStatusCode, String expectedLocation, String clientPageText) {
return createAppResponse(mechanism, expectedStatusCode, expectedLocation, clientPageText, false);
}

protected static Dispatcher createAppResponse(HttpServerAuthenticationMechanism mechanism, int expectedStatusCode,
String expectedLocation, String clientPageText, boolean isRefreshToken) {
return new Dispatcher() {
@Override
public MockResponse dispatch(RecordedRequest recordedRequest) throws InterruptedException {
Expand All @@ -201,6 +207,17 @@ public MockResponse dispatch(RecordedRequest recordedRequest) throws Interrupted
TestingHttpServerResponse response = request.getResponse();
assertEquals(expectedStatusCode, response.getStatusCode());
assertEquals(expectedLocation, response.getLocation());

if (isRefreshToken) {
HttpScope session = request.getScope(org.wildfly.security.http.Scope.SESSION);
assertNotNull("No session for token refresh", session);
OidcAccount account = (OidcAccount) session.getAttachment(OidcAccount.class.getName());
assertNotNull("No OidcAccount for token refresh", account);
RefreshableOidcSecurityContext refreshableContext = account.getOidcSecurityContext();
assertNotNull("No RefreshableOidcSecurityContext for token refresh", refreshableContext);
assertTrue("Token refresh failed", refreshableContext.refreshToken(false));
}

return new MockResponse().setBody(clientPageText);
} catch (Exception e) {
throw new RuntimeException(e);
Expand Down
12 changes: 6 additions & 6 deletions http/oidc/src/test/java/org/wildfly/security/http/oidc/OidcSecurityRealmTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public void testGetRealmIdentityWithNonOidcPrincipal() throws RealmUnavailableEx
@Test
public void testGetRealmIdentityNoRoles() throws RealmUnavailableException {
// setup
RefreshableOidcSecurityContext securityContext = new RefreshableOidcSecurityContext(new OidcClientConfiguration(), null,
RefreshableOidcSecurityContext securityContext = new RefreshableOidcSecurityContext(new OidcClientConfiguration(),
null, null, new AccessToken(new JwtClaims()), null, null, null);
OidcPrincipal principal = new OidcPrincipal("john", securityContext);

Expand Down Expand Up @@ -108,7 +108,7 @@ public void testGetRealmIdentityRolesCombined() throws RealmUnavailableException
jwtClaims.setClaim("resource_access", resourceAccess);
jwtClaims.setClaim("realm_access", createRoles("roleC", "roleD"));

RefreshableOidcSecurityContext securityContext = new RefreshableOidcSecurityContext(clientConfiguration,null,
RefreshableOidcSecurityContext securityContext = new RefreshableOidcSecurityContext(clientConfiguration,
null, null, new AccessToken(jwtClaims), null, null, null);
OidcPrincipal principal = new OidcPrincipal("john", securityContext);

Expand Down Expand Up @@ -137,7 +137,7 @@ public void testGetRealmIdentityOnlyRealmRoles() throws RealmUnavailableExceptio
jwtClaims.setClaim("resource_access", resourceAccess);
jwtClaims.setClaim("realm_access", createRoles("roleC", "roleD"));

RefreshableOidcSecurityContext securityContext = new RefreshableOidcSecurityContext(clientConfiguration,null,
RefreshableOidcSecurityContext securityContext = new RefreshableOidcSecurityContext(clientConfiguration,
null, null, new AccessToken(jwtClaims), null, null, null);
OidcPrincipal principal = new OidcPrincipal("john", securityContext);

Expand Down Expand Up @@ -165,7 +165,7 @@ public void testGetRealmIdentityOnlyResourceRoles() throws RealmUnavailableExcep
jwtClaims.setClaim("resource_access", resourceAccess);
jwtClaims.setClaim("", new RealmAccessClaim(createRoles("roleC", "roleD")));

RefreshableOidcSecurityContext securityContext = new RefreshableOidcSecurityContext(clientConfiguration,null,
RefreshableOidcSecurityContext securityContext = new RefreshableOidcSecurityContext(clientConfiguration,
null, null, new AccessToken(jwtClaims), null, null, null);
OidcPrincipal principal = new OidcPrincipal("john", securityContext);

Expand Down Expand Up @@ -193,7 +193,7 @@ public void testGetRealmIdentityNoMappings() throws RealmUnavailableException {
jwtClaims.setClaim("resource_access", resourceAccess);
jwtClaims.setClaim("", new RealmAccessClaim(createRoles("roleC", "roleD")));

RefreshableOidcSecurityContext securityContext = new RefreshableOidcSecurityContext(clientConfiguration,null,
RefreshableOidcSecurityContext securityContext = new RefreshableOidcSecurityContext(clientConfiguration,
null, null, new AccessToken(jwtClaims), null, null, null);
OidcPrincipal principal = new OidcPrincipal("john", securityContext);

Expand Down Expand Up @@ -390,7 +390,7 @@ private static String getRealmAndResourceRolesClaims(boolean includeRealmAndReso
}

private Attributes.Entry getRealmIdentityRoles(OidcClientConfiguration clientConfiguration, JwtClaims jwtClaims) throws RealmUnavailableException {
RefreshableOidcSecurityContext securityContext = new RefreshableOidcSecurityContext(clientConfiguration,null,
RefreshableOidcSecurityContext securityContext = new RefreshableOidcSecurityContext(clientConfiguration,
null, null, new AccessToken(jwtClaims), null, null, null);
OidcPrincipal principal = new OidcPrincipal("john", securityContext);

Expand Down
44 changes: 44 additions & 0 deletions http/oidc/src/test/java/org/wildfly/security/http/oidc/OidcTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,34 @@ public void testUnauthorizedAccessWithProviderUrlValidUser() throws Exception {
performTenantRequestWithProviderUrl(DAN, DAN_PASSWORD, TENANT2_ENDPOINT, TENANT1_ENDPOINT);
}

/**
* Check that a RefreshableOidcSecurityContext.tokenRefresh works now that
* a request nonce has been added to elytron.
*/
@Test
public void testRefreshToken() throws Exception {
Map<String, Object> props = new HashMap<>();
OidcClientConfiguration oidcClientConfiguration = OidcClientConfigurationBuilder.build(getOidcRefreshConfigurationInputStream());
OidcClientContext oidcClientContext = new OidcClientContext(oidcClientConfiguration);
oidcFactory = new OidcMechanismFactory(oidcClientContext);
HttpServerAuthenticationMechanism mechanism = oidcFactory.createAuthenticationMechanism(OIDC_NAME, props, getCallbackHandler());

URI requestUri = new URI(getClientUrl());
TestingHttpServerRequest request = new TestingHttpServerRequest(null, requestUri);
mechanism.evaluateRequest(request);
TestingHttpServerResponse response = request.getResponse();
assertEquals(HttpStatus.SC_MOVED_TEMPORARILY, response.getStatusCode());
assertEquals(Status.NO_AUTH, request.getResult());

client.setDispatcher(createAppResponse(mechanism, HttpStatus.SC_MOVED_TEMPORARILY,
getClientUrl(), CLIENT_PAGE_TEXT, true));

TextPage page = loginToKeycloak(KeycloakConfiguration.ALICE,
KeycloakConfiguration.ALICE_PASSWORD, requestUri, response.getLocation(),
response.getCookies()).click();
assertTrue(page.getContent().contains(CLIENT_PAGE_TEXT));
}

private void testNonExistingUserWithAuthServerUrl(String username, String password, String tenant) throws Exception {
testNonExistingUser(username, password, tenant, true);
}
Expand Down Expand Up @@ -909,5 +937,21 @@ static InputStream getTenantConfigWithProviderUrl(String tenant) {
private static final String getClientPageTestForTenant(String tenant) {
return tenant.equals(TENANT1_ENDPOINT) ? TENANT1_ENDPOINT : TENANT2_ENDPOINT + ":" + CLIENT_PAGE_TEXT;
}

private InputStream getOidcRefreshConfigurationInputStream() {
return getOidcRefreshConfigurationInputStream(CLIENT_SECRET, KEYCLOAK_CONTAINER.getAuthServerUrl());
}
private InputStream getOidcRefreshConfigurationInputStream(String clientSecret, String authServerUrl) {
String oidcConfig = "{\n" +
" \"" + Oidc.CLIENT_ID_JSON_VALUE + "\" : \"" + CLIENT_ID + "\",\n" +
" \"" + PUBLIC_CLIENT + "\" : \"false\",\n" +
" \"" + PROVIDER_URL + "\" : \"" + KEYCLOAK_CONTAINER.getAuthServerUrl() + "/realms/" + TEST_REALM + "\",\n" +
" \"" + SSL_REQUIRED + "\" : \"EXTERNAL\",\n" +
" \"" + CREDENTIALS + "\" : {\n" +
" \"" + ClientCredentialsProviderType.SECRET.getValue() + "\" : \"" + clientSecret + "\"\n" +
" }\n" +
"}";
return new ByteArrayInputStream(oidcConfig.getBytes(StandardCharsets.UTF_8));
}
}