Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@ ACCOUNT_LOCK_DURATION=30
ENCRYPTION_KEY=your-32-byte-secret-key-here

LOGIN_RATE_LIMIT_MAX=5
LOGIN_RATE_LIMIT_WINDOW=900000 //ms
LOGIN_RATE_LIMIT_WINDOW=900000

REGISTER_RATE_LIMIT_MAX=3
REGISTER_RATE_LIMIT_WINDOW=3600000 //ms
REGISTER_RATE_LIMIT_WINDOW=3600000

FORGOT_RATE_LIMIT_MAX=3
FORGOT_RATE_LIMIT_WINDOW=3600000 //ms
FORGOT_RATE_LIMIT_WINDOW=3600000
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ FROM alpine:latest
WORKDIR /app

# Install runtime dependencies (ca-certificates for HTTPS, tzdata for timezones)
RUN apk add --no-cache ca-certificates tzdata
RUN apk add --no-cache ca-certificates tzdata wget

# Create a non-root user
RUN addgroup -S authgroup && adduser -S authuser -G authgroup
Expand Down
3 changes: 2 additions & 1 deletion cmd/server/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func main() {
}
// Backfill existing tokens using their own ID as the family ID
if err := db.Exec("UPDATE refresh_tokens SET family_id = id WHERE family_id IS NULL").Error; err != nil {
log.Fatal("Failed to backfill family_id for existing refresh tokens:", err)
log.Printf("Warning: Failed to backfill family_id for existing refresh tokens (expected on fresh DB): %v", err)
}
Comment on lines 53 to 56

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Only downgrade the backfill error when the table is actually absent.

After this change, any failure in UPDATE refresh_tokens SET family_id = id ... is treated as "expected on fresh DB". On an existing database, the same branch would also swallow real permission/locking/query errors and continue with partially migrated token data. Check HasTable/missing-table explicitly and keep failing startup for all other backfill failures.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/server/main.go` around lines 53 - 56, The refresh token backfill in main
should only be downgraded when the refresh_tokens table is actually missing, not
for every Exec failure. Update the startup migration logic around db.Exec and
the existing backfill block to first detect absence with HasTable or an
equivalent missing-table check, and keep treating all other UPDATE
refresh_tokens SET family_id = id failures as fatal so startup stops on real
migration, permission, locking, or query errors.


// Auto-migrate database models
Expand All @@ -61,6 +61,7 @@ func main() {
&models.VerificationToken{},
&models.PasswordResetToken{},
&models.AuditLog{},
&models.DeviceFingerprint{},
// OAuth 2.0 Provider models
&models.OAuthClient{},
&models.AuthorizationCode{},
Expand Down
18 changes: 18 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ services:
- PORT=8080
- DATABASE_URL=host=postgres user=postgres password=postgres dbname=auth_db port=5432 sslmode=disable
- REDIS_URL=redis://auth-redis:6379
- SMTP_HOST=auth-mailpit

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Add Mailpit to the app startup dependencies.

The app now sends SMTP traffic to auth-mailpit, but Compose still only orders startup against Postgres and Redis. In local Docker runs, the first unrecognized-login alert can race Mailpit startup and fail with connection refused.

Also applies to: 52-60

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docker-compose.yml` at line 15, The app startup order in the Compose
configuration is missing a dependency on Mailpit, so SMTP alerts can race
`auth-mailpit` before it is ready. Update the service definition that currently
depends on Postgres and Redis to also wait for the Mailpit service, using the
existing app service block and the `SMTP_HOST=auth-mailpit` configuration as the
anchor, so local startup does not attempt SMTP delivery too early.

- JWT_ACCESS_SECRET=${JWT_ACCESS_SECRET:-secret}
- JWT_REFRESH_SECRET=${JWT_REFRESH_SECRET:-refresh}
depends_on:
Expand All @@ -22,6 +23,13 @@ services:
env_file:
- .env

healthcheck:
test: ["CMD", "wget", "--spider", "-q", "http://localhost:8080/health"]
interval: 30s
timeout: 10s
retries: 3
start_period: 15s

postgres:
image: postgres:15-alpine
container_name: auth-postgres
Expand All @@ -48,6 +56,16 @@ services:
networks:
- auth-network

mailpit:
image: axllent/mailpit
container_name: auth-mailpit
restart: unless-stopped
ports:
- "8025:8025"
- "1025:1025"
networks:
- auth-network

networks:
auth-network:
driver: bridge
Expand Down
23 changes: 23 additions & 0 deletions internal/handler/auth_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,29 @@ func (h *AuthHandler) ResetPassword(c *gin.Context) {
c.JSON(http.StatusOK, utils.SuccessResponse("Password has been reset successfully. You can now login with your new password.", nil))
}

