mirror of
https://github.com/keycloak/keycloak.git
synced 2026-06-09 00:52:07 -04:00
closes #47719
closes CVE-2026-4282
closes CVE-2026-4325
(cherry picked from commit 9046f20112)
Signed-off-by: mposolda <mposolda@gmail.com>
This commit is contained in:
parent
3af5de75a8
commit
18dbc74960
8 changed files with 102 additions and 13 deletions
|
|
@ -21,6 +21,7 @@ import java.util.Base64;
|
|||
import java.util.UUID;
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
import org.keycloak.common.util.SecretGenerator;
|
||||
import org.keycloak.representations.JsonWebToken;
|
||||
|
||||
import com.fasterxml.jackson.annotation.JsonIgnore;
|
||||
|
|
@ -47,7 +48,7 @@ public class DefaultActionTokenKey extends JsonWebToken implements SingleUseObje
|
|||
this.subject = userId;
|
||||
this.type = actionId;
|
||||
this.exp = Long.valueOf(absoluteExpirationInSecs);
|
||||
this.actionVerificationNonce = actionVerificationNonce == null ? UUID.randomUUID() : actionVerificationNonce;
|
||||
this.actionVerificationNonce = actionVerificationNonce == null ? SecretGenerator.getInstance().generateSecureUUID() : actionVerificationNonce;
|
||||
}
|
||||
|
||||
@JsonIgnore
|
||||
|
|
|
|||
|
|
@ -20,7 +20,6 @@ package org.keycloak.protocol.oidc.par.endpoints;
|
|||
import java.util.HashMap;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.UUID;
|
||||
|
||||
import jakarta.ws.rs.Consumes;
|
||||
import jakarta.ws.rs.POST;
|
||||
|
|
@ -33,6 +32,7 @@ import jakarta.ws.rs.core.UriBuilder;
|
|||
|
||||
import org.keycloak.OAuthErrorException;
|
||||
import org.keycloak.common.Profile;
|
||||
import org.keycloak.common.util.SecretGenerator;
|
||||
import org.keycloak.events.Details;
|
||||
import org.keycloak.events.EventBuilder;
|
||||
import org.keycloak.events.EventType;
|
||||
|
|
@ -62,8 +62,9 @@ public class ParEndpoint extends AbstractParEndpoint {
|
|||
|
||||
public static final String PAR_CREATED_TIME = "par.created.time";
|
||||
public static final String PAR_DPOP_PROOF_JKT = "par.dpop.proof.jkt";
|
||||
private static final String REQUEST_URI_PREFIX = "urn:ietf:params:oauth:request_uri:";
|
||||
public static final String REQUEST_URI_PREFIX = "urn:ietf:params:oauth:request_uri:";
|
||||
public static final int REQUEST_URI_PREFIX_LENGTH = REQUEST_URI_PREFIX.length();
|
||||
public static final String CACHE_KEY_PREFIX = "par:";
|
||||
|
||||
private final HttpRequest httpRequest;
|
||||
|
||||
|
|
@ -165,7 +166,7 @@ public class ParEndpoint extends AbstractParEndpoint {
|
|||
|
||||
Map<String, String> params = new HashMap<>();
|
||||
|
||||
String key = UUID.randomUUID().toString();
|
||||
String key = SecretGenerator.getInstance().generateSecureID();
|
||||
String requestUri = REQUEST_URI_PREFIX + key;
|
||||
|
||||
int expiresIn = realm.getParPolicy().getRequestUriLifespan();
|
||||
|
|
@ -180,7 +181,7 @@ public class ParEndpoint extends AbstractParEndpoint {
|
|||
}
|
||||
|
||||
SingleUseObjectProvider singleUseStore = session.singleUseObjects();
|
||||
singleUseStore.put(key, expiresIn, params);
|
||||
singleUseStore.put(CACHE_KEY_PREFIX + key, expiresIn, params);
|
||||
|
||||
ParResponse parResponse = new ParResponse(requestUri, expiresIn);
|
||||
|
||||
|
|
|
|||
|
|
@ -32,6 +32,7 @@ import org.keycloak.protocol.oidc.par.endpoints.ParEndpoint;
|
|||
|
||||
import org.jboss.logging.Logger;
|
||||
|
||||
import static org.keycloak.protocol.oidc.par.endpoints.ParEndpoint.CACHE_KEY_PREFIX;
|
||||
import static org.keycloak.protocol.oidc.par.endpoints.ParEndpoint.PAR_CREATED_TIME;
|
||||
import static org.keycloak.protocol.oidc.par.endpoints.ParEndpoint.PAR_DPOP_PROOF_JKT;
|
||||
|
||||
|
|
@ -60,7 +61,7 @@ public class AuthzEndpointParParser extends AuthzEndpointRequestParser {
|
|||
logger.warnf(re,"Unable to parse request_uri: %s", requestUri);
|
||||
throw new RuntimeException("Unable to parse request_uri");
|
||||
}
|
||||
Map<String, String> retrievedRequest = singleUseStore.remove(key);
|
||||
Map<String, String> retrievedRequest = singleUseStore.remove(CACHE_KEY_PREFIX + key);
|
||||
if (retrievedRequest == null) {
|
||||
throw new RuntimeException("PAR not found. not issued or used multiple times.");
|
||||
}
|
||||
|
|
|
|||
|
|
@ -31,8 +31,8 @@ import java.util.Map;
|
|||
*/
|
||||
public class OAuth2Code {
|
||||
|
||||
private static final String ID_NOTE = "id";
|
||||
private static final String EXPIRATION_NOTE = "exp";
|
||||
public static final String ID_NOTE = "id";
|
||||
public static final String EXPIRATION_NOTE = "exp";
|
||||
private static final String NONCE_NOTE = "nonce";
|
||||
private static final String SCOPE_NOTE = "scope";
|
||||
private static final String RESOURCE_NOTE = "resource";
|
||||
|
|
@ -40,7 +40,7 @@ public class OAuth2Code {
|
|||
private static final String CODE_CHALLENGE_NOTE = "code_challenge";
|
||||
private static final String CODE_CHALLENGE_METHOD_NOTE = "code_challenge_method";
|
||||
private static final String DPOP_JKT_NOTE = "dpop_jkt";
|
||||
private static final String USER_SESSION_ID_NOTE = "user_session_id";
|
||||
public static final String USER_SESSION_ID_NOTE = "user_session_id";
|
||||
|
||||
private final String id;
|
||||
|
||||
|
|
|
|||
|
|
@ -39,6 +39,7 @@ public class OAuth2CodeParser {
|
|||
private static final Logger logger = Logger.getLogger(OAuth2CodeParser.class);
|
||||
|
||||
private static final Pattern DOT = Pattern.compile("\\.");
|
||||
private static final String CACHE_KEY_PREFIX = "code:";
|
||||
|
||||
/**
|
||||
* Will persist the code to the cache and return the object with the codeData and code correctly set
|
||||
|
|
@ -54,7 +55,7 @@ public class OAuth2CodeParser {
|
|||
}
|
||||
|
||||
Map<String, String> serialized = codeData.serializeCode();
|
||||
codeStore.put(key, clientSession.getUserSession().getRealm().getAccessCodeLifespan(), serialized);
|
||||
codeStore.put(CACHE_KEY_PREFIX + key, clientSession.getUserSession().getRealm().getAccessCodeLifespan(), serialized);
|
||||
return key + "." + clientSession.getUserSession().getId() + "." + clientSession.getClient().getId();
|
||||
}
|
||||
|
||||
|
|
@ -94,7 +95,7 @@ public class OAuth2CodeParser {
|
|||
result.clientSession = userSession.getAuthenticatedClientSessionByClient(clientUUID);
|
||||
|
||||
SingleUseObjectProvider codeStore = session.singleUseObjects();
|
||||
Map<String, String> codeData = codeStore.remove(codeUUID);
|
||||
Map<String, String> codeData = codeStore.remove(CACHE_KEY_PREFIX + codeUUID);
|
||||
|
||||
// Either code not available or was already used
|
||||
if (codeData == null) {
|
||||
|
|
|
|||
|
|
@ -85,7 +85,7 @@ public class SecureParContentsExecutor implements ClientPolicyExecutorProvider<C
|
|||
|
||||
String key = requestUri.substring(ParEndpoint.REQUEST_URI_PREFIX_LENGTH);
|
||||
SingleUseObjectProvider singleUseStore = session.singleUseObjects();
|
||||
Map<String, String> requestParametersFromPAR = singleUseStore.get(key);
|
||||
Map<String, String> requestParametersFromPAR = singleUseStore.get(ParEndpoint.CACHE_KEY_PREFIX + key);
|
||||
if (requestParametersFromPAR == null) {
|
||||
throw new ClientPolicyException(OAuthErrorException.INVALID_REQUEST, "PAR not found. not issued or used multiple times.");
|
||||
}
|
||||
|
|
@ -100,7 +100,7 @@ public class SecureParContentsExecutor implements ClientPolicyExecutorProvider<C
|
|||
|
||||
for (String queryParamName : requestParametersFromQuery.keySet()) {
|
||||
if (!requestParametersNameFromPAR.contains(queryParamName) && !OIDCLoginProtocol.REQUEST_URI_PARAM.equals(queryParamName)) {
|
||||
singleUseStore.remove(key);
|
||||
singleUseStore.remove(ParEndpoint.CACHE_KEY_PREFIX + key);
|
||||
throw new ClientPolicyException(OAuthErrorException.INVALID_REQUEST, "PAR request did not include necessary parameters");
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -26,6 +26,7 @@ import java.util.concurrent.atomic.AtomicInteger;
|
|||
import jakarta.mail.MessagingException;
|
||||
import jakarta.mail.internet.MimeMessage;
|
||||
|
||||
import org.keycloak.TokenVerifier;
|
||||
import org.keycloak.admin.client.resource.ClientResource;
|
||||
import org.keycloak.admin.client.resource.UserResource;
|
||||
import org.keycloak.authentication.actiontoken.resetcred.ResetCredentialsActionToken;
|
||||
|
|
@ -33,11 +34,14 @@ import org.keycloak.authentication.authenticators.resetcred.ResetCredentialEmail
|
|||
import org.keycloak.common.constants.ServiceAccountConstants;
|
||||
import org.keycloak.common.util.KeycloakUriBuilder;
|
||||
import org.keycloak.common.util.MultivaluedHashMap;
|
||||
import org.keycloak.common.util.UriUtils;
|
||||
import org.keycloak.events.Details;
|
||||
import org.keycloak.events.Errors;
|
||||
import org.keycloak.events.EventType;
|
||||
import org.keycloak.models.AuthenticationExecutionModel;
|
||||
import org.keycloak.models.Constants;
|
||||
import org.keycloak.models.DefaultActionTokenKey;
|
||||
import org.keycloak.models.SingleUseObjectProvider;
|
||||
import org.keycloak.models.credential.PasswordCredentialModel;
|
||||
import org.keycloak.models.utils.DefaultAuthenticationFlows;
|
||||
import org.keycloak.models.utils.SystemClientUtil;
|
||||
|
|
@ -98,6 +102,8 @@ import org.openqa.selenium.WebElement;
|
|||
import org.openqa.selenium.firefox.FirefoxDriver;
|
||||
import org.openqa.selenium.htmlunit.HtmlUnitDriver;
|
||||
|
||||
import static org.keycloak.protocol.oidc.par.endpoints.ParEndpoint.REQUEST_URI_PREFIX;
|
||||
|
||||
import static org.hamcrest.MatcherAssert.assertThat;
|
||||
import static org.hamcrest.Matchers.endsWith;
|
||||
import static org.hamcrest.Matchers.is;
|
||||
|
|
@ -350,6 +356,35 @@ public class ResetPasswordTest extends AbstractTestRealmKeycloakTest {
|
|||
assertSecondPasswordResetFails(changePasswordUrl, oauth.getClientId()); // KC_RESTART doesn't exist, it was deleted after first successful reset-password flow was finished
|
||||
}
|
||||
|
||||
@Test
|
||||
public void resetPasswordTwiceWithPARRequestInBetween() throws Exception {
|
||||
String changePasswordUrl = resetPassword("login-test");
|
||||
events.clear();
|
||||
|
||||
// Try to create manually the cache key from actionToken
|
||||
String queryString = changePasswordUrl.substring(changePasswordUrl.indexOf("?") + 1);
|
||||
MultivaluedHashMap<String, String> params = UriUtils.decodeQueryString(queryString);
|
||||
String key = params.getFirst(Constants.KEY);
|
||||
TokenVerifier<DefaultActionTokenKey> tokenVerifier = TokenVerifier.create(key, DefaultActionTokenKey.class);
|
||||
DefaultActionTokenKey aToken = tokenVerifier.getToken();
|
||||
String serializedKey1 = aToken.serializeKey();
|
||||
String serializedKey2 = serializedKey1 + SingleUseObjectProvider.REVOKED_KEY;
|
||||
|
||||
// Try to send PAR request with manually created requestUri related to the manually created serialized keys
|
||||
String state = "testSuccessfulSinglePar";
|
||||
oauth.loginForm()
|
||||
.requestUri(REQUEST_URI_PREFIX + serializedKey1)
|
||||
.state(state)
|
||||
.open();
|
||||
oauth.loginForm()
|
||||
.requestUri(REQUEST_URI_PREFIX + serializedKey2)
|
||||
.state(state)
|
||||
.open();
|
||||
events.clear();
|
||||
|
||||
assertSecondPasswordResetFails(changePasswordUrl, oauth.getClientId()); // KC_RESTART doesn't exist, it was deleted after first successful reset-password flow was finished
|
||||
}
|
||||
|
||||
@Test
|
||||
public void resetPasswordForceLogin() throws IOException {
|
||||
// add the force login option in the reset-credential-email authenticator
|
||||
|
|
|
|||
|
|
@ -38,6 +38,7 @@ import org.keycloak.models.utils.KeycloakModelUtils;
|
|||
import org.keycloak.protocol.oidc.OIDCAdvancedConfigWrapper;
|
||||
import org.keycloak.protocol.oidc.OIDCConfigAttributes;
|
||||
import org.keycloak.protocol.oidc.OIDCLoginProtocol;
|
||||
import org.keycloak.protocol.oidc.utils.OAuth2Code;
|
||||
import org.keycloak.representations.AccessToken;
|
||||
import org.keycloak.representations.IDToken;
|
||||
import org.keycloak.representations.RefreshToken;
|
||||
|
|
@ -54,6 +55,7 @@ import org.keycloak.testsuite.client.resources.TestApplicationResourceUrls;
|
|||
import org.keycloak.testsuite.client.resources.TestOIDCEndpointsApplicationResource;
|
||||
import org.keycloak.testsuite.rest.resource.TestingOIDCEndpointsApplicationResource;
|
||||
import org.keycloak.testsuite.services.clientpolicy.executor.TestRaiseExceptionExecutorFactory;
|
||||
import org.keycloak.testsuite.util.AccountHelper;
|
||||
import org.keycloak.testsuite.util.ClientBuilder;
|
||||
import org.keycloak.testsuite.util.ClientPoliciesUtil.ClientPoliciesBuilder;
|
||||
import org.keycloak.testsuite.util.ClientPoliciesUtil.ClientPolicyBuilder;
|
||||
|
|
@ -72,6 +74,7 @@ import org.keycloak.util.JsonSerialization;
|
|||
import org.junit.Assert;
|
||||
import org.junit.Test;
|
||||
|
||||
import static org.keycloak.OAuthErrorException.INVALID_GRANT;
|
||||
import static org.keycloak.testsuite.AbstractAdminTest.loadJson;
|
||||
import static org.keycloak.testsuite.admin.ApiUtil.findUserByUsername;
|
||||
import static org.keycloak.testsuite.util.ClientPoliciesUtil.createClientRolesConditionConfig;
|
||||
|
|
@ -82,6 +85,7 @@ import static org.hamcrest.Matchers.startsWith;
|
|||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.assertFalse;
|
||||
import static org.junit.Assert.assertNotEquals;
|
||||
import static org.junit.Assert.assertNotNull;
|
||||
import static org.junit.Assert.assertNull;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
|
||||
|
|
@ -300,6 +304,52 @@ public class ParTest extends AbstractClientPoliciesTest {
|
|||
}
|
||||
}
|
||||
|
||||
|
||||
// Test manually created PAR request cannot be used to obtain OAuth2 code
|
||||
@Test
|
||||
public void testParDoNotClashWithAuthorizationCode() throws Exception {
|
||||
try {
|
||||
// setup PAR realm settings
|
||||
int requestUriLifespan = 45;
|
||||
setParRealmSettings(requestUriLifespan);
|
||||
|
||||
// Step 1 - Login as regular user and obtain sessionId
|
||||
oauth.doLogin(TEST_USER2_NAME, TEST_USER2_PASSWORD);
|
||||
AuthorizationEndpointResponse authzResponse = oauth.parseLoginResponse();
|
||||
String sessionId = authzResponse.getSessionState();
|
||||
String code = authzResponse.getCode();
|
||||
assertNotNull(sessionId);
|
||||
assertNotNull(code);
|
||||
|
||||
// Step 2: PAR request with some custom injected parameters
|
||||
ParRequest pReq = new ParRequest(oauth) {
|
||||
|
||||
@Override
|
||||
protected void initRequest() {
|
||||
super.initRequest();
|
||||
parameter(OAuth2Code.ID_NOTE, "some-id");
|
||||
parameter(OAuth2Code.USER_SESSION_ID_NOTE, sessionId);
|
||||
parameter(OAuth2Code.EXPIRATION_NOTE, String.valueOf(Time.currentTime() + 9999));
|
||||
}
|
||||
};
|
||||
ParResponse pResp = pReq.send();
|
||||
assertEquals(201, pResp.getStatusCode());
|
||||
String requestUri = pResp.getRequestUri();
|
||||
|
||||
// Step 3: Attempt to exchange code for token with the "fake code" from PAR
|
||||
String parId = requestUri.substring(requestUri.lastIndexOf(":" ) + 1);
|
||||
String clientUUID = ApiUtil.findClientByClientId(adminClient.realm("test"), oauth.getClientId()).toRepresentation().getId();
|
||||
String fakeCode = parId + "." + sessionId + "." + clientUUID;
|
||||
AccessTokenResponse response = oauth.doAccessTokenRequest(fakeCode);
|
||||
assertEquals(400, response.getStatusCode());
|
||||
assertEquals(INVALID_GRANT, response.getError());
|
||||
} finally {
|
||||
// Logout
|
||||
AccountHelper.logout(adminClient.realm(oauth.getRealm()), TEST_USER2_NAME);
|
||||
restoreParRealmSettings();
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testWrongSigningAlgorithmForRequestObject() throws Exception {
|
||||
try {
|
||||
|
|
|
|||
Loading…
Reference in a new issue