Skip to content

chore: super admin personal access tokens#2686

Draft
pmig wants to merge 2 commits into
mainfrom
pmig/dev-710-distr-super-admin-pats
Draft

chore: super admin personal access tokens#2686
pmig wants to merge 2 commits into
mainfrom
pmig/dev-710-distr-super-admin-pats

Conversation

@pmig

@pmig pmig commented Jun 24, 2026

Copy link
Copy Markdown
Member

No description provided.

Signed-off-by: Philip Miglinci <pmig@glasskube.com>
@pmig pmig requested review from Copilot and kosmoz June 24, 2026 14:15
@cursor

cursor Bot commented Jun 24, 2026

Copy link
Copy Markdown

PR Summary

High Risk
Touches authentication, PAT role enforcement, and super-admin vs PAT privilege separation; incorrect handling could elevate PAT scope or allow session escalation.

Overview
Super admins can manage personal access tokens scoped to the current organization, but those tokens are always read-only—on create (API override), in EffectiveUserRole, and in the UI inherit label.

Authentication changes load PATs without requiring org membership (LEFT JOIN on Organization_UserAccount). A super-admin-owned PAT carries an explicit read-only role and is treated as a normal read-only credential (not super-admin) in DbAuthenticator. RejectAccessToken blocks PATs on session/account /auth/* routes (e.g. switch-context) so a PAT cannot be exchanged for a more privileged JWT.

The nav Personal Access Tokens link is shown for super admins; token create failures surface via getFormDisplayedError toasts.

Reviewed by Cursor Bugbot for commit c03e9ba. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 6bb31eb. Configure here.

Comment thread internal/authn/authinfo/token.go

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Enables super admins to create/manage personal access tokens (PATs) while ensuring any PAT used by a super admin is effectively read-only across requests, and updates UI navigation to expose PAT management to super admins.

Changes:

  • Clamp super-admin token effective role to read_only and add coverage in EffectiveUserRole() tests.
  • Allow super admins to create/delete their own PATs (removing BlockSuperAdmin from those routes) with additional membership enforcement during creation.
  • Propagate isSuperAdmin into auth info from PATs and update UI to show PAT nav + improved create error toast handling.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
internal/types/access_token.go Makes super-admin PATs always act as read-only via EffectiveUserRole().
internal/types/access_token_test.go Adds a super-admin case to role-capping tests.
internal/handlers/settings.go Removes BlockSuperAdmin for PAT create/delete endpoints and documents the exception.
internal/handlers/settings_access_token.go Enforces org membership for super-admin PAT creation and forces read-only role.
internal/authn/authinfo/token.go Ensures PAT-derived auth info carries isSuperAdmin and documents expected behavior.
frontend/ui/src/app/components/nav-bar/nav-bar.component.html Shows “Personal Access Tokens” menu item for super admins.
frontend/ui/src/app/access-tokens/access-tokens.component.ts Treats super admins as read-only in role selection and shows backend error messages on create failures.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

http.Error(w, err.Error(), http.StatusInternalServerError)
return
}
request.UserRole = new(types.UserRoleReadOnly)
Comment thread internal/types/access_token_test.go Outdated
Comment on lines +39 to +43
superAdmin := AccessTokenWithUserAccount{
AccessToken: AccessToken{UserRole: new(UserRoleAdmin)},
UserAccount: UserAccount{IsSuperAdmin: true},
}
g.Expect(superAdmin.EffectiveUserRole()).To(Equal(UserRoleReadOnly))
Comment thread internal/authn/authinfo/token.go Outdated
Comment on lines +21 to +26
// A token issued by a super admin carries the super admin identity, so it
// has the same read access as an interactive super admin session (it can
// read all organizations). Write access is still blocked for super admins
// (BlockSuperAdmin), and EffectiveUserRole caps the role to read-only.
// Such tokens can only be created in an organization the super admin is a
// member of (enforced at creation).
Comment on lines +61 to +62
// Super admins are not organization members and may only create read-only tokens.
this.auth.isSuperAdmin() ? 'read_only' : this.auth.getClaims()?.role
Comment on lines +47 to +48
// Super admins may only create tokens in an organization they are a
// member of, and those tokens are always read-only.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly, this is in conflict with the premise that super-admins are not supposed to be a member of any organizations

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But they are created in an organization

Signed-off-by: Philip Miglinci <pmig@glasskube.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.

Comment on lines +42 to +44
// When the user has no membership in the token's organization (e.g. a
// super-admin-owned token), there is no role to cap against and the token's own
// role is authoritative.
Comment on lines +44 to +48
if auth.IsSuperAdmin() {
// Super admins are not organization members, so they may only ever
// create read-only tokens regardless of what was requested.
request.UserRole = new(types.UserRoleReadOnly)
} else if request.UserRole != nil {
Comment on lines 12 to 16
mk := func(tokenRole *UserRole, orgRole UserRole) AccessTokenWithUserAccount {
return AccessTokenWithUserAccount{
AccessToken: AccessToken{UserRole: tokenRole},
UserRole: orgRole,
UserRole: &orgRole,
}
Comment on lines +38 to +41
g.Expect(AccessTokenWithUserAccount{
AccessToken: AccessToken{UserRole: new(UserRoleAdmin)},
UserRole: nil,
}.EffectiveUserRole()).To(Equal(UserRoleReadOnly))
Comment on lines +45 to +48
g.Expect(AccessTokenWithUserAccount{
AccessToken: AccessToken{UserRole: new(UserRoleAdmin)},
UserAccount: UserAccount{IsSuperAdmin: true},
}.EffectiveUserRole()).To(Equal(UserRoleReadOnly))
@pmig pmig closed this Jun 25, 2026
// and a capped role, so it must not be exchangeable for a different or more privileged session
// (e.g. a super admin's read-only PAT minting a super-admin JWT via switch-context). It must run
// after the authentication middleware.
func RejectAccessToken(handler http.Handler) http.Handler {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar middlewares are called Block... I think this should be called BlockAccessToken

emailVerified: a.CurrentUserEmailVerified(),
tokenScope: a.TokenScope(),
userRole: nil, // Interactive super admins don't have a role
isSuperAdmin: true,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting this to true by default seems really scary. since we already have the user account, I think we could set it to user.isSuperAdmin right away

@pmig pmig reopened this Jun 30, 2026
@pmig pmig marked this pull request as draft July 3, 2026 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants