From d96838d414e81e79119f70e6d2c0fe8286eb1c40 Mon Sep 17 00:00:00 2001 From: vatsalpatel Date: Thu, 23 Apr 2026 18:36:29 +0530 Subject: [PATCH] fix: capture IP address in all audit log entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `ip_address` column in `auth.audit_log_entries` was empty for most events — login, signup, logout, OAuth, password change, token refresh — because callers of `NewAuditLogEntry` passed `""` for the ipAddress parameter. Only passkey, MFA, and one admin path populated it. Extends the IP tracking cleanup from #2341: - Drop the `ipAddress` parameter from `NewAuditLogEntry`. The function already takes `*http.Request`, and `utilities.GetIPAddress` is the only correct source (handles Sb-Forwarded-For + X-Forwarded-For). - Update all 56 call sites across 23 files. - Add regression tests for `RemoteAddr` and `X-Forwarded-For` paths. Restores usability of the audit log for IP-based security forensics. --- internal/api/admin.go | 10 +-- internal/api/external.go | 6 +- internal/api/identity.go | 2 +- internal/api/invite.go | 2 +- internal/api/logout.go | 2 +- internal/api/magic_link.go | 2 +- internal/api/mail.go | 4 +- internal/api/mfa.go | 18 ++--- internal/api/oauthserver/handlers.go | 4 +- internal/api/otp.go | 2 +- internal/api/passkey_admin.go | 3 +- internal/api/passkey_authentication.go | 2 +- internal/api/passkey_manage.go | 4 +- internal/api/passkey_registration.go | 2 +- internal/api/reauthenticate.go | 2 +- internal/api/recover.go | 2 +- internal/api/resend.go | 4 +- internal/api/signup.go | 12 +-- internal/api/token.go | 4 +- internal/api/user.go | 4 +- internal/api/verify.go | 12 +-- internal/api/web3.go | 4 +- internal/models/audit_log_entry.go | 3 +- internal/models/audit_log_entry_test.go | 98 +++++++++++++++++++++++++ internal/models/refresh_token.go | 2 +- internal/tokens/service.go | 4 +- 26 files changed, 156 insertions(+), 58 deletions(-) create mode 100644 internal/models/audit_log_entry_test.go diff --git a/internal/api/admin.go b/internal/api/admin.go index e75fbb3537..53989477ce 100644 --- a/internal/api/admin.go +++ b/internal/api/admin.go @@ -304,7 +304,7 @@ func (a *API) adminUserUpdate(w http.ResponseWriter, r *http.Request) error { } } - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, adminUser, models.UserModifiedAction, "", map[string]interface{}{ + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, adminUser, models.UserModifiedAction, map[string]interface{}{ "user_id": user.ID, "user_email": user.Email, "user_phone": user.Phone, @@ -457,7 +457,7 @@ func (a *API) adminUserCreate(w http.ResponseWriter, r *http.Request) error { user.Identities = identities - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, adminUser, models.UserSignedUpAction, "", map[string]interface{}{ + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, adminUser, models.UserSignedUpAction, map[string]interface{}{ "user_id": user.ID, "user_email": user.Email, "user_phone": user.Phone, @@ -527,7 +527,7 @@ func (a *API) adminUserDelete(w http.ResponseWriter, r *http.Request) error { } err := db.Transaction(func(tx *storage.Connection) error { - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, adminUser, models.UserDeletedAction, "", map[string]interface{}{ + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, adminUser, models.UserDeletedAction, map[string]interface{}{ "user_id": user.ID, "user_email": user.Email, "user_phone": user.Phone, @@ -579,7 +579,7 @@ func (a *API) adminUserDeleteFactor(w http.ResponseWriter, r *http.Request) erro db := a.db.WithContext(ctx) err := db.Transaction(func(tx *storage.Connection) error { - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.DeleteFactorAction, utilities.GetIPAddress(r), map[string]interface{}{ + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.DeleteFactorAction, map[string]interface{}{ "user_id": user.ID, "factor_id": factor.ID, }); terr != nil { @@ -633,7 +633,7 @@ func (a *API) adminUserUpdateFactor(w http.ResponseWriter, r *http.Request) erro } } - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, adminUser, models.UpdateFactorAction, "", map[string]interface{}{ + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, adminUser, models.UpdateFactorAction, map[string]interface{}{ "user_id": user.ID, "factor_id": factor.ID, "factor_type": factor.FactorType, diff --git a/internal/api/external.go b/internal/api/external.go index 8d3fb18f4a..51eb00c7ca 100644 --- a/internal/api/external.go +++ b/internal/api/external.go @@ -400,7 +400,7 @@ func (a *API) createAccountFromExternalIdentity(tx *storage.Connection, r *http. return 0, nil, apierrors.NewInternalServerError("Error updating user").WithInternalError(terr) } if decision.CandidateEmail.Verified || config.Mailer.Autoconfirm { - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserSignedUpAction, "", map[string]interface{}{ + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserSignedUpAction, map[string]interface{}{ "provider": providerType, }); terr != nil { return 0, nil, terr @@ -436,7 +436,7 @@ func (a *API) createAccountFromExternalIdentity(tx *storage.Connection, r *http. } } } else { - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.LoginAction, "", map[string]interface{}{ + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.LoginAction, map[string]interface{}{ "provider": providerType, }); terr != nil { return 0, nil, terr @@ -491,7 +491,7 @@ func (a *API) processInvite(r *http.Request, tx *storage.Connection, userData *p return nil, apierrors.NewInternalServerError("Database error updating user").WithInternalError(err) } - if err := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.InviteAcceptedAction, "", map[string]interface{}{ + if err := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.InviteAcceptedAction, map[string]interface{}{ "provider": providerType, }); err != nil { return nil, err diff --git a/internal/api/identity.go b/internal/api/identity.go index c125aff1b0..195635aec0 100644 --- a/internal/api/identity.go +++ b/internal/api/identity.go @@ -53,7 +53,7 @@ func (a *API) DeleteIdentity(w http.ResponseWriter, r *http.Request) error { provider := identityToBeDeleted.Provider err = db.Transaction(func(tx *storage.Connection) error { - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.IdentityUnlinkAction, "", map[string]interface{}{ + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.IdentityUnlinkAction, map[string]interface{}{ "identity_id": identityToBeDeleted.ID, "provider": identityToBeDeleted.Provider, "provider_id": identityToBeDeleted.ProviderID, diff --git a/internal/api/invite.go b/internal/api/invite.go index fd26297700..f31ccda7c5 100644 --- a/internal/api/invite.go +++ b/internal/api/invite.go @@ -83,7 +83,7 @@ func (a *API) Invite(w http.ResponseWriter, r *http.Request) error { user.Identities = []models.Identity{*identity} } - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, adminUser, models.UserInvitedAction, "", map[string]interface{}{ + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, adminUser, models.UserInvitedAction, map[string]interface{}{ "user_id": user.ID, "user_email": user.Email, }); terr != nil { diff --git a/internal/api/logout.go b/internal/api/logout.go index 310b7c0d60..a014c68310 100644 --- a/internal/api/logout.go +++ b/internal/api/logout.go @@ -44,7 +44,7 @@ func (a *API) Logout(w http.ResponseWriter, r *http.Request) error { u := getUser(ctx) err := db.Transaction(func(tx *storage.Connection) error { - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, u, models.LogoutAction, "", nil); terr != nil { + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, u, models.LogoutAction, nil); terr != nil { return terr } diff --git a/internal/api/magic_link.go b/internal/api/magic_link.go index 2393059deb..4523d54333 100644 --- a/internal/api/magic_link.go +++ b/internal/api/magic_link.go @@ -136,7 +136,7 @@ func (a *API) MagicLink(w http.ResponseWriter, r *http.Request) error { } err = db.Transaction(func(tx *storage.Connection) error { - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserRecoveryRequestedAction, "", nil); terr != nil { + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserRecoveryRequestedAction, nil); terr != nil { return terr } return a.sendMagicLink(r, tx, user, flowType) diff --git a/internal/api/mail.go b/internal/api/mail.go index 87c4841c34..6270cf37a4 100644 --- a/internal/api/mail.go +++ b/internal/api/mail.go @@ -144,7 +144,7 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { var terr error switch params.Type { case mail.MagicLinkVerification, mail.RecoveryVerification: - if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserRecoveryRequestedAction, "", nil); terr != nil { + if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserRecoveryRequestedAction, nil); terr != nil { return terr } user.RecoveryToken = hashedToken @@ -180,7 +180,7 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { } user.Identities = []models.Identity{*identity} } - if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, adminUser, models.UserInvitedAction, "", map[string]interface{}{ + if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, adminUser, models.UserInvitedAction, map[string]interface{}{ "user_id": user.ID, "user_email": user.Email, }); terr != nil { diff --git a/internal/api/mfa.go b/internal/api/mfa.go index 4b6467eadb..10f1a789c5 100644 --- a/internal/api/mfa.go +++ b/internal/api/mfa.go @@ -205,7 +205,7 @@ func (a *API) enrollPhoneFactor(w http.ResponseWriter, r *http.Request, params * if terr := tx.Create(factor); terr != nil { return terr } - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.EnrollFactorAction, utilities.GetIPAddress(r), map[string]interface{}{ + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.EnrollFactorAction, map[string]interface{}{ "factor_id": factor.ID, "factor_type": factor.FactorType, }); terr != nil { @@ -240,7 +240,7 @@ func (a *API) enrollWebAuthnFactor(w http.ResponseWriter, r *http.Request, param if terr := tx.Create(factor); terr != nil { return terr } - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.EnrollFactorAction, utilities.GetIPAddress(r), map[string]interface{}{ + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.EnrollFactorAction, map[string]interface{}{ "factor_id": factor.ID, "factor_type": factor.FactorType, }); terr != nil { @@ -309,7 +309,7 @@ func (a *API) enrollTOTPFactor(w http.ResponseWriter, r *http.Request, params *E return terr } - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.EnrollFactorAction, utilities.GetIPAddress(r), map[string]interface{}{ + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.EnrollFactorAction, map[string]interface{}{ "factor_id": factor.ID, }); terr != nil { return terr @@ -437,7 +437,7 @@ func (a *API) challengePhoneFactor(w http.ResponseWriter, r *http.Request) error return terr } - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.CreateChallengeAction, utilities.GetIPAddress(r), map[string]interface{}{ + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.CreateChallengeAction, map[string]interface{}{ "factor_id": factor.ID, "factor_status": factor.Status, }); terr != nil { @@ -469,7 +469,7 @@ func (a *API) challengeTOTPFactor(w http.ResponseWriter, r *http.Request) error if terr := factor.WriteChallengeToDatabase(tx, challenge); terr != nil { return terr } - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.CreateChallengeAction, utilities.GetIPAddress(r), map[string]interface{}{ + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.CreateChallengeAction, map[string]interface{}{ "factor_id": factor.ID, "factor_status": factor.Status, }); terr != nil { @@ -692,7 +692,7 @@ func (a *API) verifyTOTPFactor(w http.ResponseWriter, r *http.Request, params *V verified := false err = db.Transaction(func(tx *storage.Connection) error { var terr error - if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.VerifyFactorAction, utilities.GetIPAddress(r), map[string]interface{}{ + if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.VerifyFactorAction, map[string]interface{}{ "factor_id": factor.ID, "challenge_id": challenge.ID, "factor_type": factor.FactorType, @@ -844,7 +844,7 @@ func (a *API) verifyPhoneFactor(w http.ResponseWriter, r *http.Request, params * verified := false err = db.Transaction(func(tx *storage.Connection) error { var terr error - if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.VerifyFactorAction, utilities.GetIPAddress(r), map[string]interface{}{ + if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.VerifyFactorAction, map[string]interface{}{ "factor_id": factor.ID, "challenge_id": challenge.ID, "factor_type": factor.FactorType, @@ -960,7 +960,7 @@ func (a *API) verifyWebAuthnFactor(w http.ResponseWriter, r *http.Request, param verified := false err = db.Transaction(func(tx *storage.Connection) error { var terr error - if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.VerifyFactorAction, utilities.GetIPAddress(r), map[string]interface{}{ + if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.VerifyFactorAction, map[string]interface{}{ "factor_id": factor.ID, "challenge_id": challenge.ID, "factor_type": factor.FactorType, @@ -1078,7 +1078,7 @@ func (a *API) UnenrollFactor(w http.ResponseWriter, r *http.Request) error { if terr := tx.Destroy(factor); terr != nil { return terr } - if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UnenrollFactorAction, utilities.GetIPAddress(r), map[string]interface{}{ + if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UnenrollFactorAction, map[string]interface{}{ "factor_id": factor.ID, "factor_status": factor.Status, "session_id": session.ID, diff --git a/internal/api/oauthserver/handlers.go b/internal/api/oauthserver/handlers.go index ec844c85cc..9ef2b8aafb 100644 --- a/internal/api/oauthserver/handlers.go +++ b/internal/api/oauthserver/handlers.go @@ -397,7 +397,7 @@ func (s *Server) handleAuthorizationCodeGrant(ctx context.Context, w http.Respon authMethod := models.OAuthProviderAuthorizationCode // Create audit log entry for OAuth token exchange - if terr := models.NewAuditLogEntry(s.config.AuditLog, r, tx, user, models.LoginAction, "", map[string]interface{}{ + if terr := models.NewAuditLogEntry(s.config.AuditLog, r, tx, user, models.LoginAction, map[string]interface{}{ "provider_type": "oauth_provider_authorization_code", "client_id": client.ID.String(), }); terr != nil { @@ -610,7 +610,7 @@ func (s *Server) UserRevokeOAuthGrant(w http.ResponseWriter, r *http.Request) er } // Create audit log entry - if terr := models.NewAuditLogEntry(s.config.AuditLog, r, tx, user, models.TokenRevokedAction, "", map[string]interface{}{ + if terr := models.NewAuditLogEntry(s.config.AuditLog, r, tx, user, models.TokenRevokedAction, map[string]interface{}{ "oauth_client_id": clientID.String(), "action": "revoke_oauth_grant", }); terr != nil { diff --git a/internal/api/otp.go b/internal/api/otp.go index 5f12b0bbe3..9441abdf2b 100644 --- a/internal/api/otp.go +++ b/internal/api/otp.go @@ -186,7 +186,7 @@ func (a *API) SmsOtp(w http.ResponseWriter, r *http.Request) error { messageID := "" err = db.Transaction(func(tx *storage.Connection) error { - if err := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserRecoveryRequestedAction, "", map[string]interface{}{ + if err := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserRecoveryRequestedAction, map[string]interface{}{ "channel": params.Channel, }); err != nil { return err diff --git a/internal/api/passkey_admin.go b/internal/api/passkey_admin.go index 1cfa4d9109..e58fae3690 100644 --- a/internal/api/passkey_admin.go +++ b/internal/api/passkey_admin.go @@ -8,7 +8,6 @@ import ( "github.com/supabase/auth/internal/api/apierrors" "github.com/supabase/auth/internal/models" "github.com/supabase/auth/internal/storage" - "github.com/supabase/auth/internal/utilities" ) // AdminPasskeyList handles GET /admin/users/{user_id}/passkeys. @@ -58,7 +57,7 @@ func (a *API) AdminPasskeyDelete(w http.ResponseWriter, r *http.Request) error { return terr } - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, adminUser, models.PasskeyDeletedAction, utilities.GetIPAddress(r), map[string]any{ + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, adminUser, models.PasskeyDeletedAction, map[string]any{ "user_id": user.ID, "passkey_id": cred.ID, }); terr != nil { diff --git a/internal/api/passkey_authentication.go b/internal/api/passkey_authentication.go index 6967d3f0a5..8ee5e958ea 100644 --- a/internal/api/passkey_authentication.go +++ b/internal/api/passkey_authentication.go @@ -183,7 +183,7 @@ func (a *API) PasskeyAuthenticationVerify(w http.ResponseWriter, r *http.Request return terr } - if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.LoginAction, utilities.GetIPAddress(r), map[string]any{ + if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.LoginAction, map[string]any{ "passkey_id": passkeyCredential.ID, }); terr != nil { return terr diff --git a/internal/api/passkey_manage.go b/internal/api/passkey_manage.go index 0259d0876b..0c4c79ea69 100644 --- a/internal/api/passkey_manage.go +++ b/internal/api/passkey_manage.go @@ -91,7 +91,7 @@ func (a *API) PasskeyUpdate(w http.ResponseWriter, r *http.Request) error { return terr } - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.PasskeyUpdatedAction, utilities.GetIPAddress(r), map[string]any{ + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.PasskeyUpdatedAction, map[string]any{ "passkey_id": cred.ID, }); terr != nil { return terr @@ -132,7 +132,7 @@ func (a *API) PasskeyDelete(w http.ResponseWriter, r *http.Request) error { return terr } - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.PasskeyDeletedAction, utilities.GetIPAddress(r), map[string]any{ + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.PasskeyDeletedAction, map[string]any{ "passkey_id": cred.ID, }); terr != nil { return terr diff --git a/internal/api/passkey_registration.go b/internal/api/passkey_registration.go index 4b47b095e6..1d12eba07c 100644 --- a/internal/api/passkey_registration.go +++ b/internal/api/passkey_registration.go @@ -194,7 +194,7 @@ func (a *API) PasskeyRegistrationVerify(w http.ResponseWriter, r *http.Request) return terr } - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.PasskeyCreatedAction, utilities.GetIPAddress(r), map[string]any{ + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.PasskeyCreatedAction, map[string]any{ "passkey_id": passkeyCredential.ID, }); terr != nil { return terr diff --git a/internal/api/reauthenticate.go b/internal/api/reauthenticate.go index 687b559cdc..0802f2a312 100644 --- a/internal/api/reauthenticate.go +++ b/internal/api/reauthenticate.go @@ -38,7 +38,7 @@ func (a *API) Reauthenticate(w http.ResponseWriter, r *http.Request) error { messageID := "" err := db.Transaction(func(tx *storage.Connection) error { - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserReauthenticateAction, "", nil); terr != nil { + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserReauthenticateAction, nil); terr != nil { return terr } if email != "" { diff --git a/internal/api/recover.go b/internal/api/recover.go index 7c967aaf00..af717d1595 100644 --- a/internal/api/recover.go +++ b/internal/api/recover.go @@ -63,7 +63,7 @@ func (a *API) Recover(w http.ResponseWriter, r *http.Request) error { } err = db.Transaction(func(tx *storage.Connection) error { - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserRecoveryRequestedAction, "", nil); terr != nil { + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserRecoveryRequestedAction, nil); terr != nil { return terr } return a.sendPasswordRecovery(r, tx, user, flowType) diff --git a/internal/api/resend.go b/internal/api/resend.go index 22bac98ece..cbed2bb71f 100644 --- a/internal/api/resend.go +++ b/internal/api/resend.go @@ -124,7 +124,7 @@ func (a *API) Resend(w http.ResponseWriter, r *http.Request) error { err = db.Transaction(func(tx *storage.Connection) error { switch params.Type { case mail.SignupVerification: - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserConfirmationRequestedAction, "", nil); terr != nil { + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserConfirmationRequestedAction, nil); terr != nil { return terr } flowType := getFlowFromChallenge(params.CodeChallenge) @@ -135,7 +135,7 @@ func (a *API) Resend(w http.ResponseWriter, r *http.Request) error { } return a.sendConfirmation(r, tx, user, flowType) case smsVerification: - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserRecoveryRequestedAction, "", nil); terr != nil { + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserRecoveryRequestedAction, nil); terr != nil { return terr } mID, terr := a.sendPhoneConfirmation(r, tx, user, params.Phone, phoneConfirmationOtp, sms_provider.SMSProvider) diff --git a/internal/api/signup.go b/internal/api/signup.go index fcc3877ed7..1c632ded6d 100644 --- a/internal/api/signup.go +++ b/internal/api/signup.go @@ -227,7 +227,7 @@ func (a *API) Signup(w http.ResponseWriter, r *http.Request) error { if params.Provider == "email" && !user.IsConfirmed() { if config.Mailer.Autoconfirm { - if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserSignedUpAction, "", map[string]interface{}{ + if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserSignedUpAction, map[string]interface{}{ "provider": params.Provider, }); terr != nil { return terr @@ -236,7 +236,7 @@ func (a *API) Signup(w http.ResponseWriter, r *http.Request) error { return apierrors.NewInternalServerError("Database error updating user").WithInternalError(terr) } } else { - if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserConfirmationRequestedAction, "", map[string]interface{}{ + if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserConfirmationRequestedAction, map[string]interface{}{ "provider": params.Provider, }); terr != nil { return terr @@ -253,7 +253,7 @@ func (a *API) Signup(w http.ResponseWriter, r *http.Request) error { } } else if params.Provider == "phone" && !user.IsPhoneConfirmed() { if config.Sms.Autoconfirm { - if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserSignedUpAction, "", map[string]interface{}{ + if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserSignedUpAction, map[string]interface{}{ "provider": params.Provider, "channel": params.Channel, }); terr != nil { @@ -263,7 +263,7 @@ func (a *API) Signup(w http.ResponseWriter, r *http.Request) error { return apierrors.NewInternalServerError("Database error updating user").WithInternalError(terr) } } else { - if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserConfirmationRequestedAction, "", map[string]interface{}{ + if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserConfirmationRequestedAction, map[string]interface{}{ "provider": params.Provider, }); terr != nil { return terr @@ -280,7 +280,7 @@ func (a *API) Signup(w http.ResponseWriter, r *http.Request) error { if err != nil { if errors.Is(err, UserExistsError) { err = db.Transaction(func(tx *storage.Connection) error { - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserRepeatedSignUpAction, "", map[string]interface{}{ + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserRepeatedSignUpAction, map[string]interface{}{ "provider": params.Provider, }); terr != nil { return terr @@ -307,7 +307,7 @@ func (a *API) Signup(w http.ResponseWriter, r *http.Request) error { var token *AccessTokenResponse err = db.Transaction(func(tx *storage.Connection) error { var terr error - if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.LoginAction, "", map[string]interface{}{ + if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.LoginAction, map[string]interface{}{ "provider": params.Provider, }); terr != nil { return terr diff --git a/internal/api/token.go b/internal/api/token.go index 3a1acea099..62d9c94c51 100644 --- a/internal/api/token.go +++ b/internal/api/token.go @@ -187,7 +187,7 @@ func (a *API) ResourceOwnerPasswordGrant(ctx context.Context, w http.ResponseWri var token *AccessTokenResponse err = db.Transaction(func(tx *storage.Connection) error { var terr error - if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.LoginAction, "", map[string]interface{}{ + if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.LoginAction, map[string]interface{}{ "provider": provider, }); terr != nil { return terr @@ -257,7 +257,7 @@ func (a *API) PKCE(ctx context.Context, w http.ResponseWriter, r *http.Request) if err != nil { return err } - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.LoginAction, "", map[string]interface{}{ + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.LoginAction, map[string]interface{}{ "provider_type": flowState.ProviderType, }); terr != nil { return terr diff --git a/internal/api/user.go b/internal/api/user.go index 0fc2a1b5eb..e5c186c538 100644 --- a/internal/api/user.go +++ b/internal/api/user.go @@ -221,7 +221,7 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error { return apierrors.NewInternalServerError("Error during password storage").WithInternalError(terr) } - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserUpdatePasswordAction, "", nil); terr != nil { + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserUpdatePasswordAction, nil); terr != nil { return terr } @@ -289,7 +289,7 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error { } } - if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserModifiedAction, "", nil); terr != nil { + if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserModifiedAction, nil); terr != nil { return apierrors.NewInternalServerError("Error recording audit log entry").WithInternalError(terr) } diff --git a/internal/api/verify.go b/internal/api/verify.go index 212d7388eb..3a10017391 100644 --- a/internal/api/verify.go +++ b/internal/api/verify.go @@ -337,7 +337,7 @@ func (a *API) signupVerify(r *http.Request, ctx context.Context, conn *storage.C } } - if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserSignedUpAction, "", map[string]interface{}{ + if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserSignedUpAction, map[string]interface{}{ "provider": EmailProvider, }); terr != nil { return terr @@ -376,7 +376,7 @@ func (a *API) recoverVerify(r *http.Request, conn *storage.Connection, user *mod return terr } if !user.IsConfirmed() { - if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserSignedUpAction, "", map[string]interface{}{ + if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserSignedUpAction, map[string]interface{}{ "provider": EmailProvider, }); terr != nil { return terr @@ -386,7 +386,7 @@ func (a *API) recoverVerify(r *http.Request, conn *storage.Connection, user *mod return terr } } else { - if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.LoginAction, "", nil); terr != nil { + if terr = models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.LoginAction, nil); terr != nil { return terr } } @@ -407,7 +407,7 @@ func (a *API) smsVerify(r *http.Request, conn *storage.Connection, user *models. err := conn.Transaction(func(tx *storage.Connection) error { if params.Type == smsVerification { - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserSignedUpAction, "", map[string]interface{}{ + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserSignedUpAction, map[string]interface{}{ "provider": PhoneProvider, }); terr != nil { return terr @@ -416,7 +416,7 @@ func (a *API) smsVerify(r *http.Request, conn *storage.Connection, user *models. return apierrors.NewInternalServerError("Error confirming user").WithInternalError(terr) } } else if params.Type == phoneChangeVerification { - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserModifiedAction, "", nil); terr != nil { + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserModifiedAction, nil); terr != nil { return terr } if identity, terr := models.FindIdentityByIdAndProvider(tx, user.ID.String(), "phone"); terr != nil { @@ -586,7 +586,7 @@ func (a *API) emailChangeVerify(r *http.Request, conn *storage.Connection, param // one email is confirmed at this point if GOTRUE_MAILER_SECURE_EMAIL_CHANGE_ENABLED is enabled oldEmail := user.GetEmail() err := conn.Transaction(func(tx *storage.Connection) error { - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserModifiedAction, "", nil); terr != nil { + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserModifiedAction, nil); terr != nil { return terr } diff --git a/internal/api/web3.go b/internal/api/web3.go index 2f731e9108..096b830815 100644 --- a/internal/api/web3.go +++ b/internal/api/web3.go @@ -154,7 +154,7 @@ func (a *API) web3GrantSolana(ctx context.Context, w http.ResponseWriter, r *htt } createdUser = decision == models.CreateAccount - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.LoginAction, "", map[string]interface{}{ + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.LoginAction, map[string]interface{}{ "provider": providerType, "chain": params.Chain, "network": parsedMessage.ChainID, @@ -300,7 +300,7 @@ func (a *API) web3GrantEthereum(ctx context.Context, w http.ResponseWriter, r *h } createdUser = decision == models.CreateAccount - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.LoginAction, "", map[string]interface{}{ + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.LoginAction, map[string]interface{}{ "provider": providerType, "chain": params.Chain, "network": parsedMessage.ChainID, diff --git a/internal/models/audit_log_entry.go b/internal/models/audit_log_entry.go index 5481851b7a..5355b3d18a 100644 --- a/internal/models/audit_log_entry.go +++ b/internal/models/audit_log_entry.go @@ -100,8 +100,9 @@ func (AuditLogEntry) TableName() string { return tableName } -func NewAuditLogEntry(config conf.AuditLogConfiguration, r *http.Request, tx *storage.Connection, actor *User, action AuditAction, ipAddress string, traits map[string]interface{}) error { +func NewAuditLogEntry(config conf.AuditLogConfiguration, r *http.Request, tx *storage.Connection, actor *User, action AuditAction, traits map[string]interface{}) error { id := uuid.Must(uuid.NewV4()) + ipAddress := utilities.GetIPAddress(r) username := actor.GetEmail() diff --git a/internal/models/audit_log_entry_test.go b/internal/models/audit_log_entry_test.go new file mode 100644 index 0000000000..708e8ec354 --- /dev/null +++ b/internal/models/audit_log_entry_test.go @@ -0,0 +1,98 @@ +package models + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" + "github.com/supabase/auth/internal/conf" + "github.com/supabase/auth/internal/sbff" + "github.com/supabase/auth/internal/storage" + "github.com/supabase/auth/internal/storage/test" +) + +type AuditLogEntryTestSuite struct { + suite.Suite + db *storage.Connection + config *conf.GlobalConfiguration +} + +func (ts *AuditLogEntryTestSuite) SetupTest() { + TruncateAll(ts.db) +} + +func TestAuditLogEntry(t *testing.T) { + globalConfig, err := conf.LoadGlobal(modelsTestConfig) + require.NoError(t, err) + + conn, err := test.SetupDBConnection(globalConfig) + require.NoError(t, err) + + ts := &AuditLogEntryTestSuite{ + db: conn, + config: globalConfig, + } + defer ts.db.Close() + + suite.Run(t, ts) +} + +func (ts *AuditLogEntryTestSuite) TestNewAuditLogEntryPopulatesIPFromRemoteAddr() { + user, err := NewUser("", "audit-ip@example.com", "secret", "test", nil) + require.NoError(ts.T(), err) + require.NoError(ts.T(), ts.db.Create(user)) + + req := httptest.NewRequest(http.MethodPost, "/token", nil) + req.RemoteAddr = "203.0.113.42:1234" + + require.NoError(ts.T(), NewAuditLogEntry(ts.config.AuditLog, req, ts.db, user, LoginAction, nil)) + + entries, err := FindAuditLogEntries(ts.db, nil, "", nil) + require.NoError(ts.T(), err) + require.Len(ts.T(), entries, 1) + require.Equal(ts.T(), "203.0.113.42", entries[0].IPAddress) +} + +func (ts *AuditLogEntryTestSuite) TestNewAuditLogEntryPopulatesIPFromXForwardedFor() { + user, err := NewUser("", "audit-xff@example.com", "secret", "test", nil) + require.NoError(ts.T(), err) + require.NoError(ts.T(), ts.db.Create(user)) + + req := httptest.NewRequest(http.MethodPost, "/token", nil) + req.RemoteAddr = "10.0.0.1:1234" + req.Header.Set("X-Forwarded-For", "203.0.113.99") + + require.NoError(ts.T(), NewAuditLogEntry(ts.config.AuditLog, req, ts.db, user, LoginAction, nil)) + + entries, err := FindAuditLogEntries(ts.db, nil, "", nil) + require.NoError(ts.T(), err) + require.Len(ts.T(), entries, 1) + require.Equal(ts.T(), "203.0.113.99", entries[0].IPAddress) +} + +func (ts *AuditLogEntryTestSuite) TestNewAuditLogEntryPopulatesIPFromSbForwardedFor() { + user, err := NewUser("", "audit-sbff@example.com", "secret", "test", nil) + require.NoError(ts.T(), err) + require.NoError(ts.T(), ts.db.Create(user)) + + securityConfig := conf.SecurityConfiguration{SbForwardedForEnabled: true} + middleware := sbff.Middleware(&securityConfig, func(r *http.Request, err error) {}) + + handler := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + require.NoError(ts.T(), NewAuditLogEntry(ts.config.AuditLog, r, ts.db, user, LoginAction, nil)) + }) + + req := httptest.NewRequest(http.MethodPost, "/token", nil) + req.RemoteAddr = "10.0.0.1:1234" + req.Header.Set("X-Forwarded-For", "198.51.100.1") + req.Header.Set(sbff.HeaderName, "203.0.113.77") + + middleware(handler).ServeHTTP(httptest.NewRecorder(), req) + + entries, err := FindAuditLogEntries(ts.db, nil, "", nil) + require.NoError(ts.T(), err) + require.Len(ts.T(), entries, 1) + require.Equal(ts.T(), "203.0.113.77", entries[0].IPAddress) +} diff --git a/internal/models/refresh_token.go b/internal/models/refresh_token.go index 63e60fd140..9f117fe174 100644 --- a/internal/models/refresh_token.go +++ b/internal/models/refresh_token.go @@ -68,7 +68,7 @@ func GrantRefreshTokenSwap(config conf.AuditLogConfiguration, r *http.Request, t var newToken *RefreshToken err := tx.Transaction(func(rtx *storage.Connection) error { var terr error - if terr = NewAuditLogEntry(config, r, tx, user, TokenRevokedAction, "", nil); terr != nil { + if terr = NewAuditLogEntry(config, r, tx, user, TokenRevokedAction, nil); terr != nil { return errors.Wrap(terr, "error creating audit log entry") } diff --git a/internal/tokens/service.go b/internal/tokens/service.go index 2d9d4e234b..3cfdc396cd 100644 --- a/internal/tokens/service.go +++ b/internal/tokens/service.go @@ -403,7 +403,7 @@ func (s *Service) RefreshTokenGrant(ctx context.Context, db *storage.Connection, } } - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.TokenRefreshedAction, "", nil); terr != nil { + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.TokenRefreshedAction, nil); terr != nil { return terr } @@ -584,7 +584,7 @@ func (s *Service) RefreshTokenGrant(ctx context.Context, db *storage.Connection, responseHeaders.Set("sb-auth-refresh-token-counter", strconv.FormatInt(*session.RefreshTokenCounter, 10)) - if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.TokenRefreshedAction, "", nil); terr != nil { + if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.TokenRefreshedAction, nil); terr != nil { return terr } }