feat(repository): LookupAppToken() queries and repository function#6420
feat(repository): LookupAppToken() queries and repository function#6420dkanney wants to merge 19 commits intollb-app-tokenfrom
Conversation
dkanney
force-pushed
the
dkanney-get-token
branch
3 times, most recently
from
168a749 to
00e078d
Compare
Useful for determining which app token subtype table to operate on (global vs org vs project)
dkanney
force-pushed
the
dkanney-get-token
branch
from
00e078d to
6cf6372
Compare
There was a problem hiding this comment.
Pull request overview
Adds repository support for looking up full App Token details (token cipher + permissions) and updates token/grant retrieval logic and tests accordingly.
Changes:
- Introduces
LookupAppToken()plus supporting DB queries to fetch core token fields and permissions in one call (by token subtype). - Updates
CreateAppToken()to re-read tokens viaLookupAppToken()within the same transaction. - Expands tests and replaces string literals for grant scopes with constants.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/iam/role.go | Removes redundant return in a no-op method. |
| internal/apptoken/repository_token_grant.go | Uses token scope lookup to drive grant queries. |
| internal/apptoken/repository_test.go | Adds LookupAppToken tests and normalizes grant scope constants. |
| internal/apptoken/repository.go | Implements LookupAppToken, refactors token creation to use it, adds scope helpers. |
| internal/apptoken/query.go | Adds new SQL queries for global/org/project token lookup and scope-id lookup. |
| internal/apptoken/options.go | Adds option to reuse an in-flight transaction (reader/writer). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dkanney
force-pushed
the
dkanney-get-token
branch
from
6cf6372 to
3183e20
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Performs a fresh lookup to get all current values in the DB after insert
dkanney
force-pushed
the
dkanney-get-token
branch
from
3183e20 to
1b47049
Compare
| } | ||
|
|
||
| // getAppTokenScopeId returns the scope id of the app token | ||
| func getAppTokenScopeId(ctx context.Context, reader db.Reader, id string) (string, error) { |
There was a problem hiding this comment.
Can this be a method so that reader isn't being passed in? If not, can you add to the comment why it's necessary that it's not a method
There was a problem hiding this comment.
I think we need the ability to pass around a reader when calling this function inside a transaction
e.g. CreateAppToken() -> open a new Tx with its own dedicated reader -> call LookupAppToken() w/ the the Tx reader -> call getAppTokenScopeId() with the Tx reader
…istancy with input Project-scoped tokens can only apply to themselves, so the only active scope will be the project itself. No need to assign the project to its own "active_grant_scope_ids" array because we just need to check if "grant_this_scope" is true. This also matches the shape of input project tokens; those only need to specify "grant_this_scope". Also remove some extraneous grouping.
ICU-17931
Description
LookupAppToken()repository method utilizes these queries (based on token subtype) to fetch full app token information, including token cipher and permission dataPCI review checklist
Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.