Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KEYCLOAK-14019 Improvements for request_uri parameter #7714

Merged
merged 1 commit into from
Jan 18, 2021
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 @@ -109,6 +109,9 @@ public class OIDCConfigurationRepresentation {
@JsonProperty("request_uri_parameter_supported")
private Boolean requestUriParameterSupported;

@JsonProperty("require_request_uri_registration")
private Boolean requireRequestUriRegistration;

// KEYCLOAK-7451 OAuth Authorization Server Metadata for Proof Key for Code Exchange
@JsonProperty("code_challenge_methods_supported")
private List<String> codeChallengeMethodsSupported;
Expand Down Expand Up @@ -343,6 +346,14 @@ public void setRequestUriParameterSupported(Boolean requestUriParameterSupported
this.requestUriParameterSupported = requestUriParameterSupported;
}

public Boolean getRequireRequestUriRegistration() {
return requireRequestUriRegistration;
}

public void setRequireRequestUriRegistration(Boolean requireRequestUriRegistration) {
this.requireRequestUriRegistration = requireRequestUriRegistration;
}

// KEYCLOAK-7451 OAuth Authorization Server Metadata for Proof Key for Code Exchange
public List<String> getCodeChallengeMethodsSupported() {
return codeChallengeMethodsSupported;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,13 @@
import org.keycloak.authentication.authenticators.client.X509ClientAuthenticator;
import org.keycloak.jose.jws.Algorithm;
import org.keycloak.models.ClientModel;
import org.keycloak.models.Constants;
import org.keycloak.representations.idm.ClientRepresentation;

import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;

/**
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
Expand Down Expand Up @@ -79,6 +83,14 @@ public void setRequestObjectRequired(String requestObjectRequired) {
setAttribute(OIDCConfigAttributes.REQUEST_OBJECT_REQUIRED, requestObjectRequired);
}

public List<String> getRequestUris() {
return getAttributeMultivalued(OIDCConfigAttributes.REQUEST_URIS);
}

public void setRequestUris(List<String> requestUris) {
setAttributeMultivalued(OIDCConfigAttributes.REQUEST_URIS, requestUris);
}

public boolean isUseJwksUrl() {
String useJwksUrl = getAttribute(OIDCConfigAttributes.USE_JWKS_URL);
return Boolean.parseBoolean(useJwksUrl);
Expand Down Expand Up @@ -244,4 +256,20 @@ private void setAttribute(String attrKey, String attrValue) {
}
}
}

private List<String> getAttributeMultivalued(String attrKey) {
String attrValue = getAttribute(attrKey);
if (attrValue == null) return Collections.emptyList();
return Arrays.asList(Constants.CFG_DELIMITER_PATTERN.split(attrValue));
}

private void setAttributeMultivalued(String attrKey, List<String> attrValues) {
if (attrValues == null || attrValues.size() == 0) {
// Remove attribute
setAttribute(attrKey, null);
} else {
String attrValueFull = String.join(Constants.CFG_DELIMITER, attrValues);
setAttribute(attrKey, attrValueFull);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ public final class OIDCConfigAttributes {
public static final String REQUEST_OBJECT_REQUIRED_REQUEST = "request only";
public static final String REQUEST_OBJECT_REQUIRED_REQUEST_URI = "request_uri only";

public static final String REQUEST_URIS = "request.uris";

public static final String JWKS_URL = "jwks.url";

public static final String USE_JWKS_URL = "use.jwks.url";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ public Object getConfig() {

config.setRequestParameterSupported(true);
config.setRequestUriParameterSupported(true);
config.setRequireRequestUriRegistration(true);

// KEYCLOAK-7451 OAuth Authorization Server Metadata for Proof Key for Code Exchange
config.setCodeChallengeMethodsSupported(DEFAULT_CODE_CHALLENGE_METHODS_SUPPORTED);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,15 @@
import org.keycloak.protocol.oidc.OIDCAdvancedConfigWrapper;
import org.keycloak.protocol.oidc.OIDCConfigAttributes;
import org.keycloak.protocol.oidc.OIDCLoginProtocol;
import org.keycloak.protocol.oidc.utils.RedirectUtils;
import org.keycloak.services.ErrorPageException;
import org.keycloak.services.ServicesLogger;
import org.keycloak.services.messages.Messages;

import javax.ws.rs.core.MultivaluedMap;
import javax.ws.rs.core.Response;
import java.io.InputStream;
import java.util.HashSet;
import java.util.List;

/**
Expand Down Expand Up @@ -75,7 +77,14 @@ public static AuthorizationEndpointRequest parseRequest(EventBuilder event, Keyc
if (requestParam != null) {
new AuthzEndpointRequestObjectParser(session, requestParam, client).parseRequest(request);
} else if (requestUriParam != null) {
try (InputStream is = session.getProvider(HttpClientProvider.class).get(requestUriParam)) {
// Validate "requestUriParam" with allowed requestUris
List<String> requestUris = OIDCAdvancedConfigWrapper.fromClientModel(client).getRequestUris();
String requestUri = RedirectUtils.verifyRedirectUri(session, client.getRootUrl(), requestUriParam, new HashSet<>(requestUris), false);
if (requestUri == null) {
throw new RuntimeException("Specified 'request_uri' not allowed for this client.");
}

try (InputStream is = session.getProvider(HttpClientProvider.class).get(requestUri)) {
String retrievedRequest = StreamUtil.readString(is);
new AuthzEndpointRequestObjectParser(session, retrievedRequest, client).parseRequest(request);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ private static Set<String> getValidateRedirectUris(KeycloakSession session) {
.collect(Collectors.toSet());
}

private static String verifyRedirectUri(KeycloakSession session, String rootUrl, String redirectUri, Set<String> validRedirects, boolean requireRedirectUri) {
public static String verifyRedirectUri(KeycloakSession session, String rootUrl, String redirectUri, Set<String> validRedirects, boolean requireRedirectUri) {
KeycloakUriInfo uriInfo = session.getContext().getUri();
RealmModel realm = session.getContext().getRealm();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,10 @@ public static ClientRepresentation toInternal(KeycloakSession session, OIDCClien
configWrapper.setIdTokenEncryptedResponseEnc(clientOIDC.getIdTokenEncryptedResponseEnc());
}

if (clientOIDC.getRequestUris() != null) {
configWrapper.setRequestUris(clientOIDC.getRequestUris());
}

configWrapper.setTokenEndpointAuthSigningAlg(clientOIDC.getTokenEndpointAuthSigningAlg());

configWrapper.setBackchannelLogoutUrl(clientOIDC.getBackchannelLogoutUri());
Expand Down Expand Up @@ -253,6 +257,9 @@ public static OIDCClientRepresentation toExternalResponse(KeycloakSession sessio
if (config.getIdTokenEncryptedResponseEnc() != null) {
response.setIdTokenEncryptedResponseEnc(config.getIdTokenEncryptedResponseEnc());
}
if (config.getRequestUris() != null) {
response.setRequestUris(config.getRequestUris());
}
if (config.getTokenEndpointAuthSigningAlg() != null) {
response.setTokenEndpointAuthSigningAlg(config.getTokenEndpointAuthSigningAlg());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -719,6 +719,7 @@ public void testSecureRequestObjectExecutor() throws ClientRegistrationException
String clientSecret = "secret";
String cid = createClientByAdmin(clientId, (ClientRepresentation clientRep) -> {
clientRep.setSecret(clientSecret);
OIDCAdvancedConfigWrapper.fromClientRepresentation(clientRep).setRequestUris(Arrays.asList(TestApplicationResourceUrls.clientRequestUri()));
});
adminClient.realm(REALM_NAME).clients().get(cid).roles().create(RoleBuilder.create().name("sample-client-role").build());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -531,4 +531,21 @@ public void testClientWithoutScope() throws ClientRegistrationException {
assertTrue(realmDefaultClientScopes.equals(new HashSet<>(registeredDefaultClientScopes)));

}

@Test
public void testRequestUris() throws Exception {
OIDCClientRepresentation clientRep = null;
OIDCClientRepresentation response = null;

clientRep = createRep();
clientRep.setRequestUris(Arrays.asList("http://host/foo", "https://host2/bar"));

response = reg.oidc().create(clientRep);
Assert.assertNames(response.getRequestUris(), "http://host/foo", "https://host2/bar");

// Test Keycloak representation
ClientRepresentation kcClient = getClient(response.getClientId());
OIDCAdvancedConfigWrapper config = OIDCAdvancedConfigWrapper.fromClientRepresentation(kcClient);
Assert.assertNames(config.getRequestUris(), "http://host/foo", "https://host2/bar");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.keycloak.authentication.authenticators.client.JWTClientAuthenticator;
import org.keycloak.common.Profile;
import org.keycloak.common.util.Time;
import org.keycloak.common.util.UriUtils;
import org.keycloak.events.Details;
import org.keycloak.events.EventType;
import org.keycloak.jose.jws.Algorithm;
Expand Down Expand Up @@ -121,7 +122,10 @@ public void configureTestRealm(RealmRepresentation testRealm) {

@Before
public void clientConfiguration() {
ClientManager.realm(adminClient.realm("test")).clientId("test-app").directAccessGrant(true);
ClientManager.realm(adminClient.realm("test")).clientId("test-app")
.directAccessGrant(true)
.setRequestUris(TestApplicationResourceUrls.clientRequestUri());

/*
* Configure the default client ID. Seems like OAuthClient is keeping the state of clientID
* For example: If some test case configure oauth.clientId("sample-public-client"), other tests
Expand Down Expand Up @@ -786,6 +790,72 @@ public void requestUriParamUnsigned() throws Exception {
assertTrue(appPage.isCurrent());
}

@Test
public void requestUriParamWithAllowedRequestUris() throws Exception {
oauth.stateParamHardcoded("mystate1");
String validRedirectUri = oauth.getRedirectUri();
TestOIDCEndpointsApplicationResource oidcClientEndpointsResource = testingClient.testApp().oidcClientEndpoints();
oidcClientEndpointsResource.setOIDCRequest("test", "test-app", validRedirectUri, "10", Algorithm.none.toString());
ClientManager.ClientManagerBuilder clientMgrBuilder = ClientManager.realm(adminClient.realm("test")).clientId("test-app");

oauth.requestUri(TestApplicationResourceUrls.clientRequestUri());

// Test with the relative allowed request_uri - should pass
String absoluteRequestUri = TestApplicationResourceUrls.clientRequestUri();
String requestUri = absoluteRequestUri.substring(UriUtils.getOrigin(absoluteRequestUri).length());
clientMgrBuilder.setRequestUris(requestUri);

oauth.openLoginForm();
Assert.assertFalse(errorPage.isCurrent());
loginPage.assertCurrent();

// Test with the relative and star at the end - should pass
requestUri = requestUri.replace("/get-oidc-request", "/*");
clientMgrBuilder.setRequestUris(requestUri);

oauth.openLoginForm();
Assert.assertFalse(errorPage.isCurrent());
loginPage.assertCurrent();

// Test absolute and wildcard at the end - should pass
requestUri = absoluteRequestUri.replace("/get-oidc-request", "/*");
clientMgrBuilder.setRequestUris(requestUri);

oauth.openLoginForm();
Assert.assertFalse(errorPage.isCurrent());
loginPage.assertCurrent();

// Test star only as wildcard - should pass
clientMgrBuilder.setRequestUris("*");

oauth.openLoginForm();
Assert.assertFalse(errorPage.isCurrent());
loginPage.assertCurrent();

// Test with multiple request_uris - should pass
clientMgrBuilder.setRequestUris("/foo", requestUri);

oauth.openLoginForm();
Assert.assertFalse(errorPage.isCurrent());
loginPage.assertCurrent();

// Test invalid request_uris - should fail
clientMgrBuilder.setRequestUris("/foo", requestUri.replace("/*", "/foo"));

oauth.openLoginForm();
errorPage.assertCurrent();

// Test with no request_uri set at all - should fail
clientMgrBuilder.setRequestUris();

oauth.openLoginForm();
errorPage.assertCurrent();

// Revert
clientMgrBuilder.setRequestUris(TestApplicationResourceUrls.clientRequestUri());

}

@Test
public void requestUriParamSigned() throws Exception {
oauth.stateParamHardcoded("mystate3");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ public void testDiscovery() {
// Request and Request_Uri
Assert.assertTrue(oidcConfig.getRequestParameterSupported());
Assert.assertTrue(oidcConfig.getRequestUriParameterSupported());
Assert.assertTrue(oidcConfig.getRequireRequestUriRegistration());

// KEYCLOAK-7451 OAuth Authorization Server Metadata for Proof Key for Code Exchange
// PKCE support
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@

import org.keycloak.admin.client.resource.ClientResource;
import org.keycloak.admin.client.resource.RealmResource;
import org.keycloak.protocol.oidc.OIDCAdvancedConfigWrapper;
import org.keycloak.protocol.oidc.OIDCConfigAttributes;
import org.keycloak.representations.idm.ClientRepresentation;
import org.keycloak.representations.idm.ProtocolMapperRepresentation;
import org.keycloak.representations.idm.RoleRepresentation;
import org.keycloak.representations.idm.UserRepresentation;

import java.util.Arrays;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.LinkedList;
Expand Down Expand Up @@ -68,10 +71,11 @@ public void updateAttribute(String attribute, String value) {
clientResource.update(app);
}

public void directAccessGrant(Boolean enable) {
public ClientManagerBuilder directAccessGrant(Boolean enable) {
ClientRepresentation app = clientResource.toRepresentation();
app.setDirectAccessGrantsEnabled(enable);
clientResource.update(app);
return this;
}

public ClientManagerBuilder standardFlow(Boolean enable) {
Expand Down Expand Up @@ -158,6 +162,13 @@ public void removeRedirectUris(String... redirectUris) {
clientResource.update(app);
}

// Set valid values of "request_uri" parameter
public void setRequestUris(String... requestUris) {
ClientRepresentation app = clientResource.toRepresentation();
OIDCAdvancedConfigWrapper.fromClientRepresentation(app).setRequestUris(Arrays.asList(requestUris));
clientResource.update(app);
}

public UserRepresentation getServiceAccountUser() {
return clientResource.getServiceAccountUser();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,8 @@ request-object-signature-alg=Request Object Signature Algorithm
request-object-signature-alg.tooltip=JWA algorithm, which client needs to use when sending OIDC request object specified by 'request' or 'request_uri' parameters. If set to 'any', Request object can be signed by any algorithm (including 'none' ).
request-object-required=Request Object Required
request-object-required.tooltip=Specifies if the client needs to provide a request object with their authorization requests, and what method they can use for this. If set to "not required", providing a request object is optional. In all other cases, providing a request object is mandatory. If set to "request", the request object must be provided by value. If set to "request_uri", the request object must be provided by reference. If set to "request or request_uri", either method can be used.
request-uris=Valid Request URIs
request-uris.tooltip=List of valid URIs, which can be used as values of 'request_uri' parameter during OpenID Connect authentication request. There is support for the same capabilities like for Valid Redirect URIs. For example wildcards or relative paths.
fine-saml-endpoint-conf=Fine Grain SAML Endpoint Configuration
fine-saml-endpoint-conf.tooltip=Expand this section to configure exact URLs for Assertion Consumer and Single Logout Service.
assertion-consumer-post-binding-url=Assertion Consumer Service POST Binding URL
Expand Down
Loading