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

KEYCLOAK-3421 Validation for URI fragments in redirect_uri

Browse source
This commit is contained in:
Vaclav Muzikar 2016-08-31 13:07:33 +02:00
parent 16bd2fb8f8
commit 1b085d3e13
10 changed files with 392 additions and 33 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

24
services/src/main/java/org/keycloak/services/clientregistration/AbstractClientRegistrationProvider.java
View file

@ -29,8 +29,12 @@ import org.keycloak.models.utils.RepresentationToModel;
import org.keycloak.representations.idm.ClientRepresentation;
import org.keycloak.services.ErrorResponseException;
import org.keycloak.services.ForbiddenException;
import org.keycloak.services.resources.admin.AdminRoot;
import org.keycloak.services.validation.ClientValidator;
import org.keycloak.services.validation.ValidationMessages;
import javax.ws.rs.core.Response;
import java.util.Properties;
/**
* @author <a href="mailto:sthorger@redhat.com">Stian Thorgersen</a>
@ -50,6 +54,16 @@ public abstract class AbstractClientRegistrationProvider implements ClientRegist
auth.requireCreate();
ValidationMessages validationMessages = new ValidationMessages();
if (!ClientValidator.validate(client, validationMessages)) {
String errorCode = validationMessages.fieldHasError("redirectUris") ? ErrorCodes.INVALID_REDIRECT_URI : ErrorCodes.INVALID_CLIENT_METADATA;
throw new ErrorResponseException(
errorCode,
validationMessages.getStringMessages(),
Response.Status.BAD_REQUEST
);
}
try {
ClientModel clientModel = RepresentationToModel.createClient(session, session.getContext().getRealm(), client, true);
@ -104,6 +118,16 @@ public abstract class AbstractClientRegistrationProvider implements ClientRegist
throw new ErrorResponseException(ErrorCodes.INVALID_CLIENT_METADATA, "Client Identifier modified", Response.Status.BAD_REQUEST);
}
ValidationMessages validationMessages = new ValidationMessages();
if (!ClientValidator.validate(rep, validationMessages)) {
String errorCode = validationMessages.fieldHasError("redirectUris") ? ErrorCodes.INVALID_REDIRECT_URI : ErrorCodes.INVALID_CLIENT_METADATA;
throw new ErrorResponseException(
errorCode,
validationMessages.getStringMessages(),
Response.Status.BAD_REQUEST
);
}
RepresentationToModel.updateClient(rep, client);
rep = ModelToRepresentation.toRepresentation(client);

18
services/src/main/java/org/keycloak/services/resources/admin/ClientResource.java
View file

@ -40,6 +40,7 @@ import org.keycloak.representations.idm.ClientRepresentation;
import org.keycloak.representations.idm.CredentialRepresentation;
import org.keycloak.representations.idm.UserRepresentation;
import org.keycloak.representations.idm.UserSessionRepresentation;
import org.keycloak.services.ErrorResponseException;
import org.keycloak.services.ServicesLogger;
import org.keycloak.services.clientregistration.ClientRegistrationTokenUtils;
import org.keycloak.services.managers.ClientManager;
@ -48,6 +49,8 @@ import org.keycloak.services.managers.ResourceAdminManager;
import org.keycloak.services.resources.KeycloakApplication;
import org.keycloak.services.ErrorResponse;
import org.keycloak.common.util.Time;
import org.keycloak.services.validation.ClientValidator;
import org.keycloak.services.validation.ValidationMessages;
import javax.ws.rs.Consumes;
import javax.ws.rs.DELETE;
@ -63,10 +66,7 @@ import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.UriInfo;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.*;
import static java.lang.Boolean.TRUE;
@ -126,6 +126,16 @@ public class ClientResource {
throw new NotFoundException("Could not find client");
}
ValidationMessages validationMessages = new ValidationMessages();
if (!ClientValidator.validate(rep, validationMessages)) {
Properties messages = AdminRoot.getMessages(session, realm, auth.getAuth().getToken().getLocale());
throw new ErrorResponseException(
validationMessages.getStringMessages(),
validationMessages.getStringMessages(messages),
Response.Status.BAD_REQUEST
);
}
try {
updateClientFromRep(rep, client, session);
adminEvent.operation(OperationType.UPDATE).resourcePath(uriInfo).representation(rep).success();

23
services/src/main/java/org/keycloak/services/resources/admin/ClientsResource.java
View file

@ -17,7 +17,6 @@
package org.keycloak.services.resources.admin;
import org.jboss.resteasy.annotations.cache.NoCache;
import org.jboss.resteasy.spi.NotFoundException;
import org.jboss.resteasy.spi.ResteasyProviderFactory;
import org.keycloak.events.admin.OperationType;
import org.keycloak.events.admin.ResourceType;
@ -28,16 +27,13 @@ import org.keycloak.models.RealmModel;
import org.keycloak.models.utils.ModelToRepresentation;
import org.keycloak.representations.idm.ClientRepresentation;
import org.keycloak.services.ErrorResponse;
import org.keycloak.services.ErrorResponseException;
import org.keycloak.services.ServicesLogger;
import org.keycloak.services.managers.ClientManager;
import org.keycloak.services.validation.ClientValidator;
import org.keycloak.services.validation.ValidationMessages;
import javax.ws.rs.Consumes;
import javax.ws.rs.GET;
import javax.ws.rs.POST;
import javax.ws.rs.Path;
import javax.ws.rs.PathParam;
import javax.ws.rs.Produces;
import javax.ws.rs.QueryParam;
import javax.ws.rs.*;
import javax.ws.rs.core.Context;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
@ -45,6 +41,7 @@ import javax.ws.rs.core.UriInfo;
import java.util.ArrayList;
import java.util.List;
import java.util.Properties;
/**
* Base resource class for managing a realm's clients.
@ -122,6 +119,16 @@ public class ClientsResource {
public Response createClient(final @Context UriInfo uriInfo, final ClientRepresentation rep) {
auth.requireManage();
ValidationMessages validationMessages = new ValidationMessages();
if (!ClientValidator.validate(rep, validationMessages)) {
Properties messages = AdminRoot.getMessages(session, realm, auth.getAuth().getToken().getLocale());
throw new ErrorResponseException(
validationMessages.getStringMessages(),
validationMessages.getStringMessages(messages),
Response.Status.BAD_REQUEST
);
}
try {
ClientModel clientModel = ClientManager.createClient(session, realm, rep, true);

54
services/src/main/java/org/keycloak/services/validation/ClientValidator.java Normal file
View file

@ -0,0 +1,54 @@
/*
*
* * Copyright 2016 Red Hat, Inc. and/or its affiliates
* * and other contributors as indicated by the @author tags.
* *
* * Licensed under the Apache License, Version 2.0 (the "License");
* * you may not use this file except in compliance with the License.
* * You may obtain a copy of the License at
* *
* * http://www.apache.org/licenses/LICENSE-2.0
* *
* * Unless required by applicable law or agreed to in writing, software
* * distributed under the License is distributed on an "AS IS" BASIS,
* * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* * See the License for the specific language governing permissions and
* * limitations under the License.
*
*/
package org.keycloak.services.validation;
import org.keycloak.representations.idm.ClientRepresentation;
/**
* @author Vaclav Muzikar <vmuzikar@redhat.com>
*/
public class ClientValidator {
/**
* Checks if the Client's Redirect URIs doesn't contain any URI fragments (like http://example.org/auth#fragment)
*
* @see <a href="https://issues.jboss.org/browse/KEYCLOAK-3421">KEYCLOAK-3421</a>
* @param client
* @param messages
* @return true if Redirect URIs doesn't contain any URI with fragments
*/
public static boolean validate(ClientRepresentation client, ValidationMessages messages) {
boolean isValid = true;
if (client.getRedirectUris() != null) {
long urisWithFragmentCount = client.getRedirectUris().stream().filter(p -> p.contains("#")).count();
if (urisWithFragmentCount > 0) {
messages.add("redirectUris", "Redirect URIs must not contain an URI fragment", "clientRedirectURIsFragmentError");
isValid = false;
}
}
if (client.getRootUrl() != null && client.getRootUrl().contains("#")) {
messages.add("rootUrl", "Root URL must not contain an URL fragment", "clientRootURLFragmentError");
isValid = false;
}
return isValid;
}
}

