libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases Packages Activity Actions

KEYCLOAK-18052 Client Policies : Revise SecureRequestObjectExecutor to have an option for checking nbf claim

Browse source
This commit is contained in:
Takashi Norimatsu 2021-05-11 05:08:47 +09:00 committed by Marek Posolda
parent f25de94ae1
commit 355a5d65fb
4 changed files with 115 additions and 21 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

62
services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureRequestObjectExecutor.java
View file

@ -59,8 +59,20 @@ public class SecureRequestObjectExecutor implements ClientPolicyExecutorProvider
}
@Override
public void setupConfiguration(Configuration config) {
this.configuration = config;
public void setupConfiguration(SecureRequestObjectExecutor.Configuration config) {
if (config == null) {
configuration = new Configuration();
configuration.setVerifyNbf(Boolean.TRUE);
configuration.setAvailablePeriod(DEFAULT_AVAILABLE_PERIOD);
} else {
configuration = config;
if (config.isVerifyNbf() == null) {
configuration.setVerifyNbf(Boolean.TRUE);
}
if (config.getAvailablePeriod() == null) {
configuration.setAvailablePeriod(DEFAULT_AVAILABLE_PERIOD);
}
}
}
@Override
@ -72,6 +84,8 @@ public class SecureRequestObjectExecutor implements ClientPolicyExecutorProvider
public static class Configuration extends ClientPolicyExecutorConfiguration {
@JsonProperty("available-period")
protected Integer availablePeriod;
@JsonProperty("verify-nbf")
protected Boolean verifyNbf;
public Integer getAvailablePeriod() {
return availablePeriod;
@ -80,6 +94,14 @@ public class SecureRequestObjectExecutor implements ClientPolicyExecutorProvider
public void setAvailablePeriod(Integer availablePeriod) {
this.availablePeriod = availablePeriod;
}
public Boolean isVerifyNbf() {
return verifyNbf;
}
public void setVerifyNbf(Boolean verifyNbf) {
this.verifyNbf = verifyNbf;
}
}
@Override
@ -150,24 +172,28 @@ public class SecureRequestObjectExecutor implements ClientPolicyExecutorProvider
throw new ClientPolicyException(INVALID_REQUEST_OBJECT, "Request Expired");
}
// check whether "nbf" claim exists
if (requestObject.get("nbf") == null) {
logger.trace("nbf claim not incuded.");
throw new ClientPolicyException(INVALID_REQUEST_OBJECT, "Missing parameter : nbf");
}
// "nbf" check is not needed for FAPI-RW ID2 security profile
// while needed for FAPI 1.0 Advanced security profile
if (Optional.ofNullable(configuration.isVerifyNbf()).orElse(Boolean.FALSE).booleanValue()) {
// check whether "nbf" claim exists
if (requestObject.get("nbf") == null) {
logger.trace("nbf claim not incuded.");
throw new ClientPolicyException(INVALID_REQUEST_OBJECT, "Missing parameter : nbf");
}
// check whether request object not yet being processed
long nbf = requestObject.get("nbf").asLong();
if (Time.currentTime() < nbf) { // TODO: Time.currentTime() is int while nbf is long...
logger.trace("request object not yet being processed.");
throw new ClientPolicyException(INVALID_REQUEST_OBJECT, "Request not yet being processed");
}
// check whether request object not yet being processed
long nbf = requestObject.get("nbf").asLong();
if (Time.currentTime() < nbf) { // TODO: Time.currentTime() is int while nbf is long...
logger.trace("request object not yet being processed.");
throw new ClientPolicyException(INVALID_REQUEST_OBJECT, "Request not yet being processed");
}
// check whether request object's available period is short
int availablePeriod = Optional.ofNullable(configuration.getAvailablePeriod()).orElse(DEFAULT_AVAILABLE_PERIOD).intValue();
if (exp - nbf > availablePeriod) {
logger.trace("request object's available period is long.");
throw new ClientPolicyException(INVALID_REQUEST_OBJECT, "Request's available period is long");
// check whether request object's available period is short
int availablePeriod = Optional.ofNullable(configuration.getAvailablePeriod()).orElse(DEFAULT_AVAILABLE_PERIOD).intValue();
if (exp - nbf > availablePeriod) {
logger.trace("request object's available period is long.");
throw new ClientPolicyException(INVALID_REQUEST_OBJECT, "Request's available period is long");
}
}
// check whether "aud" claim exists

9
services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureRequestObjectExecutorFactory.java
View file

@ -17,6 +17,8 @@
package org.keycloak.services.clientpolicy.executor;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
@ -32,6 +34,11 @@ public class SecureRequestObjectExecutorFactory implements ClientPolicyExecutorP
public static final String PROVIDER_ID = "secure-reqobj-executor";
public static final String VERIFY_NBF = "verify-nbf";
private static final ProviderConfigProperty VERIFY_NBF_PROPERTY = new ProviderConfigProperty(
VERIFY_NBF, null, null, ProviderConfigProperty.BOOLEAN_TYPE, true);
@Override
public ClientPolicyExecutorProvider create(KeycloakSession session) {
return new SecureRequestObjectExecutor(session);
@ -61,7 +68,7 @@ public class SecureRequestObjectExecutorFactory implements ClientPolicyExecutorP
@Override
public List<ProviderConfigProperty> getConfigProperties() {
return Collections.emptyList();
return new ArrayList<>(Arrays.asList(VERIFY_NBF_PROPERTY));
}
}

3
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/AbstractClientPoliciesTest.java
View file

@ -869,9 +869,10 @@ public abstract class AbstractClientPoliciesTest extends AbstractKeycloakTest {
return config;
}
protected Object createSecureRequestObjectExecutorConfig(Integer availablePeriod) {
protected Object createSecureRequestObjectExecutorConfig(Integer availablePeriod, Boolean verifyNbf) {
SecureRequestObjectExecutor.Configuration config = new SecureRequestObjectExecutor.Configuration();
if (availablePeriod != null) config.setAvailablePeriod(availablePeriod);
if (verifyNbf != null) config.setVerifyNbf(verifyNbf);
return config;
}

62
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientPoliciesTest.java
View file

@ -923,7 +923,7 @@ public class ClientPoliciesTest extends AbstractClientPoliciesTest {
String json = (new ClientProfilesBuilder()).addProfile(
(new ClientProfileBuilder()).createProfile(PROFILE_NAME, "Prvy Profil", Boolean.FALSE, null)
.addExecutor(SecureRequestObjectExecutorFactory.PROVIDER_ID,
createSecureRequestObjectExecutorConfig(availablePeriod))
createSecureRequestObjectExecutorConfig(availablePeriod, null))
.toRepresentation()
).toString();
updateProfiles(json);
@ -1043,6 +1043,66 @@ public class ClientPoliciesTest extends AbstractClientPoliciesTest {
registerRequestObject(requestObject, clientId, Algorithm.ES256, true);
successfulLoginAndLogout(clientId, clientSecret);
// update profile : no configuration - "nbf" check and available period is 3600 sec
json = (new ClientProfilesBuilder()).addProfile(
(new ClientProfileBuilder()).createProfile(PROFILE_NAME, "Prvy Profil", Boolean.FALSE, null)
.addExecutor(SecureRequestObjectExecutorFactory.PROVIDER_ID, null)
.toRepresentation()
).toString();
updateProfiles(json);
// check whether "nbf" claim exists
requestObject = createValidRequestObjectForSecureRequestObjectExecutor(clientId);
requestObject.nbf(null);
registerRequestObject(requestObject, clientId, Algorithm.ES256, false);
oauth.openLoginForm();
assertEquals(SecureRequestObjectExecutor.INVALID_REQUEST_OBJECT, oauth.getCurrentQuery().get(OAuth2Constants.ERROR));
assertEquals("Missing parameter : nbf", oauth.getCurrentQuery().get(OAuth2Constants.ERROR_DESCRIPTION));
// check whether request object not yet being processed
requestObject = createValidRequestObjectForSecureRequestObjectExecutor(clientId);
requestObject.nbf(requestObject.getNbf() + 600);
registerRequestObject(requestObject, clientId, Algorithm.ES256, false);
oauth.openLoginForm();
assertEquals(SecureRequestObjectExecutor.INVALID_REQUEST_OBJECT, oauth.getCurrentQuery().get(OAuth2Constants.ERROR));
assertEquals("Request not yet being processed", oauth.getCurrentQuery().get(OAuth2Constants.ERROR_DESCRIPTION));
// check whether request object's available period is short
requestObject = createValidRequestObjectForSecureRequestObjectExecutor(clientId);
requestObject.exp(requestObject.getNbf() + SecureRequestObjectExecutor.DEFAULT_AVAILABLE_PERIOD + 1);
registerRequestObject(requestObject, clientId, Algorithm.ES256, false);
oauth.openLoginForm();
assertEquals(SecureRequestObjectExecutor.INVALID_REQUEST_OBJECT, oauth.getCurrentQuery().get(OAuth2Constants.ERROR));
assertEquals("Request's available period is long", oauth.getCurrentQuery().get(OAuth2Constants.ERROR_DESCRIPTION));
// update profile : not check "nbf"
json = (new ClientProfilesBuilder()).addProfile(
(new ClientProfileBuilder()).createProfile(PROFILE_NAME, "Prvy Profil", Boolean.FALSE, null)
.addExecutor(SecureRequestObjectExecutorFactory.PROVIDER_ID,
createSecureRequestObjectExecutorConfig(null, Boolean.FALSE))
.toRepresentation()
).toString();
updateProfiles(json);
// not check whether "nbf" claim exists
requestObject = createValidRequestObjectForSecureRequestObjectExecutor(clientId);
requestObject.nbf(null);
registerRequestObject(requestObject, clientId, Algorithm.ES256, false);
successfulLoginAndLogout(clientId, clientSecret);
// not check whether request object not yet being processed
requestObject = createValidRequestObjectForSecureRequestObjectExecutor(clientId);
requestObject.nbf(requestObject.getNbf() + 600);
registerRequestObject(requestObject, clientId, Algorithm.ES256, false);
successfulLoginAndLogout(clientId, clientSecret);
// not check whether request object's available period is short
requestObject = createValidRequestObjectForSecureRequestObjectExecutor(clientId);
requestObject.exp(requestObject.getNbf() + SecureRequestObjectExecutor.DEFAULT_AVAILABLE_PERIOD + 1);
registerRequestObject(requestObject, clientId, Algorithm.ES256, false);
successfulLoginAndLogout(clientId, clientSecret);
}
@Test