Skip to content

Make "all teams" an explicit choice on PATs instead of the absence of team scopes #8418

Description

@cstns

Description

Personal access tokens are scoped to teams via the AccessTokenTeamScopes join table. A token with zero rows in that table means "all teams", so "no scope data" and "deliberately global" are the same state, and the check in patAllowsTeam (forge/routes/auth/permissions.js) fails open.

That has real consequences:

  • The join table has onDelete: CASCADE on TeamId. When a team is deleted, its scope rows silently vanish, so a token that was scoped exclusively to that team goes from "scoped to team X" to zero rows, which the auth layer reads as global. Nothing in the team afterDestroy hook cleans this up (the "user leaves team" path in forge/db/controllers/Team.js guards against it, but only by deleting the token, and only for that one path).
  • An unscoped token automatically gains access to any team the user joins or creates later. The token dialog warns about this, but it's still what you get by selecting nothing.
  • Any future code path that loses or forgets scope rows escalates silently instead of failing.

The MCP OAuth consent flow (see #8415, #8416, #8417) uses the same empty-means-all convention, so it inherits all of this.

Proposal

Make "all teams" an explicit, recorded choice instead of the absence of data. The rule becomes: a token is global only if it explicitly says so; otherwise it needs a matching scope row, and zero rows matches nothing (the token goes inert instead of global). With that polarity, forgetting to record the choice produces a dead token, not a global one, and the team-deletion cascade case fixes itself.

Two places the choice could be recorded:

  • Reuse the existing scope column with a scope value like teams:all. PATs currently store an empty string there, and the column already works this way for device tokens (device:provision checked via includes). No schema change. One thing to handle: the PAT creation API currently accepts scope from the client, so the backend would need to derive this value itself and ignore client input.
  • A new allTeams boolean on AccessTokens. More explicit and queryable, at the cost of a schema migration on a table shared by all token types.

Migration

Either way it's a single set-based update, no batching: stamp the all-teams marker onto existing user tokens that have no scope rows (ownerType = 'user' and name IS NOT NULL, which covers PATs and MCP Agent tokens). Tokens that already have scope rows are correct as-is. This grandfathers existing unscoped tokens as explicitly global, so nothing breaks on deploy; the tradeoff is existing global tokens stay global until recreated.

Epic/Story

No response

Does this need to be reflected in the FlowFuse Expert?

No Expert impact

Have you provided an initial effort estimate for this issue?

I have provided an initial effort estimate

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

taskA piece of work that isn't necessarily tied to a specific Epic or Story.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions