From be4fcad60537177cb10cf3a529c4fa1c9af56183 Mon Sep 17 00:00:00 2001 From: Huxley Date: Wed, 24 Jun 2026 14:43:38 +0800 Subject: [PATCH] Refactor handlers to use MustGetUserID - Fix: replace repeated authorization checks in handlers with the new MustGetUserID helper, which panics if the user ID is missing. This simplifies handler code by eliminating redundant error handling. - Tests: update auth tests to verify the panic behavior in unprotected routes. --- internal/handler/account.go | 25 +++---------------- internal/handler/file.go | 12 ++++----- internal/middleware/auth.go | 12 +++++++++ internal/middleware/auth_test.go | 43 +++++++++++++++++++++++++++++++- 4 files changed, 64 insertions(+), 28 deletions(-) diff --git a/internal/handler/account.go b/internal/handler/account.go index 67b72f9..92ae18a 100644 --- a/internal/handler/account.go +++ b/internal/handler/account.go @@ -27,22 +27,13 @@ type createPasskeyRequest struct { // GetAccount handles GET /api/v1/account. func (h *AccountHandler) GetAccount(c *gin.Context) { - userID := middleware.GetUserID(c) - if userID == "" { - api.Error(c, http.StatusUnauthorized, "unauthorized") - return - } - + userID := middleware.MustGetUserID(c) c.JSON(http.StatusOK, gin.H{"user_id": userID}) } // ListPasskeys handles GET /api/v1/account/passkeys. func (h *AccountHandler) ListPasskeys(c *gin.Context) { - userID := middleware.GetUserID(c) - if userID == "" { - api.Error(c, http.StatusUnauthorized, "unauthorized") - return - } + userID := middleware.MustGetUserID(c) creds, err := h.authService.ListPasskeys(c.Request.Context(), userID) if err != nil { @@ -59,11 +50,7 @@ func (h *AccountHandler) ListPasskeys(c *gin.Context) { // CreatePasskey handles POST /api/v1/account/passkeys. func (h *AccountHandler) CreatePasskey(c *gin.Context) { - userID := middleware.GetUserID(c) - if userID == "" { - api.Error(c, http.StatusUnauthorized, "unauthorized") - return - } + userID := middleware.MustGetUserID(c) var req createPasskeyRequest if err := c.ShouldBindJSON(&req); err != nil { @@ -82,11 +69,7 @@ func (h *AccountHandler) CreatePasskey(c *gin.Context) { // RevokePasskey handles DELETE /api/v1/account/passkeys/:id. func (h *AccountHandler) RevokePasskey(c *gin.Context) { - userID := middleware.GetUserID(c) - if userID == "" { - api.Error(c, http.StatusUnauthorized, "unauthorized") - return - } + userID := middleware.MustGetUserID(c) passkeyID := c.Param("id") if passkeyID == "" { diff --git a/internal/handler/file.go b/internal/handler/file.go index b858d12..ce420ef 100644 --- a/internal/handler/file.go +++ b/internal/handler/file.go @@ -40,7 +40,7 @@ type updateFileRequest struct { // If the content type is multipart/form-data, it uploads a file. // If the content type is application/json, it creates a directory. func (h *FileHandler) Upload(c *gin.Context) { - userID := middleware.GetUserID(c) + userID := middleware.MustGetUserID(c) contentType := c.GetHeader("Content-Type") // Directory creation via JSON. @@ -97,7 +97,7 @@ func (h *FileHandler) Upload(c *gin.Context) { // List handles GET /api/v1/files. func (h *FileHandler) List(c *gin.Context) { - userID := middleware.GetUserID(c) + userID := middleware.MustGetUserID(c) parentIDStr := c.Query("parent_id") var parentID *string @@ -131,7 +131,7 @@ func (h *FileHandler) List(c *gin.Context) { // Get handles GET /api/v1/files/:id. func (h *FileHandler) Get(c *gin.Context) { - userID := middleware.GetUserID(c) + userID := middleware.MustGetUserID(c) fileID := c.Param("id") info, err := h.fileService.Get(c.Request.Context(), userID, fileID) @@ -145,7 +145,7 @@ func (h *FileHandler) Get(c *gin.Context) { // Download handles GET /api/v1/files/:id/content. func (h *FileHandler) Download(c *gin.Context) { - userID := middleware.GetUserID(c) + userID := middleware.MustGetUserID(c) fileID := c.Param("id") reader, info, err := h.fileService.Download(c.Request.Context(), userID, fileID) @@ -170,7 +170,7 @@ func (h *FileHandler) Download(c *gin.Context) { // Update handles PUT /api/v1/files/:id. func (h *FileHandler) Update(c *gin.Context) { - userID := middleware.GetUserID(c) + userID := middleware.MustGetUserID(c) fileID := c.Param("id") var req updateFileRequest @@ -196,7 +196,7 @@ func (h *FileHandler) Update(c *gin.Context) { // Delete handles DELETE /api/v1/files/:id. func (h *FileHandler) Delete(c *gin.Context) { - userID := middleware.GetUserID(c) + userID := middleware.MustGetUserID(c) fileID := c.Param("id") if err := h.fileService.Delete(c.Request.Context(), userID, fileID); err != nil { diff --git a/internal/middleware/auth.go b/internal/middleware/auth.go index a66deb9..bd7ab05 100644 --- a/internal/middleware/auth.go +++ b/internal/middleware/auth.go @@ -49,6 +49,7 @@ func AuthRequired(jwtSecret []byte) gin.HandlerFunc { } // GetUserID extracts the user ID injected by AuthRequired. +// Returns an empty string if the key is not present (e.g., optional auth contexts). func GetUserID(c *gin.Context) string { v, _ := c.Get(userIDKey) if v == nil { @@ -56,3 +57,14 @@ func GetUserID(c *gin.Context) string { } return v.(string) } + +// MustGetUserID extracts the user ID injected by AuthRequired. +// It panics if the key is missing — this indicates a programming error +// (a handler registered without the AuthRequired middleware). +func MustGetUserID(c *gin.Context) string { + userID := GetUserID(c) + if userID == "" { + panic("user_id not found in context — is AuthRequired middleware applied?") + } + return userID +} diff --git a/internal/middleware/auth_test.go b/internal/middleware/auth_test.go index 9d3bc14..08da150 100644 --- a/internal/middleware/auth_test.go +++ b/internal/middleware/auth_test.go @@ -17,7 +17,7 @@ func setupTestRouter(secret []byte) *gin.Engine { r := gin.New() r.Use(AuthRequired(secret)) r.GET("/protected", func(c *gin.Context) { - c.JSON(http.StatusOK, gin.H{"user_id": GetUserID(c)}) + c.JSON(http.StatusOK, gin.H{"user_id": MustGetUserID(c)}) }) return r } @@ -137,6 +137,47 @@ func TestGetUserID(t *testing.T) { } } +func TestMustGetUserID(t *testing.T) { + secret := []byte("test-secret") + token, err := auth.GenerateAccessToken("bob-99", secret, 15*time.Minute) + if err != nil { + t.Fatalf("GenerateAccessToken = %v", err) + } + + r := setupTestRouter(secret) + req := httptest.NewRequest(http.MethodGet, "/protected", nil) + req.Header.Set("Authorization", "Bearer "+token) + rec := httptest.NewRecorder() + r.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d", rec.Code) + } + + body := rec.Body.String() + if !strings.Contains(body, "bob-99") { + t.Errorf("response body %q does not contain user id", body) + } +} + +func TestMustGetUserIDPanics(t *testing.T) { + gin.SetMode(gin.TestMode) + r := gin.New() + r.GET("/naked", func(c *gin.Context) { + MustGetUserID(c) // should panic — no AuthRequired middleware applied + }) + + req := httptest.NewRequest(http.MethodGet, "/naked", nil) + rec := httptest.NewRecorder() + + defer func() { + if r := recover(); r == nil { + t.Error("expected panic, got none") + } + }() + r.ServeHTTP(rec, req) +} + func TestAuthRequiredWrongSecret(t *testing.T) { secret := []byte("test-secret") token, err := auth.GenerateAccessToken("user-1", secret, 15*time.Minute)