-
Notifications
You must be signed in to change notification settings - Fork 134
Allow regenerating the secret of an application token #1340
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
8cd8286
a476a76
803ad60
2b77c42
8c8f588
4546802
37438e7
32ed1ba
fbc64ac
72b3dc7
dae2ebe
a030c97
f28af3a
20f7050
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -168,6 +168,56 @@ Revision purgeAppIdentity(Author author, String appId) { | |||||
| .join(); | ||||||
| } | ||||||
|
|
||||||
| CompletableFuture<Token> 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<String, AppIdentity> 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()), | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't we remove the old token string and add new one to the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We only allow deactivated tokens to be regenerated. The newly generated token will be added to registry.secrets() when it is activated. centraldogma/server/src/main/java/com/linecorp/centraldogma/server/metadata/AppIdentityService.java Lines 183 to 184 in a5a7f83
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oops, my bad. I missed it. 😓 |
||||||
| 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<Revision> activateToken(Author author, String appId) { | ||||||
| requireNonNull(author, "author"); | ||||||
| requireNonNull(appId, "appId"); | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note) I understand that a user deactivates the token to call this API first. i.e. it is possible that an unlucky case happens where the token is purged before regenerate can be called.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A token has three states - Active -> Inactive(deactivated) -> Deleted
The purge scheduler only purges deleted tokens so deactivated tokens are not purged.
centraldogma/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/PurgeSchedulingService.java
Lines 151 to 155 in 384cce0