8000 [OID4VCI-FAPI2] Pass fapi2-security-profile-final-state-only-outside-request-object-not-used by tdiesler · Pull Request #49788 · keycloak/keycloak · GitHub
[go: up one dir, main page]

Skip to content
8000
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
8000 Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ public class AttestationBasedClientAuthenticator extends AbstractClientAuthentic
* Comma-separated aliases of trust-material identity providers that expose the trusted attester keys.
*/
public static final String OAUTH_CLIENT_ATTESTATION_CONFIG_TRUST_IDPS = "attester_trust_idps";
public static final String OAUTH_CLIENT_ATTESTATION_DEFAULT_TRUST_IDP_ALIAS = "abca-attester-default-trust";

@Override
public String getId() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,30 +46,21 @@ public class AuthzEndpointParParser extends AuthzEndpointRequestParser {

private final KeycloakSession session;
private final ClientModel client;
private Map<String, String> requestParams;
private String invalidRequestMessage = null;
private final Map<String, String> requestParams;

public AuthzEndpointParParser(KeycloakSession session, ClientModel client, String requestUri) {
super(session);
this.session = session;
this.client = client;
SingleUseObjectProvider singleUseStore = session.singleUseObjects();
String key;
try {
key = requestUri.substring(ParEndpoint.REQUEST_URI_PREFIX_LENGTH);
} catch (RuntimeException re) {
logger.warnf(re,"Unable to parse request_uri: %s", requestUri);
throw new RuntimeException("Unable to parse request_uri");
}
Map<String, String> retrievedRequest = singleUseStore.remove(CACHE_KEY_PREFIX + key);
Map<String, String> retrievedRequest = removeRequestObject(session, requestUri);
if (retrievedRequest == null) {
throw new RuntimeException("PAR not found. not issued or used multiple times.");
}

RealmModel realm = session.getContext().getRealm();
int expiresIn = realm.getParPolicy().getRequestUriLifespan();
long created = Long.parseLong(retrievedRequest.get(PAR_CREATED_TIME));
if (System.currentTimeMillis() - created < (expiresIn * 1000)) {
if (System.currentTimeMillis() - created < (expiresIn * 1000L)) {
requestParams = retrievedRequest;
} else {
throw new RuntimeException("PAR expired.");
Expand All @@ -87,7 +78,7 @@ public void parseRequest(AuthorizationEndpointRequest request) {

if (requestParam != null) {
// parses the request object if PAR was registered using JAR
// parameters from requets object have precedence over those sent directly in the request
// parameters from the request object have precedence over those sent directly in the request
new ParEndpointRequestObjectParser(session, requestParam, client).parseRequest(request);
} else {
super.parseRequest(request);
Expand All @@ -105,13 +96,34 @@ protected Integer getIntParameter(String paramName) {
return paramVal == null ? null : Integer.valueOf(paramVal);
}

public String getInvalidRequestMessage() {
return invalidRequestMessage;
}

@Override
protected Set<String> keySet() {
return requestParams.keySet();
}

public static Map<String, String> getRequestObject(KeycloakSession session, String requestUri) {
String key = getRequestObjectKey(requestUri);
SingleUseObjectProvider singleUseStore = session.singleUseObjects();
Map<String, String> retrievedRequest = singleUseStore.get(CACHE_KEY_PREFIX + key);
return retrievedRequest;
}

/**
* Authorization servers that enforce one-time use of request_uri values do so at the point of authorization,
* not at the point of visiting the authorization endpoint
* OpenID CT: fapi2-security-profile-final-par-ensure-reused-request-uri-prior-to-auth-completion-succeeds
*/
public static Map<String, String> removeRequestObject(KeycloakSession session, String requestUri) {
String key = getRequestObjectKey(requestUri);
return session.singleUseObjects().remove(CACHE_KEY_PREFIX + key);
}

private static String getRequestObjectKey(String requestUri) {
try {
return requestUri.substring(ParEndpoint.REQUEST_URI_PREFIX_LENGTH);
} catch (RuntimeException re) {
logger.warnf(re,"Unable to parse request_uri: %s", requestUri);
throw new RuntimeException("Unable to parse request_uri");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package org.keycloak.services.clientpolicy.executor;

import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;

Expand All @@ -28,15 +29,14 @@
import org.keycloak.models.ClientModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel;
import org.keycloak.models.SingleUseObjectProvider;
import org.keycloak.protocol.oidc.OIDCLoginProtocol;
import org.keycloak.protocol.oidc.endpoints.AuthorizationEndpoint;
import org.keycloak.protocol.oidc.endpoints.request.AuthorizationEndpointRequest;
import org.keycloak.protocol.oidc.endpoints.request.AuthorizationEndpointRequestParserProcessor;
import org.keycloak.protocol.oidc.endpoints.request.AuthzEndpointRequestObjectParser;
import org.keycloak.protocol.oidc.endpoints.request.AuthzEndpointRequestParser;
import org.keycloak.protocol.oidc.endpoints.request.RequestUriType;
import org.keycloak.protocol.oidc.par.endpoints.ParEndpoint;
import org.keycloak.protocol.oidc.par.endpoints.request.AuthzEndpointParParser;
import org.keycloak.representations.idm.ClientPolicyExecutorConfigurationRepresentation;
import org.keycloak.services.clientpolicy.ClientPolicyContext;
import org.keycloak.services.clientpolicy.ClientPolicyException;
Expand All @@ -50,7 +50,7 @@
public class SecureParContentsExecutor implements ClientPolicyExecutorProvider<ClientPolicyExecutorConfigurationRepresentation> {

protected final KeycloakSession session;
private static final Logger logger = Logger.getLogger(SecureParContentsExecutor.class);
private static final Logger log = Logger.getLogger(SecureParContentsExecutor.class);

public SecureParContentsExecutor(KeycloakSession session) {
this.session = session;
Expand All @@ -76,32 +76,34 @@ public void executeOnEvent(ClientPolicyContext context) throws ClientPolicyExcep
private void checkValidParContents(PreAuthorizationRequestContext preAuthorizationRequestContext) throws ClientPolicyException {
MultivaluedMap<String, String> requestParametersFromQuery = preAuthorizationRequestContext.getRequestParameters();
String requestUri = requestParametersFromQuery.getFirst(OIDCLoginProtocol.REQUEST_URI_PARAM);
if (requestUri == null) {
throw new ClientPolicyException(OAuthErrorException.INVALID_REQUEST, "request_uri not included.");
}
if (requestUri != null && AuthorizationEndpointRequestParserProcessor.getRequestUriType(requestUri) != RequestUriType.PAR) {
if (requestUri == null || AuthorizationEndpointRequestParserProcessor.getRequestUriType(requestUri) != RequestUriType.PAR) {
throw new ClientPolicyException(OAuthErrorException.INVALID_REQUEST, "PAR request_uri not included.");
}

String key = requestUri.substring(ParEndpoint.REQUEST_URI_PREFIX_LENGTH);
SingleUseObjectProvider singleUseStore = session.singleUseObjects();
Map<String, String> requestParametersFromPAR = singleUseStore.get(ParEndpoint.CACHE_KEY_PREFIX + key);
Map<String, String> requestParametersFromPAR = AuthzEndpointParParser.getRequestObject(session, requestUri);
if (requestParametersFromPAR == null) {
throw new ClientPolicyException(OAuthErrorException.INVALID_REQUEST, "PAR not found. not issued or used multiple times.");
throw new ClientPolicyException(OAuthErrorException.INVALID_REQUEST_URI, "PAR not found, not issued or used multiple times.");
}

Set<String> requestParametersNameFromPAR = new HashSet<>();
Set<String> requestParametersNameFromPAR;
if (requestParametersFromPAR.containsKey(OIDCLoginProtocol.REQUEST_PARAM)) {
// if PAR request includes request object (JAR), parsing the request is needed.
requestParametersNameFromPAR = getParRetrievedRequestParameters(requestParametersFromPAR, preAuthorizationRequestContext.getClientId());
} else {
requestParametersNameFromPAR = requestParametersFromPAR.keySet();
}

for (String queryParamName : requestParametersFromQuery.keySet()) {
if (!requestParametersNameFromPAR.contains(queryParamName) && !OIDCLoginProtocol.REQUEST_URI_PARAM.equals(queryParamName)) {
singleUseStore.remove(ParEndpoint.CACHE_KEY_PREFIX + key);
throw new ClientPolicyException(OAuthErrorException.INVALID_REQUEST, "PAR request did not include necessary parameters");
List<String> queryKeys = requestParametersFromQuery.keySet().stream()
.filter(it -> !it.equals(OIDCLoginProtocol.REQUEST_URI_PARAM))
.toList();

// FAPI says only parameters inside the request object should be used
//
for (String queryParam : queryKeys) {
if (!requestParametersNameFromPAR.contains(queryParam)) {
AuthzEndpointParParser.removeRequestObject(session, requestUri);
log.warnf("PAR request did not include query parameter: %s", queryParam);
throw new ClientPolicyException(OAuthErrorException.INVALID_REQUEST_OBJECT, "PAR request did not include query parameter");
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ public class SecureRequestObjectExecutor implements ClientPolicyExecutorProvider

private static final Logger logger = Logger.getLogger(SecureRequestObjectExecutor.class);

public static final Integer DEFAULT_AVAILABLE_PERIOD = Integer.valueOf(3600); // (sec) from FAPI 1.0 Advanced requirement
public static final Integer DEAULT_ALLOWED_CLOCK_SKEW = Integer.valueOf(15); // (sec) from FAPI 2.0 requirement
public static final Integer DEFAULT_AVAILABLE_PERIOD = 3600; // (sec) from FAPI 1.0 Advanced requirement
public static final Integer DEFAULT_ALLOWED_CLOCK_SKEW = 15; // (sec) from FAPI 2.0 requirement

private final KeycloakSession session;
private Configuration configuration;
Expand All @@ -64,7 +64,7 @@ public void setupConfiguration(SecureRequestObjectExecutor.Configuration config)
configuration.setVerifyNbf(Boolean.TRUE);
configuration.setAvailablePeriod(DEFAULT_AVAILABLE_PERIOD);
configuration.setEncryptionRequired(Boolean.FALSE);
configuration.setAllowedClockSkew(DEAULT_ALLOWED_CLOCK_SKEW);
configuration.setAllowedClockSkew(DEFAULT_ALLOWED_CLOCK_SKEW);
} else {
configuration = config;
if (config.isVerifyNbf() == null) {
Expand All @@ -77,7 +77,7 @@ public void setupConfiguration(SecureRequestObjectExecutor.Configuration config)
configuration.setEncryptionRequired(Boolean.FALSE);
}
if (config.getAllowedClockSkew() == null) {
configuration.setAllowedClockSkew(DEAULT_ALLOWED_CLOCK_SKEW);
configuration.setAllowedClockSkew(DEFAULT_ALLOWED_CLOCK_SKEW);
}
}
}
Expand Down Expand Up @@ -289,4 +289,4 @@ private void throwClientPolicyException(String error, String message,

throw new ClientPolicyException(error, message);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,11 @@ public AuthorizationEndpointRequest scope(String... scopes) {
return this;
}

public AuthorizationEndpointRequest state(String state) {
loginForm.state(state);
return this;
}

public boolean openLoginForm() {
loginForm.open();
String currUrl = oauth.getDriver().getCurrentUrl();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.keycloak.VCFormat;
import org.keycloak.admin.client.Keycloak;
import org.keycloak.admin.client.resource.ClientPoliciesPoliciesResource;
import org.keycloak.admin.client.resource.ClientResource;
import org.keycloak.admin.client.resource.ClientScopeResource;
import org.keycloak.admin.client.resource.ClientScopesResource;
import org.keycloak.admin.client.resource.RealmResource;
Expand Down Expand Up @@ -125,6 +126,7 @@
import static org.keycloak.OID4VCConstants.OID4VCI_ENABLED_ATTRIBUTE_KEY;
import static org.keycloak.OID4VCConstants.OPENID_CREDENTIAL;
import static org.keycloak.authentication.authenticators.client.AttestationBasedClientAuthenticator.OAUTH_CLIENT_ATTESTATION_CONFIG_TRUST_IDPS;
import static org.keycloak.authentication.authenticators.client.AttestationBasedClientAuthenticator.OAUTH_CLIENT_ATTESTATION_DEFAULT_TRUST_IDP_ALIAS;
import static org.keycloak.constants.OID4VCIConstants.CREDENTIAL_OFFER_CREATE;
import static org.keycloak.models.Constants.CREATE_DEFAULT_CLIENT_SCOPES;
import static org.keycloak.models.oid4vci.CredentialScopeModel.CRYPTOGRAPHIC_BINDING_METHODS_DEFAULT;
Expand Down Expand Up @@ -185,10 +187,10 @@ public abstract class OID4VCIssuerTestBase {
protected ManagedClient managedClient;

@InjectClient(ref = OID4VCI_ABCA_CLIENT_ID, config = OID4VCAttestationBasedClient.class)
ManagedClient managedAttestationBasedClient;
protected ManagedClient managedAttestationBasedClient;

@InjectClient(ref = OID4VCI_PUBLIC_CLIENT_ID, config = OID4VCPublicClient.class)
ManagedClient managedPublicClient;
protected ManagedClient managedPublicClient;

@InjectOAuthClient
protected OAuthClient oauth;
Expand Down Expand Up @@ -249,7 +251,6 @@ public void configureTestRealm() {
testUser.setRealmRoles(List.of(CREDENTIAL_OFFER_CREATE.getName()));
realmResource.users().get(testUser.getId()).roles().realmLevel().add(List.of(credentialOfferRole));
}

}

@BeforeEach
Expand Down Expand Up @@ -295,14 +296,6 @@ protected static void configureTrustIdentityProvider(RealmModel realm, String al
}
}

protected void setClientTrustSource(String alias) {
Map<String, String> attributes = new HashMap<>(Optional.ofNullable(abcaClient.getAttributes()).orElse(Map.of()));
attributes.put(OAUTH_CLIENT_ATTESTATION_CONFIG_TRUST_IDPS, alias);
abcaClient.setAttributes(attributes);
testRealm.admin().clients().get(abcaClient.getId()).update(abcaClient);
abcaClient = testRealm.admin().clients().get(abcaClient.getId()).toRepresentation();
}

protected static KeyWrapper getKeyFromSession(KeycloakSession keycloakSession) {
String realmName = keycloakSession.getContext().getRealm().getName();
Logger logger = Logger.getLogger(OID4VCIssuerTestBase.class);
Expand Down Expand Up @@ -493,6 +486,25 @@ protected void setVerifiableCredentialsEnabled(boolean enabled) {
realmResource.update(realm);
}

protected void setClientAttribute(ClientRepresentation client, String attrKey, String attrValue) {
setClientAttributes(client, Map.of(attrKey, attrValue));
}

protected void setClientAttributes(ClientRepresentation client, Map<String, String> attrUpdate) {
Map<String, String> attrs = client.getAttributes();
boolean updateNeeded = attrUpdate.entrySet().stream()
.anyMatch(e -> !e.getValue().equals(attrs.get(e.getKey())));
if (updateNeeded) {
ClientResource clientResource = testRealm.admin().clients().get(client.getId());
client.getAttributes().putAll(attrUpdate);
clientResource.update(client);
}
}

protected void setCredentialScopeAttribute(ClientScopeRepresentation credScope, String attrKey, String attrValue) {
setCredentialScopeAttributes(credScope, Map.of(attrKey, attrValue));
}

protected void setCredentialScopeAttributes(ClientScopeRepresentation credScope, Map<String, String> attrUpdate) {
ClientScopeResource clientScopeResource = testRealm.admin().clientScopes().get(credScope.getId());
credScope = clientScopeResource.toRepresentation();
Expand Down Expand Up @@ -950,6 +962,7 @@ public ClientBuilder configure(ClientBuilder client) {
.defaultClientScopes("basic", "profile", "roles")
.optionalClientScopes(optionalClientScopes)
.attribute(OID4VCI_ENABLED_ATTRIBUTE_KEY, "true")
.attribute(OAUTH_CLIENT_ATTESTATION_CONFIG_TRUST_IDPS, OAUTH_CLIENT_ATTESTATION_DEFAULT_TRUST_IDP_ALIAS)
.redirectUris("*");
return client;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import java.util.function.Function;

import org.keycloak.TokenVerifier;
import org.keycloak.admin.client.resource.ClientPoliciesPoliciesResource;
import org.keycloak.admin.client.resource.RealmResource;
import org.keycloak.protocol.oid4vc.clientpolicy.PredicateCredentialClientPolicy;
import org.keycloak.protocol.oid4vc.model.CredentialIssuer;
Expand All @@ -14,8 +13,6 @@
import org.keycloak.protocol.oid4vc.model.OID4VCAuthorizationDetail;
import org.keycloak.protocol.oid4vc.model.VerifiableCredential;
import org.keycloak.representations.JsonWebToken;
import org.keycloak.representations.idm.ClientPoliciesRepresentation;
import org.keycloak.representations.idm.ClientPolicyRepresentation;
import org.keycloak.testframework.annotations.KeycloakIntegrationTest;
import org.keycloak.testframework.annotations.TestSetup;
import org.keycloak.tests.oid4vc.OID4VCBasicWallet.AuthorizationEndpointRequest;
Expand Down Expand Up @@ -111,14 +108,8 @@ public void testNoOffer_Scope_RequireOfferPolicy() {

// Set client policy 'oid4vci-offer-required'
//
ClientPoliciesPoliciesResource clientPoliciesResource = testRealm.admin().clientPoliciesPoliciesResource();
ClientPoliciesRepresentation policies = clientPoliciesResource.getPolicies();
ClientPolicyRepresentation clientPolicy = policies.getPolicies().stream()
.filter(cp -> cp.getName().equals(offerRequiredPolicy.getName()))
.findFirst().orElseThrow();
Boolean wasPolicyEnabled = clientPolicy.isEnabled();
clientPolicy.setEnabled(policyEnabled);
clientPoliciesResource.updatePolicies(policies);
Boolean wasPolicyEnabled = getClientPolicy(offerRequiredPolicy.getName()).isEnabled();
setClientPolicyEnabled(offerRequiredPolicy.getName(), policyEnabled);

// Set client scope attribute 'vc.policy.offer.required'
//
Expand Down Expand Up @@ -179,8 +170,7 @@ public void testNoOffer_Scope_RequireOfferPolicy() {
assertFalse(credentialResponse.getCredentials().isEmpty(), "Credentials expected");
return true;
} finally {
clientPolicy.setEnabled(wasPolicyEnabled);
clientPoliciesResource.updatePolicies(policies);
setClientPolicyEnabled(offerRequiredPolicy.getName(), wasPolicyEnabled);

credScope.setCredentialPolicyValue(offerRequiredPolicy, wasScopeEnabled);
updateCredentialScope(credScope);
Expand Down
Loading
Loading
0