Merge pull request #10 from shibbirmcc/feature/improve-login-handler-best-practices

feat: improve login handler with security best practices

Author: shibbirmcc

handlers/login.go

@@ -1,25 +1,100 @@
 package handlers
 
 import (
+	"log"
 	"net/http"
+	"strings"
 
 	"github.com/gin-gonic/gin"
 	"github.com/shibbirmcc/user-auth-and-permissions/models"
 )
 
+// LoginResponse represents the structure of a successful login response
+type LoginResponse struct {
+	Success bool   `json:"success"`
+	Message string `json:"message"`
+	Token   string `json:"token,omitempty"`
+}
+
+// ErrorResponse represents the structure of an error response
+type ErrorResponse struct {
+	Success bool   `json:"success"`
+	Message string `json:"message"`
+	Error   string `json:"error,omitempty"`
+}
+
 func (h *UserHandler) LoginUser(c *gin.Context) {
 	var input models.LoginRequest
 
+	// Bind and validate JSON input
 	if err := c.ShouldBindJSON(&input); err != nil {
-		c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
+		// Log the validation error for debugging (without sensitive data)
+		log.Printf("Login validation error for email %s: %v", input.Email, err)
+		
+		// Return user-friendly validation error
+		errorMsg := "Invalid input data"
+		if strings.Contains(err.Error(), "email") {
+			errorMsg = "Please provide a valid email address"
+		} else if strings.Contains(err.Error(), "password") {
+			errorMsg = "Password is required"
+		}
+		
+		c.JSON(http.StatusBadRequest, ErrorResponse{
+			Success: false,
+			Message: errorMsg,
+			Error:   "validation_failed",
+		})
+		return
+	}
+
+	// Additional input validation
+	if strings.TrimSpace(input.Email) == "" {
+		log.Printf("Login attempt with empty email from IP: %s", c.ClientIP())
+		c.JSON(http.StatusBadRequest, ErrorResponse{
+			Success: false,
+			Message: "Email is required",
+			Error:   "missing_email",
+		})
+		return
+	}
+
+	if strings.TrimSpace(input.Password) == "" {
+		log.Printf("Login attempt with empty password for email: %s from IP: %s", input.Email, c.ClientIP())
+		c.JSON(http.StatusBadRequest, ErrorResponse{
+			Success: false,
+			Message: "Password is required",
+			Error:   "missing_password",
+		})
 		return
 	}
 
+	// Attempt login
 	token, err := h.userLoginService.Login(input)
 	if err != nil {
-		c.JSON(http.StatusUnauthorized, gin.H{"error": err.Error()})
+		// Log failed login attempt for security monitoring
+		log.Printf("Failed login attempt for email: %s from IP: %s - Error: %v", input.Email, c.ClientIP(), err)
+		
+		// Return generic error message to prevent information disclosure
+		c.JSON(http.StatusUnauthorized, ErrorResponse{
+			Success: false,
+			Message: "Invalid email or password",
+			Error:   "authentication_failed",
+		})
 		return
 	}
 
-	c.JSON(http.StatusOK, gin.H{"token": token})
+	// Log successful login for security monitoring
+	log.Printf("Successful login for email: %s from IP: %s", input.Email, c.ClientIP())
+
+	// Set security headers
+	c.Header("X-Content-Type-Options", "nosniff")
+	c.Header("X-Frame-Options", "DENY")
+	c.Header("X-XSS-Protection", "1; mode=block")
+
+	// Return successful response
+	c.JSON(http.StatusOK, LoginResponse{
+		Success: true,
+		Message: "Login successful",
+		Token:   token,
+	})
 }

handlers/login_test.go