// LockAccount handles account locking via email link
// @Summary Lock account
// @Tags auth
// @Accept json
// @Produce json
// @Param token query string true "Lock token"
// @Success 200 {object} utils.Response
// @Router /api/auth/lock-account [get]
func (h *AuthHandler) LockAccount(c *gin.Context) {
token := c.Query("token")
if token == "" {
c.JSON(http.StatusBadRequest, utils.ValidationErrorResponse("Token is required"))
return
}

if err := h.authService.LockAccount(token); err != nil {
c.JSON(http.StatusBadRequest, utils.ErrorResponse("Failed to lock account", err))
return
Comment on lines +173 to +180

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Map lock-account failures to the right status/code payload.

This branch turns every LockAccount error into 400, so an invalid token and an internal LockUser/RevokeAllUserTokens failure become indistinguishable to clients. It also keeps the old helper envelope instead of the handler contract required here. Please translate typed service errors into 4xx/5xx responses and emit the required {"error","code"} shape for this endpoint; if utils.ErrorResponse is still the shared path, extend that helper first so this handler stays consistent with the rest of the file. As per coding guidelines, "internal/handler/**/*.go: Return JSON error responses in format {\"error\": \"message\", \"code\": \"ERROR_CODE\"} from HTTP handlers" and "internal/{service,handler}/**/*.go: Implement custom error types in service/errors.go and convert service errors to HTTP status codes in handlers". Based on learnings, this codebase currently centralizes handler errors through utils.ErrorResponse(...), so changing the helper first avoids introducing a one-off response shape.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/handler/auth_handler.go` around lines 173 - 180, The LockAccount
failure path in auth_handler.go currently collapses all service errors into HTTP
400 and still uses the old error envelope, so update the handler around
h.authService.LockAccount to distinguish typed service errors (for example
invalid token vs LockUser/RevokeAllUserTokens failures) and map them to the
correct 4xx/5xx status codes. Return the required JSON shape with error and code
for this endpoint, and if utils.ErrorResponse is the shared response path,
extend that helper first so the Login/lock-account handler stays consistent with
the rest of the file.

Sources: Coding guidelines, Learnings

}

c.JSON(http.StatusOK, utils.SuccessResponse("Your account has been locked to prevent unauthorized access. Please reset your password to regain access.", nil))
}

// UpdateProfile handles profile updates
// @Summary Update user profile
// @Tags auth
Expand Down
29 changes: 29 additions & 0 deletions internal/models/device_fingerprint.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package models

import (
"time"

"github.com/google/uuid"
"gorm.io/gorm"
)

type DeviceFingerprint struct {
ID string `gorm:"type:uuid;primary_key" json:"id"`
UserID string `gorm:"type:uuid;not null;index" json:"userId"`
FingerprintHash string `gorm:"not null;index" json:"fingerprintHash"`
Comment on lines +12 to +13

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Enforce uniqueness for user_id + fingerprint_hash.

The current read-then-create flow can insert duplicate device rows under concurrent logins without a DB-level composite unique constraint.

Suggested fix
-	UserID          string    `gorm:"type:uuid;not null;index" json:"userId"`
-	FingerprintHash string    `gorm:"not null;index" json:"fingerprintHash"`
+	UserID          string    `gorm:"type:uuid;not null;uniqueIndex:idx_user_fingerprint" json:"userId"`
+	FingerprintHash string    `gorm:"not null;uniqueIndex:idx_user_fingerprint" json:"fingerprintHash"`
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
UserID string `gorm:"type:uuid;not null;index" json:"userId"`
FingerprintHash string `gorm:"not null;index" json:"fingerprintHash"`
UserID string `gorm:"type:uuid;not null;uniqueIndex:idx_user_fingerprint" json:"userId"`
FingerprintHash string `gorm:"not null;uniqueIndex:idx_user_fingerprint" json:"fingerprintHash"`
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/models/device_fingerprint.go` around lines 12 - 13, The
DeviceFingerprint model currently allows duplicate rows for the same user and
fingerprint under concurrent logins because `UserID` and `FingerprintHash` are
only indexed, not uniquely constrained. Update the `DeviceFingerprint` struct
tags to enforce a composite uniqueness rule on `user_id` + `fingerprint_hash` at
the DB level, and ensure any create/upsert logic that uses this model handles
the resulting unique constraint correctly.

UserAgent string `gorm:"size:500" json:"userAgent"`
IPAddress string `gorm:"size:45" json:"ipAddress"`
LastSeenAt time.Time `json:"lastSeenAt"`
CreatedAt time.Time `json:"createdAt"`
}

func (d *DeviceFingerprint) BeforeCreate(tx *gorm.DB) error {
if d.ID == "" {
d.ID = uuid.New().String()
}
return nil
}

func (DeviceFingerprint) TableName() string {
return "device_fingerprints"
}
37 changes: 37 additions & 0 deletions internal/repository/device_repository.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package repository

import (
"errors"
"time"

"github.com/roshankumar0036singh/auth-server/internal/models"
"gorm.io/gorm"
)

type DeviceRepository struct {
db *gorm.DB
}

func NewDeviceRepository(db *gorm.DB) *DeviceRepository {
return &DeviceRepository{db: db}
}

func (r *DeviceRepository) FindByFingerprint(userID, hash string) (*models.DeviceFingerprint, error) {
var device models.DeviceFingerprint
err := r.db.Where("user_id = ? AND fingerprint_hash = ?", userID, hash).First(&device).Error
if err != nil {
if errors.Is(err, gorm.ErrRecordNotFound) {
return nil, nil // Return nil if not found, instead of error
}
return nil, err
}
return &device, nil
}

func (r *DeviceRepository) Create(device *models.DeviceFingerprint) error {
return r.db.Create(device).Error
}

func (r *DeviceRepository) UpdateLastSeen(id string, lastSeen time.Time) error {
return r.db.Model(&models.DeviceFingerprint{}).Where("id = ?", id).Update("last_seen_at", lastSeen).Error
}
3 changes: 3 additions & 0 deletions internal/routes/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ func SetupRoutes(router *gin.Engine, db *gorm.DB, redisClient *redis.Client, cfg
oauthTokenRepo := repository.NewOAuthTokenRepository(db)
userConsentRepo := repository.NewUserConsentRepository(db)
oauthProviderConfigRepo := repository.NewOAuthProviderConfigRepository(db)
deviceRepo := repository.NewDeviceRepository(db)

// Initialize services
tokenService := service.NewTokenService(cfg)
Expand All @@ -53,6 +54,7 @@ func SetupRoutes(router *gin.Engine, db *gorm.DB, redisClient *redis.Client, cfg
emailService,
auditService,
mfaService,
deviceRepo,
cfg,
)

Expand Down Expand Up @@ -151,6 +153,7 @@ func SetupRoutes(router *gin.Engine, db *gorm.DB, redisClient *redis.Client, cfg
auth.POST("/resend-verification", authHandler.ResendVerification)
auth.POST("/forgot-password", authHandler.ForgotPassword)
auth.POST("/reset-password", authHandler.ResetPassword)
auth.GET("/lock-account", authHandler.LockAccount)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Avoid a destructive action on a GET route.

This endpoint locks the account, and the alert email links to it directly. Mail scanners, link preview bots, and safe-browsing crawlers routinely fetch GET links automatically, which can lock accounts without the user ever choosing to do so. Make the email land on a confirmation page and perform the lock with a POST/one-time form submission instead.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/routes/routes.go` at line 156, The auth.GET("/lock-account",
authHandler.LockAccount) route should not perform the destructive lock action
directly because it can be triggered by scanners or previews. Update the
LockAccount flow in authHandler so the GET endpoint only lands on a confirmation
page, then move the actual account-lock operation to a POST-based one-time
submission handled by the same LockAccount logic or a dedicated confirm action.
Ensure the email link points to the safe confirmation page rather than executing
the lock immediately.


// WebAuthn Login
auth.POST("/webauthn/login/begin", webAuthnHandler.BeginLogin)
Expand Down
66 changes: 66 additions & 0 deletions internal/service/auth_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ type AuthService struct {
emailService EmailSender
auditService *AuditService
mfaService *MFAService
deviceRepo *repository.DeviceRepository
config *config.Config
}

Expand All @@ -60,6 +61,7 @@ func NewAuthService(
emailService EmailSender,
auditService *AuditService,
mfaService *MFAService,
deviceRepo *repository.DeviceRepository,
cfg *config.Config,
) *AuthService {
return &AuthService{
Expand All @@ -72,6 +74,7 @@ func NewAuthService(
emailService: emailService,
auditService: auditService,
mfaService: mfaService,
deviceRepo: deviceRepo,
config: cfg,
}
}
Expand Down Expand Up @@ -299,6 +302,28 @@ func (s *AuthService) DeleteAccount(userID string) error {
return nil
}

// LockAccount locks the user account indefinitely using a valid lock token
func (s *AuthService) LockAccount(tokenString string) error {
userID, err := s.tokenService.ValidateLockToken(tokenString)
if err != nil {
return errors.New("invalid or expired lock token")
}

// Lock the account (100 years)
lockedUntil := time.Now().Add(100 * 365 * 24 * time.Hour)
if err := s.userRepo.LockUser(userID, lockedUntil); err != nil {
return errors.New("failed to lock account")
Comment on lines +307 to +315

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Return typed lock-account errors here.

These ad-hoc errors.New(...) values erase whether the failure was token validation or an internal repository problem, so the handler cannot do the required 4xx/5xx mapping for this endpoint. Please return service-defined errors from service/errors.go instead.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/service/auth_service.go` around lines 307 - 315, Return typed
service errors from the lock-account flow instead of ad-hoc errors.New values.
In AuthService’s lock logic around ValidateLockToken and LockUser, map token
validation failures to the existing invalid/expired lock-token error from
service/errors.go, and map repository failures to the service-defined internal
lock-account error so the handler can distinguish 4xx from 5xx responses. Keep
the control flow the same, but replace the generic error creation with the
shared error values used elsewhere in the service package.

Source: Coding guidelines

}

// Revoke all existing sessions for security
s.tokenRepo.RevokeAllUserTokens(userID)

// Audit Log
s.auditService.LogEvent(&userID, "ACCOUNT_LOCKED_BY_USER", "USER", userID, "", "", map[string]interface{}{"reason": "unrecognized_device_alert"})
Comment on lines +318 to +322

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Don't report success before session revocation succeeds.

RevokeAllUserTokens returns an error, but this method ignores it and still returns nil. If that update fails, the account is marked locked while existing refresh tokens remain valid.

Suggested fix
-	// Revoke all existing sessions for security
-	s.tokenRepo.RevokeAllUserTokens(userID)
+	// Revoke all existing sessions for security
+	if err := s.tokenRepo.RevokeAllUserTokens(userID); err != nil {
+		return errors.New("failed to revoke user sessions")
+	}
 
 	// Audit Log
-	s.auditService.LogEvent(&userID, "ACCOUNT_LOCKED_BY_USER", "USER", userID, "", "", map[string]interface{}{"reason": "unrecognized_device_alert"})
+	if err := s.auditService.LogEvent(&userID, "ACCOUNT_LOCKED_BY_USER", "USER", userID, "", "", map[string]interface{}{"reason": "unrecognized_device_alert"}); err != nil {
+		log.Printf("failed to write audit log for locked account %s: %v", userID, err)
+	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Revoke all existing sessions for security
s.tokenRepo.RevokeAllUserTokens(userID)
// Audit Log
s.auditService.LogEvent(&userID, "ACCOUNT_LOCKED_BY_USER", "USER", userID, "", "", map[string]interface{}{"reason": "unrecognized_device_alert"})
// Revoke all existing sessions for security
if err := s.tokenRepo.RevokeAllUserTokens(userID); err != nil {
return errors.New("failed to revoke user sessions")
}
// Audit Log
if err := s.auditService.LogEvent(&userID, "ACCOUNT_LOCKED_BY_USER", "USER", userID, "", "", map[string]interface{}{"reason": "unrecognized_device_alert"}); err != nil {
log.Printf("failed to write audit log for locked account %s: %v", userID, err)
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/service/auth_service.go` around lines 318 - 322, In the account-lock
flow in auth_service’s session revocation path, the error from
RevokeAllUserTokens is currently ignored, so the method can report success even
when tokens were not revoked. Update the method that handles locking the account
to check and propagate the RevokeAllUserTokens error before continuing, and only
write the audit event or return nil after revocation succeeds.


return nil
}

// EnableMFA generates a secret and returns it with QR code URL
func (s *AuthService) EnableMFA(userID string) (*dto.MFAEnableResponse, error) {
user, err := s.userRepo.FindByID(userID)
Expand Down Expand Up @@ -616,6 +641,9 @@ func (s *AuthService) ProcessPostLogin(ctx context.Context, user *models.User, i
log.Printf("Failed to update last login for user %s: %v", user.ID, err)
}

// Check device fingerprint
s.handleDeviceFingerprint(user, ipAddress, userAgent)

response, err := s.CreateLoginResponse(user, ipAddress, userAgent)
if err != nil {
return nil, err
Expand All @@ -629,6 +657,44 @@ func (s *AuthService) ProcessPostLogin(ctx context.Context, user *models.User, i
return response, nil
}

func (s *AuthService) handleDeviceFingerprint(user *models.User, ipAddress, userAgent string) {
fingerprintHash := utils.GenerateDeviceFingerprint(userAgent, ipAddress)
device, err := s.deviceRepo.FindByFingerprint(user.ID, fingerprintHash)
if err != nil {
log.Printf("Error checking device fingerprint: %v", err)
}

if device == nil {
// Unrecognized device
newDevice := &models.DeviceFingerprint{
UserID: user.ID,
FingerprintHash: fingerprintHash,
UserAgent: userAgent,
IPAddress: ipAddress,
LastSeenAt: time.Now(),
}
if err := s.deviceRepo.Create(newDevice); err != nil {
log.Printf("Failed to save device fingerprint: %v", err)
}

// Generate lock token and send alert email asynchronously
go func(u *models.User, ip, agent string) {
lockToken, err := s.tokenService.GenerateLockToken(u.ID)
if err != nil {
log.Printf("Failed to generate lock token for unrecognized login alert: %v", err)
return
}
err = s.emailService.SendUnrecognizedLoginAlert(u.Email, ip, agent, lockToken, s.config.App.URL)
if err != nil {
log.Printf("Failed to send unrecognized login alert to %s: %v", u.Email, err)
}
}(user, ipAddress, userAgent)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
} else {
// Update last seen
s.deviceRepo.UpdateLastSeen(device.ID, time.Now())
}
}

// LoginWithOAuth handles login or registration via OAuth provider
func (s *AuthService) LoginWithOAuth(email, oauthID, firstName, lastName, provider, ipAddress, userAgent string) (*dto.LoginResponse, error) {
user, err := s.userRepo.FindByEmail(email)
Expand Down
31 changes: 28 additions & 3 deletions internal/service/email_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,12 @@ import (
"github.com/roshankumar0036singh/auth-server/internal/config"
)

const appName = "Auth Server"

type EmailSender interface {
SendVerificationEmail(email, token, appURL string) error
SendPasswordResetEmail(email, token, appURL string) error
SendUnrecognizedLoginAlert(email, ip, userAgent, lockToken, appURL string) error
}

type EmailService struct {
Expand Down Expand Up @@ -55,7 +58,10 @@ func (s *EmailService) SendEmail(to []string, subject string, templateName strin
message += "\r\n" + body.String()

// Authenticate
auth := smtp.PlainAuth("", s.config.SMTPUser, s.config.SMTPPassword, s.config.SMTPHost)
var auth smtp.Auth
if s.config.SMTPUser != "" && s.config.SMTPPassword != "" {
auth = smtp.PlainAuth("", s.config.SMTPUser, s.config.SMTPPassword, s.config.SMTPHost)
}

// Send email
addr := fmt.Sprintf("%s:%d", s.config.SMTPHost, s.config.SMTPPort)
Expand All @@ -76,7 +82,7 @@ func (s *EmailService) SendVerificationEmail(email, token, appURL string) error
AppName string
}{
VerifyURL: verifyURL,
AppName: "Auth Server",
AppName: appName,
}

return s.SendEmail([]string{email}, "Verify your email", "verify_email.html", data)
Expand All @@ -92,8 +98,27 @@ func (s *EmailService) SendPasswordResetEmail(email, token, appURL string) error
AppName string
}{
ResetURL: resetURL,
AppName: "Auth Server",
AppName: appName,
}

return s.SendEmail([]string{email}, "Reset your password", "reset_password.html", data)
}

// SendUnrecognizedLoginAlert sends an alert for an unfamiliar device login
func (s *EmailService) SendUnrecognizedLoginAlert(email, ip, userAgent, lockToken, appURL string) error {
lockURL := fmt.Sprintf("%s/api/auth/lock-account?token=%s", appURL, lockToken)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Avoid one-click state-changing GET lock links in email alerts.

Line 107 builds a direct /api/auth/lock-account?token=... link. Since that endpoint performs the lock action, mail-client link prefetch/scanning can unintentionally lock accounts. Send users to a non-mutating confirmation page first, then execute lock via explicit confirmed action (e.g., POST).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/service/email_service.go` at line 107, The lock-account email link
in EmailService should not point directly to the state-changing API endpoint.
Update EmailService’s lockURL construction so the email sends users to a
non-mutating confirmation page first, and have the actual account lock happen
only after an explicit confirmed action such as a POST handled by the auth flow.


data := struct {
IPAddress string
UserAgent string
LockURL string
AppName string
}{
IPAddress: ip,
UserAgent: userAgent,
LockURL: lockURL,
AppName: appName,
}

return s.SendEmail([]string{email}, "Security Alert: Unrecognized Login", "unrecognized_login.html", data)
}
Loading
Loading