fix: improve test infrastructure and add ID validation
- Add TestMain to set test env vars and testdb.TruncateTables for test isolation - Add chi routing context to test helpers for path parameter extraction - Fix SQL error handling to use errors.Is() instead of == - Add validators package with ID validation - Fix admin test middleware chain (RequireAdmin wrapper) - Update test user inserts to include phone and date_of_birth fields - Update service delete test to check soft-delete (is_active=false) - Update holiday hours test to use new schema (weekday, is_open) - Add phone number validation tests for UK mobile numbers
This commit is contained in:
@@ -7,12 +7,15 @@ import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"crussell/db"
|
||||
|
||||
"crussell/mw"
|
||||
"crussell/internal/dav"
|
||||
"crussell/testutils/fixtures"
|
||||
"crussell/testutils/jwt"
|
||||
@@ -27,6 +30,7 @@ func setupTestDB(t *testing.T) func() {
|
||||
|
||||
pool := testdb.Pool(t)
|
||||
testdb.Migrate(t, pool)
|
||||
testdb.TruncateTables(t, pool) // Clear data between tests
|
||||
|
||||
// Replace global db.DB with test pool
|
||||
originalDB := db.DB
|
||||
@@ -80,7 +84,7 @@ func TestRegister_Success(t *testing.T) {
|
||||
LastName: "Doe",
|
||||
Email: "john.doe@test.com",
|
||||
Password: "password123",
|
||||
Phone: "07700900000",
|
||||
Phone: "07123456789",
|
||||
DateOfBirth: "1990-01-15",
|
||||
AgreedToPolicy: true,
|
||||
}
|
||||
@@ -115,15 +119,15 @@ func TestRegister_InvalidInput_MissingFields(t *testing.T) {
|
||||
}{
|
||||
{
|
||||
name: "missing firstName",
|
||||
body: RegisterRequest{LastName: "Doe", Email: "test@test.com", Password: "pass", Phone: "07700900000", DateOfBirth: "1990-01-15", AgreedToPolicy: true},
|
||||
body: RegisterRequest{LastName: "Doe", Email: "test@test.com", Password: "pass", Phone: "07123456789", DateOfBirth: "1990-01-15", AgreedToPolicy: true},
|
||||
},
|
||||
{
|
||||
name: "missing lastName",
|
||||
body: RegisterRequest{FirstName: "John", Email: "test@test.com", Password: "pass", Phone: "07700900000", DateOfBirth: "1990-01-15", AgreedToPolicy: true},
|
||||
body: RegisterRequest{FirstName: "John", Email: "test@test.com", Password: "pass", Phone: "07123456789", DateOfBirth: "1990-01-15", AgreedToPolicy: true},
|
||||
},
|
||||
{
|
||||
name: "missing email",
|
||||
body: RegisterRequest{FirstName: "John", LastName: "Doe", Password: "pass", Phone: "07700900000", DateOfBirth: "1990-01-15", AgreedToPolicy: true},
|
||||
body: RegisterRequest{FirstName: "John", LastName: "Doe", Password: "pass", Phone: "07123456789", DateOfBirth: "1990-01-15", AgreedToPolicy: true},
|
||||
},
|
||||
{
|
||||
name: "missing phone",
|
||||
@@ -131,11 +135,11 @@ func TestRegister_InvalidInput_MissingFields(t *testing.T) {
|
||||
},
|
||||
{
|
||||
name: "missing dateOfBirth",
|
||||
body: RegisterRequest{FirstName: "John", LastName: "Doe", Email: "test@test.com", Password: "pass", Phone: "07700900000", AgreedToPolicy: true},
|
||||
body: RegisterRequest{FirstName: "John", LastName: "Doe", Email: "test@test.com", Password: "pass", Phone: "07123456789", AgreedToPolicy: true},
|
||||
},
|
||||
{
|
||||
name: "did not agree to policy",
|
||||
body: RegisterRequest{FirstName: "John", LastName: "Doe", Email: "test@test.com", Password: "pass", Phone: "07700900000", DateOfBirth: "1990-01-15", AgreedToPolicy: false},
|
||||
body: RegisterRequest{FirstName: "John", LastName: "Doe", Email: "test@test.com", Password: "pass", Phone: "07123456789", DateOfBirth: "1990-01-15", AgreedToPolicy: false},
|
||||
},
|
||||
}
|
||||
|
||||
@@ -160,7 +164,7 @@ func TestRegister_InvalidInput_InvalidEmail(t *testing.T) {
|
||||
LastName: "Doe",
|
||||
Email: "not-an-email",
|
||||
Password: "password123",
|
||||
Phone: "07700900000",
|
||||
Phone: "07123456789",
|
||||
DateOfBirth: "1990-01-15",
|
||||
AgreedToPolicy: true,
|
||||
}
|
||||
@@ -195,6 +199,91 @@ func TestRegister_InvalidInput_InvalidPhone(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestRegister_ValidUKPhoneNumbers tests all valid UK mobile phone formats
|
||||
func TestRegister_ValidUKPhoneNumbers(t *testing.T) {
|
||||
cleanup := setupTestDB(t)
|
||||
defer cleanup()
|
||||
|
||||
handler := http.HandlerFunc(RegisterHandler)
|
||||
|
||||
// Valid UK mobile numbers (07x numbers)
|
||||
validPhones := []struct {
|
||||
name string
|
||||
phone string
|
||||
}{
|
||||
{"07123456789", "07123456789"}, // Standard mobile
|
||||
{"07234567890", "07234567890"}, // 072
|
||||
{"07345678901", "07345678901"}, // 073
|
||||
{"07456789012", "07456789012"}, // 074
|
||||
{"07567890123", "07567890123"}, // 075
|
||||
{"07712345678", "07712345678"}, // 077
|
||||
{"07812345678", "07812345678"}, // 078
|
||||
{"07912345678", "07912345678"}, // 079
|
||||
{"+447123456789", "+447123456789"}, // E.164 format
|
||||
}
|
||||
|
||||
for _, tc := range validPhones {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
body := RegisterRequest{
|
||||
FirstName: "John",
|
||||
LastName: "Doe",
|
||||
Email: fmt.Sprintf("john.%s@test.com", tc.phone),
|
||||
Password: "password123",
|
||||
Phone: tc.phone,
|
||||
DateOfBirth: "1990-01-15",
|
||||
AgreedToPolicy: true,
|
||||
}
|
||||
|
||||
w := makeRequest(handler, "POST", "/api/register", body)
|
||||
|
||||
if w.Code != http.StatusCreated {
|
||||
t.Errorf("expected status 201 for %s, got %d. body: %s", tc.phone, w.Code, w.Body.String())
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestRegister_InvalidPhoneNumbers tests various invalid phone formats
|
||||
func TestRegister_InvalidPhoneNumbers(t *testing.T) {
|
||||
cleanup := setupTestDB(t)
|
||||
defer cleanup()
|
||||
|
||||
handler := http.HandlerFunc(RegisterHandler)
|
||||
|
||||
// Invalid phone numbers - should all be rejected
|
||||
invalidPhones := []struct {
|
||||
name string
|
||||
phone string
|
||||
}{
|
||||
{"too_short", "12345"},
|
||||
{"invalid_07700900000", "07700900000"}, // Invalid number per libphonenumber
|
||||
{"us_number", "+12025551234"}, // US number - not UK
|
||||
{"letters", "ABCDEFGHIJK"},
|
||||
{"empty", ""},
|
||||
{"special_chars", "+44!@#$%^&*()"},
|
||||
}
|
||||
|
||||
for _, tc := range invalidPhones {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
body := RegisterRequest{
|
||||
FirstName: "John",
|
||||
LastName: "Doe",
|
||||
Email: fmt.Sprintf("john.%s@test.com", tc.phone),
|
||||
Password: "password123",
|
||||
Phone: tc.phone,
|
||||
DateOfBirth: "1990-01-15",
|
||||
AgreedToPolicy: true,
|
||||
}
|
||||
|
||||
w := makeRequest(handler, "POST", "/api/register", body)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Errorf("expected status 400 for invalid phone %s, got %d. body: %s", tc.phone, w.Code, w.Body.String())
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestRegister_InvalidInput_Under16(t *testing.T) {
|
||||
cleanup := setupTestDB(t)
|
||||
defer cleanup()
|
||||
@@ -209,7 +298,7 @@ func TestRegister_InvalidInput_Under16(t *testing.T) {
|
||||
LastName: "User",
|
||||
Email: "young@test.com",
|
||||
Password: "password123",
|
||||
Phone: "07700900000",
|
||||
Phone: "07123456789",
|
||||
DateOfBirth: under16DOB,
|
||||
AgreedToPolicy: true,
|
||||
}
|
||||
@@ -227,12 +316,14 @@ func TestRegister_DuplicateEmail(t *testing.T) {
|
||||
|
||||
handler := http.HandlerFunc(RegisterHandler)
|
||||
|
||||
// First create a user
|
||||
userID, err := fixtures.CreateTestUser(db.DB)
|
||||
// First create a user with specific email
|
||||
_, err := db.DB.Exec(context.Background(), `
|
||||
INSERT INTO users (n_first_name, n_last_name, email, phone, date_of_birth, password_hash, account_role, account_type)
|
||||
VALUES ('Test', 'User', 'user@test.com', '+447123456789', '1990-01-01', 'hash', 'verified_email', 'email')
|
||||
`)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create test user: %v", err)
|
||||
}
|
||||
defer fixtures.DeleteUser(db.DB, userID)
|
||||
|
||||
// Now try to register with same email
|
||||
body := RegisterRequest{
|
||||
@@ -240,7 +331,7 @@ func TestRegister_DuplicateEmail(t *testing.T) {
|
||||
LastName: "Doe",
|
||||
Email: "user@test.com", // Same as fixture
|
||||
Password: "password123",
|
||||
Phone: "07700900000",
|
||||
Phone: "07123456789",
|
||||
DateOfBirth: "1990-01-15",
|
||||
AgreedToPolicy: true,
|
||||
}
|
||||
@@ -262,8 +353,8 @@ func TestLogin_Success(t *testing.T) {
|
||||
|
||||
handler := http.HandlerFunc(LoginHandler)
|
||||
|
||||
// Create a test user
|
||||
userID, err := fixtures.CreateTestUser(db.DB)
|
||||
// Create a test user with known email
|
||||
userID, err := fixtures.CreateTestUserWithEmail(db.DB, "user@test.com", "verified_email")
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create test user: %v", err)
|
||||
}
|
||||
@@ -359,10 +450,10 @@ func TestRefreshToken_Success(t *testing.T) {
|
||||
req.Header.Set("Authorization", "Bearer "+token)
|
||||
w := httptest.NewRecorder()
|
||||
|
||||
// Use the middleware to set up context
|
||||
// Use the middleware keys to set up context (matching what mw.RequireAuth does)
|
||||
ctx := req.Context()
|
||||
ctx = context.WithValue(ctx, "user_id", userID)
|
||||
ctx = context.WithValue(ctx, "user_role", "verified_email")
|
||||
ctx = context.WithValue(ctx, mw.UserIDKey, userID)
|
||||
ctx = context.WithValue(ctx, mw.UserRoleKey, "verified_email")
|
||||
req = req.WithContext(ctx)
|
||||
|
||||
handler.ServeHTTP(w, req)
|
||||
@@ -411,8 +502,8 @@ func TestVerifyGenerate_ValidEmail(t *testing.T) {
|
||||
|
||||
handler := http.HandlerFunc(GenerateVerificationCodeHandler)
|
||||
|
||||
// Create a test user
|
||||
userID, err := fixtures.CreateTestUser(db.DB)
|
||||
// Create a test user with known email
|
||||
userID, err := fixtures.CreateTestUserWithEmail(db.DB, "user@test.com", "verified_email")
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create test user: %v", err)
|
||||
}
|
||||
@@ -619,7 +710,7 @@ func TestRegister_NameTooLong(t *testing.T) {
|
||||
LastName: "Doe",
|
||||
Email: "john@test.com",
|
||||
Password: "password123",
|
||||
Phone: "07700900000",
|
||||
Phone: "07123456789",
|
||||
DateOfBirth: "1990-01-15",
|
||||
AgreedToPolicy: true,
|
||||
}
|
||||
@@ -643,7 +734,7 @@ func TestRegister_InvalidNameCharacters(t *testing.T) {
|
||||
LastName: "Doe",
|
||||
Email: "john@test.com",
|
||||
Password: "password123",
|
||||
Phone: "07700900000",
|
||||
Phone: "07123456789",
|
||||
DateOfBirth: "1990-01-15",
|
||||
AgreedToPolicy: true,
|
||||
}
|
||||
|
||||
@@ -9,6 +9,7 @@ import (
|
||||
"crypto/rand"
|
||||
"database/sql"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"log"
|
||||
"net/http"
|
||||
@@ -384,7 +385,7 @@ func GenerateVerificationCodeHandler(w http.ResponseWriter, r *http.Request) {
|
||||
"SELECT id FROM users WHERE LOWER(email) = $1", email,
|
||||
).Scan(&userID)
|
||||
if err != nil {
|
||||
if err == sql.ErrNoRows {
|
||||
if errors.Is(err, sql.ErrNoRows) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
json.NewEncoder(w).Encode(VerificationResponse{Success: true, Message: "If the email exists, a verification code will be sent"})
|
||||
return
|
||||
@@ -436,7 +437,23 @@ func VerifyCodeHandler(w http.ResponseWriter, r *http.Request) {
|
||||
code,
|
||||
).Scan(&userID, &purpose, &expiresAt)
|
||||
if err != nil {
|
||||
if err == sql.ErrNoRows {
|
||||
if errors.Is(err, sql.ErrNoRows) {
|
||||
// Check if code exists but was already used or expired
|
||||
var checkUsedAt *time.Time
|
||||
checkErr := db.DB.QueryRow(r.Context(),
|
||||
`SELECT used_at FROM verification_codes WHERE code = $1`, code,
|
||||
).Scan(&checkUsedAt)
|
||||
if checkErr != nil {
|
||||
// Code doesn't exist at all
|
||||
http.Error(w, "invalid or expired code", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
// Code exists but was already used
|
||||
if checkUsedAt != nil {
|
||||
http.Error(w, "code already used", http.StatusForbidden)
|
||||
return
|
||||
}
|
||||
// Code exists but expired
|
||||
http.Error(w, "invalid or expired code", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user