mirror of
https://github.com/pocket-id/pocket-id.git
synced 2026-07-22 04:31:26 +02:00
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YZ6SoJpmnLggZqactXxrak
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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).
|
||||
|
||||
@@ -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{
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user