Skip to content

Commit 3130e12

Browse files
committed
Validate the token state only in the transformer when regenerating a secret
Motivation: The regeneration endpoint passed a snapshot of the authorized token into the content transformer and compared it against the current state. The snapshot is taken outside the commit lock, so the comparisons were a non-atomic double check of what the transformer already validates atomically. Modifications: - Remove the expected-token parameter and its comparisons; the token state is validated only in the content transformer. Result: - Validation happens once at the atomic point. Concurrent regenerations follow last-writer-wins semantics.
1 parent 2b77c42 commit 3130e12

1 file changed

Lines changed: 22 additions & 20 deletions

File tree

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

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ Revision purgeAppIdentity(Author author, String appId) {
165165
final Map<String, AppIdentity> newAppIds = removeFromMap(registry.appIds(), appId);
166166
// The app identity is already removed from secrets and certificateIds when destroyed.
167167
return new AppIdentityRegistry(newAppIds, registry.secrets(), registry.certificateIds());
168-
});
168+
});
169169
return appIdentityRegistryRepo.push(INTERNAL_PROJECT_DOGMA, Project.REPO_DOGMA, author,
170170
commitSummary, transformer)
171171
.join();
@@ -235,12 +235,13 @@ CompletableFuture<Token> regenerateTokenSecret(Author author, String appId,
235235
});
236236
// Read the registry back at the revision this commit produced so that the caller gets
237237
// the secret of this commit even if another commit lands right after.
238-
return appIdentityRegistryRepo.push(INTERNAL_PROJECT_DOGMA, Project.REPO_DOGMA, author,
239-
commitSummary, transformer)
240-
.thenCompose(revision -> appIdentityRegistryRepo.fetch(
241-
INTERNAL_PROJECT_DOGMA, Project.REPO_DOGMA, TOKEN_JSON,
242-
revision))
243-
.thenApply(holder -> (Token) holder.object().get(appId));
238+
return appIdentityRegistryRepo
239+
.push(INTERNAL_PROJECT_DOGMA, Project.REPO_DOGMA, author, commitSummary, transformer)
240+
.thenCompose(revision -> {
241+
return appIdentityRegistryRepo.fetch(INTERNAL_PROJECT_DOGMA, Project.REPO_DOGMA,
242+
TOKEN_JSON, revision);
243+
})
244+
.thenApply(holder -> (Token) holder.object().get(appId));
244245
}
245246

246247
CompletableFuture<Revision> activateToken(Author author, String appId) {
@@ -275,18 +276,19 @@ CompletableFuture<Revision> deactivateToken(Author author, String appId) {
275276

276277
final AppIdentityRegistryTransformer transformer = new AppIdentityRegistryTransformer(
277278
(headRevision, registry) -> {
278-
final AppIdentity appIdentity =
279-
getAppIdentityToDeactivate(headRevision, registry, appId, AppIdentityType.TOKEN);
280-
final String secret = ((Token) appIdentity).secret();
281-
assert secret != null;
282-
final Token newToken = new Token(appIdentity.appId(), secret,
283-
appIdentity.isSystemAdmin(), appIdentity.allowGuestAccess(),
284-
appIdentity.creation(), userAndTimestamp, null);
285-
final Map<String, AppIdentity> newAppIds = updateMap(registry.appIds(), appId, newToken);
286-
final Map<String, String> newSecrets =
287-
removeFromMap(registry.secrets(), secret); // Note that the key is secret not appId.
288-
return new AppIdentityRegistry(newAppIds, newSecrets, registry.certificateIds());
289-
});
279+
final AppIdentity appIdentity =
280+
getAppIdentityToDeactivate(headRevision, registry, appId, AppIdentityType.TOKEN);
281+
final String secret = ((Token) appIdentity).secret();
282+
assert secret != null;
283+
final Token newToken = new Token(appIdentity.appId(), secret,
284+
appIdentity.isSystemAdmin(),
285+
appIdentity.allowGuestAccess(),
286+
appIdentity.creation(), userAndTimestamp, null);
287+
final Map<String, AppIdentity> newAppIds = updateMap(registry.appIds(), appId, newToken);
288+
final Map<String, String> newSecrets =
289+
removeFromMap(registry.secrets(), secret); // Note that the key is secret not appId.
290+
return new AppIdentityRegistry(newAppIds, newSecrets, registry.certificateIds());
291+
});
290292
return appIdentityRegistryRepo.push(INTERNAL_PROJECT_DOGMA, Project.REPO_DOGMA, author,
291293
commitSummary, transformer);
292294
}
@@ -377,7 +379,7 @@ CompletableFuture<Revision> createCertificate(Author author, String appId, Strin
377379
Jackson.valueToTree(
378380
certificate.appId()))));
379381
return appIdentityRegistryRepo.push(INTERNAL_PROJECT_DOGMA, Project.REPO_DOGMA, author,
380-
"Add a certificate: " + certificate.id(), change);
382+
"Add a certificate: " + certificate.id(), change);
381383
}
382384

383385
CompletableFuture<Revision> destroyCertificate(Author author, String appId) {

0 commit comments

Comments
 (0)