security: at-rest key, storage not-found contract, and agent identity in FGA - #742
Merged
lakhansamani merged 26 commits intoAug 6, 2026
Merged
Conversation
The at-rest key was wired directly to --jwt-secret, which is only required for HMAC JWT types. An RS*/ES* install legitimately leaves it empty, so the key resolved to "" and HKDF-SHA256 over empty keying material (no salt, fixed info string) produced a fixed, publicly computable AES key: 62d720a3...419e. TOTP seeds were recoverable by anyone with a database copy and this source. The same empty value keyed the OTP HMAC, making stored digests reversible by brute force over the 10^6 code space — including outstanding password-reset codes, i.e. account takeover. Both failed silently: encryption "succeeded", rows carried the enc:v1: prefix, digests were the right length, nothing was logged. - add --encryption-key, resolved in Finalize as --encryption-key -> --jwt-secret -> startup error - route the five OTP HMAC sites through it (they used JWTSecret directly and so bypassed the new key entirely) - ValidateEncryptionKey is unconditional, NOT scoped to TOTP: password-reset OTPs are written whether or not TOTP is enabled, so a TOTP-scoped check leaves the reset flow hashing under an empty key - pass fixed dev keys in `make dev` and perf/run_container.sh, both of which run RS256 with no --jwt-secret Affected: 2.2.1-rc.2 through 2.4.0-rc.13, RSA/ECDSA deployments only. HMAC installs are unaffected — the fallback resolves to the same JWTSecret value used before, so existing enrolments keep working. BREAKING CHANGE: RSA/ECDSA deployments must set --encryption-key; the server refuses to start without it. Seeds written by an affected version were encrypted under a public constant and must be treated as compromised — those users re-enrol.
SAML assertion IDs and RFC 7523 client-assertion jti were consumed with a GetCache/SetCache pair. Two replays of the same assertion arriving together both observed "unseen" and were both accepted; a concurrency test reproduces it at 2/40 rounds. - add SetCacheNX to the memory-store contract: claim by creation, decided in one operation, fails closed on a store fault - redis: native SET NX - in-memory: LoadOrStore/CompareAndSwap, taking over expired entries by CAS so a racing claimant cannot slip between delete and store - db: claim by deterministic primary key (uuidv5 of the cache key), so the PK constraint decides the race with no schema change. A live row of ANY id blocks the claim — a row written by SetCache on a pre-upgrade replica carries a random uuid, would not collide, and would otherwise report the key as claimed during a rolling upgrade. The jti path keeps its pre-TokenReview read as a cheap short-circuit; the NX claim stays after TokenReview so a transient apiserver failure does not burn a still-retryable token. Atomic on SQL/Mongo/Arango/Couchbase. DynamoDB PutItem and Cassandra INSERT are upserts, so both callers can still win there — unchanged from the previous behaviour, not a regression. Configure REDIS_URL for an exact guarantee today.
Three DynamoDB getters returned (nil, nil) for an absent row where the other five return a driver not-found error: GetAuthenticatorDetailsByUserId, GetVerificationRequestByEmail, GetVerificationRequestByToken. Callers branch on err alone and then dereference, so totp.Validate, ValidateRecoveryCode, verify_email and resend_verify_email nil-panic on that backend only — invisible to CI, which runs SQLite. - DynamoDB reports absence as an error, matching every other backend - guard totp.Validate/ValidateRecoveryCode against a nil row anyway; the contract is now uniform but a panic is too severe to leave undefended - add storage.IsNotFound(err), a predicate rather than a shared sentinel: each backend reports absence with its own driver value which errors.Is cannot match against a foreign sentinel, and a sentinel declared in storage could not be wrapped by the backends because that dependency only runs one way. Shape follows apierrors.IsNotFound. - Arango/Dynamo gain a package sentinel; 42 dynamic "not found" errors wrap it. Wrapping preserves err != nil, so existing callers are unaffected and nothing changes silently. - 20 admin getters now return NotFound instead of a raw storage error, which mapped to codes.Internal — a missing row answered HTTP 500. Auth paths are deliberately excluded: a distinct 404 on VerifyEmail's GetUserByEmail would be an account-existence oracle, and org-scoped resolvers keep routing through maskNonSuperAdminError. Two static tests enforce the contract: TestNotFoundContractIsUniform compares all six backends, TestIsNotFoundRecognisesEveryBackend proves the predicate matches each one, survives %w nesting, and rejects lookalike messages.
Detached one-shot request-scoped work must be tracked so graceful shutdown drains it and a panic is recovered; an unrecovered panic in a bare goroutine takes down the whole process.
Cryptographically fine, but it couples two keys with opposite lifecycles: a signing key is meant to be rotated and rotation is cheap, while the at-rest key has no re-encryption path. While they are the same value, rotating --jwt-secret silently makes every enrolled TOTP authenticator undecryptable. A warning rather than a hard failure: unlike an empty key, which is rejected outright because the data is unprotected now, here the data IS protected and the risk is a future operator action.
openssl 3.x — the default on macOS and current Linux — writes PKCS#8
("BEGIN PRIVATE KEY") from genrsa/genpkey and PKIX ("BEGIN PUBLIC KEY")
from `rsa -pubout`. Only the older PKCS#1/SEC1 forms parsed, so keys
generated the standard way were rejected.
The failure was late and silent. Config validation passes, the server
starts, signup works — then token issuance fails with "use
ParsePKCS8PrivateKey instead for this key format" and jwks.json fails
with the PKIX equivalent. An RS256 instance looks healthy while every
login is broken and no relying party can verify a token.
Reproduced on a k3d cluster with keys from `openssl genrsa` +
`openssl rsa -pubout`; both endpoints work after the fix.
RSA and ECDSA also disagreed: the ECDSA public path already used
ParsePKIXPublicKey while RSA required PKCS#1.
Every path after AddUser can return early — the email-verification branch, the phone branch, and the MFA gate. Since 2.4.0 MFA is on by DEFAULT, so that gate fires for ordinary signups and the emissions at the bottom of SignUp became unreachable. Measured against a webhook sink on a default install: signup delivered NO webhook at all. With email verification on it delivered only user.created, because verify_email likewise returns at its own MFA gate before its RegisterEvent. Integrations that provision on signup (CRM records, welcome mail, seat accounting) silently never ran. Emit user.created and user.signup immediately after the user row is written, so they mean one thing: the account now exists. user.login stays at token issuance — a user who abandons MFA setup has signed up but not logged in, and the events should say exactly that. Verified end to end: default install and email-verification install both emit created+signup at signup, and the full journey through skip_mfa_setup emits created, signup and login exactly once each.
issueAuthResponse hardcoded ["openid","email","profile"], so every token issued after an MFA offer lost whatever scope the caller asked for. Login and signup accept a scope, but the token is minted later by skip_mfa_setup / verify_otp / webauthn, none of which see the original request. Delegation flows lost exactly the scopes they exist to attenuate. The authorization-code path is no better: the state tuple is code@@Challenge@@nonce@@redirectURI@@resource, so scope is not there to restore either. Carry it, never re-ask for it: setMFASession stashes the scope under the MFA session id and the issuance paths consume it. Adding a scope field to SkipMfaSetupRequest would have been wrong — those endpoints are unauthenticated, so a caller could self-grant scopes never requested at login, the same shape as the is_multi_factor_auth_enabled signup fix. Stored in the cache rather than on the MFA session row: no scope column exists and adding one is a migration across six backends, for state that is transient and expires with the session anyway. Consumed on read so a captured cookie cannot replay it. Verified: signup and login with a custom scope now issue tokens carrying it through skip_mfa_setup, and a request with no scope still gets the default set unchanged.
Both fixes shipped verified only by hand with curl, which protects nothing once the code moves. mfa_scope_carry_test: a custom scope requested at signup and at login survives skip_mfa_setup, and a request with no scope still gets the default set so the fix stays additive. signup_events_test: user.created and user.signup fire when the account row is written even though the MFA gate returns early, user.login does NOT fire while the token is still withheld, and the full journey through skip_mfa_setup emits each exactly once. Webhook delivery is SSRF-hardened against loopback, so these run with Env=e2e — the same switch the e2e-playground uses for its own sink. Both suites were confirmed to FAIL with their fix reverted.
Delegated tokens resolved to user:<sub> in authorization and vanished from the audit trail, so an agent had exactly its user's permissions and its actions were recorded as the human's. RFC 8693 §1.1 defines delegation as "A representing B" with A keeping its own identity; collapsing A into B is the definition of impersonation, which the token endpoint explicitly refuses. The authorization layer contradicted the token layer. - engine: refresh the cached model id periodically. Check pins an explicit AuthorizationModelId and WriteModel only updated the serving replica, so a fleet could evaluate the same request against different models indefinitely. A pinned Config.ModelID is never refreshed. - engine: add TypeNames. TypeRelations omits relation-less types, and the canonical `type agent` has none (an agent is only ever a subject), so detection built on it would silently never activate. - token: ValidateDelegatedAccessToken, a separate named path for the stateless delegated token. Skips only the session lookup, adds a strict audience match so a resource-bound token cannot authenticate here. Wired as a fallback in GetUserIDFromSessionOrAccessToken only — the OIDC /userinfo handler is deliberately untouched. - authctx: Principal carries the IMMEDIATE actor. Prior actors nested in the act chain stay informational and never influence a decision. - fga: a delegated caller is checked as agent:<client_id> AND user:<sub>; effective authority is the intersection. Enabled by the model declaring `type agent` — there is no flag, because checking an unmodelled type ERRORS rather than returning false and would deny every check. - metrics: authorizer_fga_delegated_checks_total attributes a denial to the agent or the user, which is the difference between "grant the agent a tuple" and "the user genuinely lacks access". - audit: AuditActorTypeAgent. Unaffected: OIDC /userinfo, SAML, SCIM, OAuth and client_credentials all keep the stateful validator and single-subject checks.
…gaps The delegated validator required aud == --client-id, an opaque string, while /oauth/token requires `resource` to be an absolute URI and stamps it verbatim as aud. Those conditions are mutually exclusive: no token this server can mint could ever pass. The feature was unreachable, and the test that "proved" it worked called CreateDelegatedAccessToken directly with Audience: cfg.ClientID — asserting a contract the system cannot produce. - audience is now this server's own URL, so an agent names Authorizer as the RFC 8707 resource when it wants to call Authorizer. Trailing-slash tolerant; an empty audience never matches, which also closes the degenerate case where an unset --client-id compared equal to aud:"". - subject liveness resolves user OR client. Token exchange accepts a service account as the subject (multi-hop: agent A delegates to agent B), and the old check only looked the subject up as a user — so a deactivated service account's delegation kept working for the token's full TTL. Fails closed when the subject is neither. - fail closed instead of panicking on a nil request; the audience and issuer checks both derive the host from it and parsers.GetHost does not guard nil. Tests now start at the real endpoints. agent_intersection_e2e_test.go mints through /oauth/token and drives the public GraphQL permission API, which is what caught the unreachability that unit tests hid. Known still-failing, tracked: intersection is inert on GraphQL because authctx.WithPrincipal is only called in the gRPC interceptor.
… session
Five defects found reviewing the agent-identity path, all reachable and all
silent.
- The delegation expansion was skipped whenever `user` was supplied, but
resolveFgaSubject honours self-specification for ANY caller. A delegated
agent echoing back its own subject shed the agent half of the
intersection — a one-parameter defeat of the whole protection. The gate
now keys on who the caller is, never on what they typed, and a delegated
caller may not name another subject at all (checked before the
super-admin escape, so an admin credential on the same request cannot
unlock it).
- Agent detection swallowed every error and fell back to authorizing the
user alone, so anyone able to fail the model read widened every agent.
It now denies. Blast radius is delegated callers only.
- ActorID reached the engine verbatim as an FGA subject with no shape
guard, unlike machineFgaSubject. "agent:x#member" is a userset, not the
agent that authenticated.
- A model with no `agent` type disables enforcement by design; that is now
counted as authorizer_fga_delegated_checks_total{outcome="not_enforced"}
so it is visible rather than discovered during an incident.
- Delegated tokens were unrevocable: logout, password reset and admin
session wipes all left them authenticating until their TTL ran out. They
now carry the originating session as an opaque `sid` and die with it at
Authorizer's own API. Downstream resource servers are unaffected — they
verify offline against the JWKS and never saw a revocation signal
anyway.
The caller is now resolved once and threaded through both gates; three
consumers each re-parsed the bearer token, which for a stateless delegated
token means a storage read per parse.
applyDelegationActor read the actor from authctx.Principal, which only the gRPC interceptor populates. On GraphQL — the primary surface — every delegated action was recorded as the human performing it, with no trace that anything automated was involved. Unit tests passed throughout because they injected a Principal directly. The actor is now passed in from callerTokenData, which resolves it on every transport, so attribution is transport-independent by construction rather than by a lookup that can drift again. callerTokenData was itself dropping ActorID on its gRPC branch. Covers the rewrite with a unit test and an end-to-end test that drives GraphQL with a token minted by the real /oauth/token and asserts on the stored audit row — no principal constructed anywhere.
The session lookup hits the memory store, the subject lookup hits the database. Rejecting an already-revoked delegation should not spend a DB read first. Also trim DelegationSessionID's inputs before building the key rather than only before testing them — the result is a lookup key, so a stray space baked into it addresses an entry that cannot exist.
not_enforced is the only outcome that reports a security property NOT being enforced, and it is the operator's sole signal that agent tokens are arriving unconstrained. It had no test. Also pins that the operation label comes from the caller rather than a hardcoded value, and that ordinary traffic never enters the delegated series — a noisy series is an alert that gets switched off.
The spec still configured the mock with localizedFirstName/localizedLastName from the legacy /v2/me + /v2/emailAddress pair. #740 migrated the handler and the mock's default profile to the OIDC userinfo shape, but __configure REPLACES the default wholesale, so the test was sending a payload processLinkedInUserInfo cannot read — given_name landed empty and the failure named no cause. Its comment described the removed two-call flow as current; corrected.
Three adversarial tests kept comments (and one name) written when the defects were still present — TestAdvListPermissionsHasNoIntersection asserted that it DOES intersect. A comment asserting a live vulnerability in a security file is worse than no comment: the next reader trusts it. Rewritten as regression tests for what was fixed.
…and-storage-notfound-contract # Conflicts: # e2e-playground/tests/social/linkedin.spec.ts
The Unreleased section covered the RFC 8693 exchange (#658) but not what Authorizer does with the agent identity the token carries: the perms(agent) ∩ perms(user) intersection and its model-declares-agent opt-in, delegated-token revocation via sid, and agent audit attribution. All three change security behaviour, so they belong under Security rather than Added.
lakhansamani
changed the title
This was referenced Aug 6, 2026
Injecting faults into the delegation path showed the suite catching none of them. Each test below fails on its fault and passes on correct code. - callerTokenData dropped ActorID on its authctx.Principal branch. Every delegation test drives the request branch, so agent actions over gRPC being audited as the user was invisible. - intersectObjects could return either input unintersected and pass. All enumeration tests granted the AGENT nothing, so its set was empty and the intersection was empty for a trivial reason — and the agent is subjects[0]. Now asserted on the function directly, including the case where the USER is the narrower side. - denied_by_user was never asserted. That is the Confused Deputy actually being stopped, and the one outcome an operator must not answer by widening the agent. - Multi-hop delegation into Authorizer's own API was untested. A delegated subject_token has `sid` and no `nonce`, so hop 2 must propagate sid verbatim; dropping it leaves the chain unable to authenticate at all.
c98b6f3 was flagged `!` but the changelog had zero mention of encryption. An RS256/ES256 deployment without --jwt-secret now refuses to boot, and operators on 2.2.1..2.4.0-rc.13 need to know their at-rest key was a public constant — neither fact was written down anywhere a release reader would look. Includes the rotation and re-enrollment consequence: existing ciphertext was written under the old key and will not decrypt.
Two authorization surfaces the delegation work did not reach. enforceRequiredRelations backs session, validate_session and validate_jwt_token. It hardcoded "user:<sub>" and never expanded the delegated subjects, so it answered a DIFFERENT question from check_permissions for the same token: an agent with no grant was reported as satisfying a relation the permission API denies. A gateway gating on required_relations would admit exactly what that API refuses. Reaching it needs token-type confusion — validate_jwt_token takes token_type from the REQUEST and never compares it to the token's own claim, so passing id_token for a delegated access token skips the nonce/session branch. The regression test uses that path and is verified failing without this change and passing with it. requireOrgAdmin checks org membership for the delegating USER and never looked at the actor, so an agent holding any org admin's token inherited authority over SSO connections, SAML/OIDC config, domains and membership. The FGA intersection does not reach these handlers: they ask "is this user an org admin?", not "what may this agent do?". Honest limit: I could not construct a failing repro for the org-admin path — every attempt was refused earlier in the stack for an unrelated reason — so that guard is hardening on the merits rather than a demonstrated exploit fix, and it ships without a regression test. Neither addresses the larger finding that a delegated token reaches the whole first-party API with its scope unconsulted; that needs a decision on scoping before it can be fixed.
Closes the blast-radius hole: a delegated token authenticated at Authorizer's own API and reached EVERY first-party operation with its `scope` claim never consulted. An agent granted `openid` for a downstream MCP server could read the delegating user's profile, mutate the account and deactivate it. The RFC 8693 attenuation that produced that scope was computed, returned to the caller, then ignored. Per-operation scope is how OAuth answers "what may this token do", and how Auth0, Microsoft Graph and Okta gate their own APIs. RFC 8693 returns `scope` so someone enforces it; RFC 6750 §3.1 names the failure `insufficient_scope`. Enforced for DELEGATED callers only. A first-party scope is caller-supplied and unvalidated (service.Login takes params.Scope with no allow-list), so it is a hint rather than a boundary — gating it would break existing clients for no security gain. A delegated token's ceiling is agent.allowed_scopes, which only an admin sets, so there it is real. A sensitive operation therefore needs both halves: the user's token must carry the scope AND the operator must have granted the agent a ceiling including it. Neither party can widen an agent alone, which is the shape of Microsoft's delegated permissions. Fail closed. An operation absent from the table is denied to agents whatever scope they hold, so new operations are unreachable until someone deliberately clears them — the opposite of an allowlist that widens when a contributor forgets it. That also keeps the table small: it lists what agents MAY do, not all ~200 operations. One table, two enforcement points: the gRPC interceptor (covering gRPC, the REST gateway and MCP, all of which dispatch through it) and gqlgen middleware for GraphQL. A test asserts both sides agree, since a transport where delegation behaves differently is the bug this feature already shipped once. The regression test drives the real /graphql endpoint. Its first version called GraphQLProvider directly, bypassed the middleware entirely, and passed regardless of what the gate did.
Re-audit of the gate I added in 8f9e649 found two defects in it. FAIL-OPEN. The GraphQL gate resolved the caller through GetUserIDFromSessionOrAccessToken and skipped itself whenever that errored. For a delegated token that call reads the session store and the user row, so a transient storage failure between it and the resolver's own auth would skip the gate on a request that still succeeded — the one path whose whole job is to fail closed, failing open. Whether a token is delegated, and what it is scoped to, are properties of the signed token; neither needs a database. Now decided from ParseJWTToken alone. That also makes it ~free, which matters: it runs per root field, and the old shape did a storage read each time. IsMethod. The root-field guard also tested fc.IsMethod, which reports how gqlgen resolves a field — an implementation detail of generated code. A false there would have skipped the gate entirely. Object is the schema-level fact the check actually depends on. Adds a permanent adversarial test: alias, named operation, fragment spread on the root, __typename alongside a denied field, and a denied root field batched with an allowed one. All six repelled. Verified the gRPC side covers every path that accepts a bearer token; the other two (admin secret, browser session) cannot carry a delegated token.
Auditing which injected faults the suite actually catches found three of four covered and the fourth — hop-2 dropping the incoming `sid` — caught by nothing. TestChainedDelegationKeepsItsSessionBinding had been written and verified earlier but was not in the committed file; that file holds a mix of two drafts, and this test was the casualty. Multi-hop delegation into Authorizer's own API is otherwise untested. A delegated subject_token carries `sid` and no `nonce`, so hop 2 cannot rebuild the binding and must propagate it verbatim; without it the chain cannot authenticate here at all, and the failure reads as a permissions problem rather than a lost session binding. Verified failing against the fault and passing without it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pre-2.4.0-stable hardening. Two threads: at-rest crypto + storage contract (the
original scope), and agent identity in fine-grained authorization (added after
review found the delegated path was unreachable and, once reachable, bypassable).
Security
At-rest encryption key split from the JWT secret (breaking).
--encryption-keyis now its own input. Affected 2.2.1 through 2.4.0-rc.13: with RS256/ES256 and no
--jwt-secret, the at-rest key fell back to a public constant, so TOTP secretsand recovery codes were encrypted with a key anyone could read from the source. TOTP
is disabled with a warning rather than blocking startup when no key is set.
Single-use claims are atomic (
SetCacheNX). Replay windows existed wherever aread-then-write pair stood in for a claim.
Agent authority is
perms(agent) ∩ perms(user). A delegated (RFC 8693) caller'seffective authority on
check_permissions/list_permissionsis now the intersection,per action, at request time — the Confused Deputy fix. Enumeration intersects too, or
an agent that cannot act on an object still sees it listed. Opt-in is declaring
type agentin the model, with no flag: checkingagent:<id>against a model lackingthe type errors rather than returning false, so a flag switched on against an
unprepared model would deny every delegated request.
Three bypasses found and closed during review:
userwas supplied, but self-specification isaccepted for any caller — an agent could echo back its own subject and shed the
agent half. One parameter, whole protection gone.
anyone able to fail the model read widened every agent. Now denies.
ActorIDreached the engine verbatim as a subject with no shape guard.agent:x#memberis a userset, not the agent that authenticated.Delegated tokens are revocable. They now carry the originating session as an
opaque
sid, so logout, password reset and admin session wipes stop them atAuthorizer's own API. Previously nothing did — the only lever was revoking the user.
Downstream resource servers verify offline and still see only the 5-minute TTL.
Agent actions are attributed to the agent in the audit log. The actor came from a
principal only the gRPC interceptor constructs, so every delegated action on GraphQL —
the primary surface — was recorded as the human. RFC 8693 §1.1 draws exactly this line
between delegation and impersonation.
Storage
Unified not-found contract across all 6 backends, with
storage.IsNotFound. Gettingthis wrong had already produced a DynamoDB-only nil-panic and a DB outage surfacing to
users as an invalid credential.
TestNotFoundContractIsUniformcompares the backendsstatically, so a one-backend slip fails on SQLite CI.
Also
anyone with a modern key file).
user.signupnow fires on the default signup path; it previously emitted zerowebhooks.
asyncutil.Go.Verification
go build·go vet·gofmt· full SQLite suite ·golangci-lint0 issues ·make smoke·make e2e-playground75/75 ·make e2e-playground-sdk14/14 ·dashboard vitest 43/43.
Docs: authorizerdev/docs#80. Examples: authorizerdev/examples#16.