From ca8fc76be0a2ca5fda59214743f0ca074d3db6a6 Mon Sep 17 00:00:00 2001 From: Lakhan Samani Date: Fri, 2 Oct 2026 22:08:44 +0530 Subject: [PATCH 1/5] feat(audit): add LogEventSync for evidence-critical events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Authorization changes need the audit record to be part of the operation's contract, not a side effect. LogEvent stays fire-and-forget so logins and token issuance keep the audit table off their hot path. Both paths share buildAuditLog so they cannot diverge on record shape — in particular on folding Protocol into Metadata. --- internal/audit/provider.go | 49 +++++++++--- internal/audit/provider_sync_test.go | 78 ++++++++++++++++++++ internal/service/admin_access_invite_test.go | 2 + 3 files changed, 117 insertions(+), 12 deletions(-) create mode 100644 internal/audit/provider_sync_test.go diff --git a/internal/audit/provider.go b/internal/audit/provider.go index 941ff0a89..73374a2b1 100644 --- a/internal/audit/provider.go +++ b/internal/audit/provider.go @@ -40,6 +40,20 @@ type Provider interface { // LogEvent asynchronously records an audit log entry. // It is fire-and-forget: errors are logged but not propagated. LogEvent(event Event) + + // LogEventSync records an audit log entry synchronously and returns the + // storage error. + // + // For operations where the audit record is part of the contract, not a + // side effect: an authorization change that is not evidenced is a change + // nobody can account for. Everything else — logins, token issuance — + // keeps LogEvent, so the audit table stays off those hot paths. + // + // The caller decides what a failure means. For authorization changes the + // convention is to return the error and NOT compensate: by the time this + // is called the change has already been applied, so the error means + // "applied but unevidenced", not "nothing happened". + LogEventSync(ctx context.Context, event Event) error } type provider struct { @@ -84,23 +98,34 @@ func metadataWithProtocol(meta, protocol string) string { return string(b) } +// buildAuditLog converts an Event into its storage row. Shared by LogEvent and +// LogEventSync so the two can never disagree about how a record is shaped — +// in particular, both fold Protocol into Metadata via metadataWithProtocol. +func buildAuditLog(event Event) *schemas.AuditLog { + return &schemas.AuditLog{ + ActorID: event.ActorID, + ActorType: event.ActorType, + ActorEmail: event.ActorEmail, + Action: event.Action, + ResourceType: event.ResourceType, + ResourceID: event.ResourceID, + IPAddress: event.IPAddress, + UserAgent: event.UserAgent, + Metadata: metadataWithProtocol(event.Metadata, event.Protocol), + } +} + // LogEvent asynchronously records an audit log entry. func (p *provider) LogEvent(event Event) { asyncutil.Go(p.deps.Log, func() { log := p.deps.Log.With().Str("func", "LogEvent").Logger() - auditLog := &schemas.AuditLog{ - ActorID: event.ActorID, - ActorType: event.ActorType, - ActorEmail: event.ActorEmail, - Action: event.Action, - ResourceType: event.ResourceType, - ResourceID: event.ResourceID, - IPAddress: event.IPAddress, - UserAgent: event.UserAgent, - Metadata: metadataWithProtocol(event.Metadata, event.Protocol), - } - if err := p.deps.StorageProvider.AddAuditLog(context.Background(), auditLog); err != nil { + if err := p.deps.StorageProvider.AddAuditLog(context.Background(), buildAuditLog(event)); err != nil { log.Debug().Err(err).Str("action", event.Action).Msg("Failed to add audit log") } }) } + +// LogEventSync records an audit log entry synchronously. +func (p *provider) LogEventSync(ctx context.Context, event Event) error { + return p.deps.StorageProvider.AddAuditLog(ctx, buildAuditLog(event)) +} diff --git a/internal/audit/provider_sync_test.go b/internal/audit/provider_sync_test.go new file mode 100644 index 000000000..475f61451 --- /dev/null +++ b/internal/audit/provider_sync_test.go @@ -0,0 +1,78 @@ +package audit + +import ( + "context" + "errors" + "testing" + + "github.com/rs/zerolog" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/authorizerdev/authorizer/internal/storage" + "github.com/authorizerdev/authorizer/internal/storage/schemas" +) + +// fakeAuditStore records what AddAuditLog received and can be told to fail. +// It embeds storage.Provider (nil) so it satisfies the interface without +// implementing the other ~200 methods; only AddAuditLog is ever called here. +type fakeAuditStore struct { + storage.Provider + got *schemas.AuditLog + err error +} + +func (f *fakeAuditStore) AddAuditLog(_ context.Context, l *schemas.AuditLog) error { + if f.err != nil { + return f.err + } + f.got = l + return nil +} + +func newTestProvider(st storage.Provider) Provider { + log := zerolog.Nop() + return New(&Dependencies{Log: &log, StorageProvider: st}) +} + +func TestLogEventSync_PersistsEventAndReturnsNil(t *testing.T) { + st := &fakeAuditStore{} + p := newTestProvider(st) + + err := p.LogEventSync(context.Background(), Event{ + Action: "admin.fga_tuples_written", + ActorType: "admin", + ActorID: "actor-1", + }) + + require.NoError(t, err) + require.NotNil(t, st.got) + assert.Equal(t, "admin.fga_tuples_written", st.got.Action) + assert.Equal(t, "actor-1", st.got.ActorID) +} + +func TestLogEventSync_ReturnsStorageError(t *testing.T) { + boom := errors.New("audit table unavailable") + p := newTestProvider(&fakeAuditStore{err: boom}) + + err := p.LogEventSync(context.Background(), Event{Action: "admin.fga_reset"}) + + require.Error(t, err) + assert.ErrorIs(t, err, boom, "the storage error must reach the caller unchanged") +} + +func TestLogEventSync_FoldsProtocolIntoMetadataLikeLogEvent(t *testing.T) { + st := &fakeAuditStore{} + p := newTestProvider(st) + + require.NoError(t, p.LogEventSync(context.Background(), Event{ + Action: "admin.fga_tuples_written", + Protocol: "grpc", + Metadata: `{"count":2}`, + })) + + require.NotNil(t, st.got) + // Both keys must survive: the protocol the caller set, and the metadata it + // already had. A separate builder for the sync path would drop one. + assert.JSONEq(t, `{"protocol":"grpc","count":2}`, st.got.Metadata) +} diff --git a/internal/service/admin_access_invite_test.go b/internal/service/admin_access_invite_test.go index 0b2bff832..7ba145861 100644 --- a/internal/service/admin_access_invite_test.go +++ b/internal/service/admin_access_invite_test.go @@ -91,6 +91,8 @@ type inviteAudit struct{ audit.Provider } func (inviteAudit) LogEvent(_ audit.Event) {} +func (inviteAudit) LogEventSync(_ context.Context, _ audit.Event) error { return nil } + func newInviteProvider(cfg *config.Config, st storage.Provider, tp token.Provider) *provider { log := zerolog.Nop() return &provider{ From 36de0f28df058a7575ed3ef45bb20f39832bd56d Mon Sep 17 00:00:00 2001 From: Lakhan Samani Date: Mon, 5 Oct 2026 12:36:45 +0530 Subject: [PATCH 2/5] feat(audit): record how a super-admin authenticated Super-admin is one shared AdminSecret with no per-admin identity, so an audit record cannot name a person. Recording the credential MODE is the honest alternative, and makes a shared-secret action distinguishable from a dashboard session. IsSuperAdmin is now a predicate over AdminAuthMode so the admit decision and the recorded mode cannot disagree. The session handle is never recorded - it is a live bearer credential and the dashboard renders this table. Also updates the gRPC interceptor's stubTokenProvider test double, which embeds token.Provider and overrides specific methods: it needed an AdminAuthMode override to match, since the interceptor now calls that instead of IsSuperAdmin. --- internal/authctx/principal.go | 16 +++++-- internal/constants/audit_event.go | 19 ++++++++ internal/grpcsrv/interceptors/auth.go | 4 +- internal/grpcsrv/interceptors/auth_test.go | 9 ++++ internal/token/admin_auth_mode_test.go | 50 +++++++++++++++++++++ internal/token/admin_token.go | 52 +++++++++++++--------- internal/token/provider.go | 7 +++ 7 files changed, 130 insertions(+), 27 deletions(-) create mode 100644 internal/token/admin_auth_mode_test.go diff --git a/internal/authctx/principal.go b/internal/authctx/principal.go index 6082217c2..dda3753c6 100644 --- a/internal/authctx/principal.go +++ b/internal/authctx/principal.go @@ -1,9 +1,10 @@ // Package authctx carries authentication principal details on context.Context. package authctx -import "strings" - -import "context" +import ( + "context" + "strings" +) type principalContextKey struct{} @@ -27,6 +28,15 @@ type Principal struct { // enforce per-operation scope for delegated callers. See // internal/delegatedscope. Scope []string + // AuthMode records HOW a super-admin caller authenticated — one of + // constants.AuditAuthModeAdminSession or + // constants.AuditAuthModeSharedSecret. Empty for non-admin callers. + // + // Super-admin has no per-admin identity (one shared AdminSecret), so an + // audit record cannot name a person. This records which credential was + // used instead of leaving the question blank. Never the session handle + // itself. + AuthMode string } // IsDelegated reports whether this principal is an agent acting for a user. diff --git a/internal/constants/audit_event.go b/internal/constants/audit_event.go index b9b60a0d6..4f594aeeb 100644 --- a/internal/constants/audit_event.go +++ b/internal/constants/audit_event.go @@ -29,6 +29,25 @@ const ( AuditActorTypeServiceAccount = "service_account" ) +// Audit auth-mode constants record HOW a super-admin authenticated. +// +// Super-admin is a single shared AdminSecret, or a session cookie derived from +// it — there is no per-admin identity, so an audit record cannot name a person. +// Recording the mode is the honest alternative: it says which credential was +// used, and makes a shared-secret action distinguishable from a dashboard +// session without inventing an identity the system does not have. +// +// The session HANDLE is never recorded. It is a live bearer credential and the +// dashboard renders the audit table. +const ( + // AuditAuthModeAdminSession means the caller presented a valid admin + // session cookie (dashboard login). + AuditAuthModeAdminSession = "admin_session" + // AuditAuthModeSharedSecret means the caller presented the + // x-authorizer-admin-secret header. + AuditAuthModeSharedSecret = "shared_secret" +) + // Audit resource type constants identify the type of resource affected by an auditable action. const ( // AuditResourceTypeUser represents a user entity. diff --git a/internal/grpcsrv/interceptors/auth.go b/internal/grpcsrv/interceptors/auth.go index b366ff7d4..49c90ad47 100644 --- a/internal/grpcsrv/interceptors/auth.go +++ b/internal/grpcsrv/interceptors/auth.go @@ -178,8 +178,8 @@ func Auth(tp token.Provider, log *zerolog.Logger, resolve TokenResolver) grpc.Un // is the only version of that guard that holds: merely skipping this // check would move it one layer down, since service.requireSuperAdmin // re-derives super-admin from meta.Request on its own. - if tp.IsSuperAdmin(gc) { - ctx = authctx.WithPrincipal(ctx, &authctx.Principal{IsSuperAdmin: true}) + if mode := tp.AdminAuthMode(gc); mode != "" { + ctx = authctx.WithPrincipal(ctx, &authctx.Principal{IsSuperAdmin: true, AuthMode: mode}) return handler(ctx, req) } diff --git a/internal/grpcsrv/interceptors/auth_test.go b/internal/grpcsrv/interceptors/auth_test.go index dfa62b3d1..ac57d7c37 100644 --- a/internal/grpcsrv/interceptors/auth_test.go +++ b/internal/grpcsrv/interceptors/auth_test.go @@ -39,6 +39,14 @@ func (s *stubTokenProvider) IsSuperAdmin(_ *gin.Context) bool { return s.superAdmin } +func (s *stubTokenProvider) AdminAuthMode(_ *gin.Context) string { + s.superAdminChecks++ + if s.superAdmin { + return constants.AuditAuthModeSharedSecret + } + return "" +} + func (s *stubTokenProvider) GetUserIDFromSessionOrAccessToken(_ *gin.Context) (*token.SessionOrAccessTokenData, error) { s.userChecks++ if s.tokenErr != nil { @@ -179,6 +187,7 @@ func TestAuth_AdminMethodRequiresSuperAdmin(t *testing.T) { require.True(t, ok) require.NotNil(t, p) assert.True(t, p.IsSuperAdmin) + assert.Equal(t, constants.AuditAuthModeSharedSecret, p.AuthMode) assert.Empty(t, p.UserID) return &authorizerv1.AdminMetaResponse{}, nil }) diff --git a/internal/token/admin_auth_mode_test.go b/internal/token/admin_auth_mode_test.go new file mode 100644 index 000000000..401f1dc49 --- /dev/null +++ b/internal/token/admin_auth_mode_test.go @@ -0,0 +1,50 @@ +package token + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/authorizerdev/authorizer/internal/constants" +) + +const adminSecretHeader = "x-authorizer-admin-secret" + +func TestAdminAuthMode_SharedSecretWhenHeaderMatches(t *testing.T) { + p := newProvider("correct-secret", false) + assert.Equal(t, constants.AuditAuthModeSharedSecret, + p.AdminAuthMode(newGinCtx(adminSecretHeader, "correct-secret"))) +} + +func TestAdminAuthMode_EmptyWhenNotSuperAdmin(t *testing.T) { + p := newProvider("correct-secret", false) + assert.Equal(t, "", p.AdminAuthMode(newGinCtx(adminSecretHeader, "wrong-secret"))) + assert.Equal(t, "", p.AdminAuthMode(newGinCtx(adminSecretHeader, ""))) + assert.Equal(t, "", p.AdminAuthMode(newGinCtx("", ""))) +} + +// IsSuperAdmin is re-expressed in terms of AdminAuthMode, so they must agree +// on every input. A divergence would mean a caller is admitted as super-admin +// while the audit record says they are not an admin at all. +func TestIsSuperAdmin_AgreesWithAdminAuthMode(t *testing.T) { + p := newProvider("correct-secret", false) + for _, secret := range []string{"correct-secret", "wrong-secret", ""} { + gc := newGinCtx(adminSecretHeader, secret) + assert.Equal(t, p.AdminAuthMode(gc) != "", p.IsSuperAdmin(gc), + "disagreement for secret %q", secret) + } +} + +// An unconfigured secret must never authenticate, and must never be recorded +// as though it had. +func TestAdminAuthMode_EmptyAdminSecretNeverReportsSharedSecret(t *testing.T) { + p := newProvider("", false) + assert.Equal(t, "", p.AdminAuthMode(newGinCtx(adminSecretHeader, ""))) + assert.Equal(t, "", p.AdminAuthMode(newGinCtx(adminSecretHeader, "anything"))) +} + +// With header auth disabled, even the correct secret is not super admin. +func TestAdminAuthMode_RespectsDisableAdminHeaderAuth(t *testing.T) { + p := newProvider("correct-secret", true) + assert.Equal(t, "", p.AdminAuthMode(newGinCtx(adminSecretHeader, "correct-secret"))) +} diff --git a/internal/token/admin_token.go b/internal/token/admin_token.go index afa9df28e..89aca9e72 100644 --- a/internal/token/admin_token.go +++ b/internal/token/admin_token.go @@ -3,6 +3,7 @@ package token import ( "fmt" + "github.com/authorizerdev/authorizer/internal/constants" "github.com/authorizerdev/authorizer/internal/cookie" "github.com/authorizerdev/authorizer/internal/utils" "github.com/gin-gonic/gin" @@ -37,29 +38,36 @@ func (p *provider) GetAdminAuthToken(gc *gin.Context) (string, error) { return sessionID, nil } -// IsSuperAdmin checks if user is super admin -func (p *provider) IsSuperAdmin(gc *gin.Context) bool { +// AdminAuthMode reports how the caller authenticated as super admin, or "" if +// they did not. This is the single place that decision is made; IsSuperAdmin +// is a thin predicate over it. +func (p *provider) AdminAuthMode(gc *gin.Context) string { token, err := p.GetAdminAuthToken(gc) - if err != nil { - if p.config.DisableAdminHeaderAuth { - return false - } - - // Reject header auth if no AdminSecret is configured — an unconfigured - // secret must never grant super-admin access. - if p.config.AdminSecret == "" { - return false - } - secret := gc.Request.Header.Get("x-authorizer-admin-secret") - if secret == "" { - return false - } - // Throttled: this header is an unauthenticated guess at the single - // highest-privilege credential in the system, and the only limiter in - // front of it used to be the shared 30rps budget ordinary traffic gets. - valid, _ := p.VerifyAdminSecret(utils.GetIP(gc.Request), secret) - return valid + if err == nil && token != "" { + return constants.AuditAuthModeAdminSession + } + if p.config.DisableAdminHeaderAuth { + return "" + } + // Reject header auth if no AdminSecret is configured — an unconfigured + // secret must never grant super-admin access. + if p.config.AdminSecret == "" { + return "" } + secret := gc.Request.Header.Get("x-authorizer-admin-secret") + if secret == "" { + return "" + } + // Throttled: this header is an unauthenticated guess at the single + // highest-privilege credential in the system, and the only limiter in + // front of it used to be the shared 30rps budget ordinary traffic gets. + if valid, _ := p.VerifyAdminSecret(utils.GetIP(gc.Request), secret); valid { + return constants.AuditAuthModeSharedSecret + } + return "" +} - return token != "" +// IsSuperAdmin checks if user is super admin +func (p *provider) IsSuperAdmin(gc *gin.Context) bool { + return p.AdminAuthMode(gc) != "" } diff --git a/internal/token/provider.go b/internal/token/provider.go index 2115687a8..1509952b6 100644 --- a/internal/token/provider.go +++ b/internal/token/provider.go @@ -75,6 +75,13 @@ type Provider interface { GetUserIDFromSessionOrAccessToken(gc *gin.Context) (*SessionOrAccessTokenData, error) // IsSuperAdmin checks if user is super admin IsSuperAdmin(gc *gin.Context) bool + // AdminAuthMode reports HOW the caller authenticated as super admin: + // constants.AuditAuthModeAdminSession, constants.AuditAuthModeSharedSecret, + // or "" when the caller is not a super admin at all. + // + // IsSuperAdmin is defined as AdminAuthMode(gc) != "", so the two cannot + // disagree about who is an admin. + AdminAuthMode(gc *gin.Context) string // ParseJWTToken parses jwt token ParseJWTToken(token string) (jwt.MapClaims, error) // SignJWTToken signs jwt token From fc7048f4d66b6e09313cb4c6493fed87f840c18d Mon Sep 17 00:00:00 2001 From: Lakhan Samani Date: Mon, 5 Oct 2026 12:42:41 +0530 Subject: [PATCH 3/5] test(token): cover the admin-session branch of AdminAuthMode The cookie-auth path (dashboard login) returning constants.AuditAuthModeAdminSession had no test reaching it - every existing test drove only the header/secret path, so the two mode constants could have been swapped on that branch without anything failing. That mapping is the one new behavior this task exists to produce. Adds a minimal memory_store.Provider fake (GetCache only, same embed-and-override pattern as interceptors.stubTokenProvider) and a session-backed provider fixture built on top of the existing newProvider helper, then asserts both AdminAuthMode and IsSuperAdmin agree on a valid session cookie. --- internal/token/admin_auth_mode_test.go | 60 ++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/internal/token/admin_auth_mode_test.go b/internal/token/admin_auth_mode_test.go index 401f1dc49..05f811464 100644 --- a/internal/token/admin_auth_mode_test.go +++ b/internal/token/admin_auth_mode_test.go @@ -1,15 +1,58 @@ package token import ( + "net/http" + "net/http/httptest" "testing" + "github.com/gin-gonic/gin" "github.com/stretchr/testify/assert" "github.com/authorizerdev/authorizer/internal/constants" + "github.com/authorizerdev/authorizer/internal/memory_store" ) const adminSecretHeader = "x-authorizer-admin-secret" +// fakeSessionStore answers GetCache only — the one memory_store.Provider +// method ValidateAdminSession calls. Embedding the real interface (nil) for +// everything else keeps this to exactly the method the admin-session path +// exercises, the same pattern interceptors.stubTokenProvider uses for +// token.Provider. +type fakeSessionStore struct { + memory_store.Provider + sessions map[string]string +} + +func (f *fakeSessionStore) GetCache(key string) (string, error) { + return f.sessions[key], nil +} + +// newSessionProvider reuses newProvider's config construction and wires in a +// live admin session for sessionID, so AdminAuthMode's cookie branch +// (ValidateAdminSession -> MemoryStoreProvider.GetCache) has something to hit. +func newSessionProvider(adminSecret, sessionID string) *provider { + p := newProvider(adminSecret, false) + p.dependencies = &Dependencies{ + MemoryStoreProvider: &fakeSessionStore{ + sessions: map[string]string{adminSessionCachePrefix + sessionID: "1"}, + }, + } + return p +} + +// newGinCtxWithAdminCookie builds a request carrying the admin session +// cookie, the way GetAdminAuthToken reads it. newGinCtx only sets headers, so +// this is a separate, equally small helper rather than an overload of it. +func newGinCtxWithAdminCookie(sessionID string) *gin.Context { + req := httptest.NewRequest(http.MethodPost, "/", nil) + req.AddCookie(&http.Cookie{Name: constants.AdminCookieName, Value: sessionID}) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = req + return c +} + func TestAdminAuthMode_SharedSecretWhenHeaderMatches(t *testing.T) { p := newProvider("correct-secret", false) assert.Equal(t, constants.AuditAuthModeSharedSecret, @@ -48,3 +91,20 @@ func TestAdminAuthMode_RespectsDisableAdminHeaderAuth(t *testing.T) { p := newProvider("correct-secret", true) assert.Equal(t, "", p.AdminAuthMode(newGinCtx(adminSecretHeader, "correct-secret"))) } + +// TestAdminAuthMode_AdminSessionWhenCookieValid covers the branch nothing +// else in this file reaches: a valid admin session cookie (dashboard login) +// must report AuditAuthModeAdminSession, not AuditAuthModeSharedSecret. Every +// other test here drives the header/secret path via newGinCtx, which never +// sets a cookie, so GetAdminAuthToken always failed and the session branch +// was never entered — if the two constants were swapped on this branch, or it +// returned the wrong one, nothing would fail. +func TestAdminAuthMode_AdminSessionWhenCookieValid(t *testing.T) { + const sessionID = "valid-session-id" + p := newSessionProvider("correct-secret", sessionID) + gc := newGinCtxWithAdminCookie(sessionID) + + assert.Equal(t, constants.AuditAuthModeAdminSession, p.AdminAuthMode(gc)) + // Pin the predicate and the mode together on this branch too. + assert.True(t, p.IsSuperAdmin(gc)) +} From 606a65a368a412833f2b798c65ddfde251ae6e91 Mon Sep 17 00:00:00 2001 From: Lakhan Samani Date: Mon, 5 Oct 2026 12:51:50 +0530 Subject: [PATCH 4/5] feat(scim): give the SCIM service an audit provider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit scim.Dependencies had no AuditProvider at all, so the package could not audit anything — an external IdP could provision users, create org memberships, revoke access and rewrite FGA group tuples leaving no audit row. That absence is structural, and blocks half the call sites in the next phase. Nil-safe, matching the EventsProvider convention. No events are emitted yet; the call sites land with the phase that needs them. --- cmd/root.go | 1 + internal/service/scim/audit_wiring_test.go | 59 ++++++++++++++++++++++ internal/service/scim/scim.go | 22 ++++++++ 3 files changed, 82 insertions(+) create mode 100644 internal/service/scim/audit_wiring_test.go diff --git a/cmd/root.go b/cmd/root.go index 743009d3d..912c0f75d 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -818,6 +818,7 @@ func runRoot(c *cobra.Command, args []string) { MemoryStoreProvider: memoryStoreProvider, AuthzEngine: authzEngine, EventsProvider: eventsProvider, + AuditProvider: auditProvider, }) scimHandler := scimhttp.New(&scimhttp.Dependencies{ Log: &log, diff --git a/internal/service/scim/audit_wiring_test.go b/internal/service/scim/audit_wiring_test.go new file mode 100644 index 000000000..a485eb7de --- /dev/null +++ b/internal/service/scim/audit_wiring_test.go @@ -0,0 +1,59 @@ +package scim + +import ( + "context" + "errors" + "testing" + + "github.com/rs/zerolog" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/authorizerdev/authorizer/internal/audit" +) + +type recordingAudit struct { + audit.Provider + got audit.Event + err error +} + +func (r *recordingAudit) LogEventSync(_ context.Context, e audit.Event) error { + if r.err != nil { + return r.err + } + r.got = e + return nil +} + +// scim's concrete type embeds Dependencies BY VALUE (`type provider struct { +// Dependencies }`), so fields are reached as p.AuditProvider, not p.deps.X. +func newAuditTestProvider(ap audit.Provider) *provider { + log := zerolog.Nop() + return &provider{Dependencies: Dependencies{Log: &log, AuditProvider: ap}} +} + +// A deployment that does not wire audit must still serve SCIM, matching the +// documented EventsProvider convention. +func TestLogAuditSync_NilProviderIsNoOp(t *testing.T) { + p := newAuditTestProvider(nil) + assert.NotPanics(t, func() { + require.NoError(t, p.logAuditSync(context.Background(), audit.Event{Action: "x"})) + }) +} + +func TestLogAuditSync_ForwardsEvent(t *testing.T) { + ra := &recordingAudit{} + p := newAuditTestProvider(ra) + + require.NoError(t, p.logAuditSync(context.Background(), audit.Event{Action: "scim.group_members_added"})) + + assert.Equal(t, "scim.group_members_added", ra.got.Action) +} + +func TestLogAuditSync_PropagatesError(t *testing.T) { + boom := errors.New("audit unavailable") + p := newAuditTestProvider(&recordingAudit{err: boom}) + + assert.ErrorIs(t, p.logAuditSync(context.Background(), audit.Event{Action: "x"}), boom) +} diff --git a/internal/service/scim/scim.go b/internal/service/scim/scim.go index 1b9440440..11bce1da0 100644 --- a/internal/service/scim/scim.go +++ b/internal/service/scim/scim.go @@ -23,6 +23,7 @@ import ( "golang.org/x/crypto/bcrypt" "github.com/authorizerdev/authorizer/internal/asyncutil" + "github.com/authorizerdev/authorizer/internal/audit" "github.com/authorizerdev/authorizer/internal/authorization/engine" "github.com/authorizerdev/authorizer/internal/constants" "github.com/authorizerdev/authorizer/internal/events" @@ -97,6 +98,27 @@ type Dependencies struct { // (user.provisioned/deprovisioned/scim_updated, group.created/updated/deleted). // Nil when webhooks are not wired — event firing is then a no-op. EventsProvider events.Provider + // AuditProvider records authorization-relevant SCIM operations. SCIM is + // driven by an external IdP, so its writes are the least supervised + // authorization changes in the system — and until this field existed the + // package could not audit at all. + // + // Nil when audit is not wired — logging is then a no-op, matching the + // EventsProvider convention above. + AuditProvider audit.Provider +} + +// logAuditSync records an audit event synchronously, returning the storage +// error. A nil AuditProvider is a no-op returning nil. +// +// Callers decide what a failure means: the SCIM group paths that treat a tuple +// write as non-fatal must treat a failed audit the same way, or an accepted +// partial failure becomes a failed deprovision. +func (p *provider) logAuditSync(ctx context.Context, event audit.Event) error { + if p.AuditProvider == nil { + return nil + } + return p.AuditProvider.LogEventSync(ctx, event) } // maxFilterScan bounds the org-member scan a non-indexed SCIM filter performs From c334d7fdd88d1a30e67fe3a13334bfdcb9c8e265 Mon Sep 17 00:00:00 2001 From: Lakhan Samani Date: Mon, 5 Oct 2026 13:04:18 +0530 Subject: [PATCH 5/5] test: close nil-panic hazards in embedded interface test doubles Three test doubles embed `token.Provider` or `audit.Provider` but override only a subset of methods. Embedding an interface compiles fine, but calling an overridden method on the unoverridden embedded pointer panics. This branch added two new interface methods: token.AdminAuthMode and audit.LogEventSync. Production code will call AdminAuthMode inside the requireSuperAdmin guard and LogEvent inside SCIM paths. Add minimal overrides to test doubles that would panic today: - inviteToken in admin_access_invite_test: add AdminAuthMode override consistent with existing IsSuperAdmin behaviour (returns admin_session mode) - recordingAudit in audit_wiring_test: add fire-and-forget LogEvent override - mcp_auth_test comment: reflect actual AdminAuthMode call, not IsSuperAdmin All three tests pass with these guards in place. --- internal/grpcsrv/interceptors/mcp_auth_test.go | 2 +- internal/service/admin_access_invite_test.go | 3 +++ internal/service/scim/audit_wiring_test.go | 2 ++ 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/internal/grpcsrv/interceptors/mcp_auth_test.go b/internal/grpcsrv/interceptors/mcp_auth_test.go index 38d6364d2..d01e49197 100644 --- a/internal/grpcsrv/interceptors/mcp_auth_test.go +++ b/internal/grpcsrv/interceptors/mcp_auth_test.go @@ -134,7 +134,7 @@ func TestMCPTokenResolver(t *testing.T) { // that resolver is the ONLY way in. // // Two paths in the interceptor authenticate without consulting any resolver — -// tp.IsSuperAdmin (an admin cookie or the x-authorizer-admin-secret header) and +// tp.AdminAuthMode (an admin cookie or the x-authorizer-admin-secret header) and // the Session RPC's cookie-only branch. transport.MetaFromGRPC reconstructs // cookies from gRPC metadata, so on the MCP server those would be reachable the // moment the HTTP bridge forwarded request headers. Before this guard the diff --git a/internal/service/admin_access_invite_test.go b/internal/service/admin_access_invite_test.go index 7ba145861..dc7a23b8e 100644 --- a/internal/service/admin_access_invite_test.go +++ b/internal/service/admin_access_invite_test.go @@ -15,6 +15,7 @@ import ( "github.com/authorizerdev/authorizer/internal/audit" "github.com/authorizerdev/authorizer/internal/config" + "github.com/authorizerdev/authorizer/internal/constants" "github.com/authorizerdev/authorizer/internal/email" "github.com/authorizerdev/authorizer/internal/graph/model" "github.com/authorizerdev/authorizer/internal/refs" @@ -76,6 +77,8 @@ type inviteToken struct { func (inviteToken) IsSuperAdmin(_ *gin.Context) bool { return true } +func (inviteToken) AdminAuthMode(_ *gin.Context) string { return constants.AuditAuthModeAdminSession } + func (tp inviteToken) CreateVerificationToken(cfg *token.AuthTokenConfig, _ string, _ string) (string, error) { if tp.createVerificationToken != nil { return tp.createVerificationToken(cfg) diff --git a/internal/service/scim/audit_wiring_test.go b/internal/service/scim/audit_wiring_test.go index a485eb7de..c59b661b3 100644 --- a/internal/service/scim/audit_wiring_test.go +++ b/internal/service/scim/audit_wiring_test.go @@ -18,6 +18,8 @@ type recordingAudit struct { err error } +func (recordingAudit) LogEvent(_ audit.Event) {} + func (r *recordingAudit) LogEventSync(_ context.Context, e audit.Event) error { if r.err != nil { return r.err