100
services/src/main/java/org/keycloak/services/validation/ValidationMessage.java Normal file
View file

@ -0,0 +1,100 @@
/*
*
* * Copyright 2016 Red Hat, Inc. and/or its affiliates
* * and other contributors as indicated by the @author tags.
* *
* * Licensed under the Apache License, Version 2.0 (the "License");
* * you may not use this file except in compliance with the License.
* * You may obtain a copy of the License at
* *
* * http://www.apache.org/licenses/LICENSE-2.0
* *
* * Unless required by applicable law or agreed to in writing, software
* * distributed under the License is distributed on an "AS IS" BASIS,
* * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* * See the License for the specific language governing permissions and
* * limitations under the License.
*
*/
package org.keycloak.services.validation;
import java.text.MessageFormat;
import java.util.Properties;
/**
* @author Vaclav Muzikar <vmuzikar@redhat.com>
*/
public class ValidationMessage {
private String fieldId;
private String message;
private String localizedMessageKey;
private Object[] localizedMessageParameters;
public ValidationMessage(String message) {
this.message = message;
}
public ValidationMessage(String message, String localizedMessageKey, Object... localizedMessageParameters) {
this.message = message;
this.localizedMessageKey = localizedMessageKey;
this.localizedMessageParameters = localizedMessageParameters;
}
public String getFieldId() {
return fieldId;
}
public void setFieldId(String fieldId) {
this.fieldId = fieldId;
}
public String getLocalizedMessageKey() {
return localizedMessageKey;
}
public void setLocalizedMessageKey(String localizedMessageKey) {
this.localizedMessageKey = localizedMessageKey;
}
public Object[] getLocalizedMessageParameters() {
return localizedMessageParameters;
}
public void setLocalizedMessageParameters(Object[] localizedMessageParameters) {
this.localizedMessageParameters = localizedMessageParameters;
}
public String getMessage() {
return message;
}
public String getMessage(Properties localizedMessages) {
if (getLocalizedMessageKey() != null) {
return MessageFormat.format(localizedMessages.getProperty(getLocalizedMessageKey(), getMessage()), getLocalizedMessageParameters());
}
else {
return getMessage();
}
}
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
ValidationMessage message1 = (ValidationMessage) o;
if (getFieldId() != null ? !getFieldId().equals(message1.getFieldId()) : message1.getFieldId() != null)
return false;
return getMessage() != null ? getMessage().equals(message1.getMessage()) : message1.getMessage() == null;
}
@Override
public int hashCode() {
int result = getFieldId() != null ? getFieldId().hashCode() : 0;
result = 31 * result + (getMessage() != null ? getMessage().hashCode() : 0);
return result;
}
}