@@ -54,6 +54,19 @@ func TestLoginUser(t *testing.T) {
 		// Call the handler
 		handler.LoginUser(c)
 		assert.Equal(t, http.StatusOK, w.Code)
+		
+		// Verify response structure
+		var response LoginResponse
+		err := json.Unmarshal(w.Body.Bytes(), &response)
+		assert.NoError(t, err)
+		assert.True(t, response.Success)
+		assert.Equal(t, "Login successful", response.Message)
+		assert.NotEmpty(t, response.Token)
+		
+		// Verify security headers
+		assert.Equal(t, "nosniff", w.Header().Get("X-Content-Type-Options"))
+		assert.Equal(t, "DENY", w.Header().Get("X-Frame-Options"))
+		assert.Equal(t, "1; mode=block", w.Header().Get("X-XSS-Protection"))
 	})
 
 	t.Run("bad request with invalid JSON", func(t *testing.T) {
@@ -63,18 +76,6 @@ func TestLoginUser(t *testing.T) {
 		mockLoginService := services.NewUserLoginService(mockDBService)
 		handler := &UserHandler{userRegistrationService: *mockRegService, userLoginService: *mockLoginService}
 
-		user := &models.User{
-			Email:    mocks.TestUserEmail,
-			Password: mocks.TestUserPasswordHash,
-		}
-		userDetails := &models.UserDetail{
-			FirstName: mocks.TestUserFirstName,
-			LastName:  mocks.TestUserLastName,
-		}
-
-		mockDBService.On("FindUserByEmail", user.Email).Return(user, nil)
-		mockDBService.On("FindUserDetailsByUserID", user.ID).Return(userDetails, nil)
-
 		// Create a request with invalid JSON
 		req, _ := http.NewRequest(http.MethodPost, "/auth/login", bytes.NewBuffer([]byte("{invalid-json")))
 		req.Header.Set("Content-Type", "application/json")
@@ -89,10 +90,12 @@ func TestLoginUser(t *testing.T) {
 
 		// Assert the results
 		assert.Equal(t, http.StatusBadRequest, w.Code)
-		var response map[string]string
+		var response ErrorResponse
 		err := json.Unmarshal(w.Body.Bytes(), &response)
 		assert.NoError(t, err)
-		assert.Contains(t, response["error"], "invalid character")
+		assert.False(t, response.Success)
+		assert.Equal(t, "Invalid input data", response.Message)
+		assert.Equal(t, "validation_failed", response.Error)
 	})
 
 	t.Run("Invalid Email format Error Test", func(t *testing.T) {
@@ -124,10 +127,12 @@ func TestLoginUser(t *testing.T) {
 
 		// Assert the results
 		assert.Equal(t, http.StatusBadRequest, w.Code)
-		var response map[string]string
+		var response ErrorResponse
 		err := json.Unmarshal(w.Body.Bytes(), &response)
 		assert.NoError(t, err)
-		assert.Contains(t, response["error"], "Error:Field validation for 'Email' failed on the 'email' tag")
+		assert.False(t, response.Success)
+		assert.Equal(t, "Please provide a valid email address", response.Message)
+		assert.Equal(t, "validation_failed", response.Error)
 	})
 
 	t.Run("Unauthorized Login Test", func(t *testing.T) {
@@ -169,9 +174,110 @@ func TestLoginUser(t *testing.T) {
 
 		// Assert the results
 		assert.Equal(t, http.StatusUnauthorized, w.Code)
-		var response map[string]string
+		var response ErrorResponse
+		err := json.Unmarshal(w.Body.Bytes(), &response)
+		assert.NoError(t, err)
+		assert.False(t, response.Success)
+		assert.Equal(t, "Invalid email or password", response.Message)
+		assert.Equal(t, "authentication_failed", response.Error)
+	})
+
+	t.Run("Empty email validation", func(t *testing.T) {
+		mockDBService := new(mocks.MockDatabaseOperationService)
+		mockPasswordDeliveryService := &mocks.MockPasswordDeliveryService{ShouldFail: false}
+		mockRegService := services.NewUserRegistrationService(mockPasswordDeliveryService, mockDBService)
+		mockLoginService := services.NewUserLoginService(mockDBService)
+		handler := &UserHandler{userRegistrationService: *mockRegService, userLoginService: *mockLoginService}
+
+		// Sample request data with empty email (spaces are treated as invalid email format by binding validation)
+		loginRequest := models.LoginRequest{
+			Email:    "   ",
+			Password: "password123",
+		}
+
+		// Create a request
+		requestBody, _ := json.Marshal(loginRequest)
+		req, _ := http.NewRequest(http.MethodPost, "/login", bytes.NewBuffer(requestBody))
+		req.Header.Set("Content-Type", "application/json")
+
+		// Create a ResponseRecorder to capture the response
+		w := httptest.NewRecorder()
+		c, _ := gin.CreateTestContext(w)
+		c.Request = req
+
+		handler.LoginUser(c)
+
+		// Assert the results - binding validation catches this first as invalid email format
+		assert.Equal(t, http.StatusBadRequest, w.Code)
+		var response ErrorResponse
+		err := json.Unmarshal(w.Body.Bytes(), &response)
+		assert.NoError(t, err)
+		assert.False(t, response.Success)
+		assert.Equal(t, "Please provide a valid email address", response.Message)
+		assert.Equal(t, "validation_failed", response.Error)
+	})
+
+	t.Run("Missing email field validation", func(t *testing.T) {
+		mockDBService := new(mocks.MockDatabaseOperationService)
+		mockPasswordDeliveryService := &mocks.MockPasswordDeliveryService{ShouldFail: false}
+		mockRegService := services.NewUserRegistrationService(mockPasswordDeliveryService, mockDBService)
+		mockLoginService := services.NewUserLoginService(mockDBService)
+		handler := &UserHandler{userRegistrationService: *mockRegService, userLoginService: *mockLoginService}
+
+		// Create request with missing email field entirely
+		requestBody := []byte(`{"password": "password123"}`)
+		req, _ := http.NewRequest(http.MethodPost, "/login", bytes.NewBuffer(requestBody))
+		req.Header.Set("Content-Type", "application/json")
+
+		// Create a ResponseRecorder to capture the response
+		w := httptest.NewRecorder()
+		c, _ := gin.CreateTestContext(w)
+		c.Request = req
+
+		handler.LoginUser(c)
+
+		// Assert the results - missing required field triggers general validation error
+		assert.Equal(t, http.StatusBadRequest, w.Code)
+		var response ErrorResponse
+		err := json.Unmarshal(w.Body.Bytes(), &response)
+		assert.NoError(t, err)
+		assert.False(t, response.Success)
+		assert.Equal(t, "Invalid input data", response.Message)
+		assert.Equal(t, "validation_failed", response.Error)
+	})
+
+	t.Run("Empty password validation", func(t *testing.T) {
+		mockDBService := new(mocks.MockDatabaseOperationService)
+		mockPasswordDeliveryService := &mocks.MockPasswordDeliveryService{ShouldFail: false}
+		mockRegService := services.NewUserRegistrationService(mockPasswordDeliveryService, mockDBService)
+		mockLoginService := services.NewUserLoginService(mockDBService)
+		handler := &UserHandler{userRegistrationService: *mockRegService, userLoginService: *mockLoginService}
+
+		// Sample request data with empty password
+		loginRequest := models.LoginRequest{
+			Email:    mocks.TestUserEmail,
+			Password: "   ",
+		}
+
+		// Create a request
+		requestBody, _ := json.Marshal(loginRequest)
+		req, _ := http.NewRequest(http.MethodPost, "/login", bytes.NewBuffer(requestBody))
+		req.Header.Set("Content-Type", "application/json")
+
+		// Create a ResponseRecorder to capture the response
+		w := httptest.NewRecorder()
+		c, _ := gin.CreateTestContext(w)
+		c.Request = req
+
+		handler.LoginUser(c)
+
+		// Assert the results
+		assert.Equal(t, http.StatusBadRequest, w.Code)
+		var response ErrorResponse
 		err := json.Unmarshal(w.Body.Bytes(), &response)
 		assert.NoError(t, err)
-		assert.Contains(t, response["error"], "Invalid credentials")
+		assert.False(t, response.Success)
+		assert.Equal(t, "Password is required", response.Message)
+		assert.Equal(t, "missing_password", response.Error)
 	})
 }