From 10c0f1d5d7c0ea3fbefa7eb82949f4909e9958e6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 19 Jul 2026 23:13:22 +0000 Subject: [PATCH] refactor: gate sign-in notification email with an explicit flag Pass emailLoginNotificationEnabled to CreateNewSignInWithEmail instead of the whole AppConfigModel, so the sign-in audit business logic no longer takes the config struct just to read one flag. The caller evaluates the flag from the config it already loaded. The background notification email, which is sent in a detached goroutine after the request completes, now resolves the current config from the app config service rather than threading the request's snapshot into the goroutine. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01YZ6SoJpmnLggZqactXxrak --- .../internal/bootstrap/services_bootstrap.go | 2 +- backend/internal/service/audit_log_service.go | 30 ++++++++++++------- .../service/one_time_access_service_test.go | 2 +- backend/internal/webauthn/module.go | 2 +- backend/internal/webauthn/service.go | 2 +- 5 files changed, 24 insertions(+), 14 deletions(-) diff --git a/backend/internal/bootstrap/services_bootstrap.go b/backend/internal/bootstrap/services_bootstrap.go index c677c05b..c3600f4f 100644 --- a/backend/internal/bootstrap/services_bootstrap.go +++ b/backend/internal/bootstrap/services_bootstrap.go @@ -74,7 +74,7 @@ func initServices( } svc.geoLiteService = service.NewGeoLiteService(httpClient) - svc.auditLogService = service.NewAuditLogService(db, svc.emailService, svc.geoLiteService) + svc.auditLogService = service.NewAuditLogService(db, svc.emailService, svc.geoLiteService, svc.appConfigService) svc.jwtService, err = service.NewJwtService(ctx, db, instanceID) if err != nil { return nil, fmt.Errorf("failed to create JWT service: %w", err) diff --git a/backend/internal/service/audit_log_service.go b/backend/internal/service/audit_log_service.go index 4d8bf905..f45cac89 100644 --- a/backend/internal/service/audit_log_service.go +++ b/backend/internal/service/audit_log_service.go @@ -14,16 +14,18 @@ import ( ) type AuditLogService struct { - db *gorm.DB - emailService *EmailService - geoliteService *GeoLiteService + db *gorm.DB + emailService *EmailService + geoliteService *GeoLiteService + appConfigService *appconfig.AppConfigService } -func NewAuditLogService(db *gorm.DB, emailService *EmailService, geoliteService *GeoLiteService) *AuditLogService { +func NewAuditLogService(db *gorm.DB, emailService *EmailService, geoliteService *GeoLiteService, appConfigService *appconfig.AppConfigService) *AuditLogService { return &AuditLogService{ - db: db, - emailService: emailService, - geoliteService: geoliteService, + db: db, + emailService: emailService, + geoliteService: geoliteService, + appConfigService: appConfigService, } } @@ -63,7 +65,8 @@ func (s *AuditLogService) Create(ctx context.Context, event model.AuditLogEvent, } // CreateNewSignInWithEmail creates a new audit log entry in the database and sends an email if the device hasn't been used before -func (s *AuditLogService) CreateNewSignInWithEmail(ctx context.Context, ipAddress, userAgent, userID string, tx *gorm.DB, dbConfig *appconfig.AppConfigModel) model.AuditLog { +// emailLoginNotificationEnabled gates whether the notification email is sent, so the caller decides using the config it already loaded +func (s *AuditLogService) CreateNewSignInWithEmail(ctx context.Context, ipAddress, userAgent, userID string, tx *gorm.DB, emailLoginNotificationEnabled bool) model.AuditLog { createdAuditLog, ok := s.Create(ctx, model.AuditLogEventSignIn, ipAddress, userAgent, userID, model.AuditLogData{}, tx) if !ok { // At this point the transaction has been canceled already, and error has been logged @@ -89,15 +92,22 @@ func (s *AuditLogService) CreateNewSignInWithEmail(ctx context.Context, ipAddres } // If the user hasn't logged in from the same device before and email notifications are enabled, send an email - if dbConfig.EmailLoginNotificationEnabled.IsTrue() && count <= 1 { + if emailLoginNotificationEnabled && count <= 1 { go func() { // This runs in background, so use a context without cancellation (or it would be stopped when the request ends) // We still want to have a context derived from the request's to carry over tracing info innerCtx := context.WithoutCancel(ctx) + // This runs after the request has completed, so we resolve the current config rather than threading the request's snapshot into the goroutine + dbConfig, innerErr := s.appConfigService.GetConfig(innerCtx) + if innerErr != nil { + slog.ErrorContext(innerCtx, "Failed to load app configuration to send notification email", slog.Any("error", innerErr)) + return + } + // Note we don't use the transaction here because this is running in background var user model.User - innerErr := s.db. + innerErr = s.db. WithContext(innerCtx). Where("id = ?", userID). First(&user). diff --git a/backend/internal/service/one_time_access_service_test.go b/backend/internal/service/one_time_access_service_test.go index dd82fd71..ff946043 100644 --- a/backend/internal/service/one_time_access_service_test.go +++ b/backend/internal/service/one_time_access_service_test.go @@ -18,7 +18,7 @@ func TestExchangeOneTimeAccessTokenRejectsDisabledUser(t *testing.T) { appConfig := appconfig.NewTestAppConfigService(nil) instanceID := newInstanceID(t, db) jwtService := initJwtService(t, db, instanceID, appConfig, newTestEnvConfig()) - auditLogService := NewAuditLogService(db, nil, &GeoLiteService{}) + auditLogService := NewAuditLogService(db, nil, &GeoLiteService{}, appConfig) oneTimeAccessService := NewOneTimeAccessService(db, nil, jwtService, auditLogService, nil) user := model.User{ diff --git a/backend/internal/webauthn/module.go b/backend/internal/webauthn/module.go index 5271bf69..47017145 100644 --- a/backend/internal/webauthn/module.go +++ b/backend/internal/webauthn/module.go @@ -20,7 +20,7 @@ type TokenService interface { type AuditLogger interface { Create(ctx context.Context, event model.AuditLogEvent, ipAddress, userAgent, userID string, data model.AuditLogData, tx *gorm.DB) (model.AuditLog, bool) - CreateNewSignInWithEmail(ctx context.Context, ipAddress, userAgent, userID string, tx *gorm.DB, dbConfig *appconfig.AppConfigModel) model.AuditLog + CreateNewSignInWithEmail(ctx context.Context, ipAddress, userAgent, userID string, tx *gorm.DB, emailLoginNotificationEnabled bool) model.AuditLog } // AppConfigResolver loads the current application configuration, so handlers can pass it explicitly to the service methods that need it diff --git a/backend/internal/webauthn/service.go b/backend/internal/webauthn/service.go index 8e387ca6..0443b9e9 100644 --- a/backend/internal/webauthn/service.go +++ b/backend/internal/webauthn/service.go @@ -277,7 +277,7 @@ func (s *Service) VerifyLogin(ctx context.Context, dbConfig *appconfig.AppConfig return model.User{}, "", err } - s.auditLog.CreateNewSignInWithEmail(ctx, ipAddress, userAgent, user.ID, tx, dbConfig) + s.auditLog.CreateNewSignInWithEmail(ctx, ipAddress, userAgent, user.ID, tx, dbConfig.EmailLoginNotificationEnabled.IsTrue()) err = tx.Commit().Error if err != nil {