[KEYCLOAK-12792] - Invalid nonce handling in OIDC identity brokering
This commit is contained in:
parent
199e5dfa3e
commit
fc514aa256
4 changed files with 148 additions and 7 deletions
|
@ -335,13 +335,6 @@ public abstract class AbstractOAuth2IdentityProvider<C extends OAuth2IdentityPro
|
|||
uriBuilder.queryParam(OAuth2Constants.PROMPT, prompt);
|
||||
}
|
||||
|
||||
String nonce = request.getAuthenticationSession().getClientNote(OIDCLoginProtocol.NONCE_PARAM);
|
||||
if (nonce == null || nonce.isEmpty()) {
|
||||
nonce = UUID.randomUUID().toString();
|
||||
request.getAuthenticationSession().setClientNote(OIDCLoginProtocol.NONCE_PARAM, nonce);
|
||||
}
|
||||
uriBuilder.queryParam(OIDCLoginProtocol.NONCE_PARAM, nonce);
|
||||
|
||||
String acr = request.getAuthenticationSession().getClientNote(OAuth2Constants.ACR_VALUES);
|
||||
if (acr != null) {
|
||||
uriBuilder.queryParam(OAuth2Constants.ACR_VALUES, acr);
|
||||
|
|
|
@ -21,10 +21,12 @@ import org.jboss.logging.Logger;
|
|||
import org.keycloak.OAuth2Constants;
|
||||
import org.keycloak.OAuthErrorException;
|
||||
import org.keycloak.broker.oidc.mappers.AbstractJsonUserAttributeMapper;
|
||||
import org.keycloak.broker.provider.AuthenticationRequest;
|
||||
import org.keycloak.broker.provider.BrokeredIdentityContext;
|
||||
import org.keycloak.broker.provider.ExchangeExternalToken;
|
||||
import org.keycloak.broker.provider.IdentityBrokerException;
|
||||
import org.keycloak.broker.provider.util.SimpleHttp;
|
||||
import org.keycloak.common.util.Base64Url;
|
||||
import org.keycloak.common.util.Time;
|
||||
import org.keycloak.events.Details;
|
||||
import org.keycloak.events.Errors;
|
||||
|
@ -40,6 +42,7 @@ import org.keycloak.models.KeycloakSession;
|
|||
import org.keycloak.models.RealmModel;
|
||||
import org.keycloak.models.UserModel;
|
||||
import org.keycloak.models.UserSessionModel;
|
||||
import org.keycloak.models.utils.KeycloakModelUtils;
|
||||
import org.keycloak.protocol.oidc.OIDCLoginProtocol;
|
||||
import org.keycloak.representations.AccessTokenResponse;
|
||||
import org.keycloak.representations.IDToken;
|
||||
|
@ -65,6 +68,7 @@ import javax.ws.rs.core.UriBuilder;
|
|||
import javax.ws.rs.core.UriInfo;
|
||||
import java.io.IOException;
|
||||
import java.security.PublicKey;
|
||||
import java.util.UUID;
|
||||
|
||||
/**
|
||||
* @author Pedro Igor
|
||||
|
@ -79,6 +83,7 @@ public class OIDCIdentityProvider extends AbstractOAuth2IdentityProvider<OIDCIde
|
|||
public static final String VALIDATED_ID_TOKEN = "VALIDATED_ID_TOKEN";
|
||||
public static final String ACCESS_TOKEN_EXPIRATION = "accessTokenExpiration";
|
||||
public static final String EXCHANGE_PROVIDER = "EXCHANGE_PROVIDER";
|
||||
private static final String BROKER_NONCE_PARAM = "BROKER_NONCE";
|
||||
|
||||
public OIDCIdentityProvider(KeycloakSession session, OIDCIdentityProviderConfig config) {
|
||||
super(session, config);
|
||||
|
@ -361,6 +366,8 @@ public class OIDCIdentityProvider extends AbstractOAuth2IdentityProvider<OIDCIde
|
|||
try {
|
||||
BrokeredIdentityContext identity = extractIdentity(tokenResponse, accessToken, idToken);
|
||||
|
||||
identity.getContextData().put(BROKER_NONCE_PARAM, idToken.getOtherClaims().get(OIDCLoginProtocol.NONCE_PARAM));
|
||||
|
||||
if (getConfig().isStoreToken()) {
|
||||
if (tokenResponse.getExpiresIn() > 0) {
|
||||
long accessTokenExpiration = Time.currentTime() + tokenResponse.getExpiresIn();
|
||||
|
@ -453,6 +460,7 @@ public class OIDCIdentityProvider extends AbstractOAuth2IdentityProvider<OIDCIde
|
|||
}
|
||||
if (tokenResponse != null) identity.getContextData().put(FEDERATED_ACCESS_TOKEN_RESPONSE, tokenResponse);
|
||||
if (tokenResponse != null) processAccessTokenResponse(identity, tokenResponse);
|
||||
|
||||
return identity;
|
||||
}
|
||||
|
||||
|
@ -715,4 +723,38 @@ public class OIDCIdentityProvider extends AbstractOAuth2IdentityProvider<OIDCIde
|
|||
throw new ErrorResponseException(OAuthErrorException.INVALID_TOKEN, "invalid token type", Response.Status.BAD_REQUEST);
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
protected UriBuilder createAuthorizationUrl(AuthenticationRequest request) {
|
||||
UriBuilder uriBuilder = super.createAuthorizationUrl(request);
|
||||
String nonce = Base64Url.encode(KeycloakModelUtils.generateSecret(16));
|
||||
AuthenticationSessionModel authenticationSession = request.getAuthenticationSession();
|
||||
|
||||
authenticationSession.setClientNote(BROKER_NONCE_PARAM, nonce);
|
||||
uriBuilder.queryParam(OIDCLoginProtocol.NONCE_PARAM, nonce);
|
||||
|
||||
return uriBuilder;
|
||||
}
|
||||
|
||||
@Override
|
||||
public void preprocessFederatedIdentity(KeycloakSession session, RealmModel realm, BrokeredIdentityContext context) {
|
||||
AuthenticationSessionModel authenticationSession = session.getContext().getAuthenticationSession();
|
||||
|
||||
if (authenticationSession == null) {
|
||||
// no interacting with the brokered OP, likely doing token exchanges
|
||||
return;
|
||||
}
|
||||
|
||||
String nonce = (String) context.getContextData().get(BROKER_NONCE_PARAM);
|
||||
|
||||
if (nonce == null) {
|
||||
throw new IdentityBrokerException("OpenID Provider [" + getConfig().getProviderId() + "] did not return a nonce");
|
||||
}
|
||||
|
||||
String expectedNonce = authenticationSession.getClientNote(BROKER_NONCE_PARAM);
|
||||
|
||||
if (!nonce.equals(expectedNonce)) {
|
||||
throw new ErrorResponseException(OAuthErrorException.INVALID_TOKEN, "invalid nonce", Response.Status.BAD_REQUEST);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -70,6 +70,7 @@ import org.keycloak.util.JsonSerialization;
|
|||
import org.keycloak.util.TokenUtil;
|
||||
import org.openqa.selenium.By;
|
||||
import org.openqa.selenium.WebDriver;
|
||||
import org.openqa.selenium.WebElement;
|
||||
|
||||
import javax.ws.rs.client.Entity;
|
||||
import javax.ws.rs.core.Form;
|
||||
|
@ -90,6 +91,7 @@ import java.util.Map;
|
|||
import java.util.function.Supplier;
|
||||
|
||||
import static org.keycloak.testsuite.admin.Users.getPasswordOf;
|
||||
import static org.keycloak.testsuite.util.UIUtils.clickLink;
|
||||
import static org.keycloak.testsuite.util.URLUtils.removeDefaultPorts;
|
||||
|
||||
/**
|
||||
|
@ -239,6 +241,17 @@ public class OAuthClient {
|
|||
return new AuthorizationEndpointResponse(this);
|
||||
}
|
||||
|
||||
public AuthorizationEndpointResponse doLoginSocial(String brokerId, String username, String password) {
|
||||
openLoginForm();
|
||||
WaitUtils.waitForPageToLoad();
|
||||
|
||||
WebElement socialButton = findSocialButton(brokerId);
|
||||
clickLink(socialButton);
|
||||
fillLoginForm(username, password);
|
||||
|
||||
return new AuthorizationEndpointResponse(this);
|
||||
}
|
||||
|
||||
public AuthorizationEndpointResponse doLogin(UserRepresentation user) {
|
||||
|
||||
return doLogin(user.getUsername(), getPasswordOf(user));
|
||||
|
@ -1326,6 +1339,11 @@ public class OAuthClient {
|
|||
return driver;
|
||||
}
|
||||
|
||||
private WebElement findSocialButton(String providerId) {
|
||||
String id = "zocial-" + providerId;
|
||||
return DroneUtils.getCurrentDriver().findElement(By.id(id));
|
||||
}
|
||||
|
||||
private interface StateParamProvider {
|
||||
|
||||
String getState();
|
||||
|
|
|
@ -0,0 +1,88 @@
|
|||
package org.keycloak.testsuite.broker;
|
||||
|
||||
import static org.keycloak.testsuite.broker.BrokerTestTools.waitForPage;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
|
||||
import org.junit.Assert;
|
||||
import org.junit.Ignore;
|
||||
import org.junit.Test;
|
||||
import org.keycloak.OAuth2Constants;
|
||||
import org.keycloak.admin.client.resource.UsersResource;
|
||||
import org.keycloak.jose.jws.JWSInput;
|
||||
import org.keycloak.jose.jws.JWSInputException;
|
||||
import org.keycloak.representations.AccessToken;
|
||||
import org.keycloak.representations.IDToken;
|
||||
import org.keycloak.representations.idm.ClientRepresentation;
|
||||
import org.keycloak.representations.idm.UserRepresentation;
|
||||
import org.keycloak.services.managers.AuthenticationManager;
|
||||
import org.keycloak.testsuite.arquillian.SuiteContext;
|
||||
import org.keycloak.testsuite.util.ClientBuilder;
|
||||
import org.keycloak.testsuite.util.OAuthClient;
|
||||
import org.openqa.selenium.Cookie;
|
||||
|
||||
public class KcOidcBrokerNonceParameterTest extends AbstractBrokerTest {
|
||||
|
||||
@Override
|
||||
protected BrokerConfiguration getBrokerConfiguration() {
|
||||
return new KcOidcBrokerConfiguration() {
|
||||
@Override
|
||||
public List<ClientRepresentation> createConsumerClients(SuiteContext suiteContext) {
|
||||
List<ClientRepresentation> clients = new ArrayList<>(super.createConsumerClients(suiteContext));
|
||||
|
||||
clients.add(ClientBuilder.create().clientId("consumer-client")
|
||||
.publicClient()
|
||||
.redirectUris("http://localhost:8180/auth/realms/master/app/auth/*", "https://localhost:8543/auth/realms/master/app/auth/*")
|
||||
.publicClient().build());
|
||||
|
||||
return clients;
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
@Override
|
||||
protected void loginUser() {
|
||||
updateExecutions(AbstractBrokerTest::disableUpdateProfileOnFirstLogin);
|
||||
|
||||
oauth.realm(bc.consumerRealmName());
|
||||
oauth.clientId("consumer-client");
|
||||
oauth.nonce("123456");
|
||||
|
||||
OAuthClient.AuthorizationEndpointResponse authzResponse = oauth
|
||||
.doLoginSocial(bc.getIDPAlias(), bc.getUserLogin(), bc.getUserPassword());
|
||||
String code = authzResponse.getCode();
|
||||
OAuthClient.AccessTokenResponse response = oauth.doAccessTokenRequest(code, null);
|
||||
IDToken idToken = toIdToken(response.getIdToken());
|
||||
|
||||
Assert.assertEquals("123456", idToken.getNonce());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testNonceNotSet() {
|
||||
updateExecutions(AbstractBrokerTest::disableUpdateProfileOnFirstLogin);
|
||||
|
||||
oauth.realm(bc.consumerRealmName());
|
||||
oauth.clientId("consumer-client");
|
||||
oauth.nonce(null);
|
||||
|
||||
OAuthClient.AuthorizationEndpointResponse authzResponse = oauth
|
||||
.doLoginSocial(bc.getIDPAlias(), bc.getUserLogin(), bc.getUserPassword());
|
||||
String code = authzResponse.getCode();
|
||||
OAuthClient.AccessTokenResponse response = oauth.doAccessTokenRequest(code, null);
|
||||
IDToken idToken = toIdToken(response.getIdToken());
|
||||
|
||||
Assert.assertNull(idToken.getNonce());
|
||||
}
|
||||
|
||||
protected IDToken toIdToken(String encoded) {
|
||||
IDToken idToken;
|
||||
|
||||
try {
|
||||
idToken = new JWSInput(encoded).readJsonContent(IDToken.class);
|
||||
} catch (JWSInputException cause) {
|
||||
throw new RuntimeException("Failed to deserialize RPT", cause);
|
||||
}
|
||||
return idToken;
|
||||
}
|
||||
}
|
Loading…
Reference in a new issue