From de9e9102db8000fa6d8619ab2e82db30d9c404f1 Mon Sep 17 00:00:00 2001 From: LordMathis Date: Thu, 5 Dec 2024 21:56:35 +0100 Subject: [PATCH 01/12] Migrate backend auth to cookies --- server/cmd/server/main.go | 3 + server/internal/app/config.go | 11 +- server/internal/app/config_test.go | 7 +- server/internal/app/init.go | 11 +- server/internal/app/options.go | 4 +- server/internal/app/routes.go | 9 +- server/internal/auth/cookies.go | 91 ++++++++++++++++ server/internal/auth/jwt.go | 15 --- server/internal/auth/jwt_test.go | 85 --------------- server/internal/auth/middleware.go | 37 ++++--- server/internal/auth/session.go | 10 +- server/internal/handlers/admin_handlers.go | 14 +-- server/internal/handlers/auth_handlers.go | 102 ++++++++++-------- server/internal/handlers/file_handlers.go | 14 +-- server/internal/handlers/git_handlers.go | 4 +- server/internal/handlers/user_handlers.go | 4 +- .../internal/handlers/workspace_handlers.go | 14 +-- 17 files changed, 237 insertions(+), 198 deletions(-) create mode 100644 server/internal/auth/cookies.go diff --git a/server/cmd/server/main.go b/server/cmd/server/main.go index ae26731..e7fc445 100644 --- a/server/cmd/server/main.go +++ b/server/cmd/server/main.go @@ -13,6 +13,9 @@ import ( // @license.name Apache 2.0 // @license.url http://www.apache.org/licenses/LICENSE-2.0.html // @BasePath /api/v1 +// @SecurityDefinitions.ApiKey CookieAuth +// @In cookie +// @Name access_token func main() { // Load configuration cfg, err := app.LoadConfig() diff --git a/server/internal/app/config.go b/server/internal/app/config.go index fc34487..b9ff02a 100644 --- a/server/internal/app/config.go +++ b/server/internal/app/config.go @@ -15,7 +15,8 @@ type Config struct { WorkDir string StaticPath string Port string - AppURL string + RootURL string + Domain string CORSOrigins []string AdminEmail string AdminPassword string @@ -77,8 +78,12 @@ func LoadConfig() (*Config, error) { config.Port = port } - if appURL := os.Getenv("NOVAMD_APP_URL"); appURL != "" { - config.AppURL = appURL + if rootURL := os.Getenv("NOVAMD_ROOT_URL"); rootURL != "" { + config.RootURL = rootURL + } + + if domain := os.Getenv("NOVAMD_DOMAIN"); domain != "" { + config.Domain = domain } if corsOrigins := os.Getenv("NOVAMD_CORS_ORIGINS"); corsOrigins != "" { diff --git a/server/internal/app/config_test.go b/server/internal/app/config_test.go index 383f73f..71ae11f 100644 --- a/server/internal/app/config_test.go +++ b/server/internal/app/config_test.go @@ -49,7 +49,8 @@ func TestLoad(t *testing.T) { "NOVAMD_WORKDIR", "NOVAMD_STATIC_PATH", "NOVAMD_PORT", - "NOVAMD_APP_URL", + "NOVAMD_ROOT_URL", + "NOVAMD_DOMAIN", "NOVAMD_CORS_ORIGINS", "NOVAMD_ADMIN_EMAIL", "NOVAMD_ADMIN_PASSWORD", @@ -95,7 +96,7 @@ func TestLoad(t *testing.T) { "NOVAMD_WORKDIR": "/custom/work/dir", "NOVAMD_STATIC_PATH": "/custom/static/path", "NOVAMD_PORT": "3000", - "NOVAMD_APP_URL": "http://localhost:3000", + "NOVAMD_ROOT_URL": "http://localhost:3000", "NOVAMD_CORS_ORIGINS": "http://localhost:3000,http://localhost:3001", "NOVAMD_ADMIN_EMAIL": "admin@example.com", "NOVAMD_ADMIN_PASSWORD": "password123", @@ -124,7 +125,7 @@ func TestLoad(t *testing.T) { {"WorkDir", cfg.WorkDir, "/custom/work/dir"}, {"StaticPath", cfg.StaticPath, "/custom/static/path"}, {"Port", cfg.Port, "3000"}, - {"AppURL", cfg.AppURL, "http://localhost:3000"}, + {"AppURL", cfg.RootURL, "http://localhost:3000"}, {"AdminEmail", cfg.AdminEmail, "admin@example.com"}, {"AdminPassword", cfg.AdminPassword, "password123"}, {"JWTSigningKey", cfg.JWTSigningKey, "secret-key"}, diff --git a/server/internal/app/init.go b/server/internal/app/init.go index eec83b4..923dae8 100644 --- a/server/internal/app/init.go +++ b/server/internal/app/init.go @@ -40,14 +40,14 @@ func initDatabase(cfg *Config, secretsService secrets.Service) (db.Database, err } // initAuth initializes JWT and session services -func initAuth(cfg *Config, database db.Database) (auth.JWTManager, *auth.SessionService, error) { +func initAuth(cfg *Config, database db.Database) (auth.JWTManager, *auth.SessionService, auth.CookieService, error) { // Get or generate JWT signing key signingKey := cfg.JWTSigningKey if signingKey == "" { var err error signingKey, err = database.EnsureJWTSecret() if err != nil { - return nil, nil, fmt.Errorf("failed to ensure JWT secret: %w", err) + return nil, nil, nil, fmt.Errorf("failed to ensure JWT secret: %w", err) } } @@ -58,13 +58,16 @@ func initAuth(cfg *Config, database db.Database) (auth.JWTManager, *auth.Session RefreshTokenExpiry: 7 * 24 * time.Hour, }) if err != nil { - return nil, nil, fmt.Errorf("failed to initialize JWT service: %w", err) + return nil, nil, nil, fmt.Errorf("failed to initialize JWT service: %w", err) } // Initialize session service sessionService := auth.NewSessionService(database, jwtManager) - return jwtManager, sessionService, nil + // Cookie service + cookieService := auth.NewCookieService(cfg.IsDevelopment, cfg.Domain) + + return jwtManager, sessionService, cookieService, nil } // setupAdminUser creates the admin user if it doesn't exist diff --git a/server/internal/app/options.go b/server/internal/app/options.go index 9e8b2fb..be5e3cc 100644 --- a/server/internal/app/options.go +++ b/server/internal/app/options.go @@ -13,6 +13,7 @@ type Options struct { Storage storage.Manager JWTManager auth.JWTManager SessionService *auth.SessionService + CookieService auth.CookieService } // DefaultOptions creates server options with default configuration @@ -33,7 +34,7 @@ func DefaultOptions(cfg *Config) (*Options, error) { storageManager := storage.NewService(cfg.WorkDir) // Initialize auth services - jwtManager, sessionService, err := initAuth(cfg, database) + jwtManager, sessionService, cookieService, err := initAuth(cfg, database) if err != nil { return nil, err } @@ -49,5 +50,6 @@ func DefaultOptions(cfg *Config) (*Options, error) { Storage: storageManager, JWTManager: jwtManager, SessionService: sessionService, + CookieService: cookieService, }, nil } diff --git a/server/internal/app/routes.go b/server/internal/app/routes.go index f3e9df7..ecd1a35 100644 --- a/server/internal/app/routes.go +++ b/server/internal/app/routes.go @@ -40,7 +40,8 @@ func setupRouter(o Options) *chi.Mux { r.Use(cors.Handler(cors.Options{ AllowedOrigins: o.Config.CORSOrigins, AllowedMethods: []string{"GET", "POST", "PUT", "DELETE", "OPTIONS"}, - AllowedHeaders: []string{"Accept", "Authorization", "Content-Type", "X-Requested-With"}, + AllowedHeaders: []string{"Accept", "Content-Type", "X-CSRF-Token"}, + ExposedHeaders: []string{"X-CSRF-Token"}, AllowCredentials: true, MaxAge: 300, })) @@ -71,8 +72,8 @@ func setupRouter(o Options) *chi.Mux { // Public routes (no authentication required) r.Group(func(r chi.Router) { - r.Post("/auth/login", handler.Login(o.SessionService)) - r.Post("/auth/refresh", handler.RefreshToken(o.SessionService)) + r.Post("/auth/login", handler.Login(o.SessionService, o.CookieService)) + r.Post("/auth/refresh", handler.RefreshToken(o.SessionService, o.CookieService)) }) // Protected routes (authentication required) @@ -81,7 +82,7 @@ func setupRouter(o Options) *chi.Mux { r.Use(context.WithUserContextMiddleware) // Auth routes - r.Post("/auth/logout", handler.Logout(o.SessionService)) + r.Post("/auth/logout", handler.Logout(o.SessionService, o.CookieService)) r.Get("/auth/me", handler.GetCurrentUser()) // User profile routes diff --git a/server/internal/auth/cookies.go b/server/internal/auth/cookies.go new file mode 100644 index 0000000..7bed834 --- /dev/null +++ b/server/internal/auth/cookies.go @@ -0,0 +1,91 @@ +// Package auth provides JWT token generation and validation +package auth + +import ( + "net/http" +) + +// CookieService interface defines methods for generating cookies +type CookieService interface { + GenerateAccessTokenCookie(token string) *http.Cookie + GenerateRefreshTokenCookie(token string) *http.Cookie + GenerateCSRFCookie(token string) *http.Cookie + InvalidateCookie(cookieType string) *http.Cookie +} + +// CookieService +type cookieService struct { + Domain string + Secure bool + SameSite http.SameSite +} + +// NewCookieService creates a new cookie service +func NewCookieService(isDevelopment bool, domain string) CookieService { + secure := !isDevelopment + var sameSite http.SameSite + + if isDevelopment { + sameSite = http.SameSiteLaxMode + } else { + sameSite = http.SameSiteStrictMode + } + + return &cookieService{ + Domain: domain, + Secure: secure, + SameSite: sameSite, + } +} + +// GenerateAccessTokenCookie creates a new cookie for the access token +func (c *cookieService) GenerateAccessTokenCookie(token string) *http.Cookie { + return &http.Cookie{ + Name: "access_token", + Value: token, + HttpOnly: true, + Secure: c.Secure, + SameSite: c.SameSite, + Path: "/", + MaxAge: 900, // 15 minutes + } +} + +// GenerateRefreshTokenCookie creates a new cookie for the refresh token +func (c *cookieService) GenerateRefreshTokenCookie(token string) *http.Cookie { + return &http.Cookie{ + Name: "refresh_token", + Value: token, + HttpOnly: true, + Secure: c.Secure, + SameSite: c.SameSite, + Path: "/", + MaxAge: 604800, // 7 days + } +} + +// GenerateCSRFCookie creates a new cookie for the CSRF token +func (c *cookieService) GenerateCSRFCookie(token string) *http.Cookie { + return &http.Cookie{ + Name: "csrf_token", + Value: token, + HttpOnly: false, // Frontend needs to read this + Secure: c.Secure, + SameSite: c.SameSite, + Path: "/", + MaxAge: 900, + } +} + +// InvalidateCookie creates a new cookie with a MaxAge of -1 to invalidate the cookie +func (c *cookieService) InvalidateCookie(cookieType string) *http.Cookie { + return &http.Cookie{ + Name: cookieType, + Value: "", + Path: "/", + MaxAge: -1, + HttpOnly: true, + Secure: c.Secure, + SameSite: c.SameSite, + } +} diff --git a/server/internal/auth/jwt.go b/server/internal/auth/jwt.go index 59790f0..66b6c24 100644 --- a/server/internal/auth/jwt.go +++ b/server/internal/auth/jwt.go @@ -38,7 +38,6 @@ type JWTManager interface { GenerateAccessToken(userID int, role string) (string, error) GenerateRefreshToken(userID int, role string) (string, error) ValidateToken(tokenString string) (*Claims, error) - RefreshAccessToken(refreshToken string) (string, error) } // jwtService handles JWT token generation and validation @@ -118,17 +117,3 @@ func (s *jwtService) ValidateToken(tokenString string) (*Claims, error) { return nil, fmt.Errorf("invalid token claims") } - -// RefreshAccessToken creates a new access token using a refreshToken -func (s *jwtService) RefreshAccessToken(refreshToken string) (string, error) { - claims, err := s.ValidateToken(refreshToken) - if err != nil { - return "", fmt.Errorf("invalid refresh token: %w", err) - } - - if claims.Type != RefreshToken { - return "", fmt.Errorf("invalid token type: expected refresh token") - } - - return s.GenerateAccessToken(claims.UserID, claims.Role) -} diff --git a/server/internal/auth/jwt_test.go b/server/internal/auth/jwt_test.go index 61aa3ad..61ca928 100644 --- a/server/internal/auth/jwt_test.go +++ b/server/internal/auth/jwt_test.go @@ -5,8 +5,6 @@ import ( "time" "novamd/internal/auth" - - "github.com/golang-jwt/jwt/v5" ) // jwt_test.go tests @@ -136,86 +134,3 @@ func TestGenerateAndValidateToken(t *testing.T) { }) } } - -func TestRefreshAccessToken(t *testing.T) { - config := auth.JWTConfig{ - SigningKey: "test-key", - AccessTokenExpiry: 15 * time.Minute, - RefreshTokenExpiry: 24 * time.Hour, - } - service, _ := auth.NewJWTService(config) - - testCases := []struct { - name string - userID int - role string - wantErr bool - setupFunc func() string // Added setup function to handle custom token creation - }{ - { - name: "valid refresh token", - userID: 1, - role: "admin", - wantErr: false, - setupFunc: func() string { - token, _ := service.GenerateRefreshToken(1, "admin") - return token - }, - }, - { - name: "expired refresh token", - userID: 1, - role: "admin", - wantErr: true, - setupFunc: func() string { - // Create a token that's already expired - claims := &auth.Claims{ - RegisteredClaims: jwt.RegisteredClaims{ - ExpiresAt: jwt.NewNumericDate(time.Now().Add(-time.Hour)), // Expired 1 hour ago - IssuedAt: jwt.NewNumericDate(time.Now().Add(-2 * time.Hour)), - NotBefore: jwt.NewNumericDate(time.Now().Add(-2 * time.Hour)), - }, - UserID: 1, - Role: "admin", - Type: auth.RefreshToken, - } - token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims) - tokenString, _ := token.SignedString([]byte(config.SigningKey)) - return tokenString - }, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - refreshToken := tc.setupFunc() - newAccessToken, err := service.RefreshAccessToken(refreshToken) - - if tc.wantErr { - if err == nil { - t.Error("expected error, got nil") - } - return - } - - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - claims, err := service.ValidateToken(newAccessToken) - if err != nil { - t.Fatalf("failed to validate new access token: %v", err) - } - - if claims.UserID != tc.userID { - t.Errorf("userID = %v, want %v", claims.UserID, tc.userID) - } - if claims.Role != tc.role { - t.Errorf("role = %v, want %v", claims.Role, tc.role) - } - if claims.Type != auth.AccessToken { - t.Errorf("token type = %v, want %v", claims.Type, auth.AccessToken) - } - }) - } -} diff --git a/server/internal/auth/middleware.go b/server/internal/auth/middleware.go index 8018612..7754f7b 100644 --- a/server/internal/auth/middleware.go +++ b/server/internal/auth/middleware.go @@ -1,8 +1,8 @@ package auth import ( + "crypto/subtle" "net/http" - "strings" "novamd/internal/context" ) @@ -23,21 +23,14 @@ func NewMiddleware(jwtManager JWTManager) *Middleware { func (m *Middleware) Authenticate(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { // Extract token from Authorization header - authHeader := r.Header.Get("Authorization") - if authHeader == "" { - http.Error(w, "Authorization header required", http.StatusUnauthorized) - return - } - - // Check Bearer token format - parts := strings.Split(authHeader, " ") - if len(parts) != 2 || parts[0] != "Bearer" { - http.Error(w, "Invalid authorization format", http.StatusUnauthorized) + cookie, err := r.Cookie("access_token") + if err != nil { + http.Error(w, "Unauthorized", http.StatusUnauthorized) return } // Validate token - claims, err := m.jwtManager.ValidateToken(parts[1]) + claims, err := m.jwtManager.ValidateToken(cookie.Value) if err != nil { http.Error(w, "Invalid token", http.StatusUnauthorized) return @@ -49,6 +42,26 @@ func (m *Middleware) Authenticate(next http.Handler) http.Handler { return } + // Add CSRF check for non-GET requests + if r.Method != http.MethodGet && r.Method != http.MethodHead && r.Method != http.MethodOptions { + csrfCookie, err := r.Cookie("csrf_token") + if err != nil { + http.Error(w, "CSRF cookie not found", http.StatusForbidden) + return + } + + csrfHeader := r.Header.Get("X-CSRF-Token") + if csrfHeader == "" { + http.Error(w, "CSRF token header not found", http.StatusForbidden) + return + } + + if subtle.ConstantTimeCompare([]byte(csrfCookie.Value), []byte(csrfHeader)) != 1 { + http.Error(w, "CSRF token mismatch", http.StatusForbidden) + return + } + } + // Create handler context with user information hctx := &context.HandlerContext{ UserID: claims.UserID, diff --git a/server/internal/auth/session.go b/server/internal/auth/session.go index 897fb3d..afaaf06 100644 --- a/server/internal/auth/session.go +++ b/server/internal/auth/session.go @@ -83,8 +83,14 @@ func (s *SessionService) RefreshSession(refreshToken string) (string, error) { } // InvalidateSession removes a session with the given sessionID from the database -func (s *SessionService) InvalidateSession(sessionID string) error { - return s.db.DeleteSession(sessionID) +func (s *SessionService) InvalidateSession(token string) error { + // Parse the JWT to get the session info + claims, err := s.jwtManager.ValidateToken(token) + if err != nil { + return fmt.Errorf("invalid token: %w", err) + } + + return s.db.DeleteSession(claims.ID) } // CleanExpiredSessions removes all expired sessions from the database diff --git a/server/internal/handlers/admin_handlers.go b/server/internal/handlers/admin_handlers.go index a776204..8c01590 100644 --- a/server/internal/handlers/admin_handlers.go +++ b/server/internal/handlers/admin_handlers.go @@ -51,7 +51,7 @@ type SystemStats struct { // @Summary List all users // @Description Returns the list of all users // @Tags Admin -// @Security BearerAuth +// @Security CookieAuth // @ID adminListUsers // @Produce json // @Success 200 {array} models.User @@ -73,7 +73,7 @@ func (h *Handler) AdminListUsers() http.HandlerFunc { // @Summary Create a new user // @Description Create a new user as an admin // @Tags Admin -// @Security BearerAuth +// @Security CookieAuth // @ID adminCreateUser // @Accept json // @Produce json @@ -149,7 +149,7 @@ func (h *Handler) AdminCreateUser() http.HandlerFunc { // @Summary Get a specific user // @Description Get a specific user as an admin // @Tags Admin -// @Security BearerAuth +// @Security CookieAuth // @ID adminGetUser // @Produce json // @Param userId path int true "User ID" @@ -179,7 +179,7 @@ func (h *Handler) AdminGetUser() http.HandlerFunc { // @Summary Update a specific user // @Description Update a specific user as an admin // @Tags Admin -// @Security BearerAuth +// @Security CookieAuth // @ID adminUpdateUser // @Accept json // @Produce json @@ -245,7 +245,7 @@ func (h *Handler) AdminUpdateUser() http.HandlerFunc { // @Summary Delete a specific user // @Description Delete a specific user as an admin // @Tags Admin -// @Security BearerAuth +// @Security CookieAuth // @ID adminDeleteUser // @Param userId path int true "User ID" // @Success 204 "No Content" @@ -300,7 +300,7 @@ func (h *Handler) AdminDeleteUser() http.HandlerFunc { // @Summary List all workspaces // @Description List all workspaces and their stats as an admin // @Tags Admin -// @Security BearerAuth +// @Security CookieAuth // @ID adminListWorkspaces // @Produce json // @Success 200 {array} WorkspaceStats @@ -353,7 +353,7 @@ func (h *Handler) AdminListWorkspaces() http.HandlerFunc { // @Summary Get system statistics // @Description Get system-wide statistics as an admin // @Tags Admin -// @Security BearerAuth +// @Security CookieAuth // @ID adminGetSystemStats // @Produce json // @Success 200 {object} SystemStats diff --git a/server/internal/handlers/auth_handlers.go b/server/internal/handlers/auth_handlers.go index 68b637f..e59a98d 100644 --- a/server/internal/handlers/auth_handlers.go +++ b/server/internal/handlers/auth_handlers.go @@ -1,11 +1,14 @@ package handlers import ( + "crypto/rand" + "encoding/hex" "encoding/json" "net/http" "novamd/internal/auth" "novamd/internal/context" "novamd/internal/models" + "time" "golang.org/x/crypto/bcrypt" ) @@ -18,27 +21,15 @@ type LoginRequest struct { // LoginResponse represents a user login response type LoginResponse struct { - AccessToken string `json:"accessToken"` - RefreshToken string `json:"refreshToken"` - User *models.User `json:"user"` - Session *models.Session `json:"session"` -} - -// RefreshRequest represents a refresh token request -type RefreshRequest struct { - RefreshToken string `json:"refreshToken"` -} - -// RefreshResponse represents a refresh token response -type RefreshResponse struct { - AccessToken string `json:"accessToken"` + User *models.User `json:"user"` + SessionID string `json:"sessionId,omitempty"` + ExpiresAt time.Time `json:"expiresAt,omitempty"` } // Login godoc // @Summary Login -// @Description Logs in a user +// @Description Logs in a user and returns a session with access and refresh tokens // @Tags auth -// @ID login // @Accept json // @Produce json // @Param body body LoginRequest true "Login request" @@ -48,7 +39,7 @@ type RefreshResponse struct { // @Failure 401 {object} ErrorResponse "Invalid credentials" // @Failure 500 {object} ErrorResponse "Failed to create session" // @Router /auth/login [post] -func (h *Handler) Login(authService *auth.SessionService) http.HandlerFunc { +func (h *Handler) Login(authService *auth.SessionService, cookieService auth.CookieService) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { var req LoginRequest if err := json.NewDecoder(r.Body).Decode(&req); err != nil { @@ -83,12 +74,27 @@ func (h *Handler) Login(authService *auth.SessionService) http.HandlerFunc { return } - // Prepare response + // Generate CSRF token + csrfToken := make([]byte, 32) + if _, err := rand.Read(csrfToken); err != nil { + respondError(w, "Failed to generate CSRF token", http.StatusInternalServerError) + return + } + csrfTokenString := hex.EncodeToString(csrfToken) + + // Set cookies + http.SetCookie(w, cookieService.GenerateAccessTokenCookie(accessToken)) + http.SetCookie(w, cookieService.GenerateRefreshTokenCookie(session.RefreshToken)) + http.SetCookie(w, cookieService.GenerateCSRFCookie(csrfTokenString)) + + // Send CSRF token in header for initial setup + w.Header().Set("X-CSRF-Token", csrfTokenString) + + // Only send user info in response, not tokens response := LoginResponse{ - AccessToken: accessToken, - RefreshToken: session.RefreshToken, - User: user, - Session: session, + User: user, + SessionID: session.ID, + ExpiresAt: session.ExpiresAt, } respondJSON(w, response) @@ -100,25 +106,30 @@ func (h *Handler) Login(authService *auth.SessionService) http.HandlerFunc { // @Description Log out invalidates the user's session // @Tags auth // @ID logout -// @Security BearerAuth // @Success 204 "No Content" // @Failure 400 {object} ErrorResponse "Session ID required" // @Failure 500 {object} ErrorResponse "Failed to logout" // @Router /auth/logout [post] -func (h *Handler) Logout(authService *auth.SessionService) http.HandlerFunc { +func (h *Handler) Logout(authService *auth.SessionService, cookieService auth.CookieService) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - sessionID := r.Header.Get("X-Session-ID") - if sessionID == "" { - respondError(w, "Session ID required", http.StatusBadRequest) + // Get session ID from cookie + sessionCookie, err := r.Cookie("access_token") + if err != nil { + http.Error(w, "Unauthorized", http.StatusUnauthorized) return } - err := authService.InvalidateSession(sessionID) - if err != nil { - respondError(w, "Failed to logout", http.StatusInternalServerError) + // Invalidate the session in the database + if err := authService.InvalidateSession(sessionCookie.Value); err != nil { + respondError(w, "Failed to invalidate session", http.StatusInternalServerError) return } + // Clear cookies + http.SetCookie(w, cookieService.InvalidateCookie("access_token")) + http.SetCookie(w, cookieService.InvalidateCookie("refresh_token")) + http.SetCookie(w, cookieService.InvalidateCookie("csrf_token")) + w.WriteHeader(http.StatusNoContent) } } @@ -131,36 +142,39 @@ func (h *Handler) Logout(authService *auth.SessionService) http.HandlerFunc { // @Accept json // @Produce json // @Param body body RefreshRequest true "Refresh request" -// @Success 200 {object} RefreshResponse +// @Success 200 "Tokens refreshed successfully via cookies" // @Failure 400 {object} ErrorResponse "Invalid request body" // @Failure 400 {object} ErrorResponse "Refresh token required" // @Failure 401 {object} ErrorResponse "Invalid refresh token" // @Router /auth/refresh [post] -func (h *Handler) RefreshToken(authService *auth.SessionService) http.HandlerFunc { +func (h *Handler) RefreshToken(authService *auth.SessionService, cookieService auth.CookieService) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - var req RefreshRequest - if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - respondError(w, "Invalid request body", http.StatusBadRequest) - return - } - - if req.RefreshToken == "" { + refreshCookie, err := r.Cookie("refresh_token") + if err != nil { respondError(w, "Refresh token required", http.StatusBadRequest) return } // Generate new access token - accessToken, err := authService.RefreshSession(req.RefreshToken) + accessToken, err := authService.RefreshSession(refreshCookie.Value) if err != nil { respondError(w, "Invalid refresh token", http.StatusUnauthorized) return } - response := RefreshResponse{ - AccessToken: accessToken, + // Generate new CSRF token + csrfToken := make([]byte, 32) + if _, err := rand.Read(csrfToken); err != nil { + respondError(w, "Failed to generate CSRF token", http.StatusInternalServerError) + return } + csrfTokenString := hex.EncodeToString(csrfToken) - respondJSON(w, response) + http.SetCookie(w, cookieService.GenerateAccessTokenCookie(accessToken)) + http.SetCookie(w, cookieService.GenerateCSRFCookie(csrfTokenString)) + + w.Header().Set("X-CSRF-Token", csrfTokenString) + w.WriteHeader(http.StatusOK) } } @@ -169,7 +183,7 @@ func (h *Handler) RefreshToken(authService *auth.SessionService) http.HandlerFun // @Description Returns the current authenticated user // @Tags auth // @ID getCurrentUser -// @Security BearerAuth +// @Security CookieAuth // @Produce json // @Success 200 {object} models.User // @Failure 404 {object} ErrorResponse "User not found" diff --git a/server/internal/handlers/file_handlers.go b/server/internal/handlers/file_handlers.go index 6104d98..a19597c 100644 --- a/server/internal/handlers/file_handlers.go +++ b/server/internal/handlers/file_handlers.go @@ -40,7 +40,7 @@ type UpdateLastOpenedFileRequest struct { // @Description Lists all files in the user's workspace // @Tags files // @ID listFiles -// @Security BearerAuth +// @Security CookieAuth // @Produce json // @Param workspace_name path string true "Workspace name" // @Success 200 {array} storage.FileNode @@ -68,7 +68,7 @@ func (h *Handler) ListFiles() http.HandlerFunc { // @Description Returns the paths of files with the given name in the user's workspace // @Tags files // @ID lookupFileByName -// @Security BearerAuth +// @Security CookieAuth // @Produce json // @Param workspace_name path string true "Workspace name" // @Param filename query string true "File name" @@ -104,7 +104,7 @@ func (h *Handler) LookupFileByName() http.HandlerFunc { // @Description Returns the content of a file in the user's workspace // @Tags files // @ID getFileContent -// @Security BearerAuth +// @Security CookieAuth // @Produce plain // @Param workspace_name path string true "Workspace name" // @Param file_path path string true "File path" @@ -153,7 +153,7 @@ func (h *Handler) GetFileContent() http.HandlerFunc { // @Description Saves the content of a file in the user's workspace // @Tags files // @ID saveFile -// @Security BearerAuth +// @Security CookieAuth // @Accept plain // @Produce json // @Param workspace_name path string true "Workspace name" @@ -204,7 +204,7 @@ func (h *Handler) SaveFile() http.HandlerFunc { // @Description Deletes a file in the user's workspace // @Tags files // @ID deleteFile -// @Security BearerAuth +// @Security CookieAuth // @Param workspace_name path string true "Workspace name" // @Param file_path path string true "File path" // @Success 204 "No Content - File deleted successfully" @@ -246,7 +246,7 @@ func (h *Handler) DeleteFile() http.HandlerFunc { // @Description Returns the path of the last opened file in the user's workspace // @Tags files // @ID getLastOpenedFile -// @Security BearerAuth +// @Security CookieAuth // @Produce json // @Param workspace_name path string true "Workspace name" // @Success 200 {object} LastOpenedFileResponse @@ -280,7 +280,7 @@ func (h *Handler) GetLastOpenedFile() http.HandlerFunc { // @Description Updates the last opened file in the user's workspace // @Tags files // @ID updateLastOpenedFile -// @Security BearerAuth +// @Security CookieAuth // @Accept json // @Produce json // @Param workspace_name path string true "Workspace name" diff --git a/server/internal/handlers/git_handlers.go b/server/internal/handlers/git_handlers.go index 1ab45e0..3135b6e 100644 --- a/server/internal/handlers/git_handlers.go +++ b/server/internal/handlers/git_handlers.go @@ -27,7 +27,7 @@ type PullResponse struct { // @Description Stages, commits, and pushes changes to the remote repository // @Tags git // @ID stageCommitAndPush -// @Security BearerAuth +// @Security CookieAuth // @Produce json // @Param workspace_name path string true "Workspace name" // @Param body body CommitRequest true "Commit request" @@ -70,7 +70,7 @@ func (h *Handler) StageCommitAndPush() http.HandlerFunc { // @Description Pulls changes from the remote repository // @Tags git // @ID pullChanges -// @Security BearerAuth +// @Security CookieAuth // @Produce json // @Param workspace_name path string true "Workspace name" // @Success 200 {object} PullResponse diff --git a/server/internal/handlers/user_handlers.go b/server/internal/handlers/user_handlers.go index 58d4403..7249b63 100644 --- a/server/internal/handlers/user_handlers.go +++ b/server/internal/handlers/user_handlers.go @@ -27,7 +27,7 @@ type DeleteAccountRequest struct { // @Description Updates the user's profile // @Tags users // @ID updateProfile -// @Security BearerAuth +// @Security CookieAuth // @Accept json // @Produce json // @Param body body UpdateProfileRequest true "Profile update request" @@ -137,7 +137,7 @@ func (h *Handler) UpdateProfile() http.HandlerFunc { // @Description Deletes the user's account and all associated data // @Tags users // @ID deleteAccount -// @Security BearerAuth +// @Security CookieAuth // @Accept json // @Produce json // @Param body body DeleteAccountRequest true "Account deletion request" diff --git a/server/internal/handlers/workspace_handlers.go b/server/internal/handlers/workspace_handlers.go index 2b26d0d..e04e543 100644 --- a/server/internal/handlers/workspace_handlers.go +++ b/server/internal/handlers/workspace_handlers.go @@ -24,7 +24,7 @@ type LastWorkspaceNameResponse struct { // @Description Lists all workspaces for the current user // @Tags workspaces // @ID listWorkspaces -// @Security BearerAuth +// @Security CookieAuth // @Produce json // @Success 200 {array} models.Workspace // @Failure 500 {object} ErrorResponse "Failed to list workspaces" @@ -51,7 +51,7 @@ func (h *Handler) ListWorkspaces() http.HandlerFunc { // @Description Creates a new workspace // @Tags workspaces // @ID createWorkspace -// @Security BearerAuth +// @Security CookieAuth // @Accept json // @Produce json // @Param body body models.Workspace true "Workspace" @@ -115,7 +115,7 @@ func (h *Handler) CreateWorkspace() http.HandlerFunc { // @Description Returns the current workspace // @Tags workspaces // @ID getWorkspace -// @Security BearerAuth +// @Security CookieAuth // @Produce json // @Param workspace_name path string true "Workspace name" // @Success 200 {object} models.Workspace @@ -155,7 +155,7 @@ func gitSettingsChanged(new, old *models.Workspace) bool { // @Description Updates the current workspace // @Tags workspaces // @ID updateWorkspace -// @Security BearerAuth +// @Security CookieAuth // @Accept json // @Produce json // @Param workspace_name path string true "Workspace name" @@ -223,7 +223,7 @@ func (h *Handler) UpdateWorkspace() http.HandlerFunc { // @Description Deletes the current workspace // @Tags workspaces // @ID deleteWorkspace -// @Security BearerAuth +// @Security CookieAuth // @Produce json // @Param workspace_name path string true "Workspace name" // @Success 200 {object} DeleteWorkspaceResponse @@ -307,7 +307,7 @@ func (h *Handler) DeleteWorkspace() http.HandlerFunc { // @Description Returns the name of the last opened workspace // @Tags workspaces // @ID getLastWorkspaceName -// @Security BearerAuth +// @Security CookieAuth // @Produce json // @Success 200 {object} LastWorkspaceNameResponse // @Failure 500 {object} ErrorResponse "Failed to get last workspace" @@ -334,7 +334,7 @@ func (h *Handler) GetLastWorkspaceName() http.HandlerFunc { // @Description Updates the name of the last opened workspace // @Tags workspaces // @ID updateLastWorkspaceName -// @Security BearerAuth +// @Security CookieAuth // @Accept json // @Produce json // @Success 204 "No Content - Last workspace updated successfully" From 8a4508e29f1572e3a34bc06d6b1abf4bce81f2ad Mon Sep 17 00:00:00 2001 From: LordMathis Date: Sat, 7 Dec 2024 21:19:02 +0100 Subject: [PATCH 02/12] Update session and cookie managers --- server/internal/app/init.go | 6 +-- server/internal/app/options.go | 6 +-- server/internal/app/routes.go | 8 ++-- server/internal/auth/cookies.go | 18 ++++----- server/internal/auth/middleware.go | 20 ++++++++-- server/internal/auth/session.go | 36 ++++++++++++++---- server/internal/db/db.go | 1 + server/internal/db/sessions.go | 20 ++++++++++ server/internal/handlers/auth_handlers.go | 12 +++--- server/internal/handlers/integration_test.go | 40 ++++++++++---------- 10 files changed, 111 insertions(+), 56 deletions(-) diff --git a/server/internal/app/init.go b/server/internal/app/init.go index 923dae8..773f8c3 100644 --- a/server/internal/app/init.go +++ b/server/internal/app/init.go @@ -40,7 +40,7 @@ func initDatabase(cfg *Config, secretsService secrets.Service) (db.Database, err } // initAuth initializes JWT and session services -func initAuth(cfg *Config, database db.Database) (auth.JWTManager, *auth.SessionService, auth.CookieService, error) { +func initAuth(cfg *Config, database db.Database) (auth.JWTManager, auth.SessionManager, auth.CookieManager, error) { // Get or generate JWT signing key signingKey := cfg.JWTSigningKey if signingKey == "" { @@ -62,12 +62,12 @@ func initAuth(cfg *Config, database db.Database) (auth.JWTManager, *auth.Session } // Initialize session service - sessionService := auth.NewSessionService(database, jwtManager) + sessionManager := auth.NewSessionService(database, jwtManager) // Cookie service cookieService := auth.NewCookieService(cfg.IsDevelopment, cfg.Domain) - return jwtManager, sessionService, cookieService, nil + return jwtManager, sessionManager, cookieService, nil } // setupAdminUser creates the admin user if it doesn't exist diff --git a/server/internal/app/options.go b/server/internal/app/options.go index be5e3cc..21a8fa5 100644 --- a/server/internal/app/options.go +++ b/server/internal/app/options.go @@ -12,8 +12,8 @@ type Options struct { Database db.Database Storage storage.Manager JWTManager auth.JWTManager - SessionService *auth.SessionService - CookieService auth.CookieService + SessionManager auth.SessionManager + CookieService auth.CookieManager } // DefaultOptions creates server options with default configuration @@ -49,7 +49,7 @@ func DefaultOptions(cfg *Config) (*Options, error) { Database: database, Storage: storageManager, JWTManager: jwtManager, - SessionService: sessionService, + SessionManager: sessionService, CookieService: cookieService, }, nil } diff --git a/server/internal/app/routes.go b/server/internal/app/routes.go index ecd1a35..644dee6 100644 --- a/server/internal/app/routes.go +++ b/server/internal/app/routes.go @@ -48,7 +48,7 @@ func setupRouter(o Options) *chi.Mux { } // Initialize auth middleware and handler - authMiddleware := auth.NewMiddleware(o.JWTManager) + authMiddleware := auth.NewMiddleware(o.JWTManager, o.SessionManager, o.CookieService) handler := &handlers.Handler{ DB: o.Database, Storage: o.Storage, @@ -72,8 +72,8 @@ func setupRouter(o Options) *chi.Mux { // Public routes (no authentication required) r.Group(func(r chi.Router) { - r.Post("/auth/login", handler.Login(o.SessionService, o.CookieService)) - r.Post("/auth/refresh", handler.RefreshToken(o.SessionService, o.CookieService)) + r.Post("/auth/login", handler.Login(o.SessionManager, o.CookieService)) + r.Post("/auth/refresh", handler.RefreshToken(o.SessionManager, o.CookieService)) }) // Protected routes (authentication required) @@ -82,7 +82,7 @@ func setupRouter(o Options) *chi.Mux { r.Use(context.WithUserContextMiddleware) // Auth routes - r.Post("/auth/logout", handler.Logout(o.SessionService, o.CookieService)) + r.Post("/auth/logout", handler.Logout(o.SessionManager, o.CookieService)) r.Get("/auth/me", handler.GetCurrentUser()) // User profile routes diff --git a/server/internal/auth/cookies.go b/server/internal/auth/cookies.go index 7bed834..7249bad 100644 --- a/server/internal/auth/cookies.go +++ b/server/internal/auth/cookies.go @@ -5,8 +5,8 @@ import ( "net/http" ) -// CookieService interface defines methods for generating cookies -type CookieService interface { +// CookieManager interface defines methods for generating cookies +type CookieManager interface { GenerateAccessTokenCookie(token string) *http.Cookie GenerateRefreshTokenCookie(token string) *http.Cookie GenerateCSRFCookie(token string) *http.Cookie @@ -14,14 +14,14 @@ type CookieService interface { } // CookieService -type cookieService struct { +type cookieManager struct { Domain string Secure bool SameSite http.SameSite } // NewCookieService creates a new cookie service -func NewCookieService(isDevelopment bool, domain string) CookieService { +func NewCookieService(isDevelopment bool, domain string) CookieManager { secure := !isDevelopment var sameSite http.SameSite @@ -31,7 +31,7 @@ func NewCookieService(isDevelopment bool, domain string) CookieService { sameSite = http.SameSiteStrictMode } - return &cookieService{ + return &cookieManager{ Domain: domain, Secure: secure, SameSite: sameSite, @@ -39,7 +39,7 @@ func NewCookieService(isDevelopment bool, domain string) CookieService { } // GenerateAccessTokenCookie creates a new cookie for the access token -func (c *cookieService) GenerateAccessTokenCookie(token string) *http.Cookie { +func (c *cookieManager) GenerateAccessTokenCookie(token string) *http.Cookie { return &http.Cookie{ Name: "access_token", Value: token, @@ -52,7 +52,7 @@ func (c *cookieService) GenerateAccessTokenCookie(token string) *http.Cookie { } // GenerateRefreshTokenCookie creates a new cookie for the refresh token -func (c *cookieService) GenerateRefreshTokenCookie(token string) *http.Cookie { +func (c *cookieManager) GenerateRefreshTokenCookie(token string) *http.Cookie { return &http.Cookie{ Name: "refresh_token", Value: token, @@ -65,7 +65,7 @@ func (c *cookieService) GenerateRefreshTokenCookie(token string) *http.Cookie { } // GenerateCSRFCookie creates a new cookie for the CSRF token -func (c *cookieService) GenerateCSRFCookie(token string) *http.Cookie { +func (c *cookieManager) GenerateCSRFCookie(token string) *http.Cookie { return &http.Cookie{ Name: "csrf_token", Value: token, @@ -78,7 +78,7 @@ func (c *cookieService) GenerateCSRFCookie(token string) *http.Cookie { } // InvalidateCookie creates a new cookie with a MaxAge of -1 to invalidate the cookie -func (c *cookieService) InvalidateCookie(cookieType string) *http.Cookie { +func (c *cookieManager) InvalidateCookie(cookieType string) *http.Cookie { return &http.Cookie{ Name: cookieType, Value: "", diff --git a/server/internal/auth/middleware.go b/server/internal/auth/middleware.go index 7754f7b..6748fa3 100644 --- a/server/internal/auth/middleware.go +++ b/server/internal/auth/middleware.go @@ -9,13 +9,17 @@ import ( // Middleware handles JWT authentication for protected routes type Middleware struct { - jwtManager JWTManager + jwtManager JWTManager + sessionManager SessionManager + cookieManager CookieManager } // NewMiddleware creates a new authentication middleware -func NewMiddleware(jwtManager JWTManager) *Middleware { +func NewMiddleware(jwtManager JWTManager, sessionManager SessionManager, cookieManager CookieManager) *Middleware { return &Middleware{ - jwtManager: jwtManager, + jwtManager: jwtManager, + sessionManager: sessionManager, + cookieManager: cookieManager, } } @@ -42,6 +46,16 @@ func (m *Middleware) Authenticate(next http.Handler) http.Handler { return } + // Check if session is still valid in database + session, err := m.sessionManager.ValidateSession(claims.ID) + if err != nil || session == nil { + m.cookieManager.InvalidateCookie("access_token") + m.cookieManager.InvalidateCookie("refresh_token") + m.cookieManager.InvalidateCookie("csrf_token") + http.Error(w, "Session invalid or expired", http.StatusUnauthorized) + return + } + // Add CSRF check for non-GET requests if r.Method != http.MethodGet && r.Method != http.MethodHead && r.Method != http.MethodOptions { csrfCookie, err := r.Cookie("csrf_token") diff --git a/server/internal/auth/session.go b/server/internal/auth/session.go index afaaf06..21d0090 100644 --- a/server/internal/auth/session.go +++ b/server/internal/auth/session.go @@ -9,22 +9,30 @@ import ( "github.com/google/uuid" ) -// SessionService manages user sessions in the database -type SessionService struct { +type SessionManager interface { + CreateSession(userID int, role string) (*models.Session, string, error) + RefreshSession(refreshToken string) (string, error) + ValidateSession(sessionID string) (*models.Session, error) + InvalidateSession(token string) error + CleanExpiredSessions() error +} + +// sessionManager manages user sessions in the database +type sessionManager struct { db db.SessionStore // Database store for sessions jwtManager JWTManager // JWT Manager for token operations } // NewSessionService creates a new session service with the given database and JWT manager -func NewSessionService(db db.SessionStore, jwtManager JWTManager) *SessionService { - return &SessionService{ +func NewSessionService(db db.SessionStore, jwtManager JWTManager) *sessionManager { + return &sessionManager{ db: db, jwtManager: jwtManager, } } // CreateSession creates a new user session for a user with the given userID and role -func (s *SessionService) CreateSession(userID int, role string) (*models.Session, string, error) { +func (s *sessionManager) CreateSession(userID int, role string) (*models.Session, string, error) { // Generate both access and refresh tokens accessToken, err := s.jwtManager.GenerateAccessToken(userID, role) if err != nil { @@ -60,7 +68,7 @@ func (s *SessionService) CreateSession(userID int, role string) (*models.Session } // RefreshSession creates a new access token using a refreshToken -func (s *SessionService) RefreshSession(refreshToken string) (string, error) { +func (s *sessionManager) RefreshSession(refreshToken string) (string, error) { // Get session from database first session, err := s.db.GetSessionByRefreshToken(refreshToken) if err != nil { @@ -82,8 +90,20 @@ func (s *SessionService) RefreshSession(refreshToken string) (string, error) { return s.jwtManager.GenerateAccessToken(claims.UserID, claims.Role) } +// ValidateSession checks if a session with the given sessionID is valid +func (s *sessionManager) ValidateSession(sessionID string) (*models.Session, error) { + + // Get the session from the database + session, err := s.db.GetSessionByID(sessionID) + if err != nil { + return nil, fmt.Errorf("failed to get session: %w", err) + } + + return session, nil +} + // InvalidateSession removes a session with the given sessionID from the database -func (s *SessionService) InvalidateSession(token string) error { +func (s *sessionManager) InvalidateSession(token string) error { // Parse the JWT to get the session info claims, err := s.jwtManager.ValidateToken(token) if err != nil { @@ -94,6 +114,6 @@ func (s *SessionService) InvalidateSession(token string) error { } // CleanExpiredSessions removes all expired sessions from the database -func (s *SessionService) CleanExpiredSessions() error { +func (s *sessionManager) CleanExpiredSessions() error { return s.db.CleanExpiredSessions() } diff --git a/server/internal/db/db.go b/server/internal/db/db.go index a54ba4a..a0c21eb 100644 --- a/server/internal/db/db.go +++ b/server/internal/db/db.go @@ -53,6 +53,7 @@ type WorkspaceStore interface { type SessionStore interface { CreateSession(session *models.Session) error GetSessionByRefreshToken(refreshToken string) (*models.Session, error) + GetSessionByID(sessionID string) (*models.Session, error) DeleteSession(sessionID string) error CleanExpiredSessions() error } diff --git a/server/internal/db/sessions.go b/server/internal/db/sessions.go index ecc4a72..79b9231 100644 --- a/server/internal/db/sessions.go +++ b/server/internal/db/sessions.go @@ -41,6 +41,26 @@ func (db *database) GetSessionByRefreshToken(refreshToken string) (*models.Sessi return session, nil } +// GetSessionByID retrieves a session by its ID +func (db *database) GetSessionByID(sessionID string) (*models.Session, error) { + session := &models.Session{} + err := db.QueryRow(` + SELECT id, user_id, refresh_token, expires_at, created_at + FROM sessions + WHERE id = ? AND expires_at > ?`, + sessionID, time.Now(), + ).Scan(&session.ID, &session.UserID, &session.RefreshToken, &session.ExpiresAt, &session.CreatedAt) + + if err == sql.ErrNoRows { + return nil, fmt.Errorf("session not found") + } + if err != nil { + return nil, fmt.Errorf("failed to fetch session: %w", err) + } + + return session, nil +} + // DeleteSession removes a session from the database func (db *database) DeleteSession(sessionID string) error { result, err := db.Exec("DELETE FROM sessions WHERE id = ?", sessionID) diff --git a/server/internal/handlers/auth_handlers.go b/server/internal/handlers/auth_handlers.go index e59a98d..f55c650 100644 --- a/server/internal/handlers/auth_handlers.go +++ b/server/internal/handlers/auth_handlers.go @@ -39,7 +39,7 @@ type LoginResponse struct { // @Failure 401 {object} ErrorResponse "Invalid credentials" // @Failure 500 {object} ErrorResponse "Failed to create session" // @Router /auth/login [post] -func (h *Handler) Login(authService *auth.SessionService, cookieService auth.CookieService) http.HandlerFunc { +func (h *Handler) Login(authManager auth.SessionManager, cookieService auth.CookieManager) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { var req LoginRequest if err := json.NewDecoder(r.Body).Decode(&req); err != nil { @@ -68,7 +68,7 @@ func (h *Handler) Login(authService *auth.SessionService, cookieService auth.Coo } // Create session and generate tokens - session, accessToken, err := authService.CreateSession(user.ID, string(user.Role)) + session, accessToken, err := authManager.CreateSession(user.ID, string(user.Role)) if err != nil { respondError(w, "Failed to create session", http.StatusInternalServerError) return @@ -110,7 +110,7 @@ func (h *Handler) Login(authService *auth.SessionService, cookieService auth.Coo // @Failure 400 {object} ErrorResponse "Session ID required" // @Failure 500 {object} ErrorResponse "Failed to logout" // @Router /auth/logout [post] -func (h *Handler) Logout(authService *auth.SessionService, cookieService auth.CookieService) http.HandlerFunc { +func (h *Handler) Logout(authManager auth.SessionManager, cookieService auth.CookieManager) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { // Get session ID from cookie sessionCookie, err := r.Cookie("access_token") @@ -120,7 +120,7 @@ func (h *Handler) Logout(authService *auth.SessionService, cookieService auth.Co } // Invalidate the session in the database - if err := authService.InvalidateSession(sessionCookie.Value); err != nil { + if err := authManager.InvalidateSession(sessionCookie.Value); err != nil { respondError(w, "Failed to invalidate session", http.StatusInternalServerError) return } @@ -147,7 +147,7 @@ func (h *Handler) Logout(authService *auth.SessionService, cookieService auth.Co // @Failure 400 {object} ErrorResponse "Refresh token required" // @Failure 401 {object} ErrorResponse "Invalid refresh token" // @Router /auth/refresh [post] -func (h *Handler) RefreshToken(authService *auth.SessionService, cookieService auth.CookieService) http.HandlerFunc { +func (h *Handler) RefreshToken(authManager auth.SessionManager, cookieService auth.CookieManager) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { refreshCookie, err := r.Cookie("refresh_token") if err != nil { @@ -156,7 +156,7 @@ func (h *Handler) RefreshToken(authService *auth.SessionService, cookieService a } // Generate new access token - accessToken, err := authService.RefreshSession(refreshCookie.Value) + accessToken, err := authManager.RefreshSession(refreshCookie.Value) if err != nil { respondError(w, "Invalid refresh token", http.StatusUnauthorized) return diff --git a/server/internal/handlers/integration_test.go b/server/internal/handlers/integration_test.go index 047966f..4d8b6aa 100644 --- a/server/internal/handlers/integration_test.go +++ b/server/internal/handlers/integration_test.go @@ -24,17 +24,17 @@ import ( // testHarness encapsulates all the dependencies needed for testing type testHarness struct { - Server *app.Server - DB db.TestDatabase - Storage storage.Manager - JWTManager auth.JWTManager - SessionSvc *auth.SessionService - AdminUser *models.User - AdminToken string - RegularUser *models.User - RegularToken string - TempDirectory string - MockGit *MockGitClient + Server *app.Server + DB db.TestDatabase + Storage storage.Manager + JWTManager auth.JWTManager + SessionManager auth.SessionManager + AdminUser *models.User + AdminToken string + RegularUser *models.User + RegularToken string + TempDirectory string + MockGit *MockGitClient } // setupTestHarness creates a new test environment @@ -104,20 +104,20 @@ func setupTestHarness(t *testing.T) *testHarness { Database: database, Storage: storageSvc, JWTManager: jwtSvc, - SessionService: sessionSvc, + SessionManager: sessionSvc, } // Create server srv := app.NewServer(serverOpts) h := &testHarness{ - Server: srv, - DB: database, - Storage: storageSvc, - JWTManager: jwtSvc, - SessionSvc: sessionSvc, - TempDirectory: tempDir, - MockGit: mockGit, + Server: srv, + DB: database, + Storage: storageSvc, + JWTManager: jwtSvc, + SessionManager: sessionSvc, + TempDirectory: tempDir, + MockGit: mockGit, } // Create test users @@ -172,7 +172,7 @@ func (h *testHarness) createTestUser(t *testing.T, email, password string, role t.Fatalf("Failed to initialize user workspace: %v", err) } - session, accessToken, err := h.SessionSvc.CreateSession(user.ID, string(user.Role)) + session, accessToken, err := h.SessionManager.CreateSession(user.ID, string(user.Role)) if err != nil { t.Fatalf("Failed to create session: %v", err) } From ad4af2f82d9449252e8322ffb4da2e087881fdab Mon Sep 17 00:00:00 2001 From: LordMathis Date: Sat, 7 Dec 2024 21:41:37 +0100 Subject: [PATCH 03/12] Update auth test --- server/internal/auth/middleware_test.go | 156 ++++++++++++++++--- server/internal/auth/session_test.go | 196 +++++++++++++++++------- 2 files changed, 269 insertions(+), 83 deletions(-) diff --git a/server/internal/auth/middleware_test.go b/server/internal/auth/middleware_test.go index 06c44be..983c3d3 100644 --- a/server/internal/auth/middleware_test.go +++ b/server/internal/auth/middleware_test.go @@ -3,6 +3,7 @@ package auth_test import ( "net/http" "net/http/httptest" + "strings" "testing" "time" @@ -11,6 +12,42 @@ import ( "novamd/internal/models" ) +// Mock SessionManager +type mockSessionManager struct { + sessions map[string]*models.Session +} + +func newMockSessionManager() *mockSessionManager { + return &mockSessionManager{ + sessions: make(map[string]*models.Session), + } +} + +func (m *mockSessionManager) CreateSession(userID int, role string) (*models.Session, string, error) { + return nil, "", nil // Not needed for these tests +} + +func (m *mockSessionManager) RefreshSession(refreshToken string) (string, error) { + return "", nil // Not needed for these tests +} + +func (m *mockSessionManager) ValidateSession(sessionID string) (*models.Session, error) { + session, exists := m.sessions[sessionID] + if !exists { + return nil, nil + } + return session, nil +} + +func (m *mockSessionManager) InvalidateSession(token string) error { + delete(m.sessions, token) + return nil +} + +func (m *mockSessionManager) CleanExpiredSessions() error { + return nil +} + // Complete mockResponseWriter implementation type mockResponseWriter struct { headers http.Header @@ -44,62 +81,122 @@ func TestAuthenticateMiddleware(t *testing.T) { RefreshTokenExpiry: 24 * time.Hour, } jwtService, _ := auth.NewJWTService(config) - middleware := auth.NewMiddleware(jwtService) + sessionManager := newMockSessionManager() + cookieManager := auth.NewCookieService(true, "localhost") + middleware := auth.NewMiddleware(jwtService, sessionManager, cookieManager) testCases := []struct { name string - setupAuth func() string + setupRequest func() *http.Request + setupSession func(sessionID string) + method string wantStatusCode int }{ { - name: "valid token", - setupAuth: func() string { + name: "valid token with valid session", + setupRequest: func() *http.Request { + req := httptest.NewRequest("GET", "/test", nil) token, _ := jwtService.GenerateAccessToken(1, "admin") - return token + cookie := cookieManager.GenerateAccessTokenCookie(token) + req.AddCookie(cookie) + return req }, + setupSession: func(sessionID string) { + sessionManager.sessions[sessionID] = &models.Session{ + ID: sessionID, + UserID: 1, + ExpiresAt: time.Now().Add(15 * time.Minute), + } + }, + method: "GET", wantStatusCode: http.StatusOK, }, { - name: "missing auth header", - setupAuth: func() string { - return "" + name: "valid token but invalid session", + setupRequest: func() *http.Request { + req := httptest.NewRequest("GET", "/test", nil) + token, _ := jwtService.GenerateAccessToken(1, "admin") + cookie := cookieManager.GenerateAccessTokenCookie(token) + req.AddCookie(cookie) + return req }, + setupSession: func(sessionID string) {}, // No session setup + method: "GET", wantStatusCode: http.StatusUnauthorized, }, { - name: "invalid auth format", - setupAuth: func() string { - return "InvalidFormat token" + name: "missing auth cookie", + setupRequest: func() *http.Request { + return httptest.NewRequest("GET", "/test", nil) }, + setupSession: func(sessionID string) {}, + method: "GET", wantStatusCode: http.StatusUnauthorized, }, { - name: "invalid token", - setupAuth: func() string { - return "Bearer invalid.token.here" + name: "POST request without CSRF token", + setupRequest: func() *http.Request { + req := httptest.NewRequest("POST", "/test", nil) + token, _ := jwtService.GenerateAccessToken(1, "admin") + cookie := cookieManager.GenerateAccessTokenCookie(token) + req.AddCookie(cookie) + return req }, - wantStatusCode: http.StatusUnauthorized, + setupSession: func(sessionID string) { + sessionManager.sessions[sessionID] = &models.Session{ + ID: sessionID, + UserID: 1, + ExpiresAt: time.Now().Add(15 * time.Minute), + } + }, + method: "POST", + wantStatusCode: http.StatusForbidden, + }, + { + name: "POST request with valid CSRF token", + setupRequest: func() *http.Request { + req := httptest.NewRequest("POST", "/test", nil) + token, _ := jwtService.GenerateAccessToken(1, "admin") + cookie := cookieManager.GenerateAccessTokenCookie(token) + req.AddCookie(cookie) + + csrfToken := "test-csrf-token" + csrfCookie := cookieManager.GenerateCSRFCookie(csrfToken) + req.AddCookie(csrfCookie) + req.Header.Set("X-CSRF-Token", csrfToken) + return req + }, + setupSession: func(sessionID string) { + sessionManager.sessions[sessionID] = &models.Session{ + ID: sessionID, + UserID: 1, + ExpiresAt: time.Now().Add(15 * time.Minute), + } + }, + method: "POST", + wantStatusCode: http.StatusOK, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - // Create test request - req := httptest.NewRequest("GET", "/test", nil) - if token := tc.setupAuth(); token != "" { - req.Header.Set("Authorization", "Bearer "+token) - } - - // Create response recorder + req := tc.setupRequest() w := newMockResponseWriter() // Create test handler nextCalled := false - next := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { nextCalled = true w.WriteHeader(http.StatusOK) }) + // If we have a valid token, set up the session + if cookie, err := req.Cookie("access_token"); err == nil { + if claims, err := jwtService.ValidateToken(cookie.Value); err == nil { + tc.setupSession(claims.ID) + } + } + // Execute middleware middleware.Authenticate(next).ServeHTTP(w, req) @@ -115,6 +212,15 @@ func TestAuthenticateMiddleware(t *testing.T) { if tc.wantStatusCode != http.StatusOK && nextCalled { t.Error("next handler was called when it shouldn't have been") } + + // For unauthorized responses, check if cookies were invalidated + if w.statusCode == http.StatusUnauthorized { + for _, cookie := range w.Header()["Set-Cookie"] { + if strings.Contains(cookie, "Max-Age=0") { + t.Error("cookies were not properly invalidated") + } + } + } }) } } @@ -126,7 +232,7 @@ func TestRequireRole(t *testing.T) { RefreshTokenExpiry: 24 * time.Hour, } jwtService, _ := auth.NewJWTService(config) - middleware := auth.NewMiddleware(jwtService) + middleware := auth.NewMiddleware(jwtService, &mockSessionManager{}, auth.NewCookieService(true, "localhost")) testCases := []struct { name string @@ -198,7 +304,7 @@ func TestRequireWorkspaceAccess(t *testing.T) { SigningKey: "test-key", } jwtService, _ := auth.NewJWTService(config) - middleware := auth.NewMiddleware(jwtService) + middleware := auth.NewMiddleware(jwtService, &mockSessionManager{}, auth.NewCookieService(true, "localhost")) testCases := []struct { name string diff --git a/server/internal/auth/session_test.go b/server/internal/auth/session_test.go index 5457c37..8464fae 100644 --- a/server/internal/auth/session_test.go +++ b/server/internal/auth/session_test.go @@ -13,7 +13,7 @@ import ( // Mock SessionStore type mockSessionStore struct { sessions map[string]*models.Session - sessionsByToken map[string]*models.Session // Added index by refresh token + sessionsByToken map[string]*models.Session } func newMockSessionStore() *mockSessionStore { @@ -29,6 +29,17 @@ func (m *mockSessionStore) CreateSession(session *models.Session) error { return nil } +func (m *mockSessionStore) GetSessionByID(sessionID string) (*models.Session, error) { + session, exists := m.sessions[sessionID] + if !exists { + return nil, errors.New("session not found") + } + if session.ExpiresAt.Before(time.Now()) { + return nil, errors.New("session expired") + } + return session, nil +} + func (m *mockSessionStore) GetSessionByRefreshToken(refreshToken string) (*models.Session, error) { session, exists := m.sessionsByToken[refreshToken] if !exists { @@ -111,9 +122,9 @@ func TestCreateSession(t *testing.T) { } // Verify the session was stored - storedSession, exists := mockDB.sessions[session.ID] - if !exists { - t.Error("session was not stored in database") + storedSession, err := mockDB.GetSessionByID(session.ID) + if err != nil { + t.Errorf("failed to get stored session: %v", err) } if storedSession.RefreshToken != session.RefreshToken { t.Error("stored refresh token doesn't match") @@ -138,6 +149,97 @@ func TestCreateSession(t *testing.T) { } } +func TestValidateSession(t *testing.T) { + config := auth.JWTConfig{ + SigningKey: "test-key", + AccessTokenExpiry: 15 * time.Minute, + RefreshTokenExpiry: 24 * time.Hour, + } + jwtService, _ := auth.NewJWTService(config) + mockDB := newMockSessionStore() + sessionService := auth.NewSessionService(mockDB, jwtService) + + testCases := []struct { + name string + setupSession func() string + wantErr bool + errorContains string + }{ + { + name: "valid session", + setupSession: func() string { + session := &models.Session{ + ID: "test-session-1", + UserID: 1, + ExpiresAt: time.Now().Add(24 * time.Hour), + CreatedAt: time.Now(), + } + if err := mockDB.CreateSession(session); err != nil { + t.Fatalf("failed to create session: %v", err) + } + + return session.ID + }, + wantErr: false, + }, + { + name: "expired session", + setupSession: func() string { + session := &models.Session{ + ID: "test-session-2", + UserID: 1, + ExpiresAt: time.Now().Add(-1 * time.Hour), + CreatedAt: time.Now().Add(-2 * time.Hour), + } + if err := mockDB.CreateSession(session); err != nil { + t.Fatalf("failed to create session: %v", err) + } + return session.ID + }, + wantErr: true, + errorContains: "session expired", + }, + { + name: "non-existent session", + setupSession: func() string { + return "non-existent-session-id" + }, + wantErr: true, + errorContains: "session not found", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + sessionID := tc.setupSession() + session, err := sessionService.ValidateSession(sessionID) + + if tc.wantErr { + if err == nil { + t.Error("expected error, got nil") + } else if tc.errorContains != "" && !strings.Contains(err.Error(), tc.errorContains) { + t.Errorf("error = %v, want error containing %v", err, tc.errorContains) + } + return + } + + if err != nil { + t.Errorf("unexpected error: %v", err) + return + } + + if session == nil { + t.Error("expected session, got nil") + return + } + + if session.ID != sessionID { + t.Errorf("session ID = %v, want %v", session.ID, sessionID) + } + }) + } +} + func TestRefreshSession(t *testing.T) { config := auth.JWTConfig{ SigningKey: "test-key", @@ -180,7 +282,7 @@ func TestRefreshSession(t *testing.T) { ID: "test-session-2", UserID: 1, RefreshToken: token, - ExpiresAt: time.Now().Add(-1 * time.Hour), // Expired + ExpiresAt: time.Now().Add(-1 * time.Hour), CreatedAt: time.Now().Add(-2 * time.Hour), } if err := mockDB.CreateSession(session); err != nil { @@ -233,7 +335,7 @@ func TestRefreshSession(t *testing.T) { } } -func TestInvalidateSession(t *testing.T) { +func TestCleanExpiredSessions(t *testing.T) { config := auth.JWTConfig{ SigningKey: "test-key", AccessTokenExpiry: 15 * time.Minute, @@ -243,62 +345,40 @@ func TestInvalidateSession(t *testing.T) { mockDB := newMockSessionStore() sessionService := auth.NewSessionService(mockDB, jwtService) - testCases := []struct { - name string - setupSession func() string - wantErr bool - errorContains string - }{ - { - name: "valid session invalidation", - setupSession: func() string { - session := &models.Session{ - ID: "test-session-1", - UserID: 1, - RefreshToken: "valid-token", - ExpiresAt: time.Now().Add(24 * time.Hour), - CreatedAt: time.Now(), - } - if err := mockDB.CreateSession(session); err != nil { - t.Fatalf("failed to create session: %v", err) - } - return session.ID - }, - wantErr: false, - }, - { - name: "non-existent session", - setupSession: func() string { - return "non-existent-session-id" - }, - wantErr: true, - errorContains: "session not found", - }, + // Create test sessions + validSession := &models.Session{ + ID: "valid-session", + UserID: 1, + ExpiresAt: time.Now().Add(24 * time.Hour), + CreatedAt: time.Now(), + } + if err := mockDB.CreateSession(validSession); err != nil { + t.Fatalf("failed to create valid session: %v", err) } - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - sessionID := tc.setupSession() - err := sessionService.InvalidateSession(sessionID) + expiredSession := &models.Session{ + ID: "expired-session", + UserID: 2, + ExpiresAt: time.Now().Add(-1 * time.Hour), + CreatedAt: time.Now().Add(-2 * time.Hour), + } + if err := mockDB.CreateSession(expiredSession); err != nil { + t.Fatalf("failed to create expired session: %v", err) + } - if tc.wantErr { - if err == nil { - t.Error("expected error, got nil") - } else if !strings.Contains(err.Error(), tc.errorContains) { - t.Errorf("error = %v, want error containing %v", err, tc.errorContains) - } - return - } + // Clean expired sessions + err := sessionService.CleanExpiredSessions() + if err != nil { + t.Errorf("unexpected error cleaning sessions: %v", err) + } - if err != nil { - t.Errorf("unexpected error: %v", err) - return - } + // Verify valid session still exists + if _, err := mockDB.GetSessionByID(validSession.ID); err != nil { + t.Error("valid session was incorrectly removed") + } - // Verify session was removed - if _, exists := mockDB.sessions[sessionID]; exists { - t.Error("session still exists after invalidation") - } - }) + // Verify expired session was removed + if _, err := mockDB.GetSessionByID(expiredSession.ID); err == nil { + t.Error("expired session was not removed") } } From 5633406f5cf67a248f60ed381fb1a54927b6f63c Mon Sep 17 00:00:00 2001 From: LordMathis Date: Sat, 7 Dec 2024 23:09:57 +0100 Subject: [PATCH 04/12] Update handler integration tests --- .../admin_handlers_integration_test.go | 76 ++--- .../auth_handlers_integration_test.go | 313 ++++++++++++------ .../file_handlers_integration_test.go | 46 +-- .../handlers/git_handlers_integration_test.go | 26 +- server/internal/handlers/integration_test.go | 115 +++++-- .../user_handlers_integration_test.go | 47 +-- .../workspace_handlers_integration_test.go | 48 +-- 7 files changed, 427 insertions(+), 244 deletions(-) diff --git a/server/internal/handlers/admin_handlers_integration_test.go b/server/internal/handlers/admin_handlers_integration_test.go index 121ea4b..3a4d618 100644 --- a/server/internal/handlers/admin_handlers_integration_test.go +++ b/server/internal/handlers/admin_handlers_integration_test.go @@ -34,8 +34,8 @@ func TestAdminHandlers_Integration(t *testing.T) { t.Run("user management", func(t *testing.T) { t.Run("list users", func(t *testing.T) { - // Test with admin token - rr := h.makeRequest(t, http.MethodGet, "/api/v1/admin/users", nil, h.AdminToken, nil) + // Test with admin session + rr := h.makeRequest(t, http.MethodGet, "/api/v1/admin/users", nil, h.AdminSession, nil) require.Equal(t, http.StatusOK, rr.Code) var users []*models.User @@ -47,12 +47,12 @@ func TestAdminHandlers_Integration(t *testing.T) { assert.True(t, containsUser(users, h.AdminUser), "Admin user not found in users list") assert.True(t, containsUser(users, h.RegularUser), "Regular user not found in users list") - // Test with non-admin token - rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/users", nil, h.RegularToken, nil) + // Test with non-admin session + rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/users", nil, h.RegularSession, nil) assert.Equal(t, http.StatusForbidden, rr.Code) - // Test without token - rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/users", nil, "", nil) + // Test without session + rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/users", nil, nil, nil) assert.Equal(t, http.StatusUnauthorized, rr.Code) }) @@ -64,8 +64,8 @@ func TestAdminHandlers_Integration(t *testing.T) { Role: models.RoleEditor, } - // Test with admin token - rr := h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", createReq, h.AdminToken, nil) + // Test with admin session + rr := h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", createReq, h.AdminSession, nil) require.Equal(t, http.StatusOK, rr.Code) var createdUser models.User @@ -77,7 +77,7 @@ func TestAdminHandlers_Integration(t *testing.T) { assert.NotZero(t, createdUser.LastWorkspaceID) // Test duplicate email - rr = h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", createReq, h.AdminToken, nil) + rr = h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", createReq, h.AdminSession, nil) assert.Equal(t, http.StatusConflict, rr.Code) // Test invalid request (missing required fields) @@ -85,19 +85,19 @@ func TestAdminHandlers_Integration(t *testing.T) { Email: "invalid@test.com", // Missing password and role } - rr = h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", invalidReq, h.AdminToken, nil) + rr = h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", invalidReq, h.AdminSession, nil) assert.Equal(t, http.StatusBadRequest, rr.Code) - // Test with non-admin token - rr = h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", createReq, h.RegularToken, nil) + // Test with non-admin session + rr = h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", createReq, h.RegularSession, nil) assert.Equal(t, http.StatusForbidden, rr.Code) }) t.Run("get user", func(t *testing.T) { path := fmt.Sprintf("/api/v1/admin/users/%d", h.RegularUser.ID) - // Test with admin token - rr := h.makeRequest(t, http.MethodGet, path, nil, h.AdminToken, nil) + // Test with admin session + rr := h.makeRequest(t, http.MethodGet, path, nil, h.AdminSession, nil) require.Equal(t, http.StatusOK, rr.Code) var user models.User @@ -106,11 +106,11 @@ func TestAdminHandlers_Integration(t *testing.T) { assert.Equal(t, h.RegularUser.ID, user.ID) // Test non-existent user - rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/users/999999", nil, h.AdminToken, nil) + rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/users/999999", nil, h.AdminSession, nil) assert.Equal(t, http.StatusNotFound, rr.Code) - // Test with non-admin token - rr = h.makeRequest(t, http.MethodGet, path, nil, h.RegularToken, nil) + // Test with non-admin session + rr = h.makeRequest(t, http.MethodGet, path, nil, h.RegularSession, nil) assert.Equal(t, http.StatusForbidden, rr.Code) }) @@ -121,8 +121,8 @@ func TestAdminHandlers_Integration(t *testing.T) { Role: models.RoleViewer, } - // Test with admin token - rr := h.makeRequest(t, http.MethodPut, path, updateReq, h.AdminToken, nil) + // Test with admin session + rr := h.makeRequest(t, http.MethodPut, path, updateReq, h.AdminSession, nil) require.Equal(t, http.StatusOK, rr.Code) var updatedUser models.User @@ -131,8 +131,8 @@ func TestAdminHandlers_Integration(t *testing.T) { assert.Equal(t, updateReq.DisplayName, updatedUser.DisplayName) assert.Equal(t, updateReq.Role, updatedUser.Role) - // Test with non-admin token - rr = h.makeRequest(t, http.MethodPut, path, updateReq, h.RegularToken, nil) + // Test with non-admin session + rr = h.makeRequest(t, http.MethodPut, path, updateReq, h.RegularSession, nil) assert.Equal(t, http.StatusForbidden, rr.Code) }) @@ -145,7 +145,7 @@ func TestAdminHandlers_Integration(t *testing.T) { Role: models.RoleEditor, } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", createReq, h.AdminToken, nil) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", createReq, h.AdminSession, nil) require.Equal(t, http.StatusOK, rr.Code) var createdUser models.User @@ -156,19 +156,19 @@ func TestAdminHandlers_Integration(t *testing.T) { // Test deleting own account (should fail) adminPath := fmt.Sprintf("/api/v1/admin/users/%d", h.AdminUser.ID) - rr = h.makeRequest(t, http.MethodDelete, adminPath, nil, h.AdminToken, nil) + rr = h.makeRequest(t, http.MethodDelete, adminPath, nil, h.AdminSession, nil) assert.Equal(t, http.StatusBadRequest, rr.Code) - // Test with admin token - rr = h.makeRequest(t, http.MethodDelete, path, nil, h.AdminToken, nil) + // Test with admin session + rr = h.makeRequest(t, http.MethodDelete, path, nil, h.AdminSession, nil) assert.Equal(t, http.StatusNoContent, rr.Code) // Verify user is deleted - rr = h.makeRequest(t, http.MethodGet, path, nil, h.AdminToken, nil) + rr = h.makeRequest(t, http.MethodGet, path, nil, h.AdminSession, nil) assert.Equal(t, http.StatusNotFound, rr.Code) - // Test with non-admin token - rr = h.makeRequest(t, http.MethodDelete, path, nil, h.RegularToken, nil) + // Test with non-admin session + rr = h.makeRequest(t, http.MethodDelete, path, nil, h.RegularSession, nil) assert.Equal(t, http.StatusForbidden, rr.Code) }) }) @@ -181,11 +181,11 @@ func TestAdminHandlers_Integration(t *testing.T) { Name: "Test Workspace", } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) - // Test with admin token - rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/workspaces", nil, h.AdminToken, nil) + // Test with admin session + rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/workspaces", nil, h.AdminSession, nil) require.Equal(t, http.StatusOK, rr.Code) var workspaces []*handlers.WorkspaceStats @@ -206,8 +206,8 @@ func TestAdminHandlers_Integration(t *testing.T) { assert.GreaterOrEqual(t, ws.TotalSize, int64(0)) } - // Test with non-admin token - rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/workspaces", nil, h.RegularToken, nil) + // Test with non-admin session + rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/workspaces", nil, h.RegularSession, nil) assert.Equal(t, http.StatusForbidden, rr.Code) }) }) @@ -218,11 +218,11 @@ func TestAdminHandlers_Integration(t *testing.T) { UserID: h.RegularUser.ID, Name: "Stats Test Workspace", } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) - // Test with admin token - rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/stats", nil, h.AdminToken, nil) + // Test with admin session + rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/stats", nil, h.AdminSession, nil) require.Equal(t, http.StatusOK, rr.Code) var stats handlers.SystemStats @@ -236,8 +236,8 @@ func TestAdminHandlers_Integration(t *testing.T) { assert.GreaterOrEqual(t, stats.TotalFiles, 0) assert.GreaterOrEqual(t, stats.TotalSize, int64(0)) - // Test with non-admin token - rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/stats", nil, h.RegularToken, nil) + // Test with non-admin session + rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/stats", nil, h.RegularSession, nil) assert.Equal(t, http.StatusForbidden, rr.Code) }) } diff --git a/server/internal/handlers/auth_handlers_integration_test.go b/server/internal/handlers/auth_handlers_integration_test.go index 6db4153..fa979a6 100644 --- a/server/internal/handlers/auth_handlers_integration_test.go +++ b/server/internal/handlers/auth_handlers_integration_test.go @@ -4,8 +4,12 @@ package handlers_test import ( "encoding/json" + "io" "net/http" + "net/http/httptest" + "strings" "testing" + "time" "novamd/internal/handlers" "novamd/internal/models" @@ -25,40 +29,58 @@ func TestAuthHandlers_Integration(t *testing.T) { Password: "admin123", } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/auth/login", loginReq, "", nil) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/auth/login", loginReq, nil, nil) require.Equal(t, http.StatusOK, rr.Code) + // Verify all required cookies are present with correct attributes + cookies := rr.Result().Cookies() + var foundAccessToken, foundRefreshToken, foundCSRF bool + for _, cookie := range cookies { + switch cookie.Name { + case "access_token": + foundAccessToken = true + assert.True(t, cookie.HttpOnly, "access_token cookie must be HttpOnly") + assert.Equal(t, http.SameSiteStrictMode, cookie.SameSite) + assert.Equal(t, 900, cookie.MaxAge) // 15 minutes + case "refresh_token": + foundRefreshToken = true + assert.True(t, cookie.HttpOnly, "refresh_token cookie must be HttpOnly") + assert.Equal(t, http.SameSiteStrictMode, cookie.SameSite) + assert.Equal(t, 604800, cookie.MaxAge) // 7 days + case "csrf_token": + foundCSRF = true + assert.False(t, cookie.HttpOnly, "csrf_token cookie must not be HttpOnly") + assert.Equal(t, http.SameSiteStrictMode, cookie.SameSite) + assert.Equal(t, 900, cookie.MaxAge) // 15 minutes + } + } + assert.True(t, foundAccessToken, "access_token cookie not found") + assert.True(t, foundRefreshToken, "refresh_token cookie not found") + assert.True(t, foundCSRF, "csrf_token cookie not found") + + // Verify CSRF token is in both cookie and header, and they match + var csrfCookie *http.Cookie + for _, cookie := range rr.Result().Cookies() { + if cookie.Name == "csrf_token" { + csrfCookie = cookie + break + } + } + require.NotNil(t, csrfCookie, "csrf_token cookie not found") + csrfHeader := rr.Header().Get("X-CSRF-Token") + assert.Equal(t, csrfCookie.Value, csrfHeader) + + // Verify response body var resp handlers.LoginResponse err := json.NewDecoder(rr.Body).Decode(&resp) require.NoError(t, err) - - assert.NotEmpty(t, resp.AccessToken) - assert.NotEmpty(t, resp.RefreshToken) + assert.NotEmpty(t, resp.SessionID) + assert.False(t, resp.ExpiresAt.IsZero()) assert.NotNil(t, resp.User) assert.Equal(t, loginReq.Email, resp.User.Email) assert.Equal(t, models.RoleAdmin, resp.User.Role) }) - t.Run("successful login - regular user", func(t *testing.T) { - loginReq := handlers.LoginRequest{ - Email: "user@test.com", - Password: "user123", - } - - rr := h.makeRequest(t, http.MethodPost, "/api/v1/auth/login", loginReq, "", nil) - require.Equal(t, http.StatusOK, rr.Code) - - var resp handlers.LoginResponse - err := json.NewDecoder(rr.Body).Decode(&resp) - require.NoError(t, err) - - assert.NotEmpty(t, resp.AccessToken) - assert.NotEmpty(t, resp.RefreshToken) - assert.NotNil(t, resp.User) - assert.Equal(t, loginReq.Email, resp.User.Email) - assert.Equal(t, models.RoleEditor, resp.User.Role) - }) - t.Run("login failures", func(t *testing.T) { tests := []struct { name string @@ -97,12 +119,26 @@ func TestAuthHandlers_Integration(t *testing.T) { }, wantCode: http.StatusBadRequest, }, + { + name: "malformed JSON", + request: handlers.LoginRequest{}, // Will be overridden with bad JSON + wantCode: http.StatusBadRequest, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - rr := h.makeRequest(t, http.MethodPost, "/api/v1/auth/login", tt.request, "", nil) + var rr *httptest.ResponseRecorder + if tt.name == "malformed JSON" { + // Need lower level helper to send malformed JSON + req := h.newRequest(t, http.MethodPost, "/api/v1/auth/login", nil) + req.Body = io.NopCloser(strings.NewReader("{bad json")) + rr = h.executeRequest(req) + } else { + rr = h.makeRequest(t, http.MethodPost, "/api/v1/auth/login", tt.request, nil, nil) + } assert.Equal(t, tt.wantCode, rr.Code) + assert.Empty(t, rr.Result().Cookies(), "failed login should not set cookies") }) } }) @@ -110,58 +146,76 @@ func TestAuthHandlers_Integration(t *testing.T) { t.Run("refresh token", func(t *testing.T) { t.Run("successful token refresh", func(t *testing.T) { - // First login to get refresh token - loginReq := handlers.LoginRequest{ - Email: "user@test.com", - Password: "user123", - } - - rr := h.makeRequest(t, http.MethodPost, "/api/v1/auth/login", loginReq, "", nil) + // Need lower level helpers for precise cookie control + req := h.newRequest(t, http.MethodPost, "/api/v1/auth/refresh", nil) + h.addAuthCookies(t, req, h.RegularSession, true) // Adds both tokens + rr := h.executeRequest(req) require.Equal(t, http.StatusOK, rr.Code) - var loginResp handlers.LoginResponse - err := json.NewDecoder(rr.Body).Decode(&loginResp) - require.NoError(t, err) - - // Now try to refresh the token - refreshReq := handlers.RefreshRequest{ - RefreshToken: loginResp.RefreshToken, + // Verify new cookies + cookies := rr.Result().Cookies() + var foundAccessToken, foundCSRF bool + for _, cookie := range cookies { + switch cookie.Name { + case "access_token": + foundAccessToken = true + assert.Equal(t, 900, cookie.MaxAge) + case "csrf_token": + foundCSRF = true + assert.Equal(t, 900, cookie.MaxAge) + case "refresh_token": + t.Error("refresh token should not be renewed") + } } - - rr = h.makeRequest(t, http.MethodPost, "/api/v1/auth/refresh", refreshReq, "", nil) - require.Equal(t, http.StatusOK, rr.Code) - - var refreshResp handlers.RefreshResponse - err = json.NewDecoder(rr.Body).Decode(&refreshResp) - require.NoError(t, err) - assert.NotEmpty(t, refreshResp.AccessToken) + assert.True(t, foundAccessToken, "new access_token cookie not found") + assert.True(t, foundCSRF, "new csrf_token cookie not found") }) - t.Run("refresh failures", func(t *testing.T) { + t.Run("refresh token edge cases", func(t *testing.T) { tests := []struct { name string - request handlers.RefreshRequest + setup func(*http.Request) wantCode int }{ { - name: "invalid refresh token", - request: handlers.RefreshRequest{ - RefreshToken: "invalid-token", + name: "missing refresh token cookie", + setup: func(req *http.Request) { + // Only add access token + token, _ := h.JWTManager.GenerateAccessToken(h.RegularSession.UserID, "admin") + req.AddCookie(h.CookieManager.GenerateAccessTokenCookie(token)) + }, + wantCode: http.StatusBadRequest, + }, + { + name: "expired refresh token", + setup: func(req *http.Request) { + expiredSession := &models.Session{ + ID: "expired", + UserID: h.RegularUser.ID, + RefreshToken: "expired-token", + ExpiresAt: time.Now().Add(-1 * time.Hour), + } + h.addAuthCookies(t, req, expiredSession, true) }, wantCode: http.StatusUnauthorized, }, { - name: "empty refresh token", - request: handlers.RefreshRequest{ - RefreshToken: "", + name: "invalid refresh token format", + setup: func(req *http.Request) { + req.AddCookie(&http.Cookie{ + Name: "refresh_token", + Value: "invalid-format", + }) }, - wantCode: http.StatusBadRequest, + wantCode: http.StatusUnauthorized, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - rr := h.makeRequest(t, http.MethodPost, "/api/v1/auth/refresh", tt.request, "", nil) + req := h.newRequest(t, http.MethodPost, "/api/v1/auth/refresh", nil) + tt.setup(req) + rr := h.executeRequest(req) assert.Equal(t, tt.wantCode, rr.Code) }) } @@ -170,63 +224,136 @@ func TestAuthHandlers_Integration(t *testing.T) { t.Run("logout", func(t *testing.T) { t.Run("successful logout", func(t *testing.T) { - // First login to get session - loginReq := handlers.LoginRequest{ - Email: "user@test.com", - Password: "user123", - } - - rr := h.makeRequest(t, http.MethodPost, "/api/v1/auth/login", loginReq, "", nil) - require.Equal(t, http.StatusOK, rr.Code) - - var loginResp handlers.LoginResponse - err := json.NewDecoder(rr.Body).Decode(&loginResp) - require.NoError(t, err) - - // Now logout using session ID from login response - headers := map[string]string{ - "X-Session-ID": loginResp.Session.ID, - } - rr = h.makeRequest(t, http.MethodPost, "/api/v1/auth/logout", nil, loginResp.AccessToken, headers) + // Need CSRF token for POST request + req := h.newRequest(t, http.MethodPost, "/api/v1/auth/logout", nil) + csrfToken := h.addAuthCookies(t, req, h.RegularSession, true) + req.Header.Set("X-CSRF-Token", csrfToken) + rr := h.executeRequest(req) require.Equal(t, http.StatusNoContent, rr.Code) - // Try to use the refresh token - should fail - refreshReq := handlers.RefreshRequest{ - RefreshToken: loginResp.RefreshToken, + // Verify cookies are properly invalidated + for _, cookie := range rr.Result().Cookies() { + assert.True(t, cookie.MaxAge < 0, "cookie should be invalidated") + assert.True(t, cookie.Expires.Before(time.Now()), "cookie should be expired") } - rr = h.makeRequest(t, http.MethodPost, "/api/v1/auth/refresh", refreshReq, "", nil) + // Verify session is actually invalidated + rr = h.makeRequest(t, http.MethodGet, "/api/v1/auth/me", nil, h.RegularSession, nil) assert.Equal(t, http.StatusUnauthorized, rr.Code) }) - t.Run("logout without session ID", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodPost, "/api/v1/auth/logout", nil, h.RegularToken, nil) - assert.Equal(t, http.StatusBadRequest, rr.Code) + t.Run("logout edge cases", func(t *testing.T) { + tests := []struct { + name string + setup func(*http.Request) + wantCode int + }{ + { + name: "missing CSRF token", + setup: func(req *http.Request) { + h.addAuthCookies(t, req, h.RegularSession, true) + // Deliberately not setting X-CSRF-Token header + }, + wantCode: http.StatusForbidden, + }, + { + name: "mismatched CSRF token", + setup: func(req *http.Request) { + h.addAuthCookies(t, req, h.RegularSession, true) + req.Header.Set("X-CSRF-Token", "wrong-token") + }, + wantCode: http.StatusForbidden, + }, + { + name: "missing auth cookies", + setup: func(req *http.Request) { + // No setup - testing completely unauthenticated request + }, + wantCode: http.StatusUnauthorized, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := h.newRequest(t, http.MethodPost, "/api/v1/auth/logout", nil) + tt.setup(req) + rr := h.executeRequest(req) + assert.Equal(t, tt.wantCode, rr.Code) + }) + } }) }) t.Run("get current user", func(t *testing.T) { t.Run("successful get current user", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodGet, "/api/v1/auth/me", nil, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodGet, "/api/v1/auth/me", nil, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) var user models.User err := json.NewDecoder(rr.Body).Decode(&user) require.NoError(t, err) - - assert.Equal(t, h.RegularUser.ID, user.ID) assert.Equal(t, h.RegularUser.Email, user.Email) - assert.Equal(t, h.RegularUser.Role, user.Role) }) - t.Run("get current user without token", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodGet, "/api/v1/auth/me", nil, "", nil) - assert.Equal(t, http.StatusUnauthorized, rr.Code) - }) + t.Run("auth edge cases", func(t *testing.T) { + tests := []struct { + name string + setup func(*http.Request) + wantCode int + }{ + { + name: "missing auth cookie", + setup: func(req *http.Request) { + // No setup - testing unauthenticated request + }, + wantCode: http.StatusUnauthorized, + }, + { + name: "invalid session ID", + setup: func(req *http.Request) { + invalidSession := &models.Session{ + ID: "invalid", + UserID: 999, + RefreshToken: "invalid", + ExpiresAt: time.Now().Add(time.Hour), + } + h.addAuthCookies(t, req, invalidSession, false) + }, + wantCode: http.StatusUnauthorized, + }, + { + name: "expired session", + setup: func(req *http.Request) { + expiredSession := &models.Session{ + ID: "expired", + UserID: h.RegularUser.ID, + RefreshToken: "expired-token", + ExpiresAt: time.Now().Add(-1 * time.Hour), + } + h.addAuthCookies(t, req, expiredSession, false) + }, + wantCode: http.StatusUnauthorized, + }, + { + name: "malformed access token", + setup: func(req *http.Request) { + req.AddCookie(&http.Cookie{ + Name: "access_token", + Value: "malformed-token", + }) + }, + wantCode: http.StatusUnauthorized, + }, + } - t.Run("get current user with invalid token", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodGet, "/api/v1/auth/me", nil, "invalid-token", nil) - assert.Equal(t, http.StatusUnauthorized, rr.Code) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := h.newRequest(t, http.MethodGet, "/api/v1/auth/me", nil) + tt.setup(req) + rr := h.executeRequest(req) + assert.Equal(t, tt.wantCode, rr.Code) + }) + } }) }) } diff --git a/server/internal/handlers/file_handlers_integration_test.go b/server/internal/handlers/file_handlers_integration_test.go index 516c508..e317575 100644 --- a/server/internal/handlers/file_handlers_integration_test.go +++ b/server/internal/handlers/file_handlers_integration_test.go @@ -27,7 +27,7 @@ func TestFileHandlers_Integration(t *testing.T) { UserID: h.RegularUser.ID, Name: "File Test Workspace", } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) err := json.NewDecoder(rr.Body).Decode(workspace) @@ -37,7 +37,7 @@ func TestFileHandlers_Integration(t *testing.T) { baseURL := fmt.Sprintf("/api/v1/workspaces/%s/files", url.PathEscape(workspace.Name)) t.Run("list empty directory", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodGet, baseURL, nil, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodGet, baseURL, nil, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) var files []storage.FileNode @@ -51,16 +51,16 @@ func TestFileHandlers_Integration(t *testing.T) { filePath := "test.md" // Save file - rr := h.makeRequestRaw(t, http.MethodPost, baseURL+"/"+filePath, strings.NewReader(content), h.RegularToken, nil) + rr := h.makeRequestRaw(t, http.MethodPost, baseURL+"/"+filePath, strings.NewReader(content), h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) // Get file content - rr = h.makeRequest(t, http.MethodGet, baseURL+"/"+filePath, nil, h.RegularToken, nil) + rr = h.makeRequest(t, http.MethodGet, baseURL+"/"+filePath, nil, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) assert.Equal(t, content, rr.Body.String()) // List directory should now show the file - rr = h.makeRequest(t, http.MethodGet, baseURL, nil, h.RegularToken, nil) + rr = h.makeRequest(t, http.MethodGet, baseURL, nil, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) var files []storage.FileNode @@ -80,12 +80,12 @@ func TestFileHandlers_Integration(t *testing.T) { // Create all files for path, content := range files { - rr := h.makeRequest(t, http.MethodPost, baseURL+"/"+path, content, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPost, baseURL+"/"+path, content, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) } // List all files - rr := h.makeRequest(t, http.MethodGet, baseURL, nil, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodGet, baseURL, nil, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) var fileNodes []storage.FileNode @@ -116,11 +116,11 @@ func TestFileHandlers_Integration(t *testing.T) { // Look up a file that exists in multiple locations filename := "readme.md" dupContent := "Another readme" - rr := h.makeRequest(t, http.MethodPost, baseURL+"/projects/"+filename, dupContent, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPost, baseURL+"/projects/"+filename, dupContent, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) // Search for the file - rr = h.makeRequest(t, http.MethodGet, baseURL+"/lookup?filename="+filename, nil, h.RegularToken, nil) + rr = h.makeRequest(t, http.MethodGet, baseURL+"/lookup?filename="+filename, nil, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) var response struct { @@ -131,7 +131,7 @@ func TestFileHandlers_Integration(t *testing.T) { assert.Len(t, response.Paths, 2) // Search for non-existent file - rr = h.makeRequest(t, http.MethodGet, baseURL+"/lookup?filename=nonexistent.md", nil, h.RegularToken, nil) + rr = h.makeRequest(t, http.MethodGet, baseURL+"/lookup?filename=nonexistent.md", nil, h.RegularSession, nil) assert.Equal(t, http.StatusNotFound, rr.Code) }) @@ -140,21 +140,21 @@ func TestFileHandlers_Integration(t *testing.T) { content := "This file will be deleted" // Create file - rr := h.makeRequest(t, http.MethodPost, baseURL+"/"+filePath, content, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPost, baseURL+"/"+filePath, content, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) // Delete file - rr = h.makeRequest(t, http.MethodDelete, baseURL+"/"+filePath, nil, h.RegularToken, nil) + rr = h.makeRequest(t, http.MethodDelete, baseURL+"/"+filePath, nil, h.RegularSession, nil) require.Equal(t, http.StatusNoContent, rr.Code) // Verify file is gone - rr = h.makeRequest(t, http.MethodGet, baseURL+"/"+filePath, nil, h.RegularToken, nil) + rr = h.makeRequest(t, http.MethodGet, baseURL+"/"+filePath, nil, h.RegularSession, nil) assert.Equal(t, http.StatusNotFound, rr.Code) }) t.Run("last opened file", func(t *testing.T) { // Initially should be empty - rr := h.makeRequest(t, http.MethodGet, baseURL+"/last", nil, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodGet, baseURL+"/last", nil, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) var response struct { @@ -170,11 +170,11 @@ func TestFileHandlers_Integration(t *testing.T) { }{ FilePath: "docs/readme.md", } - rr = h.makeRequest(t, http.MethodPut, baseURL+"/last", updateReq, h.RegularToken, nil) + rr = h.makeRequest(t, http.MethodPut, baseURL+"/last", updateReq, h.RegularSession, nil) require.Equal(t, http.StatusNoContent, rr.Code) // Verify update - rr = h.makeRequest(t, http.MethodGet, baseURL+"/last", nil, h.RegularToken, nil) + rr = h.makeRequest(t, http.MethodGet, baseURL+"/last", nil, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) err = json.NewDecoder(rr.Body).Decode(&response) @@ -183,7 +183,7 @@ func TestFileHandlers_Integration(t *testing.T) { // Test invalid file path updateReq.FilePath = "nonexistent.md" - rr = h.makeRequest(t, http.MethodPut, baseURL+"/last", updateReq, h.RegularToken, nil) + rr = h.makeRequest(t, http.MethodPut, baseURL+"/last", updateReq, h.RegularSession, nil) assert.Equal(t, http.StatusNotFound, rr.Code) }) @@ -204,12 +204,12 @@ func TestFileHandlers_Integration(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - // Test without token - rr := h.makeRequest(t, tc.method, tc.path, tc.body, "", nil) + // Test without session + rr := h.makeRequest(t, tc.method, tc.path, tc.body, nil, nil) assert.Equal(t, http.StatusUnauthorized, rr.Code) - // Test with wrong user's token - rr = h.makeRequest(t, tc.method, tc.path, tc.body, h.AdminToken, nil) + // Test with wrong user's session + rr = h.makeRequest(t, tc.method, tc.path, tc.body, h.AdminSession, nil) assert.Equal(t, http.StatusNotFound, rr.Code) }) } @@ -226,11 +226,11 @@ func TestFileHandlers_Integration(t *testing.T) { for _, path := range maliciousPaths { t.Run(path, func(t *testing.T) { // Try to read - rr := h.makeRequest(t, http.MethodGet, baseURL+"/"+path, nil, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodGet, baseURL+"/"+path, nil, h.RegularSession, nil) assert.Equal(t, http.StatusBadRequest, rr.Code) // Try to write - rr = h.makeRequest(t, http.MethodPost, baseURL+"/"+path, "malicious content", h.RegularToken, nil) + rr = h.makeRequest(t, http.MethodPost, baseURL+"/"+path, "malicious content", h.RegularSession, nil) assert.Equal(t, http.StatusBadRequest, rr.Code) }) } diff --git a/server/internal/handlers/git_handlers_integration_test.go b/server/internal/handlers/git_handlers_integration_test.go index c7706c8..f754974 100644 --- a/server/internal/handlers/git_handlers_integration_test.go +++ b/server/internal/handlers/git_handlers_integration_test.go @@ -32,7 +32,7 @@ func TestGitHandlers_Integration(t *testing.T) { GitCommitMsgTemplate: "Update: {{message}}", } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) err := json.NewDecoder(rr.Body).Decode(workspace) @@ -50,7 +50,7 @@ func TestGitHandlers_Integration(t *testing.T) { "message": commitMsg, } - rr := h.makeRequest(t, http.MethodPost, baseURL+"/commit", requestBody, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPost, baseURL+"/commit", requestBody, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) var response map[string]string @@ -70,7 +70,7 @@ func TestGitHandlers_Integration(t *testing.T) { "message": "", } - rr := h.makeRequest(t, http.MethodPost, baseURL+"/commit", requestBody, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPost, baseURL+"/commit", requestBody, h.RegularSession, nil) assert.Equal(t, http.StatusBadRequest, rr.Code) assert.Equal(t, 0, h.MockGit.GetCommitCount(), "Commit should not be called") }) @@ -83,7 +83,7 @@ func TestGitHandlers_Integration(t *testing.T) { "message": "Test message", } - rr := h.makeRequest(t, http.MethodPost, baseURL+"/commit", requestBody, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPost, baseURL+"/commit", requestBody, h.RegularSession, nil) assert.Equal(t, http.StatusInternalServerError, rr.Code) h.MockGit.SetError(nil) // Reset error state @@ -94,7 +94,7 @@ func TestGitHandlers_Integration(t *testing.T) { h.MockGit.Reset() t.Run("successful pull", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodPost, baseURL+"/pull", nil, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPost, baseURL+"/pull", nil, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) var response map[string]string @@ -109,7 +109,7 @@ func TestGitHandlers_Integration(t *testing.T) { h.MockGit.Reset() h.MockGit.SetError(fmt.Errorf("mock git error")) - rr := h.makeRequest(t, http.MethodPost, baseURL+"/pull", nil, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPost, baseURL+"/pull", nil, h.RegularSession, nil) assert.Equal(t, http.StatusInternalServerError, rr.Code) h.MockGit.SetError(nil) // Reset error state @@ -140,12 +140,12 @@ func TestGitHandlers_Integration(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - // Test without token - rr := h.makeRequest(t, tc.method, tc.path, tc.body, "", nil) + // Test without session + rr := h.makeRequest(t, tc.method, tc.path, tc.body, nil, nil) assert.Equal(t, http.StatusUnauthorized, rr.Code) - // Test with wrong user's token - rr = h.makeRequest(t, tc.method, tc.path, tc.body, h.AdminToken, nil) + // Test with wrong user's session + rr = h.makeRequest(t, tc.method, tc.path, tc.body, h.AdminSession, nil) assert.Equal(t, http.StatusNotFound, rr.Code) }) } @@ -160,7 +160,7 @@ func TestGitHandlers_Integration(t *testing.T) { Name: "Non-Git Workspace", } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", nonGitWorkspace, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", nonGitWorkspace, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) err := json.NewDecoder(rr.Body).Decode(nonGitWorkspace) @@ -170,11 +170,11 @@ func TestGitHandlers_Integration(t *testing.T) { // Try to commit commitMsg := map[string]string{"message": "test"} - rr = h.makeRequest(t, http.MethodPost, nonGitBaseURL+"/commit", commitMsg, h.RegularToken, nil) + rr = h.makeRequest(t, http.MethodPost, nonGitBaseURL+"/commit", commitMsg, h.RegularSession, nil) assert.Equal(t, http.StatusInternalServerError, rr.Code) // Try to pull - rr = h.makeRequest(t, http.MethodPost, nonGitBaseURL+"/pull", nil, h.RegularToken, nil) + rr = h.makeRequest(t, http.MethodPost, nonGitBaseURL+"/pull", nil, h.RegularSession, nil) assert.Equal(t, http.StatusInternalServerError, rr.Code) }) }) diff --git a/server/internal/handlers/integration_test.go b/server/internal/handlers/integration_test.go index 4d8b6aa..5377a8f 100644 --- a/server/internal/handlers/integration_test.go +++ b/server/internal/handlers/integration_test.go @@ -6,6 +6,7 @@ import ( "bytes" "encoding/json" "io" + "net/http" "net/http/httptest" "os" "testing" @@ -29,10 +30,11 @@ type testHarness struct { Storage storage.Manager JWTManager auth.JWTManager SessionManager auth.SessionManager + CookieManager auth.CookieManager AdminUser *models.User - AdminToken string + AdminSession *models.Session RegularUser *models.User - RegularToken string + RegularSession *models.Session TempDirectory string MockGit *MockGitClient } @@ -86,6 +88,9 @@ func setupTestHarness(t *testing.T) *testHarness { // Initialize session service sessionSvc := auth.NewSessionService(database, jwtSvc) + // Initialize cookie service + cookieSvc := auth.NewCookieService(true, "localhost") + // Create test config testConfig := &app.Config{ DBPath: ":memory:", @@ -116,18 +121,19 @@ func setupTestHarness(t *testing.T) *testHarness { Storage: storageSvc, JWTManager: jwtSvc, SessionManager: sessionSvc, + CookieManager: cookieSvc, TempDirectory: tempDir, MockGit: mockGit, } // Create test users - adminUser, adminToken := h.createTestUser(t, "admin@test.com", "admin123", models.RoleAdmin) - regularUser, regularToken := h.createTestUser(t, "user@test.com", "user123", models.RoleEditor) + adminUser, adminSession := h.createTestUser(t, "admin@test.com", "admin123", models.RoleAdmin) + regularUser, regularSession := h.createTestUser(t, "user@test.com", "user123", models.RoleEditor) h.AdminUser = adminUser - h.AdminToken = adminToken + h.AdminSession = adminSession h.RegularUser = regularUser - h.RegularToken = regularToken + h.RegularSession = regularSession return h } @@ -146,7 +152,7 @@ func (h *testHarness) teardown(t *testing.T) { } // createTestUser creates a test user and returns the user and access token -func (h *testHarness) createTestUser(t *testing.T, email, password string, role models.UserRole) (*models.User, string) { +func (h *testHarness) createTestUser(t *testing.T, email, password string, role models.UserRole) (*models.User, *models.Session) { t.Helper() hashedPassword, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost) @@ -172,25 +178,19 @@ func (h *testHarness) createTestUser(t *testing.T, email, password string, role t.Fatalf("Failed to initialize user workspace: %v", err) } - session, accessToken, err := h.SessionManager.CreateSession(user.ID, string(user.Role)) + session, _, err := h.SessionManager.CreateSession(user.ID, string(user.Role)) if err != nil { t.Fatalf("Failed to create session: %v", err) } - if session == nil || accessToken == "" { - t.Fatal("Failed to get valid session or token") - } - - return user, accessToken + return user, session } -// makeRequest is a helper function to make HTTP requests in tests -func (h *testHarness) makeRequest(t *testing.T, method, path string, body interface{}, token string, headers map[string]string) *httptest.ResponseRecorder { +func (h *testHarness) newRequest(t *testing.T, method, path string, body interface{}) *http.Request { t.Helper() var reqBody []byte var err error - if body != nil { reqBody, err = json.Marshal(body) if err != nil { @@ -199,38 +199,87 @@ func (h *testHarness) makeRequest(t *testing.T, method, path string, body interf } req := httptest.NewRequest(method, path, bytes.NewBuffer(reqBody)) - if token != "" { - req.Header.Set("Authorization", "Bearer "+token) - } req.Header.Set("Content-Type", "application/json") + return req +} - // Add any additional headers - for k, v := range headers { - req.Header.Set(k, v) - } +// newRequestRaw creates a new request with raw body +func (h *testHarness) newRequestRaw(t *testing.T, method, path string, body io.Reader) *http.Request { + t.Helper() + return httptest.NewRequest(method, path, body) +} +// executeRequest executes the request and returns response recorder +func (h *testHarness) executeRequest(req *http.Request) *httptest.ResponseRecorder { rr := httptest.NewRecorder() h.Server.Router().ServeHTTP(rr, req) - return rr } -// makeRequestRaw is a helper function to make HTTP requests with raw body content -func (h *testHarness) makeRequestRaw(t *testing.T, method, path string, body io.Reader, token string, headers map[string]string) *httptest.ResponseRecorder { +// addAuthCookies adds authentication cookies to request +func (h *testHarness) addAuthCookies(t *testing.T, req *http.Request, session *models.Session, addCSRF bool) string { t.Helper() - req := httptest.NewRequest(method, path, body) - if token != "" { - req.Header.Set("Authorization", "Bearer "+token) + if session == nil { + return "" } - // Add any additional headers + accessToken, err := h.JWTManager.GenerateAccessToken(session.UserID, "admin") + if err != nil { + t.Fatalf("Failed to generate access token: %v", err) + } + + req.AddCookie(h.CookieManager.GenerateAccessTokenCookie(accessToken)) + req.AddCookie(h.CookieManager.GenerateRefreshTokenCookie(session.RefreshToken)) + + if addCSRF { + csrfToken := "test-csrf-token" + req.AddCookie(h.CookieManager.GenerateCSRFCookie(csrfToken)) + return csrfToken + } + return "" +} + +// makeRequest is the main helper for making JSON requests +func (h *testHarness) makeRequest(t *testing.T, method, path string, body interface{}, session *models.Session, headers map[string]string) *httptest.ResponseRecorder { + t.Helper() + + req := h.newRequest(t, method, path, body) + + // Add custom headers for k, v := range headers { req.Header.Set(k, v) } - rr := httptest.NewRecorder() - h.Server.Router().ServeHTTP(rr, req) + if session != nil { + needsCSRF := method != http.MethodGet && method != http.MethodHead && method != http.MethodOptions + csrfToken := h.addAuthCookies(t, req, session, needsCSRF) + if needsCSRF { + req.Header.Set("X-CSRF-Token", csrfToken) + } + } - return rr + return h.executeRequest(req) +} + +// makeRequestRawWithHeaders adds support for custom headers with raw body +func (h *testHarness) makeRequestRaw(t *testing.T, method, path string, body io.Reader, session *models.Session, headers map[string]string) *httptest.ResponseRecorder { + t.Helper() + + req := h.newRequestRaw(t, method, path, body) + + // Add custom headers + for k, v := range headers { + req.Header.Set(k, v) + } + + if session != nil { + needsCSRF := method != http.MethodGet && method != http.MethodHead && method != http.MethodOptions + csrfToken := h.addAuthCookies(t, req, session, needsCSRF) + if needsCSRF { + req.Header.Set("X-CSRF-Token", csrfToken) + } + } + + return h.executeRequest(req) } diff --git a/server/internal/handlers/user_handlers_integration_test.go b/server/internal/handlers/user_handlers_integration_test.go index 9c41772..378703e 100644 --- a/server/internal/handlers/user_handlers_integration_test.go +++ b/server/internal/handlers/user_handlers_integration_test.go @@ -23,7 +23,7 @@ func TestUserHandlers_Integration(t *testing.T) { t.Run("get current user", func(t *testing.T) { t.Run("successful get", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodGet, "/api/v1/auth/me", nil, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodGet, "/api/v1/auth/me", nil, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) var user models.User @@ -38,7 +38,7 @@ func TestUserHandlers_Integration(t *testing.T) { }) t.Run("unauthorized", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodGet, "/api/v1/auth/me", nil, "", nil) + rr := h.makeRequest(t, http.MethodGet, "/api/v1/auth/me", nil, nil, nil) assert.Equal(t, http.StatusUnauthorized, rr.Code) }) }) @@ -49,7 +49,7 @@ func TestUserHandlers_Integration(t *testing.T) { DisplayName: "Updated Name", } - rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) var user models.User @@ -64,7 +64,7 @@ func TestUserHandlers_Integration(t *testing.T) { CurrentPassword: currentPassword, } - rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) var user models.User @@ -80,7 +80,7 @@ func TestUserHandlers_Integration(t *testing.T) { Email: "anotheremail@test.com", } - rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession, nil) assert.Equal(t, http.StatusBadRequest, rr.Code) }) @@ -90,7 +90,7 @@ func TestUserHandlers_Integration(t *testing.T) { CurrentPassword: "wrongpassword", } - rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession, nil) assert.Equal(t, http.StatusUnauthorized, rr.Code) }) @@ -100,7 +100,7 @@ func TestUserHandlers_Integration(t *testing.T) { NewPassword: "newpassword123", } - rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) // Verify can login with new password @@ -109,7 +109,7 @@ func TestUserHandlers_Integration(t *testing.T) { Password: "newpassword123", } - rr = h.makeRequest(t, http.MethodPost, "/api/v1/auth/login", loginReq, "", nil) + rr = h.makeRequest(t, http.MethodPost, "/api/v1/auth/login", loginReq, nil, nil) assert.Equal(t, http.StatusOK, rr.Code) currentPassword = updateReq.NewPassword @@ -120,7 +120,7 @@ func TestUserHandlers_Integration(t *testing.T) { NewPassword: "newpass123", } - rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession, nil) assert.Equal(t, http.StatusBadRequest, rr.Code) }) @@ -130,7 +130,7 @@ func TestUserHandlers_Integration(t *testing.T) { NewPassword: "newpass123", } - rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession, nil) assert.Equal(t, http.StatusUnauthorized, rr.Code) }) @@ -140,7 +140,7 @@ func TestUserHandlers_Integration(t *testing.T) { NewPassword: "short", } - rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession, nil) assert.Equal(t, http.StatusBadRequest, rr.Code) }) @@ -150,7 +150,7 @@ func TestUserHandlers_Integration(t *testing.T) { CurrentPassword: currentPassword, } - rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession, nil) assert.Equal(t, http.StatusConflict, rr.Code) }) }) @@ -164,37 +164,44 @@ func TestUserHandlers_Integration(t *testing.T) { Role: models.RoleEditor, } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", createReq, h.AdminToken, nil) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", createReq, h.AdminSession, nil) require.Equal(t, http.StatusOK, rr.Code) var newUser models.User err := json.NewDecoder(rr.Body).Decode(&newUser) require.NoError(t, err) - // Get token for new user + // Get session for new user loginReq := handlers.LoginRequest{ Email: createReq.Email, Password: createReq.Password, } - rr = h.makeRequest(t, http.MethodPost, "/api/v1/auth/login", loginReq, "", nil) + rr = h.makeRequest(t, http.MethodPost, "/api/v1/auth/login", loginReq, nil, nil) require.Equal(t, http.StatusOK, rr.Code) var loginResp handlers.LoginResponse err = json.NewDecoder(rr.Body).Decode(&loginResp) require.NoError(t, err) - userToken := loginResp.AccessToken + + // Create a session struct for the new user + userSession := &models.Session{ + ID: loginResp.SessionID, + UserID: newUser.ID, + RefreshToken: "", + ExpiresAt: loginResp.ExpiresAt, + } t.Run("successful delete", func(t *testing.T) { deleteReq := handlers.DeleteAccountRequest{ Password: createReq.Password, } - rr := h.makeRequest(t, http.MethodDelete, "/api/v1/profile", deleteReq, userToken, nil) + rr := h.makeRequest(t, http.MethodDelete, "/api/v1/profile", deleteReq, userSession, nil) require.Equal(t, http.StatusNoContent, rr.Code) // Verify user is deleted - rr = h.makeRequest(t, http.MethodPost, "/api/v1/auth/login", loginReq, "", nil) + rr = h.makeRequest(t, http.MethodPost, "/api/v1/auth/login", loginReq, nil, nil) assert.Equal(t, http.StatusUnauthorized, rr.Code) }) @@ -203,7 +210,7 @@ func TestUserHandlers_Integration(t *testing.T) { Password: "wrongpassword", } - rr := h.makeRequest(t, http.MethodDelete, "/api/v1/profile", deleteReq, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodDelete, "/api/v1/profile", deleteReq, h.RegularSession, nil) assert.Equal(t, http.StatusUnauthorized, rr.Code) }) @@ -212,7 +219,7 @@ func TestUserHandlers_Integration(t *testing.T) { Password: "admin123", // Admin password from test harness } - rr := h.makeRequest(t, http.MethodDelete, "/api/v1/profile", deleteReq, h.AdminToken, nil) + rr := h.makeRequest(t, http.MethodDelete, "/api/v1/profile", deleteReq, h.AdminSession, nil) assert.Equal(t, http.StatusForbidden, rr.Code) }) }) diff --git a/server/internal/handlers/workspace_handlers_integration_test.go b/server/internal/handlers/workspace_handlers_integration_test.go index e07efc0..e973b83 100644 --- a/server/internal/handlers/workspace_handlers_integration_test.go +++ b/server/internal/handlers/workspace_handlers_integration_test.go @@ -20,7 +20,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { t.Run("list workspaces", func(t *testing.T) { t.Run("successful list", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodGet, "/api/v1/workspaces", nil, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodGet, "/api/v1/workspaces", nil, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) var workspaces []*models.Workspace @@ -30,7 +30,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { }) t.Run("unauthorized", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodGet, "/api/v1/workspaces", nil, "", nil) + rr := h.makeRequest(t, http.MethodGet, "/api/v1/workspaces", nil, nil, nil) assert.Equal(t, http.StatusUnauthorized, rr.Code) }) }) @@ -41,7 +41,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { Name: "Test Workspace", } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) var created models.Workspace @@ -64,7 +64,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { GitCommitEmail: "test@example.com", } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) var created models.Workspace @@ -86,7 +86,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { // Missing required Git settings } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession, nil) assert.Equal(t, http.StatusBadRequest, rr.Code) }) }) @@ -95,7 +95,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { workspace := &models.Workspace{ Name: "Test Workspace Operations", } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) err := json.NewDecoder(rr.Body).Decode(workspace) require.NoError(t, err) @@ -105,7 +105,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { t.Run("get workspace", func(t *testing.T) { t.Run("successful get", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodGet, baseURL, nil, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodGet, baseURL, nil, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) var got models.Workspace @@ -116,13 +116,13 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { }) t.Run("nonexistent workspace", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodGet, "/api/v1/workspaces/nonexistent", nil, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodGet, "/api/v1/workspaces/nonexistent", nil, h.RegularSession, nil) assert.Equal(t, http.StatusNotFound, rr.Code) }) t.Run("unauthorized access", func(t *testing.T) { // Try accessing with another user's token - rr := h.makeRequest(t, http.MethodGet, baseURL, nil, h.AdminToken, nil) + rr := h.makeRequest(t, http.MethodGet, baseURL, nil, h.AdminSession, nil) assert.Equal(t, http.StatusNotFound, rr.Code) }) }) @@ -131,7 +131,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { t.Run("update name", func(t *testing.T) { workspace.Name = "Updated Workspace" - rr := h.makeRequest(t, http.MethodPut, baseURL, workspace, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPut, baseURL, workspace, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) var updated models.Workspace @@ -152,7 +152,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { ShowHiddenFiles: true, } - rr := h.makeRequest(t, http.MethodPut, baseURL, update, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPut, baseURL, update, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) var updated models.Workspace @@ -176,7 +176,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { GitCommitEmail: "test@example.com", } - rr := h.makeRequest(t, http.MethodPut, baseURL, update, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPut, baseURL, update, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) var updated models.Workspace @@ -200,14 +200,14 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { // Missing required Git settings } - rr := h.makeRequest(t, http.MethodPut, baseURL, update, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPut, baseURL, update, h.RegularSession, nil) assert.Equal(t, http.StatusBadRequest, rr.Code) }) }) t.Run("last workspace", func(t *testing.T) { t.Run("get last workspace", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodGet, "/api/v1/workspaces/last", nil, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodGet, "/api/v1/workspaces/last", nil, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) var response struct { @@ -225,11 +225,11 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { WorkspaceName: workspace.Name, } - rr := h.makeRequest(t, http.MethodPut, "/api/v1/workspaces/last", req, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPut, "/api/v1/workspaces/last", req, h.RegularSession, nil) require.Equal(t, http.StatusNoContent, rr.Code) // Verify the update - rr = h.makeRequest(t, http.MethodGet, "/api/v1/workspaces/last", nil, h.RegularToken, nil) + rr = h.makeRequest(t, http.MethodGet, "/api/v1/workspaces/last", nil, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) var response struct { @@ -243,7 +243,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { t.Run("delete workspace", func(t *testing.T) { // Get current workspaces to know how many we have - rr := h.makeRequest(t, http.MethodGet, "/api/v1/workspaces", nil, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodGet, "/api/v1/workspaces", nil, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) var existingWorkspaces []*models.Workspace @@ -254,13 +254,13 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { newWorkspace := &models.Workspace{ Name: "Workspace To Delete", } - rr = h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", newWorkspace, h.RegularToken, nil) + rr = h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", newWorkspace, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) err = json.NewDecoder(rr.Body).Decode(newWorkspace) require.NoError(t, err) t.Run("successful delete", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodDelete, "/api/v1/workspaces/"+url.PathEscape(newWorkspace.Name), nil, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodDelete, "/api/v1/workspaces/"+url.PathEscape(newWorkspace.Name), nil, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) var response struct { @@ -271,7 +271,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { assert.NotEmpty(t, response.NextWorkspaceName) // Verify workspace is deleted - rr = h.makeRequest(t, http.MethodGet, "/api/v1/workspaces/"+url.PathEscape(newWorkspace.Name), nil, h.RegularToken, nil) + rr = h.makeRequest(t, http.MethodGet, "/api/v1/workspaces/"+url.PathEscape(newWorkspace.Name), nil, h.RegularSession, nil) assert.Equal(t, http.StatusNotFound, rr.Code) }) @@ -279,13 +279,13 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { // Delete all but one workspace for i := 0; i < len(existingWorkspaces)-1; i++ { ws := existingWorkspaces[i] - rr := h.makeRequest(t, http.MethodDelete, "/api/v1/workspaces/"+url.PathEscape(ws.Name), nil, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodDelete, "/api/v1/workspaces/"+url.PathEscape(ws.Name), nil, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) } // Try to delete the last remaining workspace lastWs := existingWorkspaces[len(existingWorkspaces)-1] - rr := h.makeRequest(t, http.MethodDelete, "/api/v1/workspaces/"+url.PathEscape(lastWs.Name), nil, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodDelete, "/api/v1/workspaces/"+url.PathEscape(lastWs.Name), nil, h.RegularSession, nil) assert.Equal(t, http.StatusBadRequest, rr.Code) }) @@ -294,11 +294,11 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { workspace := &models.Workspace{ Name: "Unauthorized Delete Test", } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularToken, nil) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession, nil) require.Equal(t, http.StatusOK, rr.Code) // Try to delete with wrong user's token - rr = h.makeRequest(t, http.MethodDelete, "/api/v1/workspaces/"+url.PathEscape(workspace.Name), nil, h.AdminToken, nil) + rr = h.makeRequest(t, http.MethodDelete, "/api/v1/workspaces/"+url.PathEscape(workspace.Name), nil, h.AdminSession, nil) assert.Equal(t, http.StatusNotFound, rr.Code) }) }) From 2e1ccb45d6f34a79efaf0ff761fefb772fe04e68 Mon Sep 17 00:00:00 2001 From: LordMathis Date: Sat, 7 Dec 2024 23:16:12 +0100 Subject: [PATCH 05/12] Remove extra argument --- .../admin_handlers_integration_test.go | 46 +++++++++--------- .../auth_handlers_integration_test.go | 8 ++-- .../file_handlers_integration_test.go | 42 ++++++++-------- .../handlers/git_handlers_integration_test.go | 22 ++++----- server/internal/handlers/integration_test.go | 14 +----- .../user_handlers_integration_test.go | 36 +++++++------- .../workspace_handlers_integration_test.go | 48 +++++++++---------- 7 files changed, 103 insertions(+), 113 deletions(-) diff --git a/server/internal/handlers/admin_handlers_integration_test.go b/server/internal/handlers/admin_handlers_integration_test.go index 3a4d618..d633f92 100644 --- a/server/internal/handlers/admin_handlers_integration_test.go +++ b/server/internal/handlers/admin_handlers_integration_test.go @@ -35,7 +35,7 @@ func TestAdminHandlers_Integration(t *testing.T) { t.Run("user management", func(t *testing.T) { t.Run("list users", func(t *testing.T) { // Test with admin session - rr := h.makeRequest(t, http.MethodGet, "/api/v1/admin/users", nil, h.AdminSession, nil) + rr := h.makeRequest(t, http.MethodGet, "/api/v1/admin/users", nil, h.AdminSession) require.Equal(t, http.StatusOK, rr.Code) var users []*models.User @@ -48,11 +48,11 @@ func TestAdminHandlers_Integration(t *testing.T) { assert.True(t, containsUser(users, h.RegularUser), "Regular user not found in users list") // Test with non-admin session - rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/users", nil, h.RegularSession, nil) + rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/users", nil, h.RegularSession) assert.Equal(t, http.StatusForbidden, rr.Code) // Test without session - rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/users", nil, nil, nil) + rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/users", nil, nil) assert.Equal(t, http.StatusUnauthorized, rr.Code) }) @@ -65,7 +65,7 @@ func TestAdminHandlers_Integration(t *testing.T) { } // Test with admin session - rr := h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", createReq, h.AdminSession, nil) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", createReq, h.AdminSession) require.Equal(t, http.StatusOK, rr.Code) var createdUser models.User @@ -77,7 +77,7 @@ func TestAdminHandlers_Integration(t *testing.T) { assert.NotZero(t, createdUser.LastWorkspaceID) // Test duplicate email - rr = h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", createReq, h.AdminSession, nil) + rr = h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", createReq, h.AdminSession) assert.Equal(t, http.StatusConflict, rr.Code) // Test invalid request (missing required fields) @@ -85,11 +85,11 @@ func TestAdminHandlers_Integration(t *testing.T) { Email: "invalid@test.com", // Missing password and role } - rr = h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", invalidReq, h.AdminSession, nil) + rr = h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", invalidReq, h.AdminSession) assert.Equal(t, http.StatusBadRequest, rr.Code) // Test with non-admin session - rr = h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", createReq, h.RegularSession, nil) + rr = h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", createReq, h.RegularSession) assert.Equal(t, http.StatusForbidden, rr.Code) }) @@ -97,7 +97,7 @@ func TestAdminHandlers_Integration(t *testing.T) { path := fmt.Sprintf("/api/v1/admin/users/%d", h.RegularUser.ID) // Test with admin session - rr := h.makeRequest(t, http.MethodGet, path, nil, h.AdminSession, nil) + rr := h.makeRequest(t, http.MethodGet, path, nil, h.AdminSession) require.Equal(t, http.StatusOK, rr.Code) var user models.User @@ -106,11 +106,11 @@ func TestAdminHandlers_Integration(t *testing.T) { assert.Equal(t, h.RegularUser.ID, user.ID) // Test non-existent user - rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/users/999999", nil, h.AdminSession, nil) + rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/users/999999", nil, h.AdminSession) assert.Equal(t, http.StatusNotFound, rr.Code) // Test with non-admin session - rr = h.makeRequest(t, http.MethodGet, path, nil, h.RegularSession, nil) + rr = h.makeRequest(t, http.MethodGet, path, nil, h.RegularSession) assert.Equal(t, http.StatusForbidden, rr.Code) }) @@ -122,7 +122,7 @@ func TestAdminHandlers_Integration(t *testing.T) { } // Test with admin session - rr := h.makeRequest(t, http.MethodPut, path, updateReq, h.AdminSession, nil) + rr := h.makeRequest(t, http.MethodPut, path, updateReq, h.AdminSession) require.Equal(t, http.StatusOK, rr.Code) var updatedUser models.User @@ -132,7 +132,7 @@ func TestAdminHandlers_Integration(t *testing.T) { assert.Equal(t, updateReq.Role, updatedUser.Role) // Test with non-admin session - rr = h.makeRequest(t, http.MethodPut, path, updateReq, h.RegularSession, nil) + rr = h.makeRequest(t, http.MethodPut, path, updateReq, h.RegularSession) assert.Equal(t, http.StatusForbidden, rr.Code) }) @@ -145,7 +145,7 @@ func TestAdminHandlers_Integration(t *testing.T) { Role: models.RoleEditor, } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", createReq, h.AdminSession, nil) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", createReq, h.AdminSession) require.Equal(t, http.StatusOK, rr.Code) var createdUser models.User @@ -156,19 +156,19 @@ func TestAdminHandlers_Integration(t *testing.T) { // Test deleting own account (should fail) adminPath := fmt.Sprintf("/api/v1/admin/users/%d", h.AdminUser.ID) - rr = h.makeRequest(t, http.MethodDelete, adminPath, nil, h.AdminSession, nil) + rr = h.makeRequest(t, http.MethodDelete, adminPath, nil, h.AdminSession) assert.Equal(t, http.StatusBadRequest, rr.Code) // Test with admin session - rr = h.makeRequest(t, http.MethodDelete, path, nil, h.AdminSession, nil) + rr = h.makeRequest(t, http.MethodDelete, path, nil, h.AdminSession) assert.Equal(t, http.StatusNoContent, rr.Code) // Verify user is deleted - rr = h.makeRequest(t, http.MethodGet, path, nil, h.AdminSession, nil) + rr = h.makeRequest(t, http.MethodGet, path, nil, h.AdminSession) assert.Equal(t, http.StatusNotFound, rr.Code) // Test with non-admin session - rr = h.makeRequest(t, http.MethodDelete, path, nil, h.RegularSession, nil) + rr = h.makeRequest(t, http.MethodDelete, path, nil, h.RegularSession) assert.Equal(t, http.StatusForbidden, rr.Code) }) }) @@ -181,11 +181,11 @@ func TestAdminHandlers_Integration(t *testing.T) { Name: "Test Workspace", } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) // Test with admin session - rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/workspaces", nil, h.AdminSession, nil) + rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/workspaces", nil, h.AdminSession) require.Equal(t, http.StatusOK, rr.Code) var workspaces []*handlers.WorkspaceStats @@ -207,7 +207,7 @@ func TestAdminHandlers_Integration(t *testing.T) { } // Test with non-admin session - rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/workspaces", nil, h.RegularSession, nil) + rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/workspaces", nil, h.RegularSession) assert.Equal(t, http.StatusForbidden, rr.Code) }) }) @@ -218,11 +218,11 @@ func TestAdminHandlers_Integration(t *testing.T) { UserID: h.RegularUser.ID, Name: "Stats Test Workspace", } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) // Test with admin session - rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/stats", nil, h.AdminSession, nil) + rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/stats", nil, h.AdminSession) require.Equal(t, http.StatusOK, rr.Code) var stats handlers.SystemStats @@ -237,7 +237,7 @@ func TestAdminHandlers_Integration(t *testing.T) { assert.GreaterOrEqual(t, stats.TotalSize, int64(0)) // Test with non-admin session - rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/stats", nil, h.RegularSession, nil) + rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/stats", nil, h.RegularSession) assert.Equal(t, http.StatusForbidden, rr.Code) }) } diff --git a/server/internal/handlers/auth_handlers_integration_test.go b/server/internal/handlers/auth_handlers_integration_test.go index fa979a6..4214aef 100644 --- a/server/internal/handlers/auth_handlers_integration_test.go +++ b/server/internal/handlers/auth_handlers_integration_test.go @@ -29,7 +29,7 @@ func TestAuthHandlers_Integration(t *testing.T) { Password: "admin123", } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/auth/login", loginReq, nil, nil) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/auth/login", loginReq, nil) require.Equal(t, http.StatusOK, rr.Code) // Verify all required cookies are present with correct attributes @@ -135,7 +135,7 @@ func TestAuthHandlers_Integration(t *testing.T) { req.Body = io.NopCloser(strings.NewReader("{bad json")) rr = h.executeRequest(req) } else { - rr = h.makeRequest(t, http.MethodPost, "/api/v1/auth/login", tt.request, nil, nil) + rr = h.makeRequest(t, http.MethodPost, "/api/v1/auth/login", tt.request, nil) } assert.Equal(t, tt.wantCode, rr.Code) assert.Empty(t, rr.Result().Cookies(), "failed login should not set cookies") @@ -238,7 +238,7 @@ func TestAuthHandlers_Integration(t *testing.T) { } // Verify session is actually invalidated - rr = h.makeRequest(t, http.MethodGet, "/api/v1/auth/me", nil, h.RegularSession, nil) + rr = h.makeRequest(t, http.MethodGet, "/api/v1/auth/me", nil, h.RegularSession) assert.Equal(t, http.StatusUnauthorized, rr.Code) }) @@ -286,7 +286,7 @@ func TestAuthHandlers_Integration(t *testing.T) { t.Run("get current user", func(t *testing.T) { t.Run("successful get current user", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodGet, "/api/v1/auth/me", nil, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodGet, "/api/v1/auth/me", nil, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) var user models.User diff --git a/server/internal/handlers/file_handlers_integration_test.go b/server/internal/handlers/file_handlers_integration_test.go index e317575..0a72fab 100644 --- a/server/internal/handlers/file_handlers_integration_test.go +++ b/server/internal/handlers/file_handlers_integration_test.go @@ -27,7 +27,7 @@ func TestFileHandlers_Integration(t *testing.T) { UserID: h.RegularUser.ID, Name: "File Test Workspace", } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) err := json.NewDecoder(rr.Body).Decode(workspace) @@ -37,7 +37,7 @@ func TestFileHandlers_Integration(t *testing.T) { baseURL := fmt.Sprintf("/api/v1/workspaces/%s/files", url.PathEscape(workspace.Name)) t.Run("list empty directory", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodGet, baseURL, nil, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodGet, baseURL, nil, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) var files []storage.FileNode @@ -51,16 +51,16 @@ func TestFileHandlers_Integration(t *testing.T) { filePath := "test.md" // Save file - rr := h.makeRequestRaw(t, http.MethodPost, baseURL+"/"+filePath, strings.NewReader(content), h.RegularSession, nil) + rr := h.makeRequestRaw(t, http.MethodPost, baseURL+"/"+filePath, strings.NewReader(content), h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) // Get file content - rr = h.makeRequest(t, http.MethodGet, baseURL+"/"+filePath, nil, h.RegularSession, nil) + rr = h.makeRequest(t, http.MethodGet, baseURL+"/"+filePath, nil, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) assert.Equal(t, content, rr.Body.String()) // List directory should now show the file - rr = h.makeRequest(t, http.MethodGet, baseURL, nil, h.RegularSession, nil) + rr = h.makeRequest(t, http.MethodGet, baseURL, nil, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) var files []storage.FileNode @@ -80,12 +80,12 @@ func TestFileHandlers_Integration(t *testing.T) { // Create all files for path, content := range files { - rr := h.makeRequest(t, http.MethodPost, baseURL+"/"+path, content, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPost, baseURL+"/"+path, content, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) } // List all files - rr := h.makeRequest(t, http.MethodGet, baseURL, nil, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodGet, baseURL, nil, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) var fileNodes []storage.FileNode @@ -116,11 +116,11 @@ func TestFileHandlers_Integration(t *testing.T) { // Look up a file that exists in multiple locations filename := "readme.md" dupContent := "Another readme" - rr := h.makeRequest(t, http.MethodPost, baseURL+"/projects/"+filename, dupContent, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPost, baseURL+"/projects/"+filename, dupContent, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) // Search for the file - rr = h.makeRequest(t, http.MethodGet, baseURL+"/lookup?filename="+filename, nil, h.RegularSession, nil) + rr = h.makeRequest(t, http.MethodGet, baseURL+"/lookup?filename="+filename, nil, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) var response struct { @@ -131,7 +131,7 @@ func TestFileHandlers_Integration(t *testing.T) { assert.Len(t, response.Paths, 2) // Search for non-existent file - rr = h.makeRequest(t, http.MethodGet, baseURL+"/lookup?filename=nonexistent.md", nil, h.RegularSession, nil) + rr = h.makeRequest(t, http.MethodGet, baseURL+"/lookup?filename=nonexistent.md", nil, h.RegularSession) assert.Equal(t, http.StatusNotFound, rr.Code) }) @@ -140,21 +140,21 @@ func TestFileHandlers_Integration(t *testing.T) { content := "This file will be deleted" // Create file - rr := h.makeRequest(t, http.MethodPost, baseURL+"/"+filePath, content, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPost, baseURL+"/"+filePath, content, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) // Delete file - rr = h.makeRequest(t, http.MethodDelete, baseURL+"/"+filePath, nil, h.RegularSession, nil) + rr = h.makeRequest(t, http.MethodDelete, baseURL+"/"+filePath, nil, h.RegularSession) require.Equal(t, http.StatusNoContent, rr.Code) // Verify file is gone - rr = h.makeRequest(t, http.MethodGet, baseURL+"/"+filePath, nil, h.RegularSession, nil) + rr = h.makeRequest(t, http.MethodGet, baseURL+"/"+filePath, nil, h.RegularSession) assert.Equal(t, http.StatusNotFound, rr.Code) }) t.Run("last opened file", func(t *testing.T) { // Initially should be empty - rr := h.makeRequest(t, http.MethodGet, baseURL+"/last", nil, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodGet, baseURL+"/last", nil, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) var response struct { @@ -170,11 +170,11 @@ func TestFileHandlers_Integration(t *testing.T) { }{ FilePath: "docs/readme.md", } - rr = h.makeRequest(t, http.MethodPut, baseURL+"/last", updateReq, h.RegularSession, nil) + rr = h.makeRequest(t, http.MethodPut, baseURL+"/last", updateReq, h.RegularSession) require.Equal(t, http.StatusNoContent, rr.Code) // Verify update - rr = h.makeRequest(t, http.MethodGet, baseURL+"/last", nil, h.RegularSession, nil) + rr = h.makeRequest(t, http.MethodGet, baseURL+"/last", nil, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) err = json.NewDecoder(rr.Body).Decode(&response) @@ -183,7 +183,7 @@ func TestFileHandlers_Integration(t *testing.T) { // Test invalid file path updateReq.FilePath = "nonexistent.md" - rr = h.makeRequest(t, http.MethodPut, baseURL+"/last", updateReq, h.RegularSession, nil) + rr = h.makeRequest(t, http.MethodPut, baseURL+"/last", updateReq, h.RegularSession) assert.Equal(t, http.StatusNotFound, rr.Code) }) @@ -205,11 +205,11 @@ func TestFileHandlers_Integration(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { // Test without session - rr := h.makeRequest(t, tc.method, tc.path, tc.body, nil, nil) + rr := h.makeRequest(t, tc.method, tc.path, tc.body, nil) assert.Equal(t, http.StatusUnauthorized, rr.Code) // Test with wrong user's session - rr = h.makeRequest(t, tc.method, tc.path, tc.body, h.AdminSession, nil) + rr = h.makeRequest(t, tc.method, tc.path, tc.body, h.AdminSession) assert.Equal(t, http.StatusNotFound, rr.Code) }) } @@ -226,11 +226,11 @@ func TestFileHandlers_Integration(t *testing.T) { for _, path := range maliciousPaths { t.Run(path, func(t *testing.T) { // Try to read - rr := h.makeRequest(t, http.MethodGet, baseURL+"/"+path, nil, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodGet, baseURL+"/"+path, nil, h.RegularSession) assert.Equal(t, http.StatusBadRequest, rr.Code) // Try to write - rr = h.makeRequest(t, http.MethodPost, baseURL+"/"+path, "malicious content", h.RegularSession, nil) + rr = h.makeRequest(t, http.MethodPost, baseURL+"/"+path, "malicious content", h.RegularSession) assert.Equal(t, http.StatusBadRequest, rr.Code) }) } diff --git a/server/internal/handlers/git_handlers_integration_test.go b/server/internal/handlers/git_handlers_integration_test.go index f754974..c9ddba2 100644 --- a/server/internal/handlers/git_handlers_integration_test.go +++ b/server/internal/handlers/git_handlers_integration_test.go @@ -32,7 +32,7 @@ func TestGitHandlers_Integration(t *testing.T) { GitCommitMsgTemplate: "Update: {{message}}", } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) err := json.NewDecoder(rr.Body).Decode(workspace) @@ -50,7 +50,7 @@ func TestGitHandlers_Integration(t *testing.T) { "message": commitMsg, } - rr := h.makeRequest(t, http.MethodPost, baseURL+"/commit", requestBody, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPost, baseURL+"/commit", requestBody, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) var response map[string]string @@ -70,7 +70,7 @@ func TestGitHandlers_Integration(t *testing.T) { "message": "", } - rr := h.makeRequest(t, http.MethodPost, baseURL+"/commit", requestBody, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPost, baseURL+"/commit", requestBody, h.RegularSession) assert.Equal(t, http.StatusBadRequest, rr.Code) assert.Equal(t, 0, h.MockGit.GetCommitCount(), "Commit should not be called") }) @@ -83,7 +83,7 @@ func TestGitHandlers_Integration(t *testing.T) { "message": "Test message", } - rr := h.makeRequest(t, http.MethodPost, baseURL+"/commit", requestBody, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPost, baseURL+"/commit", requestBody, h.RegularSession) assert.Equal(t, http.StatusInternalServerError, rr.Code) h.MockGit.SetError(nil) // Reset error state @@ -94,7 +94,7 @@ func TestGitHandlers_Integration(t *testing.T) { h.MockGit.Reset() t.Run("successful pull", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodPost, baseURL+"/pull", nil, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPost, baseURL+"/pull", nil, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) var response map[string]string @@ -109,7 +109,7 @@ func TestGitHandlers_Integration(t *testing.T) { h.MockGit.Reset() h.MockGit.SetError(fmt.Errorf("mock git error")) - rr := h.makeRequest(t, http.MethodPost, baseURL+"/pull", nil, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPost, baseURL+"/pull", nil, h.RegularSession) assert.Equal(t, http.StatusInternalServerError, rr.Code) h.MockGit.SetError(nil) // Reset error state @@ -141,11 +141,11 @@ func TestGitHandlers_Integration(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { // Test without session - rr := h.makeRequest(t, tc.method, tc.path, tc.body, nil, nil) + rr := h.makeRequest(t, tc.method, tc.path, tc.body, nil) assert.Equal(t, http.StatusUnauthorized, rr.Code) // Test with wrong user's session - rr = h.makeRequest(t, tc.method, tc.path, tc.body, h.AdminSession, nil) + rr = h.makeRequest(t, tc.method, tc.path, tc.body, h.AdminSession) assert.Equal(t, http.StatusNotFound, rr.Code) }) } @@ -160,7 +160,7 @@ func TestGitHandlers_Integration(t *testing.T) { Name: "Non-Git Workspace", } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", nonGitWorkspace, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", nonGitWorkspace, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) err := json.NewDecoder(rr.Body).Decode(nonGitWorkspace) @@ -170,11 +170,11 @@ func TestGitHandlers_Integration(t *testing.T) { // Try to commit commitMsg := map[string]string{"message": "test"} - rr = h.makeRequest(t, http.MethodPost, nonGitBaseURL+"/commit", commitMsg, h.RegularSession, nil) + rr = h.makeRequest(t, http.MethodPost, nonGitBaseURL+"/commit", commitMsg, h.RegularSession) assert.Equal(t, http.StatusInternalServerError, rr.Code) // Try to pull - rr = h.makeRequest(t, http.MethodPost, nonGitBaseURL+"/pull", nil, h.RegularSession, nil) + rr = h.makeRequest(t, http.MethodPost, nonGitBaseURL+"/pull", nil, h.RegularSession) assert.Equal(t, http.StatusInternalServerError, rr.Code) }) }) diff --git a/server/internal/handlers/integration_test.go b/server/internal/handlers/integration_test.go index 5377a8f..bdb53fd 100644 --- a/server/internal/handlers/integration_test.go +++ b/server/internal/handlers/integration_test.go @@ -241,16 +241,11 @@ func (h *testHarness) addAuthCookies(t *testing.T, req *http.Request, session *m } // makeRequest is the main helper for making JSON requests -func (h *testHarness) makeRequest(t *testing.T, method, path string, body interface{}, session *models.Session, headers map[string]string) *httptest.ResponseRecorder { +func (h *testHarness) makeRequest(t *testing.T, method, path string, body interface{}, session *models.Session) *httptest.ResponseRecorder { t.Helper() req := h.newRequest(t, method, path, body) - // Add custom headers - for k, v := range headers { - req.Header.Set(k, v) - } - if session != nil { needsCSRF := method != http.MethodGet && method != http.MethodHead && method != http.MethodOptions csrfToken := h.addAuthCookies(t, req, session, needsCSRF) @@ -263,16 +258,11 @@ func (h *testHarness) makeRequest(t *testing.T, method, path string, body interf } // makeRequestRawWithHeaders adds support for custom headers with raw body -func (h *testHarness) makeRequestRaw(t *testing.T, method, path string, body io.Reader, session *models.Session, headers map[string]string) *httptest.ResponseRecorder { +func (h *testHarness) makeRequestRaw(t *testing.T, method, path string, body io.Reader, session *models.Session) *httptest.ResponseRecorder { t.Helper() req := h.newRequestRaw(t, method, path, body) - // Add custom headers - for k, v := range headers { - req.Header.Set(k, v) - } - if session != nil { needsCSRF := method != http.MethodGet && method != http.MethodHead && method != http.MethodOptions csrfToken := h.addAuthCookies(t, req, session, needsCSRF) diff --git a/server/internal/handlers/user_handlers_integration_test.go b/server/internal/handlers/user_handlers_integration_test.go index 378703e..480d64a 100644 --- a/server/internal/handlers/user_handlers_integration_test.go +++ b/server/internal/handlers/user_handlers_integration_test.go @@ -23,7 +23,7 @@ func TestUserHandlers_Integration(t *testing.T) { t.Run("get current user", func(t *testing.T) { t.Run("successful get", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodGet, "/api/v1/auth/me", nil, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodGet, "/api/v1/auth/me", nil, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) var user models.User @@ -38,7 +38,7 @@ func TestUserHandlers_Integration(t *testing.T) { }) t.Run("unauthorized", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodGet, "/api/v1/auth/me", nil, nil, nil) + rr := h.makeRequest(t, http.MethodGet, "/api/v1/auth/me", nil, nil) assert.Equal(t, http.StatusUnauthorized, rr.Code) }) }) @@ -49,7 +49,7 @@ func TestUserHandlers_Integration(t *testing.T) { DisplayName: "Updated Name", } - rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) var user models.User @@ -64,7 +64,7 @@ func TestUserHandlers_Integration(t *testing.T) { CurrentPassword: currentPassword, } - rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) var user models.User @@ -80,7 +80,7 @@ func TestUserHandlers_Integration(t *testing.T) { Email: "anotheremail@test.com", } - rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession) assert.Equal(t, http.StatusBadRequest, rr.Code) }) @@ -90,7 +90,7 @@ func TestUserHandlers_Integration(t *testing.T) { CurrentPassword: "wrongpassword", } - rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession) assert.Equal(t, http.StatusUnauthorized, rr.Code) }) @@ -100,7 +100,7 @@ func TestUserHandlers_Integration(t *testing.T) { NewPassword: "newpassword123", } - rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) // Verify can login with new password @@ -109,7 +109,7 @@ func TestUserHandlers_Integration(t *testing.T) { Password: "newpassword123", } - rr = h.makeRequest(t, http.MethodPost, "/api/v1/auth/login", loginReq, nil, nil) + rr = h.makeRequest(t, http.MethodPost, "/api/v1/auth/login", loginReq, nil) assert.Equal(t, http.StatusOK, rr.Code) currentPassword = updateReq.NewPassword @@ -120,7 +120,7 @@ func TestUserHandlers_Integration(t *testing.T) { NewPassword: "newpass123", } - rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession) assert.Equal(t, http.StatusBadRequest, rr.Code) }) @@ -130,7 +130,7 @@ func TestUserHandlers_Integration(t *testing.T) { NewPassword: "newpass123", } - rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession) assert.Equal(t, http.StatusUnauthorized, rr.Code) }) @@ -140,7 +140,7 @@ func TestUserHandlers_Integration(t *testing.T) { NewPassword: "short", } - rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession) assert.Equal(t, http.StatusBadRequest, rr.Code) }) @@ -150,7 +150,7 @@ func TestUserHandlers_Integration(t *testing.T) { CurrentPassword: currentPassword, } - rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession) assert.Equal(t, http.StatusConflict, rr.Code) }) }) @@ -164,7 +164,7 @@ func TestUserHandlers_Integration(t *testing.T) { Role: models.RoleEditor, } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", createReq, h.AdminSession, nil) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", createReq, h.AdminSession) require.Equal(t, http.StatusOK, rr.Code) var newUser models.User @@ -177,7 +177,7 @@ func TestUserHandlers_Integration(t *testing.T) { Password: createReq.Password, } - rr = h.makeRequest(t, http.MethodPost, "/api/v1/auth/login", loginReq, nil, nil) + rr = h.makeRequest(t, http.MethodPost, "/api/v1/auth/login", loginReq, nil) require.Equal(t, http.StatusOK, rr.Code) var loginResp handlers.LoginResponse @@ -197,11 +197,11 @@ func TestUserHandlers_Integration(t *testing.T) { Password: createReq.Password, } - rr := h.makeRequest(t, http.MethodDelete, "/api/v1/profile", deleteReq, userSession, nil) + rr := h.makeRequest(t, http.MethodDelete, "/api/v1/profile", deleteReq, userSession) require.Equal(t, http.StatusNoContent, rr.Code) // Verify user is deleted - rr = h.makeRequest(t, http.MethodPost, "/api/v1/auth/login", loginReq, nil, nil) + rr = h.makeRequest(t, http.MethodPost, "/api/v1/auth/login", loginReq, nil) assert.Equal(t, http.StatusUnauthorized, rr.Code) }) @@ -210,7 +210,7 @@ func TestUserHandlers_Integration(t *testing.T) { Password: "wrongpassword", } - rr := h.makeRequest(t, http.MethodDelete, "/api/v1/profile", deleteReq, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodDelete, "/api/v1/profile", deleteReq, h.RegularSession) assert.Equal(t, http.StatusUnauthorized, rr.Code) }) @@ -219,7 +219,7 @@ func TestUserHandlers_Integration(t *testing.T) { Password: "admin123", // Admin password from test harness } - rr := h.makeRequest(t, http.MethodDelete, "/api/v1/profile", deleteReq, h.AdminSession, nil) + rr := h.makeRequest(t, http.MethodDelete, "/api/v1/profile", deleteReq, h.AdminSession) assert.Equal(t, http.StatusForbidden, rr.Code) }) }) diff --git a/server/internal/handlers/workspace_handlers_integration_test.go b/server/internal/handlers/workspace_handlers_integration_test.go index e973b83..dbb92d7 100644 --- a/server/internal/handlers/workspace_handlers_integration_test.go +++ b/server/internal/handlers/workspace_handlers_integration_test.go @@ -20,7 +20,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { t.Run("list workspaces", func(t *testing.T) { t.Run("successful list", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodGet, "/api/v1/workspaces", nil, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodGet, "/api/v1/workspaces", nil, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) var workspaces []*models.Workspace @@ -30,7 +30,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { }) t.Run("unauthorized", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodGet, "/api/v1/workspaces", nil, nil, nil) + rr := h.makeRequest(t, http.MethodGet, "/api/v1/workspaces", nil, nil) assert.Equal(t, http.StatusUnauthorized, rr.Code) }) }) @@ -41,7 +41,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { Name: "Test Workspace", } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) var created models.Workspace @@ -64,7 +64,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { GitCommitEmail: "test@example.com", } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) var created models.Workspace @@ -86,7 +86,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { // Missing required Git settings } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession) assert.Equal(t, http.StatusBadRequest, rr.Code) }) }) @@ -95,7 +95,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { workspace := &models.Workspace{ Name: "Test Workspace Operations", } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) err := json.NewDecoder(rr.Body).Decode(workspace) require.NoError(t, err) @@ -105,7 +105,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { t.Run("get workspace", func(t *testing.T) { t.Run("successful get", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodGet, baseURL, nil, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodGet, baseURL, nil, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) var got models.Workspace @@ -116,13 +116,13 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { }) t.Run("nonexistent workspace", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodGet, "/api/v1/workspaces/nonexistent", nil, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodGet, "/api/v1/workspaces/nonexistent", nil, h.RegularSession) assert.Equal(t, http.StatusNotFound, rr.Code) }) t.Run("unauthorized access", func(t *testing.T) { // Try accessing with another user's token - rr := h.makeRequest(t, http.MethodGet, baseURL, nil, h.AdminSession, nil) + rr := h.makeRequest(t, http.MethodGet, baseURL, nil, h.AdminSession) assert.Equal(t, http.StatusNotFound, rr.Code) }) }) @@ -131,7 +131,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { t.Run("update name", func(t *testing.T) { workspace.Name = "Updated Workspace" - rr := h.makeRequest(t, http.MethodPut, baseURL, workspace, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPut, baseURL, workspace, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) var updated models.Workspace @@ -152,7 +152,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { ShowHiddenFiles: true, } - rr := h.makeRequest(t, http.MethodPut, baseURL, update, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPut, baseURL, update, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) var updated models.Workspace @@ -176,7 +176,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { GitCommitEmail: "test@example.com", } - rr := h.makeRequest(t, http.MethodPut, baseURL, update, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPut, baseURL, update, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) var updated models.Workspace @@ -200,14 +200,14 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { // Missing required Git settings } - rr := h.makeRequest(t, http.MethodPut, baseURL, update, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPut, baseURL, update, h.RegularSession) assert.Equal(t, http.StatusBadRequest, rr.Code) }) }) t.Run("last workspace", func(t *testing.T) { t.Run("get last workspace", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodGet, "/api/v1/workspaces/last", nil, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodGet, "/api/v1/workspaces/last", nil, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) var response struct { @@ -225,11 +225,11 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { WorkspaceName: workspace.Name, } - rr := h.makeRequest(t, http.MethodPut, "/api/v1/workspaces/last", req, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPut, "/api/v1/workspaces/last", req, h.RegularSession) require.Equal(t, http.StatusNoContent, rr.Code) // Verify the update - rr = h.makeRequest(t, http.MethodGet, "/api/v1/workspaces/last", nil, h.RegularSession, nil) + rr = h.makeRequest(t, http.MethodGet, "/api/v1/workspaces/last", nil, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) var response struct { @@ -243,7 +243,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { t.Run("delete workspace", func(t *testing.T) { // Get current workspaces to know how many we have - rr := h.makeRequest(t, http.MethodGet, "/api/v1/workspaces", nil, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodGet, "/api/v1/workspaces", nil, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) var existingWorkspaces []*models.Workspace @@ -254,13 +254,13 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { newWorkspace := &models.Workspace{ Name: "Workspace To Delete", } - rr = h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", newWorkspace, h.RegularSession, nil) + rr = h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", newWorkspace, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) err = json.NewDecoder(rr.Body).Decode(newWorkspace) require.NoError(t, err) t.Run("successful delete", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodDelete, "/api/v1/workspaces/"+url.PathEscape(newWorkspace.Name), nil, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodDelete, "/api/v1/workspaces/"+url.PathEscape(newWorkspace.Name), nil, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) var response struct { @@ -271,7 +271,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { assert.NotEmpty(t, response.NextWorkspaceName) // Verify workspace is deleted - rr = h.makeRequest(t, http.MethodGet, "/api/v1/workspaces/"+url.PathEscape(newWorkspace.Name), nil, h.RegularSession, nil) + rr = h.makeRequest(t, http.MethodGet, "/api/v1/workspaces/"+url.PathEscape(newWorkspace.Name), nil, h.RegularSession) assert.Equal(t, http.StatusNotFound, rr.Code) }) @@ -279,13 +279,13 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { // Delete all but one workspace for i := 0; i < len(existingWorkspaces)-1; i++ { ws := existingWorkspaces[i] - rr := h.makeRequest(t, http.MethodDelete, "/api/v1/workspaces/"+url.PathEscape(ws.Name), nil, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodDelete, "/api/v1/workspaces/"+url.PathEscape(ws.Name), nil, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) } // Try to delete the last remaining workspace lastWs := existingWorkspaces[len(existingWorkspaces)-1] - rr := h.makeRequest(t, http.MethodDelete, "/api/v1/workspaces/"+url.PathEscape(lastWs.Name), nil, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodDelete, "/api/v1/workspaces/"+url.PathEscape(lastWs.Name), nil, h.RegularSession) assert.Equal(t, http.StatusBadRequest, rr.Code) }) @@ -294,11 +294,11 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { workspace := &models.Workspace{ Name: "Unauthorized Delete Test", } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession, nil) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession) require.Equal(t, http.StatusOK, rr.Code) // Try to delete with wrong user's token - rr = h.makeRequest(t, http.MethodDelete, "/api/v1/workspaces/"+url.PathEscape(workspace.Name), nil, h.AdminSession, nil) + rr = h.makeRequest(t, http.MethodDelete, "/api/v1/workspaces/"+url.PathEscape(workspace.Name), nil, h.AdminSession) assert.Equal(t, http.StatusNotFound, rr.Code) }) }) From 69af630332fdfd7a857e27506b251953837d5bd8 Mon Sep 17 00:00:00 2001 From: LordMathis Date: Sun, 8 Dec 2024 15:03:39 +0100 Subject: [PATCH 06/12] Update tests to use test user struct --- .../admin_handlers_integration_test.go | 60 +++++------ .../auth_handlers_integration_test.go | 52 ++++++--- .../file_handlers_integration_test.go | 42 ++++---- .../handlers/git_handlers_integration_test.go | 24 ++--- server/internal/handlers/integration_test.go | 101 +++++++++--------- .../user_handlers_integration_test.go | 82 +++++--------- .../workspace_handlers_integration_test.go | 48 ++++----- 7 files changed, 200 insertions(+), 209 deletions(-) diff --git a/server/internal/handlers/admin_handlers_integration_test.go b/server/internal/handlers/admin_handlers_integration_test.go index d633f92..2fc5bae 100644 --- a/server/internal/handlers/admin_handlers_integration_test.go +++ b/server/internal/handlers/admin_handlers_integration_test.go @@ -35,7 +35,7 @@ func TestAdminHandlers_Integration(t *testing.T) { t.Run("user management", func(t *testing.T) { t.Run("list users", func(t *testing.T) { // Test with admin session - rr := h.makeRequest(t, http.MethodGet, "/api/v1/admin/users", nil, h.AdminSession) + rr := h.makeRequest(t, http.MethodGet, "/api/v1/admin/users", nil, h.AdminTestUser) require.Equal(t, http.StatusOK, rr.Code) var users []*models.User @@ -44,11 +44,11 @@ func TestAdminHandlers_Integration(t *testing.T) { // Should have at least our admin and regular test users assert.GreaterOrEqual(t, len(users), 2) - assert.True(t, containsUser(users, h.AdminUser), "Admin user not found in users list") - assert.True(t, containsUser(users, h.RegularUser), "Regular user not found in users list") + assert.True(t, containsUser(users, h.AdminTestUser.userModel), "Admin user not found in users list") + assert.True(t, containsUser(users, h.RegularTestUser.userModel), "Regular user not found in users list") // Test with non-admin session - rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/users", nil, h.RegularSession) + rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/users", nil, h.RegularTestUser) assert.Equal(t, http.StatusForbidden, rr.Code) // Test without session @@ -65,7 +65,7 @@ func TestAdminHandlers_Integration(t *testing.T) { } // Test with admin session - rr := h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", createReq, h.AdminSession) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", createReq, h.AdminTestUser) require.Equal(t, http.StatusOK, rr.Code) var createdUser models.User @@ -77,7 +77,7 @@ func TestAdminHandlers_Integration(t *testing.T) { assert.NotZero(t, createdUser.LastWorkspaceID) // Test duplicate email - rr = h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", createReq, h.AdminSession) + rr = h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", createReq, h.AdminTestUser) assert.Equal(t, http.StatusConflict, rr.Code) // Test invalid request (missing required fields) @@ -85,44 +85,44 @@ func TestAdminHandlers_Integration(t *testing.T) { Email: "invalid@test.com", // Missing password and role } - rr = h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", invalidReq, h.AdminSession) + rr = h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", invalidReq, h.AdminTestUser) assert.Equal(t, http.StatusBadRequest, rr.Code) // Test with non-admin session - rr = h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", createReq, h.RegularSession) + rr = h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", createReq, h.RegularTestUser) assert.Equal(t, http.StatusForbidden, rr.Code) }) t.Run("get user", func(t *testing.T) { - path := fmt.Sprintf("/api/v1/admin/users/%d", h.RegularUser.ID) + path := fmt.Sprintf("/api/v1/admin/users/%d", h.RegularTestUser.session.UserID) // Test with admin session - rr := h.makeRequest(t, http.MethodGet, path, nil, h.AdminSession) + rr := h.makeRequest(t, http.MethodGet, path, nil, h.AdminTestUser) require.Equal(t, http.StatusOK, rr.Code) var user models.User err := json.NewDecoder(rr.Body).Decode(&user) require.NoError(t, err) - assert.Equal(t, h.RegularUser.ID, user.ID) + assert.Equal(t, h.RegularTestUser.session.UserID, user.ID) // Test non-existent user - rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/users/999999", nil, h.AdminSession) + rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/users/999999", nil, h.AdminTestUser) assert.Equal(t, http.StatusNotFound, rr.Code) // Test with non-admin session - rr = h.makeRequest(t, http.MethodGet, path, nil, h.RegularSession) + rr = h.makeRequest(t, http.MethodGet, path, nil, h.RegularTestUser) assert.Equal(t, http.StatusForbidden, rr.Code) }) t.Run("update user", func(t *testing.T) { - path := fmt.Sprintf("/api/v1/admin/users/%d", h.RegularUser.ID) + path := fmt.Sprintf("/api/v1/admin/users/%d", h.RegularTestUser.session.UserID) updateReq := handlers.UpdateUserRequest{ DisplayName: "Updated Name", Role: models.RoleViewer, } // Test with admin session - rr := h.makeRequest(t, http.MethodPut, path, updateReq, h.AdminSession) + rr := h.makeRequest(t, http.MethodPut, path, updateReq, h.AdminTestUser) require.Equal(t, http.StatusOK, rr.Code) var updatedUser models.User @@ -132,7 +132,7 @@ func TestAdminHandlers_Integration(t *testing.T) { assert.Equal(t, updateReq.Role, updatedUser.Role) // Test with non-admin session - rr = h.makeRequest(t, http.MethodPut, path, updateReq, h.RegularSession) + rr = h.makeRequest(t, http.MethodPut, path, updateReq, h.RegularTestUser) assert.Equal(t, http.StatusForbidden, rr.Code) }) @@ -145,7 +145,7 @@ func TestAdminHandlers_Integration(t *testing.T) { Role: models.RoleEditor, } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", createReq, h.AdminSession) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", createReq, h.AdminTestUser) require.Equal(t, http.StatusOK, rr.Code) var createdUser models.User @@ -155,20 +155,20 @@ func TestAdminHandlers_Integration(t *testing.T) { path := fmt.Sprintf("/api/v1/admin/users/%d", createdUser.ID) // Test deleting own account (should fail) - adminPath := fmt.Sprintf("/api/v1/admin/users/%d", h.AdminUser.ID) - rr = h.makeRequest(t, http.MethodDelete, adminPath, nil, h.AdminSession) + adminPath := fmt.Sprintf("/api/v1/admin/users/%d", h.AdminTestUser.session.UserID) + rr = h.makeRequest(t, http.MethodDelete, adminPath, nil, h.AdminTestUser) assert.Equal(t, http.StatusBadRequest, rr.Code) // Test with admin session - rr = h.makeRequest(t, http.MethodDelete, path, nil, h.AdminSession) + rr = h.makeRequest(t, http.MethodDelete, path, nil, h.AdminTestUser) assert.Equal(t, http.StatusNoContent, rr.Code) // Verify user is deleted - rr = h.makeRequest(t, http.MethodGet, path, nil, h.AdminSession) + rr = h.makeRequest(t, http.MethodGet, path, nil, h.AdminTestUser) assert.Equal(t, http.StatusNotFound, rr.Code) // Test with non-admin session - rr = h.makeRequest(t, http.MethodDelete, path, nil, h.RegularSession) + rr = h.makeRequest(t, http.MethodDelete, path, nil, h.RegularTestUser) assert.Equal(t, http.StatusForbidden, rr.Code) }) }) @@ -177,15 +177,15 @@ func TestAdminHandlers_Integration(t *testing.T) { t.Run("list workspaces", func(t *testing.T) { // Create a test workspace first workspace := &models.Workspace{ - UserID: h.RegularUser.ID, + UserID: h.RegularTestUser.session.UserID, Name: "Test Workspace", } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) // Test with admin session - rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/workspaces", nil, h.AdminSession) + rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/workspaces", nil, h.AdminTestUser) require.Equal(t, http.StatusOK, rr.Code) var workspaces []*handlers.WorkspaceStats @@ -207,7 +207,7 @@ func TestAdminHandlers_Integration(t *testing.T) { } // Test with non-admin session - rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/workspaces", nil, h.RegularSession) + rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/workspaces", nil, h.RegularTestUser) assert.Equal(t, http.StatusForbidden, rr.Code) }) }) @@ -215,14 +215,14 @@ func TestAdminHandlers_Integration(t *testing.T) { t.Run("system stats", func(t *testing.T) { // Create some test data workspace := &models.Workspace{ - UserID: h.RegularUser.ID, + UserID: h.RegularTestUser.session.UserID, Name: "Stats Test Workspace", } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) // Test with admin session - rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/stats", nil, h.AdminSession) + rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/stats", nil, h.AdminTestUser) require.Equal(t, http.StatusOK, rr.Code) var stats handlers.SystemStats @@ -237,7 +237,7 @@ func TestAdminHandlers_Integration(t *testing.T) { assert.GreaterOrEqual(t, stats.TotalSize, int64(0)) // Test with non-admin session - rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/stats", nil, h.RegularSession) + rr = h.makeRequest(t, http.MethodGet, "/api/v1/admin/stats", nil, h.RegularTestUser) assert.Equal(t, http.StatusForbidden, rr.Code) }) } diff --git a/server/internal/handlers/auth_handlers_integration_test.go b/server/internal/handlers/auth_handlers_integration_test.go index 4214aef..51703f0 100644 --- a/server/internal/handlers/auth_handlers_integration_test.go +++ b/server/internal/handlers/auth_handlers_integration_test.go @@ -40,17 +40,17 @@ func TestAuthHandlers_Integration(t *testing.T) { case "access_token": foundAccessToken = true assert.True(t, cookie.HttpOnly, "access_token cookie must be HttpOnly") - assert.Equal(t, http.SameSiteStrictMode, cookie.SameSite) + assert.Equal(t, http.SameSiteLaxMode, cookie.SameSite) assert.Equal(t, 900, cookie.MaxAge) // 15 minutes case "refresh_token": foundRefreshToken = true assert.True(t, cookie.HttpOnly, "refresh_token cookie must be HttpOnly") - assert.Equal(t, http.SameSiteStrictMode, cookie.SameSite) + assert.Equal(t, http.SameSiteLaxMode, cookie.SameSite) assert.Equal(t, 604800, cookie.MaxAge) // 7 days case "csrf_token": foundCSRF = true assert.False(t, cookie.HttpOnly, "csrf_token cookie must not be HttpOnly") - assert.Equal(t, http.SameSiteStrictMode, cookie.SameSite) + assert.Equal(t, http.SameSiteLaxMode, cookie.SameSite) assert.Equal(t, 900, cookie.MaxAge) // 15 minutes } } @@ -148,7 +148,8 @@ func TestAuthHandlers_Integration(t *testing.T) { t.Run("successful token refresh", func(t *testing.T) { // Need lower level helpers for precise cookie control req := h.newRequest(t, http.MethodPost, "/api/v1/auth/refresh", nil) - h.addAuthCookies(t, req, h.RegularSession, true) // Adds both tokens + h.addAuthCookies(t, req, h.RegularTestUser) // Adds both tokens + h.addCSRFCookie(t, req) rr := h.executeRequest(req) require.Equal(t, http.StatusOK, rr.Code) @@ -181,8 +182,7 @@ func TestAuthHandlers_Integration(t *testing.T) { name: "missing refresh token cookie", setup: func(req *http.Request) { // Only add access token - token, _ := h.JWTManager.GenerateAccessToken(h.RegularSession.UserID, "admin") - req.AddCookie(h.CookieManager.GenerateAccessTokenCookie(token)) + req.AddCookie(h.CookieManager.GenerateAccessTokenCookie(h.RegularTestUser.accessToken)) }, wantCode: http.StatusBadRequest, }, @@ -191,11 +191,16 @@ func TestAuthHandlers_Integration(t *testing.T) { setup: func(req *http.Request) { expiredSession := &models.Session{ ID: "expired", - UserID: h.RegularUser.ID, + UserID: h.RegularTestUser.session.UserID, RefreshToken: "expired-token", ExpiresAt: time.Now().Add(-1 * time.Hour), } - h.addAuthCookies(t, req, expiredSession, true) + expiredSessionUser := &testUser{ + userModel: h.RegularTestUser.userModel, + accessToken: h.RegularTestUser.accessToken, + session: expiredSession, + } + h.addAuthCookies(t, req, expiredSessionUser) }, wantCode: http.StatusUnauthorized, }, @@ -226,7 +231,8 @@ func TestAuthHandlers_Integration(t *testing.T) { t.Run("successful logout", func(t *testing.T) { // Need CSRF token for POST request req := h.newRequest(t, http.MethodPost, "/api/v1/auth/logout", nil) - csrfToken := h.addAuthCookies(t, req, h.RegularSession, true) + h.addAuthCookies(t, req, h.RegularTestUser) + csrfToken := h.addCSRFCookie(t, req) req.Header.Set("X-CSRF-Token", csrfToken) rr := h.executeRequest(req) require.Equal(t, http.StatusNoContent, rr.Code) @@ -238,7 +244,7 @@ func TestAuthHandlers_Integration(t *testing.T) { } // Verify session is actually invalidated - rr = h.makeRequest(t, http.MethodGet, "/api/v1/auth/me", nil, h.RegularSession) + rr = h.makeRequest(t, http.MethodGet, "/api/v1/auth/me", nil, h.RegularTestUser) assert.Equal(t, http.StatusUnauthorized, rr.Code) }) @@ -251,7 +257,8 @@ func TestAuthHandlers_Integration(t *testing.T) { { name: "missing CSRF token", setup: func(req *http.Request) { - h.addAuthCookies(t, req, h.RegularSession, true) + h.addAuthCookies(t, req, h.RegularTestUser) + h.addCSRFCookie(t, req) // Deliberately not setting X-CSRF-Token header }, wantCode: http.StatusForbidden, @@ -259,7 +266,8 @@ func TestAuthHandlers_Integration(t *testing.T) { { name: "mismatched CSRF token", setup: func(req *http.Request) { - h.addAuthCookies(t, req, h.RegularSession, true) + h.addAuthCookies(t, req, h.RegularTestUser) + h.addCSRFCookie(t, req) req.Header.Set("X-CSRF-Token", "wrong-token") }, wantCode: http.StatusForbidden, @@ -286,13 +294,13 @@ func TestAuthHandlers_Integration(t *testing.T) { t.Run("get current user", func(t *testing.T) { t.Run("successful get current user", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodGet, "/api/v1/auth/me", nil, h.RegularSession) + rr := h.makeRequest(t, http.MethodGet, "/api/v1/auth/me", nil, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) var user models.User err := json.NewDecoder(rr.Body).Decode(&user) require.NoError(t, err) - assert.Equal(t, h.RegularUser.Email, user.Email) + assert.Equal(t, h.RegularTestUser.userModel.Email, user.Email) }) t.Run("auth edge cases", func(t *testing.T) { @@ -317,7 +325,12 @@ func TestAuthHandlers_Integration(t *testing.T) { RefreshToken: "invalid", ExpiresAt: time.Now().Add(time.Hour), } - h.addAuthCookies(t, req, invalidSession, false) + invalidSessionUser := &testUser{ + userModel: h.RegularTestUser.userModel, + accessToken: h.RegularTestUser.accessToken, + session: invalidSession, + } + h.addAuthCookies(t, req, invalidSessionUser) }, wantCode: http.StatusUnauthorized, }, @@ -326,11 +339,16 @@ func TestAuthHandlers_Integration(t *testing.T) { setup: func(req *http.Request) { expiredSession := &models.Session{ ID: "expired", - UserID: h.RegularUser.ID, + UserID: h.RegularTestUser.session.UserID, RefreshToken: "expired-token", ExpiresAt: time.Now().Add(-1 * time.Hour), } - h.addAuthCookies(t, req, expiredSession, false) + expiredSessionUser := &testUser{ + userModel: h.RegularTestUser.userModel, + accessToken: h.RegularTestUser.accessToken, + session: expiredSession, + } + h.addAuthCookies(t, req, expiredSessionUser) }, wantCode: http.StatusUnauthorized, }, diff --git a/server/internal/handlers/file_handlers_integration_test.go b/server/internal/handlers/file_handlers_integration_test.go index 0a72fab..3888786 100644 --- a/server/internal/handlers/file_handlers_integration_test.go +++ b/server/internal/handlers/file_handlers_integration_test.go @@ -24,10 +24,10 @@ func TestFileHandlers_Integration(t *testing.T) { t.Run("file operations", func(t *testing.T) { // Setup: Create a workspace first workspace := &models.Workspace{ - UserID: h.RegularUser.ID, + UserID: h.RegularTestUser.session.UserID, Name: "File Test Workspace", } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) err := json.NewDecoder(rr.Body).Decode(workspace) @@ -37,7 +37,7 @@ func TestFileHandlers_Integration(t *testing.T) { baseURL := fmt.Sprintf("/api/v1/workspaces/%s/files", url.PathEscape(workspace.Name)) t.Run("list empty directory", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodGet, baseURL, nil, h.RegularSession) + rr := h.makeRequest(t, http.MethodGet, baseURL, nil, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) var files []storage.FileNode @@ -51,16 +51,16 @@ func TestFileHandlers_Integration(t *testing.T) { filePath := "test.md" // Save file - rr := h.makeRequestRaw(t, http.MethodPost, baseURL+"/"+filePath, strings.NewReader(content), h.RegularSession) + rr := h.makeRequestRaw(t, http.MethodPost, baseURL+"/"+filePath, strings.NewReader(content), h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) // Get file content - rr = h.makeRequest(t, http.MethodGet, baseURL+"/"+filePath, nil, h.RegularSession) + rr = h.makeRequest(t, http.MethodGet, baseURL+"/"+filePath, nil, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) assert.Equal(t, content, rr.Body.String()) // List directory should now show the file - rr = h.makeRequest(t, http.MethodGet, baseURL, nil, h.RegularSession) + rr = h.makeRequest(t, http.MethodGet, baseURL, nil, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) var files []storage.FileNode @@ -80,12 +80,12 @@ func TestFileHandlers_Integration(t *testing.T) { // Create all files for path, content := range files { - rr := h.makeRequest(t, http.MethodPost, baseURL+"/"+path, content, h.RegularSession) + rr := h.makeRequest(t, http.MethodPost, baseURL+"/"+path, content, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) } // List all files - rr := h.makeRequest(t, http.MethodGet, baseURL, nil, h.RegularSession) + rr := h.makeRequest(t, http.MethodGet, baseURL, nil, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) var fileNodes []storage.FileNode @@ -116,11 +116,11 @@ func TestFileHandlers_Integration(t *testing.T) { // Look up a file that exists in multiple locations filename := "readme.md" dupContent := "Another readme" - rr := h.makeRequest(t, http.MethodPost, baseURL+"/projects/"+filename, dupContent, h.RegularSession) + rr := h.makeRequest(t, http.MethodPost, baseURL+"/projects/"+filename, dupContent, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) // Search for the file - rr = h.makeRequest(t, http.MethodGet, baseURL+"/lookup?filename="+filename, nil, h.RegularSession) + rr = h.makeRequest(t, http.MethodGet, baseURL+"/lookup?filename="+filename, nil, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) var response struct { @@ -131,7 +131,7 @@ func TestFileHandlers_Integration(t *testing.T) { assert.Len(t, response.Paths, 2) // Search for non-existent file - rr = h.makeRequest(t, http.MethodGet, baseURL+"/lookup?filename=nonexistent.md", nil, h.RegularSession) + rr = h.makeRequest(t, http.MethodGet, baseURL+"/lookup?filename=nonexistent.md", nil, h.RegularTestUser) assert.Equal(t, http.StatusNotFound, rr.Code) }) @@ -140,21 +140,21 @@ func TestFileHandlers_Integration(t *testing.T) { content := "This file will be deleted" // Create file - rr := h.makeRequest(t, http.MethodPost, baseURL+"/"+filePath, content, h.RegularSession) + rr := h.makeRequest(t, http.MethodPost, baseURL+"/"+filePath, content, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) // Delete file - rr = h.makeRequest(t, http.MethodDelete, baseURL+"/"+filePath, nil, h.RegularSession) + rr = h.makeRequest(t, http.MethodDelete, baseURL+"/"+filePath, nil, h.RegularTestUser) require.Equal(t, http.StatusNoContent, rr.Code) // Verify file is gone - rr = h.makeRequest(t, http.MethodGet, baseURL+"/"+filePath, nil, h.RegularSession) + rr = h.makeRequest(t, http.MethodGet, baseURL+"/"+filePath, nil, h.RegularTestUser) assert.Equal(t, http.StatusNotFound, rr.Code) }) t.Run("last opened file", func(t *testing.T) { // Initially should be empty - rr := h.makeRequest(t, http.MethodGet, baseURL+"/last", nil, h.RegularSession) + rr := h.makeRequest(t, http.MethodGet, baseURL+"/last", nil, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) var response struct { @@ -170,11 +170,11 @@ func TestFileHandlers_Integration(t *testing.T) { }{ FilePath: "docs/readme.md", } - rr = h.makeRequest(t, http.MethodPut, baseURL+"/last", updateReq, h.RegularSession) + rr = h.makeRequest(t, http.MethodPut, baseURL+"/last", updateReq, h.RegularTestUser) require.Equal(t, http.StatusNoContent, rr.Code) // Verify update - rr = h.makeRequest(t, http.MethodGet, baseURL+"/last", nil, h.RegularSession) + rr = h.makeRequest(t, http.MethodGet, baseURL+"/last", nil, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) err = json.NewDecoder(rr.Body).Decode(&response) @@ -183,7 +183,7 @@ func TestFileHandlers_Integration(t *testing.T) { // Test invalid file path updateReq.FilePath = "nonexistent.md" - rr = h.makeRequest(t, http.MethodPut, baseURL+"/last", updateReq, h.RegularSession) + rr = h.makeRequest(t, http.MethodPut, baseURL+"/last", updateReq, h.RegularTestUser) assert.Equal(t, http.StatusNotFound, rr.Code) }) @@ -209,7 +209,7 @@ func TestFileHandlers_Integration(t *testing.T) { assert.Equal(t, http.StatusUnauthorized, rr.Code) // Test with wrong user's session - rr = h.makeRequest(t, tc.method, tc.path, tc.body, h.AdminSession) + rr = h.makeRequest(t, tc.method, tc.path, tc.body, h.AdminTestUser) assert.Equal(t, http.StatusNotFound, rr.Code) }) } @@ -226,11 +226,11 @@ func TestFileHandlers_Integration(t *testing.T) { for _, path := range maliciousPaths { t.Run(path, func(t *testing.T) { // Try to read - rr := h.makeRequest(t, http.MethodGet, baseURL+"/"+path, nil, h.RegularSession) + rr := h.makeRequest(t, http.MethodGet, baseURL+"/"+path, nil, h.RegularTestUser) assert.Equal(t, http.StatusBadRequest, rr.Code) // Try to write - rr = h.makeRequest(t, http.MethodPost, baseURL+"/"+path, "malicious content", h.RegularSession) + rr = h.makeRequest(t, http.MethodPost, baseURL+"/"+path, "malicious content", h.RegularTestUser) assert.Equal(t, http.StatusBadRequest, rr.Code) }) } diff --git a/server/internal/handlers/git_handlers_integration_test.go b/server/internal/handlers/git_handlers_integration_test.go index c9ddba2..ee6b9ca 100644 --- a/server/internal/handlers/git_handlers_integration_test.go +++ b/server/internal/handlers/git_handlers_integration_test.go @@ -22,7 +22,7 @@ func TestGitHandlers_Integration(t *testing.T) { t.Run("git operations", func(t *testing.T) { // Setup: Create a workspace with Git enabled workspace := &models.Workspace{ - UserID: h.RegularUser.ID, + UserID: h.RegularTestUser.session.UserID, Name: "Git Test Workspace", GitEnabled: true, GitURL: "https://github.com/test/repo.git", @@ -32,7 +32,7 @@ func TestGitHandlers_Integration(t *testing.T) { GitCommitMsgTemplate: "Update: {{message}}", } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) err := json.NewDecoder(rr.Body).Decode(workspace) @@ -50,7 +50,7 @@ func TestGitHandlers_Integration(t *testing.T) { "message": commitMsg, } - rr := h.makeRequest(t, http.MethodPost, baseURL+"/commit", requestBody, h.RegularSession) + rr := h.makeRequest(t, http.MethodPost, baseURL+"/commit", requestBody, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) var response map[string]string @@ -70,7 +70,7 @@ func TestGitHandlers_Integration(t *testing.T) { "message": "", } - rr := h.makeRequest(t, http.MethodPost, baseURL+"/commit", requestBody, h.RegularSession) + rr := h.makeRequest(t, http.MethodPost, baseURL+"/commit", requestBody, h.RegularTestUser) assert.Equal(t, http.StatusBadRequest, rr.Code) assert.Equal(t, 0, h.MockGit.GetCommitCount(), "Commit should not be called") }) @@ -83,7 +83,7 @@ func TestGitHandlers_Integration(t *testing.T) { "message": "Test message", } - rr := h.makeRequest(t, http.MethodPost, baseURL+"/commit", requestBody, h.RegularSession) + rr := h.makeRequest(t, http.MethodPost, baseURL+"/commit", requestBody, h.RegularTestUser) assert.Equal(t, http.StatusInternalServerError, rr.Code) h.MockGit.SetError(nil) // Reset error state @@ -94,7 +94,7 @@ func TestGitHandlers_Integration(t *testing.T) { h.MockGit.Reset() t.Run("successful pull", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodPost, baseURL+"/pull", nil, h.RegularSession) + rr := h.makeRequest(t, http.MethodPost, baseURL+"/pull", nil, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) var response map[string]string @@ -109,7 +109,7 @@ func TestGitHandlers_Integration(t *testing.T) { h.MockGit.Reset() h.MockGit.SetError(fmt.Errorf("mock git error")) - rr := h.makeRequest(t, http.MethodPost, baseURL+"/pull", nil, h.RegularSession) + rr := h.makeRequest(t, http.MethodPost, baseURL+"/pull", nil, h.RegularTestUser) assert.Equal(t, http.StatusInternalServerError, rr.Code) h.MockGit.SetError(nil) // Reset error state @@ -145,7 +145,7 @@ func TestGitHandlers_Integration(t *testing.T) { assert.Equal(t, http.StatusUnauthorized, rr.Code) // Test with wrong user's session - rr = h.makeRequest(t, tc.method, tc.path, tc.body, h.AdminSession) + rr = h.makeRequest(t, tc.method, tc.path, tc.body, h.AdminTestUser) assert.Equal(t, http.StatusNotFound, rr.Code) }) } @@ -156,11 +156,11 @@ func TestGitHandlers_Integration(t *testing.T) { // Create a workspace without Git enabled nonGitWorkspace := &models.Workspace{ - UserID: h.RegularUser.ID, + UserID: h.RegularTestUser.session.UserID, Name: "Non-Git Workspace", } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", nonGitWorkspace, h.RegularSession) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", nonGitWorkspace, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) err := json.NewDecoder(rr.Body).Decode(nonGitWorkspace) @@ -170,11 +170,11 @@ func TestGitHandlers_Integration(t *testing.T) { // Try to commit commitMsg := map[string]string{"message": "test"} - rr = h.makeRequest(t, http.MethodPost, nonGitBaseURL+"/commit", commitMsg, h.RegularSession) + rr = h.makeRequest(t, http.MethodPost, nonGitBaseURL+"/commit", commitMsg, h.RegularTestUser) assert.Equal(t, http.StatusInternalServerError, rr.Code) // Try to pull - rr = h.makeRequest(t, http.MethodPost, nonGitBaseURL+"/pull", nil, h.RegularSession) + rr = h.makeRequest(t, http.MethodPost, nonGitBaseURL+"/pull", nil, h.RegularTestUser) assert.Equal(t, http.StatusInternalServerError, rr.Code) }) }) diff --git a/server/internal/handlers/integration_test.go b/server/internal/handlers/integration_test.go index bdb53fd..5e91a9d 100644 --- a/server/internal/handlers/integration_test.go +++ b/server/internal/handlers/integration_test.go @@ -25,18 +25,22 @@ import ( // testHarness encapsulates all the dependencies needed for testing type testHarness struct { - Server *app.Server - DB db.TestDatabase - Storage storage.Manager - JWTManager auth.JWTManager - SessionManager auth.SessionManager - CookieManager auth.CookieManager - AdminUser *models.User - AdminSession *models.Session - RegularUser *models.User - RegularSession *models.Session - TempDirectory string - MockGit *MockGitClient + Server *app.Server + DB db.TestDatabase + Storage storage.Manager + JWTManager auth.JWTManager + SessionManager auth.SessionManager + CookieManager auth.CookieManager + AdminTestUser *testUser + RegularTestUser *testUser + TempDirectory string + MockGit *MockGitClient +} + +type testUser struct { + userModel *models.User + accessToken string + session *models.Session } // setupTestHarness creates a new test environment @@ -110,6 +114,7 @@ func setupTestHarness(t *testing.T) *testHarness { Storage: storageSvc, JWTManager: jwtSvc, SessionManager: sessionSvc, + CookieService: cookieSvc, } // Create server @@ -127,13 +132,11 @@ func setupTestHarness(t *testing.T) *testHarness { } // Create test users - adminUser, adminSession := h.createTestUser(t, "admin@test.com", "admin123", models.RoleAdmin) - regularUser, regularSession := h.createTestUser(t, "user@test.com", "user123", models.RoleEditor) + adminTestUser := h.createTestUser(t, "admin@test.com", "admin123", models.RoleAdmin) + regularTestUser := h.createTestUser(t, "user@test.com", "user123", models.RoleEditor) - h.AdminUser = adminUser - h.AdminSession = adminSession - h.RegularUser = regularUser - h.RegularSession = regularSession + h.AdminTestUser = adminTestUser + h.RegularTestUser = regularTestUser return h } @@ -152,7 +155,7 @@ func (h *testHarness) teardown(t *testing.T) { } // createTestUser creates a test user and returns the user and access token -func (h *testHarness) createTestUser(t *testing.T, email, password string, role models.UserRole) (*models.User, *models.Session) { +func (h *testHarness) createTestUser(t *testing.T, email, password string, role models.UserRole) *testUser { t.Helper() hashedPassword, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost) @@ -178,12 +181,16 @@ func (h *testHarness) createTestUser(t *testing.T, email, password string, role t.Fatalf("Failed to initialize user workspace: %v", err) } - session, _, err := h.SessionManager.CreateSession(user.ID, string(user.Role)) + session, accessToken, err := h.SessionManager.CreateSession(user.ID, string(user.Role)) if err != nil { t.Fatalf("Failed to create session: %v", err) } - return user, session + return &testUser{ + userModel: user, + accessToken: accessToken, + session: session, + } } func (h *testHarness) newRequest(t *testing.T, method, path string, body interface{}) *http.Request { @@ -217,58 +224,52 @@ func (h *testHarness) executeRequest(req *http.Request) *httptest.ResponseRecord } // addAuthCookies adds authentication cookies to request -func (h *testHarness) addAuthCookies(t *testing.T, req *http.Request, session *models.Session, addCSRF bool) string { +func (h *testHarness) addAuthCookies(t *testing.T, req *http.Request, testUser *testUser) { t.Helper() - if session == nil { - return "" + if testUser == nil || testUser.session == nil { + return } - accessToken, err := h.JWTManager.GenerateAccessToken(session.UserID, "admin") - if err != nil { - t.Fatalf("Failed to generate access token: %v", err) - } + req.AddCookie(h.CookieManager.GenerateAccessTokenCookie(testUser.accessToken)) + req.AddCookie(h.CookieManager.GenerateRefreshTokenCookie(testUser.session.RefreshToken)) +} - req.AddCookie(h.CookieManager.GenerateAccessTokenCookie(accessToken)) - req.AddCookie(h.CookieManager.GenerateRefreshTokenCookie(session.RefreshToken)) +func (h *testHarness) addCSRFCookie(t *testing.T, req *http.Request) string { + t.Helper() - if addCSRF { - csrfToken := "test-csrf-token" - req.AddCookie(h.CookieManager.GenerateCSRFCookie(csrfToken)) - return csrfToken - } - return "" + csrfToken := "test-csrf-token" + req.AddCookie(h.CookieManager.GenerateCSRFCookie(csrfToken)) + return csrfToken } // makeRequest is the main helper for making JSON requests -func (h *testHarness) makeRequest(t *testing.T, method, path string, body interface{}, session *models.Session) *httptest.ResponseRecorder { +func (h *testHarness) makeRequest(t *testing.T, method, path string, body interface{}, testUser *testUser) *httptest.ResponseRecorder { t.Helper() req := h.newRequest(t, method, path, body) + h.addAuthCookies(t, req, testUser) - if session != nil { - needsCSRF := method != http.MethodGet && method != http.MethodHead && method != http.MethodOptions - csrfToken := h.addAuthCookies(t, req, session, needsCSRF) - if needsCSRF { - req.Header.Set("X-CSRF-Token", csrfToken) - } + needsCSRF := method != http.MethodGet && method != http.MethodHead && method != http.MethodOptions + if needsCSRF { + csrfToken := h.addCSRFCookie(t, req) + req.Header.Set("X-CSRF-Token", csrfToken) } return h.executeRequest(req) } // makeRequestRawWithHeaders adds support for custom headers with raw body -func (h *testHarness) makeRequestRaw(t *testing.T, method, path string, body io.Reader, session *models.Session) *httptest.ResponseRecorder { +func (h *testHarness) makeRequestRaw(t *testing.T, method, path string, body io.Reader, testUser *testUser) *httptest.ResponseRecorder { t.Helper() req := h.newRequestRaw(t, method, path, body) + h.addAuthCookies(t, req, testUser) - if session != nil { - needsCSRF := method != http.MethodGet && method != http.MethodHead && method != http.MethodOptions - csrfToken := h.addAuthCookies(t, req, session, needsCSRF) - if needsCSRF { - req.Header.Set("X-CSRF-Token", csrfToken) - } + needsCSRF := method != http.MethodGet && method != http.MethodHead && method != http.MethodOptions + if needsCSRF { + csrfToken := h.addCSRFCookie(t, req) + req.Header.Set("X-CSRF-Token", csrfToken) } return h.executeRequest(req) diff --git a/server/internal/handlers/user_handlers_integration_test.go b/server/internal/handlers/user_handlers_integration_test.go index 480d64a..5246228 100644 --- a/server/internal/handlers/user_handlers_integration_test.go +++ b/server/internal/handlers/user_handlers_integration_test.go @@ -18,22 +18,22 @@ func TestUserHandlers_Integration(t *testing.T) { h := setupTestHarness(t) defer h.teardown(t) - currentEmail := h.RegularUser.Email + currentEmail := h.RegularTestUser.userModel.Email currentPassword := "user123" t.Run("get current user", func(t *testing.T) { t.Run("successful get", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodGet, "/api/v1/auth/me", nil, h.RegularSession) + rr := h.makeRequest(t, http.MethodGet, "/api/v1/auth/me", nil, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) var user models.User err := json.NewDecoder(rr.Body).Decode(&user) require.NoError(t, err) - assert.Equal(t, h.RegularUser.ID, user.ID) - assert.Equal(t, h.RegularUser.Email, user.Email) - assert.Equal(t, h.RegularUser.DisplayName, user.DisplayName) - assert.Equal(t, h.RegularUser.Role, user.Role) + assert.Equal(t, h.RegularTestUser.userModel.ID, user.ID) + assert.Equal(t, h.RegularTestUser.userModel.Email, user.Email) + assert.Equal(t, h.RegularTestUser.userModel.DisplayName, user.DisplayName) + assert.Equal(t, h.RegularTestUser.userModel.Role, user.Role) assert.Empty(t, user.PasswordHash, "Password hash should not be included in response") }) @@ -49,7 +49,7 @@ func TestUserHandlers_Integration(t *testing.T) { DisplayName: "Updated Name", } - rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession) + rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) var user models.User @@ -64,7 +64,7 @@ func TestUserHandlers_Integration(t *testing.T) { CurrentPassword: currentPassword, } - rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession) + rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) var user models.User @@ -80,7 +80,7 @@ func TestUserHandlers_Integration(t *testing.T) { Email: "anotheremail@test.com", } - rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession) + rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularTestUser) assert.Equal(t, http.StatusBadRequest, rr.Code) }) @@ -90,7 +90,7 @@ func TestUserHandlers_Integration(t *testing.T) { CurrentPassword: "wrongpassword", } - rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession) + rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularTestUser) assert.Equal(t, http.StatusUnauthorized, rr.Code) }) @@ -100,7 +100,7 @@ func TestUserHandlers_Integration(t *testing.T) { NewPassword: "newpassword123", } - rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession) + rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) // Verify can login with new password @@ -120,7 +120,7 @@ func TestUserHandlers_Integration(t *testing.T) { NewPassword: "newpass123", } - rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession) + rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularTestUser) assert.Equal(t, http.StatusBadRequest, rr.Code) }) @@ -130,7 +130,7 @@ func TestUserHandlers_Integration(t *testing.T) { NewPassword: "newpass123", } - rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession) + rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularTestUser) assert.Equal(t, http.StatusUnauthorized, rr.Code) }) @@ -140,68 +140,40 @@ func TestUserHandlers_Integration(t *testing.T) { NewPassword: "short", } - rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession) + rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularTestUser) assert.Equal(t, http.StatusBadRequest, rr.Code) }) t.Run("duplicate email", func(t *testing.T) { updateReq := handlers.UpdateProfileRequest{ - Email: h.AdminUser.Email, + Email: h.AdminTestUser.userModel.Email, CurrentPassword: currentPassword, } - rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularSession) + rr := h.makeRequest(t, http.MethodPut, "/api/v1/profile", updateReq, h.RegularTestUser) assert.Equal(t, http.StatusConflict, rr.Code) }) }) t.Run("delete account", func(t *testing.T) { - // Create a new user that we can delete - createReq := handlers.CreateUserRequest{ - Email: "todelete@test.com", - DisplayName: "To Delete", - Password: "password123", - Role: models.RoleEditor, - } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/admin/users", createReq, h.AdminSession) - require.Equal(t, http.StatusOK, rr.Code) - - var newUser models.User - err := json.NewDecoder(rr.Body).Decode(&newUser) - require.NoError(t, err) - - // Get session for new user - loginReq := handlers.LoginRequest{ - Email: createReq.Email, - Password: createReq.Password, - } - - rr = h.makeRequest(t, http.MethodPost, "/api/v1/auth/login", loginReq, nil) - require.Equal(t, http.StatusOK, rr.Code) - - var loginResp handlers.LoginResponse - err = json.NewDecoder(rr.Body).Decode(&loginResp) - require.NoError(t, err) - - // Create a session struct for the new user - userSession := &models.Session{ - ID: loginResp.SessionID, - UserID: newUser.ID, - RefreshToken: "", - ExpiresAt: loginResp.ExpiresAt, - } + deleteUserPassword := "password123" + testDeleteUser := h.createTestUser(t, "todelete@test.com", deleteUserPassword, models.RoleEditor) t.Run("successful delete", func(t *testing.T) { deleteReq := handlers.DeleteAccountRequest{ - Password: createReq.Password, + Password: deleteUserPassword, } - rr := h.makeRequest(t, http.MethodDelete, "/api/v1/profile", deleteReq, userSession) + rr := h.makeRequest(t, http.MethodDelete, "/api/v1/profile", deleteReq, testDeleteUser) require.Equal(t, http.StatusNoContent, rr.Code) // Verify user is deleted - rr = h.makeRequest(t, http.MethodPost, "/api/v1/auth/login", loginReq, nil) + loginReq := handlers.LoginRequest{ + Email: testDeleteUser.userModel.Email, + Password: deleteUserPassword, + } + rr = h.makeRequest(t, http.MethodPost, "/api/v1/auth/login", loginReq, testDeleteUser) assert.Equal(t, http.StatusUnauthorized, rr.Code) }) @@ -210,7 +182,7 @@ func TestUserHandlers_Integration(t *testing.T) { Password: "wrongpassword", } - rr := h.makeRequest(t, http.MethodDelete, "/api/v1/profile", deleteReq, h.RegularSession) + rr := h.makeRequest(t, http.MethodDelete, "/api/v1/profile", deleteReq, h.RegularTestUser) assert.Equal(t, http.StatusUnauthorized, rr.Code) }) @@ -219,7 +191,7 @@ func TestUserHandlers_Integration(t *testing.T) { Password: "admin123", // Admin password from test harness } - rr := h.makeRequest(t, http.MethodDelete, "/api/v1/profile", deleteReq, h.AdminSession) + rr := h.makeRequest(t, http.MethodDelete, "/api/v1/profile", deleteReq, h.AdminTestUser) assert.Equal(t, http.StatusForbidden, rr.Code) }) }) diff --git a/server/internal/handlers/workspace_handlers_integration_test.go b/server/internal/handlers/workspace_handlers_integration_test.go index dbb92d7..d1b95a6 100644 --- a/server/internal/handlers/workspace_handlers_integration_test.go +++ b/server/internal/handlers/workspace_handlers_integration_test.go @@ -20,7 +20,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { t.Run("list workspaces", func(t *testing.T) { t.Run("successful list", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodGet, "/api/v1/workspaces", nil, h.RegularSession) + rr := h.makeRequest(t, http.MethodGet, "/api/v1/workspaces", nil, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) var workspaces []*models.Workspace @@ -41,14 +41,14 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { Name: "Test Workspace", } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) var created models.Workspace err := json.NewDecoder(rr.Body).Decode(&created) require.NoError(t, err) assert.Equal(t, workspace.Name, created.Name) - assert.Equal(t, h.RegularUser.ID, created.UserID) + assert.Equal(t, h.RegularTestUser.session.UserID, created.UserID) assert.NotZero(t, created.ID) }) @@ -64,7 +64,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { GitCommitEmail: "test@example.com", } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) var created models.Workspace @@ -86,7 +86,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { // Missing required Git settings } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularTestUser) assert.Equal(t, http.StatusBadRequest, rr.Code) }) }) @@ -95,7 +95,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { workspace := &models.Workspace{ Name: "Test Workspace Operations", } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) err := json.NewDecoder(rr.Body).Decode(workspace) require.NoError(t, err) @@ -105,7 +105,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { t.Run("get workspace", func(t *testing.T) { t.Run("successful get", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodGet, baseURL, nil, h.RegularSession) + rr := h.makeRequest(t, http.MethodGet, baseURL, nil, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) var got models.Workspace @@ -116,13 +116,13 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { }) t.Run("nonexistent workspace", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodGet, "/api/v1/workspaces/nonexistent", nil, h.RegularSession) + rr := h.makeRequest(t, http.MethodGet, "/api/v1/workspaces/nonexistent", nil, h.RegularTestUser) assert.Equal(t, http.StatusNotFound, rr.Code) }) t.Run("unauthorized access", func(t *testing.T) { // Try accessing with another user's token - rr := h.makeRequest(t, http.MethodGet, baseURL, nil, h.AdminSession) + rr := h.makeRequest(t, http.MethodGet, baseURL, nil, h.AdminTestUser) assert.Equal(t, http.StatusNotFound, rr.Code) }) }) @@ -131,7 +131,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { t.Run("update name", func(t *testing.T) { workspace.Name = "Updated Workspace" - rr := h.makeRequest(t, http.MethodPut, baseURL, workspace, h.RegularSession) + rr := h.makeRequest(t, http.MethodPut, baseURL, workspace, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) var updated models.Workspace @@ -152,7 +152,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { ShowHiddenFiles: true, } - rr := h.makeRequest(t, http.MethodPut, baseURL, update, h.RegularSession) + rr := h.makeRequest(t, http.MethodPut, baseURL, update, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) var updated models.Workspace @@ -176,7 +176,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { GitCommitEmail: "test@example.com", } - rr := h.makeRequest(t, http.MethodPut, baseURL, update, h.RegularSession) + rr := h.makeRequest(t, http.MethodPut, baseURL, update, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) var updated models.Workspace @@ -200,14 +200,14 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { // Missing required Git settings } - rr := h.makeRequest(t, http.MethodPut, baseURL, update, h.RegularSession) + rr := h.makeRequest(t, http.MethodPut, baseURL, update, h.RegularTestUser) assert.Equal(t, http.StatusBadRequest, rr.Code) }) }) t.Run("last workspace", func(t *testing.T) { t.Run("get last workspace", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodGet, "/api/v1/workspaces/last", nil, h.RegularSession) + rr := h.makeRequest(t, http.MethodGet, "/api/v1/workspaces/last", nil, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) var response struct { @@ -225,11 +225,11 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { WorkspaceName: workspace.Name, } - rr := h.makeRequest(t, http.MethodPut, "/api/v1/workspaces/last", req, h.RegularSession) + rr := h.makeRequest(t, http.MethodPut, "/api/v1/workspaces/last", req, h.RegularTestUser) require.Equal(t, http.StatusNoContent, rr.Code) // Verify the update - rr = h.makeRequest(t, http.MethodGet, "/api/v1/workspaces/last", nil, h.RegularSession) + rr = h.makeRequest(t, http.MethodGet, "/api/v1/workspaces/last", nil, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) var response struct { @@ -243,7 +243,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { t.Run("delete workspace", func(t *testing.T) { // Get current workspaces to know how many we have - rr := h.makeRequest(t, http.MethodGet, "/api/v1/workspaces", nil, h.RegularSession) + rr := h.makeRequest(t, http.MethodGet, "/api/v1/workspaces", nil, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) var existingWorkspaces []*models.Workspace @@ -254,13 +254,13 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { newWorkspace := &models.Workspace{ Name: "Workspace To Delete", } - rr = h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", newWorkspace, h.RegularSession) + rr = h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", newWorkspace, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) err = json.NewDecoder(rr.Body).Decode(newWorkspace) require.NoError(t, err) t.Run("successful delete", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodDelete, "/api/v1/workspaces/"+url.PathEscape(newWorkspace.Name), nil, h.RegularSession) + rr := h.makeRequest(t, http.MethodDelete, "/api/v1/workspaces/"+url.PathEscape(newWorkspace.Name), nil, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) var response struct { @@ -271,7 +271,7 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { assert.NotEmpty(t, response.NextWorkspaceName) // Verify workspace is deleted - rr = h.makeRequest(t, http.MethodGet, "/api/v1/workspaces/"+url.PathEscape(newWorkspace.Name), nil, h.RegularSession) + rr = h.makeRequest(t, http.MethodGet, "/api/v1/workspaces/"+url.PathEscape(newWorkspace.Name), nil, h.RegularTestUser) assert.Equal(t, http.StatusNotFound, rr.Code) }) @@ -279,13 +279,13 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { // Delete all but one workspace for i := 0; i < len(existingWorkspaces)-1; i++ { ws := existingWorkspaces[i] - rr := h.makeRequest(t, http.MethodDelete, "/api/v1/workspaces/"+url.PathEscape(ws.Name), nil, h.RegularSession) + rr := h.makeRequest(t, http.MethodDelete, "/api/v1/workspaces/"+url.PathEscape(ws.Name), nil, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) } // Try to delete the last remaining workspace lastWs := existingWorkspaces[len(existingWorkspaces)-1] - rr := h.makeRequest(t, http.MethodDelete, "/api/v1/workspaces/"+url.PathEscape(lastWs.Name), nil, h.RegularSession) + rr := h.makeRequest(t, http.MethodDelete, "/api/v1/workspaces/"+url.PathEscape(lastWs.Name), nil, h.RegularTestUser) assert.Equal(t, http.StatusBadRequest, rr.Code) }) @@ -294,11 +294,11 @@ func TestWorkspaceHandlers_Integration(t *testing.T) { workspace := &models.Workspace{ Name: "Unauthorized Delete Test", } - rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularSession) + rr := h.makeRequest(t, http.MethodPost, "/api/v1/workspaces", workspace, h.RegularTestUser) require.Equal(t, http.StatusOK, rr.Code) // Try to delete with wrong user's token - rr = h.makeRequest(t, http.MethodDelete, "/api/v1/workspaces/"+url.PathEscape(workspace.Name), nil, h.AdminSession) + rr = h.makeRequest(t, http.MethodDelete, "/api/v1/workspaces/"+url.PathEscape(workspace.Name), nil, h.AdminTestUser) assert.Equal(t, http.StatusNotFound, rr.Code) }) }) From 2268ea48f28044b56187d67138506f3f653041ec Mon Sep 17 00:00:00 2001 From: LordMathis Date: Sun, 8 Dec 2024 17:13:34 +0100 Subject: [PATCH 07/12] Fix session validation --- server/internal/auth/jwt.go | 17 ++++++------ server/internal/auth/jwt_test.go | 5 ++-- server/internal/auth/middleware_test.go | 37 +++++++++++++------------ server/internal/auth/session.go | 14 +++++++--- server/internal/auth/session_test.go | 4 +-- 5 files changed, 43 insertions(+), 34 deletions(-) diff --git a/server/internal/auth/jwt.go b/server/internal/auth/jwt.go index 66b6c24..9df0de0 100644 --- a/server/internal/auth/jwt.go +++ b/server/internal/auth/jwt.go @@ -3,7 +3,6 @@ package auth import ( "crypto/rand" - "encoding/hex" "fmt" "time" @@ -35,8 +34,8 @@ type JWTConfig struct { // JWTManager defines the interface for managing JWT tokens type JWTManager interface { - GenerateAccessToken(userID int, role string) (string, error) - GenerateRefreshToken(userID int, role string) (string, error) + GenerateAccessToken(userID int, role string, sessionID string) (string, error) + GenerateRefreshToken(userID int, role string, sessionID string) (string, error) ValidateToken(tokenString string) (*Claims, error) } @@ -62,17 +61,17 @@ func NewJWTService(config JWTConfig) (JWTManager, error) { } // GenerateAccessToken creates a new access token for a user with the given userID and role -func (s *jwtService) GenerateAccessToken(userID int, role string) (string, error) { - return s.generateToken(userID, role, AccessToken, s.config.AccessTokenExpiry) +func (s *jwtService) GenerateAccessToken(userID int, role, sessionID string) (string, error) { + return s.generateToken(userID, role, sessionID, AccessToken, s.config.AccessTokenExpiry) } // GenerateRefreshToken creates a new refresh token for a user with the given userID and role -func (s *jwtService) GenerateRefreshToken(userID int, role string) (string, error) { - return s.generateToken(userID, role, RefreshToken, s.config.RefreshTokenExpiry) +func (s *jwtService) GenerateRefreshToken(userID int, role, sessionID string) (string, error) { + return s.generateToken(userID, role, sessionID, RefreshToken, s.config.RefreshTokenExpiry) } // generateToken is an internal helper function that creates a new JWT token -func (s *jwtService) generateToken(userID int, role string, tokenType TokenType, expiry time.Duration) (string, error) { +func (s *jwtService) generateToken(userID int, role string, sessionID string, tokenType TokenType, expiry time.Duration) (string, error) { now := time.Now() // Add a random nonce to ensure uniqueness @@ -86,7 +85,7 @@ func (s *jwtService) generateToken(userID int, role string, tokenType TokenType, ExpiresAt: jwt.NewNumericDate(now.Add(expiry)), IssuedAt: jwt.NewNumericDate(now), NotBefore: jwt.NewNumericDate(now), - ID: hex.EncodeToString(nonce), + ID: sessionID, }, UserID: userID, Role: role, diff --git a/server/internal/auth/jwt_test.go b/server/internal/auth/jwt_test.go index 61ca928..14b1a50 100644 --- a/server/internal/auth/jwt_test.go +++ b/server/internal/auth/jwt_test.go @@ -1,3 +1,4 @@ +// Package auth_test provides tests for the auth package package auth_test import ( @@ -98,9 +99,9 @@ func TestGenerateAndValidateToken(t *testing.T) { // Generate token based on type if tc.tokenType == auth.AccessToken { - token, err = service.GenerateAccessToken(tc.userID, tc.role) + token, err = service.GenerateAccessToken(tc.userID, tc.role, "") } else { - token, err = service.GenerateRefreshToken(tc.userID, tc.role) + token, err = service.GenerateRefreshToken(tc.userID, tc.role, "") } if err != nil { diff --git a/server/internal/auth/middleware_test.go b/server/internal/auth/middleware_test.go index 983c3d3..3bac129 100644 --- a/server/internal/auth/middleware_test.go +++ b/server/internal/auth/middleware_test.go @@ -1,6 +1,7 @@ package auth_test import ( + "fmt" "net/http" "net/http/httptest" "strings" @@ -23,18 +24,18 @@ func newMockSessionManager() *mockSessionManager { } } -func (m *mockSessionManager) CreateSession(userID int, role string) (*models.Session, string, error) { +func (m *mockSessionManager) CreateSession(_ int, _ string) (*models.Session, string, error) { return nil, "", nil // Not needed for these tests } -func (m *mockSessionManager) RefreshSession(refreshToken string) (string, error) { +func (m *mockSessionManager) RefreshSession(_ string) (string, error) { return "", nil // Not needed for these tests } func (m *mockSessionManager) ValidateSession(sessionID string) (*models.Session, error) { session, exists := m.sessions[sessionID] if !exists { - return nil, nil + return nil, fmt.Errorf("session not found") } return session, nil } @@ -87,16 +88,16 @@ func TestAuthenticateMiddleware(t *testing.T) { testCases := []struct { name string - setupRequest func() *http.Request + setupRequest func(sessionID string) *http.Request setupSession func(sessionID string) method string wantStatusCode int }{ { name: "valid token with valid session", - setupRequest: func() *http.Request { + setupRequest: func(sessionID string) *http.Request { req := httptest.NewRequest("GET", "/test", nil) - token, _ := jwtService.GenerateAccessToken(1, "admin") + token, _ := jwtService.GenerateAccessToken(1, "admin", sessionID) cookie := cookieManager.GenerateAccessTokenCookie(token) req.AddCookie(cookie) return req @@ -113,31 +114,31 @@ func TestAuthenticateMiddleware(t *testing.T) { }, { name: "valid token but invalid session", - setupRequest: func() *http.Request { + setupRequest: func(sessionID string) *http.Request { req := httptest.NewRequest("GET", "/test", nil) - token, _ := jwtService.GenerateAccessToken(1, "admin") + token, _ := jwtService.GenerateAccessToken(1, "admin", sessionID) cookie := cookieManager.GenerateAccessTokenCookie(token) req.AddCookie(cookie) return req }, - setupSession: func(sessionID string) {}, // No session setup + setupSession: func(_ string) {}, // No session setup method: "GET", wantStatusCode: http.StatusUnauthorized, }, { name: "missing auth cookie", - setupRequest: func() *http.Request { + setupRequest: func(_ string) *http.Request { return httptest.NewRequest("GET", "/test", nil) }, - setupSession: func(sessionID string) {}, + setupSession: func(_ string) {}, method: "GET", wantStatusCode: http.StatusUnauthorized, }, { name: "POST request without CSRF token", - setupRequest: func() *http.Request { + setupRequest: func(sessionID string) *http.Request { req := httptest.NewRequest("POST", "/test", nil) - token, _ := jwtService.GenerateAccessToken(1, "admin") + token, _ := jwtService.GenerateAccessToken(1, "admin", sessionID) cookie := cookieManager.GenerateAccessTokenCookie(token) req.AddCookie(cookie) return req @@ -154,9 +155,9 @@ func TestAuthenticateMiddleware(t *testing.T) { }, { name: "POST request with valid CSRF token", - setupRequest: func() *http.Request { + setupRequest: func(sessionID string) *http.Request { req := httptest.NewRequest("POST", "/test", nil) - token, _ := jwtService.GenerateAccessToken(1, "admin") + token, _ := jwtService.GenerateAccessToken(1, "admin", sessionID) cookie := cookieManager.GenerateAccessTokenCookie(token) req.AddCookie(cookie) @@ -180,12 +181,14 @@ func TestAuthenticateMiddleware(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - req := tc.setupRequest() + sessionID := tc.name + + req := tc.setupRequest(sessionID) w := newMockResponseWriter() // Create test handler nextCalled := false - next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + next := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { nextCalled = true w.WriteHeader(http.StatusOK) }) diff --git a/server/internal/auth/session.go b/server/internal/auth/session.go index 21d0090..bb3df67 100644 --- a/server/internal/auth/session.go +++ b/server/internal/auth/session.go @@ -9,6 +9,7 @@ import ( "github.com/google/uuid" ) +// SessionManager is an interface for managing user sessions type SessionManager interface { CreateSession(userID int, role string) (*models.Session, string, error) RefreshSession(refreshToken string) (string, error) @@ -24,6 +25,7 @@ type sessionManager struct { } // NewSessionService creates a new session service with the given database and JWT manager +// revive:disable:unexported-return func NewSessionService(db db.SessionStore, jwtManager JWTManager) *sessionManager { return &sessionManager{ db: db, @@ -33,13 +35,17 @@ func NewSessionService(db db.SessionStore, jwtManager JWTManager) *sessionManage // CreateSession creates a new user session for a user with the given userID and role func (s *sessionManager) CreateSession(userID int, role string) (*models.Session, string, error) { + + // Generate a new session ID + sessionID := uuid.New().String() + // Generate both access and refresh tokens - accessToken, err := s.jwtManager.GenerateAccessToken(userID, role) + accessToken, err := s.jwtManager.GenerateAccessToken(userID, role, sessionID) if err != nil { return nil, "", fmt.Errorf("failed to generate access token: %w", err) } - refreshToken, err := s.jwtManager.GenerateRefreshToken(userID, role) + refreshToken, err := s.jwtManager.GenerateRefreshToken(userID, role, sessionID) if err != nil { return nil, "", fmt.Errorf("failed to generate refresh token: %w", err) } @@ -52,7 +58,7 @@ func (s *sessionManager) CreateSession(userID int, role string) (*models.Session // Create a new session record session := &models.Session{ - ID: uuid.New().String(), + ID: sessionID, UserID: userID, RefreshToken: refreshToken, ExpiresAt: claims.ExpiresAt.Time, @@ -87,7 +93,7 @@ func (s *sessionManager) RefreshSession(refreshToken string) (string, error) { } // Generate a new access token - return s.jwtManager.GenerateAccessToken(claims.UserID, claims.Role) + return s.jwtManager.GenerateAccessToken(claims.UserID, claims.Role, session.ID) } // ValidateSession checks if a session with the given sessionID is valid diff --git a/server/internal/auth/session_test.go b/server/internal/auth/session_test.go index 8464fae..91410fb 100644 --- a/server/internal/auth/session_test.go +++ b/server/internal/auth/session_test.go @@ -259,7 +259,7 @@ func TestRefreshSession(t *testing.T) { { name: "valid refresh token", setupSession: func() string { - token, _ := jwtService.GenerateRefreshToken(1, "admin") + token, _ := jwtService.GenerateRefreshToken(1, "admin", "test-session-1") session := &models.Session{ ID: "test-session-1", UserID: 1, @@ -277,7 +277,7 @@ func TestRefreshSession(t *testing.T) { { name: "expired refresh token", setupSession: func() string { - token, _ := jwtService.GenerateRefreshToken(1, "admin") + token, _ := jwtService.GenerateRefreshToken(1, "admin", "test-session-2") session := &models.Session{ ID: "test-session-2", UserID: 1, From ed6ac312ed8556fa8523ce2cb212bb8b2157c780 Mon Sep 17 00:00:00 2001 From: LordMathis Date: Sun, 8 Dec 2024 17:22:39 +0100 Subject: [PATCH 08/12] Fix failing logout tests --- .../auth_handlers_integration_test.go | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/server/internal/handlers/auth_handlers_integration_test.go b/server/internal/handlers/auth_handlers_integration_test.go index 51703f0..70c72ab 100644 --- a/server/internal/handlers/auth_handlers_integration_test.go +++ b/server/internal/handlers/auth_handlers_integration_test.go @@ -251,13 +251,13 @@ func TestAuthHandlers_Integration(t *testing.T) { t.Run("logout edge cases", func(t *testing.T) { tests := []struct { name string - setup func(*http.Request) + setup func(*http.Request, *testUser) wantCode int }{ { name: "missing CSRF token", - setup: func(req *http.Request) { - h.addAuthCookies(t, req, h.RegularTestUser) + setup: func(req *http.Request, tu *testUser) { + h.addAuthCookies(t, req, tu) h.addCSRFCookie(t, req) // Deliberately not setting X-CSRF-Token header }, @@ -265,8 +265,8 @@ func TestAuthHandlers_Integration(t *testing.T) { }, { name: "mismatched CSRF token", - setup: func(req *http.Request) { - h.addAuthCookies(t, req, h.RegularTestUser) + setup: func(req *http.Request, tu *testUser) { + h.addAuthCookies(t, req, tu) h.addCSRFCookie(t, req) req.Header.Set("X-CSRF-Token", "wrong-token") }, @@ -274,7 +274,7 @@ func TestAuthHandlers_Integration(t *testing.T) { }, { name: "missing auth cookies", - setup: func(req *http.Request) { + setup: func(req *http.Request, tu *testUser) { // No setup - testing completely unauthenticated request }, wantCode: http.StatusUnauthorized, @@ -283,8 +283,12 @@ func TestAuthHandlers_Integration(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + // Create a unique user for each test case + // Construct a unique email address from test name + uniqueUserEmail := strings.Replace(tt.name, " ", "", -1) + "@test.com" + logoutTestUser := h.createTestUser(t, uniqueUserEmail, "password123", models.RoleEditor) req := h.newRequest(t, http.MethodPost, "/api/v1/auth/logout", nil) - tt.setup(req) + tt.setup(req, logoutTestUser) rr := h.executeRequest(req) assert.Equal(t, tt.wantCode, rr.Code) }) @@ -293,14 +297,17 @@ func TestAuthHandlers_Integration(t *testing.T) { }) t.Run("get current user", func(t *testing.T) { + + getTestUser := h.createTestUser(t, "testgetuser@test.com", "password123", models.RoleEditor) + t.Run("successful get current user", func(t *testing.T) { - rr := h.makeRequest(t, http.MethodGet, "/api/v1/auth/me", nil, h.RegularTestUser) + rr := h.makeRequest(t, http.MethodGet, "/api/v1/auth/me", nil, getTestUser) require.Equal(t, http.StatusOK, rr.Code) var user models.User err := json.NewDecoder(rr.Body).Decode(&user) require.NoError(t, err) - assert.Equal(t, h.RegularTestUser.userModel.Email, user.Email) + assert.Equal(t, getTestUser.userModel.Email, user.Email) }) t.Run("auth edge cases", func(t *testing.T) { From 8ebbe1e076a615ff5546a28bef1d10eb12842557 Mon Sep 17 00:00:00 2001 From: LordMathis Date: Sun, 8 Dec 2024 17:43:50 +0100 Subject: [PATCH 09/12] Update api docs --- server/internal/handlers/auth_handlers.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/server/internal/handlers/auth_handlers.go b/server/internal/handlers/auth_handlers.go index f55c650..8fb2391 100644 --- a/server/internal/handlers/auth_handlers.go +++ b/server/internal/handlers/auth_handlers.go @@ -34,10 +34,12 @@ type LoginResponse struct { // @Produce json // @Param body body LoginRequest true "Login request" // @Success 200 {object} LoginResponse +// @Header 200 {string} X-CSRF-Token "CSRF token for future requests" // @Failure 400 {object} ErrorResponse "Invalid request body" // @Failure 400 {object} ErrorResponse "Email and password are required" // @Failure 401 {object} ErrorResponse "Invalid credentials" // @Failure 500 {object} ErrorResponse "Failed to create session" +// @Failure 500 {object} ErrorResponse "Failed to generate CSRF token" // @Router /auth/login [post] func (h *Handler) Login(authManager auth.SessionManager, cookieService auth.CookieManager) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { @@ -142,10 +144,11 @@ func (h *Handler) Logout(authManager auth.SessionManager, cookieService auth.Coo // @Accept json // @Produce json // @Param body body RefreshRequest true "Refresh request" -// @Success 200 "Tokens refreshed successfully via cookies" -// @Failure 400 {object} ErrorResponse "Invalid request body" +// @Success 200 +// @Header 200 {string} X-CSRF-Token "New CSRF token" // @Failure 400 {object} ErrorResponse "Refresh token required" // @Failure 401 {object} ErrorResponse "Invalid refresh token" +// @Failure 500 {object} ErrorResponse "Failed to generate CSRF token" // @Router /auth/refresh [post] func (h *Handler) RefreshToken(authManager auth.SessionManager, cookieService auth.CookieManager) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { From a7a97caa6ae3500656d3f8136abf243378874972 Mon Sep 17 00:00:00 2001 From: LordMathis Date: Sun, 8 Dec 2024 17:46:59 +0100 Subject: [PATCH 10/12] Regenerate api docs --- server/docs/docs.go | 147 ++++++++-------------- server/docs/swagger.json | 147 ++++++++-------------- server/docs/swagger.yaml | 119 +++++++----------- server/internal/handlers/auth_handlers.go | 1 - 4 files changed, 155 insertions(+), 259 deletions(-) diff --git a/server/docs/docs.go b/server/docs/docs.go index 47dc923..e2a6464 100644 --- a/server/docs/docs.go +++ b/server/docs/docs.go @@ -23,7 +23,7 @@ const docTemplate = `{ "get": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Get system-wide statistics as an admin", @@ -55,7 +55,7 @@ const docTemplate = `{ "get": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Returns the list of all users", @@ -88,7 +88,7 @@ const docTemplate = `{ "post": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Create a new user as an admin", @@ -146,7 +146,7 @@ const docTemplate = `{ "get": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Get a specific user as an admin", @@ -191,7 +191,7 @@ const docTemplate = `{ "put": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Update a specific user as an admin", @@ -254,7 +254,7 @@ const docTemplate = `{ "delete": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Delete a specific user as an admin", @@ -307,7 +307,7 @@ const docTemplate = `{ "get": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "List all workspaces and their stats as an admin", @@ -340,7 +340,7 @@ const docTemplate = `{ }, "/auth/login": { "post": { - "description": "Logs in a user", + "description": "Logs in a user and returns a session with access and refresh tokens", "consumes": [ "application/json" ], @@ -351,7 +351,6 @@ const docTemplate = `{ "auth" ], "summary": "Login", - "operationId": "login", "parameters": [ { "description": "Login request", @@ -368,6 +367,12 @@ const docTemplate = `{ "description": "OK", "schema": { "$ref": "#/definitions/handlers.LoginResponse" + }, + "headers": { + "X-CSRF-Token": { + "type": "string", + "description": "CSRF token for future requests" + } } }, "400": { @@ -383,7 +388,7 @@ const docTemplate = `{ } }, "500": { - "description": "Failed to create session", + "description": "Failed to generate CSRF token", "schema": { "$ref": "#/definitions/handlers.ErrorResponse" } @@ -393,11 +398,6 @@ const docTemplate = `{ }, "/auth/logout": { "post": { - "security": [ - { - "BearerAuth": [] - } - ], "description": "Log out invalidates the user's session", "tags": [ "auth" @@ -427,7 +427,7 @@ const docTemplate = `{ "get": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Returns the current authenticated user", @@ -469,22 +469,14 @@ const docTemplate = `{ ], "summary": "Refresh token", "operationId": "refreshToken", - "parameters": [ - { - "description": "Refresh request", - "name": "body", - "in": "body", - "required": true, - "schema": { - "$ref": "#/definitions/handlers.RefreshRequest" - } - } - ], "responses": { "200": { "description": "OK", - "schema": { - "$ref": "#/definitions/handlers.RefreshResponse" + "headers": { + "X-CSRF-Token": { + "type": "string", + "description": "New CSRF token" + } } }, "400": { @@ -498,6 +490,12 @@ const docTemplate = `{ "schema": { "$ref": "#/definitions/handlers.ErrorResponse" } + }, + "500": { + "description": "Failed to generate CSRF token", + "schema": { + "$ref": "#/definitions/handlers.ErrorResponse" + } } } } @@ -506,7 +504,7 @@ const docTemplate = `{ "put": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Updates the user's profile", @@ -574,7 +572,7 @@ const docTemplate = `{ "delete": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Deletes the user's account and all associated data", @@ -641,7 +639,7 @@ const docTemplate = `{ "get": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Lists all workspaces for the current user", @@ -674,7 +672,7 @@ const docTemplate = `{ "post": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Creates a new workspace", @@ -726,7 +724,7 @@ const docTemplate = `{ "get": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Returns the name of the last opened workspace", @@ -756,7 +754,7 @@ const docTemplate = `{ "put": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Updates the name of the last opened workspace", @@ -794,7 +792,7 @@ const docTemplate = `{ "get": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Returns the current workspace", @@ -833,7 +831,7 @@ const docTemplate = `{ "put": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Updates the current workspace", @@ -890,7 +888,7 @@ const docTemplate = `{ "delete": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Deletes the current workspace", @@ -937,7 +935,7 @@ const docTemplate = `{ "get": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Lists all files in the user's workspace", @@ -981,7 +979,7 @@ const docTemplate = `{ "get": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Returns the path of the last opened file in the user's workspace", @@ -1026,7 +1024,7 @@ const docTemplate = `{ "put": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Updates the last opened file in the user's workspace", @@ -1088,7 +1086,7 @@ const docTemplate = `{ "get": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Returns the paths of files with the given name in the user's workspace", @@ -1142,7 +1140,7 @@ const docTemplate = `{ "get": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Returns the content of a file in the user's workspace", @@ -1200,7 +1198,7 @@ const docTemplate = `{ "post": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Saves the content of a file in the user's workspace", @@ -1255,7 +1253,7 @@ const docTemplate = `{ "delete": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Deletes a file in the user's workspace", @@ -1309,7 +1307,7 @@ const docTemplate = `{ "post": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Stages, commits, and pushes changes to the remote repository", @@ -1365,7 +1363,7 @@ const docTemplate = `{ "post": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Pulls changes from the remote repository", @@ -1493,15 +1491,12 @@ const docTemplate = `{ "handlers.LoginResponse": { "type": "object", "properties": { - "accessToken": { + "expiresAt": { "type": "string" }, - "refreshToken": { + "sessionId": { "type": "string" }, - "session": { - "$ref": "#/definitions/models.Session" - }, "user": { "$ref": "#/definitions/models.User" } @@ -1527,22 +1522,6 @@ const docTemplate = `{ } } }, - "handlers.RefreshRequest": { - "type": "object", - "properties": { - "refreshToken": { - "type": "string" - } - } - }, - "handlers.RefreshResponse": { - "type": "object", - "properties": { - "accessToken": { - "type": "string" - } - } - }, "handlers.SaveFileResponse": { "type": "object", "properties": { @@ -1646,31 +1625,6 @@ const docTemplate = `{ } } }, - "models.Session": { - "type": "object", - "properties": { - "createdAt": { - "description": "When this session was created", - "type": "string" - }, - "expiresAt": { - "description": "When this session expires", - "type": "string" - }, - "id": { - "description": "Unique session identifier", - "type": "string" - }, - "refreshToken": { - "description": "The refresh token associated with this session", - "type": "string" - }, - "userID": { - "description": "ID of the user this session belongs to", - "type": "integer" - } - } - }, "models.User": { "type": "object", "required": [ @@ -1807,6 +1761,13 @@ const docTemplate = `{ } } } + }, + "securityDefinitions": { + "CookieAuth": { + "type": "apiKey", + "name": "access_token", + "in": "cookie" + } } }` diff --git a/server/docs/swagger.json b/server/docs/swagger.json index 52ca878..9f7b36c 100644 --- a/server/docs/swagger.json +++ b/server/docs/swagger.json @@ -16,7 +16,7 @@ "get": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Get system-wide statistics as an admin", @@ -48,7 +48,7 @@ "get": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Returns the list of all users", @@ -81,7 +81,7 @@ "post": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Create a new user as an admin", @@ -139,7 +139,7 @@ "get": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Get a specific user as an admin", @@ -184,7 +184,7 @@ "put": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Update a specific user as an admin", @@ -247,7 +247,7 @@ "delete": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Delete a specific user as an admin", @@ -300,7 +300,7 @@ "get": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "List all workspaces and their stats as an admin", @@ -333,7 +333,7 @@ }, "/auth/login": { "post": { - "description": "Logs in a user", + "description": "Logs in a user and returns a session with access and refresh tokens", "consumes": [ "application/json" ], @@ -344,7 +344,6 @@ "auth" ], "summary": "Login", - "operationId": "login", "parameters": [ { "description": "Login request", @@ -361,6 +360,12 @@ "description": "OK", "schema": { "$ref": "#/definitions/handlers.LoginResponse" + }, + "headers": { + "X-CSRF-Token": { + "type": "string", + "description": "CSRF token for future requests" + } } }, "400": { @@ -376,7 +381,7 @@ } }, "500": { - "description": "Failed to create session", + "description": "Failed to generate CSRF token", "schema": { "$ref": "#/definitions/handlers.ErrorResponse" } @@ -386,11 +391,6 @@ }, "/auth/logout": { "post": { - "security": [ - { - "BearerAuth": [] - } - ], "description": "Log out invalidates the user's session", "tags": [ "auth" @@ -420,7 +420,7 @@ "get": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Returns the current authenticated user", @@ -462,22 +462,14 @@ ], "summary": "Refresh token", "operationId": "refreshToken", - "parameters": [ - { - "description": "Refresh request", - "name": "body", - "in": "body", - "required": true, - "schema": { - "$ref": "#/definitions/handlers.RefreshRequest" - } - } - ], "responses": { "200": { "description": "OK", - "schema": { - "$ref": "#/definitions/handlers.RefreshResponse" + "headers": { + "X-CSRF-Token": { + "type": "string", + "description": "New CSRF token" + } } }, "400": { @@ -491,6 +483,12 @@ "schema": { "$ref": "#/definitions/handlers.ErrorResponse" } + }, + "500": { + "description": "Failed to generate CSRF token", + "schema": { + "$ref": "#/definitions/handlers.ErrorResponse" + } } } } @@ -499,7 +497,7 @@ "put": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Updates the user's profile", @@ -567,7 +565,7 @@ "delete": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Deletes the user's account and all associated data", @@ -634,7 +632,7 @@ "get": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Lists all workspaces for the current user", @@ -667,7 +665,7 @@ "post": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Creates a new workspace", @@ -719,7 +717,7 @@ "get": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Returns the name of the last opened workspace", @@ -749,7 +747,7 @@ "put": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Updates the name of the last opened workspace", @@ -787,7 +785,7 @@ "get": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Returns the current workspace", @@ -826,7 +824,7 @@ "put": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Updates the current workspace", @@ -883,7 +881,7 @@ "delete": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Deletes the current workspace", @@ -930,7 +928,7 @@ "get": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Lists all files in the user's workspace", @@ -974,7 +972,7 @@ "get": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Returns the path of the last opened file in the user's workspace", @@ -1019,7 +1017,7 @@ "put": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Updates the last opened file in the user's workspace", @@ -1081,7 +1079,7 @@ "get": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Returns the paths of files with the given name in the user's workspace", @@ -1135,7 +1133,7 @@ "get": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Returns the content of a file in the user's workspace", @@ -1193,7 +1191,7 @@ "post": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Saves the content of a file in the user's workspace", @@ -1248,7 +1246,7 @@ "delete": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Deletes a file in the user's workspace", @@ -1302,7 +1300,7 @@ "post": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Stages, commits, and pushes changes to the remote repository", @@ -1358,7 +1356,7 @@ "post": { "security": [ { - "BearerAuth": [] + "CookieAuth": [] } ], "description": "Pulls changes from the remote repository", @@ -1486,15 +1484,12 @@ "handlers.LoginResponse": { "type": "object", "properties": { - "accessToken": { + "expiresAt": { "type": "string" }, - "refreshToken": { + "sessionId": { "type": "string" }, - "session": { - "$ref": "#/definitions/models.Session" - }, "user": { "$ref": "#/definitions/models.User" } @@ -1520,22 +1515,6 @@ } } }, - "handlers.RefreshRequest": { - "type": "object", - "properties": { - "refreshToken": { - "type": "string" - } - } - }, - "handlers.RefreshResponse": { - "type": "object", - "properties": { - "accessToken": { - "type": "string" - } - } - }, "handlers.SaveFileResponse": { "type": "object", "properties": { @@ -1639,31 +1618,6 @@ } } }, - "models.Session": { - "type": "object", - "properties": { - "createdAt": { - "description": "When this session was created", - "type": "string" - }, - "expiresAt": { - "description": "When this session expires", - "type": "string" - }, - "id": { - "description": "Unique session identifier", - "type": "string" - }, - "refreshToken": { - "description": "The refresh token associated with this session", - "type": "string" - }, - "userID": { - "description": "ID of the user this session belongs to", - "type": "integer" - } - } - }, "models.User": { "type": "object", "required": [ @@ -1800,5 +1754,12 @@ } } } + }, + "securityDefinitions": { + "CookieAuth": { + "type": "apiKey", + "name": "access_token", + "in": "cookie" + } } } \ No newline at end of file diff --git a/server/docs/swagger.yaml b/server/docs/swagger.yaml index 505cd38..ce00e3a 100644 --- a/server/docs/swagger.yaml +++ b/server/docs/swagger.yaml @@ -57,12 +57,10 @@ definitions: type: object handlers.LoginResponse: properties: - accessToken: + expiresAt: type: string - refreshToken: + sessionId: type: string - session: - $ref: '#/definitions/models.Session' user: $ref: '#/definitions/models.User' type: object @@ -79,16 +77,6 @@ definitions: example: Pulled changes from remote type: string type: object - handlers.RefreshRequest: - properties: - refreshToken: - type: string - type: object - handlers.RefreshResponse: - properties: - accessToken: - type: string - type: object handlers.SaveFileResponse: properties: filePath: @@ -156,24 +144,6 @@ definitions: workspaceName: type: string type: object - models.Session: - properties: - createdAt: - description: When this session was created - type: string - expiresAt: - description: When this session expires - type: string - id: - description: Unique session identifier - type: string - refreshToken: - description: The refresh token associated with this session - type: string - userID: - description: ID of the user this session belongs to - type: integer - type: object models.User: properties: createdAt: @@ -292,7 +262,7 @@ paths: schema: $ref: '#/definitions/handlers.ErrorResponse' security: - - BearerAuth: [] + - CookieAuth: [] summary: Get system statistics tags: - Admin @@ -314,7 +284,7 @@ paths: schema: $ref: '#/definitions/handlers.ErrorResponse' security: - - BearerAuth: [] + - CookieAuth: [] summary: List all users tags: - Admin @@ -350,7 +320,7 @@ paths: schema: $ref: '#/definitions/handlers.ErrorResponse' security: - - BearerAuth: [] + - CookieAuth: [] summary: Create a new user tags: - Admin @@ -384,7 +354,7 @@ paths: schema: $ref: '#/definitions/handlers.ErrorResponse' security: - - BearerAuth: [] + - CookieAuth: [] summary: Delete a specific user tags: - Admin @@ -413,7 +383,7 @@ paths: schema: $ref: '#/definitions/handlers.ErrorResponse' security: - - BearerAuth: [] + - CookieAuth: [] summary: Get a specific user tags: - Admin @@ -454,7 +424,7 @@ paths: schema: $ref: '#/definitions/handlers.ErrorResponse' security: - - BearerAuth: [] + - CookieAuth: [] summary: Update a specific user tags: - Admin @@ -476,7 +446,7 @@ paths: schema: $ref: '#/definitions/handlers.ErrorResponse' security: - - BearerAuth: [] + - CookieAuth: [] summary: List all workspaces tags: - Admin @@ -484,8 +454,7 @@ paths: post: consumes: - application/json - description: Logs in a user - operationId: login + description: Logs in a user and returns a session with access and refresh tokens parameters: - description: Login request in: body @@ -498,6 +467,10 @@ paths: responses: "200": description: OK + headers: + X-CSRF-Token: + description: CSRF token for future requests + type: string schema: $ref: '#/definitions/handlers.LoginResponse' "400": @@ -509,7 +482,7 @@ paths: schema: $ref: '#/definitions/handlers.ErrorResponse' "500": - description: Failed to create session + description: Failed to generate CSRF token schema: $ref: '#/definitions/handlers.ErrorResponse' summary: Login @@ -530,8 +503,6 @@ paths: description: Failed to logout schema: $ref: '#/definitions/handlers.ErrorResponse' - security: - - BearerAuth: [] summary: Logout tags: - auth @@ -551,7 +522,7 @@ paths: schema: $ref: '#/definitions/handlers.ErrorResponse' security: - - BearerAuth: [] + - CookieAuth: [] summary: Get current user tags: - auth @@ -561,20 +532,15 @@ paths: - application/json description: Refreshes the access token using the refresh token operationId: refreshToken - parameters: - - description: Refresh request - in: body - name: body - required: true - schema: - $ref: '#/definitions/handlers.RefreshRequest' produces: - application/json responses: "200": description: OK - schema: - $ref: '#/definitions/handlers.RefreshResponse' + headers: + X-CSRF-Token: + description: New CSRF token + type: string "400": description: Refresh token required schema: @@ -583,6 +549,10 @@ paths: description: Invalid refresh token schema: $ref: '#/definitions/handlers.ErrorResponse' + "500": + description: Failed to generate CSRF token + schema: + $ref: '#/definitions/handlers.ErrorResponse' summary: Refresh token tags: - auth @@ -625,7 +595,7 @@ paths: schema: $ref: '#/definitions/handlers.ErrorResponse' security: - - BearerAuth: [] + - CookieAuth: [] summary: Delete account tags: - users @@ -669,7 +639,7 @@ paths: schema: $ref: '#/definitions/handlers.ErrorResponse' security: - - BearerAuth: [] + - CookieAuth: [] summary: Update profile tags: - users @@ -691,7 +661,7 @@ paths: schema: $ref: '#/definitions/handlers.ErrorResponse' security: - - BearerAuth: [] + - CookieAuth: [] summary: List workspaces tags: - workspaces @@ -723,7 +693,7 @@ paths: schema: $ref: '#/definitions/handlers.ErrorResponse' security: - - BearerAuth: [] + - CookieAuth: [] summary: Create workspace tags: - workspaces @@ -753,7 +723,7 @@ paths: schema: $ref: '#/definitions/handlers.ErrorResponse' security: - - BearerAuth: [] + - CookieAuth: [] summary: Delete workspace tags: - workspaces @@ -778,7 +748,7 @@ paths: schema: $ref: '#/definitions/handlers.ErrorResponse' security: - - BearerAuth: [] + - CookieAuth: [] summary: Get workspace tags: - workspaces @@ -815,7 +785,7 @@ paths: schema: $ref: '#/definitions/handlers.ErrorResponse' security: - - BearerAuth: [] + - CookieAuth: [] summary: Update workspace tags: - workspaces @@ -843,7 +813,7 @@ paths: schema: $ref: '#/definitions/handlers.ErrorResponse' security: - - BearerAuth: [] + - CookieAuth: [] summary: List files tags: - files @@ -878,7 +848,7 @@ paths: schema: $ref: '#/definitions/handlers.ErrorResponse' security: - - BearerAuth: [] + - CookieAuth: [] summary: Delete file tags: - files @@ -916,7 +886,7 @@ paths: schema: $ref: '#/definitions/handlers.ErrorResponse' security: - - BearerAuth: [] + - CookieAuth: [] summary: Get file content tags: - files @@ -952,7 +922,7 @@ paths: schema: $ref: '#/definitions/handlers.ErrorResponse' security: - - BearerAuth: [] + - CookieAuth: [] summary: Save file tags: - files @@ -982,7 +952,7 @@ paths: schema: $ref: '#/definitions/handlers.ErrorResponse' security: - - BearerAuth: [] + - CookieAuth: [] summary: Get last opened file tags: - files @@ -1021,7 +991,7 @@ paths: schema: $ref: '#/definitions/handlers.ErrorResponse' security: - - BearerAuth: [] + - CookieAuth: [] summary: Update last opened file tags: - files @@ -1056,7 +1026,7 @@ paths: schema: $ref: '#/definitions/handlers.ErrorResponse' security: - - BearerAuth: [] + - CookieAuth: [] summary: Lookup file by name tags: - files @@ -1092,7 +1062,7 @@ paths: schema: $ref: '#/definitions/handlers.ErrorResponse' security: - - BearerAuth: [] + - CookieAuth: [] summary: Stage, commit, and push changes tags: - git @@ -1118,7 +1088,7 @@ paths: schema: $ref: '#/definitions/handlers.ErrorResponse' security: - - BearerAuth: [] + - CookieAuth: [] summary: Pull changes from remote tags: - git @@ -1138,7 +1108,7 @@ paths: schema: $ref: '#/definitions/handlers.ErrorResponse' security: - - BearerAuth: [] + - CookieAuth: [] summary: Get last workspace name tags: - workspaces @@ -1161,8 +1131,13 @@ paths: schema: $ref: '#/definitions/handlers.ErrorResponse' security: - - BearerAuth: [] + - CookieAuth: [] summary: Update last workspace name tags: - workspaces +securityDefinitions: + CookieAuth: + in: cookie + name: access_token + type: apiKey swagger: "2.0" diff --git a/server/internal/handlers/auth_handlers.go b/server/internal/handlers/auth_handlers.go index 8fb2391..2f1cbb2 100644 --- a/server/internal/handlers/auth_handlers.go +++ b/server/internal/handlers/auth_handlers.go @@ -143,7 +143,6 @@ func (h *Handler) Logout(authManager auth.SessionManager, cookieService auth.Coo // @ID refreshToken // @Accept json // @Produce json -// @Param body body RefreshRequest true "Refresh request" // @Success 200 // @Header 200 {string} X-CSRF-Token "New CSRF token" // @Failure 400 {object} ErrorResponse "Refresh token required" From 8a62c9e306ec0cb64b536f659370d1f0e833b97e Mon Sep 17 00:00:00 2001 From: LordMathis Date: Sun, 8 Dec 2024 17:47:44 +0100 Subject: [PATCH 11/12] Regenerate docs --- server/documentation.md | 602 ++++++++++++++++++++++++++++------------ 1 file changed, 432 insertions(+), 170 deletions(-) diff --git a/server/documentation.md b/server/documentation.md index 76e9edb..a738170 100644 --- a/server/documentation.md +++ b/server/documentation.md @@ -5,9 +5,9 @@ Generated documentation for all packages in the NovaMD project. ## Table of Contents - [cmd/server](#cmd-server) +- [docs](#docs) - [internal/app](#internal-app) - [internal/auth](#internal-auth) -- [internal/config](#internal-config) - [internal/context](#internal-context) - [internal/db](#internal-db) - [internal/git](#internal-git) @@ -23,6 +23,31 @@ Package main provides the entry point for the application. It loads the configuration, initializes the server, and starts the server. ``` +## docs + +```go +package docs // import "novamd/docs" + +Package docs Code generated by swaggo/swag. DO NOT EDIT + +VARIABLES + +var SwaggerInfo = &swag.Spec{ + Version: "1.0", + Host: "", + BasePath: "/api/v1", + Schemes: []string{}, + Title: "NovaMD API", + Description: "This is the API for NovaMD markdown note taking app.", + InfoInstanceName: "swagger", + SwaggerTemplate: docTemplate, + LeftDelim: "{{", + RightDelim: "}}", +} + SwaggerInfo holds exported Swagger Info so clients can modify it + +``` + ## internal/app ```go @@ -31,124 +56,6 @@ package app // import "novamd/internal/app" Package app provides application-level functionality for initializing and running the server -FUNCTIONS - -func SetupRoutes(r chi.Router, db db.Database, s storage.Manager, authMiddleware *auth.Middleware, sessionService *auth.SessionService) - SetupRoutes configures the API routes - - -TYPES - -type Server struct { - // Has unexported fields. -} - Server represents the HTTP server and its dependencies - -func NewServer(cfg *config.Config) (*Server, error) - NewServer initializes a new server instance with all dependencies - -func (s *Server) Close() error - Close handles graceful shutdown of server dependencies - -func (s *Server) Start() error - Start configures and starts the HTTP server - -``` - -## internal/auth - -```go -package auth // import "novamd/internal/auth" - -Package auth provides JWT token generation and validation - -TYPES - -type Claims struct { - jwt.RegisteredClaims // Embedded standard JWT claims - UserID int `json:"uid"` // User identifier - Role string `json:"role"` // User role (admin, editor, viewer) - Type TokenType `json:"type"` // Token type (access or refresh) -} - Claims represents the custom claims we store in JWT tokens - -type JWTConfig struct { - SigningKey string // Secret key used to sign tokens - AccessTokenExpiry time.Duration // How long access tokens are valid - RefreshTokenExpiry time.Duration // How long refresh tokens are valid -} - JWTConfig holds the configuration for the JWT service - -type JWTManager interface { - GenerateAccessToken(userID int, role string) (string, error) - GenerateRefreshToken(userID int, role string) (string, error) - ValidateToken(tokenString string) (*Claims, error) - RefreshAccessToken(refreshToken string) (string, error) -} - JWTManager defines the interface for managing JWT tokens - -func NewJWTService(config JWTConfig) (JWTManager, error) - NewJWTService creates a new JWT service with the provided configuration - Returns an error if the signing key is missing - -type Middleware struct { - // Has unexported fields. -} - Middleware handles JWT authentication for protected routes - -func NewMiddleware(jwtManager JWTManager) *Middleware - NewMiddleware creates a new authentication middleware - -func (m *Middleware) Authenticate(next http.Handler) http.Handler - Authenticate middleware validates JWT tokens and sets user information in - context - -func (m *Middleware) RequireRole(role string) func(http.Handler) http.Handler - RequireRole returns a middleware that ensures the user has the required role - -func (m *Middleware) RequireWorkspaceAccess(next http.Handler) http.Handler - RequireWorkspaceAccess returns a middleware that ensures the user has access - to the workspace - -type SessionService struct { - // Has unexported fields. -} - SessionService manages user sessions in the database - -func NewSessionService(db db.SessionStore, jwtManager JWTManager) *SessionService - NewSessionService creates a new session service with the given database and - JWT manager - -func (s *SessionService) CleanExpiredSessions() error - CleanExpiredSessions removes all expired sessions from the database - -func (s *SessionService) CreateSession(userID int, role string) (*models.Session, string, error) - CreateSession creates a new user session for a user with the given userID - and role - -func (s *SessionService) InvalidateSession(sessionID string) error - InvalidateSession removes a session with the given sessionID from the - database - -func (s *SessionService) RefreshSession(refreshToken string) (string, error) - RefreshSession creates a new access token using a refreshToken - -type TokenType string - TokenType represents the type of JWT token (access or refresh) - -const ( - AccessToken TokenType = "access" // AccessToken - Short-lived token for API access - RefreshToken TokenType = "refresh" // RefreshToken - Long-lived token for obtaining new access tokens -) -``` - -## internal/config - -```go -package config // import "novamd/internal/config" - -Package config provides the configuration for the application - TYPES type Config struct { @@ -156,7 +63,8 @@ type Config struct { WorkDir string StaticPath string Port string - AppURL string + RootURL string + Domain string CORSOrigins []string AdminEmail string AdminPassword string @@ -171,12 +79,132 @@ type Config struct { func DefaultConfig() *Config DefaultConfig returns a new Config instance with default values -func Load() (*Config, error) - Load creates a new Config instance with values from environment variables +func LoadConfig() (*Config, error) + LoadConfig creates a new Config instance with values from environment + variables -func (c *Config) Validate() error - Validate checks if the configuration is valid +type Options struct { + Config *Config + Database db.Database + Storage storage.Manager + JWTManager auth.JWTManager + SessionManager auth.SessionManager + CookieService auth.CookieManager +} + Options holds all dependencies and configuration for the server +func DefaultOptions(cfg *Config) (*Options, error) + DefaultOptions creates server options with default configuration + +type Server struct { + // Has unexported fields. +} + Server represents the HTTP server and its dependencies + +func NewServer(options *Options) *Server + NewServer creates a new server instance with the given options + +func (s *Server) Close() error + Close handles graceful shutdown of server dependencies + +func (s *Server) Router() chi.Router + Router returns the chi router for testing + +func (s *Server) Start() error + Start configures and starts the HTTP server + +``` + +## internal/auth + +```go +package auth // import "novamd/internal/auth" + +Package auth provides JWT token generation and validation + +Package auth provides JWT token generation and validation + +FUNCTIONS + +func NewSessionService(db db.SessionStore, jwtManager JWTManager) *sessionManager + NewSessionService creates a new session service with the given database and + JWT manager revive:disable:unexported-return + + +TYPES + +type Claims struct { + jwt.RegisteredClaims // Embedded standard JWT claims + UserID int `json:"uid"` // User identifier + Role string `json:"role"` // User role (admin, editor, viewer) + Type TokenType `json:"type"` // Token type (access or refresh) +} + Claims represents the custom claims we store in JWT tokens + +type CookieManager interface { + GenerateAccessTokenCookie(token string) *http.Cookie + GenerateRefreshTokenCookie(token string) *http.Cookie + GenerateCSRFCookie(token string) *http.Cookie + InvalidateCookie(cookieType string) *http.Cookie +} + CookieManager interface defines methods for generating cookies + +func NewCookieService(isDevelopment bool, domain string) CookieManager + NewCookieService creates a new cookie service + +type JWTConfig struct { + SigningKey string // Secret key used to sign tokens + AccessTokenExpiry time.Duration // How long access tokens are valid + RefreshTokenExpiry time.Duration // How long refresh tokens are valid +} + JWTConfig holds the configuration for the JWT service + +type JWTManager interface { + GenerateAccessToken(userID int, role string, sessionID string) (string, error) + GenerateRefreshToken(userID int, role string, sessionID string) (string, error) + ValidateToken(tokenString string) (*Claims, error) +} + JWTManager defines the interface for managing JWT tokens + +func NewJWTService(config JWTConfig) (JWTManager, error) + NewJWTService creates a new JWT service with the provided configuration + Returns an error if the signing key is missing + +type Middleware struct { + // Has unexported fields. +} + Middleware handles JWT authentication for protected routes + +func NewMiddleware(jwtManager JWTManager, sessionManager SessionManager, cookieManager CookieManager) *Middleware + NewMiddleware creates a new authentication middleware + +func (m *Middleware) Authenticate(next http.Handler) http.Handler + Authenticate middleware validates JWT tokens and sets user information in + context + +func (m *Middleware) RequireRole(role string) func(http.Handler) http.Handler + RequireRole returns a middleware that ensures the user has the required role + +func (m *Middleware) RequireWorkspaceAccess(next http.Handler) http.Handler + RequireWorkspaceAccess returns a middleware that ensures the user has access + to the workspace + +type SessionManager interface { + CreateSession(userID int, role string) (*models.Session, string, error) + RefreshSession(refreshToken string) (string, error) + ValidateSession(sessionID string) (*models.Session, error) + InvalidateSession(token string) error + CleanExpiredSessions() error +} + SessionManager is an interface for managing user sessions + +type TokenType string + TokenType represents the type of JWT token (access or refresh) + +const ( + AccessToken TokenType = "access" // AccessToken - Short-lived token for API access + RefreshToken TokenType = "refresh" // RefreshToken - Long-lived token for obtaining new access tokens +) ``` ## internal/context @@ -271,6 +299,7 @@ type Migration struct { type SessionStore interface { CreateSession(session *models.Session) error GetSessionByRefreshToken(refreshToken string) (*models.Session, error) + GetSessionByID(sessionID string) (*models.Session, error) DeleteSession(sessionID string) error CleanExpiredSessions() error } @@ -350,7 +379,7 @@ TYPES type Client interface { Clone() error Pull() error - Commit(message string) error + Commit(message string) (CommitHash, error) Push() error EnsureRepo() error } @@ -359,6 +388,12 @@ type Client interface { func New(url, username, token, workDir, commitName, commitEmail string) Client New creates a new git Client instance +type CommitHash plumbing.Hash + CommitHash represents a Git commit hash + +func (h CommitHash) String() string + String returns the string representation of the CommitHash + type Config struct { URL string Username string @@ -380,6 +415,16 @@ Package handlers contains the request handlers for the api routes. TYPES +type CommitRequest struct { + Message string `json:"message" example:"Initial commit"` +} + CommitRequest represents a request to commit changes + +type CommitResponse struct { + CommitHash string `json:"commitHash" example:"a1b2c3d4"` +} + CommitResponse represents a response to a commit request + type CreateUserRequest struct { Email string `json:"email"` DisplayName string `json:"displayName"` @@ -393,6 +438,17 @@ type DeleteAccountRequest struct { } DeleteAccountRequest represents a user account deletion request +type DeleteWorkspaceResponse struct { + NextWorkspaceName string `json:"nextWorkspaceName"` +} + DeleteWorkspaceResponse contains the name of the next workspace after + deleting the current one + +type ErrorResponse struct { + Message string `json:"message"` +} + ErrorResponse is a generic error response + type Handler struct { DB db.Database Storage storage.Manager @@ -403,92 +459,286 @@ func NewHandler(db db.Database, s storage.Manager) *Handler NewHandler creates a new handler with the given dependencies func (h *Handler) AdminCreateUser() http.HandlerFunc - AdminCreateUser creates a new user + AdminCreateUser godoc @Summary Create a new user @Description Create a + new user as an admin @Tags Admin @Security CookieAuth @ID adminCreateUser + @Accept json @Produce json @Param user body CreateUserRequest true + "User details" @Success 200 {object} models.User @Failure 400 {object} + ErrorResponse "Invalid request body" @Failure 400 {object} ErrorResponse + "Email, password, and role are required" @Failure 400 {object} ErrorResponse + "Password must be at least 8 characters" @Failure 409 {object} ErrorResponse + "Email already exists" @Failure 500 {object} ErrorResponse "Failed to + hash password" @Failure 500 {object} ErrorResponse "Failed to create user" + @Failure 500 {object} ErrorResponse "Failed to initialize user workspace" + @Router /admin/users [post] func (h *Handler) AdminDeleteUser() http.HandlerFunc - AdminDeleteUser deletes a specific user + AdminDeleteUser godoc @Summary Delete a specific user @Description + Delete a specific user as an admin @Tags Admin @Security CookieAuth @ID + adminDeleteUser @Param userId path int true "User ID" @Success 204 "No + Content" @Failure 400 {object} ErrorResponse "Invalid user ID" @Failure + 400 {object} ErrorResponse "Cannot delete your own account" @Failure 403 + {object} ErrorResponse "Cannot delete other admin users" @Failure 404 + {object} ErrorResponse "User not found" @Failure 500 {object} ErrorResponse + "Failed to delete user" @Router /admin/users/{userId} [delete] func (h *Handler) AdminGetSystemStats() http.HandlerFunc - AdminGetSystemStats returns system-wide statistics for admins + AdminGetSystemStats godoc @Summary Get system statistics @Description Get + system-wide statistics as an admin @Tags Admin @Security CookieAuth @ID + adminGetSystemStats @Produce json @Success 200 {object} SystemStats @Failure + 500 {object} ErrorResponse "Failed to get user stats" @Failure 500 {object} + ErrorResponse "Failed to get file stats" @Router /admin/stats [get] func (h *Handler) AdminGetUser() http.HandlerFunc - AdminGetUser gets a specific user by ID + AdminGetUser godoc @Summary Get a specific user @Description Get a specific + user as an admin @Tags Admin @Security CookieAuth @ID adminGetUser @Produce + json @Param userId path int true "User ID" @Success 200 {object} models.User + @Failure 400 {object} ErrorResponse "Invalid user ID" @Failure 404 {object} + ErrorResponse "User not found" @Router /admin/users/{userId} [get] func (h *Handler) AdminListUsers() http.HandlerFunc - AdminListUsers returns a list of all users + AdminListUsers godoc @Summary List all users @Description Returns the list + of all users @Tags Admin @Security CookieAuth @ID adminListUsers @Produce + json @Success 200 {array} models.User @Failure 500 {object} ErrorResponse + "Failed to list users" @Router /admin/users [get] func (h *Handler) AdminListWorkspaces() http.HandlerFunc - AdminListWorkspaces returns a list of all workspaces and their stats + AdminListWorkspaces godoc @Summary List all workspaces @Description List + all workspaces and their stats as an admin @Tags Admin @Security CookieAuth + @ID adminListWorkspaces @Produce json @Success 200 {array} WorkspaceStats + @Failure 500 {object} ErrorResponse "Failed to list workspaces" @Failure + 500 {object} ErrorResponse "Failed to get user" @Failure 500 {object} + ErrorResponse "Failed to get file stats" @Router /admin/workspaces [get] func (h *Handler) AdminUpdateUser() http.HandlerFunc - AdminUpdateUser updates a specific user + AdminUpdateUser godoc @Summary Update a specific user @Description + Update a specific user as an admin @Tags Admin @Security CookieAuth @ID + adminUpdateUser @Accept json @Produce json @Param userId path int true + "User ID" @Param user body UpdateUserRequest true "User details" @Success + 200 {object} models.User @Failure 400 {object} ErrorResponse "Invalid user + ID" @Failure 400 {object} ErrorResponse "Invalid request body" @Failure 404 + {object} ErrorResponse "User not found" @Failure 500 {object} ErrorResponse + "Failed to hash password" @Failure 500 {object} ErrorResponse "Failed to + update user" @Router /admin/users/{userId} [put] func (h *Handler) CreateWorkspace() http.HandlerFunc - CreateWorkspace creates a new workspace + CreateWorkspace godoc @Summary Create workspace @Description Creates a new + workspace @Tags workspaces @ID createWorkspace @Security CookieAuth @Accept + json @Produce json @Param body body models.Workspace true "Workspace" + @Success 200 {object} models.Workspace @Failure 400 {object} ErrorResponse + "Invalid request body" @Failure 400 {object} ErrorResponse "Invalid + workspace" @Failure 500 {object} ErrorResponse "Failed to create workspace" + @Failure 500 {object} ErrorResponse "Failed to initialize workspace + directory" @Failure 500 {object} ErrorResponse "Failed to setup git repo" + @Router /workspaces [post] func (h *Handler) DeleteAccount() http.HandlerFunc - DeleteAccount handles user account deletion + DeleteAccount godoc @Summary Delete account @Description Deletes the user's + account and all associated data @Tags users @ID deleteAccount @Security + CookieAuth @Accept json @Produce json @Param body body DeleteAccountRequest + true "Account deletion request" @Success 204 "No Content - Account deleted + successfully" @Failure 400 {object} ErrorResponse "Invalid request body" + @Failure 401 {object} ErrorResponse "Password is incorrect" @Failure 403 + {object} ErrorResponse "Cannot delete the last admin account" @Failure 404 + {object} ErrorResponse "User not found" @Failure 500 {object} ErrorResponse + "Failed to verify admin status" @Failure 500 {object} ErrorResponse "Failed + to delete account" @Router /profile [delete] func (h *Handler) DeleteFile() http.HandlerFunc - DeleteFile deletes a file + DeleteFile godoc @Summary Delete file @Description Deletes a file in + the user's workspace @Tags files @ID deleteFile @Security CookieAuth + @Param workspace_name path string true "Workspace name" @Param + file_path path string true "File path" @Success 204 "No Content + - File deleted successfully" @Failure 400 {object} ErrorResponse + "Invalid file path" @Failure 404 {object} ErrorResponse "File not + found" @Failure 500 {object} ErrorResponse "Failed to delete file" + @Failure 500 {object} ErrorResponse "Failed to write response" @Router + /workspaces/{workspace_name}/files/{file_path} [delete] func (h *Handler) DeleteWorkspace() http.HandlerFunc - DeleteWorkspace deletes the current workspace + DeleteWorkspace godoc @Summary Delete workspace @Description Deletes + the current workspace @Tags workspaces @ID deleteWorkspace @Security + CookieAuth @Produce json @Param workspace_name path string true "Workspace + name" @Success 200 {object} DeleteWorkspaceResponse @Failure 400 {object} + ErrorResponse "Cannot delete the last workspace" @Failure 500 {object} + ErrorResponse "Failed to get workspaces" @Failure 500 {object} ErrorResponse + "Failed to start transaction" @Failure 500 {object} ErrorResponse "Failed + to update last workspace" @Failure 500 {object} ErrorResponse "Failed to + delete workspace" @Failure 500 {object} ErrorResponse "Failed to rollback + transaction" @Failure 500 {object} ErrorResponse "Failed to commit + transaction" @Router /workspaces/{workspace_name} [delete] func (h *Handler) GetCurrentUser() http.HandlerFunc - GetCurrentUser returns the currently authenticated user + GetCurrentUser godoc @Summary Get current user @Description Returns + the current authenticated user @Tags auth @ID getCurrentUser @Security + CookieAuth @Produce json @Success 200 {object} models.User @Failure 404 + {object} ErrorResponse "User not found" @Router /auth/me [get] func (h *Handler) GetFileContent() http.HandlerFunc - GetFileContent returns the content of a file + GetFileContent godoc @Summary Get file content @Description Returns the + content of a file in the user's workspace @Tags files @ID getFileContent + @Security CookieAuth @Produce plain @Param workspace_name path string + true "Workspace name" @Param file_path path string true "File path" + @Success 200 {string} string "Raw file content" @Failure 400 {object} + ErrorResponse "Invalid file path" @Failure 404 {object} ErrorResponse + "File not found" @Failure 500 {object} ErrorResponse "Failed to read file" + @Failure 500 {object} ErrorResponse "Failed to write response" @Router + /workspaces/{workspace_name}/files/{file_path} [get] func (h *Handler) GetLastOpenedFile() http.HandlerFunc - GetLastOpenedFile returns the last opened file in the workspace + GetLastOpenedFile godoc @Summary Get last opened file @Description + Returns the path of the last opened file in the user's workspace @Tags + files @ID getLastOpenedFile @Security CookieAuth @Produce json @Param + workspace_name path string true "Workspace name" @Success 200 {object} + LastOpenedFileResponse @Failure 400 {object} ErrorResponse "Invalid file + path" @Failure 500 {object} ErrorResponse "Failed to get last opened file" + @Router /workspaces/{workspace_name}/files/last [get] func (h *Handler) GetLastWorkspaceName() http.HandlerFunc - GetLastWorkspaceName returns the name of the last opened workspace + GetLastWorkspaceName godoc @Summary Get last workspace name @Description + Returns the name of the last opened workspace @Tags workspaces @ID + getLastWorkspaceName @Security CookieAuth @Produce json @Success 200 + {object} LastWorkspaceNameResponse @Failure 500 {object} ErrorResponse + "Failed to get last workspace" @Router /workspaces/last [get] func (h *Handler) GetWorkspace() http.HandlerFunc - GetWorkspace returns the current workspace + GetWorkspace godoc @Summary Get workspace @Description Returns the current + workspace @Tags workspaces @ID getWorkspace @Security CookieAuth @Produce + json @Param workspace_name path string true "Workspace name" @Success 200 + {object} models.Workspace @Failure 500 {object} ErrorResponse "Internal + server error" @Router /workspaces/{workspace_name} [get] func (h *Handler) ListFiles() http.HandlerFunc - ListFiles returns a list of all files in the workspace + ListFiles godoc @Summary List files @Description Lists all files in the + user's workspace @Tags files @ID listFiles @Security CookieAuth @Produce + json @Param workspace_name path string true "Workspace name" @Success 200 + {array} storage.FileNode @Failure 500 {object} ErrorResponse "Failed to list + files" @Router /workspaces/{workspace_name}/files [get] func (h *Handler) ListWorkspaces() http.HandlerFunc - ListWorkspaces returns a list of all workspaces for the current user + ListWorkspaces godoc @Summary List workspaces @Description Lists all + workspaces for the current user @Tags workspaces @ID listWorkspaces + @Security CookieAuth @Produce json @Success 200 {array} models.Workspace + @Failure 500 {object} ErrorResponse "Failed to list workspaces" @Router + /workspaces [get] -func (h *Handler) Login(authService *auth.SessionService) http.HandlerFunc - Login handles user authentication and returns JWT tokens +func (h *Handler) Login(authManager auth.SessionManager, cookieService auth.CookieManager) http.HandlerFunc + Login godoc @Summary Login @Description Logs in a user and returns a + session with access and refresh tokens @Tags auth @Accept json @Produce + json @Param body body LoginRequest true "Login request" @Success 200 + {object} LoginResponse @Header 200 {string} X-CSRF-Token "CSRF token for + future requests" @Failure 400 {object} ErrorResponse "Invalid request + body" @Failure 400 {object} ErrorResponse "Email and password are required" + @Failure 401 {object} ErrorResponse "Invalid credentials" @Failure 500 + {object} ErrorResponse "Failed to create session" @Failure 500 {object} + ErrorResponse "Failed to generate CSRF token" @Router /auth/login [post] -func (h *Handler) Logout(authService *auth.SessionService) http.HandlerFunc - Logout invalidates the user's session +func (h *Handler) Logout(authManager auth.SessionManager, cookieService auth.CookieManager) http.HandlerFunc + Logout godoc @Summary Logout @Description Log out invalidates the user's + session @Tags auth @ID logout @Success 204 "No Content" @Failure 400 + {object} ErrorResponse "Session ID required" @Failure 500 {object} + ErrorResponse "Failed to logout" @Router /auth/logout [post] func (h *Handler) LookupFileByName() http.HandlerFunc - LookupFileByName returns the paths of files with the given name + LookupFileByName godoc @Summary Lookup file by name @Description Returns the + paths of files with the given name in the user's workspace @Tags files @ID + lookupFileByName @Security CookieAuth @Produce json @Param workspace_name + path string true "Workspace name" @Param filename query string true + "File name" @Success 200 {object} LookupResponse @Failure 400 {object} + ErrorResponse "Filename is required" @Failure 404 {object} ErrorResponse + "File not found" @Router /workspaces/{workspace_name}/files/lookup [get] func (h *Handler) PullChanges() http.HandlerFunc - PullChanges pulls changes from the remote repository + PullChanges godoc @Summary Pull changes from remote @Description Pulls + changes from the remote repository @Tags git @ID pullChanges @Security + CookieAuth @Produce json @Param workspace_name path string true "Workspace + name" @Success 200 {object} PullResponse @Failure 500 {object} ErrorResponse + "Failed to pull changes" @Router /workspaces/{workspace_name}/git/pull + [post] -func (h *Handler) RefreshToken(authService *auth.SessionService) http.HandlerFunc - RefreshToken generates a new access token using a refresh token +func (h *Handler) RefreshToken(authManager auth.SessionManager, cookieService auth.CookieManager) http.HandlerFunc + RefreshToken godoc @Summary Refresh token @Description Refreshes the access + token using the refresh token @Tags auth @ID refreshToken @Accept json + @Produce json @Success 200 @Header 200 {string} X-CSRF-Token "New CSRF + token" @Failure 400 {object} ErrorResponse "Refresh token required" @Failure + 401 {object} ErrorResponse "Invalid refresh token" @Failure 500 {object} + ErrorResponse "Failed to generate CSRF token" @Router /auth/refresh [post] func (h *Handler) SaveFile() http.HandlerFunc - SaveFile saves the content of a file + SaveFile godoc @Summary Save file @Description Saves the content of a file + in the user's workspace @Tags files @ID saveFile @Security CookieAuth + @Accept plain @Produce json @Param workspace_name path string true + "Workspace name" @Param file_path path string true "File path" @Success + 200 {object} SaveFileResponse @Failure 400 {object} ErrorResponse "Failed + to read request body" @Failure 400 {object} ErrorResponse "Invalid file + path" @Failure 500 {object} ErrorResponse "Failed to save file" @Router + /workspaces/{workspace_name}/files/{file_path} [post] func (h *Handler) StageCommitAndPush() http.HandlerFunc - StageCommitAndPush stages, commits, and pushes changes to the remote - repository + StageCommitAndPush godoc @Summary Stage, commit, and push changes + @Description Stages, commits, and pushes changes to the remote repository + @Tags git @ID stageCommitAndPush @Security CookieAuth @Produce json + @Param workspace_name path string true "Workspace name" @Param body body + CommitRequest true "Commit request" @Success 200 {object} CommitResponse + @Failure 400 {object} ErrorResponse "Invalid request body" @Failure + 400 {object} ErrorResponse "Commit message is required" @Failure 500 + {object} ErrorResponse "Failed to stage, commit, and push changes" @Router + /workspaces/{workspace_name}/git/commit [post] func (h *Handler) UpdateLastOpenedFile() http.HandlerFunc - UpdateLastOpenedFile updates the last opened file in the workspace + UpdateLastOpenedFile godoc @Summary Update last opened file @Description + Updates the last opened file in the user's workspace @Tags files @ID + updateLastOpenedFile @Security CookieAuth @Accept json @Produce json + @Param workspace_name path string true "Workspace name" @Param body + body UpdateLastOpenedFileRequest true "Update last opened file request" + @Success 204 "No Content - Last opened file updated successfully" @Failure + 400 {object} ErrorResponse "Invalid request body" @Failure 400 {object} + ErrorResponse "Invalid file path" @Failure 404 {object} ErrorResponse "File + not found" @Failure 500 {object} ErrorResponse "Failed to update file" + @Router /workspaces/{workspace_name}/files/last [put] func (h *Handler) UpdateLastWorkspaceName() http.HandlerFunc - UpdateLastWorkspaceName updates the name of the last opened workspace + UpdateLastWorkspaceName godoc @Summary Update last workspace name + @Description Updates the name of the last opened workspace @Tags workspaces + @ID updateLastWorkspaceName @Security CookieAuth @Accept json @Produce json + @Success 204 "No Content - Last workspace updated successfully" @Failure + 400 {object} ErrorResponse "Invalid request body" @Failure 500 {object} + ErrorResponse "Failed to update last workspace" @Router /workspaces/last + [put] func (h *Handler) UpdateProfile() http.HandlerFunc - UpdateProfile updates the current user's profile + UpdateProfile godoc @Summary Update profile @Description Updates the + user's profile @Tags users @ID updateProfile @Security CookieAuth @Accept + json @Produce json @Param body body UpdateProfileRequest true "Profile + update request" @Success 200 {object} models.User @Failure 400 {object} + ErrorResponse "Invalid request body" @Failure 400 {object} ErrorResponse + "Current password is required to change password" @Failure 400 {object} + ErrorResponse "New password must be at least 8 characters long" @Failure + 400 {object} ErrorResponse "Current password is required to change email" + @Failure 401 {object} ErrorResponse "Current password is incorrect" + @Failure 404 {object} ErrorResponse "User not found" @Failure 409 {object} + ErrorResponse "Email already in use" @Failure 500 {object} ErrorResponse + "Failed to process new password" @Failure 500 {object} ErrorResponse "Failed + to update profile" @Router /profile [put] func (h *Handler) UpdateWorkspace() http.HandlerFunc - UpdateWorkspace updates the current workspace + UpdateWorkspace godoc @Summary Update workspace @Description Updates + the current workspace @Tags workspaces @ID updateWorkspace @Security + CookieAuth @Accept json @Produce json @Param workspace_name path string + true "Workspace name" @Param body body models.Workspace true "Workspace" + @Success 200 {object} models.Workspace @Failure 400 {object} ErrorResponse + "Invalid request body" @Failure 500 {object} ErrorResponse "Failed to update + workspace" @Failure 500 {object} ErrorResponse "Failed to setup git repo" + @Router /workspaces/{workspace_name} [put] + +type LastOpenedFileResponse struct { + LastOpenedFilePath string `json:"lastOpenedFilePath"` +} + LastOpenedFileResponse represents a response to a last opened file request + +type LastWorkspaceNameResponse struct { + LastWorkspaceName string `json:"lastWorkspaceName"` +} + LastWorkspaceNameResponse contains the name of the last opened workspace type LoginRequest struct { Email string `json:"email"` @@ -497,22 +747,28 @@ type LoginRequest struct { LoginRequest represents a user login request type LoginResponse struct { - AccessToken string `json:"accessToken"` - RefreshToken string `json:"refreshToken"` - User *models.User `json:"user"` - Session *models.Session `json:"session"` + User *models.User `json:"user"` + SessionID string `json:"sessionId,omitempty"` + ExpiresAt time.Time `json:"expiresAt,omitempty"` } LoginResponse represents a user login response -type RefreshRequest struct { - RefreshToken string `json:"refreshToken"` +type LookupResponse struct { + Paths []string `json:"paths"` } - RefreshRequest represents a refresh token request + LookupResponse represents a response to a file lookup request -type RefreshResponse struct { - AccessToken string `json:"accessToken"` +type PullResponse struct { + Message string `json:"message" example:"Pulled changes from remote"` } - RefreshResponse represents a refresh token response + PullResponse represents a response to a pull http request + +type SaveFileResponse struct { + FilePath string `json:"filePath"` + Size int64 `json:"size"` + UpdatedAt time.Time `json:"updatedAt"` +} + SaveFileResponse represents a response to a save file request type StaticHandler struct { // Has unexported fields. @@ -532,6 +788,12 @@ type SystemStats struct { } SystemStats holds system-wide statistics +type UpdateLastOpenedFileRequest struct { + FilePath string `json:"filePath"` +} + UpdateLastOpenedFileRequest represents a request to update the last opened + file + type UpdateProfileRequest struct { DisplayName string `json:"displayName"` Email string `json:"email"` @@ -733,7 +995,7 @@ func (e *PathValidationError) Error() string type RepositoryManager interface { SetupGitRepo(userID, workspaceID int, gitURL, gitUser, gitToken, commitName, commitEmail string) error DisableGitRepo(userID, workspaceID int) - StageCommitAndPush(userID, workspaceID int, message string) error + StageCommitAndPush(userID, workspaceID int, message string) (git.CommitHash, error) Pull(userID, workspaceID int) error } RepositoryManager defines the interface for managing Git repositories. @@ -809,7 +1071,7 @@ func (s *Service) SetupGitRepo(userID, workspaceID int, gitURL, gitUser, gitToke The repository is cloned from the given gitURL using the given gitUser and gitToken. -func (s *Service) StageCommitAndPush(userID, workspaceID int, message string) error +func (s *Service) StageCommitAndPush(userID, workspaceID int, message string) (git.CommitHash, error) StageCommitAndPush stages, commit with the message, and pushes the changes to the Git repository. The git repository belongs to the given userID and is associated with the given workspaceID. From 26a208123a2d36424d1c55ef7a7fd57fd77dcdf1 Mon Sep 17 00:00:00 2001 From: LordMathis Date: Mon, 9 Dec 2024 20:57:56 +0100 Subject: [PATCH 12/12] Implement cookie auth on frontend --- app/src/contexts/AuthContext.jsx | 28 ++++--------- app/src/services/authApi.js | 69 +++++++++++++++----------------- 2 files changed, 41 insertions(+), 56 deletions(-) diff --git a/app/src/contexts/AuthContext.jsx b/app/src/contexts/AuthContext.jsx index a93d0c5..4878e05 100644 --- a/app/src/contexts/AuthContext.jsx +++ b/app/src/contexts/AuthContext.jsx @@ -19,16 +19,10 @@ export const AuthProvider = ({ children }) => { useEffect(() => { const initializeAuth = async () => { try { - const storedToken = localStorage.getItem('accessToken'); - if (storedToken) { - authApi.setAuthToken(storedToken); - const userData = await authApi.getCurrentUser(); - setUser(userData); - } + const userData = await authApi.getCurrentUser(); + setUser(userData); } catch (error) { console.error('Failed to initialize auth:', error); - localStorage.removeItem('accessToken'); - authApi.clearAuthToken(); } finally { setLoading(false); setInitialized(true); @@ -40,12 +34,7 @@ export const AuthProvider = ({ children }) => { const login = useCallback(async (email, password) => { try { - const { accessToken, user: userData } = await authApi.login( - email, - password - ); - localStorage.setItem('accessToken', accessToken); - authApi.setAuthToken(accessToken); + const { user: userData } = await authApi.login(email, password); setUser(userData); notifications.show({ title: 'Success', @@ -70,18 +59,17 @@ export const AuthProvider = ({ children }) => { } catch (error) { console.error('Logout failed:', error); } finally { - localStorage.removeItem('accessToken'); - authApi.clearAuthToken(); setUser(null); } }, []); const refreshToken = useCallback(async () => { try { - const { accessToken } = await authApi.refreshToken(); - localStorage.setItem('accessToken', accessToken); - authApi.setAuthToken(accessToken); - return true; + const success = await authApi.refreshToken(); + if (!success) { + await logout(); + } + return success; } catch (error) { console.error('Token refresh failed:', error); await logout(); diff --git a/app/src/services/authApi.js b/app/src/services/authApi.js index 1144f0c..9a65633 100644 --- a/app/src/services/authApi.js +++ b/app/src/services/authApi.js @@ -1,40 +1,32 @@ import { API_BASE_URL } from '../utils/constants'; -let authToken = null; - -export const setAuthToken = (token) => { - authToken = token; -}; - -export const clearAuthToken = () => { - authToken = null; -}; - -export const getAuthHeaders = () => { - const headers = { - 'Content-Type': 'application/json', - }; - - if (authToken) { - headers['Authorization'] = `Bearer ${authToken}`; - } - - return headers; -}; - -// Update the existing apiCall function to include auth headers export const apiCall = async (url, options = {}) => { try { const headers = { - ...getAuthHeaders(), + 'Content-Type': 'application/json', ...options.headers, }; + if (options.method && options.method !== 'GET') { + const csrfToken = document.cookie + .split('; ') + .find((row) => row.startsWith('csrf_token=')) + ?.split('=')[1]; + if (csrfToken) { + headers['X-CSRF-Token'] = csrfToken; + } + } + const response = await fetch(url, { ...options, headers, + credentials: 'include', }); + if (response.status === 429) { + throw new Error('Rate limit exceeded'); + } + // Handle 401 responses if (response.status === 401) { const isRefreshEndpoint = url.endsWith('/auth/refresh'); @@ -42,13 +34,14 @@ export const apiCall = async (url, options = {}) => { // Attempt token refresh and retry the request const refreshSuccess = await refreshToken(); if (refreshSuccess) { - // Retry the original request with the new token + // Retry the original request return apiCall(url, options); } } throw new Error('Authentication failed'); } + // Handle other error responses if (!response.ok && response.status !== 204) { const errorData = await response.json().catch(() => null); throw new Error( @@ -56,6 +49,7 @@ export const apiCall = async (url, options = {}) => { ); } + // Return null for 204 responses if (response.status === 204) { return null; } @@ -73,26 +67,29 @@ export const login = async (email, password) => { method: 'POST', body: JSON.stringify({ email, password }), }); - return response.json(); + + const data = await response.json(); + // No need to store tokens as they're in cookies now + return data; }; export const logout = async () => { - const sessionId = localStorage.getItem('sessionId'); await apiCall(`${API_BASE_URL}/auth/logout`, { method: 'POST', - headers: { - 'X-Session-ID': sessionId, - }, }); + return; }; export const refreshToken = async () => { - const refreshToken = localStorage.getItem('refreshToken'); - const response = await apiCall(`${API_BASE_URL}/auth/refresh`, { - method: 'POST', - body: JSON.stringify({ refreshToken }), - }); - return response.json(); + try { + const response = await apiCall(`${API_BASE_URL}/auth/refresh`, { + method: 'POST', + }); + return response.status === 200; + } catch (error) { + console.error('Token refresh failed:', error); + return false; + } }; export const getCurrentUser = async () => {