From f6c75b349737fbe90fdacd2ccf6d6931d631af5d Mon Sep 17 00:00:00 2001 From: Benjamin Date: Sun, 23 Nov 2025 00:33:35 +0100 Subject: [PATCH] refactor(security): use structured logger to prevent sensitive data exposure Changes: - Replace fmt.Println with logger.Logger.Warn in config loading - Remove client_id and auth URLs from OAuth provider logs - Remove user email, name, and sub from authentication logs - Use structured logging for worker shutdown errors - Fix MagicLink status logging (only log when actually enabled) --- .../infrastructure/auth/oauth_provider.go | 25 ++++++------------- .../internal/infrastructure/config/config.go | 3 ++- backend/pkg/crypto/ed25519.go | 4 +-- backend/pkg/web/server.go | 11 ++++---- 4 files changed, 18 insertions(+), 25 deletions(-) diff --git a/backend/internal/infrastructure/auth/oauth_provider.go b/backend/internal/infrastructure/auth/oauth_provider.go index 4710b3e..65a9d56 100644 --- a/backend/internal/infrastructure/auth/oauth_provider.go +++ b/backend/internal/infrastructure/auth/oauth_provider.go @@ -56,10 +56,7 @@ func NewOAuthProvider(config OAuthProviderConfig) *OAuthProvider { }, } - logger.Logger.Info("OAuth provider configured", - "client_id", config.ClientID, - "auth_url", config.AuthURL, - "redirect_url", oauthConfig.RedirectURL) + logger.Logger.Info("OAuth provider configured successfully") return &OAuthProvider{ oauthConfig: oauthConfig, @@ -289,23 +286,17 @@ func (p *OAuthProvider) HandleCallback(ctx context.Context, w http.ResponseWrite } if !p.IsAllowedDomain(user.Email) { - logger.Logger.Warn("User domain not allowed", - "user_email", user.Email, - "allowed_domain", p.allowedDomain) + logger.Logger.Warn("User domain not allowed") return nil, nextURL, models.ErrDomainNotAllowed } - logger.Logger.Info("OAuth callback successful", - "user_email", user.Email, - "user_name", user.Name) + logger.Logger.Info("OAuth callback successful") // Store refresh token if available if token.RefreshToken != "" && p.sessionSvc.sessionRepo != nil { if err := p.sessionSvc.StoreRefreshToken(ctx, w, r, token, user); err != nil { // Log error but don't fail the authentication - logger.Logger.Error("Failed to store refresh token (non-fatal)", - "user_sub", user.Sub, - "error", err.Error()) + logger.Logger.Error("Failed to store refresh token (non-fatal)", "error", err.Error()) } } @@ -379,12 +370,12 @@ func (p *OAuthProvider) parseUserInfo(resp *http.Response) (*models.User, error) user.Name = name logger.Logger.Debug("Extracted OAuth user identifiers", - "sub", user.Sub, - "email_present", user.Email != "", - "name_present", user.Name != "") + "has_sub", user.Sub != "", + "has_email", user.Email != "", + "has_name", user.Name != "") if !user.IsValid() { - return nil, fmt.Errorf("invalid user data extracted: sub=%s, email=%s", user.Sub, user.Email) + return nil, fmt.Errorf("invalid user data extracted") } return user, nil diff --git a/backend/internal/infrastructure/config/config.go b/backend/internal/infrastructure/config/config.go index ecf9770..043673f 100644 --- a/backend/internal/infrastructure/config/config.go +++ b/backend/internal/infrastructure/config/config.go @@ -7,6 +7,7 @@ import ( "os" "strings" + "github.com/btouchard/ackify-ce/backend/pkg/logger" "github.com/gorilla/securecookie" ) @@ -240,7 +241,7 @@ func parseCookieSecret() ([]byte, error) { raw := os.Getenv("ACKIFY_OAUTH_COOKIE_SECRET") if raw == "" { secret := securecookie.GenerateRandomKey(32) - fmt.Println("[WARN] ACKIFY_OAUTH_COOKIE_SECRET not set, generated volatile secret (sessions reset on restart)") + logger.Logger.Warn("OAuth cookie secret not set, sessions will reset on restart") return secret, nil } diff --git a/backend/pkg/crypto/ed25519.go b/backend/pkg/crypto/ed25519.go index 6c9d9bd..0cc0059 100644 --- a/backend/pkg/crypto/ed25519.go +++ b/backend/pkg/crypto/ed25519.go @@ -12,6 +12,7 @@ import ( "time" "github.com/btouchard/ackify-ce/backend/internal/domain/models" + "github.com/btouchard/ackify-ce/backend/pkg/logger" ) // Ed25519Signer provides cryptographic signature operations using Ed25519 elliptic curve algorithm @@ -85,8 +86,7 @@ func loadOrGenerateKeys() (ed25519.PrivateKey, ed25519.PublicKey, error) { return nil, nil, fmt.Errorf("failed to generate keys: %w", err) } - // Do not print private keys. Warn about ephemeral key usage only. - fmt.Println("[WARN] Generated ephemeral Ed25519 keypair. Set ACKIFY_ED25519_PRIVATE_KEY to persist across restarts.") + logger.Logger.Warn("Ed25519 private key not set, signatures will be different on restart") return privateKey, publicKey, nil } diff --git a/backend/pkg/web/server.go b/backend/pkg/web/server.go index 51ac628..8597a44 100644 --- a/backend/pkg/web/server.go +++ b/backend/pkg/web/server.go @@ -120,13 +120,15 @@ func NewServer(ctx context.Context, cfg *config.Config, frontend embed.FS, versi BaseURL: cfg.App.BaseURL, AppName: cfg.App.Organisation, }) - logger.Logger.Info("Magic Link authentication enabled") // Initialize Magic Link cleanup worker var magicLinkWorker *workers.MagicLinkCleanupWorker if cfg.Auth.MagicLinkEnabled { + logger.Logger.Info("Magic Link authentication enabled") magicLinkWorker = workers.NewMagicLinkCleanupWorker(magicLinkService, 1*time.Hour) go magicLinkWorker.Start(ctx) + } else { + logger.Logger.Info("Magic Link authentication disabled") } // Initialize reminder service with async support (needs magicLinkService) @@ -218,22 +220,21 @@ func (s *Server) Shutdown(ctx context.Context) error { // Stop OAuth session worker first if it exists if s.sessionWorker != nil { if err := s.sessionWorker.Stop(); err != nil { - fmt.Printf("Warning: failed to stop OAuth session worker: %v\n", err) + logger.Logger.Warn("Failed to stop OAuth session worker", "error", err) } } // Stop email worker if it exists if s.emailWorker != nil { if err := s.emailWorker.Stop(); err != nil { - // Log but don't fail shutdown - fmt.Printf("Warning: failed to stop email worker: %v\n", err) + logger.Logger.Warn("Failed to stop email worker", "error", err) } } // Stop webhook worker if s.webhookWorker != nil { if err := s.webhookWorker.Stop(); err != nil { - fmt.Printf("Warning: failed to stop webhook worker: %v\n", err) + logger.Logger.Warn("Failed to stop webhook worker", "error", err) } }