diff --git a/rest/admin-v2/api/src/main/java/org/keycloak/services/ServiceException.java b/rest/admin-v2/api/src/main/java/org/keycloak/services/ServiceException.java index 88e186a775c..a0572145cc2 100644 --- a/rest/admin-v2/api/src/main/java/org/keycloak/services/ServiceException.java +++ b/rest/admin-v2/api/src/main/java/org/keycloak/services/ServiceException.java @@ -2,6 +2,7 @@ package org.keycloak.services; import java.util.Optional; +import jakarta.ws.rs.WebApplicationException; import jakarta.ws.rs.core.Response; public class ServiceException extends RuntimeException { @@ -20,8 +21,25 @@ public class ServiceException extends RuntimeException { this.suggestedHttpResponseStatus = suggestedStatus; } + public ServiceException(Response.Status suggestedStatus) { + super(); + this.suggestedHttpResponseStatus = suggestedStatus; + } + public Optional getSuggestedResponseStatus() { return Optional.ofNullable(suggestedHttpResponseStatus); } + public WebApplicationException toWebApplicationException() { + return toWebApplicationException(Response.Status.BAD_REQUEST); + } + + public WebApplicationException toWebApplicationException(Response.Status orReturnStatus) { + if (getMessage() != null) { + return new WebApplicationException(getMessage(), getSuggestedResponseStatus().orElse(orReturnStatus)); + } else { + return new WebApplicationException(getSuggestedResponseStatus().orElse(orReturnStatus)); + } + } + } diff --git a/rest/admin-v2/api/src/main/java/org/keycloak/services/client/ClientService.java b/rest/admin-v2/api/src/main/java/org/keycloak/services/client/ClientService.java index c95dfa5153d..6dfae06ff94 100644 --- a/rest/admin-v2/api/src/main/java/org/keycloak/services/client/ClientService.java +++ b/rest/admin-v2/api/src/main/java/org/keycloak/services/client/ClientService.java @@ -27,12 +27,22 @@ public interface ClientService extends Service { record CreateOrUpdateResult(BaseClientRepresentation representation, boolean created) {} - Optional getClient(RealmModel realm, String clientId, ClientProjectionOptions projectionOptions); + default Optional getClient(RealmModel realm, String clientId) throws ServiceException { + return getClient(realm, clientId, null); + } + + Optional getClient(RealmModel realm, String clientId, ClientProjectionOptions projectionOptions) throws ServiceException; + + default Stream getClients(RealmModel realm) { + return getClients(realm, null, null, null); + } Stream getClients(RealmModel realm, ClientProjectionOptions projectionOptions, ClientSearchOptions searchOptions, ClientSortAndSliceOptions sortAndSliceOptions); Stream deleteClients(RealmModel realm, ClientSearchOptions searchOptions); + void deleteClient(RealmModel realm, String clientId) throws ServiceException; + CreateOrUpdateResult createOrUpdate(RealmModel realm, BaseClientRepresentation client, boolean allowUpdate) throws ServiceException; } diff --git a/rest/admin-v2/api/src/main/java/org/keycloak/services/client/DefaultClientService.java b/rest/admin-v2/api/src/main/java/org/keycloak/services/client/DefaultClientService.java index 514b56e5a42..e99672ee45a 100644 --- a/rest/admin-v2/api/src/main/java/org/keycloak/services/client/DefaultClientService.java +++ b/rest/admin-v2/api/src/main/java/org/keycloak/services/client/DefaultClientService.java @@ -7,6 +7,8 @@ import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; +import jakarta.annotation.Nonnull; +import jakarta.annotation.Nullable; import jakarta.ws.rs.NotFoundException; import jakarta.ws.rs.core.Response; @@ -25,33 +27,54 @@ import org.keycloak.services.ServiceException; import org.keycloak.services.resources.admin.ClientResource; import org.keycloak.services.resources.admin.ClientsResource; import org.keycloak.services.resources.admin.RealmAdminResource; +import org.keycloak.services.resources.admin.fgap.AdminPermissionEvaluator; import org.keycloak.validation.jakarta.HibernateValidatorProvider; import org.keycloak.validation.jakarta.JakartaValidatorProvider; +import org.apache.http.HttpEntity; +import org.apache.http.util.EntityUtils; + // TODO public class DefaultClientService implements ClientService { private final KeycloakSession session; private final JakartaValidatorProvider validator; - private final RealmAdminResource realmAdminResource; + private final AdminPermissionEvaluator permissions; + + // v1 resources + private final RealmAdminResource realmResource; private final ClientsResource clientsResource; private ClientResource clientResource; - public DefaultClientService(KeycloakSession session, RealmAdminResource realmAdminResource, ClientResource clientResource) { + public DefaultClientService(@Nonnull KeycloakSession session, + @Nonnull AdminPermissionEvaluator permissions, + @Nonnull RealmAdminResource realmResource, + @Nullable ClientResource clientResource) { this.session = session; - this.realmAdminResource = realmAdminResource; - this.clientResource = clientResource; - - this.clientsResource = realmAdminResource.getClients(); + this.permissions = permissions; this.validator = new HibernateValidatorProvider(); + + this.realmResource = realmResource; + this.clientsResource = realmResource.getClients(); + this.clientResource = clientResource; } - public DefaultClientService(KeycloakSession session, RealmAdminResource realmAdminResource) { - this(session, realmAdminResource, null); + public DefaultClientService(@Nonnull KeycloakSession session, + @Nonnull AdminPermissionEvaluator permissions, + @Nonnull RealmAdminResource realmResource) { + this(session, permissions, realmResource, null); + } + + protected void avoidClientIdPhishing() throws ServiceException { + if (clientResource == null && !permissions.clients().canList()) { + // we do this to make sure somebody can't phish client IDs + throw new ServiceException(Response.Status.FORBIDDEN); + } } @Override - public Optional getClient(RealmModel realm, String clientId, ClientProjectionOptions projectionOptions) { + public Optional getClient(RealmModel realm, String clientId, ClientProjectionOptions projectionOptions) throws ServiceException { // TODO: is the access map on the representation needed + avoidClientIdPhishing(); return Optional.ofNullable(clientResource).map(ClientResource::viewClientModel) .map(model -> session.getProvider(ClientModelMapper.class, model.getProtocol()).fromModel(model)); } @@ -82,7 +105,11 @@ public class DefaultClientService implements ClientService { } model = mapper.toModel(client, clientResource.viewClientModel()); var rep = ModelToRepresentation.toRepresentation(model, session); - clientResource.update(rep); + + try (var response = clientResource.update(rep)) { + // close response and consume payload due to performance reasons + EntityUtils.consumeQuietly((HttpEntity) response.getEntity()); + } } else { created = true; validator.validate(client, CreateClientDefault.class); // TODO improve it to avoid second validation when we know it is create and not update @@ -115,6 +142,15 @@ public class DefaultClientService implements ClientService { return null; } + @Override + public void deleteClient(RealmModel realm, String clientId) throws ServiceException { + avoidClientIdPhishing(); + if (clientResource == null) { + throw new ServiceException("Cannot find the specified client", Response.Status.NOT_FOUND); + } + clientResource.deleteClient(); + } + /** * Declaratively manage client roles - ensures the client has exactly the roles specified in 'rolesFromRep' *

