Skip to content

Commit 8cd8286

Browse files
ikhoonikhoon
committed
Allow regenerating the secret of an application token
Motivation: When the secret of an application token is leaked, the only remedy has been to deactivate or delete the token and create a new one. This loses the roles and permissions granted to the application ID and forces every project to register the new token again. There should be a way to revoke the leaked secret and issue a new one in place. Modifications: - Add `POST /api/v1/appIdentities/{appId}/secret` which revokes the current secret and returns the token with a newly-generated secret in a single commit. Only the token creator or a system administrator is allowed to call it, the same as deletion. - Preserve the deactivation state when regenerating; the new secret of a deactivated token does not authenticate until the token is activated. - Add a 'Regenerate secret' action with a confirmation dialog to the application identities settings page. The new secret is displayed once, with a warning if the token is inactive. - Stop including the whole file content, which may contain secrets, in the exception message raised when a content transformer fails. Result: - Users can rotate a leaked token secret in place; the old secret stops working immediately and the application ID keeps its roles and permissions.
1 parent 5bc736b commit 8cd8286

13 files changed

Lines changed: 700 additions & 4 deletions

File tree

‎server/src/main/java/com/linecorp/centraldogma/server/internal/api/sysadmin/AppIdentityRegistryService.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,28 @@ public CompletableFuture<AppIdentity> updateAppIdentity(ServiceRequestContext ct
248248
});
249249
}
250250

