mirror of
https://github.com/NoFxAiOS/nofx.git
synced 2026-06-06 05:51:19 +08:00
refactor(auth): remove OTP flows from login/register/reset
This commit is contained in:
@@ -1,252 +0,0 @@
|
||||
package api
|
||||
|
||||
import (
|
||||
"testing"
|
||||
)
|
||||
|
||||
// MockUser Mock user structure
|
||||
type MockUser struct {
|
||||
ID int
|
||||
Email string
|
||||
OTPSecret string
|
||||
OTPVerified bool
|
||||
}
|
||||
|
||||
// TestOTPRefetchLogic Test OTP refetch logic
|
||||
func TestOTPRefetchLogic(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
existingUser *MockUser
|
||||
userExists bool
|
||||
expectedAction string // "allow_refetch", "reject_duplicate", "create_new"
|
||||
expectedMessage string
|
||||
}{
|
||||
{
|
||||
name: "New user registration - email does not exist",
|
||||
existingUser: nil,
|
||||
userExists: false,
|
||||
expectedAction: "create_new",
|
||||
expectedMessage: "Create new user",
|
||||
},
|
||||
{
|
||||
name: "Incomplete OTP verification - allow refetch",
|
||||
existingUser: &MockUser{
|
||||
ID: 1,
|
||||
Email: "test@example.com",
|
||||
OTPSecret: "SECRET123",
|
||||
OTPVerified: false,
|
||||
},
|
||||
userExists: true,
|
||||
expectedAction: "allow_refetch",
|
||||
expectedMessage: "Incomplete registration detected, please continue OTP setup",
|
||||
},
|
||||
{
|
||||
name: "Completed OTP verification - reject duplicate registration",
|
||||
existingUser: &MockUser{
|
||||
ID: 2,
|
||||
Email: "verified@example.com",
|
||||
OTPSecret: "SECRET456",
|
||||
OTPVerified: true,
|
||||
},
|
||||
userExists: true,
|
||||
expectedAction: "reject_duplicate",
|
||||
expectedMessage: "Email already registered",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
// Simulate logic processing flow
|
||||
var actualAction string
|
||||
var actualMessage string
|
||||
|
||||
if !tt.userExists {
|
||||
// User does not exist, create new user
|
||||
actualAction = "create_new"
|
||||
actualMessage = "Create new user"
|
||||
} else {
|
||||
// User exists, check OTP verification status
|
||||
if !tt.existingUser.OTPVerified {
|
||||
// OTP verification incomplete, allow refetch
|
||||
actualAction = "allow_refetch"
|
||||
actualMessage = "Incomplete registration detected, please continue OTP setup"
|
||||
} else {
|
||||
// Verification completed, reject duplicate registration
|
||||
actualAction = "reject_duplicate"
|
||||
actualMessage = "Email already registered"
|
||||
}
|
||||
}
|
||||
|
||||
// Verify results
|
||||
if actualAction != tt.expectedAction {
|
||||
t.Errorf("Action mismatch: got %s, want %s", actualAction, tt.expectedAction)
|
||||
}
|
||||
if actualMessage != tt.expectedMessage {
|
||||
t.Errorf("Message mismatch: got %s, want %s", actualMessage, tt.expectedMessage)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestOTPVerificationStates Test OTP verification state determination
|
||||
func TestOTPVerificationStates(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
otpVerified bool
|
||||
shouldAllowRefetch bool
|
||||
}{
|
||||
{
|
||||
name: "OTP verified - disallow refetch",
|
||||
otpVerified: true,
|
||||
shouldAllowRefetch: false,
|
||||
},
|
||||
{
|
||||
name: "OTP not verified - allow refetch",
|
||||
otpVerified: false,
|
||||
shouldAllowRefetch: true,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
// Simulate verification logic
|
||||
allowRefetch := !tt.otpVerified
|
||||
|
||||
if allowRefetch != tt.shouldAllowRefetch {
|
||||
t.Errorf("Refetch logic error: OTPVerified=%v, allowRefetch=%v, expected=%v",
|
||||
tt.otpVerified, allowRefetch, tt.shouldAllowRefetch)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestRegistrationFlow Test complete registration flow logic branches
|
||||
func TestRegistrationFlow(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
scenario string
|
||||
userExists bool
|
||||
otpVerified bool
|
||||
expectHTTPCode int // Simulated HTTP status code
|
||||
expectResponse string
|
||||
}{
|
||||
{
|
||||
name: "Scenario 1: New user first registration",
|
||||
scenario: "New user first accesses registration endpoint",
|
||||
userExists: false,
|
||||
otpVerified: false,
|
||||
expectHTTPCode: 200,
|
||||
expectResponse: "Create user and return OTP setup information",
|
||||
},
|
||||
{
|
||||
name: "Scenario 2: User re-accesses after interrupting registration",
|
||||
scenario: "User registered previously but did not complete OTP setup, now re-accessing",
|
||||
userExists: true,
|
||||
otpVerified: false,
|
||||
expectHTTPCode: 200,
|
||||
expectResponse: "Return existing user's OTP information, allow continuation",
|
||||
},
|
||||
{
|
||||
name: "Scenario 3: Registered user attempts duplicate registration",
|
||||
scenario: "User already completed registration, attempts to register again with same email",
|
||||
userExists: true,
|
||||
otpVerified: true,
|
||||
expectHTTPCode: 409, // Conflict
|
||||
expectResponse: "Email already registered",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
// Simulate registration flow logic
|
||||
var actualHTTPCode int
|
||||
var actualResponse string
|
||||
|
||||
if !tt.userExists {
|
||||
// New user, create and return OTP information
|
||||
actualHTTPCode = 200
|
||||
actualResponse = "Create user and return OTP setup information"
|
||||
} else {
|
||||
// User exists
|
||||
if !tt.otpVerified {
|
||||
// OTP verification incomplete, allow refetch
|
||||
actualHTTPCode = 200
|
||||
actualResponse = "Return existing user's OTP information, allow continuation"
|
||||
} else {
|
||||
// Verification completed, reject duplicate registration
|
||||
actualHTTPCode = 409
|
||||
actualResponse = "Email already registered"
|
||||
}
|
||||
}
|
||||
|
||||
// Verify
|
||||
if actualHTTPCode != tt.expectHTTPCode {
|
||||
t.Errorf("HTTP code mismatch: got %d, want %d (scenario: %s)",
|
||||
actualHTTPCode, tt.expectHTTPCode, tt.scenario)
|
||||
}
|
||||
if actualResponse != tt.expectResponse {
|
||||
t.Errorf("Response mismatch: got %s, want %s (scenario: %s)",
|
||||
actualResponse, tt.expectResponse, tt.scenario)
|
||||
}
|
||||
|
||||
t.Logf("✓ %s: HTTP %d, %s", tt.scenario, actualHTTPCode, actualResponse)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestEdgeCases Test edge cases
|
||||
func TestEdgeCases(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
user *MockUser
|
||||
expectAllow bool
|
||||
description string
|
||||
}{
|
||||
{
|
||||
name: "User ID is 0 - treated as new user",
|
||||
user: &MockUser{
|
||||
ID: 0,
|
||||
Email: "new@example.com",
|
||||
OTPVerified: false,
|
||||
},
|
||||
expectAllow: true,
|
||||
description: "ID of 0 usually indicates user has not been created yet",
|
||||
},
|
||||
{
|
||||
name: "OTPSecret is empty - still can refetch",
|
||||
user: &MockUser{
|
||||
ID: 1,
|
||||
Email: "test@example.com",
|
||||
OTPSecret: "",
|
||||
OTPVerified: false,
|
||||
},
|
||||
expectAllow: true,
|
||||
description: "Even if OTPSecret is empty, as long as not verified, refetch is allowed",
|
||||
},
|
||||
{
|
||||
name: "OTPSecret exists but already verified - not allowed",
|
||||
user: &MockUser{
|
||||
ID: 2,
|
||||
Email: "verified@example.com",
|
||||
OTPSecret: "SECRET789",
|
||||
OTPVerified: true,
|
||||
},
|
||||
expectAllow: false,
|
||||
description: "Users with verified OTP cannot refetch",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
// Core logic: as long as OTPVerified is false, refetch is allowed
|
||||
allowRefetch := !tt.user.OTPVerified
|
||||
|
||||
if allowRefetch != tt.expectAllow {
|
||||
t.Errorf("Edge case failed: %s\nUser: ID=%d, OTPVerified=%v\nExpected allow=%v, got=%v",
|
||||
tt.description, tt.user.ID, tt.user.OTPVerified, tt.expectAllow, allowRefetch)
|
||||
}
|
||||
|
||||
t.Logf("✓ %s", tt.description)
|
||||
})
|
||||
}
|
||||
}
|
||||
142
api/server.go
142
api/server.go
@@ -142,8 +142,6 @@ func (s *Server) setupRoutes() {
|
||||
// Authentication related routes (no authentication required)
|
||||
api.POST("/register", s.handleRegister)
|
||||
api.POST("/login", s.handleLogin)
|
||||
api.POST("/verify-otp", s.handleVerifyOTP)
|
||||
api.POST("/complete-registration", s.handleCompleteRegistration)
|
||||
|
||||
// Routes requiring authentication
|
||||
protected := api.Group("/", s.authMiddleware())
|
||||
@@ -3095,29 +3093,9 @@ func (s *Server) handleRegister(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
|
||||
// Check if email already exists (must check before maxUsers to allow incomplete OTP users)
|
||||
existingUser, err := s.store.User().GetByEmail(req.Email)
|
||||
// Check if email already exists
|
||||
_, err := s.store.User().GetByEmail(req.Email)
|
||||
if err == nil {
|
||||
// User exists, check OTP verification status
|
||||
if !existingUser.OTPVerified {
|
||||
// OTP not verified, verify password first for security
|
||||
if !auth.CheckPassword(req.Password, existingUser.PasswordHash) {
|
||||
c.JSON(http.StatusUnauthorized, gin.H{"error": "Email or password incorrect"})
|
||||
return
|
||||
}
|
||||
// Password correct, allow user to continue OTP setup
|
||||
// Return existing OTP information
|
||||
qrCodeURL := auth.GetOTPQRCodeURL(existingUser.OTPSecret, req.Email)
|
||||
c.JSON(http.StatusOK, gin.H{
|
||||
"user_id": existingUser.ID,
|
||||
"email": existingUser.Email,
|
||||
"otp_secret": existingUser.OTPSecret,
|
||||
"qr_code_url": qrCodeURL,
|
||||
"message": "Incomplete registration detected, please continue OTP setup",
|
||||
})
|
||||
return
|
||||
}
|
||||
// OTP already verified, reject duplicate registration
|
||||
c.JSON(http.StatusConflict, gin.H{"error": "Email already registered"})
|
||||
return
|
||||
}
|
||||
@@ -3143,21 +3121,12 @@ func (s *Server) handleRegister(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
|
||||
// Generate OTP secret
|
||||
otpSecret, err := auth.GenerateOTPSecret()
|
||||
if err != nil {
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": "OTP secret generation failed"})
|
||||
return
|
||||
}
|
||||
|
||||
// Create user (unverified OTP status)
|
||||
// Create user
|
||||
userID := uuid.New().String()
|
||||
user := &store.User{
|
||||
ID: userID,
|
||||
Email: req.Email,
|
||||
PasswordHash: passwordHash,
|
||||
OTPSecret: otpSecret,
|
||||
OTPVerified: false,
|
||||
}
|
||||
|
||||
err = s.store.User().Create(user)
|
||||
@@ -3166,49 +3135,6 @@ func (s *Server) handleRegister(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
|
||||
// Return OTP setup information
|
||||
qrCodeURL := auth.GetOTPQRCodeURL(otpSecret, req.Email)
|
||||
c.JSON(http.StatusOK, gin.H{
|
||||
"user_id": userID,
|
||||
"email": req.Email,
|
||||
"otp_secret": otpSecret,
|
||||
"qr_code_url": qrCodeURL,
|
||||
"message": "Please scan the QR code with Google Authenticator and verify OTP",
|
||||
})
|
||||
}
|
||||
|
||||
// handleCompleteRegistration Complete registration (verify OTP)
|
||||
func (s *Server) handleCompleteRegistration(c *gin.Context) {
|
||||
var req struct {
|
||||
UserID string `json:"user_id" binding:"required"`
|
||||
OTPCode string `json:"otp_code" binding:"required"`
|
||||
}
|
||||
|
||||
if err := c.ShouldBindJSON(&req); err != nil {
|
||||
SafeBadRequest(c, "Invalid request parameters")
|
||||
return
|
||||
}
|
||||
|
||||
// Get user information
|
||||
user, err := s.store.User().GetByID(req.UserID)
|
||||
if err != nil {
|
||||
SafeNotFound(c, "User")
|
||||
return
|
||||
}
|
||||
|
||||
// Verify OTP
|
||||
if !auth.VerifyOTP(user.OTPSecret, req.OTPCode) {
|
||||
c.JSON(http.StatusBadRequest, gin.H{"error": "OTP code error"})
|
||||
return
|
||||
}
|
||||
|
||||
// Update user OTP verified status
|
||||
err = s.store.User().UpdateOTPVerified(req.UserID, true)
|
||||
if err != nil {
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to update user status"})
|
||||
return
|
||||
}
|
||||
|
||||
// Generate JWT token
|
||||
token, err := auth.GenerateJWT(user.ID, user.Email)
|
||||
if err != nil {
|
||||
@@ -3226,7 +3152,7 @@ func (s *Server) handleCompleteRegistration(c *gin.Context) {
|
||||
"token": token,
|
||||
"user_id": user.ID,
|
||||
"email": user.Email,
|
||||
"message": "Registration completed",
|
||||
"message": "Registration successful",
|
||||
})
|
||||
}
|
||||
|
||||
@@ -3255,56 +3181,7 @@ func (s *Server) handleLogin(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
|
||||
// Check if OTP is verified
|
||||
if !user.OTPVerified {
|
||||
// Return OTP info so user can complete setup
|
||||
qrCodeURL := auth.GetOTPQRCodeURL(user.OTPSecret, user.Email)
|
||||
c.JSON(http.StatusOK, gin.H{
|
||||
"user_id": user.ID,
|
||||
"email": user.Email,
|
||||
"otp_secret": user.OTPSecret,
|
||||
"qr_code_url": qrCodeURL,
|
||||
"requires_otp_setup": true,
|
||||
"message": "Please complete OTP setup first",
|
||||
})
|
||||
return
|
||||
}
|
||||
|
||||
// Return status requiring OTP verification
|
||||
c.JSON(http.StatusOK, gin.H{
|
||||
"user_id": user.ID,
|
||||
"email": user.Email,
|
||||
"message": "Please enter Google Authenticator code",
|
||||
"requires_otp": true,
|
||||
})
|
||||
}
|
||||
|
||||
// handleVerifyOTP Verify OTP and complete login
|
||||
func (s *Server) handleVerifyOTP(c *gin.Context) {
|
||||
var req struct {
|
||||
UserID string `json:"user_id" binding:"required"`
|
||||
OTPCode string `json:"otp_code" binding:"required"`
|
||||
}
|
||||
|
||||
if err := c.ShouldBindJSON(&req); err != nil {
|
||||
SafeBadRequest(c, "Invalid request parameters")
|
||||
return
|
||||
}
|
||||
|
||||
// Get user information
|
||||
user, err := s.store.User().GetByID(req.UserID)
|
||||
if err != nil {
|
||||
SafeNotFound(c, "User")
|
||||
return
|
||||
}
|
||||
|
||||
// Verify OTP
|
||||
if !auth.VerifyOTP(user.OTPSecret, req.OTPCode) {
|
||||
c.JSON(http.StatusBadRequest, gin.H{"error": "Verification code error"})
|
||||
return
|
||||
}
|
||||
|
||||
// Generate JWT token
|
||||
// Issue token directly after password verification.
|
||||
token, err := auth.GenerateJWT(user.ID, user.Email)
|
||||
if err != nil {
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to generate token"})
|
||||
@@ -3319,12 +3196,11 @@ func (s *Server) handleVerifyOTP(c *gin.Context) {
|
||||
})
|
||||
}
|
||||
|
||||
// handleResetPassword Reset password (via email + OTP verification)
|
||||
// handleResetPassword Reset password via email and new password
|
||||
func (s *Server) handleResetPassword(c *gin.Context) {
|
||||
var req struct {
|
||||
Email string `json:"email" binding:"required,email"`
|
||||
NewPassword string `json:"new_password" binding:"required,min=6"`
|
||||
OTPCode string `json:"otp_code" binding:"required"`
|
||||
}
|
||||
|
||||
if err := c.ShouldBindJSON(&req); err != nil {
|
||||
@@ -3339,12 +3215,6 @@ func (s *Server) handleResetPassword(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
|
||||
// Verify OTP
|
||||
if !auth.VerifyOTP(user.OTPSecret, req.OTPCode) {
|
||||
c.JSON(http.StatusBadRequest, gin.H{"error": "Google Authenticator code error"})
|
||||
return
|
||||
}
|
||||
|
||||
// Generate new password hash
|
||||
newPasswordHash, err := auth.HashPassword(req.NewPassword)
|
||||
if err != nil {
|
||||
|
||||
Reference in New Issue
Block a user