FlowFuse / FlowFuse/flowfuse

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

Open
#8,418 0 comments 0 reactions 1 assignee Claimed by @cstns View on GitHub
task
Dominant language
JavaScript
Stars
400
Forks
89
Avg merge
1d 20h
Merged PRs (30d)
149

Description

### 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

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.