@@ -133,7 +169,12 @@ public class DefaultClientService implements ClientService { // Add missing roles (in desiredRoleNames but not in currentRoleNames) desiredRoleNames.stream() .filter(roleName -> !currentRoleNames.contains(roleName)) - .forEach(roleName -> roleResource.createRole(new RoleRepresentation(roleName, "", false))); + .forEach(roleName -> { + try (var response = roleResource.createRole(new RoleRepresentation(roleName, "", false))) { + // close response and consume payload due to performance reasons + EntityUtils.consumeQuietly((HttpEntity) response.getEntity()); + } + }); // Remove extra roles (in currentRoleNames but not in desiredRoleNames) currentRoleNames.stream() @@ -156,10 +197,10 @@ public class DefaultClientService implements ClientService { } var clientRoleResource = clientResource.getRoleContainerResource(); - var realmRoleResource = realmAdminResource.getRoleContainerResource(); + var realmRoleResource = realmResource.getRoleContainerResource(); var serviceAccountUser = session.users().getServiceAccount(model); - var serviceAccountRoleResource = realmAdminResource.users().user(clientResource.getServiceAccountUser().getId()).getRoleMappings(); + var serviceAccountRoleResource = realmResource.users().user(clientResource.getServiceAccountUser().getId()).getRoleMappings(); Set desiredRoleNames = Optional.ofNullable(rep.getServiceAccountRoles()).orElse(Collections.emptySet()); Set currentRoles = serviceAccountUser.getRoleMappingsStream().collect(Collectors.toSet()); diff --git a/rest/admin-v2/rest/src/main/java/org/keycloak/admin/api/DefaultAdminApi.java b/rest/admin-v2/rest/src/main/java/org/keycloak/admin/api/DefaultAdminApi.java index b66b2e54654..5490a3085ac 100644 --- a/rest/admin-v2/rest/src/main/java/org/keycloak/admin/api/DefaultAdminApi.java +++ b/rest/admin-v2/rest/src/main/java/org/keycloak/admin/api/DefaultAdminApi.java @@ -8,29 +8,31 @@ import org.keycloak.admin.api.client.ClientsApi; import org.keycloak.admin.api.client.DefaultClientsApi; import org.keycloak.models.KeycloakSession; import org.keycloak.protocol.oidc.TokenManager; -import org.keycloak.services.resources.admin.AdminAuth; import org.keycloak.services.resources.admin.AdminRoot; import org.keycloak.services.resources.admin.RealmAdminResource; import org.keycloak.services.resources.admin.RealmsAdminResource; +import org.keycloak.services.resources.admin.fgap.AdminPermissionEvaluator; +import org.keycloak.services.resources.admin.fgap.AdminPermissions; public class DefaultAdminApi implements AdminApi { private final KeycloakSession session; - private final RealmsAdminResource realmsAdminResource; + private final AdminPermissionEvaluator permissions; + + // v1 resources private final RealmAdminResource realmAdminResource; - private final AdminAuth auth; public DefaultAdminApi(KeycloakSession session, String realmName) { this.session = session; - this.auth = AdminRoot.authenticateRealmAdminRequest(session); - this.realmsAdminResource = new RealmsAdminResource(session, auth, new TokenManager()); - this.realmAdminResource = realmsAdminResource.getRealmAdmin(realmName); + var authInfo = AdminRoot.authenticateRealmAdminRequest(session); + this.permissions = AdminPermissions.evaluator(session, authInfo.getRealm(), authInfo); + this.realmAdminResource = new RealmsAdminResource(session, authInfo, new TokenManager()).getRealmAdmin(realmName); } @Path("clients/{version:v\\d+}") @Override public ClientsApi clients(@PathParam("version") String version) { return switch (version) { - case "v2" -> new DefaultClientsApi(session, realmAdminResource); + case "v2" -> new DefaultClientsApi(session, permissions, realmAdminResource); default -> throw new NotFoundException(); }; } diff --git a/rest/admin-v2/rest/src/main/java/org/keycloak/admin/api/client/DefaultClientApi.java b/rest/admin-v2/rest/src/main/java/org/keycloak/admin/api/client/DefaultClientApi.java index 42972055846..109b5552e12 100644 --- a/rest/admin-v2/rest/src/main/java/org/keycloak/admin/api/client/DefaultClientApi.java +++ b/rest/admin-v2/rest/src/main/java/org/keycloak/admin/api/client/DefaultClientApi.java @@ -3,6 +3,8 @@ package org.keycloak.admin.api.client; import java.io.IOException; import java.util.Objects; +import jakarta.annotation.Nonnull; +import jakarta.annotation.Nullable; import jakarta.ws.rs.DELETE; import jakarta.ws.rs.GET; import jakarta.ws.rs.NotFoundException; @@ -23,6 +25,7 @@ import org.keycloak.services.client.ClientService; import org.keycloak.services.client.DefaultClientService; import org.keycloak.services.resources.admin.ClientResource; import org.keycloak.services.resources.admin.RealmAdminResource; +import org.keycloak.services.resources.admin.fgap.AdminPermissionEvaluator; import org.keycloak.services.util.ObjectMapperResolver; import com.fasterxml.jackson.core.JsonProcessingException; @@ -31,33 +34,35 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.ObjectReader; public class DefaultClientApi implements ClientApi { - - private final KeycloakSession session; - private final RealmModel realm; - private final ClientService clientService; - - private final ClientResource clientResource; - private final String clientId; - private final ObjectMapper objectMapper; - private static final ObjectMapper MAPPER = new ObjectMapperResolver().getContext(null); - public DefaultClientApi(KeycloakSession session, RealmAdminResource realmAdminResource, ClientResource clientResource, String clientId) { + private final KeycloakSession session; + private final String clientId; + private final RealmModel realm; + private final ClientService clientService; + private final ObjectMapper objectMapper; + + public DefaultClientApi(@Nonnull KeycloakSession session, + @Nonnull String clientId, + @Nonnull AdminPermissionEvaluator permissions, + @Nonnull RealmAdminResource realmAdminResource, + @Nullable ClientResource clientResource) { this.session = session; - this.clientResource = clientResource; this.clientId = clientId; - + this.clientService = new DefaultClientService(session, permissions, realmAdminResource, clientResource); this.realm = Objects.requireNonNull(session.getContext().getRealm()); - this.clientService = new DefaultClientService(session, realmAdminResource, clientResource); - this.objectMapper = MAPPER; } @GET @Override public BaseClientRepresentation getClient() { - return clientService.getClient(realm, clientId, null) - .orElseThrow(() -> new NotFoundException("Cannot find the specified client")); + try { + return clientService.getClient(realm, clientId) + .orElseThrow(() -> new NotFoundException("Cannot find the specified client")); + } catch (ServiceException e) { + throw e.toWebApplicationException(Response.Status.NOT_FOUND); + } } @PUT @@ -71,7 +76,7 @@ public class DefaultClientApi implements ClientApi { var result = clientService.createOrUpdate(realm, client, true); return Response.status(result.created() ? Response.Status.CREATED : Response.Status.OK).entity(result.representation()).build(); } catch (ServiceException e) { - throw new WebApplicationException(e.getMessage(), e.getSuggestedResponseStatus().orElse(Response.Status.BAD_REQUEST)); + throw e.toWebApplicationException(); } } @@ -104,10 +109,11 @@ public class DefaultClientApi implements ClientApi { @DELETE @Override public void deleteClient() { - if (clientResource == null) { - throw new NotFoundException("Cannot find the specified client"); + try { + clientService.deleteClient(realm, clientId); + } catch (ServiceException e) { + throw e.toWebApplicationException(); } - clientResource.deleteClient(); } static void validateUnknownFields(BaseClientRepresentation rep) { diff --git a/rest/admin-v2/rest/src/main/java/org/keycloak/admin/api/client/DefaultClientsApi.java b/rest/admin-v2/rest/src/main/java/org/keycloak/admin/api/client/DefaultClientsApi.java index d1a09db9ac4..91b658358a2 100644 --- a/rest/admin-v2/rest/src/main/java/org/keycloak/admin/api/client/DefaultClientsApi.java +++ b/rest/admin-v2/rest/src/main/java/org/keycloak/admin/api/client/DefaultClientsApi.java @@ -4,12 +4,12 @@ import java.util.Objects; import java.util.Optional; import java.util.stream.Stream; +import jakarta.annotation.Nonnull; import jakarta.validation.Valid; import jakarta.ws.rs.GET; import jakarta.ws.rs.POST; import jakarta.ws.rs.Path; import jakarta.ws.rs.PathParam; -import jakarta.ws.rs.WebApplicationException; import jakarta.ws.rs.core.Response; import org.keycloak.models.KeycloakSession; @@ -21,23 +21,30 @@ import org.keycloak.services.client.ClientService; import org.keycloak.services.client.DefaultClientService; import org.keycloak.services.resources.admin.ClientsResource; import org.keycloak.services.resources.admin.RealmAdminResource; +import org.keycloak.services.resources.admin.fgap.AdminPermissionEvaluator; import org.keycloak.validation.jakarta.HibernateValidatorProvider; import org.keycloak.validation.jakarta.JakartaValidatorProvider; public class DefaultClientsApi implements ClientsApi { private final KeycloakSession session; + private final AdminPermissionEvaluator permissions; private final RealmModel realm; private final ClientService clientService; private final JakartaValidatorProvider validator; + + // v1 resources private final RealmAdminResource realmAdminResource; private final ClientsResource clientsResource; - public DefaultClientsApi(KeycloakSession session, RealmAdminResource realmAdminResource) { + public DefaultClientsApi(@Nonnull KeycloakSession session, + @Nonnull AdminPermissionEvaluator permissions, + @Nonnull RealmAdminResource realmAdminResource) { this.session = session; + this.permissions = permissions; this.realmAdminResource = realmAdminResource; this.realm = Objects.requireNonNull(session.getContext().getRealm()); - this.clientService = new DefaultClientService(session, realmAdminResource); + this.clientService = new DefaultClientService(session, permissions, realmAdminResource); this.validator = new HibernateValidatorProvider(); this.clientsResource = realmAdminResource.getClients(); } @@ -45,7 +52,7 @@ public class DefaultClientsApi implements ClientsApi { @GET @Override public Stream getClients() { - return clientService.getClients(realm, null, null, null); + return clientService.getClients(realm); } @POST @@ -58,7 +65,7 @@ public class DefaultClientsApi implements ClientsApi { .entity(clientService.createOrUpdate(realm, client, false).representation()) .build(); } catch (ServiceException e) { - throw new WebApplicationException(e.getMessage(), e.getSuggestedResponseStatus().orElse(Response.Status.BAD_REQUEST)); + throw e.toWebApplicationException(); } } @@ -66,7 +73,7 @@ public class DefaultClientsApi implements ClientsApi { @Override public ClientApi client(@PathParam("id") String clientId) { var client = Optional.ofNullable(session.clients().getClientByClientId(realm, clientId)); - return new DefaultClientApi(session, realmAdminResource, client.map(c -> clientsResource.getClient(c.getId())).orElse(null), clientId); + return new DefaultClientApi(session, clientId, permissions, realmAdminResource, client.map(c -> clientsResource.getClient(c.getId())).orElse(null)); } } diff --git a/rest/admin-v2/tests/src/test/java/org/keycloak/tests/admin/client/v2/ClientApiV2AuthorizationTest.java b/rest/admin-v2/tests/src/test/java/org/keycloak/tests/admin/client/v2/ClientApiV2AuthorizationTest.java index 632ecdad656..c1733a3341e 100644 --- a/rest/admin-v2/tests/src/test/java/org/keycloak/tests/admin/client/v2/ClientApiV2AuthorizationTest.java +++ b/rest/admin-v2/tests/src/test/java/org/keycloak/tests/admin/client/v2/ClientApiV2AuthorizationTest.java @@ -40,6 +40,7 @@ import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -52,7 +53,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; * Based on this spike GitHub Issue #45940 */ @KeycloakIntegrationTest(config = ClientApiV2AuthorizationTest.AdminV2WithAuthzConfig.class) -public class ClientApiV2AuthorizationTest extends AbstractClientApiV2Test{ +public class ClientApiV2AuthorizationTest extends AbstractClientApiV2Test { @InjectHttpClient CloseableHttpClient client; @@ -137,7 +138,7 @@ public class ClientApiV2AuthorizationTest extends AbstractClientApiV2Test{ request.setEntity(new StringEntity(mapper.writeValueAsString(createClientRep("test-create-realm-admin", "new-role1", "new-role2")))); try (var response = client.execute(request)) { EntityUtils.consumeQuietly(response.getEntity()); - assertEquals(201, response.getStatusLine().getStatusCode()); + assertThat(response.getStatusLine().getStatusCode(), is(201)); } // manage-clients: should be able to create clients (has requireManage) @@ -147,7 +148,7 @@ public class ClientApiV2AuthorizationTest extends AbstractClientApiV2Test{ request.setEntity(new StringEntity(mapper.writeValueAsString(createClientRep("test-create-manage", "new-role1", "new-role2")))); try (var response = client.execute(request)) { EntityUtils.consumeQuietly(response.getEntity()); - assertEquals(201, response.getStatusLine().getStatusCode()); + assertThat(response.getStatusLine().getStatusCode(), is(201)); } // view-clients: should get 403 (lacks requireManage) @@ -156,19 +157,19 @@ public class ClientApiV2AuthorizationTest extends AbstractClientApiV2Test{ request.setHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON); request.setEntity(new StringEntity(mapper.writeValueAsString(createClientRep("test-create-view", "new-role1", "new-role2")))); try (var response = client.execute(request)) { - assertEquals(403, response.getStatusLine().getStatusCode()); + assertThat(response.getStatusLine().getStatusCode(), is(403)); } // query-clients: should get 403 (lacks requireManage) setAuthHeader(request, adminClients.get("query-clients")); try (var response = client.execute(request)) { - assertEquals(403, response.getStatusLine().getStatusCode()); + assertThat(response.getStatusLine().getStatusCode(), is(403)); } // no-access: should get 403 (lacks requireManage) setAuthHeader(request, adminClients.get("no-access")); try (var response = client.execute(request)) { - assertEquals(403, response.getStatusLine().getStatusCode()); + assertThat(response.getStatusLine().getStatusCode(), is(403)); } } @@ -201,13 +202,13 @@ public class ClientApiV2AuthorizationTest extends AbstractClientApiV2Test{ // query-clients: should get 403 (can list but not view individual clients) setAuthHeader(request, adminClients.get("query-clients")); try (var response = client.execute(request)) { - assertEquals(403, response.getStatusLine().getStatusCode()); + assertThat(response.getStatusLine().getStatusCode(), is(403)); } // no-access: should get 403 (lacks requireView) setAuthHeader(request, adminClients.get("no-access")); try (var response = client.execute(request)) { - assertEquals(403, response.getStatusLine().getStatusCode()); + assertThat(response.getStatusLine().getStatusCode(), is(403)); } } @@ -222,28 +223,27 @@ public class ClientApiV2AuthorizationTest extends AbstractClientApiV2Test{ // view-clients: should get 404 (has canList, client doesn't exist) setAuthHeader(request, adminClients.get("view-clients")); try (var response = client.execute(request)) { - assertEquals(404, response.getStatusLine().getStatusCode()); + assertThat(response.getStatusLine().getStatusCode(), is(404)); } // manage-clients: should get 404 (has canList, client doesn't exist) setAuthHeader(request, adminClients.get("manage-clients")); try (var response = client.execute(request)) { - assertEquals(404, response.getStatusLine().getStatusCode()); + assertThat(response.getStatusLine().getStatusCode(), is(404)); } // query-clients: should get 404 (has canList, client doesn't exist) setAuthHeader(request, adminClients.get("query-clients")); try (var response = client.execute(request)) { - assertEquals(404, response.getStatusLine().getStatusCode()); + assertThat(response.getStatusLine().getStatusCode(), is(404)); } - // TODO - our V2 logic does not handle the anti-ID phishing yet - // TODO - issue https://github.com/keycloak/keycloak/issues/46010 // no-access: should get 403 (lacks canList, prevents ID phishing) - // setAuthHeader(request, clients.get("no-access")); - // try (var response = client.execute(request)) { - // assertEquals(403, response.getStatusLine().getStatusCode()); - // } + setAuthHeader(request, adminClients.get("no-access")); + try (var response = client.execute(request)) { + assertThat(response.getStatusLine().getStatusCode(), is(403)); + assertThat(EntityUtils.toString(response.getEntity()), containsString("HTTP 403 Forbidden")); + } } /** @@ -259,7 +259,7 @@ public class ClientApiV2AuthorizationTest extends AbstractClientApiV2Test{ request.setHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON); request.setEntity(new StringEntity(mapper.writeValueAsString(createClientRep("test-update-admin", "role123", "my-role")))); try (var response = client.execute(request)) { - assertEquals(200, response.getStatusLine().getStatusCode()); + assertThat(response.getStatusLine().getStatusCode(), is(200)); OIDCClientRepresentation client = mapper.createParser(response.getEntity().getContent()) .readValueAs(OIDCClientRepresentation.class); assertThat(client.getClientId(), is("test-update-admin")); @@ -273,7 +273,7 @@ public class ClientApiV2AuthorizationTest extends AbstractClientApiV2Test{ request.setHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON); request.setEntity(new StringEntity(mapper.writeValueAsString(createClientRep("test-update-manage", "role123", "my-role")))); try (var response = client.execute(request)) { - assertEquals(200, response.getStatusLine().getStatusCode()); + assertThat(response.getStatusLine().getStatusCode(), is(200)); OIDCClientRepresentation client = mapper.createParser(response.getEntity().getContent()) .readValueAs(OIDCClientRepresentation.class); assertThat(client.getClientId(), is("test-update-manage")); @@ -287,13 +287,13 @@ public class ClientApiV2AuthorizationTest extends AbstractClientApiV2Test{ request.setHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON); request.setEntity(new StringEntity(mapper.writeValueAsString(createClientRep("test-update-view", "role123", "my-role")))); try (var response = client.execute(request)) { - assertEquals(403, response.getStatusLine().getStatusCode()); + assertThat(response.getStatusLine().getStatusCode(), is(403)); } // query-clients: should get 403 (lacks requireConfigure) setAuthHeader(request, adminClients.get("query-clients")); try (var response = client.execute(request)) { - assertEquals(403, response.getStatusLine().getStatusCode()); + assertThat(response.getStatusLine().getStatusCode(), is(403)); } } @@ -313,7 +313,7 @@ public class ClientApiV2AuthorizationTest extends AbstractClientApiV2Test{ request.setEntity(new StringEntity(mapper.writeValueAsString(patch))); try (var response = client.execute(request)) { EntityUtils.consumeQuietly(response.getEntity()); - assertEquals(200, response.getStatusLine().getStatusCode()); + assertThat(response.getStatusLine().getStatusCode(), is(200)); } // view-clients: should get 403 (lacks requireConfigure) @@ -323,19 +323,39 @@ public class ClientApiV2AuthorizationTest extends AbstractClientApiV2Test{ request.setHeader(HttpHeaders.CONTENT_TYPE, "application/merge-patch+json"); request.setEntity(new StringEntity(mapper.writeValueAsString(patch))); try (var response = client.execute(request)) { - assertEquals(403, response.getStatusLine().getStatusCode()); + assertThat(response.getStatusLine().getStatusCode(), is(403)); } // query-clients: should get 403 (lacks requireConfigure) setAuthHeader(request, adminClients.get("query-clients")); try (var response = client.execute(request)) { - assertEquals(403, response.getStatusLine().getStatusCode()); + assertThat(response.getStatusLine().getStatusCode(), is(403)); } // manage-clients: should be able to patch clients (has manage-clients) setAuthHeader(request, adminClients.get("manage-clients")); try (var response = client.execute(request)) { - assertEquals(200, response.getStatusLine().getStatusCode()); + assertThat(response.getStatusLine().getStatusCode(), is(200)); + } + + // does not exist + request = new HttpPatch(getClientsApiUrl() + "/does-not-exist"); + request.setHeader(HttpHeaders.CONTENT_TYPE, "application/merge-patch+json"); + patch = new OIDCClientRepresentation(); + patch.setDescription("Patched-non-existing"); + request.setEntity(new StringEntity(mapper.writeValueAsString(patch))); + + // no-access: not existing - should get 403 (lacks canList, prevents ID phishing) + setAuthHeader(request, adminClients.get("no-access")); + try (var response = client.execute(request)) { + assertThat(response.getStatusLine().getStatusCode(), is(403)); + assertThat(EntityUtils.toString(response.getEntity()), containsString("HTTP 403 Forbidden")); + } + + // view-clients: not existing - should get 404 + setAuthHeader(request, adminClients.get("view-clients")); + try (var response = client.execute(request)) { + assertThat(response.getStatusLine().getStatusCode(), is(404)); } } @@ -366,19 +386,33 @@ public class ClientApiV2AuthorizationTest extends AbstractClientApiV2Test{ request = new HttpDelete(getClientsApiUrl() + "/test-delete-view"); setAuthHeader(request, adminClients.get("view-clients")); try (var response = client.execute(request)) { - assertEquals(403, response.getStatusLine().getStatusCode()); + assertThat(response.getStatusLine().getStatusCode(), is(403)); } // query-clients: should get 403 (lacks requireManage) setAuthHeader(request, adminClients.get("query-clients")); try (var response = client.execute(request)) { - assertEquals(403, response.getStatusLine().getStatusCode()); + assertThat(response.getStatusLine().getStatusCode(), is(403)); } // no-access: should get 403 (lacks requireManage) setAuthHeader(request, adminClients.get("no-access")); try (var response = client.execute(request)) { - assertEquals(403, response.getStatusLine().getStatusCode()); + assertThat(response.getStatusLine().getStatusCode(), is(403)); + } + + // no-access: not existing - should get 403 (lacks canList, prevents ID phishing) + request = new HttpDelete(getClientsApiUrl() + "/does-not-exist"); + setAuthHeader(request, adminClients.get("no-access")); + try (var response = client.execute(request)) { + assertThat(response.getStatusLine().getStatusCode(), is(403)); + assertThat(EntityUtils.toString(response.getEntity()), containsString("HTTP 403 Forbidden")); + } + + // view-clients: not existing - should get 404 + setAuthHeader(request, adminClients.get("view-clients")); + try (var response = client.execute(request)) { + assertThat(response.getStatusLine().getStatusCode(), is(404)); } } @@ -396,7 +430,7 @@ public class ClientApiV2AuthorizationTest extends AbstractClientApiV2Test{ setAuthHeader(request, adminClients.get("realm-admin")); try (var response = client.execute(request)) { EntityUtils.consumeQuietly(response.getEntity()); - assertEquals(200, response.getStatusLine().getStatusCode()); + assertThat(response.getStatusLine().getStatusCode(), is(200)); } // Test accessing non-existent realm - should return 404 @@ -404,7 +438,7 @@ public class ClientApiV2AuthorizationTest extends AbstractClientApiV2Test{ request = new HttpGet(nonExistentRealmUrl); setAuthHeader(request, adminClients.get("realm-admin")); try (var response = client.execute(request)) { - assertEquals(404, response.getStatusLine().getStatusCode()); + assertThat(response.getStatusLine().getStatusCode(), is(404)); } // When accessing a different realm from a non-administration realm, should return 403 @@ -414,7 +448,7 @@ public class ClientApiV2AuthorizationTest extends AbstractClientApiV2Test{ request = new HttpGet(differentRealmUrl); setAuthHeader(request, adminClients.get("realm-admin")); try (var response = client.execute(request)) { - assertEquals(403, response.getStatusLine().getStatusCode()); + assertThat(response.getStatusLine().getStatusCode(), is(403)); } } @@ -433,7 +467,7 @@ public class ClientApiV2AuthorizationTest extends AbstractClientApiV2Test{ request.setEntity(new StringEntity(mapper.writeValueAsString(createClientRep(clientId, "test-role1", "test-role2")))); try (var response = client.execute(request)) { EntityUtils.consumeQuietly(response.getEntity()); - assertEquals(201, response.getStatusLine().getStatusCode()); + assertThat(response.getStatusLine().getStatusCode(), is(201)); } }