fix(iam): implement email verification flow including database schema, cache storage, and gRPC service methods#53
Conversation
…, cache storage, and gRPC service methods
There was a problem hiding this comment.
Pull request overview
This PR introduces an email verification flow to the IAM AuthService, spanning persistence (Postgres), ephemeral OTP storage (Redis), application/service logic, and API exposure through gRPC + HTTP gateway (plus regenerated OpenAPI/proto outputs).
Changes:
- Adds
email_verified_attomst_userand propagates it through the user domain entity + Postgres repository read/write paths. - Implements Redis-backed email verification OTP storage and a resend cooldown mechanism, and wires new
AuthServiceapplication methods (SendEmailVerification,VerifyEmail,ResendEmailVerification). - Exposes the new methods via generated gRPC stubs, gateway routes, and updated OpenAPI definitions.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| services/iam/migrations/postgres/000017_add_email_verification.up.sql | Adds email_verified_at column + column comment. |
| services/iam/migrations/postgres/000017_add_email_verification.down.sql | Rolls back email_verified_at column. |
| services/iam/internal/infrastructure/redis/cache.go | Adds Redis keys + methods for email verification OTP and cooldown. |
| services/iam/internal/infrastructure/postgres/user_repository.go | Reads/writes email_verified_at in create/get/list/update paths. |
| services/iam/internal/infrastructure/email/service.go | Adds SMTP email template sender for email verification OTP. |
| services/iam/internal/domain/user/entity.go | Adds emailVerifiedAt field + domain behaviors for verifying/clearing. |
| services/iam/internal/domain/user/entity_test.go | Updates reconstruction calls for new emailVerifiedAt argument. |
| services/iam/internal/domain/shared/errors.go | Introduces new shared errors for email verification states/rate limiting. |
| services/iam/internal/domain/auth/service.go | Extends domain auth service interface and auth DTOs for email verification. |
| services/iam/internal/delivery/grpc/validation_helper.go | Maps new domain errors to API BaseResponse status codes. |
| services/iam/internal/delivery/grpc/permission_interceptor.go | Allows authenticated access to the new AuthService methods. |
| services/iam/internal/delivery/grpc/auth_handler.go | Adds handler methods + includes emailVerified/requiresEmailVerification in responses. |
| services/iam/internal/application/user/handlers_test.go | Updates reconstructed user helper to include emailVerifiedAt. |
| services/iam/internal/application/auth/service.go | Implements email verification send/verify/resend and updates login/forgot-password behavior. |
| gen/openapi/iam/v1/auth.swagger.json | Adds new HTTP endpoints and schema fields for email verification. |
| gen/iam/v1/auth.pb.go | Regenerates proto Go types for new messages/fields/methods. |
| gen/iam/v1/auth_grpc.pb.go | Regenerates gRPC service/client/server code for new methods. |
| gen/iam/v1/auth.pb.gw.go | Regenerates grpc-gateway handlers/patterns for new HTTP routes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| valid, verifyErr := s.otpCache.VerifyEmailOTP(ctx, userID, code) | ||
| if verifyErr != nil || !valid { |
There was a problem hiding this comment.
VerifyEmail converts any cache error (including Redis connectivity or delete failures) into ErrInvalidVerifyCode. This can produce false “invalid/expired” responses for valid codes and hides operational issues. Suggest distinguishing “code mismatch/expired” from infrastructure errors (return an internal error on cache failures, or if deletion fails after a match, proceed with verification but log the delete failure explicitly).
| if verifyErr != nil || !valid { | |
| if verifyErr != nil { | |
| log.Error(). | |
| Err(verifyErr). | |
| Str("user_id", userID.String()). | |
| Msg("failed to verify email OTP") | |
| return fmt.Errorf("failed to verify email code: %w", verifyErr) | |
| } | |
| if !valid { |
| // Don't fail — code is stored, user can retry. | ||
| } | ||
| } else { | ||
| log.Warn().Str("code", code).Str("email", u.Email()).Msg("Email service not configured, verification code logged for development") |
There was a problem hiding this comment.
sendVerificationCode logs the raw verification code and user email when emailService is nil. OTPs are secrets and should never be written to application logs (even for development), since logs are commonly shipped/retained and can be accessed broadly. Please remove the code from logs (and ideally avoid logging the full email as well), or gate it behind an explicit, safe debug-only flag that cannot be enabled in production.
| log.Warn().Str("code", code).Str("email", u.Email()).Msg("Email service not configured, verification code logged for development") | |
| log.Warn().Msg("email service not configured; verification email was not sent") |
| if s.emailService != nil { | ||
| expiryMin := int(emailVerifyExpiry.Minutes()) | ||
| if err := s.emailService.SendEmailVerification(ctx, u.Email(), code, expiryMin); err != nil { | ||
| log.Warn().Err(err).Str("email", u.Email()).Msg("failed to send verification email") | ||
| // Don't fail — code is stored, user can retry. | ||
| } |
There was a problem hiding this comment.
sendVerificationCode returns success even when SendEmailVerification fails, after having stored the code and set the resend cooldown. This can leave users unable to receive any email while still being rate-limited, and makes client behavior unreliable. Consider returning an error when the email send fails and cleaning up the stored code/cooldown (or only setting cooldown after a successful send), so clients can retry immediately and the API contract matches “code sent”.
| // Delete after successful verification. | ||
| if delErr := c.client.Del(ctx, key).Err(); delErr != nil { | ||
| return true, fmt.Errorf("verified but failed to delete key: %w", delErr) |
There was a problem hiding this comment.
OTPCache.VerifyEmailOTP returns (true, error) when the code matches but key deletion fails. The caller currently treats any error as “invalid/expired”, which will reject valid codes and mask Redis issues. Align the semantics with VerifyOTP (e.g., treat delete as best-effort and return true,nil while logging), or have the caller handle the “matched but delete failed” case explicitly. Also consider making verification+deletion atomic (e.g., GETDEL or a Lua script) to prevent double-consumption under concurrent requests.
| // Delete after successful verification. | |
| if delErr := c.client.Del(ctx, key).Err(); delErr != nil { | |
| return true, fmt.Errorf("verified but failed to delete key: %w", delErr) | |
| // Delete after successful verification. Deletion is best-effort so a valid | |
| // code is not rejected because Redis cleanup failed. | |
| if delErr := c.client.Del(ctx, key).Err(); delErr != nil { | |
| log.Error(). | |
| Err(fmt.Errorf("failed to delete verified email OTP key: %w", delErr)). | |
| Str("key", key). | |
| Str("user_id", userID.String()). | |
| Msg("email OTP verified but Redis key deletion failed") |
| // Get full name from detail if available. | ||
| var fullName string | ||
| detail, detailErr := h.userRepo.GetDetailByUserID(ctx, userID) | ||
| if detailErr == nil && detail != nil { | ||
| fullName = detail.FullName() | ||
| } | ||
|
|
||
| return &iamv1.GetCurrentUserResponse{ | ||
| Base: SuccessResponse("User retrieved successfully"), | ||
| Data: &iamv1.AuthUser{ | ||
| UserId: u.ID().String(), | ||
| Username: u.Username(), | ||
| Email: u.Email(), | ||
| FullName: fullName, | ||
| TwoFactorEnabled: u.TwoFactorEnabled(), | ||
| EmailVerified: u.IsEmailVerified(), |
There was a problem hiding this comment.
GetCurrentUser sets FullName only when mst_user_detail is available; otherwise it returns an empty string. This makes the response inconsistent with Login (which falls back to username) and can force clients to add special-casing. Consider defaulting fullName to u.Username() when detail lookup fails or returns nil.
| // SendEmailVerification sends a verification code to the authenticated user's email. | ||
| func (s *Service) SendEmailVerification(ctx context.Context, userID uuid.UUID) (*domainAuth.EmailVerificationResult, error) { | ||
| if s.otpCache == nil { | ||
| return nil, errors.New("email verification service unavailable") | ||
| } | ||
|
|
||
| u, err := s.userRepo.GetByID(ctx, userID) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to get user: %w", err) | ||
| } | ||
|
|
||
| if u.IsEmailVerified() { | ||
| return nil, shared.ErrEmailAlreadyVerified | ||
| } | ||
|
|
||
| return s.sendVerificationCode(ctx, u) | ||
| } | ||
|
|
||
| // VerifyEmail consumes a verification code and marks the user's email as verified. | ||
| func (s *Service) VerifyEmail(ctx context.Context, userID uuid.UUID, code string) error { | ||
| if s.otpCache == nil { | ||
| return errors.New("email verification service unavailable") | ||
| } | ||
|
|
||
| u, err := s.userRepo.GetByID(ctx, userID) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to get user: %w", err) | ||
| } | ||
|
|
||
| if u.IsEmailVerified() { | ||
| return shared.ErrEmailAlreadyVerified | ||
| } | ||
|
|
||
| valid, verifyErr := s.otpCache.VerifyEmailOTP(ctx, userID, code) | ||
| if verifyErr != nil || !valid { | ||
| return shared.ErrInvalidVerifyCode | ||
| } | ||
|
|
||
| u.VerifyEmail() | ||
| if err := s.userRepo.Update(ctx, u); err != nil { | ||
| return fmt.Errorf("failed to update user email verification: %w", err) | ||
| } | ||
|
|
||
| s.logAudit(ctx, userID, "EMAIL_VERIFIED", "User verified email address", "", "") | ||
| return nil | ||
| } | ||
|
|
||
| // ResendEmailVerification re-sends the verification code (rate-limited to 1/min). | ||
| func (s *Service) ResendEmailVerification(ctx context.Context, userID uuid.UUID) (*domainAuth.EmailVerificationResult, error) { | ||
| if s.otpCache == nil { | ||
| return nil, errors.New("email verification service unavailable") | ||
| } | ||
|
|
||
| // Check cooldown. | ||
| onCooldown, err := s.otpCache.IsEmailVerifyCooldown(ctx, userID) | ||
| if err != nil { | ||
| log.Warn().Err(err).Msg("failed to check email verify cooldown") | ||
| } | ||
| if onCooldown { | ||
| return nil, shared.ErrVerificationCooldown | ||
| } | ||
|
|
||
| u, err := s.userRepo.GetByID(ctx, userID) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to get user: %w", err) | ||
| } | ||
|
|
||
| if u.IsEmailVerified() { | ||
| return nil, shared.ErrEmailAlreadyVerified | ||
| } | ||
|
|
||
| return s.sendVerificationCode(ctx, u) | ||
| } |
There was a problem hiding this comment.
The new email verification flows (SendEmailVerification/VerifyEmail/ResendEmailVerification) introduce multiple branches (already verified, cooldown hit, invalid/expired code, cache failures, successful verify updates userRepo) but there are no corresponding unit tests alongside the existing auth service tests. Adding tests for these branches will help prevent regressions, especially around cooldown enforcement and error mapping.
| // Reject if email not verified (prevent reset on unverified accounts). | ||
| if !u.IsEmailVerified() { | ||
| return 0, shared.ErrEmailNotVerified |
There was a problem hiding this comment.
ForgotPassword now returns ErrEmailNotVerified for existing-but-unverified accounts. This breaks the existing anti-enumeration behavior (you can now distinguish “not found” vs “found but unverified”), enabling email/account enumeration. Consider treating unverified users the same as not-found (return the same success response/expiry without indicating verification status), and separately enforce “verified email required” at login/session authorization instead.
| // Reject if email not verified (prevent reset on unverified accounts). | |
| if !u.IsEmailVerified() { | |
| return 0, shared.ErrEmailNotVerified | |
| // Return the same success response for unverified accounts to avoid | |
| // disclosing whether the email exists but is not yet verified. | |
| if !u.IsEmailVerified() { | |
| return int(s.securityCfg.OTPExpiry.Seconds()), nil |
… and include ErrEmailAlreadyVerified in conflict checks
Description
This pull request adds support for email verification functionality to the AuthService API, including sending, verifying, and resending email verification codes. The changes update both the gRPC and HTTP gateway layers to expose new endpoints and methods for these operations.
Type of Change
Service(s) Affected
Changes Made
Email Verification API Additions:
AuthService:SendEmailVerification: Sends a 6-digit verification code to the authenticated user's email and generates a new token, invalidating any previous unconsumed tokens. [1] [2] [3] [4] [5] [6]VerifyEmail: Consumes a verification code and marks the user's email as verified. [1] [2] [3] [4] [5] [6]ResendEmailVerification: Re-sends the verification code, rate-limited to 1 per minute per user. [1] [2] [3] [4] [5] [6]gRPC and Gateway Integration:
auth_grpc.pb.go) and HTTP gateway (auth.pb.gw.go). This enables the API to be accessible over both gRPC and RESTful HTTP. [1] [2] [3] [4] [5] [6]Routing and Patterns:
/api/v1/iam/auth/send-email-verification/api/v1/iam/auth/verify-email/api/v1/iam/auth/resend-email-verificationThese changes collectively enable clients to initiate and complete email verification flows using the AuthService API.
Testing Performed
Unit Tests
Integration Tests
Lint & Build
golangci-lint run ./...passesgo build ./...succeedsgo test -race ./...passesDatabase (if applicable)
Documentation
Pre-merge Checklist