Skip to content

feat(auth): stamp session roles and declare what routes require - #51

Open
juicycleff wants to merge 1 commit into
mainfrom
feat/session-roles-declared-auth
Open

feat(auth): stamp session roles and declare what routes require#51
juicycleff wants to merge 1 commit into
mainfrom
feat/session-roles-declared-auth

Conversation

@juicycleff

@juicycleff juicycleff commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Forge can carry a route's authorization requirement into the OpenAPI document and out into a generated client, so a UI can ask canCall() before it renders a button. Authsome held every fact that needs, and kept all of them somewhere the document could not reach. Roles lived in warden. The auth requirement lived in a description string. The session cookie lived in a fallback branch of Authenticate. Which routes needed auth lived in a hand-written manifest.

This wires all of it up.

Blocked by xraph/forge#58.

Read this first

The branch does not build yet, and CI will say so. It needs AuthContext.Roles, which is on forge's feat/client-generator-auth (xraph/forge#58) and not in any release. go.mod is deliberately unchanged: locally you uncomment the replace and point it at a forge checkout, but committing that would break everyone else. This merges after forge ships.

Roles on the session

Roles are resolved once, when a session is issued, and stored on it. Stamped means stale, and that is the trade: a role granted or revoked after sign-in does not reach an existing session, in exchange for keeping authentication free of an RBAC lookup on every request. Revoking a role means revoking the session. Anything that must be current at the instant it is checked belongs in a permission check against warden, not here.

Sessions are created from six places: Engine.IssueSession, two paths in service.go, the oauth2provider and magiclink plugins, and the synthetic sessions the apikey plugin builds. All six end at store.CreateSession, so the stamp lives in a decorator around the store rather than at each issue site, where the seventh path would forget it. The cost is that a persistence call now makes an RBAC lookup, which is the point: the alternative is one on every authenticated request.

Rotation re-stamps. Without that, a role grant waits for the user to sign out and back in, which on a long-lived session is days. Re-stamping on refresh bounds it to the refresh interval, and a refresh is rare enough that the lookup does not matter.

The two failure policies differ on purpose

A failed lookup at issue time fails the sign-in. The tempting alternative is logging and continuing with no roles, since an empty role set denies rather than allows and so fails closed. What rules it out is that the stamp is persisted: a momentary failure would hand someone a session that authenticates fine and can reach no route declaring a role, for its whole lifetime, with nothing in the UI to explain it and no recovery but signing out. Failing is transient by comparison. You retry and get a correct session.

A failed lookup during rotation keeps the roles the session already carried. Rotation has a fallback that issue time does not, roles that resolved successfully once, and refusing would log you out over a blip.

Both are tested, and the issue-time test asserts nothing was persisted rather than only that an error came back. Failing after the insert would leave exactly the session the policy exists to prevent.

What routes declare

The descriptions and the enforcement disagreed. Every admin group is gated by one RequirePermission(engine, action, resource) call, not by a role, so each group now declares that permission next to the middleware it mirrors: manage:user on admin and bulk, manage:app on the two app-config groups, manage:environment on environments, read:security_event on security events. The Requires admin role. sentences came out of the descriptions, because they described something that was not happening.

rbac.PermissionString(action, resource) gives that flattening one home. Forge takes one string per permission and it becomes a constant in every generated client, so manage:user here and user:manage three files away would both compile and silently never match.

Only handleGrantPlatformOwner checks a role itself, against PlatformOwnerSlug, so only it declares one.

Two descriptions are left alone deliberately. POST /v1/admin/apps and its delete both claim "Requires platform admin role" and nothing enforces it: they sit in the manage:user group like everything else. Declaring platform-admin would write a fiction into the document. Deleting the sentence would quietly erase a real discrepancy. It is either a missing check or a stale doc, and that is a call for review to make.

Eight groups now declare forge.WithGroupAuth("session", "session-cookie"). Both, because the admin dashboard authenticates by cookie and declaring only the bearer scheme would have a generated client attach the wrong credential. The hand-maintained manifest in api.go stays for now: it is a public unauthenticated endpoint, and removing it is a breaking change that deserves its own decision.

The cookie that was never declared

SessionProvider has always accepted either a bearer header or the session cookie, but one provider declares one security scheme and its scheme said bearer. Generated clients therefore only ever sent the header, and the browser flow the server supports was invisible to every one of them.

