feat(admin): change the client scopes of an API client

application/aam-backend-service/src/main/kotlin/com/aamdigital/aambackendservice/admin/README.md

@@ -6,16 +6,17 @@ Parts that depend on other systems live in their own sub-package and report thei
 like `apiclient` (`admin.api-client`, only active with Keycloak admin access: `ConditionalOnKeycloakAdminAccess`).
 
 ## API clients (`apiclient`)
-Administrators of the app create, list, delete and regenerate the secret of Keycloak clients for external systems
-(`ApiClientController`). The backend does this with its own Keycloak admin access (`manage-clients`),
+Administrators of the app create, list and delete Keycloak clients for external systems, and change their client
+scopes or regenerate their secret (`ApiClientController`). The backend does this with its own Keycloak admin access (`manage-clients`),
 so the users of the app need no Keycloak admin role for it.
 
 - `KeycloakApiClientAdministration` talks to the Keycloak Admin REST API. It is the security boundary:
   any client without the `aam.api-client.managed` attribute and the `api-` client id prefix is treated as if it
   did not exist, because `manage-clients` would allow changing every client of the realm (including `app` and
   `aam-backend`).
-- Creating one validates the request (`NewApiClient.kt`): the client scopes offered are those of the enabled
-  feature modules (`ClientScopeRequest` beans).
+- Creating one or changing its client scopes validates the request (`NewApiClient.kt`): the client scopes offered
+  are those of the enabled feature modules (`ClientScopeRequest` beans). Changing the client scopes only adds and
+  removes offered ones; Keycloak's own Default scopes that the client got when it was created (like `profile`) stay.
 
 ## Access Control
 `RequiresAdminAccess`: the `FRONTEND_CLIENT` authority (token issued to the app's Keycloak client)

application/aam-backend-service/src/main/kotlin/com/aamdigital/aambackendservice/admin/apiclient/controller/ApiClientController.kt

@@ -18,6 +18,7 @@ import org.springframework.web.bind.annotation.DeleteMapping
 import org.springframework.web.bind.annotation.GetMapping
 import org.springframework.web.bind.annotation.PathVariable
 import org.springframework.web.bind.annotation.PostMapping
+import org.springframework.web.bind.annotation.PutMapping
 import org.springframework.web.bind.annotation.RequestBody
 import org.springframework.web.bind.annotation.RequestMapping
 import org.springframework.web.bind.annotation.RestController
@@ -65,6 +66,15 @@ data class CreateApiClientRequestDto(
     val clientId: String? = null
 )
 
+/**
+ * Request body of `PUT /v1/admin/api-client/{clientId}/scope`.
+ *
+ * @property scopes client scopes (of [ApiClientScopeDto]) the API client has from now on; at least one
+ */
+data class UpdateApiClientScopesRequestDto(
+    val scopes: List<String>
+)
+
 /**
  * The credentials of an API client, for the external system to request access tokens with the client credentials
  * flow. Only returned when the API client is created or its secret is regenerated.
@@ -147,6 +157,24 @@ class ApiClientController(
             ResponseEntity.ok(credentials.toDto())
         }
 
+    @PutMapping("/{clientId}/scope")
+    fun updateScopes(
+        @PathVariable clientId: String,
+        @RequestBody request: UpdateApiClientScopesRequestDto,
+        authentication: JwtAuthenticationToken
+    ): ResponseEntity<Any> =
+        handleErrors("change the client scopes of", authentication) {
+            val apiClient = apiClientAdministration.updateScopes(clientId, request.scopes.toSet())
+            logger.info(
+                "[audit] User '{}' ({}) changed the client scopes of API client '{}' to {}",
+                authentication.userLabel(),
+                authentication.token.subject,
+                clientId,
+                request.scopes.sorted()
+            )
+            ResponseEntity.ok(apiClient.toDto())
+        }
+
     @DeleteMapping("/{clientId}")
     fun deleteApiClient(
         @PathVariable clientId: String,

application/aam-backend-service/src/main/kotlin/com/aamdigital/aambackendservice/admin/apiclient/core/ApiClientAdministration.kt

@@ -53,5 +53,17 @@ interface ApiClientAdministration {
      */
     fun regenerateSecret(clientId: String): ApiClientCredentials
 
+    /**
+     * Replaces the client scopes of the client, i.e. the access it has. Access tokens issued before keep their
+     * client scopes until they expire.
+     *
+     * @param scopes client scopes to give the API client; only those offered to API clients
+     * @return the API client with its new client scopes
+     */
+    fun updateScopes(
+        clientId: String,
+        scopes: Set<String>
+    ): ApiClient
+
     fun deleteApiClient(clientId: String)
 }

