feat(repository): Implement Delete app token functionality #6341
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mikemountain
force-pushed
the
mikemountain-apptoken-repo-delete
branch
from
49d1825 to
1eb7f9e
Compare
AprilMay0
reviewed
Jan 30, 2026
Collaborator
AprilMay0
left a comment
AprilMay0
left a comment
There was a problem hiding this comment.
I'm wondering if we should delete directly from the app_token table rather than the subtype tables. The app_token table cascades down to the subtype tables, so deleting from app_token will automatically cascade to app_token_org/app_token_global/app_token_project and trigger the insert_deleted_id trigger to record the deletion in app_token_deleted.
bosorawis
reviewed
Feb 2, 2026
internal/apptoken/repository.go
Outdated
Comment on lines
591
to
611
| scopeType, err := getAppTokenScopeType(ctx, r.reader, publicId) | ||
| if err != nil { | ||
| return db.NoRowsAffected, errors.Wrap(ctx, err, op, errors.WithMsg("cannot get scope for app token %s", publicId)) | ||
| } | ||
|
|
||
| var tokenToDelete any | ||
| switch scopeType { | ||
| case scope.Global: | ||
| gToken := allocGlobalAppToken() | ||
| gToken.PublicId = publicId | ||
| tokenToDelete = &gToken | ||
| case scope.Org: | ||
| oToken := allocOrgAppToken() | ||
| oToken.PublicId = publicId | ||
| tokenToDelete = &oToken | ||
| case scope.Project: | ||
| pToken := allocProjectAppToken() | ||
| pToken.PublicId = publicId | ||
| tokenToDelete = &pToken | ||
| default: | ||
| return db.NoRowsAffected, errors.New(ctx, errors.Unknown, op, fmt.Sprintf("unknown scope type for app token: %s", publicId)) |
Collaborator
There was a problem hiding this comment.
You should be able to just delete from app_token table and rely on on delete cascade trigger to delete the rest
bosorawis
reviewed
Feb 2, 2026
internal/apptoken/repository_test.go
Outdated
| assert.True(errors.IsNotFoundError(err)) | ||
| }) | ||
|
|
||
| t.Run("invalid-id", func(t *testing.T) { |
Collaborator
There was a problem hiding this comment.
Add a test that deletes a non-existent token.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR adds the functionality to delete an apptoken.
PCI review checklist
Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.