83
services/src/main/java/org/keycloak/services/validation/ValidationMessages.java Normal file
View file

@ -0,0 +1,83 @@
/*
*
* * Copyright 2016 Red Hat, Inc. and/or its affiliates
* * and other contributors as indicated by the @author tags.
* *
* * Licensed under the Apache License, Version 2.0 (the "License");
* * you may not use this file except in compliance with the License.
* * You may obtain a copy of the License at
* *
* * http://www.apache.org/licenses/LICENSE-2.0
* *
* * Unless required by applicable law or agreed to in writing, software
* * distributed under the License is distributed on an "AS IS" BASIS,
* * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* * See the License for the specific language governing permissions and
* * limitations under the License.
*
*/
package org.keycloak.services.validation;
import java.util.*;
import java.util.function.Function;
import java.util.stream.Collectors;
/**
* @author Vaclav Muzikar <vmuzikar@redhat.com>
*/
public class ValidationMessages {
private Set<ValidationMessage> messages = new LinkedHashSet<>();
public ValidationMessages() {}
public ValidationMessages(String... messages) {
for (String message : messages) {
add(message);
}
}
public void add(String message) {
messages.add(new ValidationMessage(message));
}
public void add(String message, String localizedMessageKey) {
messages.add(new ValidationMessage(message, localizedMessageKey));
}
public void add(String fieldId, String message, String localizedMessageKey) {
ValidationMessage validationMessage = new ValidationMessage(message, localizedMessageKey);
validationMessage.setFieldId(fieldId);
add(validationMessage);
}
public void add(ValidationMessage message) {
messages.add(message);
}
public boolean fieldHasError(String fieldId) {
for (ValidationMessage message : messages) {
if (message.getFieldId().equals(fieldId)) {
return true;
}
}
return false;
}
public Set<ValidationMessage> getMessages() {
return Collections.unmodifiableSet(messages);
}
protected String getStringMessages(Function<? super ValidationMessage, ? extends String> function) {
return messages.stream().map(function).collect(Collectors.joining("; "));
}
public String getStringMessages() {
return getStringMessages(ValidationMessage::getMessage);
}
public String getStringMessages(Properties localizedMessages) {
return getStringMessages(x -> x.getMessage(localizedMessages));
}
}

