diff --git a/internal/constants/audit_event.go b/internal/constants/audit_event.go index a9d82fc09..b9b60a0d6 100644 --- a/internal/constants/audit_event.go +++ b/internal/constants/audit_event.go @@ -78,7 +78,9 @@ const ( AuditSignupEvent = "user.signup" // AuditLogoutEvent is logged when a user logs out. AuditLogoutEvent = "user.logout" - // AuditPasswordChangedEvent is logged when a user changes their password. + // AuditPasswordChangedEvent is logged when a user changes their own password + // via UpdateProfile. Emitted IN ADDITION to AuditProfileUpdatedEvent, so a + // password change is findable without reading every profile edit. AuditPasswordChangedEvent = "user.password_changed" // AuditPasswordResetEvent is logged when a user resets their password via token or OTP. AuditPasswordResetEvent = "user.password_reset" @@ -96,7 +98,10 @@ const ( AuditWebauthnCredentialDeletedEvent = "user.webauthn_credential_deleted" // AuditMFAEnabledEvent is logged when a user enables multi-factor authentication. AuditMFAEnabledEvent = "user.mfa_enabled" - // AuditMFADisabledEvent is logged when a user disables multi-factor authentication. + // AuditMFADisabledEvent is logged when a user disables multi-factor + // authentication via UpdateProfile. Emitted IN ADDITION to + // AuditProfileUpdatedEvent — turning MFA off is a security event, not a + // profile edit. AuditMFADisabledEvent = "user.mfa_disabled" // AuditMFALockedEvent is logged when a user locks their own account // after losing access to their MFA factor(s). @@ -122,7 +127,11 @@ const ( AuditAdminLoginFailedEvent = "admin.login_failed" // AuditAdminLogoutEvent is logged when an admin logs out. AuditAdminLogoutEvent = "admin.logout" - // AuditAdminUserCreatedEvent is logged when an admin creates a user. + // AuditAdminUserCreatedEvent: RESERVED, never emitted. There is no admin + // create-user operation — AdminProvider exposes Users/User/UpdateUser/DeleteUser + // and creation happens through the invite flow, which logs + // AuditAdminInviteSentEvent. Kept so a future create path has a name; it is not + // evidence of a missing log call. AuditAdminUserCreatedEvent = "admin.user_created" // AuditAdminUserUpdatedEvent is logged when an admin updates a user. AuditAdminUserUpdatedEvent = "admin.user_updated" @@ -134,7 +143,7 @@ const ( AuditAdminAccessEnabledEvent = "admin.access_enabled" // AuditAdminInviteSentEvent is logged when an admin sends a user invitation. AuditAdminInviteSentEvent = "admin.invite_sent" - // AuditAdminConfigChangedEvent is logged when an admin modifies server configuration. + // AuditAdminConfigChangedEvent: RESERVED, never emitted — server configuration is CLI-flag driven in v2 and cannot be modified at runtime, so nothing can emit this. AuditAdminConfigChangedEvent = "admin.config_changed" // AuditAdminWebhookCreatedEvent is logged when an admin creates a webhook. AuditAdminWebhookCreatedEvent = "admin.webhook_created" @@ -168,7 +177,7 @@ const ( AuditSSOLoginInitiatedEvent = "sso.login_initiated" // AuditSSOCallbackSuccessEvent is logged when an org OIDC SSO callback succeeds. AuditSSOCallbackSuccessEvent = "sso.callback_success" - // AuditSSOCallbackFailedEvent is logged when an org OIDC SSO callback fails. + // AuditSSOCallbackFailedEvent: RESERVED, never emitted — the org OIDC callback currently records failures in logs and metrics only. AuditSSOCallbackFailedEvent = "sso.callback_failed" // AuditSAMLLoginInitiatedEvent is logged when an org SAML SP login is started. AuditSAMLLoginInitiatedEvent = "saml.login_initiated" @@ -199,7 +208,7 @@ const ( // AuditTokenRevokedEvent is logged when a token is revoked. AuditTokenRevokedEvent = "token.revoked" - // AuditSessionCreatedEvent is logged when a new session is created. + // AuditSessionCreatedEvent: RESERVED, never emitted — session creation is covered by the login/signup events that cause it. AuditSessionCreatedEvent = "session.created" // AuditSessionTerminatedEvent is logged when a session is terminated. AuditSessionTerminatedEvent = "session.terminated" @@ -212,11 +221,11 @@ const ( AuditClientDeletedEvent = "admin.client_deleted" // AuditClientSecretRotatedEvent is logged when a client secret is rotated. AuditClientSecretRotatedEvent = "admin.client_secret_rotated" - // AuditClientDeactivatedEvent is logged when an admin disables a client. + // AuditClientDeactivatedEvent: RESERVED, never emitted — client activation state is changed through UpdateClient, which logs the generic admin client-updated event. // Distinct from the generic update event so incident responders can query the kill-switch // signal directly without scanning all update payloads. AuditClientDeactivatedEvent = "admin.client_deactivated" - // AuditClientActivatedEvent is logged when an admin re-enables a client. + // AuditClientActivatedEvent: RESERVED, never emitted — as above. AuditClientActivatedEvent = "admin.client_activated" // AuditOrgOIDCConnectionCreatedEvent is logged when an admin creates an org OIDC connection. @@ -237,7 +246,7 @@ const ( AuditTrustedIssuerUpdatedEvent = "admin.trusted_issuer_updated" // AuditTrustedIssuerDeletedEvent is logged when an admin deletes a trusted issuer. AuditTrustedIssuerDeletedEvent = "admin.trusted_issuer_deleted" - // AuditTrustedIssuerTokenReviewChangedEvent is logged when EnableTokenReview is toggled. + // AuditTrustedIssuerTokenReviewChangedEvent: RESERVED, never emitted — UpdateTrustedIssuer logs a single admin trusted-issuer-updated event covering every field. // Downgrading from online (true) to offline (false) is a security-posture change and // must be queryable independently of generic trusted_issuer_updated events. AuditTrustedIssuerTokenReviewChangedEvent = "admin.trusted_issuer_token_review_changed" @@ -315,7 +324,9 @@ const ( // which is the opposite of what a delegation surface needs: the whole point of // the act chain is that an agent's activity is attributable. AuditTokenExchangeFailedEvent = "token.exchange_failed" - // AuditWorkloadAuthEvent is logged when a workload authenticates via client_assertion - // (K8s SA token, SPIFFE JWT-SVID, or generic OIDC workload token). + // AuditWorkloadAuthEvent: RESERVED, never emitted — a workload authenticating + // via client_assertion (K8s SA token, SPIFFE JWT-SVID, or generic OIDC + // workload token) succeeds through the client_credentials path, which logs + // AuditTokenClientCredentialsEvent. AuditWorkloadAuthEvent = "token.workload_auth" ) diff --git a/internal/constants/grant_types.go b/internal/constants/grant_types.go index 30db37443..a82eebcf1 100644 --- a/internal/constants/grant_types.go +++ b/internal/constants/grant_types.go @@ -33,17 +33,15 @@ const ( ClientAssertionTypeJWTSPIFFE = "urn:ietf:params:oauth:client-assertion-type:jwt-spiffe" ) -// RFC 8693 token type URNs used in subject_token_type and issued_token_type. +// RFC 8693 token type URNs. Only the two this server ACCEPTS are listed — see +// isSupportedExchangeTokenType. The refresh_token and id_token URNs the RFC also +// defines were declared here and never referenced; add them back alongside the +// code that accepts them, so the list stays a statement about this server rather +// than a copy of the registry. const ( // TokenTypeURNAccessToken identifies an OAuth2 access token. TokenTypeURNAccessToken = "urn:ietf:params:oauth:token-type:access_token" - // TokenTypeURNRefreshToken identifies an OAuth2 refresh token. - TokenTypeURNRefreshToken = "urn:ietf:params:oauth:token-type:refresh_token" - - // TokenTypeURNIDToken identifies an OpenID Connect ID token. - TokenTypeURNIDToken = "urn:ietf:params:oauth:token-type:id_token" - // TokenTypeURNJWT identifies a generic JWT. TokenTypeURNJWT = "urn:ietf:params:oauth:token-type:jwt" ) diff --git a/internal/constants/oauth_info_urls.go b/internal/constants/oauth_info_urls.go index 9170dab3e..aff5b39b5 100644 --- a/internal/constants/oauth_info_urls.go +++ b/internal/constants/oauth_info_urls.go @@ -1,9 +1,6 @@ package constants const ( - // Ref: https://github.com/qor/auth/blob/master/providers/google/google.go - // deprecated and not used. instead we follow open id approach for google login - GoogleUserInfoURL = "https://www.googleapis.com/oauth2/v3/userinfo" // Ref: https://github.com/qor/auth/blob/master/providers/facebook/facebook.go#L18 FacebookUserInfoURL = "https://graph.facebook.com/me?fields=id,first_name,last_name,name,email,picture&access_token=" // Ref: https://docs.github.com/en/developers/apps/building-github-apps/identifying-and-authorizing-users-for-github-apps#3-your-github-app-accesses-the-api-with-the-users-access-token @@ -54,7 +51,4 @@ const ( // flat object (id/username/avatar/email at the top level), unlike // /oauth2/@me's nested `{"user": {...}}` shape. DiscordUserInfoURL = "https://discord.com/api/users/@me" - // Get microsoft user info. - // Ref: https://learn.microsoft.com/en-us/azure/active-directory/develop/userinfo - MicrosoftUserInfoURL = "https://graph.microsoft.com/oidc/userinfo" ) diff --git a/internal/constants/webhook_event_authorization.go b/internal/constants/webhook_event_authorization.go deleted file mode 100644 index 059ed8d07..000000000 --- a/internal/constants/webhook_event_authorization.go +++ /dev/null @@ -1,31 +0,0 @@ -package constants - -const ( - // ResourceCreatedWebhookEvent is fired when an authorization resource is created. - ResourceCreatedWebhookEvent = "resource.created" - // ResourceUpdatedWebhookEvent is fired when an authorization resource is updated. - ResourceUpdatedWebhookEvent = "resource.updated" - // ResourceDeletedWebhookEvent is fired when an authorization resource is deleted. - ResourceDeletedWebhookEvent = "resource.deleted" - // ScopeCreatedWebhookEvent is fired when an authorization scope is created. - ScopeCreatedWebhookEvent = "scope.created" - // ScopeUpdatedWebhookEvent is fired when an authorization scope is updated. - ScopeUpdatedWebhookEvent = "scope.updated" - // ScopeDeletedWebhookEvent is fired when an authorization scope is deleted. - ScopeDeletedWebhookEvent = "scope.deleted" - // PolicyCreatedWebhookEvent is fired when an authorization policy is created. - PolicyCreatedWebhookEvent = "policy.created" - // PolicyUpdatedWebhookEvent is fired when an authorization policy is updated. - PolicyUpdatedWebhookEvent = "policy.updated" - // PolicyDeletedWebhookEvent is fired when an authorization policy is deleted. - PolicyDeletedWebhookEvent = "policy.deleted" - // PermissionCreatedWebhookEvent is fired when an authorization permission is created. - PermissionCreatedWebhookEvent = "permission.created" - // PermissionUpdatedWebhookEvent is fired when an authorization permission is updated. - PermissionUpdatedWebhookEvent = "permission.updated" - // PermissionDeletedWebhookEvent is fired when an authorization permission is deleted. - PermissionDeletedWebhookEvent = "permission.deleted" - // PermissionCheckDeniedWebhookEvent is fired when a permission check is denied - // in enforcing mode. Useful for agent kill-switches and security alerting. - PermissionCheckDeniedWebhookEvent = "permission.check_denied" -) diff --git a/internal/grpcsrv/interceptors/interceptors_test.go b/internal/grpcsrv/interceptors/interceptors_test.go index 85ef8cbda..141567b0a 100644 --- a/internal/grpcsrv/interceptors/interceptors_test.go +++ b/internal/grpcsrv/interceptors/interceptors_test.go @@ -144,7 +144,7 @@ func TestValidate_NonProtoRequestPassesThrough(t *testing.T) { require.NoError(t, err, "non-proto requests must not be rejected by the validator") } -// TestValidate_PreservesInvariant guards against regressions where someone +// TestValidate_BuildsCleanly guards against regressions where someone // makes Validate() return a non-functional middleware (e.g. by reordering // the protovalidate.New() call). If the validator itself fails to build, // callers must learn about it at startup, not at first request. diff --git a/internal/integration_tests/profile_security_audit_test.go b/internal/integration_tests/profile_security_audit_test.go new file mode 100644 index 000000000..31d86e671 --- /dev/null +++ b/internal/integration_tests/profile_security_audit_test.go @@ -0,0 +1,90 @@ +package integration_tests + +import ( + "testing" + "time" + + "github.com/google/uuid" + "github.com/stretchr/testify/require" + + "github.com/authorizerdev/authorizer/internal/constants" + "github.com/authorizerdev/authorizer/internal/graph/model" + "github.com/authorizerdev/authorizer/internal/refs" +) + +// A password change and an MFA disable are security events. Both were folded +// into AuditProfileUpdatedEvent, indistinguishable from a display-name edit — +// while AuditPasswordChangedEvent and AuditMFADisabledEvent sat declared and +// never emitted. Anyone auditing "who changed a password" had nothing to query. +func TestProfileSecurityEventsAreAuditedSeparately(t *testing.T) { + cfg := getTestConfig() + ts := initTestSetup(t, cfg) + _, ctx := createContext(ts) + + email := "profile_audit_" + uuid.NewString() + "@authorizer.dev" + const 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) + require.NotNil(t, loginRes.AccessToken) + ts.GinContext.Request.Header.Set("Authorization", "Bearer "+*loginRes.AccessToken) + + awaitAudit := func(t *testing.T, action, userID string) { + t.Helper() + require.Eventually(t, func() bool { + logs, _, lErr := ts.StorageProvider.ListAuditLogs(ctx, + &model.Pagination{Limit: 50, Page: 1}, + map[string]interface{}{"action": action}) + if lErr != nil { + return false + } + for _, l := range logs { + if l.ActorID == userID { + return true + } + } + return false + }, 5*time.Second, 25*time.Millisecond, + "no %s audit entry for the user that performed it", action) + } + + user, err := ts.StorageProvider.GetUserByEmail(ctx, email) + require.NoError(t, err) + + t.Run("a password change is recorded as its own event", func(t *testing.T) { + newPassword := "Password@1234" + _, err := ts.GraphQLProvider.UpdateProfile(ctx, &model.UpdateProfileRequest{ + OldPassword: refs.NewStringRef(password), + NewPassword: refs.NewStringRef(newPassword), + ConfirmNewPassword: refs.NewStringRef(newPassword), + }) + require.NoError(t, err) + + awaitAudit(t, constants.AuditPasswordChangedEvent, user.ID) + // The generic event still fires, so nothing consuming it today breaks. + awaitAudit(t, constants.AuditProfileUpdatedEvent, user.ID) + }) + + t.Run("disabling MFA is recorded as its own event", func(t *testing.T) { + // Enable directly in storage: the enable path has its own gating, and the + // event under test is the DISABLE. + u, gErr := ts.StorageProvider.GetUserByEmail(ctx, email) + require.NoError(t, gErr) + u.IsMultiFactorAuthEnabled = refs.NewBoolRef(true) + _, uErr := ts.StorageProvider.UpdateUser(ctx, u) + require.NoError(t, uErr) + + _, err := ts.GraphQLProvider.UpdateProfile(ctx, &model.UpdateProfileRequest{ + IsMultiFactorAuthEnabled: refs.NewBoolRef(false), + }) + require.NoError(t, err) + + awaitAudit(t, constants.AuditMFADisabledEvent, user.ID) + }) +} diff --git a/internal/mcp/schema_test.go b/internal/mcp/schema_test.go index 67ad830b6..f39dba9af 100644 --- a/internal/mcp/schema_test.go +++ b/internal/mcp/schema_test.go @@ -66,7 +66,7 @@ func TestSchemaForMessage_CycleSafe(t *testing.T) { assert.Equal(t, "object", schema.Type) } -// TestSchemaForMessage_ScalarOnly walks a request that's purely scalars +// TestSchemaForMessage_AllScalarKinds walks a request that's purely scalars // (no nested message). Profile takes no arguments at all; Session takes // a few list-of-string + nested PermissionInput. func TestSchemaForMessage_AllScalarKinds(t *testing.T) { diff --git a/internal/memory_store/db/cache.go b/internal/memory_store/db/cache.go index 18b4ac916..c24ba3770 100644 --- a/internal/memory_store/db/cache.go +++ b/internal/memory_store/db/cache.go @@ -198,7 +198,7 @@ func (p *provider) SetCacheNX(key string, value string, ttlSeconds int64) (bool, // // - The storage layer cannot distinguish "no such row" from "read failed" — // GetSessionTokenByUserIDAndKey returns a plain error for both, and each of -// the 7 providers returns a different one (gorm.ErrRecordNotFound, +// the 6 providers returns a different one (gorm.ErrRecordNotFound, // gocql.ErrNotFound, a bare errors.New, a gocb error...). Propagating the // error would turn every ordinary cache miss into a failure. // - Both replay callers already discard it: saml_sp.go only reports a replay @@ -254,7 +254,7 @@ func (p *provider) GetCache(key string) (string, error) { // approximately right instead of reliably wrong. For an exact distributed // counter configure REDIS_URL (the Redis provider uses a native atomic INCR); // the upgrade path here is a compare-and-set or native increment in the storage -// layer, which is a per-provider change across all 7 backends. +// layer, which is a per-provider change across all 6 backends. func (p *provider) IncrementCache(key string, ttlSeconds int64) (int64, error) { incrementMutex.Lock() defer incrementMutex.Unlock() @@ -292,7 +292,7 @@ func (p *provider) IncrementCache(key string, ttlSeconds int64) (int64, error) { // indexed. An exact-key delete rides the (user_id, key_name) index instead. // // If a caller ever genuinely needs prefix expansion here it will silently no-op, -// so: add a prefix-delete to storage.Provider (all 7 backends) and implement it +// so: add a prefix-delete to storage.Provider (all 6 backends) and implement it // properly rather than reaching for GetAllSessionTokens. func (p *provider) DeleteCacheByPrefix(prefix string) error { ctx := context.Background() diff --git a/internal/service/magic_link_login.go b/internal/service/magic_link_login.go index b9822d87b..de2a1612c 100644 --- a/internal/service/magic_link_login.go +++ b/internal/service/magic_link_login.go @@ -10,6 +10,7 @@ import ( "github.com/authorizerdev/authorizer/internal/audit" "github.com/authorizerdev/authorizer/internal/constants" "github.com/authorizerdev/authorizer/internal/graph/model" + "github.com/authorizerdev/authorizer/internal/metrics" "github.com/authorizerdev/authorizer/internal/refs" "github.com/authorizerdev/authorizer/internal/storage/schemas" "github.com/authorizerdev/authorizer/internal/token" @@ -225,6 +226,10 @@ func (p *provider) MagicLinkLogin(ctx context.Context, meta RequestMetadata, par }) } + // Metered like every other authentication entry point. EventMagicLink was + // declared and never recorded, so magic-link volume was invisible next to + // login/signup/oauth on the same dashboard. + metrics.RecordAuthEvent(metrics.EventMagicLink, metrics.StatusSuccess) p.AuditProvider.LogEvent(audit.Event{ Action: constants.AuditMagicLinkRequestedEvent, Protocol: meta.Protocol, ActorID: user.ID, diff --git a/internal/service/update_profile.go b/internal/service/update_profile.go index 7c9fe5899..7e3d04f2e 100644 --- a/internal/service/update_profile.go +++ b/internal/service/update_profile.go @@ -100,6 +100,7 @@ func (p *provider) UpdateProfile(ctx context.Context, meta RequestMetadata, para appDataString = string(appDataBytes) user.AppData = &appDataString } + mfaDisabled := false // Check if the user is trying to enable or disable multi-factor authentication (MFA) if params.IsMultiFactorAuthEnabled != nil && refs.BoolValue(user.IsMultiFactorAuthEnabled) != refs.BoolValue(params.IsMultiFactorAuthEnabled) { // Only gate the enable action; disabling is always allowed (subject to the @@ -115,6 +116,12 @@ func (p *provider) UpdateProfile(ctx context.Context, meta RequestMetadata, para return nil, nil, FailedPrecondition("cannot disable multi factor authentication as it is enforced by organization") } + // Disabling MFA is a security-relevant action in its own right, not a + // profile edit. Recorded below as AuditMFADisabledEvent so it is findable + // without reading every profile update; enabling is already covered by the + // MFA setup flow's own events. + mfaDisabled = refs.BoolValue(user.IsMultiFactorAuthEnabled) && + !refs.BoolValue(params.IsMultiFactorAuthEnabled) user.IsMultiFactorAuthEnabled = params.IsMultiFactorAuthEnabled } @@ -278,6 +285,29 @@ func (p *provider) UpdateProfile(ctx context.Context, meta RequestMetadata, para IPAddress: meta.IPAddress, UserAgent: meta.UserAgent, })) + // A password change and an MFA disable are security events, and folding them + // into AuditProfileUpdatedEvent made them indistinguishable from a display-name + // edit — the constants for both existed and nothing ever emitted them. Logged + // IN ADDITION to the generic event so nothing that consumes + // user.profile_updated today stops seeing these updates. + securityEvent := func(action string) { + p.AuditProvider.LogEvent(applyDelegationActor(tokenData.ActorID, audit.Event{ + Action: action, + Protocol: meta.Protocol, ActorID: user.ID, + ActorType: constants.AuditActorTypeUser, + ActorEmail: refs.StringValue(user.Email), + ResourceType: constants.AuditResourceTypeUser, + ResourceID: user.ID, + IPAddress: meta.IPAddress, + UserAgent: meta.UserAgent, + })) + } + if isPasswordChanging { + securityEvent(constants.AuditPasswordChangedEvent) + } + if mfaDisabled { + securityEvent(constants.AuditMFADisabledEvent) + } message := `Profile details updated successfully.` if hasEmailChanged { message += `For the email change we have sent new verification email, please verify and continue` diff --git a/internal/service/verify_otp.go b/internal/service/verify_otp.go index 9eb3fe8aa..f15961762 100644 --- a/internal/service/verify_otp.go +++ b/internal/service/verify_otp.go @@ -301,6 +301,9 @@ func (p *provider) VerifyOTP(ctx context.Context, meta RequestMetadata, params * loginMethod = constants.AuthRecipeMethodMobileOTP } if isEmailVerification { + // EventVerifyOTP was declared and asserted by a test, but never recorded + // in production — OTP verification volume and failure rate were invisible. + metrics.RecordAuthEvent(metrics.EventVerifyOTP, metrics.StatusSuccess) p.AuditProvider.LogEvent(audit.Event{ Action: constants.AuditEmailVerifiedEvent, Protocol: meta.Protocol, ActorID: user.ID, @@ -312,6 +315,7 @@ func (p *provider) VerifyOTP(ctx context.Context, meta RequestMetadata, params * UserAgent: meta.UserAgent, }) } else { + metrics.RecordAuthEvent(metrics.EventVerifyOTP, metrics.StatusSuccess) p.AuditProvider.LogEvent(audit.Event{ Action: constants.AuditPhoneVerifiedEvent, Protocol: meta.Protocol, ActorID: user.ID,