CookieSessionProvider declares the other half, {apiKey, cookie, authsome_session_token}. It embeds the session provider and overrides three methods, delegating authentication wholesale, so the two schemes cannot drift into disagreeing about what a valid session is. The cookie name is per-app at runtime and OpenAPI needs it static, so the scheme declares the default and a renamed deployment regenerates its clients.

/me returns the principal

A generated client needs the principal before its capability surface answers anything, and /me returned a profile with no roles, so every consumer had to assemble one from the admin role-listing endpoints by hand. MeResponse embeds *user.User, so every existing key keeps its place and roles is the addition. Non-breaking.

It returns the session's roles rather than a fresh lookup on purpose: a fresh list would show roles the current session cannot exercise, and a client would enable a control the server then refuses.

Also in here

Session principal identity had no column in any SQL store. PrincipalKind and ServiceAccountID were set in memory and never persisted, so a service-account session came back from Postgres, SQLite or Mongo looking like an ordinary user session, with UserID at its zero value. That work was written in parallel in the same tree and its hunks sit next to the roles column in the same three models.go files, the same migrations and the same conformance suite, so it could not be split out cleanly.

Verification

  • go build ./... and go vet clean
  • go test -short green on the root package, all four store backends, and ./api/...
  • testSessionRolesRoundTrip lives in the shared conformance suite, so SQLite, Postgres, Mongo and memory all assert it from one place

Two dead ends are worth knowing, because only the database catches them. The roles column started as NOT NULL DEFAULT '', copied from add_session_impersonation: a roleless session sends an explicit NULL, and an explicit NULL beats a column default, so every session without roles was rejected. Making it nullable then broke the read instead, because json.RawMessage cannot scan a nil driver value and every pre-existing row would fail to load. What holds is the pattern the file already used for metadata: always marshal, never write NULL, keep the column NOT NULL with a default that backfills. Both migration comments say so, since the next person will otherwise simplify it back.

Worth a look during review

With the fail-closed policy, ListUserRolesInApp is now on the critical path of every sign-in. A slow RBAC store is a slow login rather than a slow authorization check, so it is worth confirming it cannot hang.

Roles are stored as JSON rather than the comma-separated form APIKeyModel.Scopes uses. A slug containing a comma would split into two role names nobody was granted, and these strings come back as an authorization decision, so the encoding must not be able to invent a member. Mongo keeps a native array.

Forge can now carry a route's authorization requirement into the OpenAPI
document and out into generated clients, but only if the server declares
it and puts the subject's roles on the auth context. Authsome did
neither, so every declared role requirement would have denied everyone.

Roles are resolved once, when a session is issued, and stored on it.
Sessions are created from six places and all of them end at
store.CreateSession, so the stamp lives in a decorator around the store
rather than at each issue site, where the seventh path would forget it.
Rotation re-stamps, which bounds staleness to the refresh interval
instead of the whole session lifetime.

The two failure policies differ on purpose. A failed lookup at issue
time fails the sign-in, because the stamp is persisted and continuing
would hand someone a session that authenticates for days and can reach
no route declaring a role. A failed lookup during rotation keeps the
roles the session already carried, because there it has something to
fall back on and refusing would log the user out over a blip.

Routes now declare what the middleware beside them already enforces:
the six admin groups gate on a permission, not a role, so they declare
that permission rather than the "Requires admin role" prose their
descriptions carried. Only handleGrantPlatformOwner checks a role
itself, so only it declares one. Two descriptions claiming platform
admin are left alone: nothing enforces them, and declaring a fiction is
worse than leaving the discrepancy visible.

CookieSessionProvider declares the session cookie that SessionProvider
has always accepted and never described, so a generated client can do
the browser flow. It delegates authentication wholesale, so the two
schemes cannot disagree about what a valid session is.

/me returns the session's roles alongside the profile, which is what a
generated client needs before its capability surface can answer
anything. user.User is embedded, so existing keys do not move.

Also carries the session principal-identity persistence written in
parallel in this tree: PrincipalKind and ServiceAccountID had no column
in any SQL store, so a service-account session came back from Postgres,
SQLite or Mongo looking like an ordinary user session. Its columns,
mappings and round-trip tests are interleaved with the roles column in
the same files and could not be split out cleanly.

go.mod is deliberately not part of this commit. It carries a local
replace pointing at a forge checkout, because AuthContext.Roles is not
in a released forge yet. This does not build until that ships.
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.

1 participant