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.
This commit is contained in:
@@ -27,22 +27,13 @@ type createPasskeyRequest struct {
|
|||||||
|
|
||||||
// GetAccount handles GET /api/v1/account.
|
// GetAccount handles GET /api/v1/account.
|
||||||
func (h *AccountHandler) GetAccount(c *gin.Context) {
|
func (h *AccountHandler) GetAccount(c *gin.Context) {
|
||||||
userID := middleware.GetUserID(c)
|
userID := middleware.MustGetUserID(c)
|
||||||
if userID == "" {
|
|
||||||
api.Error(c, http.StatusUnauthorized, "unauthorized")
|
|
||||||
return
|
|
||||||
}
|
|
||||||
|
|
||||||
c.JSON(http.StatusOK, gin.H{"user_id": userID})
|
c.JSON(http.StatusOK, gin.H{"user_id": userID})
|
||||||
}
|
}
|
||||||
|
|
||||||
// ListPasskeys handles GET /api/v1/account/passkeys.
|
// ListPasskeys handles GET /api/v1/account/passkeys.
|
||||||
func (h *AccountHandler) ListPasskeys(c *gin.Context) {
|
func (h *AccountHandler) ListPasskeys(c *gin.Context) {
|
||||||
userID := middleware.GetUserID(c)
|
userID := middleware.MustGetUserID(c)
|
||||||
if userID == "" {
|
|
||||||
api.Error(c, http.StatusUnauthorized, "unauthorized")
|
|
||||||
return
|
|
||||||
}
|
|
||||||
|
|
||||||
creds, err := h.authService.ListPasskeys(c.Request.Context(), userID)
|
creds, err := h.authService.ListPasskeys(c.Request.Context(), userID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -59,11 +50,7 @@ func (h *AccountHandler) ListPasskeys(c *gin.Context) {
|
|||||||
|
|
||||||
// CreatePasskey handles POST /api/v1/account/passkeys.
|
// CreatePasskey handles POST /api/v1/account/passkeys.
|
||||||
func (h *AccountHandler) CreatePasskey(c *gin.Context) {
|
func (h *AccountHandler) CreatePasskey(c *gin.Context) {
|
||||||
userID := middleware.GetUserID(c)
|
userID := middleware.MustGetUserID(c)
|
||||||
if userID == "" {
|
|
||||||
api.Error(c, http.StatusUnauthorized, "unauthorized")
|
|
||||||
return
|
|
||||||
}
|
|
||||||
|
|
||||||
var req createPasskeyRequest
|
var req createPasskeyRequest
|
||||||
if err := c.ShouldBindJSON(&req); err != nil {
|
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.
|
// RevokePasskey handles DELETE /api/v1/account/passkeys/:id.
|
||||||
func (h *AccountHandler) RevokePasskey(c *gin.Context) {
|
func (h *AccountHandler) RevokePasskey(c *gin.Context) {
|
||||||
userID := middleware.GetUserID(c)
|
userID := middleware.MustGetUserID(c)
|
||||||
if userID == "" {
|
|
||||||
api.Error(c, http.StatusUnauthorized, "unauthorized")
|
|
||||||
return
|
|
||||||
}
|
|
||||||
|
|
||||||
passkeyID := c.Param("id")
|
passkeyID := c.Param("id")
|
||||||
if passkeyID == "" {
|
if passkeyID == "" {
|
||||||
|
|||||||
@@ -40,7 +40,7 @@ type updateFileRequest struct {
|
|||||||
// If the content type is multipart/form-data, it uploads a file.
|
// If the content type is multipart/form-data, it uploads a file.
|
||||||
// If the content type is application/json, it creates a directory.
|
// If the content type is application/json, it creates a directory.
|
||||||
func (h *FileHandler) Upload(c *gin.Context) {
|
func (h *FileHandler) Upload(c *gin.Context) {
|
||||||
userID := middleware.GetUserID(c)
|
userID := middleware.MustGetUserID(c)
|
||||||
contentType := c.GetHeader("Content-Type")
|
contentType := c.GetHeader("Content-Type")
|
||||||
|
|
||||||
// Directory creation via JSON.
|
// Directory creation via JSON.
|
||||||
@@ -97,7 +97,7 @@ func (h *FileHandler) Upload(c *gin.Context) {
|
|||||||
|
|
||||||
// List handles GET /api/v1/files.
|
// List handles GET /api/v1/files.
|
||||||
func (h *FileHandler) List(c *gin.Context) {
|
func (h *FileHandler) List(c *gin.Context) {
|
||||||
userID := middleware.GetUserID(c)
|
userID := middleware.MustGetUserID(c)
|
||||||
|
|
||||||
parentIDStr := c.Query("parent_id")
|
parentIDStr := c.Query("parent_id")
|
||||||
var parentID *string
|
var parentID *string
|
||||||
@@ -131,7 +131,7 @@ func (h *FileHandler) List(c *gin.Context) {
|
|||||||
|
|
||||||
// Get handles GET /api/v1/files/:id.
|
// Get handles GET /api/v1/files/:id.
|
||||||
func (h *FileHandler) Get(c *gin.Context) {
|
func (h *FileHandler) Get(c *gin.Context) {
|
||||||
userID := middleware.GetUserID(c)
|
userID := middleware.MustGetUserID(c)
|
||||||
fileID := c.Param("id")
|
fileID := c.Param("id")
|
||||||
|
|
||||||
info, err := h.fileService.Get(c.Request.Context(), userID, fileID)
|
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.
|
// Download handles GET /api/v1/files/:id/content.
|
||||||
func (h *FileHandler) Download(c *gin.Context) {
|
func (h *FileHandler) Download(c *gin.Context) {
|
||||||
userID := middleware.GetUserID(c)
|
userID := middleware.MustGetUserID(c)
|
||||||
fileID := c.Param("id")
|
fileID := c.Param("id")
|
||||||
|
|
||||||
reader, info, err := h.fileService.Download(c.Request.Context(), userID, fileID)
|
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.
|
// Update handles PUT /api/v1/files/:id.
|
||||||
func (h *FileHandler) Update(c *gin.Context) {
|
func (h *FileHandler) Update(c *gin.Context) {
|
||||||
userID := middleware.GetUserID(c)
|
userID := middleware.MustGetUserID(c)
|
||||||
fileID := c.Param("id")
|
fileID := c.Param("id")
|
||||||
|
|
||||||
var req updateFileRequest
|
var req updateFileRequest
|
||||||
@@ -196,7 +196,7 @@ func (h *FileHandler) Update(c *gin.Context) {
|
|||||||
|
|
||||||
// Delete handles DELETE /api/v1/files/:id.
|
// Delete handles DELETE /api/v1/files/:id.
|
||||||
func (h *FileHandler) Delete(c *gin.Context) {
|
func (h *FileHandler) Delete(c *gin.Context) {
|
||||||
userID := middleware.GetUserID(c)
|
userID := middleware.MustGetUserID(c)
|
||||||
fileID := c.Param("id")
|
fileID := c.Param("id")
|
||||||
|
|
||||||
if err := h.fileService.Delete(c.Request.Context(), userID, fileID); err != nil {
|
if err := h.fileService.Delete(c.Request.Context(), userID, fileID); err != nil {
|
||||||
|
|||||||
@@ -49,6 +49,7 @@ func AuthRequired(jwtSecret []byte) gin.HandlerFunc {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// GetUserID extracts the user ID injected by AuthRequired.
|
// 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 {
|
func GetUserID(c *gin.Context) string {
|
||||||
v, _ := c.Get(userIDKey)
|
v, _ := c.Get(userIDKey)
|
||||||
if v == nil {
|
if v == nil {
|
||||||
@@ -56,3 +57,14 @@ func GetUserID(c *gin.Context) string {
|
|||||||
}
|
}
|
||||||
return v.(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
|
||||||
|
}
|
||||||
|
|||||||
@@ -17,7 +17,7 @@ func setupTestRouter(secret []byte) *gin.Engine {
|
|||||||
r := gin.New()
|
r := gin.New()
|
||||||
r.Use(AuthRequired(secret))
|
r.Use(AuthRequired(secret))
|
||||||
r.GET("/protected", func(c *gin.Context) {
|
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
|
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) {
|
func TestAuthRequiredWrongSecret(t *testing.T) {
|
||||||
secret := []byte("test-secret")
|
secret := []byte("test-secret")
|
||||||
token, err := auth.GenerateAccessToken("user-1", secret, 15*time.Minute)
|
token, err := auth.GenerateAccessToken("user-1", secret, 15*time.Minute)
|
||||||
|
|||||||
Reference in New Issue
Block a user