application/aam-backend-service/src/main/kotlin/com/aamdigital/aambackendservice/admin/apiclient/core/KeycloakApiClientAdministration.kt

@@ -112,6 +112,29 @@ class KeycloakApiClientAdministration(
             ApiClientCredentials(clientId = clientId, clientSecret = secret.value)
         }
 
+    override fun updateScopes(
+        clientId: String,
+        scopes: Set<String>
+    ): ApiClient {
+        validateScopes(scopes, offeredScopes)
+        return call("change the client scopes of API client '$clientId'") {
+            val client = findApiClientRepresentation(clientId)
+            val scopeIds = findClientScopeIds(scopes)
+            val clientResource = realmResource.clients().get(client.id)
+
+            // only the offered client scopes are the access the API client is given here; Keycloak's own default
+            // scopes (like "profile") that it got when it was created stay as they are
+            clientResource
+                .defaultClientScopes
+                .filter { it.name in offeredScopes && it.name !in scopes }
+                .forEach { clientResource.removeDefaultClientScope(it.id) }
+            val currentScopeIds = clientResource.defaultClientScopes.map { it.id }.toSet()
+            assignDefaultClientScopes(clientResource, scopeIds.filter { it !in currentScopeIds })
+
+            client.toApiClient()
+        }
+    }
+
     override fun deleteApiClient(clientId: String) =
         call("delete API client '$clientId'") {
             val client = findApiClientRepresentation(clientId)

application/aam-backend-service/src/main/kotlin/com/aamdigital/aambackendservice/admin/apiclient/core/NewApiClient.kt

@@ -43,7 +43,6 @@ internal fun validateNewApiClient(
     scopes: Set<String>,
     offeredScopes: Set<String>
 ) {
-    val unknownScopes = scopes - offeredScopes
     val (code, message) =
         when {
             name.isBlank() || name.length > MAX_API_CLIENT_NAME_LENGTH -> {
@@ -57,6 +56,26 @@ internal fun validateNewApiClient(
                     "digits, '.', '_' or '-' (64 characters at most)"
             }
 
+            else -> {
+                return validateScopes(scopes, offeredScopes)
+            }
+        }
+    throw InvalidArgumentException(message = message, code = code)
+}
+
+/**
+ * Checks the client scopes to give an API client (when creating it or changing its access).
+ *
+ * @param offeredScopes the client scopes that API clients can be given
+ * @throws InvalidArgumentException with an [ApiClientAdministrationError] code if the scopes are invalid
+ */
+internal fun validateScopes(
+    scopes: Set<String>,
+    offeredScopes: Set<String>
+) {
+    val unknownScopes = scopes - offeredScopes
+    val (code, message) =
+        when {
             scopes.isEmpty() -> {
                 ApiClientAdministrationError.NO_SCOPE to "An API client needs at least one client scope"
             }

application/aam-backend-service/src/test/kotlin/com/aamdigital/aambackendservice/admin/apiclient/controller/ApiClientControllerTest.kt

@@ -6,6 +6,7 @@ import ch.qos.logback.classic.spi.ILoggingEvent
 import ch.qos.logback.core.read.ListAppender
 import com.aamdigital.aambackendservice.admin.apiclient.core.ApiClientAdministration
 import com.aamdigital.aambackendservice.admin.apiclient.core.ApiClientAdministrationError
+import com.aamdigital.aambackendservice.admin.apiclient.domain.ApiClient
 import com.aamdigital.aambackendservice.admin.apiclient.domain.ApiClientCredentials
 import com.aamdigital.aambackendservice.common.error.NotFoundException
 import org.assertj.core.api.Assertions.assertThat
@@ -97,6 +98,39 @@ class ApiClientControllerTest {
             .doesNotContain(SECRET)
     }
 
+    @Test
+    fun `should log who changed the client scopes of an API client to which ones`() {
+        // Given
+        whenever(apiClientAdministration.updateScopes(CLIENT_ID, setOf("reporting_read", "reporting_write")))
+            .thenReturn(
+                ApiClient(
+                    clientId = CLIENT_ID,
+                    name = "CRM",
+                    scopes = setOf("reporting_read", "reporting_write"),
+                    createdBy = null,
+                    createdAt = null
+                )
+            )
+
+        // When
+        val response =
+            controller.updateScopes(
+                CLIENT_ID,
+                UpdateApiClientScopesRequestDto(scopes = listOf("reporting_write", "reporting_read")),
+                user
+            )
+
+        // Then
+        assertThat(response.statusCode).isEqualTo(HttpStatus.OK)
+        assertThat(auditLog().formattedMessage)
+            .contains(
+                "manager@example.org",
+                "changed the client scopes",
+                CLIENT_ID,
+                "[reporting_read, reporting_write]"
+            )
+    }
+
     @Test
     fun `should log who deleted an API client`() {
         // When

application/aam-backend-service/src/test/kotlin/com/aamdigital/aambackendservice/admin/apiclient/core/KeycloakApiClientAdministrationIntegrationTest.kt

@@ -179,6 +179,43 @@ class KeycloakApiClientAdministrationIntegrationTest {
             .isInstanceOf(HttpClientErrorException::class.java)
     }
 
+    @Test
+    fun `should replace the client scopes of an API client, so that its new access tokens carry only those`() {
+        // Given
+        val created = create("test update scopes", setOf("reporting_read", "third_party_authentication"))
+        val keycloakScopesBefore = keycloakScopesOf(created.clientId)
+
+        // When
+        val updated = administration.updateScopes(created.clientId, setOf("reporting_read", "reporting_write"))
+
+        // Then
+        assertThat(updated.scopes)
+            .contains("reporting_read", "reporting_write")
+            .doesNotContain("third_party_authentication")
+        val token = authTestingService.fetchToken(created.clientId, created.clientSecret, REALM)
+        assertThat(claimsOf(token).get("scope").textValue().split(" "))
+            .contains("reporting_read", "reporting_write")
+            .doesNotContain("third_party_authentication")
+        // Keycloak's own default scopes (like "profile") stay as they were
+        assertThat(keycloakScopesOf(created.clientId)).isEqualTo(keycloakScopesBefore)
+    }
+
+    @Test
+    fun `should not change the client scopes of an API client to ones that are not offered`() {
+        // Given
+        val created = create("test update unknown scope", setOf("reporting_read"))
+
+        // Then
+        assertThatThrownBy { administration.updateScopes(created.clientId, setOf("offline_access")) }
+            .extracting("code")
+            .isEqualTo(ApiClientAdministrationError.UNKNOWN_SCOPE)
+        assertThatThrownBy { administration.updateScopes(created.clientId, emptySet()) }
+            .extracting("code")
+            .isEqualTo(ApiClientAdministrationError.NO_SCOPE)
+        assertThat(administration.findApiClients().single { it.clientId == created.clientId }.scopes)
+            .contains("reporting_read")
+    }
+
     @Test
     fun `should delete an API client`() {
         // Given
@@ -215,6 +252,8 @@ class KeycloakApiClientAdministrationIntegrationTest {
                 .isEqualTo(ApiClientAdministrationError.API_CLIENT_NOT_FOUND)
             assertThatThrownBy { administration.regenerateSecret(clientId) }
                 .isInstanceOf(NotFoundException::class.java)
+            assertThatThrownBy { administration.updateScopes(clientId, setOf("reporting_read")) }
+                .isInstanceOf(NotFoundException::class.java)
             assertThat(clients.findByClientId(clientId)).hasSize(1)
         }
     }
@@ -295,6 +334,17 @@ class KeycloakApiClientAdministrationIntegrationTest {
         ).doesNotContain("test missing scope")
     }
 
+    /** The Default client scopes of the client that are not offered to API clients, like Keycloak's "profile". */
+    private fun keycloakScopesOf(clientId: String): Set<String> {
+        val clients = admin().realm(REALM).clients()
+        return clients
+            .get(clients.findByClientId(clientId).single().id)
+            .defaultClientScopes
+            .map { it.name }
+            .filter { it !in setOf("reporting_read", "reporting_write", "third_party_authentication") }
+            .toSet()
+    }
+
     private fun claimsOf(accessToken: String?): JsonNode {
         val payload = checkNotNull(accessToken).split(".")[1]
         return jacksonObjectMapper().readTree(Base64.getUrlDecoder().decode(payload))

application/aam-backend-service/src/test/kotlin/com/aamdigital/aambackendservice/e2e/CucumberIntegrationTest.kt

@@ -244,6 +244,29 @@ class CucumberIntegrationTest(
         exchange("$prefix$storedId$suffix", HttpMethod.POST)
     }
 
+    @When("the client calls PUT {} with stored id and suffix {} and body {}")
+    @Throws(Throwable::class)
+    fun `the client issues PUT endpoint with stored id, suffix and body`(
+        prefix: String,
+        suffix: String,
+        body: String
+    ) {
+        exchange(
+            "$prefix$storedId$suffix",
+            HttpMethod.PUT,
+            File("src/test/resources/database/documents/$body.json").readText()
+        )
+    }
+
+    @When("the client calls PUT {} with body {}")
+    @Throws(Throwable::class)
+    fun `the client issues PUT endpoint with body`(
+        endpoint: String,
+        body: String
+    ) {
+        exchange(endpoint, HttpMethod.PUT, File("src/test/resources/database/documents/$body.json").readText())
+    }
+
     @When("the client calls DELETE {} with stored id")
     @Throws(Throwable::class)
     fun `the client issues DELETE endpoint with stored id`(endpoint: String) {

application/aam-backend-service/src/test/resources/cucumber/features/admin/api-client.feature

@@ -76,6 +76,8 @@ Feature: Self-service API clients, managed by the administrators of the app
         Then the client receives status code of 404
         When the client calls POST /v1/admin/api-client/aam-backend/secret without body
         Then the client receives status code of 404
+        When the client calls PUT /v1/admin/api-client/app/scope with body UpdateApiClientScopesRequest_read
+        Then the client receives status code of 404
 
     Scenario: An administrator who manages accounts can give the third_party_authentication client scope
         Given signed in as user account-admin with password account-admin-password through client app in realm dummy-realm
@@ -84,3 +86,17 @@ Feature: Self-service API clients, managed by the administrators of the app
         Given the client stores the clientId from latest response
         When the client calls DELETE /v1/admin/api-client/ with stored id
         Then the client receives status code of 204
+
+    Scenario: An administrator who manages accounts changes the client scopes of an API client
+        Given signed in as user account-admin with password account-admin-password through client app in realm dummy-realm
+        When the client calls POST /v1/admin/api-client with body CreateApiClientRequest_reporting
+        Then the client receives status code of 201
+        Given the client stores the clientId from latest response
+        When the client calls PUT /v1/admin/api-client/ with stored id and suffix /scope and body UpdateApiClientScopesRequest_read
+        Then the client receives status code of 200
+        Then the client receives a non-empty value for property clientId
+        When the client calls PUT /v1/admin/api-client/ with stored id and suffix /scope and body UpdateApiClientScopesRequest_unknown
+        Then the client receives status code of 400
+        Then the client receives value UNKNOWN_SCOPE for property errorCode
+        When the client calls DELETE /v1/admin/api-client/ with stored id
+        Then the client receives status code of 204

application/aam-backend-service/src/test/resources/database/documents/UpdateApiClientScopesRequest_read.json

@@ -0,0 +1,3 @@
+{
+  "scopes": ["reporting_read"]
+}

application/aam-backend-service/src/test/resources/database/documents/UpdateApiClientScopesRequest_unknown.json

@@ -0,0 +1,3 @@
+{
+  "scopes": ["offline_access"]
+}