KEYCLOAK-12768: Prevent reserved characters in URLs

This commit is contained in:
Stan Silvert 2020-02-27 08:44:47 -05:00 committed by Stian Thorgersen
parent 256bbff769
commit fff8571cfd
28 changed files with 196 additions and 15 deletions

View file

@ -59,6 +59,7 @@ import org.keycloak.services.clientregistration.policy.DefaultClientRegistration
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import org.keycloak.utils.ReservedCharValidator;
/**
* Per request object
@ -98,6 +99,7 @@ public class RealmManager {
public RealmModel createRealm(String id, String name) {
if (id == null) id = KeycloakModelUtils.generateId();
ReservedCharValidator.validate(name);
RealmModel realm = model.createRealm(id, name);
realm.setName(name);
@ -505,6 +507,7 @@ public class RealmManager {
id = KeycloakModelUtils.generateId();
}
RealmModel realm = model.createRealm(id, rep.getRealm());
ReservedCharValidator.validate(rep.getRealm());
realm.setName(rep.getRealm());
// setup defaults

View file

@ -72,6 +72,7 @@ import java.util.Map;
import java.util.Optional;
import static javax.ws.rs.core.Response.Status.NOT_FOUND;
import org.keycloak.utils.ReservedCharValidator;
/**
* @resource Authentication Management
@ -215,6 +216,8 @@ public class AuthenticationManagementResource {
return ErrorResponse.exists("Flow " + flow.getAlias() + " already exists");
}
ReservedCharValidator.validate(flow.getAlias());
AuthenticationFlowModel createdModel = realm.addAuthenticationFlow(RepresentationToModel.toModel(flow));
flow.setId(createdModel.getId());
@ -789,6 +792,8 @@ public class AuthenticationManagementResource {
public Response newExecutionConfig(@PathParam("executionId") String execution, AuthenticatorConfigRepresentation json) {
auth.realm().requireManageRealm();
ReservedCharValidator.validate(json.getAlias());
AuthenticationExecutionModel model = realm.getAuthenticationExecutionById(execution);
if (model == null) {
session.getTransactionManager().setRollbackOnly();
@ -1137,6 +1142,8 @@ public class AuthenticationManagementResource {
public Response createAuthenticatorConfig(AuthenticatorConfigRepresentation rep) {
auth.realm().requireManageRealm();
ReservedCharValidator.validate(rep.getAlias());
AuthenticatorConfigModel config = realm.addAuthenticatorConfig(RepresentationToModel.toModel(rep));
adminEvent.operation(OperationType.CREATE).resource(ResourceType.AUTHENTICATOR_CONFIG).resourcePath(session.getContext().getUri(), config.getId()).representation(rep).success();
return Response.created(session.getContext().getUri().getAbsolutePathBuilder().path(config.getId()).build()).build();
@ -1202,11 +1209,12 @@ public class AuthenticationManagementResource {
public void updateAuthenticatorConfig(@PathParam("id") String id, AuthenticatorConfigRepresentation rep) {
auth.realm().requireManageRealm();
ReservedCharValidator.validate(rep.getAlias());
AuthenticatorConfigModel exists = realm.getAuthenticatorConfigById(id);
if (exists == null) {
throw new NotFoundException("Could not find authenticator config");
}
exists.setAlias(rep.getAlias());
exists.setConfig(RepresentationToModel.removeEmptyString(rep.getConfig()));
realm.updateAuthenticatorConfig(exists);

View file

@ -87,6 +87,7 @@ import java.util.Map;
import java.util.Properties;
import static java.lang.Boolean.TRUE;
import org.keycloak.utils.ReservedCharValidator;
/**
@ -565,6 +566,9 @@ public class ClientResource {
if (node == null) {
throw new BadRequestException("Node not found in params");
}
ReservedCharValidator.validate(node);
if (logger.isDebugEnabled()) logger.debug("Register node: " + node);
client.registerNode(node, Time.currentTime());
adminEvent.operation(OperationType.CREATE).resource(ResourceType.CLUSTER_NODE).resourcePath(session.getContext().getUri(), node).success();

View file

@ -55,6 +55,7 @@ import java.util.List;
import java.util.Map;
import static javax.ws.rs.core.Response.Status.BAD_REQUEST;
import org.keycloak.utils.ReservedCharValidator;
/**
* @resource Identity Providers
@ -134,6 +135,9 @@ public class IdentityProvidersResource {
if (!(data.containsKey("providerId") && data.containsKey("fromUrl"))) {
throw new BadRequestException();
}
ReservedCharValidator.validate((String)data.get("alias"));
String providerId = data.get("providerId").toString();
String from = data.get("fromUrl").toString();
InputStream inputStream = session.getProvider(HttpClientProvider.class).get(from);
@ -182,6 +186,8 @@ public class IdentityProvidersResource {
public Response create(IdentityProviderRepresentation representation) {
this.auth.realm().requireManageIdentityProviders();
ReservedCharValidator.validate(representation.getAlias());
try {
IdentityProviderModel identityProvider = RepresentationToModel.toModel(realm, representation, session);
this.realm.addIdentityProvider(identityProvider);

View file

@ -106,6 +106,7 @@ import java.util.stream.Collectors;
import static org.keycloak.models.utils.StripSecretsUtils.stripForExport;
import static org.keycloak.util.JsonSerialization.readValue;
import org.keycloak.utils.ReservedCharValidator;
/**
* Base resource class for the admin REST api of one realm
@ -390,6 +391,8 @@ public class RealmAdminResource {
return ErrorResponse.error("Can't rename master realm", Status.BAD_REQUEST);
}
ReservedCharValidator.validate(rep.getRealm());
try {
if (!Constants.GENERATE.equals(rep.getPublicKey()) && (rep.getPrivateKey() != null && rep.getPublicKey() != null)) {
try {

View file

@ -17,7 +17,6 @@
package org.keycloak.services.resources.admin;
import org.apache.commons.lang.StringUtils;
import org.jboss.resteasy.annotations.cache.NoCache;
import javax.ws.rs.NotFoundException;
import org.keycloak.events.admin.OperationType;
@ -61,6 +60,7 @@ import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;
import org.keycloak.utils.ReservedCharValidator;
/**
* @resource Roles
@ -137,6 +137,8 @@ public class RoleContainerResource extends RoleResource {
throw new BadRequestException();
}
ReservedCharValidator.validate(rep.getName());
try {
RoleModel role = roleContainer.addRole(rep.getName());
role.setDescription(rep.getDescription());

View file

@ -0,0 +1,53 @@
/*
* Copyright 2020 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.utils;
import javax.ws.rs.BadRequestException;
import org.jboss.logging.Logger;
/**
*
* @author Stan Silvert
*/
public class ReservedCharValidator {
protected static final Logger logger = Logger.getLogger(ReservedCharValidator.class);
// https://tools.ietf.org/html/rfc3986#section-2.2
private static final String[] RESERVED_CHARS = { ":", "/", "?", "#", "[", "@", "!", "$",
"&", "(", ")", "*", "+", ",", ";", "=",
"]", "[", "\\" };
private ReservedCharValidator() {}
public static void validate(String str) throws ReservedCharException {
if (str == null) return;
for (String c : RESERVED_CHARS) {
if (str.contains(c)) {
String message = "Character '" + c + "' not allowed.";
ReservedCharException e = new ReservedCharException(message);
logger.warn(message, e);
throw e;
}
}
}
public static class ReservedCharException extends BadRequestException {
ReservedCharException(String msg) {
super(msg);
}
}
}

