Merge pull request #4288 from pedroigor/KEYCLOAK-5135
[KEYCLOAK-5135] - Wrong comparison when checking for duplicate resources during creation
This commit is contained in:
Pedro Igor
GitHub
commit
4b7c61111c
5 changed files with 258 additions and 60 deletions
|
|
@ -76,6 +76,7 @@ import org.keycloak.models.ScopeContainerModel;
|
|||
import org.keycloak.models.UserConsentModel;
|
||||
import org.keycloak.models.UserCredentialModel;
|
||||
import org.keycloak.models.UserModel;
|
||||
import org.keycloak.models.UserProvider;
|
||||
import org.keycloak.provider.ProviderConfigProperty;
|
||||
import org.keycloak.representations.idm.ApplicationRepresentation;
|
||||
import org.keycloak.representations.idm.AuthenticationExecutionExportRepresentation;
|
||||
|
|
@ -2245,10 +2246,10 @@ public class RepresentationToModel {
|
|||
existing.setType(resource.getType());
|
||||
existing.setUri(resource.getUri());
|
||||
existing.setIconUri(resource.getIconUri());
|
||||
|
||||
existing.updateScopes(resource.getScopes().stream()
|
||||
.map((ScopeRepresentation scope) -> toModel(scope, resourceServer, authorization))
|
||||
.collect(Collectors.toSet()));
|
||||
|
||||
return existing;
|
||||
}
|
||||
|
||||
|
|
@ -2259,11 +2260,30 @@ public class RepresentationToModel {
|
|||
owner.setId(resourceServer.getClientId());
|
||||
}
|
||||
|
||||
if (owner.getId() == null) {
|
||||
String ownerId = owner.getId();
|
||||
|
||||
if (ownerId == null) {
|
||||
throw new RuntimeException("No owner specified for resource [" + resource.getName() + "].");
|
||||
}
|
||||
|
||||
Resource model = resourceStore.create(resource.getName(), resourceServer, owner.getId());
|
||||
if (!resourceServer.getClientId().equals(ownerId)) {
|
||||
RealmModel realm = authorization.getRealm();
|
||||
KeycloakSession keycloakSession = authorization.getKeycloakSession();
|
||||
UserProvider users = keycloakSession.users();
|
||||
UserModel ownerModel = users.getUserById(ownerId, realm);
|
||||
|
||||
if (ownerModel == null) {
|
||||
ownerModel = users.getUserByUsername(ownerId, realm);
|
||||
}
|
||||
|
||||
if (ownerModel == null) {
|
||||
throw new RuntimeException("Owner must be a valid username or user identifier. If the resource server, the client id or null.");
|
||||
}
|
||||
|
||||
owner.setId(ownerModel.getId());
|
||||
}
|
||||
|
||||
Resource model = resourceStore.create(resource.getName(), resourceServer, ownerId);
|
||||
|
||||
model.setType(resource.getType());
|
||||
model.setUri(resource.getUri());
|
||||
|
|
|
|||
|
|
@ -101,39 +101,24 @@ public class ResourceSetService {
|
|||
Resource existingResource = storeFactory.getResourceStore().findByName(resource.getName(), this.resourceServer.getId());
|
||||
ResourceOwnerRepresentation owner = resource.getOwner();
|
||||
|
||||
if (existingResource != null && existingResource.getResourceServer().getId().equals(this.resourceServer.getId())
|
||||
&& existingResource.getOwner().equals(owner)) {
|
||||
if (owner == null) {
|
||||
owner = new ResourceOwnerRepresentation();
|
||||
owner.setId(resourceServer.getClientId());
|
||||
}
|
||||
|
||||
String ownerId = owner.getId();
|
||||
|
||||
if (ownerId == null) {
|
||||
return ErrorResponse.error("You must specify the resource owner.", Status.BAD_REQUEST);
|
||||
}
|
||||
|
||||
if (existingResource != null && existingResource.getOwner().equals(ownerId)) {
|
||||
return ErrorResponse.exists("Resource with name [" + resource.getName() + "] already exists.");
|
||||
}
|
||||
|
||||
if (owner != null) {
|
||||
String ownerId = owner.getId();
|
||||
|
||||
if (ownerId != null) {
|
||||
if (!resourceServer.getClientId().equals(ownerId)) {
|
||||
RealmModel realm = authorization.getRealm();
|
||||
KeycloakSession keycloakSession = authorization.getKeycloakSession();
|
||||
UserProvider users = keycloakSession.users();
|
||||
UserModel ownerModel = users.getUserById(ownerId, realm);
|
||||
|
||||
if (ownerModel == null) {
|
||||
ownerModel = users.getUserByUsername(ownerId, realm);
|
||||
}
|
||||
|
||||
if (ownerModel == null) {
|
||||
return ErrorResponse.error("Owner must be a valid username or user identifier. If the resource server, the client id or null.", Status.BAD_REQUEST);
|
||||
}
|
||||
|
||||
owner.setId(ownerModel.getId());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Resource model = toModel(resource, this.resourceServer, authorization);
|
||||
|
||||
ResourceRepresentation representation = new ResourceRepresentation();
|
||||
|
||||
representation.setId(model.getId());
|
||||
representation.setId(toModel(resource, this.resourceServer, authorization).getId());
|
||||
|
||||
return Response.status(Status.CREATED).entity(representation).build();
|
||||
}
|
||||
|
|
|
|||
|
|
@ -20,10 +20,12 @@ package org.keycloak.testsuite.admin.client.authorization;
|
|||
import org.junit.After;
|
||||
import org.junit.Before;
|
||||
import org.junit.BeforeClass;
|
||||
import org.keycloak.admin.client.resource.AuthorizationResource;
|
||||
import org.keycloak.admin.client.resource.ClientResource;
|
||||
import org.keycloak.admin.client.resource.ResourceScopeResource;
|
||||
import org.keycloak.admin.client.resource.ResourceScopesResource;
|
||||
import org.keycloak.representations.idm.ClientRepresentation;
|
||||
import org.keycloak.representations.idm.authorization.ResourceServerRepresentation;
|
||||
import org.keycloak.representations.idm.authorization.ScopeRepresentation;
|
||||
import org.keycloak.testsuite.ProfileAssume;
|
||||
import org.keycloak.testsuite.admin.client.AbstractClientTest;
|
||||
|
|
@ -38,7 +40,7 @@ import static org.junit.Assert.assertFalse;
|
|||
*/
|
||||
public abstract class AbstractAuthorizationTest extends AbstractClientTest {
|
||||
|
||||
protected static final String RESOURCE_SERVER_CLIENT_ID = "test-resource-server";
|
||||
protected static final String RESOURCE_SERVER_CLIENT_ID = "resource-server-test";
|
||||
|
||||
@BeforeClass
|
||||
public static void enabled() {
|
||||
|
|
@ -73,8 +75,17 @@ public abstract class AbstractAuthorizationTest extends AbstractClientTest {
|
|||
|
||||
resourceServer.setAuthorizationServicesEnabled(true);
|
||||
resourceServer.setServiceAccountsEnabled(true);
|
||||
resourceServer.setPublicClient(false);
|
||||
resourceServer.setSecret("secret");
|
||||
|
||||
getClientResource().update(resourceServer);
|
||||
|
||||
AuthorizationResource authorization = getClientResource().authorization();
|
||||
ResourceServerRepresentation settings = authorization.exportSettings();
|
||||
|
||||
settings.setAllowRemoteResourceManagement(true);
|
||||
|
||||
authorization.update(settings);
|
||||
}
|
||||
|
||||
protected ResourceScopeResource createDefaultScope() {
|
||||
|
|
|
|||
|
|
@ -22,17 +22,22 @@ import org.junit.Before;
|
|||
import org.junit.Test;
|
||||
import org.keycloak.admin.client.resource.ResourceResource;
|
||||
import org.keycloak.admin.client.resource.ResourcesResource;
|
||||
import org.keycloak.authorization.client.util.HttpResponseException;
|
||||
import org.keycloak.representations.idm.RealmRepresentation;
|
||||
import org.keycloak.representations.idm.authorization.ResourceRepresentation;
|
||||
import org.keycloak.representations.idm.authorization.ScopeRepresentation;
|
||||
|
||||
import javax.ws.rs.NotFoundException;
|
||||
import javax.ws.rs.core.Response;
|
||||
import java.util.HashSet;
|
||||
import java.util.List;
|
||||
import java.util.Set;
|
||||
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.assertFalse;
|
||||
import static org.junit.Assert.assertNotNull;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
import static org.junit.Assert.fail;
|
||||
|
||||
/**
|
||||
*
|
||||
|
|
@ -47,9 +52,29 @@ public class ResourceManagementTest extends AbstractAuthorizationTest {
|
|||
enableAuthorizationServices();
|
||||
}
|
||||
|
||||
@Override
|
||||
public void addTestRealms(List<RealmRepresentation> testRealms) {
|
||||
RealmRepresentation testRealmRep = new RealmRepresentation();
|
||||
testRealmRep.setId("authz-test");
|
||||
testRealmRep.setRealm("authz-test");
|
||||
testRealmRep.setEnabled(true);
|
||||
testRealms.add(testRealmRep);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void setDefaultPageUriParameters() {
|
||||
super.setDefaultPageUriParameters();
|
||||
testRealmPage.setAuthRealm("authz-test");
|
||||
}
|
||||
|
||||
@Override
|
||||
protected String getRealmId() {
|
||||
return "authz-test";
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testCreate() {
|
||||
ResourceRepresentation newResource = createResource().toRepresentation();
|
||||
ResourceRepresentation newResource = createResource();
|
||||
|
||||
assertEquals("Test Resource", newResource.getName());
|
||||
assertEquals("/test/*", newResource.getUri());
|
||||
|
|
@ -57,18 +82,35 @@ public class ResourceManagementTest extends AbstractAuthorizationTest {
|
|||
assertEquals("icon-test-resource", newResource.getIconUri());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void failCreateWithSameName() {
|
||||
ResourceRepresentation newResource = createResource();
|
||||
|
||||
try {
|
||||
doCreateResource(newResource);
|
||||
fail("Can not create resources with the same name and owner");
|
||||
} catch (Exception e) {
|
||||
assertEquals(HttpResponseException.class, e.getCause().getClass());
|
||||
assertEquals(409, HttpResponseException.class.cast(e.getCause()).getStatusCode());
|
||||
}
|
||||
|
||||
newResource.setName(newResource.getName() + " Another");
|
||||
|
||||
newResource = doCreateResource(newResource);
|
||||
|
||||
assertNotNull(newResource.getId());
|
||||
assertEquals("Test Resource Another", newResource.getName());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testUpdate() {
|
||||
ResourceResource resourceResource = createResource();
|
||||
ResourceRepresentation resource = resourceResource.toRepresentation();
|
||||
ResourceRepresentation resource = createResource();
|
||||
|
||||
resource.setType("changed");
|
||||
resource.setIconUri("changed");
|
||||
resource.setUri("changed");
|
||||
|
||||
resourceResource.update(resource);
|
||||
|
||||
resource = resourceResource.toRepresentation();
|
||||
resource = doUpdateResource(resource);
|
||||
|
||||
assertEquals("changed", resource.getIconUri());
|
||||
assertEquals("changed", resource.getType());
|
||||
|
|
@ -77,17 +119,16 @@ public class ResourceManagementTest extends AbstractAuthorizationTest {
|
|||
|
||||
@Test(expected = NotFoundException.class)
|
||||
public void testDelete() {
|
||||
ResourceResource resourceResource = createResource();
|
||||
ResourceRepresentation resource = createResource();
|
||||
|
||||
resourceResource.remove();
|
||||
doRemoveResource(resource);
|
||||
|
||||
resourceResource.toRepresentation();
|
||||
getClientResource().authorization().resources().resource(resource.getId()).toRepresentation();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testAssociateScopes() {
|
||||
ResourceResource resourceResource = createResourceWithDefaultScopes();
|
||||
ResourceRepresentation updated = resourceResource.toRepresentation();
|
||||
ResourceRepresentation updated = createResourceWithDefaultScopes();
|
||||
|
||||
assertEquals(3, updated.getScopes().size());
|
||||
|
||||
|
|
@ -98,8 +139,7 @@ public class ResourceManagementTest extends AbstractAuthorizationTest {
|
|||
|
||||
@Test
|
||||
public void testUpdateScopes() {
|
||||
ResourceResource resourceResource = createResourceWithDefaultScopes();
|
||||
ResourceRepresentation resource = resourceResource.toRepresentation();
|
||||
ResourceRepresentation resource = createResourceWithDefaultScopes();
|
||||
Set<ScopeRepresentation> scopes = new HashSet<>(resource.getScopes());
|
||||
|
||||
assertEquals(3, scopes.size());
|
||||
|
|
@ -107,9 +147,7 @@ public class ResourceManagementTest extends AbstractAuthorizationTest {
|
|||
|
||||
resource.setScopes(scopes);
|
||||
|
||||
resourceResource.update(resource);
|
||||
|
||||
ResourceRepresentation updated = resourceResource.toRepresentation();
|
||||
ResourceRepresentation updated = doUpdateResource(resource);
|
||||
|
||||
assertEquals(2, resource.getScopes().size());
|
||||
|
||||
|
|
@ -124,16 +162,13 @@ public class ResourceManagementTest extends AbstractAuthorizationTest {
|
|||
|
||||
updated.setScopes(scopes);
|
||||
|
||||
resourceResource.update(updated);
|
||||
|
||||
updated = resourceResource.toRepresentation();
|
||||
updated = doUpdateResource(updated);
|
||||
|
||||
assertEquals(0, updated.getScopes().size());
|
||||
}
|
||||
|
||||
private ResourceResource createResourceWithDefaultScopes() {
|
||||
ResourceResource resourceResource = createResource();
|
||||
ResourceRepresentation resource = resourceResource.toRepresentation();
|
||||
private ResourceRepresentation createResourceWithDefaultScopes() {
|
||||
ResourceRepresentation resource = createResource();
|
||||
|
||||
assertEquals(0, resource.getScopes().size());
|
||||
|
||||
|
|
@ -145,9 +180,7 @@ public class ResourceManagementTest extends AbstractAuthorizationTest {
|
|||
|
||||
resource.setScopes(scopes);
|
||||
|
||||
resourceResource.update(resource);
|
||||
|
||||
return resourceResource;
|
||||
return doUpdateResource(resource);
|
||||
}
|
||||
|
||||
private boolean containsScope(String scopeName, ResourceRepresentation resource) {
|
||||
|
|
@ -164,7 +197,7 @@ public class ResourceManagementTest extends AbstractAuthorizationTest {
|
|||
return false;
|
||||
}
|
||||
|
||||
private ResourceResource createResource() {
|
||||
private ResourceRepresentation createResource() {
|
||||
ResourceRepresentation newResource = new ResourceRepresentation();
|
||||
|
||||
newResource.setName("Test Resource");
|
||||
|
|
@ -172,14 +205,36 @@ public class ResourceManagementTest extends AbstractAuthorizationTest {
|
|||
newResource.setType("test-resource");
|
||||
newResource.setIconUri("icon-test-resource");
|
||||
|
||||
return doCreateResource(newResource);
|
||||
}
|
||||
|
||||
protected ResourceRepresentation doCreateResource(ResourceRepresentation newResource) {
|
||||
ResourcesResource resources = getClientResource().authorization().resources();
|
||||
|
||||
Response response = resources.create(newResource);
|
||||
|
||||
assertEquals(Response.Status.CREATED.getStatusCode(), response.getStatus());
|
||||
int status = response.getStatus();
|
||||
|
||||
if (status != Response.Status.CREATED.getStatusCode()) {
|
||||
throw new RuntimeException(new HttpResponseException("Error", status, "", null));
|
||||
}
|
||||
|
||||
ResourceRepresentation stored = response.readEntity(ResourceRepresentation.class);
|
||||
|
||||
return resources.resource(stored.getId());
|
||||
return resources.resource(stored.getId()).toRepresentation();
|
||||
}
|
||||
|
||||
protected ResourceRepresentation doUpdateResource(ResourceRepresentation resource) {
|
||||
ResourcesResource resources = getClientResource().authorization().resources();
|
||||
ResourceResource existing = resources.resource(resource.getId());
|
||||
|
||||
existing.update(resource);
|
||||
|
||||
return resources.resource(resource.getId()).toRepresentation();
|
||||
}
|
||||
|
||||
protected void doRemoveResource(ResourceRepresentation resource) {
|
||||
ResourcesResource resources = getClientResource().authorization().resources();
|
||||
resources.resource(resource.getId()).remove();
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,127 @@
|
|||
/*
|
||||
* Copyright 2017 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.testsuite.admin.client.authorization;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.util.stream.Collectors;
|
||||
|
||||
import org.jetbrains.annotations.NotNull;
|
||||
import org.keycloak.authorization.client.AuthzClient;
|
||||
import org.keycloak.authorization.client.Configuration;
|
||||
import org.keycloak.authorization.client.representation.RegistrationResponse;
|
||||
import org.keycloak.representations.idm.authorization.ResourceOwnerRepresentation;
|
||||
import org.keycloak.representations.idm.authorization.ResourceRepresentation;
|
||||
import org.keycloak.representations.idm.authorization.ScopeRepresentation;
|
||||
import org.keycloak.util.JsonSerialization;
|
||||
|
||||
/**
|
||||
*
|
||||
* @author <a href="mailto:psilva@redhat.com">Pedro Igor</a>
|
||||
*/
|
||||
public class ResourceManagementWithAuthzClientTest extends ResourceManagementTest {
|
||||
|
||||
private AuthzClient authzClient;
|
||||
|
||||
@Override
|
||||
protected ResourceRepresentation doCreateResource(ResourceRepresentation newResource) {
|
||||
org.keycloak.authorization.client.representation.ResourceRepresentation resource = toResourceRepresentation(newResource);
|
||||
|
||||
AuthzClient authzClient = getAuthzClient();
|
||||
RegistrationResponse response = authzClient.protection().resource().create(resource);
|
||||
|
||||
return toResourceRepresentation(authzClient, response.getId());
|
||||
}
|
||||
|
||||
@Override
|
||||
protected ResourceRepresentation doUpdateResource(ResourceRepresentation resource) {
|
||||
AuthzClient authzClient = getAuthzClient();
|
||||
|
||||
authzClient.protection().resource().update(toResourceRepresentation(resource));
|
||||
|
||||
return toResourceRepresentation(authzClient, resource.getId());
|
||||
}
|
||||
|
||||
@Override
|
||||
protected void doRemoveResource(ResourceRepresentation resource) {
|
||||
getAuthzClient().protection().resource().delete(resource.getId());
|
||||
}
|
||||
|
||||
private ResourceRepresentation toResourceRepresentation(AuthzClient authzClient, String id) {
|
||||
org.keycloak.authorization.client.representation.ResourceRepresentation created = authzClient.protection().resource().findById(id).getResourceDescription();
|
||||
ResourceRepresentation resourceRepresentation = new ResourceRepresentation();
|
||||
|
||||
resourceRepresentation.setId(created.getId());
|
||||
resourceRepresentation.setName(created.getName());
|
||||
resourceRepresentation.setIconUri(created.getIconUri());
|
||||
resourceRepresentation.setUri(created.getUri());
|
||||
resourceRepresentation.setType(created.getType());
|
||||
ResourceOwnerRepresentation owner = new ResourceOwnerRepresentation();
|
||||
|
||||
owner.setId(created.getOwner());
|
||||
|
||||
resourceRepresentation.setOwner(owner);
|
||||
resourceRepresentation.setScopes(created.getScopes().stream().map(scopeRepresentation -> {
|
||||
ScopeRepresentation scope = new ScopeRepresentation();
|
||||
|
||||
scope.setId(scopeRepresentation.getId());
|
||||
scope.setName(scopeRepresentation.getName());
|
||||
scope.setIconUri(scopeRepresentation.getIconUri());
|
||||
|
||||
return scope;
|
||||
}).collect(Collectors.toSet()));
|
||||
|
||||
return resourceRepresentation;
|
||||
}
|
||||
|
||||
private org.keycloak.authorization.client.representation.ResourceRepresentation toResourceRepresentation(ResourceRepresentation newResource) {
|
||||
org.keycloak.authorization.client.representation.ResourceRepresentation resource = new org.keycloak.authorization.client.representation.ResourceRepresentation();
|
||||
|
||||
resource.setId(newResource.getId());
|
||||
resource.setName(newResource.getName());
|
||||
resource.setIconUri(newResource.getIconUri());
|
||||
resource.setUri(newResource.getUri());
|
||||
resource.setType(newResource.getType());
|
||||
|
||||
if (newResource.getOwner() != null) {
|
||||
resource.setOwner(newResource.getOwner().getId());
|
||||
}
|
||||
|
||||
resource.setScopes(newResource.getScopes().stream().map(scopeRepresentation -> {
|
||||
org.keycloak.authorization.client.representation.ScopeRepresentation scope = new org.keycloak.authorization.client.representation.ScopeRepresentation();
|
||||
|
||||
scope.setName(scopeRepresentation.getName());
|
||||
scope.setIconUri(scopeRepresentation.getIconUri());
|
||||
|
||||
return scope;
|
||||
}).collect(Collectors.toSet()));
|
||||
|
||||
return resource;
|
||||
}
|
||||
|
||||
private AuthzClient getAuthzClient() {
|
||||
if (authzClient == null) {
|
||||
try {
|
||||
authzClient = AuthzClient.create(JsonSerialization.readValue(getClass().getResourceAsStream("/authorization-test/default-keycloak.json"), Configuration.class));
|
||||
} catch (IOException cause) {
|
||||
throw new RuntimeException("Failed to create authz client", cause);
|
||||
}
|
||||
}
|
||||
|
||||
return authzClient;
|
||||
}
|
||||
}
|
||||
Loading…
Reference in a new issue