From d22c71d6d44969806815c4e16d25812a0f358d35 Mon Sep 17 00:00:00 2001 From: Lakhan Samani Date: Thu, 9 Jul 2026 17:34:10 +0530 Subject: [PATCH 1/2] feat(fga): resolve client_credentials callers as service_account subjects - A caller presenting a machine (client_credentials) access token now resolves to service_account: in check_permissions and list_permissions, instead of a user subject. - Classification keys only on the login_method claim; delegated (RFC 8693, act chain) and user tokens keep resolving to user:. The delegation guard is therefore structural, not a runtime check. - Reuse the public client_id (not the internal surrogate id) as the FGA subject id so admin tuples and the client registry share one key. - Fail closed: unresolvable/non-service_account client denies; a model without the service_account type denies (engine errors on the unknown subject type) and never falls back to a user subject. - allowed_scopes stays the coarse ceiling at token issuance; FGA is additive below it. Admin tuple writes are already free-form, so no change is needed to reference service_account subjects. --- .../fga_service_account_test.go | 342 ++++++++++++++++++ internal/service/fga.go | 114 ++++-- 2 files changed, 434 insertions(+), 22 deletions(-) create mode 100644 internal/integration_tests/fga_service_account_test.go diff --git a/internal/integration_tests/fga_service_account_test.go b/internal/integration_tests/fga_service_account_test.go new file mode 100644 index 00000000..e228de65 --- /dev/null +++ b/internal/integration_tests/fga_service_account_test.go @@ -0,0 +1,342 @@ +package integration_tests + +import ( + "context" + "fmt" + "net/http" + "net/url" + "testing" + + "github.com/gin-gonic/gin" + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/crypto/bcrypt" + + "github.com/authorizerdev/authorizer/internal/constants" + "github.com/authorizerdev/authorizer/internal/graph/model" + "github.com/authorizerdev/authorizer/internal/storage/schemas" +) + +// fgaServiceAccountModel adds the service_account subject type alongside user, +// which is the opt-in that turns autonomous client_credentials callers into FGA +// subjects (locked decision: works only if the model declares service_account). +const fgaServiceAccountModel = `model + schema 1.1 +type user +type service_account +type document + relations + define viewer: [user, service_account] + define can_view: viewer +` + +// createServiceAccountWithClientID inserts an active service_account whose PUBLIC +// client_id is DISTINCT from its surrogate ID, and returns (surrogateID, +// publicClientID, plaintextSecret). The distinct client_id lets the tests prove +// the FGA subject keys on client_id, never the internal surrogate id. +func createServiceAccountWithClientID(t *testing.T, ts *testSetup, allowedScopes string) (string, string, string) { + t.Helper() + secret := "cc-secret-" + uuid.New().String() + hash, err := bcrypt.GenerateFromPassword([]byte(secret), ccClientCost) + require.NoError(t, err) + publicClientID := "svc-" + uuid.New().String() + sa, err := ts.StorageProvider.AddClient(context.Background(), &schemas.Client{ + Name: "cc-fga-sa-" + uuid.New().String(), + Kind: constants.ClientKindServiceAccount, + ClientID: publicClientID, + ClientSecret: string(hash), + AllowedScopes: allowedScopes, + IsActive: true, + }) + require.NoError(t, err) + require.NotEqual(t, sa.ID, sa.ClientID, "test requires a client_id distinct from the surrogate id") + return sa.ID, publicClientID, secret +} + +// mintMachineToken runs the real client_credentials grant against /oauth/token +// and returns the issued access token. The issuer host is pinned to +// ccTestAuthorizerURL via X-Authorizer-URL so validation on the GraphQL path can +// reproduce the same iss. +func mintMachineToken(t *testing.T, tokenRouter http.Handler, publicClientID, secret, scope string) string { + t.Helper() + form := url.Values{} + form.Set("grant_type", constants.GrantTypeClientCredentials) + if scope != "" { + form.Set("scope", scope) + } + w := postClientCredentials(tokenRouter, form, []string{publicClientID, secret}) + require.Equal(t, http.StatusOK, w.Code, "token issuance failed: %s", w.Body.String()) + body := decodeJSON(t, w) + token, _ := body["access_token"].(string) + require.NotEmpty(t, token, "must return an access_token") + return token +} + +// presentMachineToken pins the current gin request to a machine bearer token: +// it drops any session cookie and pins X-Authorizer-URL so the iss check matches +// the token minted through /oauth/token. +func presentMachineToken(ts *testSetup, token string) { + clearCookies(ts) + ts.GinContext.Request.Header.Set("Authorization", "Bearer "+token) + ts.GinContext.Request.Header.Set("X-Authorizer-URL", ccTestAuthorizerURL) +} + +// TestFGAServiceAccountSubject exercises machine-token subject resolution in the +// public FGA check/list path: an autonomous client_credentials caller resolves +// to service_account: and is checked against the model/tuples, with +// the fail-closed and delegation-guard rules the locked design requires. +func TestFGAServiceAccountSubject(t *testing.T) { + cfg := getTestConfig() + ts, _ := initFGATestSetup(t, cfg) + _, ctx := createContext(ts) + + tokenRouter := gin.New() + tokenRouter.POST("/oauth/token", ts.HttpProvider.TokenHandler()) + + // Admin installs a model that declares service_account. + setAdminCookie(t, ts) + modelRes, err := ts.GraphQLProvider.FgaWriteModel(ctx, &model.FgaWriteModelInput{Dsl: fgaServiceAccountModel}) + require.NoError(t, err) + require.NotNil(t, modelRes) + + saID, saClientID, saSecret := createServiceAccountWithClientID(t, ts, "openid") + + // Grant the service account (keyed on its PUBLIC client_id) viewer on one + // document only. Also grant a decoy tuple keyed on the SURROGATE id to prove + // the surrogate is never the subject. + setAdminCookie(t, ts) + _, err = ts.GraphQLProvider.FgaWriteTuples(ctx, &model.FgaWriteTuplesInput{ + Tuples: []*model.FgaTupleInput{ + {User: "service_account:" + saClientID, Relation: "viewer", Object: "document:sa-granted"}, + {User: "service_account:" + saID, Relation: "viewer", Object: "document:sa-surrogate-decoy"}, + }, + }) + require.NoError(t, err) + + // (a) machine token + model WITH service_account type → allow/deny per tuples. + t.Run("machine token resolves to service_account: and is allowed on granted object", func(t *testing.T) { + token := mintMachineToken(t, tokenRouter, saClientID, saSecret, "") + presentMachineToken(ts, token) + + res, err := ts.GraphQLProvider.CheckPermissions(ctx, &model.CheckPermissionsInput{ + Checks: []*model.PermissionCheckInput{{Relation: "can_view", Object: "document:sa-granted"}}, + }) + require.NoError(t, err) + require.NotNil(t, res) + assert.True(t, res.Results[0].Allowed, "machine caller must be allowed on its granted object") + }) + + t.Run("machine token denied on ungranted object", func(t *testing.T) { + token := mintMachineToken(t, tokenRouter, saClientID, saSecret, "") + presentMachineToken(ts, token) + + res, err := ts.GraphQLProvider.CheckPermissions(ctx, &model.CheckPermissionsInput{ + Checks: []*model.PermissionCheckInput{{Relation: "can_view", Object: "document:sa-other"}}, + }) + require.NoError(t, err) + require.NotNil(t, res) + assert.False(t, res.Results[0].Allowed, "machine caller must be denied on an ungranted object") + }) + + // Decision: reuse client_id, NEVER the surrogate id. + t.Run("machine subject is client_id not the surrogate id", func(t *testing.T) { + token := mintMachineToken(t, tokenRouter, saClientID, saSecret, "") + presentMachineToken(ts, token) + + res, err := ts.GraphQLProvider.CheckPermissions(ctx, &model.CheckPermissionsInput{ + Checks: []*model.PermissionCheckInput{{Relation: "can_view", Object: "document:sa-surrogate-decoy"}}, + }) + require.NoError(t, err) + require.NotNil(t, res) + assert.False(t, res.Results[0].Allowed, + "a tuple keyed on the surrogate id must NOT grant the machine caller (subject is client_id)") + }) + + // list_permissions for the machine caller enumerates only the service + // account's own objects. + t.Run("list_permissions returns only the machine caller's objects", func(t *testing.T) { + token := mintMachineToken(t, tokenRouter, saClientID, saSecret, "") + presentMachineToken(ts, token) + + res, err := ts.GraphQLProvider.ListPermissions(ctx, &model.ListPermissionsInput{}) + require.NoError(t, err) + require.NotNil(t, res) + assert.Contains(t, res.Objects, "document:sa-granted") + assert.NotContains(t, res.Objects, "document:sa-surrogate-decoy", + "the surrogate-keyed decoy must not appear for the client_id subject") + }) + + // A machine caller may self-pin its own subject explicitly, but must be + // REJECTED when probing any other subject (it is never a super-admin). + t.Run("machine caller may self-pin but cannot probe another subject", func(t *testing.T) { + token := mintMachineToken(t, tokenRouter, saClientID, saSecret, "") + presentMachineToken(ts, token) + + self := "service_account:" + saClientID + res, err := ts.GraphQLProvider.CheckPermissions(ctx, &model.CheckPermissionsInput{ + Checks: []*model.PermissionCheckInput{{Relation: "can_view", Object: "document:sa-granted"}}, + User: &self, + }) + require.NoError(t, err, "self-specification must be honored") + require.NotNil(t, res) + assert.True(t, res.Results[0].Allowed) + + other := "user:someone-else" + res, err = ts.GraphQLProvider.CheckPermissions(ctx, &model.CheckPermissionsInput{ + Checks: []*model.PermissionCheckInput{{Relation: "can_view", Object: "document:sa-granted"}}, + User: &other, + }) + assert.Error(t, err, "machine caller must not probe another subject") + assert.Nil(t, res) + assert.Contains(t, err.Error(), "not authorized to query authorization for another subject") + }) + + // (c)+(d) A user/session token still resolves to user: and NEVER inherits + // the service account's grants — even though a service_account with tuples + // exists. A real RFC 8693 delegated token carries a user sub + an act chain + // and no service_account login_method, so it is classified here exactly like + // this user token: the guard is structural (see callerOwnSubject). + t.Run("user token stays user: and does not inherit service_account grants", func(t *testing.T) { + clearCookies(ts) + ts.GinContext.Request.Header.Del("Authorization") + ts.GinContext.Request.Header.Del("X-Authorizer-URL") + + email := "fga_sa_user_" + uuid.New().String() + "@authorizer.dev" + password := "Password@123" + _, err := ts.GraphQLProvider.SignUp(ctx, &model.SignUpRequest{ + Email: &email, Password: password, ConfirmPassword: password, + }) + require.NoError(t, err) + loginRes, err := ts.GraphQLProvider.Login(ctx, &model.LoginRequest{Email: &email, Password: password}) + require.NoError(t, err) + userID := loginRes.User.ID + sessionToken := latestAppSessionCookie(ts) + require.NotEmpty(t, sessionToken) + + // Grant THIS user viewer on a different document. + setAdminCookie(t, ts) + _, err = ts.GraphQLProvider.FgaWriteTuples(ctx, &model.FgaWriteTuplesInput{ + Tuples: []*model.FgaTupleInput{ + {User: "user:" + userID, Relation: "viewer", Object: "document:user-doc"}, + }, + }) + require.NoError(t, err) + + clearCookies(ts) + ts.GinContext.Request.Header.Set("Cookie", fmt.Sprintf("%s_session=%s", constants.AppCookieName, sessionToken)) + + // Allowed on the user's own grant. + res, err := ts.GraphQLProvider.CheckPermissions(ctx, &model.CheckPermissionsInput{ + Checks: []*model.PermissionCheckInput{{Relation: "can_view", Object: "document:user-doc"}}, + }) + require.NoError(t, err) + assert.True(t, res.Results[0].Allowed, "user resolves to user: and sees its own grant") + + // Denied on the service account's grant — no leakage across subject types. + res, err = ts.GraphQLProvider.CheckPermissions(ctx, &model.CheckPermissionsInput{ + Checks: []*model.PermissionCheckInput{{Relation: "can_view", Object: "document:sa-granted"}}, + }) + require.NoError(t, err) + assert.False(t, res.Results[0].Allowed, + "a user token must never inherit a service_account's grants") + }) +} + +// TestFGAServiceAccountModelWithoutType asserts the fail-closed rule: when the +// active model does NOT declare service_account, a machine caller is DENIED and +// never falls back to a user subject. +func TestFGAServiceAccountModelWithoutType(t *testing.T) { + cfg := getTestConfig() + ts, _ := initFGATestSetup(t, cfg) + _, ctx := createContext(ts) + + tokenRouter := gin.New() + tokenRouter.POST("/oauth/token", ts.HttpProvider.TokenHandler()) + + // Model declares ONLY user — no service_account type. + setAdminCookie(t, ts) + _, err := ts.GraphQLProvider.FgaWriteModel(ctx, &model.FgaWriteModelInput{Dsl: fgaTestModel}) + require.NoError(t, err) + + _, saClientID, saSecret := createServiceAccountWithClientID(t, ts, "openid") + + token := mintMachineToken(t, tokenRouter, saClientID, saSecret, "") + presentMachineToken(ts, token) + + res, err := ts.GraphQLProvider.CheckPermissions(ctx, &model.CheckPermissionsInput{ + Checks: []*model.PermissionCheckInput{{Relation: "can_view", Object: "document:1"}}, + }) + // Fail-closed: either the engine rejects the unknown subject type (error) or + // returns a no-match deny. Never allowed, never a user-subject fallback. + if err != nil { + assert.Nil(t, res, "a model without service_account must fail closed for machine callers") + } else { + require.NotNil(t, res) + assert.False(t, res.Results[0].Allowed, + "a model without service_account must deny machine callers (fail closed)") + } +} + +// TestFGAServiceAccountScopeCeilingIndependentOfFGA proves the two layers are +// independent (locked decision: allowed_scopes stays the coarse ceiling at +// issuance; FGA is additive below it): +// - a scope outside allowed_scopes is rejected at the TOKEN endpoint, with no +// FGA involvement; +// - a validly-scoped token still needs a passing FGA check at the resource. +func TestFGAServiceAccountScopeCeilingIndependentOfFGA(t *testing.T) { + cfg := getTestConfig() + ts, _ := initFGATestSetup(t, cfg) + _, ctx := createContext(ts) + + tokenRouter := gin.New() + tokenRouter.POST("/oauth/token", ts.HttpProvider.TokenHandler()) + + setAdminCookie(t, ts) + _, err := ts.GraphQLProvider.FgaWriteModel(ctx, &model.FgaWriteModelInput{Dsl: fgaServiceAccountModel}) + require.NoError(t, err) + + // allowed_scopes = "read" only. + _, saClientID, saSecret := createServiceAccountWithClientID(t, ts, "read") + + // Scope ceiling enforced at issuance — a scope outside allowed_scopes is + // rejected by the token endpoint, independent of any FGA state. + t.Run("scope outside allowed_scopes is rejected at the token endpoint", func(t *testing.T) { + form := url.Values{} + form.Set("grant_type", constants.GrantTypeClientCredentials) + form.Set("scope", "write") + w := postClientCredentials(tokenRouter, form, []string{saClientID, saSecret}) + require.Equal(t, http.StatusBadRequest, w.Code, "body: %s", w.Body.String()) + body := decodeJSON(t, w) + assert.Equal(t, "invalid_scope", body["error"]) + }) + + // A validly-scoped token is issued, but FGA still governs the resource: no + // tuple → denied, even though the token is valid and in-scope. + t.Run("valid scoped token still needs a passing FGA check", func(t *testing.T) { + token := mintMachineToken(t, tokenRouter, saClientID, saSecret, "read") + presentMachineToken(ts, token) + + res, err := ts.GraphQLProvider.CheckPermissions(ctx, &model.CheckPermissionsInput{ + Checks: []*model.PermissionCheckInput{{Relation: "can_view", Object: "document:scoped-doc"}}, + }) + require.NoError(t, err) + assert.False(t, res.Results[0].Allowed, "a valid scoped token still needs an FGA grant") + + // Grant the tuple; now the SAME token passes — both layers satisfied. + setAdminCookie(t, ts) + _, err = ts.GraphQLProvider.FgaWriteTuples(ctx, &model.FgaWriteTuplesInput{ + Tuples: []*model.FgaTupleInput{ + {User: "service_account:" + saClientID, Relation: "viewer", Object: "document:scoped-doc"}, + }, + }) + require.NoError(t, err) + presentMachineToken(ts, token) + + res, err = ts.GraphQLProvider.CheckPermissions(ctx, &model.CheckPermissionsInput{ + Checks: []*model.PermissionCheckInput{{Relation: "can_view", Object: "document:scoped-doc"}}, + }) + require.NoError(t, err) + assert.True(t, res.Results[0].Allowed, "in-scope token with an FGA grant passes both layers") + }) +} diff --git a/internal/service/fga.go b/internal/service/fga.go index 29fc1918..d781ab84 100644 --- a/internal/service/fga.go +++ b/internal/service/fga.go @@ -10,6 +10,7 @@ import ( "github.com/authorizerdev/authorizer/internal/authctx" "github.com/authorizerdev/authorizer/internal/authorization/engine" + "github.com/authorizerdev/authorizer/internal/constants" "github.com/authorizerdev/authorizer/internal/graph/model" ) @@ -40,36 +41,29 @@ const maxContextualTuplesPerCheck = 100 // client-supplied explicitUser. // // Rules (fail-closed): -// - explicitUser empty → the caller's own token subject ("user:" from -// the JWT / session cookie). This is the default and the common case. +// - explicitUser empty → the caller's own subject (see callerOwnSubject): +// "user:" for a human/session caller, or "service_account:" +// for an autonomous client_credentials (machine) caller. This is the +// default and the common case. // - explicitUser set → normalized (a bare id becomes "user:") and // validated, then honored only when the caller is a super-admin OR the -// subject equals the caller's own token subject. Anything else is -// REJECTED — never silently self-pinned, never silently ignored — -// because honoring it would let an end user probe another subject's -// access (IDOR / info disclosure). -// -// TODO(phase-2 M2M): machine-to-machine / client-credentials callers should -// also be allowed to pass an explicit user once that caller type exists; -// extend the trust check here (the rule must stay centralized in this one -// helper). +// subject equals the caller's own subject. Anything else is REJECTED — +// never silently self-pinned, never silently ignored — because honoring it +// would let a caller probe another subject's access (IDOR / info +// disclosure). A machine caller may therefore only self-pin (its +// "service_account:") or be denied; it is never a super-admin. func (p *provider) resolveFgaSubject(ctx context.Context, meta RequestMetadata, explicitUser string) (string, error) { explicitUser = strings.TrimSpace(explicitUser) - // The caller's own subject, when they carry a user token/session. Resolved - // lazily-ish here because both branches may need it. - ownSubject := "" - if principal, ok := authctx.FromContext(ctx); ok && strings.TrimSpace(principal.UserID) != "" { - ownSubject = "user:" + principal.UserID - } else { - gc := &gin.Context{Request: meta.Request} - if tokenData, terr := p.TokenProvider.GetUserIDFromSessionOrAccessToken(gc); terr == nil && strings.TrimSpace(tokenData.UserID) != "" { - ownSubject = "user:" + tokenData.UserID - } + // The caller's own subject, when they carry a user/session/machine token. + // Fail-closed: a machine token whose client cannot be resolved errors here. + ownSubject, err := p.callerOwnSubject(ctx, meta) + if err != nil { + return "", err } if explicitUser == "" { - // Default: pin to the caller's own token subject. + // Default: pin to the caller's own subject. if ownSubject == "" { return "", Unauthenticated("unauthorized") } @@ -100,6 +94,82 @@ func (p *provider) resolveFgaSubject(ctx context.Context, meta RequestMetadata, return "", PermissionDenied("not authorized to query authorization for another subject") } +// callerOwnSubject returns the caller's canonical OpenFGA subject derived from +// their authenticated token/session, or "" when the request carries no user or +// machine credential (e.g. a super admin authenticated only by the admin +// cookie/secret). It is the single place that classifies a caller as a machine +// (client_credentials) subject vs a human user subject. +// +// MACHINE vs USER vs DELEGATED — the classification keys ONLY on the token's +// login_method claim: +// - login_method == constants.AuthRecipeMethodServiceAccount is stamped +// EXCLUSIVELY on client_credentials machine tokens +// (token.createMachineAccessToken). Those tokens have no resource-owner user +// (sub is the service account's surrogate id) and never carry an RFC 8693 +// `act` delegation claim. Such a caller resolves to +// "service_account:". +// - every other login_method (human recipes, sso) resolves to "user:". +// +// This makes the delegation guard structural, not a runtime check: an RFC 8693 +// delegated token (token.CreateDelegatedAccessToken) is stateless, carries a +// user `sub` plus an `act` chain, and carries NO login_method claim — so it can +// never be classified as a machine subject and always resolves to "user:". +// The security-critical rule (delegated and user tokens stay user subjects; only +// autonomous machine tokens become service_account subjects) holds by +// construction. +func (p *provider) callerOwnSubject(ctx context.Context, meta RequestMetadata) (string, error) { + callerID, loginMethod := "", "" + if principal, ok := authctx.FromContext(ctx); ok && strings.TrimSpace(principal.UserID) != "" { + callerID = principal.UserID + loginMethod = principal.LoginMethod + } else { + gc := &gin.Context{Request: meta.Request} + if tokenData, terr := p.TokenProvider.GetUserIDFromSessionOrAccessToken(gc); terr == nil && strings.TrimSpace(tokenData.UserID) != "" { + callerID = tokenData.UserID + loginMethod = tokenData.LoginMethod + } + } + if callerID == "" { + return "", nil + } + if loginMethod == constants.AuthRecipeMethodServiceAccount { + return p.machineFgaSubject(ctx, callerID) + } + return "user:" + callerID, nil +} + +// machineFgaSubject maps an authenticated client_credentials caller — whose +// token `sub` is the service account's SURROGATE id (schemas.Client.ID) — to its +// OpenFGA subject "service_account:". It resolves the PUBLIC client_id +// so that admin-written tuples and the client registry share one key (locked +// decision: reuse client_id, never the internal surrogate id). +// +// Fail-closed: any lookup failure — or a client whose kind is not +// service_account — denies rather than falling back to any other subject. A +// machine caller is therefore NEVER silently promoted to a user subject. +// +// If the deployment's authorization model does not declare the service_account +// type, the downstream engine Check/ListObjects on this subject fails closed +// (error or no-match deny) — the caller can never inherit a user's access. +func (p *provider) machineFgaSubject(ctx context.Context, serviceAccountID string) (string, error) { + client, err := p.StorageProvider.GetClientByID(ctx, serviceAccountID) + if err != nil || client == nil { + return "", PermissionDenied("unauthorized") + } + // Defense in depth: only service_account clients are FGA subjects; an + // interactive client must never become one. Machine tokens are only ever + // issued to service_account clients, so this is a belt-and-suspenders guard. + if client.Kind != constants.ClientKindServiceAccount { + return "", PermissionDenied("unauthorized") + } + clientID := strings.TrimSpace(client.ClientID) + if clientID == "" { + // Legacy rows may carry an empty client_id; storage defaults it to ID. + clientID = client.ID + } + return "service_account:" + clientID, nil +} + // normalizeFgaSubject turns a bare id into the canonical "user:" form; // values that already carry a type ("type:id") pass through unchanged. func normalizeFgaSubject(user string) string { From 295e12a84e84670724db617075b2cc8274760a47 Mon Sep 17 00:00:00 2001 From: Lakhan Samani Date: Thu, 9 Jul 2026 17:45:54 +0530 Subject: [PATCH 2/2] harden(fga): deny deactivated service accounts and validate subject id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Security-review LOWs: FGA is an authorization decision surface, so a deactivated service account is denied at check time even while its token remains valid elsewhere (delete already failed closed). The resolved client_id is also rejected if it ever contains tuple syntax (:#@/whitespace) — defense in depth; it is a server UUID today. --- .../fga_service_account_test.go | 26 +++++++++++++++++++ internal/service/fga.go | 12 +++++++++ 2 files changed, 38 insertions(+) diff --git a/internal/integration_tests/fga_service_account_test.go b/internal/integration_tests/fga_service_account_test.go index e228de65..1b60d45f 100644 --- a/internal/integration_tests/fga_service_account_test.go +++ b/internal/integration_tests/fga_service_account_test.go @@ -15,6 +15,7 @@ import ( "github.com/authorizerdev/authorizer/internal/constants" "github.com/authorizerdev/authorizer/internal/graph/model" + "github.com/authorizerdev/authorizer/internal/refs" "github.com/authorizerdev/authorizer/internal/storage/schemas" ) @@ -139,6 +140,31 @@ func TestFGAServiceAccountSubject(t *testing.T) { assert.False(t, res.Results[0].Allowed, "machine caller must be denied on an ungranted object") }) + t.Run("deactivated service account is denied even with a live token", func(t *testing.T) { + token := mintMachineToken(t, tokenRouter, saClientID, saSecret, "") + + setAdminCookie(t, ts) + _, err := ts.GraphQLProvider.UpdateClient(ctx, &model.UpdateClientRequest{ + ID: saID, + IsActive: refs.NewBoolRef(false), + }) + require.NoError(t, err) + defer func() { + setAdminCookie(t, ts) + _, err := ts.GraphQLProvider.UpdateClient(ctx, &model.UpdateClientRequest{ + ID: saID, + IsActive: refs.NewBoolRef(true), + }) + require.NoError(t, err) + }() + + presentMachineToken(ts, token) + _, err = ts.GraphQLProvider.CheckPermissions(ctx, &model.CheckPermissionsInput{ + Checks: []*model.PermissionCheckInput{{Relation: "can_view", Object: "document:sa-granted"}}, + }) + require.Error(t, err, "FGA must deny a deactivated service account even if its token is still valid") + }) + // Decision: reuse client_id, NEVER the surrogate id. t.Run("machine subject is client_id not the surrogate id", func(t *testing.T) { token := mintMachineToken(t, tokenRouter, saClientID, saSecret, "") diff --git a/internal/service/fga.go b/internal/service/fga.go index d781ab84..aa8e3394 100644 --- a/internal/service/fga.go +++ b/internal/service/fga.go @@ -162,11 +162,23 @@ func (p *provider) machineFgaSubject(ctx context.Context, serviceAccountID strin if client.Kind != constants.ClientKindServiceAccount { return "", PermissionDenied("unauthorized") } + // FGA is an authorization decision surface: deny deactivated service + // accounts here even though their tokens stay valid until exp elsewhere + // (issuance already blocks new tokens; this gives revocation teeth where + // it matters most). + if !client.IsActive { + return "", PermissionDenied("unauthorized") + } clientID := strings.TrimSpace(client.ClientID) if clientID == "" { // Legacy rows may carry an empty client_id; storage defaults it to ID. clientID = client.ID } + // Defense in depth: client_id is a server-generated UUID today, but the + // subject string must never smuggle tuple syntax if that ever changes. + if strings.ContainsAny(clientID, ":#@ \t\n") { + return "", PermissionDenied("unauthorized") + } return "service_account:" + clientID, nil }