54
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/ClientTest.java
View file

@ -33,16 +33,14 @@ import org.keycloak.representations.adapters.action.PushNotBeforeAction;
import org.keycloak.representations.adapters.action.TestAvailabilityAction;
import org.keycloak.representations.idm.*;
import javax.ws.rs.BadRequestException;
import javax.ws.rs.NotFoundException;
import javax.ws.rs.core.Response;
import java.io.IOException;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.*;
import org.keycloak.services.ErrorResponseException;
import org.keycloak.testsuite.Assert;
import org.keycloak.testsuite.util.AdminEventPaths;
import org.keycloak.testsuite.util.ClientBuilder;
@ -212,6 +210,52 @@ public class ClientTest extends AbstractAdminTest {
assertEquals("service-account-serviceclient", userRep.getUsername());
}
// KEYCLOAK-3421
@Test
public void createClientWithFragments() {
ClientRepresentation client = ClientBuilder.create()
.clientId("client-with-fragment")
.rootUrl("http://localhost/base#someFragment")
.redirectUris("http://localhost/auth", "http://localhost/auth#fragment", "http://localhost/auth*", "/relative")
.build();
Response response = realm.clients().create(client);
assertUriFragmentError(response);
}
// KEYCLOAK-3421
@Test
public void updateClientWithFragments() {
ClientRepresentation client = ClientBuilder.create()
.clientId("client-with-fragment")
.redirectUris("http://localhost/auth", "http://localhost/auth*")
.build();
Response response = realm.clients().create(client);
ClientResource clientResource = realm.clients().get(ApiUtil.getCreatedId(response));
client = clientResource.toRepresentation();
client.setRootUrl("http://localhost/base#someFragment");
List<String> redirectUris = client.getRedirectUris();
redirectUris.add("http://localhost/auth#fragment");
redirectUris.add("/relative");
client.setRedirectUris(redirectUris);
try {
clientResource.update(client);
fail("Should fail");
}
catch (BadRequestException e) {
assertUriFragmentError(e.getResponse());
}
}
private void assertUriFragmentError(Response response) {
assertEquals(response.getStatus(), 400);
String error = response.readEntity(OAuth2ErrorRepresentation.class).getError();
assertTrue("Error response doesn't mention Redirect URIs fragments", error.contains("Redirect URIs must not contain an URI fragment"));
assertTrue("Error response doesn't mention Root URL fragments", error.contains("Root URL must not contain an URL fragment"));
}
@Test
public void pushRevocation() {
testingClient.testApp().clearAdminActions();

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

@ -102,6 +102,23 @@ public class OIDCClientRegistrationTest extends AbstractClientRegistrationTest {
return response;
}
private void assertCreateFail(OIDCClientRepresentation client, int expectedStatusCode) {
assertCreateFail(client, expectedStatusCode, null);
}
private void assertCreateFail(OIDCClientRepresentation client, int expectedStatusCode, String expectedErrorContains) {
try {
reg.oidc().create(client);
Assert.fail("Not expected to successfuly register client");
} catch (ClientRegistrationException expected) {
HttpErrorException httpEx = (HttpErrorException) expected.getCause();
Assert.assertEquals(expectedStatusCode, httpEx.getStatusLine().getStatusCode());
if (expectedErrorContains != null) {
assertTrue("Error response doesn't contain expected text", httpEx.getErrorResponse().contains(expectedErrorContains));
}
}
}
@Test
public void testCreateWithTrustedHost() throws Exception {
reg.auth(null);
@ -109,17 +126,10 @@ public class OIDCClientRegistrationTest extends AbstractClientRegistrationTest {
OIDCClientRepresentation client = createRep();
// Failed to create client
try {
reg.oidc().create(client);
Assert.fail("Not expected to successfuly register client");
} catch (ClientRegistrationException expected) {
HttpErrorException httpEx = (HttpErrorException) expected.getCause();
Assert.assertEquals(401, httpEx.getStatusLine().getStatusCode());
}
assertCreateFail(client, 401);
// Create trusted host entry
Response response = adminClient.realm(REALM_NAME).clientRegistrationTrustedHost().create(ClientRegistrationTrustedHostRepresentation.create("localhost", 2, 2));
Assert.assertEquals(201, response.getStatus());
createTrustedHost("localhost", 2);
// Successfully register client
reg.oidc().create(client);
@ -132,13 +142,20 @@ public class OIDCClientRegistrationTest extends AbstractClientRegistrationTest {
reg.oidc().create(client);
// Failed to create 3rd client
try {
reg.oidc().create(client);
Assert.fail("Not expected to successfuly register client");
} catch (ClientRegistrationException expected) {
HttpErrorException httpEx = (HttpErrorException) expected.getCause();
Assert.assertEquals(401, httpEx.getStatusLine().getStatusCode());
}
assertCreateFail(client, 401);
}
// KEYCLOAK-3421
@Test
public void createClientWithUriFragment() {
reg.auth(null);
createTrustedHost("localhost", 1);
OIDCClientRepresentation client = createRep();
client.setRedirectUris(Arrays.asList("http://localhost/auth", "http://localhost/auth#fragment", "http://localhost/auth*"));
assertCreateFail(client, 400, "URI fragment");
}
@Test
@ -323,4 +340,9 @@ public class OIDCClientRegistrationTest extends AbstractClientRegistrationTest {
}
}
private void createTrustedHost(String name, int count) {
Response response = adminClient.realm(REALM_NAME).clientRegistrationTrustedHost().create(ClientRegistrationTrustedHostRepresentation.create(name, count, count));
Assert.assertEquals(201, response.getStatus());
}
}

3
themes/src/main/resources/theme/base/admin/messages/messages_en.properties
View file

@ -12,3 +12,6 @@ ldapErrorMissingClientId=Client ID needs to be provided in config when Realm Rol
ldapErrorCantPreserveGroupInheritanceWithUIDMembershipType=Not possible to preserve group inheritance and use UID membership type together.
ldapErrorCantWriteOnlyForReadOnlyLdap=Can't set write only when LDAP provider mode is not WRITABLE
ldapErrorCantWriteOnlyAndReadOnly=Can't set write-only and read-only together
clientRedirectURIsFragmentError=Redirect URIs must not contain an URI fragment
clientRootURLFragmentError=Root URL must not contain an URL fragment

12
themes/src/main/resources/theme/base/admin/resources/js/controllers/clients.js
View file

@ -1111,6 +1111,12 @@ module.controller('ClientDetailCtrl', function($scope, realm, client, templates,
}, $scope.client, function() {
$route.reload();
Notifications.success("Your changes have been saved to the client.");
}, function(error) {
if (error.status == 400 && error.data.error_description) {
Notifications.error(error.data.error_description);
} else {
Notifications.error('Unexpected error when updating client');
}
});
}
};
@ -1225,6 +1231,12 @@ module.controller('CreateClientCtrl', function($scope, realm, client, templates,
var id = l.substring(l.lastIndexOf("/") + 1);
$location.url("/realms/" + realm.realm + "/clients/" + id);
Notifications.success("The client has been created.");
}, function(error) {
if (error.status == 400 && error.data.error_description) {
Notifications.error(error.data.error_description);
} else {
Notifications.error('Unexpected error when creating client');
}
});
};