Skip to content

fix(auth): API tokens from Settings authenticate, persist and stay in their tenant (#782) - #853

Merged
alex-dembele merged 5 commits into
masterfrom
782-fixauth-les-jetons-api-generes-depuis-reglages-nauthentifient-rien-et-ne-persistent-pas
Oct 2, 2026
Merged

alex-dembele merged 5 commits into
masterfrom
782-fixauth-les-jetons-api-generes-depuis-reglages-nauthentifient-rien-et-ne-persistent-pas

Conversation

@alex-dembele

@alex-dembele alex-dembele commented Oct 1, 2026 •

Copy link
Copy Markdown
Member

Closes #782

Settings created API tokens through TokenService, which kept them in a map that a restart emptied. The middleware that would have checked them was never mounted, and the screen copied [object Object] while announcing success. This PR keeps the other implementation, PersonalAccessTokenService: it was already stored in Postgres and mounted on every /api/v1 route. The legacy stack is deleted, and Settings now uses /auth/pat. No ADR is needed, because this is the path criterion 5 names.

Besides the removal, three behaviour changes:

  • Each token carries tenant_id. The middleware resolves the session for that tenant with the org resolver, not the owner's default org, so a token from org A never acts in org B and stops when its owner leaves A. List, revoke and the last-used stamp filter on tenant_id AND user_id. The hash lookup stays unscoped because it is the authentication step; the registry says so. Existing rows get users.default_org_id, which is the tenant they already resolved to. Rows whose owner has no default org are deleted: they never authenticated a request. This runs in PrepareForAutoMigrate and in migration 0067, which is guarded for a database where AutoMigrate has not built the table yet.
  • Token format. Tokens are orsk_<8-hex>_<64-hex>, and the older format still validates.
  • "*" scope. It now means "the owner's own permissions". Before, it intersected to nothing for any non-admin.

Migration number: 0066 is taken by #849, so this uses 0067.

Verification

Backend, with Postgres available (DATABASE_URL set to a throwaway local cluster):

$ go vet ./...                       # clean
$ go test ./...                      # 79 packages ok, 0 FAIL
$ go test ./internal/middleware/ -run 'PAT|LooksLike|PermsGrant' -v
--- PASS: TestPAT_AuthenticatesProtectedRoute
--- PASS: TestPAT_CrossTenant
--- PASS: TestLooksLikePAT
--- PASS: TestPermsGrant
$ go test ./internal/infrastructure/repository/ -run TestPATRepo -v
--- PASS: TestPATRepo_Postgres_SurvivesRestart
--- PASS: TestPATRepo_TenantIsolation
$ go test ./internal/infrastructure/database/ -run Backfill -v
--- PASS: TestBackfillPersonalAccessTokenTenant_Postgres

Sabotage checks:

  • Dropping tenant_id from the repository predicates fails TestPATRepo_TenantIsolation ("tenant A lists [.. ..], want only its own token").
  • Putting back the old String(res.data.token) handling fails the [object Object] regression test.

Frontend:

$ npx vitest run src/features/settings/__tests__/apiTokensPanel.test.tsx
 Tests  5 passed (5)
$ npx tsc -b --noEmit                 # what CI's type-check runs
4 errors, none in files this PR touches:
  ChangePasswordCard.tsx, OrganizationProfileForm.tsx  TS2305 useSuccessFeedback  -> #854
  MembersView.tsx (DeleteButton), CreateRiskModal.tsx (ScrollProgress) TS2304   -> #845

The first version of this PR body said tsc --noEmit -p . was clean. That command checks nothing in this repo, because tsconfig.json has "files": [] and only references the app config. The output above is the real check.

Live run: branch server on a fresh Postgres 18 cluster, migrations applied to version 67, not dirty.

created: CI/CD prefix 2667ab45 value starts orsk_2667ab45_… len 78
with token:    200        (GET /api/v1/risks)
without token: 401
tampered:      401
-- server killed and restarted --
after restart, with token:    200
after restart, without token: 401
list: [('CI/CD', '2667ab45', True)]   # last_used_at stamped

In the browser, Settings › Jetons API shows the reveal panel with an orsk_… value. An instrumented navigator.clipboard.writeText received string:orsk_5486e8f4_95… and resolved. A fetch('/api/v1/risks') with cookies omitted and the token taken from the panel returned 200, and 401 without it. Revoking from the row menu removed the row, showed "Jeton révoqué", and the same token then got 401. Screenshot: .playwright-mcp/782-token-reveal-light.png.

Honest remainders

  • The Settings page cannot render on master today. src/shared/ds/index.ts lost its useSuccessFeedback export in merge e074589, so the route throws before any tab paints. That is now fix(frontend): Réglages plante sur master, export useSuccessFeedback perdu au merge e074589e #854. I re-added the export locally for the browser pass only and reverted it; it is not in this PR. vitest run shows 19 failures on master, and the same 19 fail with this PR's changes stashed, so none are new. Correcting what I first wrote here: the missing export explains 9 of them (changePasswordCard 4, organizationProfileForm 4, deceptive-ui 1). The other 10 (members 9, CreateRiskModal 1) are fix(frontend): restore imports lost in merge 269e54fc (#840) #845's lost imports. With both fixes applied locally, all 40 tests in those files pass.
  • The cross-tenant case is now proven on a live two-organization boot as well; see the comment below.
  • In the live browser run I could not read the clipboard back, because that browser shares the desktop clipboard. The value written was checked by instrumenting writeText; the vitest suite asserts it under a controlled clipboard.
  • The dark-theme rendering of the reveal panel was not screenshotted. It uses the same tokens (--bg-elevated, --border-strong) as the input beside it.
  • docs/DOCUMENTATION_GUIDE.md still names the deleted files. It is a dated Phase 2 record whose paths were already wrong (internal/core/domain/...), so I left it alone.

…d in (#782)

A PAT had no tenant of its own. The middleware resolved the owner's default
organization on every request, so changing that default moved every token the
person held, and listing or revoking was filtered on user_id alone.

The token now stores tenant_id. The middleware resolves the session with the
org resolver for that tenant, which refuses an owner who is no longer an
active member there. List, revoke and the last-used stamp filter on tenant_id
and user_id; the hash lookup stays unscoped because it is the authentication
step and the row it returns names the tenant.

Existing rows take users.default_org_id, the tenant they already acted in.
Rows whose owner has no default organization never authenticated anything and
are deleted. This runs before AutoMigrate and again in migration 0067.

New tokens read orsk_<8-hex>_<secret> so a leaked one is recognisable;
the older <8-hex>_<secret> form still validates. A "*" scope now means the
owner's own permissions, where before it intersected to nothing for any
member who was not an administrator.

Signed-off-by: alex-dembele <alexandredembele16@gmail.com>
TokenService kept tokens in a map that a restart emptied, and the middleware
that checked them (tokenauth.go) was never mounted, so no request carrying one
was ever accepted. It is removed with its handler, the /tokens routes, the
APIToken model and their tests. Personal access tokens on /auth/pat are now the
only API token implementation.

The authz coverage list, the isolation registry and the schema test drop their
/tokens and APIToken entries with it.

Signed-off-by: alex-dembele <alexandredembele16@gmail.com>
#782)

The tab posted to /tokens, typed the answer as { token?: string } while the
server sent an object, and copied res.data.token. The clipboard got
"[object Object]" and the toast still said the token was created and copied.

The tab moves to its own component on /auth/pat. The secret is shown once in a
panel with a copy button and stays until the user dismisses it. The toast says
"copied" only when the clipboard write resolved, and an answer with no secret
string is reported as a failure. Revoke is a DELETE, removed from the table
optimistically and restored if the server refuses; the dead "revoked" facet
goes with it, since a revoked PAT no longer exists. The name is checked with
zod before the request.

Signed-off-by: alex-dembele <alexandredembele16@gmail.com>
Both API references described /tokens, an in-memory service whose
middleware was never mounted, plus rotate, update and per-token rate
limit calls that never worked. They now describe /auth/pat as it runs:
create, list and revoke, the orsk_ format, the one-time secret and the
tenant binding.

Signed-off-by: alex-dembele <alexandredembele16@gmail.com>
@alex-dembele

Copy link
Copy Markdown
Member Author

Live cross-tenant proof for criterion 4, run on this branch with a fresh Postgres 18 database (migrations at 67, not dirty).

Setup: the seeded admin is in org "OpenDefender" (A). Bob registered through POST /auth/register, which created "Org B". Each user created one risk with their session and minted one PAT. personal_access_tokens holds OpenDefender|PAT_A|admin@… and Org B|PAT_B|bob@….

1) baseline: two orgs, one token each
  PAT_A GET /risks -> 200 ['A-only risk']
  PAT_B GET /risks -> 200 ['B-only risk']
  PAT_A GET B's risk by id -> 404
  PAT_B GET A's risk by id -> 404
  PAT_A GET /auth/pat -> 200 ['PAT_A']

2) admin now also in Org B, and Org B is their DEFAULT org
  PAT_A GET /risks -> 200 ['A-only risk']
  PAT_B GET /risks -> 200 ['B-only risk']
  PAT_A GET B's risk by id -> 404
  PAT_B GET A's risk by id -> 404
  PAT_A GET /auth/pat -> 200 ['PAT_A']

3) admin's membership in Org A (the token's home) revoked; still an active admin of Org B
  PAT_A GET /risks -> 401 []
  PAT_B GET /risks -> 200 ['B-only risk']
  PAT_A GET B's risk by id -> 401
  PAT_B GET A's risk by id -> 404
  PAT_A GET /auth/pat -> 401 []

Step 2 is the case master gets wrong. Master resolved a PAT through the owner's default org, so PAT_A would have started reading Org B the moment B became the default. Step 3 shows the token does not fall back to another org its owner still belongs to.

For steps 2 and 3, the membership row and the default_org_id change were written directly in SQL on the throwaway database, as test setup, instead of going through the invitation flow.

…reglages-nauthentifient-rien-et-ne-persistent-pas
@alex-dembele
alex-dembele merged commit e5f4d89 into master Oct 2, 2026
13 of 25 checks passed
@alex-dembele
alex-dembele deleted the 782-fixauth-les-jetons-api-generes-depuis-reglages-nauthentifient-rien-et-ne-persistent-pas branch October 2, 2026 10:01
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.

fix(auth): les jetons API generes depuis Reglages nauthentifient rien et ne persistent pas

1 participant