251+
/**
252+
* POST /appIdentities/{appId}/secret
253+
*
254+
* <p>Regenerates the secret of the token of the specified {@code appId}. The old secret is revoked
255+
* immediately and the token with a newly-generated secret is returned.
256+
*/
257+
@Post("/appIdentities/{appId}/secret")
258+
public CompletableFuture<Token> regenerateTokenSecret(ServiceRequestContext ctx,
259+
@Param String appId,
260+
Author author, User loginUser) {
261+
return getTokenOrRespondForbidden(ctx, appId, loginUser).thenCompose(
262+
token -> {
263+
if (token.isDeleted()) {
264+
throw new IllegalArgumentException(
265+
"You can't regenerate the secret of the token scheduled for deletion.");
266+
}
267+
// Pass the creation metadata of the authorized token so that a token recreated
268+
// with the same application ID in the meantime is not rotated.
269+
return mds.regenerateTokenSecret(author, appId, token.creation());
270+
});
271+
}
272+
251273
/**
252274
* PATCH /appIdentities/{appId}/level
253275
*

‎server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/TransformingChangesApplier.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,8 @@ int doApply(Revision headRevision, DirCache dirCache,
7676
} catch (CentralDogmaException e) {
7777
throw e;
7878
} catch (Exception e) {
79-
throw new ChangeConflictException("failed to transform the content: " + oldJsonNode +
80-
" transformer: " + transformer, e);
79+
// Do not include the old content in the message; it may contain sensitive data such as secrets.
80+
throw new ChangeConflictException("failed to transform the content with " + transformer, e);
8181
}
8282
return 0;
8383
}

‎server/src/main/java/com/linecorp/centraldogma/server/metadata/AppIdentityService.java‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@
3232
import java.util.Map;
3333
import java.util.UUID;
3434
import java.util.concurrent.CompletableFuture;
35+
import java.util.concurrent.atomic.AtomicReference;
36+
37+
import org.jspecify.annotations.Nullable;
3538

3639
import com.fasterxml.jackson.core.JsonPointer;
3740
import com.fasterxml.jackson.databind.JsonNode;
@@ -168,6 +171,64 @@ Revision purgeAppIdentity(Author author, String appId) {
168171
.join();
169172
}
170173

174+
CompletableFuture<Token> regenerateTokenSecret(Author author, String appId) {
175+
return regenerateTokenSecret(author, appId, null);
176+
}
177+
178+
CompletableFuture<Token> regenerateTokenSecret(Author author, String appId,
179+
@Nullable UserAndTimestamp expectedCreation) {
180+
requireNonNull(author, "author");
181+
requireNonNull(appId, "appId");
182+
183+
final String commitSummary = "Regenerate the secret of the token: " + appId;
184+
185+
// Capture the regenerated token so that the caller gets the secret this commit produced
186+
// even if another commit lands right after this one.
187+
final AtomicReference<Token> newTokenRef = new AtomicReference<>();
188+
final AppIdentityRegistryTransformer transformer = new AppIdentityRegistryTransformer(
189+
(headRevision, registry) -> {
190+
final AppIdentity appIdentity = registry.get(appId); // Raise an exception if not found.
191+
if (appIdentity.deletion() != null) {
192+
// Note that a ChangeConflictException is raised instead of an
193+
// IllegalArgumentException so that the storage layer does not wrap it with
194+
// another exception.
195+
throw new ChangeConflictException(
196+
"The app identity is already destroyed: " + appId);
197+
}
198+
throwIfInvalidType(appId, appIdentity, AppIdentityType.TOKEN);
199+
if (expectedCreation != null && !expectedCreation.equals(appIdentity.creation())) {
200+
// The token the caller was authorized for has been recreated in the meantime.
201+
throw new ChangeConflictException(
202+
"The app identity has been recreated concurrently: " + appId);
203+
}
204+
205+
final Token token = (Token) appIdentity;
206+
final String oldSecret = token.secret();
207+
assert oldSecret != null;
208+
final String newSecret = SECRET_PREFIX + UUID.randomUUID();
209+
if (registry.secrets().containsKey(newSecret)) {
210+
throw new ChangeConflictException("Secret already exists");
211+
}
212+
213+
final Token newToken = new Token(token.appId(), newSecret, token.isSystemAdmin(),
214+
token.allowGuestAccess(), token.creation(),
215+
token.deactivation(), null);
216+
newTokenRef.set(newToken);
217+
final Map<String, AppIdentity> newAppIds =
218+
updateMap(registry.appIds(), appId, newToken);
219+
final Map<String, String> secretsWithoutOld =
220+
removeFromMap(registry.secrets(), oldSecret);
221+
// A deactivated token has no entry in the secret map.
222+
final Map<String, String> newSecrets =
223+
token.isActive() ? addToMap(secretsWithoutOld, newSecret, appId)
224+
: secretsWithoutOld;
225+
return new AppIdentityRegistry(newAppIds, newSecrets, registry.certificateIds());
226+
});
227+
return appIdentityRegistryRepo.push(INTERNAL_PROJECT_DOGMA, Project.REPO_DOGMA, author,
228+
commitSummary, transformer)
229+
.thenApply(unused -> newTokenRef.get());
230+
}
231+
171232
CompletableFuture<Revision> activateToken(Author author, String appId) {
172233
requireNonNull(author, "author");
173234
requireNonNull(appId, "appId");

‎server/src/main/java/com/linecorp/centraldogma/server/metadata/MetadataService.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1165,6 +1165,27 @@ public CompletableFuture<Revision> deactivateToken(Author author, String appId)
11651165
return appIdentityService.deactivateToken(author, appId);
11661166
}
11671167

1168+
/**
1169+
* Regenerates the secret of the {@link Token} of the specified {@code appId} and returns the
1170+
* {@link Token} with the newly-generated secret. The old secret is revoked immediately.
1171+
*/
1172+
public CompletableFuture<Token> regenerateTokenSecret(Author author, String appId) {
1173+
return appIdentityService.regenerateTokenSecret(author, appId);
1174+
}
1175+
1176+
/**
1177+
* Regenerates the secret of the {@link Token} of the specified {@code appId} and returns the
1178+
* {@link Token} with the newly-generated secret. The old secret is revoked immediately.
1179+
* The regeneration fails with a {@link ChangeConflictException} if the token's creation metadata
1180+
* does not match {@code expectedCreation}, which prevents rotating a token that was recreated
1181+
* with the same application ID after the caller was authorized.
1182+
*/
1183+
public CompletableFuture<Token> regenerateTokenSecret(Author author, String appId,
1184+
UserAndTimestamp expectedCreation) {
1185+
requireNonNull(expectedCreation, "expectedCreation");
1186+
return appIdentityService.regenerateTokenSecret(author, appId, expectedCreation);
1187+
}
1188+
11681189
/**
11691190
* Returns an {@link AppIdentity} which has the specified {@code appId}.
11701191
*/

‎server/src/test/java/com/linecorp/centraldogma/server/internal/api/AppIdentityRegistryServiceViaHttpTest.java‎

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import static com.linecorp.centraldogma.internal.api.v1.HttpApiV1Constants.API_V1_PATH_PREFIX;
1919
import static com.linecorp.centraldogma.testing.internal.auth.TestAuthMessageUtil.getAccessToken;
2020
import static org.assertj.core.api.Assertions.assertThat;
21+
import static org.awaitility.Awaitility.await;
2122

