Skip to content

Commit

Permalink
Validate Saml URLs inside DefaultClientValidationProvider (#142)
Browse files Browse the repository at this point in the history 
Closes keycloak/keycloak-private#62

Signed-off-by: rmartinc <[email protected]>
  • Loading branch information
@rmartinc
rmartinc authored Mar 23, 2024
1 parent e310604 commit abd03e3
Show file tree
Hide file tree
Showing 4 changed files with 174 additions and 18 deletions.
61 changes: 60 additions & 1 deletion services/src/main/java/org/keycloak/validation/DefaultClientValidationProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.keycloak.protocol.oidc.utils.PairwiseSubMapperUtils;
import org.keycloak.protocol.oidc.utils.PairwiseSubMapperValidator;
import org.keycloak.protocol.oidc.utils.SubjectType;
import org.keycloak.protocol.saml.SamlProtocol;
import org.keycloak.representations.idm.ProtocolMapperRepresentation;
import org.keycloak.representations.oidc.OIDCClientRepresentation;
import org.keycloak.services.util.ResolveRelative;
Expand Down Expand Up @@ -76,7 +77,52 @@ private enum FieldMessages {
TOS_URI(ClientModel.TOS_URI,
"Terms of service URL is not a valid URL", "tosURLInvalid",
null, null,
"Terms of service URL uses an illegal scheme", "tosURLIllegalSchemeError");
"Terms of service URL uses an illegal scheme", "tosURLIllegalSchemeError"),

ADMIN_URL("masterSamlProcessingUrl",
"Master SAML Processing URL is not a valid URL", "adminUrlURLInvalid",
null, null,
"Master SAML Processing URL uses an illegal scheme", "adminUrlURLIllegalSchemeError"),

SAML_ASSERTION_CONSUMER_URL_POST_URI(SamlProtocol.SAML_ASSERTION_CONSUMER_URL_POST_ATTRIBUTE,
"Assertion Consumer Service POST Binding URL is not a valid URL", "samlAssertionConsumerUrlPostURLInvalid",
null, null,
"Assertion Consumer Service POST Binding URL uses an illegal scheme", "samlAssertionConsumerUrlPostURLIllegalSchemeError"),

SAML_ASSERTION_CONSUMER_URL_REDIRECT_URI(SamlProtocol.SAML_ASSERTION_CONSUMER_URL_REDIRECT_ATTRIBUTE,
"Assertion Consumer Service Redirect Binding URL is not a valid URL", "samlAssertionConsumerUrlRedirectURLInvalid",
null, null,
"Assertion Consumer Service Redirect Binding URL uses an illegal scheme", "samlAssertionConsumerUrlRedirectURLIllegalSchemeError"),

SAML_ASSERTION_CONSUMER_URL_ARTIFACT_URI(SamlProtocol.SAML_ASSERTION_CONSUMER_URL_ARTIFACT_ATTRIBUTE,
"Artifact Binding URL is not a valid URL", "samlAssertionConsumerUrlArtifactURLInvalid",
null, null,
"Artifact Binding URL uses an illegal scheme", "samlAssertionConsumerUrlArtifactURLIllegalSchemeError"),

SAML_SINGLE_LOGOUT_SERVICE_URL_POST_URI(SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_POST_ATTRIBUTE,
"Logout Service POST Binding URL is not a valid URL", "samlLogoutServiceUrlPostURLInvalid",
null, null,
"Logout Service POST Binding URL uses an illegal scheme", "samlLogoutServiceUrlPostURLIllegalSchemeError"),

SAML_SINGLE_LOGOUT_SERVICE_URL_ARTIFACT_URI(SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_ARTIFACT_ATTRIBUTE,
"Logout Service ARTIFACT Binding URL is not a valid URL", "samlLogoutServiceUrlArtifactURLInvalid",
null, null,
"Logout Service ARTIFACT Binding URL uses an illegal scheme", "samlLogoutServiceUrlArtifactURLIllegalSchemeError"),

SAML_SINGLE_LOGOUT_SERVICE_URL_REDIRECT_URI(SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_REDIRECT_ATTRIBUTE,
"Logout Service Redirect Binding URL is not a valid URL", "samlLogoutServiceUrlRedirectURLInvalid",
null, null,
"Logout Service Redirect Binding URL uses an illegal scheme", "samlLogoutServiceUrlRedirectURLIllegalSchemeError"),

SAML_SINGLE_LOGOUT_SERVICE_URL_SOAP_URI(SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_SOAP_ATTRIBUTE,
"Logout Service SOAP Binding URL is not a valid URL", "samlLogoutServiceUrlSoapURLInvalid",
null, null,
"Logout Service SOAP Binding URL uses an illegal scheme", "samlAssertionConsumerUrlPostURLIllegalSchemeError"),

SAML_ARTIFACT_RESOLUTION_SERVICE_URL_URI(SamlProtocol.SAML_ARTIFACT_RESOLUTION_SERVICE_URL_ATTRIBUTE,
"Artifact Resolution Service is not a valid URL", "samlAssertionConsumerUrlPostURLInvalid",
null, null,
"Artifact Resolution Service uses an illegal scheme", "samlAssertionConsumerUrlPostURLIllegalSchemeError");

private String fieldId;

Expand Down Expand Up @@ -174,6 +220,19 @@ private void validateUrls(ValidationContext<ClientModel> context) {
checkUriLogo(FieldMessages.LOGO_URI, client.getAttribute(ClientModel.LOGO_URI), context);
checkUri(FieldMessages.POLICY_URI, client.getAttribute(ClientModel.POLICY_URI), context, true, false);
checkUri(FieldMessages.TOS_URI, client.getAttribute(ClientModel.TOS_URI), context, true, false);

// extra validation URLs for SAML clients
if (SamlProtocol.LOGIN_PROTOCOL.equals(client.getProtocol())) {
checkUri(FieldMessages.ADMIN_URL, client.getManagementUrl(), context, true, false);
checkUri(FieldMessages.SAML_ASSERTION_CONSUMER_URL_POST_URI, client.getAttribute(SamlProtocol.SAML_ASSERTION_CONSUMER_URL_POST_ATTRIBUTE), context, true, false);
checkUri(FieldMessages.SAML_ASSERTION_CONSUMER_URL_REDIRECT_URI, client.getAttribute(SamlProtocol.SAML_ASSERTION_CONSUMER_URL_REDIRECT_ATTRIBUTE), context, true, false);
checkUri(FieldMessages.SAML_ASSERTION_CONSUMER_URL_ARTIFACT_URI, client.getAttribute(SamlProtocol.SAML_ASSERTION_CONSUMER_URL_ARTIFACT_ATTRIBUTE), context, true, false);
checkUri(FieldMessages.SAML_SINGLE_LOGOUT_SERVICE_URL_POST_URI, client.getAttribute(SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_POST_ATTRIBUTE), context, true, false);
checkUri(FieldMessages.SAML_SINGLE_LOGOUT_SERVICE_URL_ARTIFACT_URI, client.getAttribute(SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_ARTIFACT_ATTRIBUTE), context, true, false);
checkUri(FieldMessages.SAML_SINGLE_LOGOUT_SERVICE_URL_REDIRECT_URI, client.getAttribute(SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_REDIRECT_ATTRIBUTE), context, true, false);
checkUri(FieldMessages.SAML_SINGLE_LOGOUT_SERVICE_URL_SOAP_URI, client.getAttribute(SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_SOAP_ATTRIBUTE), context, true, false);
checkUri(FieldMessages.SAML_ARTIFACT_RESOLUTION_SERVICE_URL_URI, client.getAttribute(SamlProtocol.SAML_ARTIFACT_RESOLUTION_SERVICE_URL_ATTRIBUTE), context, true, false);
}
}

private void checkUri(FieldMessages field, String url, ValidationContext<ClientModel> context, boolean checkValidUrl, boolean checkFragment) {
Expand Down
52 changes: 52 additions & 0 deletions ...egration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/ClientTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import org.keycloak.protocol.oidc.OIDCConfigAttributes;
import org.keycloak.protocol.oidc.OIDCLoginProtocol;
import org.keycloak.protocol.oidc.OIDCLoginProtocolFactory;
import org.keycloak.protocol.saml.SamlProtocol;
import org.keycloak.representations.adapters.action.GlobalRequestResult;
import org.keycloak.representations.adapters.action.PushNotBeforeAction;
import org.keycloak.representations.adapters.action.TestAvailabilityAction;
Expand All @@ -72,6 +73,7 @@
import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
Expand All @@ -95,11 +97,18 @@ public void getClients() {
}

private ClientRepresentation createClient() {
return createClient(null);
}

private ClientRepresentation createClient(String protocol) {
ClientRepresentation rep = new ClientRepresentation();
rep.setClientId("my-app");
rep.setDescription("my-app description");
rep.setEnabled(true);
rep.setPublicClient(true);
if (protocol != null) {
rep.setProtocol(protocol);
}
Response response = realm.clients().create(rep);
response.close();
String id = ApiUtil.getCreatedId(response);
Expand Down Expand Up @@ -185,6 +194,49 @@ public void testFragmentProhibitedClientValidation() {
);
}

@Test
public void testSamlSpecificUrls() {
testSamlSpecificUrls(true, "javascript:alert('TEST')", "data:text/html;base64,PHNjcmlwdD5jb25maXJtKGRvY3VtZW50LmRvbWFpbik7PC9zY3JpcHQ+");
testSamlSpecificUrls(false, "javascript:alert('TEST')", "data:text/html;base64,PHNjcmlwdD5jb25maXJtKGRvY3VtZW50LmRvbWFpbik7PC9zY3JpcHQ+");
}

private void testSamlSpecificUrls(boolean create, String... testUrls) {
ClientRepresentation rep;
if (create) {
rep = new ClientRepresentation();
rep.setClientId("my-app2");
rep.setEnabled(true);
rep.setProtocol(SamlProtocol.LOGIN_PROTOCOL);
}
else {
rep = createClient(SamlProtocol.LOGIN_PROTOCOL);
}
rep.setAttributes(new HashMap<>());

Map<String, String> attrs = Map.of(
SamlProtocol.SAML_ASSERTION_CONSUMER_URL_POST_ATTRIBUTE, "Assertion Consumer Service POST Binding URL",
SamlProtocol.SAML_ASSERTION_CONSUMER_URL_REDIRECT_ATTRIBUTE, "Assertion Consumer Service Redirect Binding URL",
SamlProtocol.SAML_ASSERTION_CONSUMER_URL_ARTIFACT_ATTRIBUTE, "Artifact Binding URL",
SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_POST_ATTRIBUTE, "Logout Service POST Binding URL",
SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_ARTIFACT_ATTRIBUTE, "Logout Service ARTIFACT Binding URL",
SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_REDIRECT_ATTRIBUTE, "Logout Service Redirect Binding URL",
SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_SOAP_ATTRIBUTE, "Logout Service SOAP Binding URL",
SamlProtocol.SAML_ARTIFACT_RESOLUTION_SERVICE_URL_ATTRIBUTE, "Artifact Resolution Service");

for (String testUrl : testUrls) {
// admin url
rep.setAdminUrl(testUrl);
createOrUpdateClientExpectingValidationErrors(rep, create, "Master SAML Processing URL uses an illegal scheme");
rep.setAdminUrl(null);
// attributes
for (Map.Entry<String, String> entry : attrs.entrySet()) {
rep.getAttributes().put(entry.getKey(), testUrl);
createOrUpdateClientExpectingValidationErrors(rep, create, entry.getValue() + " uses an illegal scheme");
rep.getAttributes().remove(entry.getKey());
}
}
}

private void testClientUriValidation(String expectedRootUrlError, String expectedBaseUrlError, String expectedBackchannelLogoutUrlError, String expectedRedirectUrisError, String... testUrls) {
testClientUriValidation(false, expectedRootUrlError, expectedBaseUrlError, expectedBackchannelLogoutUrlError, expectedRedirectUrisError, testUrls);
testClientUriValidation(true, expectedRootUrlError, expectedBaseUrlError, expectedBackchannelLogoutUrlError, expectedRedirectUrisError, testUrls);
Expand Down
32 changes: 16 additions & 16 deletions ...illian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/InstallationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,30 +229,30 @@ public void testSamlMetadataSpDescriptorPost() throws Exception {
attrNamesAndValues.clear();

//fallback to adminUrl
updater.setAdminUrl("admin-url").update();
updater.setAdminUrl("https://admin-url").update();
assertAdminEvents.assertEvent(getRealmId(), OperationType.UPDATE, AdminEventPaths.clientResourcePath(samlClientId), ResourceType.CLIENT);
doc = getDocumentFromXmlString(updater.getResource().getInstallationProvider(SamlSPDescriptorClientInstallation.SAML_CLIENT_INSTALATION_SP_DESCRIPTOR));
attrNamesAndValues.put("Binding", JBossSAMLURIConstants.SAML_HTTP_POST_BINDING.get());
attrNamesAndValues.put("Location", "admin-url");
attrNamesAndValues.put("Location", "https://admin-url");
assertElements(doc, METADATA_NSURI.get(), "SingleLogoutService", attrNamesAndValues);
assertElements(doc, METADATA_NSURI.get(), "AssertionConsumerService", attrNamesAndValues);
attrNamesAndValues.clear();

//fine grained
updater.setAttribute(SamlProtocol.SAML_ASSERTION_CONSUMER_URL_POST_ATTRIBUTE, "saml-assertion-post-url")
.setAttribute(SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_POST_ATTRIBUTE, "saml-logout-post-url")
.setAttribute(SamlProtocol.SAML_ASSERTION_CONSUMER_URL_REDIRECT_ATTRIBUTE, "saml-assertion-redirect-url")
.setAttribute(SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_REDIRECT_ATTRIBUTE, "saml-logout-redirect-url")
updater.setAttribute(SamlProtocol.SAML_ASSERTION_CONSUMER_URL_POST_ATTRIBUTE, "https://saml-assertion-post-url")
.setAttribute(SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_POST_ATTRIBUTE, "https://saml-logout-post-url")
.setAttribute(SamlProtocol.SAML_ASSERTION_CONSUMER_URL_REDIRECT_ATTRIBUTE, "https://saml-assertion-redirect-url")
.setAttribute(SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_REDIRECT_ATTRIBUTE, "https://saml-logout-redirect-url")
.update();
assertAdminEvents.assertEvent(getRealmId(), OperationType.UPDATE, AdminEventPaths.clientResourcePath(samlClientId), ResourceType.CLIENT);

doc = getDocumentFromXmlString(updater.getResource().getInstallationProvider(SamlSPDescriptorClientInstallation.SAML_CLIENT_INSTALATION_SP_DESCRIPTOR));
attrNamesAndValues.put("Binding", JBossSAMLURIConstants.SAML_HTTP_POST_BINDING.get());
attrNamesAndValues.put("Location", "saml-logout-post-url");
attrNamesAndValues.put("Location", "https://saml-logout-post-url");
assertElements(doc, METADATA_NSURI.get(), "SingleLogoutService", attrNamesAndValues);
attrNamesAndValues.clear();
attrNamesAndValues.put("Binding", JBossSAMLURIConstants.SAML_HTTP_POST_BINDING.get());
attrNamesAndValues.put("Location", "saml-assertion-post-url");
attrNamesAndValues.put("Location", "https://saml-assertion-post-url");
assertElements(doc, METADATA_NSURI.get(), "AssertionConsumerService", attrNamesAndValues);
}
assertAdminEvents.assertEvent(getRealmId(), OperationType.UPDATE, AdminEventPaths.clientResourcePath(samlClientId), ResourceType.CLIENT);
Expand All @@ -277,29 +277,29 @@ public void testSamlMetadataSpDescriptorRedirect() throws Exception {
attrNamesAndValues.clear();

//fallback to adminUrl
updater.setAdminUrl("admin-url").update();
updater.setAdminUrl("https://admin-url").update();
assertAdminEvents.assertEvent(getRealmId(), OperationType.UPDATE, AdminEventPaths.clientResourcePath(samlClientId), ResourceType.CLIENT);
doc = getDocumentFromXmlString(updater.getResource().getInstallationProvider(SamlSPDescriptorClientInstallation.SAML_CLIENT_INSTALATION_SP_DESCRIPTOR));
attrNamesAndValues.put("Binding", JBossSAMLURIConstants.SAML_HTTP_REDIRECT_BINDING.get());
attrNamesAndValues.put("Location", "admin-url");
attrNamesAndValues.put("Location", "https://admin-url");
assertElements(doc, METADATA_NSURI.get(), "SingleLogoutService", attrNamesAndValues);
assertElements(doc, METADATA_NSURI.get(), "AssertionConsumerService", attrNamesAndValues);
attrNamesAndValues.clear();

//fine grained
updater.setAttribute(SamlProtocol.SAML_ASSERTION_CONSUMER_URL_POST_ATTRIBUTE, "saml-assertion-post-url")
.setAttribute(SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_POST_ATTRIBUTE, "saml-logout-post-url")
.setAttribute(SamlProtocol.SAML_ASSERTION_CONSUMER_URL_REDIRECT_ATTRIBUTE, "saml-assertion-redirect-url")
.setAttribute(SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_REDIRECT_ATTRIBUTE, "saml-logout-redirect-url")
updater.setAttribute(SamlProtocol.SAML_ASSERTION_CONSUMER_URL_POST_ATTRIBUTE, "https://saml-assertion-post-url")
.setAttribute(SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_POST_ATTRIBUTE, "https://saml-logout-post-url")
.setAttribute(SamlProtocol.SAML_ASSERTION_CONSUMER_URL_REDIRECT_ATTRIBUTE, "https://saml-assertion-redirect-url")
.setAttribute(SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_REDIRECT_ATTRIBUTE, "https://saml-logout-redirect-url")
.update();
assertAdminEvents.assertEvent(getRealmId(), OperationType.UPDATE, AdminEventPaths.clientResourcePath(samlClientId), ResourceType.CLIENT);
doc = getDocumentFromXmlString(updater.getResource().getInstallationProvider(SamlSPDescriptorClientInstallation.SAML_CLIENT_INSTALATION_SP_DESCRIPTOR));
attrNamesAndValues.put("Binding", JBossSAMLURIConstants.SAML_HTTP_REDIRECT_BINDING.get());
attrNamesAndValues.put("Location", "saml-logout-redirect-url");
attrNamesAndValues.put("Location", "https://saml-logout-redirect-url");
assertElements(doc, METADATA_NSURI.get(), "SingleLogoutService", attrNamesAndValues);
attrNamesAndValues.clear();
attrNamesAndValues.put("Binding", JBossSAMLURIConstants.SAML_HTTP_REDIRECT_BINDING.get());
attrNamesAndValues.put("Location", "saml-assertion-redirect-url");
attrNamesAndValues.put("Location", "https://saml-assertion-redirect-url");
assertElements(doc, METADATA_NSURI.get(), "AssertionConsumerService", attrNamesAndValues);
}
assertAdminEvents.assertEvent(getRealmId(), OperationType.UPDATE, AdminEventPaths.clientResourcePath(samlClientId), ResourceType.CLIENT);
Expand Down
Loading

0 comments on commit abd03e3

Please sign in to comment.