Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
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;
Expand All @@ -47,7 +48,7 @@ public DefaultActionTokenKey(String userId, String actionId, int absoluteExpirat
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
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;
Expand All @@ -33,6 +32,7 @@

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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -165,7 +166,7 @@ public Response request() {

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();
Expand All @@ -180,7 +181,7 @@ public Response request() {
}

SingleUseObjectProvider singleUseStore = session.singleUseObjects();
singleUseStore.put(key, expiresIn, params);
singleUseStore.put(CACHE_KEY_PREFIX + key, expiresIn, params);

ParResponse parResponse = new ParResponse(requestUri, expiresIn);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@

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;

Expand Down Expand Up @@ -60,7 +61,7 @@ public AuthzEndpointParParser(KeycloakSession session, ClientModel client, Strin
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.");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,16 @@
*/
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";
private static final String REDIRECT_URI_PARAM_NOTE = "redirectUri";
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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -54,7 +55,7 @@ public static String persistCode(KeycloakSession session, AuthenticatedClientSes
}

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();
}

Expand Down Expand Up @@ -94,7 +95,7 @@ public static ParseResult parseCode(KeycloakSession session, String code, RealmM
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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ private void checkValidParContents(PreAuthorizationRequestContext preAuthorizati

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.");
}
Expand All @@ -100,7 +100,7 @@ private void checkValidParContents(PreAuthorizationRequestContext preAuthorizati

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");
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,22 @@
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;
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;
Expand Down Expand Up @@ -98,6 +102,8 @@
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;
Expand Down Expand Up @@ -350,6 +356,35 @@ public void resetPasswordTwice() throws IOException {
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
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;
Expand All @@ -54,6 +55,7 @@
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;
Expand All @@ -72,6 +74,7 @@
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;
Expand All @@ -82,6 +85,7 @@
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;

Expand Down Expand Up @@ -300,6 +304,52 @@ public void testSuccessfulSingleParPublicClient() throws Exception {
}
}


// 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 {
Expand Down
Loading