diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/sysadmin/AppIdentityRegistryService.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/sysadmin/AppIdentityRegistryService.java index 0131e6dc9..8932e96f1 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/sysadmin/AppIdentityRegistryService.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/sysadmin/AppIdentityRegistryService.java @@ -248,6 +248,33 @@ public CompletableFuture updateAppIdentity(ServiceRequestContext ct }); } + /** + * POST /appIdentities/{appId}/secret + * + *

Regenerates the secret of the deactivated token of the specified {@code appId} and returns the + * token with a newly-generated secret. The token must be deactivated first and the new secret does + * not authenticate until the token is activated, so that the new secret can be distributed to the + * clients before it takes effect. + */ + @Post("/appIdentities/{appId}/secret") + public CompletableFuture regenerateTokenSecret(ServiceRequestContext ctx, + @Param String appId, + Author author, User loginUser) { + // Permission is checked at the HTTP layer; the metadata layer is caller-agnostic. + return getTokenOrRespondForbidden(ctx, appId, loginUser).thenCompose( + token -> { + if (token.isDeleted()) { + throw new IllegalArgumentException( + "You can't regenerate the secret of the token scheduled for deletion."); + } + if (token.isActive()) { + throw new IllegalArgumentException( + "You can't regenerate the secret of an active token. Deactivate it first."); + } + return mds.regenerateTokenSecret(author, appId); + }); + } + /** * PATCH /appIdentities/{appId}/level * diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/TransformingChangesApplier.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/TransformingChangesApplier.java index 943ef6956..feb0da8fc 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/TransformingChangesApplier.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/TransformingChangesApplier.java @@ -76,8 +76,8 @@ int doApply(Revision headRevision, DirCache dirCache, } catch (CentralDogmaException e) { throw e; } catch (Exception e) { - throw new ChangeConflictException("failed to transform the content: " + oldJsonNode + - " transformer: " + transformer, e); + // Do not include the old content in the message; it may contain sensitive data such as secrets. + throw new ChangeConflictException("failed to transform the content with " + transformer, e); } return 0; } diff --git a/server/src/main/java/com/linecorp/centraldogma/server/metadata/AppIdentityService.java b/server/src/main/java/com/linecorp/centraldogma/server/metadata/AppIdentityService.java index db742735c..160cb209a 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/metadata/AppIdentityService.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/metadata/AppIdentityService.java @@ -168,6 +168,56 @@ Revision purgeAppIdentity(Author author, String appId) { .join(); } + CompletableFuture regenerateTokenSecret(Author author, String appId) { + requireNonNull(author, "author"); + requireNonNull(appId, "appId"); + + final String commitSummary = "Regenerate the secret of the token: " + appId; + + final AppIdentityRegistryTransformer transformer = new AppIdentityRegistryTransformer( + (headRevision, registry) -> { + final AppIdentity appIdentity = registry.get(appId); // Raise an exception if not found. + if (appIdentity.deletion() != null) { + // Note that a ChangeConflictException is raised instead of an + // IllegalArgumentException so that the storage layer does not wrap it with + // another exception. + throw new ChangeConflictException( + "The app identity is already destroyed: " + appId); + } + throwIfInvalidType(appId, appIdentity, AppIdentityType.TOKEN); + if (appIdentity.deactivation() == null) { + // Regenerating the secret of an active token would break its clients with no + // way to prepare, so the token must be deactivated first. + throw new ChangeConflictException( + "The token must be deactivated before regenerating its secret: " + appId); + } + + final Token token = (Token) appIdentity; + final String newSecret = SECRET_PREFIX + UUID.randomUUID(); + if (registry.secrets().containsKey(newSecret)) { + throw new ChangeConflictException("Secret already exists"); + } + + final Token newToken = new Token(token.appId(), newSecret, token.isSystemAdmin(), + token.allowGuestAccess(), token.creation(), + token.deactivation(), null); + final Map newAppIds = + updateMap(registry.appIds(), appId, newToken); + // A deactivated token has no entry in the secret map; create a new map so that + // the new registry does not share the mutable map with the old one. + return new AppIdentityRegistry(newAppIds, ImmutableMap.copyOf(registry.secrets()), + registry.certificateIds()); + }); + // Read the registry back at the revision this commit produced so that the caller gets + // the secret of this commit even if another commit lands right after. + return appIdentityRegistryRepo.push(INTERNAL_PROJECT_DOGMA, Project.REPO_DOGMA, author, + commitSummary, transformer) + .thenCompose(revision -> appIdentityRegistryRepo.fetch( + INTERNAL_PROJECT_DOGMA, Project.REPO_DOGMA, TOKEN_JSON, + revision)) + .thenApply(holder -> (Token) holder.object().get(appId)); + } + CompletableFuture activateToken(Author author, String appId) { requireNonNull(author, "author"); requireNonNull(appId, "appId"); diff --git a/server/src/main/java/com/linecorp/centraldogma/server/metadata/MetadataService.java b/server/src/main/java/com/linecorp/centraldogma/server/metadata/MetadataService.java index e1a108be2..e6ef290ab 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/metadata/MetadataService.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/metadata/MetadataService.java @@ -1165,6 +1165,16 @@ public CompletableFuture deactivateToken(Author author, String appId) return appIdentityService.deactivateToken(author, appId); } + /** + * Regenerates the secret of the deactivated {@link Token} of the specified {@code appId} and + * returns the {@link Token} with the newly-generated secret. The token must be deactivated first + * and the new secret does not authenticate until the token is activated. The regeneration fails + * with a {@link ChangeConflictException} if the token is still active. + */ + public CompletableFuture regenerateTokenSecret(Author author, String appId) { + return appIdentityService.regenerateTokenSecret(author, appId); + } + /** * Returns an {@link AppIdentity} which has the specified {@code appId}. */ diff --git a/server/src/main/java/com/linecorp/centraldogma/server/metadata/RepositorySupport.java b/server/src/main/java/com/linecorp/centraldogma/server/metadata/RepositorySupport.java index 3a0b8fb0c..7b80e79f2 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/metadata/RepositorySupport.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/metadata/RepositorySupport.java @@ -66,6 +66,13 @@ CompletableFuture> fetch(String projectName, String repoNa return fetch(projectManager().get(projectName).repos().get(repoName), path); } + CompletableFuture> fetch(String projectName, String repoName, String path, + Revision revision) { + requireNonNull(projectName, "projectName"); + requireNonNull(repoName, "repoName"); + return fetch(projectManager().get(projectName).repos().get(repoName), path, revision); + } + private CompletableFuture> fetch(Repository repository, String path) { requireNonNull(path, "path"); final Revision revision = normalize(repository); diff --git a/server/src/test/java/com/linecorp/centraldogma/server/internal/api/AppIdentityRegistryServiceViaHttpTest.java b/server/src/test/java/com/linecorp/centraldogma/server/internal/api/AppIdentityRegistryServiceViaHttpTest.java index 4dcc44429..f961e8936 100644 --- a/server/src/test/java/com/linecorp/centraldogma/server/internal/api/AppIdentityRegistryServiceViaHttpTest.java +++ b/server/src/test/java/com/linecorp/centraldogma/server/internal/api/AppIdentityRegistryServiceViaHttpTest.java @@ -18,8 +18,10 @@ import static com.linecorp.centraldogma.internal.api.v1.HttpApiV1Constants.API_V1_PATH_PREFIX; import static com.linecorp.centraldogma.testing.internal.auth.TestAuthMessageUtil.getAccessToken; import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; import java.net.URI; +import java.net.UnknownHostException; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -29,9 +31,11 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonMappingException; import com.fasterxml.jackson.databind.JsonNode; +import com.google.common.collect.ImmutableMap; import com.linecorp.armeria.client.WebClient; import com.linecorp.armeria.common.AggregatedHttpResponse; +import com.linecorp.armeria.common.Cookie; import com.linecorp.armeria.common.HttpData; import com.linecorp.armeria.common.HttpHeaderNames; import com.linecorp.armeria.common.HttpMethod; @@ -40,8 +44,14 @@ import com.linecorp.armeria.common.QueryParams; import com.linecorp.armeria.common.RequestHeaders; import com.linecorp.armeria.common.auth.AuthToken; +import com.linecorp.centraldogma.client.CentralDogma; +import com.linecorp.centraldogma.client.armeria.ArmeriaCentralDogmaBuilder; +import com.linecorp.centraldogma.common.Change; +import com.linecorp.centraldogma.common.Entry; +import com.linecorp.centraldogma.common.ProjectRole; import com.linecorp.centraldogma.internal.Jackson; import com.linecorp.centraldogma.server.CentralDogmaBuilder; +import com.linecorp.centraldogma.server.internal.api.MetadataApiService.IdAndProjectRole; import com.linecorp.centraldogma.testing.internal.auth.TestAuthMessageUtil; import com.linecorp.centraldogma.testing.internal.auth.TestAuthProviderFactory; import com.linecorp.centraldogma.testing.junit.CentralDogmaExtension; @@ -58,19 +68,390 @@ protected void configure(CentralDogmaBuilder builder) { } }; + private static String systemAdminAccessToken; private static WebClient systemAdminClient; @BeforeAll static void setUp() throws JsonMappingException, JsonParseException { final URI uri = dogma.httpClient().uri(); + systemAdminAccessToken = getAccessToken(dogma.httpClient(), + TestAuthMessageUtil.USERNAME, + TestAuthMessageUtil.PASSWORD, + true); systemAdminClient = WebClient.builder(uri) - .auth(AuthToken.ofOAuth2(getAccessToken(dogma.httpClient(), - TestAuthMessageUtil.USERNAME, - TestAuthMessageUtil.PASSWORD, - true))) + .auth(AuthToken.ofOAuth2(systemAdminAccessToken)) .build(); } + @Test + void regenerateTokenSecret() throws JsonProcessingException { + final AggregatedHttpResponse createResponse = + systemAdminClient.post(API_V1_PATH_PREFIX + "appIdentities", + QueryParams.of("appId", "forRegenerate", "type", "TOKEN", + "isSystemAdmin", false), + HttpData.empty()) + .aggregate() + .join(); + assertThat(createResponse.status()).isEqualTo(HttpStatus.CREATED); + final JsonNode created = Jackson.readTree(createResponse.contentUtf8()); + final String oldSecret = created.get("secret").asText(); + + // The old secret authenticates requests. + assertThat(newTokenClient(oldSecret).get(API_V1_PATH_PREFIX + "appIdentities").aggregate().join() + .status()).isEqualTo(HttpStatus.OK); + + // Deactivate the token to revoke the old secret. + setStatus(systemAdminClient, "forRegenerate", "inactive"); + // The registry used by the authorizer is updated asynchronously so await the changes. + await().untilAsserted(() -> { + assertThat(newTokenClient(oldSecret).get(API_V1_PATH_PREFIX + "appIdentities").aggregate().join() + .status()).isEqualTo(HttpStatus.UNAUTHORIZED); + }); + + final AggregatedHttpResponse regenerateResponse = + systemAdminClient.post(API_V1_PATH_PREFIX + "appIdentities/forRegenerate/secret", + HttpData.empty()) + .aggregate() + .join(); + assertThat(regenerateResponse.status()).isEqualTo(HttpStatus.OK); + final JsonNode regenerated = Jackson.readTree(regenerateResponse.contentUtf8()); + assertThat(regenerated.get("appId").asText()).isEqualTo("forRegenerate"); + final String newSecret = regenerated.get("secret").asText(); + assertThat(newSecret).startsWith("appToken-") + .isNotEqualTo(oldSecret); + // Everything except the secret is preserved. + assertThat(regenerated.get("creation")).isEqualTo(created.get("creation")); + assertThat(regenerated.get("systemAdmin").asBoolean()).isFalse(); + assertThat(regenerated.get("allowGuestAccess").asBoolean()) + .isEqualTo(created.get("allowGuestAccess").asBoolean()); + // The token remains deactivated so neither secret authenticates yet. + assertThat(regenerated.get("deactivation")).isNotNull(); + await().untilAsserted(() -> { + assertThat(newTokenClient(newSecret).get(API_V1_PATH_PREFIX + "appIdentities").aggregate().join() + .status()).isEqualTo(HttpStatus.UNAUTHORIZED); + assertThat(newTokenClient(oldSecret).get(API_V1_PATH_PREFIX + "appIdentities").aggregate().join() + .status()).isEqualTo(HttpStatus.UNAUTHORIZED); + }); + + // Activating the token makes the new secret usable while the old one stays revoked. + setStatus(systemAdminClient, "forRegenerate", "active"); + await().untilAsserted(() -> { + assertThat(newTokenClient(newSecret).get(API_V1_PATH_PREFIX + "appIdentities").aggregate().join() + .status()).isEqualTo(HttpStatus.OK); + assertThat(newTokenClient(oldSecret).get(API_V1_PATH_PREFIX + "appIdentities").aggregate().join() + .status()).isEqualTo(HttpStatus.UNAUTHORIZED); + }); + } + + @Test + void rotatedTokenKeepsItsRolesAndPermissions() throws UnknownHostException, JsonProcessingException { + // Scaffold a project with a file, using the system administrator's token. + final CentralDogma adminDogmaClient = newDogmaClient(systemAdminAccessToken); + adminDogmaClient.createProject("rotationProj").join(); + adminDogmaClient.createRepository("rotationProj", "rotationRepo").join(); + adminDogmaClient.forRepo("rotationProj", "rotationRepo") + .commit("Add a.txt", Change.ofTextUpsert("/a.txt", "foo")) + .push() + .join(); + + // Create a token and register it to the project as a member. + final AggregatedHttpResponse createResponse = + systemAdminClient.post(API_V1_PATH_PREFIX + "appIdentities", + QueryParams.of("appId", "forRoles", "type", "TOKEN", + "isSystemAdmin", false), + HttpData.empty()) + .aggregate() + .join(); + assertThat(createResponse.status()).isEqualTo(HttpStatus.CREATED); + final String oldSecret = Jackson.readTree(createResponse.contentUtf8()).get("secret").asText(); + assertThat(systemAdminClient.blocking().prepare() + .post(API_V1_PATH_PREFIX + "metadata/rotationProj/appIdentities") + .contentJson(new IdAndProjectRole("forRoles", ProjectRole.MEMBER)) + .execute() + .status()).isEqualTo(HttpStatus.OK); + + // The token can read the repository with the old secret. + final CentralDogma oldSecretClient = newDogmaClient(oldSecret); + await().untilAsserted(() -> { + final Entry entry = oldSecretClient.forRepo("rotationProj", "rotationRepo") + .file("/a.txt").get().join(); + assertThat(entry.contentAsText().trim()).isEqualTo("foo"); + }); + + // Rotate the secret: deactivate, regenerate and activate. + setStatus(systemAdminClient, "forRoles", "inactive"); + final AggregatedHttpResponse regenerateResponse = + systemAdminClient.post(API_V1_PATH_PREFIX + "appIdentities/forRoles/secret", + HttpData.empty()) + .aggregate() + .join(); + assertThat(regenerateResponse.status()).isEqualTo(HttpStatus.OK); + final String newSecret = Jackson.readTree(regenerateResponse.contentUtf8()).get("secret").asText(); + setStatus(systemAdminClient, "forRoles", "active"); + + // The token keeps its project role: the new secret can read the repository without + // being registered again, while the old secret cannot authenticate anymore. + final CentralDogma newSecretClient = newDogmaClient(newSecret); + await().untilAsserted(() -> { + final Entry entry = newSecretClient.forRepo("rotationProj", "rotationRepo") + .file("/a.txt").get().join(); + assertThat(entry.contentAsText().trim()).isEqualTo("foo"); + assertThat(newTokenClient(oldSecret).get(API_V1_PATH_PREFIX + "appIdentities").aggregate() + .join().status()).isEqualTo(HttpStatus.UNAUTHORIZED); + }); + } + + @Test + void systemAdminCanRegenerateOthersTokenSecret() throws JsonProcessingException { + // A non-admin user creates and deactivates its own token. + final WebClient userClient = + WebClient.builder(dogma.httpClient().uri()) + .auth(AuthToken.ofOAuth2(getAccessToken(dogma.httpClient(), + TestAuthMessageUtil.USERNAME2, + TestAuthMessageUtil.PASSWORD2, + "adminBypassLogin", + false))) + .build(); + assertThat(userClient.post(API_V1_PATH_PREFIX + "appIdentities", + QueryParams.of("appId", "rotatedByAdmin", "type", "TOKEN", + "isSystemAdmin", false), + HttpData.empty()) + .aggregate() + .join() + .status()).isEqualTo(HttpStatus.CREATED); + setStatus(userClient, "rotatedByAdmin", "inactive"); + + // A system administrator can regenerate the secret of a token it does not own. + assertThat(systemAdminClient.post(API_V1_PATH_PREFIX + "appIdentities/rotatedByAdmin/secret", + HttpData.empty()) + .aggregate() + .join() + .status()).isEqualTo(HttpStatus.OK); + } + + @Test + void consecutiveRegenerationsKeepOnlyTheLastSecret() throws JsonProcessingException { + final AggregatedHttpResponse createResponse = + systemAdminClient.post(API_V1_PATH_PREFIX + "appIdentities", + QueryParams.of("appId", "forTwice", "type", "TOKEN", + "isSystemAdmin", false), + HttpData.empty()) + .aggregate() + .join(); + assertThat(createResponse.status()).isEqualTo(HttpStatus.CREATED); + setStatus(systemAdminClient, "forTwice", "inactive"); + + // Both regenerations succeed but only the last secret survives. + final AggregatedHttpResponse firstResponse = + systemAdminClient.post(API_V1_PATH_PREFIX + "appIdentities/forTwice/secret", + HttpData.empty()) + .aggregate() + .join(); + assertThat(firstResponse.status()).isEqualTo(HttpStatus.OK); + final String firstSecret = Jackson.readTree(firstResponse.contentUtf8()).get("secret").asText(); + final AggregatedHttpResponse secondResponse = + systemAdminClient.post(API_V1_PATH_PREFIX + "appIdentities/forTwice/secret", + HttpData.empty()) + .aggregate() + .join(); + assertThat(secondResponse.status()).isEqualTo(HttpStatus.OK); + final String secondSecret = Jackson.readTree(secondResponse.contentUtf8()).get("secret").asText(); + assertThat(secondSecret).isNotEqualTo(firstSecret); + + setStatus(systemAdminClient, "forTwice", "active"); + await().untilAsserted(() -> { + assertThat(newTokenClient(secondSecret).get(API_V1_PATH_PREFIX + "appIdentities").aggregate() + .join().status()).isEqualTo(HttpStatus.OK); + assertThat(newTokenClient(firstSecret).get(API_V1_PATH_PREFIX + "appIdentities").aggregate() + .join().status()).isEqualTo(HttpStatus.UNAUTHORIZED); + }); + } + + @Test + void cannotRegenerateSecretOfPurgedToken() { + assertThat(systemAdminClient.post(API_V1_PATH_PREFIX + "appIdentities", + QueryParams.of("appId", "forPurged", "type", "TOKEN", + "isSystemAdmin", false), + HttpData.empty()) + .aggregate() + .join() + .status()).isEqualTo(HttpStatus.CREATED); + assertThat(systemAdminClient.delete(API_V1_PATH_PREFIX + "appIdentities/forPurged") + .aggregate() + .join() + .status()).isEqualTo(HttpStatus.NO_CONTENT); + assertThat(systemAdminClient.delete(API_V1_PATH_PREFIX + "appIdentities/forPurged/removed") + .aggregate() + .join() + .status()).isEqualTo(HttpStatus.NO_CONTENT); + + assertThat(systemAdminClient.post(API_V1_PATH_PREFIX + "appIdentities/forPurged/secret", + HttpData.empty()) + .aggregate() + .join() + .status()).isEqualTo(HttpStatus.NOT_FOUND); + } + + @Test + void cannotRegenerateSecretWithoutAuthentication() { + assertThat(WebClient.of(dogma.httpClient().uri()) + .post(API_V1_PATH_PREFIX + "appIdentities/nonexistent/secret", HttpData.empty()) + .aggregate() + .join() + .status()).isEqualTo(HttpStatus.UNAUTHORIZED); + } + + @Test + void cannotRegenerateSecretWithoutCsrfToken() { + assertThat(systemAdminClient.post(API_V1_PATH_PREFIX + "appIdentities", + QueryParams.of("appId", "forCsrf", "type", "TOKEN", + "isSystemAdmin", false), + HttpData.empty()) + .aggregate() + .join() + .status()).isEqualTo(HttpStatus.CREATED); + + // A session-cookie request without the CSRF token header must be rejected. + final AggregatedHttpResponse loginResponse = + TestAuthMessageUtil.login(dogma.httpClient(), + TestAuthMessageUtil.USERNAME, + TestAuthMessageUtil.PASSWORD); + assertThat(loginResponse.status()).isEqualTo(HttpStatus.OK); + final Cookie sessionCookie = TestAuthMessageUtil.getSessionCookie(loginResponse); + final RequestHeaders headers = + RequestHeaders.builder(HttpMethod.POST, API_V1_PATH_PREFIX + "appIdentities/forCsrf/secret") + .cookie(sessionCookie) + .build(); + assertThat(dogma.httpClient().execute(headers, HttpData.empty()).aggregate().join() + .status()).isEqualTo(HttpStatus.FORBIDDEN); + } + + private static CentralDogma newDogmaClient(String accessToken) throws UnknownHostException { + return new ArmeriaCentralDogmaBuilder() + .host(dogma.serverAddress().getHostString(), dogma.serverAddress().getPort()) + .accessToken(accessToken) + .build(); + } + + @Test + void cannotRegenerateSecretOfActiveToken() { + assertThat(systemAdminClient.post(API_V1_PATH_PREFIX + "appIdentities", + QueryParams.of("appId", "forActive", "type", "TOKEN", + "isSystemAdmin", false), + HttpData.empty()) + .aggregate() + .join() + .status()).isEqualTo(HttpStatus.CREATED); + + final AggregatedHttpResponse response = + systemAdminClient.post(API_V1_PATH_PREFIX + "appIdentities/forActive/secret", + HttpData.empty()) + .aggregate() + .join(); + assertThat(response.status()).isEqualTo(HttpStatus.BAD_REQUEST); + assertThat(response.contentUtf8()).contains("Deactivate it first"); + } + + @Test + void ownerCanRegenerateOwnTokenSecret() throws JsonProcessingException { + // A non-admin user creates its own token, deactivates it and regenerates its secret. + final WebClient userClient = + WebClient.builder(dogma.httpClient().uri()) + .auth(AuthToken.ofOAuth2(getAccessToken(dogma.httpClient(), + TestAuthMessageUtil.USERNAME2, + TestAuthMessageUtil.PASSWORD2, + "ownerAppId", + false))) + .build(); + assertThat(userClient.post(API_V1_PATH_PREFIX + "appIdentities", + QueryParams.of("appId", "ownedByUser2", "type", "TOKEN", + "isSystemAdmin", false), + HttpData.empty()) + .aggregate() + .join() + .status()).isEqualTo(HttpStatus.CREATED); + setStatus(userClient, "ownedByUser2", "inactive"); + + final AggregatedHttpResponse response = + userClient.post(API_V1_PATH_PREFIX + "appIdentities/ownedByUser2/secret", HttpData.empty()) + .aggregate() + .join(); + assertThat(response.status()).isEqualTo(HttpStatus.OK); + } + + @Test + void cannotRegenerateSecretOfMissingToken() { + assertThat(systemAdminClient.post(API_V1_PATH_PREFIX + "appIdentities/nonexistent/secret", + HttpData.empty()) + .aggregate() + .join() + .status()).isEqualTo(HttpStatus.NOT_FOUND); + } + + @Test + void cannotRegenerateSecretWithoutPermission() { + assertThat(systemAdminClient.post(API_V1_PATH_PREFIX + "appIdentities", + QueryParams.of("appId", "ownedBySystemAdmin", "type", "TOKEN", + "isSystemAdmin", false), + HttpData.empty()) + .aggregate() + .join() + .status()).isEqualTo(HttpStatus.CREATED); + + final WebClient userClient = + WebClient.builder(dogma.httpClient().uri()) + .auth(AuthToken.ofOAuth2(getAccessToken(dogma.httpClient(), + TestAuthMessageUtil.USERNAME2, + TestAuthMessageUtil.PASSWORD2, + "appIdOfUser2", + false))) + .build(); + assertThat(userClient.post(API_V1_PATH_PREFIX + "appIdentities/ownedBySystemAdmin/secret", + HttpData.empty()) + .aggregate() + .join() + .status()).isEqualTo(HttpStatus.FORBIDDEN); + } + + @Test + void cannotRegenerateSecretOfDestroyedToken() { + assertThat(systemAdminClient.post(API_V1_PATH_PREFIX + "appIdentities", + QueryParams.of("appId", "forDestroyed", "type", "TOKEN", + "isSystemAdmin", false), + HttpData.empty()) + .aggregate() + .join() + .status()).isEqualTo(HttpStatus.CREATED); + // The DELETE method always responds with 204 No Content on success. + assertThat(systemAdminClient.delete(API_V1_PATH_PREFIX + "appIdentities/forDestroyed") + .aggregate() + .join() + .status()).isEqualTo(HttpStatus.NO_CONTENT); + + final AggregatedHttpResponse response = + systemAdminClient.post(API_V1_PATH_PREFIX + "appIdentities/forDestroyed/secret", + HttpData.empty()) + .aggregate() + .join(); + assertThat(response.status()).isEqualTo(HttpStatus.BAD_REQUEST); + assertThat(response.contentUtf8()).contains("scheduled for deletion"); + } + + private static WebClient newTokenClient(String secret) { + return WebClient.builder(dogma.httpClient().uri()) + .auth(AuthToken.ofOAuth2(secret)) + .build(); + } + + private static void setStatus(WebClient client, String appId, String status) { + final AggregatedHttpResponse response = + client.blocking().prepare() + .patch(API_V1_PATH_PREFIX + "appIdentities/" + appId) + .contentJson(ImmutableMap.of("status", status)) + .execute(); + assertThat(response.status()).isEqualTo(HttpStatus.OK); + } + @Test void createTokenAndUpdateLevel() throws JsonProcessingException { assertThat(systemAdminClient.post(API_V1_PATH_PREFIX + "appIdentities", diff --git a/server/src/test/java/com/linecorp/centraldogma/server/metadata/MetadataServiceTest.java b/server/src/test/java/com/linecorp/centraldogma/server/metadata/MetadataServiceTest.java index 8a897da20..0b0182b16 100644 --- a/server/src/test/java/com/linecorp/centraldogma/server/metadata/MetadataServiceTest.java +++ b/server/src/test/java/com/linecorp/centraldogma/server/metadata/MetadataServiceTest.java @@ -554,6 +554,81 @@ void tokenActivationAndDeactivation() { assertThat(mds.activateToken(author, app1).join().major()).isEqualTo(revision.major() + 1); } + @Test + void regenerateTokenSecret() { + final MetadataService mds = newMetadataService(manager); + + mds.createToken(author, app1).join(); + await().untilAsserted(() -> assertThat(mds.getAppIdentityRegistry().getOrDefault(app1, null)) + .isNotNull()); + final Token token = (Token) mds.getAppIdentityRegistry().get(app1); + final String oldSecret = token.secret(); + assertThat(oldSecret).isNotNull(); + + mds.deactivateToken(author, app1).join(); + await().untilAsserted(() -> assertThat(mds.getAppIdentityRegistry().get(app1).isActive()).isFalse()); + + final Token returned = mds.regenerateTokenSecret(author, app1).join(); + final String newSecret = returned.secret(); + assertThat(newSecret).isNotNull() + .startsWith("appToken-") + .isNotEqualTo(oldSecret); + assertThat(returned.creation()).isEqualTo(token.creation()); + + // The registry has the token with the new secret. + await().untilAsserted(() -> assertThat(((Token) mds.getAppIdentityRegistry().get(app1)).secret()) + .isEqualTo(newSecret)); + + // The token remains deactivated so neither secret resolves to it yet. + assertThat(returned.isActive()).isFalse(); + assertThatThrownBy(() -> mds.findTokenBySecret(newSecret)) + .isInstanceOf(AppIdentityNotFoundException.class); + assertThatThrownBy(() -> mds.findTokenBySecret(oldSecret)) + .isInstanceOf(AppIdentityNotFoundException.class); + + // Activating the token makes the new secret usable while the old one stays revoked. + mds.activateToken(author, app1).join(); + await().untilAsserted(() -> assertThat(mds.getAppIdentityRegistry().get(app1).isActive()).isTrue()); + assertThat(mds.findTokenBySecret(newSecret).appId()).isEqualTo(app1); + assertThatThrownBy(() -> mds.findTokenBySecret(oldSecret)) + .isInstanceOf(AppIdentityNotFoundException.class); + } + + @Test + void cannotRegenerateSecretOfActiveToken() { + final MetadataService mds = newMetadataService(manager); + + mds.createToken(author, app1).join(); + + assertThatThrownBy(() -> mds.regenerateTokenSecret(author, app1).join()) + .hasCauseInstanceOf(ChangeConflictException.class) + .hasStackTraceContaining("must be deactivated"); + } + + @Test + void cannotRegenerateSecretOfDestroyedToken() { + final MetadataService mds = newMetadataService(manager); + + mds.createToken(author, app1).join(); + mds.destroyToken(author, app1).join(); + + assertThatThrownBy(() -> mds.regenerateTokenSecret(author, app1).join()) + .hasCauseInstanceOf(ChangeConflictException.class) + .hasStackTraceContaining("already destroyed"); + } + + @Test + void cannotRegenerateSecretOfCertificate() { + final MetadataService mds = newMetadataService(manager); + + mds.createCertificate(author, cert1, certificateId1, false).join(); + + // An IllegalArgumentException raised in a transformer is wrapped with a ChangeConflictException. + assertThatThrownBy(() -> mds.regenerateTokenSecret(author, cert1).join()) + .hasRootCauseInstanceOf(IllegalArgumentException.class) + .hasStackTraceContaining("not a TOKEN"); + } + @Test void certificateActivationAndDeactivation() { final MetadataService mds = newMetadataService(manager); diff --git a/site/src/sphinx/auth.rst b/site/src/sphinx/auth.rst index 5bf206ac6..47ce71130 100644 --- a/site/src/sphinx/auth.rst +++ b/site/src/sphinx/auth.rst @@ -366,7 +366,20 @@ request comes from. Anyone who is logged into the Central Dogma can create a new ``Application Token``, and the token is shared for everyone. So any owner of a project can add any token to their project. However only both the token -creator and the system administrator are allowed to deactivate and/or remove the token. +creator and the system administrator are allowed to deactivate, remove and/or regenerate the token. + +If the secret of a token is leaked, the token creator or a system administrator can rotate the secret +without losing the roles and permissions granted to the application ID: + +1. Deactivate the token. The leaked secret is revoked and stops authenticating. Note that it may take + a short time for the change to be propagated to the authorization cache of each server. +2. Regenerate the secret with the ``Regenerate secret`` button of the web UI or + ``POST /api/v1/appIdentities/{appId}/secret``. A newly-generated secret is issued to the same + application ID, but it does not authenticate yet because the token is still deactivated. A token + must be deactivated before its secret is regenerated. +3. Distribute the new secret to the clients of the token. +4. Activate the token. The new secret starts authenticating once the activation is propagated to the + authorization cache of each server. There are two levels of a token, which are ``System Admin`` and ``User``. ``System Admin`` level token can be created by only the system administrators. A client who sends a request with the token is allowed to access diff --git a/webapp/src/dogma/features/api/apiSlice.ts b/webapp/src/dogma/features/api/apiSlice.ts index e5d65d02c..6db895a37 100644 --- a/webapp/src/dogma/features/api/apiSlice.ts +++ b/webapp/src/dogma/features/api/apiSlice.ts @@ -367,6 +367,14 @@ export const apiSlice = createApi({ }), invalidatesTags: ['AppIdentity'], }), + regenerateAppIdentitySecret: builder.mutation({ + query: ({ appId }) => ({ + url: `/api/v1/appIdentities/${appId}/secret`, + method: 'POST', + }), + // Refetching remounts the table cell that shows the new secret in a modal, so the caller + // invalidates the 'AppIdentity' tag when the modal is closed instead. + }), getProjectMirrors: builder.query({ query: (projectName) => `/api/v1/projects/${projectName}/mirrors`, providesTags: ['Metadata'], @@ -659,6 +667,7 @@ export const { useDeactivateAppIdentityMutation, useActivateAppIdentityMutation, useDeleteAppIdentityMutation, + useRegenerateAppIdentitySecretMutation, // File useGetFilesQuery, useGetFileContentQuery, diff --git a/webapp/src/dogma/features/app-identity/DisplaySecretModal.tsx b/webapp/src/dogma/features/app-identity/DisplaySecretModal.tsx index 81f93a0c6..a0b58727f 100644 --- a/webapp/src/dogma/features/app-identity/DisplaySecretModal.tsx +++ b/webapp/src/dogma/features/app-identity/DisplaySecretModal.tsx @@ -1,4 +1,6 @@ import { + Alert, + AlertIcon, Button, HStack, IconButton, @@ -26,10 +28,12 @@ export const DisplaySecretModal = ({ isOpen, onClose, response, + title = 'Application identity generated', }: { isOpen: boolean; onClose: () => void; response: AppIdentityDto; + title?: string; }) => { const dispatch = useAppDispatch(); if (!response) return; @@ -38,9 +42,16 @@ export const DisplaySecretModal = ({ - Application identity generated + {title} + {response.deactivation && ( + + + This app identity is inactive. The new secret will not work until the app identity is activated, + so distribute it to the clients before activating. + + )} diff --git a/webapp/src/dogma/features/app-identity/RegenerateAppIdentitySecret.tsx b/webapp/src/dogma/features/app-identity/RegenerateAppIdentitySecret.tsx new file mode 100644 index 000000000..9345761d9 --- /dev/null +++ b/webapp/src/dogma/features/app-identity/RegenerateAppIdentitySecret.tsx @@ -0,0 +1,134 @@ +import { + Button, + HStack, + Modal, + ModalBody, + ModalCloseButton, + ModalContent, + ModalFooter, + ModalHeader, + ModalOverlay, + Text, + useDisclosure, +} from '@chakra-ui/react'; +import { apiSlice, useRegenerateAppIdentitySecretMutation } from 'dogma/features/api/apiSlice'; +import { AppIdentityDto } from 'dogma/features/app-identity/AppIdentity'; +import { DisplaySecretModal } from 'dogma/features/app-identity/DisplaySecretModal'; +import { newNotification } from 'dogma/features/notification/notificationSlice'; +import ErrorMessageParser from 'dogma/features/services/ErrorMessageParser'; +import { useAppDispatch } from 'dogma/hooks'; +import { useEffect, useRef, useState } from 'react'; +import { MdRefresh } from 'react-icons/md'; + +export const RegenerateAppIdentitySecret = ({ + appId, + hidden, + onRegenerated, +}: { + appId: string; + hidden: boolean; + onRegenerated?: () => void; +}) => { + const { isOpen, onToggle, onClose } = useDisclosure(); + const { + isOpen: isSecretModalOpen, + onToggle: onSecretModalToggle, + onClose: onSecretModalClose, + } = useDisclosure(); + const dispatch = useAppDispatch(); + const [regenerateSecret, { isLoading, reset }] = useRegenerateAppIdentitySecretMutation(); + const [appIdentityDetail, setAppIdentityDetail] = useState(null); + const mounted = useRef(true); + const invalidationPending = useRef(false); + useEffect(() => { + mounted.current = true; + // Refresh the list even if this component is unmounted before the secret modal is closed. + return () => { + mounted.current = false; + if (invalidationPending.current) { + invalidationPending.current = false; + dispatch(apiSlice.util.invalidateTags(['AppIdentity'])); + } + }; + }, [dispatch]); + const handleRegenerate = async () => { + try { + const response = await regenerateSecret({ appId }).unwrap(); + if (!mounted.current) { + // Unmounted while the request was in flight; refresh the list right away. + dispatch(apiSlice.util.invalidateTags(['AppIdentity'])); + return; + } + invalidationPending.current = true; + setAppIdentityDetail(response); + onClose(); + onSecretModalToggle(); + } catch (error) { + dispatch( + newNotification( + `Failed to regenerate the secret of ${appId}`, + ErrorMessageParser.parse(error), + 'error', + ), + ); + } + }; + const handleSecretModalClose = () => { + onSecretModalClose(); + setAppIdentityDetail(null); + reset(); + onRegenerated?.(); + // Refetch after the secret modal is closed; refetching earlier remounts this table cell + // and closes the modal before the user sees the new secret. + invalidationPending.current = false; + dispatch(apiSlice.util.invalidateTags(['AppIdentity'])); + }; + return ( + <> + + + + + Are you sure? + + + + Regenerate the secret of application identity {`${appId}`}? The app identity stays inactive and + the new secret will not work until the app identity is activated. + + + + + + + + + + + + + ); +}; diff --git a/webapp/src/pages/app/settings/app-identities/index.tsx b/webapp/src/pages/app/settings/app-identities/index.tsx index f6667b653..516c15258 100644 --- a/webapp/src/pages/app/settings/app-identities/index.tsx +++ b/webapp/src/pages/app/settings/app-identities/index.tsx @@ -8,16 +8,25 @@ import { UserRole } from 'dogma/common/components/UserRole'; import { DataTableClientPagination } from 'dogma/common/components/table/DataTableClientPagination'; import { useGetAppIdentitiesQuery } from 'dogma/features/api/apiSlice'; import { AppIdentityDto, isToken, isCertificate } from 'dogma/features/app-identity/AppIdentity'; -import { useMemo } from 'react'; +import { useCallback, useMemo, useState } from 'react'; import { DeactivateAppIdentity } from 'dogma/features/app-identity/DeactivateAppIdentity'; import { ActivateAppIdentity } from 'dogma/features/app-identity/ActivateAppIdentity'; import { DeleteAppIdentity } from 'dogma/features/app-identity/DeleteAppIdentity'; +import { RegenerateAppIdentitySecret } from 'dogma/features/app-identity/RegenerateAppIdentitySecret'; import { Deferred } from 'dogma/common/components/Deferred'; import SettingView from 'dogma/features/settings/SettingView'; import { useAppSelector } from 'dogma/hooks'; const AppIdentityPage = () => { const systemAdmin = useAppSelector((state) => state.auth.user?.systemAdmin ?? false); + // Tokens whose secret was regenerated during the current deactivation, keyed by + // `appId@deactivationTimestamp`. Hides the regenerate action until the token is + // deactivated again so that another regeneration does not revoke a secret being + // distributed to the clients. + const [regeneratedKeys, setRegeneratedKeys] = useState>(new Set()); + const markRegenerated = useCallback((key: string) => { + setRegeneratedKeys((prev) => new Set(prev).add(key)); + }, []); const columnHelper = createColumnHelper(); const columns = useMemo( () => [ @@ -74,18 +83,31 @@ const AppIdentityPage = () => { header: 'Status', }), columnHelper.accessor((row: AppIdentityDto) => row.deactivation, { - cell: (info) => ( - - - ), + cell: (info) => { + const regeneratedKey = `${info.row.original.appId}@${info.getValue()?.timestamp}`; + return ( + + + ); + }, header: 'Actions', enableSorting: false, }), ], - [columnHelper, systemAdmin], + [columnHelper, systemAdmin, regeneratedKeys, markRegenerated], ); const { data, error, isLoading } = useGetAppIdentitiesQuery(); return ( diff --git a/webapp/tests/dogma/features/app-identity/RegenerateAppIdentitySecret.test.tsx b/webapp/tests/dogma/features/app-identity/RegenerateAppIdentitySecret.test.tsx new file mode 100644 index 000000000..5defe60d5 --- /dev/null +++ b/webapp/tests/dogma/features/app-identity/RegenerateAppIdentitySecret.test.tsx @@ -0,0 +1,151 @@ +import '@testing-library/jest-dom'; +import { screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { ReactElement } from 'react'; +import { renderWithProviders } from 'dogma/util/test-utils'; +import { RegenerateAppIdentitySecret } from 'dogma/features/app-identity/RegenerateAppIdentitySecret'; +import { apiSlice, useRegenerateAppIdentitySecretMutation } from 'dogma/features/api/apiSlice'; +import { setupStore } from 'dogma/store'; + +jest.mock('dogma/features/api/apiSlice', () => ({ + ...jest.requireActual('dogma/features/api/apiSlice'), + useRegenerateAppIdentitySecretMutation: jest.fn(), +})); + +// A regenerated token is always deactivated; the server rejects regenerating an active token. +const regeneratedToken = { + appId: 'app-token-1', + type: 'TOKEN', + systemAdmin: false, + allowGuestAccess: false, + secret: 'appToken-regenerated-secret', + creation: { user: 'user@example.com', timestamp: '2024-01-01T00:00:00Z' }, + deactivation: { user: 'user@example.com', timestamp: '2024-01-02T00:00:00Z' }, +}; + +// Renders with a store whose dispatched actions are recorded, so the tests can assert +// when the AppIdentity cache invalidation is dispatched. +function renderWithDispatchSpy(ui: ReactElement) { + const store = setupStore({}); + const dispatched: unknown[] = []; + const originalDispatch = store.dispatch; + store.dispatch = ((action: never) => { + dispatched.push(action); + return originalDispatch(action); + }) as typeof store.dispatch; + const hasInvalidation = () => dispatched.some((action) => apiSlice.util.invalidateTags.match(action)); + return { hasInvalidation, ...renderWithProviders(ui, { store }) }; +} + +describe('RegenerateAppIdentitySecret', () => { + let regenerateSecret: jest.Mock; + let reset: jest.Mock; + + beforeEach(() => { + regenerateSecret = jest.fn(); + reset = jest.fn(); + (useRegenerateAppIdentitySecretMutation as jest.Mock).mockReturnValue([ + regenerateSecret, + { isLoading: false, reset }, + ]); + }); + + it('asks for confirmation before regenerating', async () => { + regenerateSecret.mockReturnValue({ unwrap: () => Promise.resolve(regeneratedToken) }); + renderWithProviders(