2223
import java.net.URI;
2324

@@ -71,6 +72,166 @@ static void setUp() throws JsonMappingException, JsonParseException {
7172
.build();
7273
}
7374

75+
@Test
76+
void regenerateTokenSecret() throws JsonProcessingException {
77+
final AggregatedHttpResponse createResponse =
78+
systemAdminClient.post(API_V1_PATH_PREFIX + "appIdentities",
79+
QueryParams.of("appId", "forRegenerate", "type", "TOKEN",
80+
"isSystemAdmin", false),
81+
HttpData.empty())
82+
.aggregate()
83+
.join();
84+
assertThat(createResponse.status()).isEqualTo(HttpStatus.CREATED);
85+
final String oldSecret = Jackson.readTree(createResponse.contentUtf8()).get("secret").asText();
86+
87+
// The old secret authenticates requests.
88+
assertThat(newTokenClient(oldSecret).get(API_V1_PATH_PREFIX + "appIdentities").aggregate().join()
89+
.status()).isEqualTo(HttpStatus.OK);
90+
91+
final AggregatedHttpResponse regenerateResponse =
92+
systemAdminClient.post(API_V1_PATH_PREFIX + "appIdentities/forRegenerate/secret",
93+
HttpData.empty())
94+
.aggregate()
95+
.join();
96+
assertThat(regenerateResponse.status()).isEqualTo(HttpStatus.OK);
97+
final JsonNode regenerated = Jackson.readTree(regenerateResponse.contentUtf8());
98+
assertThat(regenerated.get("appId").asText()).isEqualTo("forRegenerate");
99+
final String newSecret = regenerated.get("secret").asText();
100+
assertThat(newSecret).startsWith("appToken-")
101+
.isNotEqualTo(oldSecret);
102+
103+
// The old secret is revoked and the new secret authenticates requests.
104+
// The registry used by the authorizer is updated asynchronously so await the changes.
105+
await().untilAsserted(() -> {
106+
assertThat(newTokenClient(oldSecret).get(API_V1_PATH_PREFIX + "appIdentities").aggregate().join()
107+
.status()).isEqualTo(HttpStatus.UNAUTHORIZED);
108+
assertThat(newTokenClient(newSecret).get(API_V1_PATH_PREFIX + "appIdentities").aggregate().join()
109+
.status()).isEqualTo(HttpStatus.OK);
110+
});
111+
}
112+
113+
@Test
114+
void ownerCanRegenerateOwnTokenSecret() throws JsonProcessingException {
115+
// A non-admin user creates its own token and regenerates its secret.
116+
final WebClient userClient =
117+
WebClient.builder(dogma.httpClient().uri())
118+
.auth(AuthToken.ofOAuth2(getAccessToken(dogma.httpClient(),
119+
TestAuthMessageUtil.USERNAME2,
120+
TestAuthMessageUtil.PASSWORD2,
121+
"ownerAppId",
122+
false)))
123+
.build();
124+
assertThat(userClient.post(API_V1_PATH_PREFIX + "appIdentities",
125+
QueryParams.of("appId", "ownedByUser2", "type", "TOKEN",
126+
"isSystemAdmin", false),
127+
HttpData.empty())
128+
.aggregate()
129+
.join()
130+
.status()).isEqualTo(HttpStatus.CREATED);
131+
132+
final AggregatedHttpResponse response =
133+
userClient.post(API_V1_PATH_PREFIX + "appIdentities/ownedByUser2/secret", HttpData.empty())
134+
.aggregate()
135+
.join();
136+
assertThat(response.status()).isEqualTo(HttpStatus.OK);
137+
}
138+
139+
@Test
140+
void regenerateSecretOfDeactivatedTokenViaHttp() throws JsonProcessingException {
141+
assertThat(systemAdminClient.post(API_V1_PATH_PREFIX + "appIdentities",
142+
QueryParams.of("appId", "forInactive", "type", "TOKEN",
143+
"isSystemAdmin", false),
144+
HttpData.empty())
145+
.aggregate()
146+
.join()
147+
.status()).isEqualTo(HttpStatus.CREATED);
148+
final RequestHeaders headers = RequestHeaders.of(HttpMethod.PATCH,
149+
API_V1_PATH_PREFIX + "appIdentities/forInactive",
150+
HttpHeaderNames.CONTENT_TYPE, MediaType.JSON);
151+
assertThat(systemAdminClient.execute(headers, "{\"status\":\"inactive\"}").aggregate().join()
152+
.status()).isEqualTo(HttpStatus.OK);
153+
154+
final AggregatedHttpResponse response =
155+
systemAdminClient.post(API_V1_PATH_PREFIX + "appIdentities/forInactive/secret",
156+
HttpData.empty())
157+
.aggregate()
158+
.join();
159+
assertThat(response.status()).isEqualTo(HttpStatus.OK);
160+
final JsonNode regenerated = Jackson.readTree(response.contentUtf8());
161+
final String newSecret = regenerated.get("secret").asText();
162+
assertThat(newSecret).startsWith("appToken-");
163+
// The token remains deactivated so the new secret does not authenticate until activation.
164+
assertThat(regenerated.get("deactivation")).isNotNull();
165+
await().untilAsserted(() -> {
166+
assertThat(newTokenClient(newSecret).get(API_V1_PATH_PREFIX + "appIdentities").aggregate().join()
167+
.status()).isEqualTo(HttpStatus.UNAUTHORIZED);
168+
});
169+
}
170+
171+
@Test
172+
void cannotRegenerateSecretOfMissingToken() {
173+
assertThat(systemAdminClient.post(API_V1_PATH_PREFIX + "appIdentities/nonexistent/secret",
174+
HttpData.empty())
175+
.aggregate()
176+
.join()
177+
.status()).isEqualTo(HttpStatus.NOT_FOUND);
178+
}
179+
180+
@Test
181+
void cannotRegenerateSecretWithoutPermission() {
182+
assertThat(systemAdminClient.post(API_V1_PATH_PREFIX + "appIdentities",
183+
QueryParams.of("appId", "ownedBySystemAdmin", "type", "TOKEN",
184+
"isSystemAdmin", false),
185+
HttpData.empty())
186+
.aggregate()
187+
.join()
188+
.status()).isEqualTo(HttpStatus.CREATED);
189+
190+
final WebClient userClient =
191+
WebClient.builder(dogma.httpClient().uri())
192+
.auth(AuthToken.ofOAuth2(getAccessToken(dogma.httpClient(),
193+
TestAuthMessageUtil.USERNAME2,
194+
TestAuthMessageUtil.PASSWORD2,
195+
"appIdOfUser2",
196+
false)))
197+
.build();
198+
assertThat(userClient.post(API_V1_PATH_PREFIX + "appIdentities/ownedBySystemAdmin/secret",
199+
HttpData.empty())
200+
.aggregate()
201+
.join()
202+
.status()).isEqualTo(HttpStatus.FORBIDDEN);
203+
}
204+
205+
@Test
206+
void cannotRegenerateSecretOfDestroyedToken() {
207+
assertThat(systemAdminClient.post(API_V1_PATH_PREFIX + "appIdentities",
208+
QueryParams.of("appId", "forDestroyed", "type", "TOKEN",
209+
"isSystemAdmin", false),
210+
HttpData.empty())
211+
.aggregate()
212+
.join()
213+
.status()).isEqualTo(HttpStatus.CREATED);
214+
// The DELETE method always responds with 204 No Content on success.
215+
assertThat(systemAdminClient.delete(API_V1_PATH_PREFIX + "appIdentities/forDestroyed")
216+
.aggregate()
217+
.join()
218+
.status()).isEqualTo(HttpStatus.NO_CONTENT);
219+
220+
final AggregatedHttpResponse response =
221+
systemAdminClient.post(API_V1_PATH_PREFIX + "appIdentities/forDestroyed/secret",
222+
HttpData.empty())
223+
.aggregate()
224+
.join();
225+
assertThat(response.status()).isEqualTo(HttpStatus.BAD_REQUEST);
226+
assertThat(response.contentUtf8()).contains("scheduled for deletion");
227+
}
228+
229+
private static WebClient newTokenClient(String secret) {
230+
return WebClient.builder(dogma.httpClient().uri())
231+
.auth(AuthToken.ofOAuth2(secret))
232+
.build();
233+
}
234+
74235
@Test
75236
void createTokenAndUpdateLevel() throws JsonProcessingException {
76237
assertThat(systemAdminClient.post(API_V1_PATH_PREFIX + "appIdentities",

0 commit comments

Comments
 (0)