View file

@ -40,7 +40,7 @@ public class PatternFlyClosableAlert extends AbstractPatternFlyAlert {
}
public boolean isWarning() {
return checkAlertType("waring");
return checkAlertType("warning");
}
public boolean isDanger() {

View file

@ -422,6 +422,16 @@ public class ClientTest extends AbstractAdminTest {
return client;
}
@Test (expected = BadRequestException.class)
public void testAddNodeWithReservedCharacter() {
testingClient.testApp().clearAdminActions();
ClientRepresentation client = createAppClient();
String id = client.getId();
realm.clients().get(id).registerNode(Collections.singletonMap("node", "foo#"));
}
@Test
public void nodes() {
testingClient.testApp().clearAdminActions();

View file

@ -123,6 +123,17 @@ public class IdentityProviderTest extends AbstractAdminTest {
Assert.assertNames(realm.identityProviders().findAll(), "google", "facebook");
}
@Test
public void testCreateWithReservedCharacterForAlias() {
IdentityProviderRepresentation newIdentityProvider = createRep("ne$&w-identity-provider", "oidc");
newIdentityProvider.getConfig().put("clientId", "clientId");
newIdentityProvider.getConfig().put("clientSecret", "some secret value");
Response response = realm.identityProviders().create(newIdentityProvider);
Assert.assertEquals(400, response.getStatus());
}
@Test
public void testCreate() {
IdentityProviderRepresentation newIdentityProvider = createRep("new-identity-provider", "oidc");

View file

@ -34,6 +34,7 @@ import javax.ws.rs.core.Response;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import javax.ws.rs.BadRequestException;
/**
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
@ -58,6 +59,12 @@ public class AuthenticatorConfigTest extends AbstractAuthenticationTest {
executionId = exec.getId();
}
@Test
public void testCreateConfigWithReservedChar() {
AuthenticatorConfigRepresentation cfg = newConfig("f!oo", IdpCreateUserIfUniqueAuthenticatorFactory.REQUIRE_PASSWORD_UPDATE_AFTER_REGISTRATION, "true");
Response resp = authMgmtResource.newExecutionConfig(executionId, cfg);
Assert.assertEquals(400, resp.getStatus());
}
@Test
public void testCreateConfig() {
@ -80,6 +87,15 @@ public class AuthenticatorConfigTest extends AbstractAuthenticationTest {
assertAdminEvents.assertEvent(REALM_NAME, OperationType.DELETE, AdminEventPaths.authExecutionConfigPath(cfgId), ResourceType.AUTHENTICATOR_CONFIG);
}
@Test (expected = BadRequestException.class)
public void testUpdateConfigWithBadChar() {
AuthenticatorConfigRepresentation cfg = newConfig("foo", IdpCreateUserIfUniqueAuthenticatorFactory.REQUIRE_PASSWORD_UPDATE_AFTER_REGISTRATION, "true");
String cfgId = createConfig(executionId, cfg);
AuthenticatorConfigRepresentation cfgRep = authMgmtResource.getAuthenticatorConfig(cfgId);
cfgRep.setAlias("Bad@Char");
authMgmtResource.updateAuthenticatorConfig(cfgRep.getId(), cfgRep);
}
@Test
public void testUpdateConfig() {

View file

@ -72,6 +72,12 @@ public class FlowTest extends AbstractAuthenticationTest {
authMgmtResource.addExecutionFlow(parentAlias, data);
}
@Test
public void testAddFlowWithRestrictedCharInAlias() {
Response resp = authMgmtResource.createFlow(newFlow("fo]o", "Browser flow", "basic-flow", true, false));
Assert.assertEquals(400, resp.getStatus());
}
@Test
public void testAddRemoveFlow() {

View file

@ -39,6 +39,7 @@ import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import javax.ws.rs.BadRequestException;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
@ -88,6 +89,12 @@ public class ClientRolesTest extends AbstractClientTest {
assertTrue(hasRole(rolesRsc, "role1"));
}
@Test(expected = BadRequestException.class)
public void testAddRoleWithReservedCharacter() {
RoleRepresentation role1 = makeRole("r&ole1");
rolesRsc.create(role1);
}
@Test
public void testRemoveRole() {
RoleRepresentation role2 = makeRole("role2");

View file

@ -158,6 +158,13 @@ public class RealmTest extends AbstractAdminTest {
Assert.assertNames(adminClient.realms().findAll(), "master", AuthRealm.TEST, REALM_NAME);
}
@Test(expected = BadRequestException.class)
public void createRealmRejectReservedChar() {
RealmRepresentation rep = new RealmRepresentation();
rep.setRealm("new-re;alm");
adminClient.realms().create(rep);
}
/**
* Checks attributes exposed as fields are not also included as attributes
*/
@ -379,6 +386,13 @@ public class RealmTest extends AbstractAdminTest {
checkRealmEventsConfigRepresentation(repOrig, actual);
}
@Test(expected = BadRequestException.class)
public void updateRealmWithReservedCharInName() {
RealmRepresentation rep = realm.toRepresentation();
rep.setRealm("fo#o");
realm.update(rep);
}
@Test
public void updateRealm() {
// first change

View file

@ -138,6 +138,10 @@ public abstract class AbstractConsoleTest extends AbstractAuthTest {
alert.assertDanger();
}
public void assertAlertWarning() {
alert.assertWarning();
}
public ConfigureMenu configure() {
return adminConsoleRealmPage.configure();
}

View file

@ -112,6 +112,21 @@ public class RealmRolesTest extends AbstractRolesTest {
assertNotNull(realmRolesPage.table().findRole(name));
}
// KEYCLOAK-12768: Certain characters in names cause bad URIs. Disallow.
@Test
public void testAddRoleWithBadCharsInName() {
String roleName = "hello;:]!@#role";
assertCurrentUrlEquals(realmRolesPage);
realmRolesPage.table().addRole();
assertCurrentUrlEquals(createRolePage);
createRolePage.form().setName(roleName);
assertAlertWarning();
createRolePage.form().save();
assertAlertSuccess();
realmRolesPage.navigateTo();
assertNotNull(realmRolesPage.table().findRole("hellorole"));
}
@Test
public void testAddExistingRole() {
addRole(testRole);

View file

@ -1608,3 +1608,5 @@ subjectdn-tooltip=A regular expression for validating Subject DN in the Client C
pkce-code-challenge-method=Proof Key for Code Exchange Code Challenge Method
pkce-code-challenge-method.tooltip=Choose which code challenge method for PKCE is used. If not specified, keycloak does not applies PKCE to a client unless the client sends an authorization request with appropriate code challenge and code exchange method.
key-not-allowed-here=Key '{{character}}' is not allowed here.

View file

@ -2384,6 +2384,23 @@ module.directive('kcEnter', function() {
};
});
// Don't allow URI reserved characters
module.directive('kcNoReservedChars', function (Notifications, $translate) {
return function($scope, element) {
element.bind("keydown keypress", function(event) {
var keyPressed = String.fromCharCode(event.which || event.keyCode || 0);
// ] and ' can not be used inside a character set on POSIX and GNU
if (keyPressed.match('[:/?#[@!$&()*+,;=]') || keyPressed === ']' || keyPressed === '\'') {
event.preventDefault();
$scope.$apply(function() {
Notifications.warn($translate.instant('key-not-allowed-here', {character: keyPressed}));
});
}
});
};
});
module.directive('kcSave', function ($compile, $timeout, Notifications) {
var clickDelay = 500; // 500 ms

View file

@ -26,7 +26,7 @@
<div class="form-group clearfix">
<label class="col-md-2 control-label" for="name">{{:: 'alias' | translate}}</label>
<div class="col-md-6">
<input class="form-control" id="name" type="text" ng-model="config.alias" data-ng-readonly="!create">
<input kc-no-reserved-chars class="form-control" id="name" type="text" ng-model="config.alias" data-ng-readonly="!create">
</div>
<kc-tooltip>{{:: 'authenticator.alias.tooltip' | translate}}</kc-tooltip>
</div>

View file

@ -17,7 +17,7 @@
<div class="form-group">
<label class="col-md-2 control-label" for="host">{{:: 'host' | translate}}</label>
<div class="col-sm-6">
<input ng-disabled="!create" class="form-control" type="text" id="host" name="host" data-ng-model="node.host" required>
<input kc-no-reserved-chars ng-disabled="!create" class="form-control" type="text" id="host" name="host" data-ng-model="node.host" required>
</div>
</div>
<div ng-hide="create" class="form-group">

View file

@ -17,7 +17,7 @@
<label class="col-md-2 control-label" for="name">{{:: 'role-name' | translate}} <span class="required" data-ng-show="create">*</span></label>
<div class="col-md-6">
<input class="form-control" type="text" id="name" name="name" data-ng-model="role.name" autofocus
<input kc-no-reserved-chars class="form-control" type="text" id="name" name="name" data-ng-model="role.name" autofocus
required data-ng-readonly="!create">
</div>
</div>

View file

@ -7,7 +7,7 @@
<div class="form-group">
<label class="col-md-2 control-label" for="alias">{{:: 'alias' | translate}} </label>
<div class="col-sm-6">
<input class="form-control" type="text" id="alias" name="alias" data-ng-model="flow.alias" autofocus required>
<input kc-no-reserved-chars class="form-control" type="text" id="alias" name="alias" data-ng-model="flow.alias" autofocus required>
</div>
<kc-tooltip>{{:: 'flow.alias.tooltip' | translate}}</kc-tooltip>
</div>

View file

@ -22,7 +22,7 @@
<label for="name" class="col-sm-2 control-label">{{:: 'name' | translate}} <span class="required">*</span></label>
<div class="col-md-6">
<input class="form-control" type="text" id="name" name="name" data-ng-model="realm.realm" autofocusrequired>
<input class="form-control" kc-no-reserved-chars type="text" id="name" name="name" data-ng-model="realm.realm" autofocusrequired>
</div>
</div>
<div class="form-group">

View file

@ -5,7 +5,7 @@
<div class="form-group">
<label class="col-md-2 control-label" for="name"><span class="required">*</span> {{:: 'name' | translate}}</label>
<div class="col-md-6">
<input class="form-control" data-ng-disabled="disableRename" type="text" id="name" name="name" data-ng-model="realmName" autofocus required>
<input kc-no-reserved-chars class="form-control" data-ng-disabled="disableRename" type="text" id="name" name="name" data-ng-model="realmName" autofocus required>
</div>
</div>

View file

@ -22,7 +22,7 @@
<div class="form-group clearfix">
<label class="col-md-2 control-label" for="identifier"><span class="required">*</span> {{:: 'alias' | translate}}</label>
<div class="col-md-6">
<input class="form-control" id="identifier" type="text" ng-model="identityProvider.alias" data-ng-readonly="!newIdentityProvider" required>
<input kc-no-reserved-chars class="form-control" id="identifier" type="text" ng-model="identityProvider.alias" data-ng-readonly="!newIdentityProvider" required>
</div>
<kc-tooltip>{{:: 'identity-provider.alias.tooltip' | translate}}</kc-tooltip>
</div>

View file

@ -22,7 +22,7 @@
<div class="form-group clearfix">
<label class="col-md-2 control-label" for="identifier"><span class="required">*</span> {{:: 'alias' | translate}}</label>
<div class="col-md-6">
<input class="form-control" id="identifier" type="text" ng-model="identityProvider.alias" data-ng-readonly="!newIdentityProvider" required>
<input kc-no-reserved-chars class="form-control" id="identifier" type="text" ng-model="identityProvider.alias" data-ng-readonly="!newIdentityProvider" required>
</div>
<kc-tooltip>{{:: 'identity-provider.alias.tooltip' | translate}}</kc-tooltip>
</div>

View file

@ -14,7 +14,7 @@
<label class="col-md-2 control-label" for="name"><span class="required" data-ng-show="create">*</span> {{:: 'role-name' | translate}}</label>
<div class="col-md-6">
<input class="form-control" type="text" id="name" name="name" data-ng-model="role.name" autofocus
<input kc-no-reserved-chars class="form-control" type="text" id="name" name="name" data-ng-model="role.name" autofocus
required data-ng-readonly="!create">
</div>
</div>

View file

@ -3,7 +3,7 @@
<label class="col-md-2 control-label">{{:: option.label | translate}}</label>
<div class="col-md-6" data-ng-if="option.type == 'String'">
<input class="form-control" type="text" data-ng-model="config[ option.name ]" >
<input kc-no-reserved-chars class="form-control" type="text" data-ng-model="config[ option.name ]" >
</div>
<div class="col-md-6" data-ng-if="option.type == 'Password'">
<input class="form-control" type="password" data-ng-model="config[ option.name ]" >
@ -19,7 +19,7 @@
<div class="col-md-6" data-ng-if="option.type == 'Role'">
<div class="row">
<div class="col-md-8">
<input class="form-control" type="text" data-ng-model="config[ option.name ]" >
<input kc-no-reserved-chars class="form-control" type="text" data-ng-model="config[ option.name ]" >
</div>
<div class="col-md-2">
<button type="button" data-ng-click="openRoleSelector(option.name, config)" class="btn btn-default" tooltip-placement="top" tooltip-trigger="mouseover mouseout" tooltip="{{:: 'selectRole.tooltip' | translate}}">{{:: 'selectRole.label' | translate}}</button>