fix: disallow API key renewal and creation with API key authentication (#1334)

backend/internal/common/errors.go

@@ -280,6 +280,13 @@ func (e *APIKeyExpirationDateError) Error() string {
 }
 func (e *APIKeyExpirationDateError) HttpStatusCode() int { return http.StatusBadRequest }
 
+type APIKeyAuthNotAllowedError struct{}
+
+func (e *APIKeyAuthNotAllowedError) Error() string {
+	return "API key authentication is not allowed for this endpoint"
+}
+func (e *APIKeyAuthNotAllowedError) HttpStatusCode() int { return http.StatusForbidden }
+
 type OidcInvalidRefreshTokenError struct{}
 
 func (e *OidcInvalidRefreshTokenError) Error() string {

backend/internal/controller/api_key_controller.go

@@ -26,12 +26,11 @@ func NewApiKeyController(group *gin.RouterGroup, authMiddleware *middleware.Auth
 	uc := &ApiKeyController{apiKeyService: apiKeyService}
 
 	apiKeyGroup := group.Group("/api-keys")
-	apiKeyGroup.Use(authMiddleware.WithAdminNotRequired().Add())
 	{
-		apiKeyGroup.GET("", uc.listApiKeysHandler)
+		apiKeyGroup.GET("", authMiddleware.WithAdminNotRequired().Add(), uc.listApiKeysHandler)
-		apiKeyGroup.POST("", uc.createApiKeyHandler)
+		apiKeyGroup.POST("", authMiddleware.WithAdminNotRequired().WithApiKeyAuthDisabled().Add(), uc.createApiKeyHandler)
-		apiKeyGroup.POST("/:id/renew", uc.renewApiKeyHandler)
+		apiKeyGroup.POST("/:id/renew", authMiddleware.WithAdminNotRequired().WithApiKeyAuthDisabled().Add(), uc.renewApiKeyHandler)
-		apiKeyGroup.DELETE("/:id", uc.revokeApiKeyHandler)
+		apiKeyGroup.DELETE("/:id", authMiddleware.WithAdminNotRequired().Add(), uc.revokeApiKeyHandler)
 	}
 }
 

backend/internal/middleware/auth_middleware.go

@@ -18,6 +18,7 @@ type AuthMiddleware struct {
 type AuthOptions struct {
 	AdminRequired   bool
 	SuccessOptional bool
+	AllowApiKeyAuth bool
 }
 
 func NewAuthMiddleware(
@@ -31,6 +32,7 @@ func NewAuthMiddleware(
 		options: AuthOptions{
 			AdminRequired:   true,
 			SuccessOptional: false,
+			AllowApiKeyAuth: true,
 		},
 	}
 }
@@ -59,6 +61,17 @@ func (m *AuthMiddleware) WithSuccessOptional() *AuthMiddleware {
 	return clone
 }
 
+// WithApiKeyAuthDisabled disables API key authentication fallback and requires JWT auth.
+func (m *AuthMiddleware) WithApiKeyAuthDisabled() *AuthMiddleware {
+	clone := &AuthMiddleware{
+		apiKeyMiddleware: m.apiKeyMiddleware,
+		jwtMiddleware:    m.jwtMiddleware,
+		options:          m.options,
+	}
+	clone.options.AllowApiKeyAuth = false
+	return clone
+}
+
 func (m *AuthMiddleware) Add() gin.HandlerFunc {
 	return func(c *gin.Context) {
 		userID, isAdmin, err := m.jwtMiddleware.Verify(c, m.options.AdminRequired)
@@ -79,6 +92,21 @@ func (m *AuthMiddleware) Add() gin.HandlerFunc {
 			return
 		}
 
+		if !m.options.AllowApiKeyAuth {
+			if m.options.SuccessOptional {
+				c.Next()
+				return
+			}
+
+			c.Abort()
+			if c.GetHeader("X-API-Key") != "" {
+				_ = c.Error(&common.APIKeyAuthNotAllowedError{})
+				return
+			}
+			_ = c.Error(err)
+			return
+		}
+
 		// JWT auth failed, try API key auth
 		userID, isAdmin, err = m.apiKeyMiddleware.Verify(c, m.options.AdminRequired)
 		if err == nil {

backend/internal/middleware/auth_middleware_test.go

@@ -0,0 +1,104 @@
+package middleware
+
+import (
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+	"time"
+
+	"github.com/gin-gonic/gin"
+	"github.com/stretchr/testify/require"
+	"gorm.io/gorm"
+
+	"github.com/pocket-id/pocket-id/backend/internal/common"
+	"github.com/pocket-id/pocket-id/backend/internal/dto"
+	"github.com/pocket-id/pocket-id/backend/internal/model"
+	datatype "github.com/pocket-id/pocket-id/backend/internal/model/types"
+	"github.com/pocket-id/pocket-id/backend/internal/service"
+	testutils "github.com/pocket-id/pocket-id/backend/internal/utils/testing"
+)
+
+func TestWithApiKeyAuthDisabled(t *testing.T) {
+	gin.SetMode(gin.TestMode)
+
+	originalEnvConfig := common.EnvConfig
+	defer func() {
+		common.EnvConfig = originalEnvConfig
+	}()
+	common.EnvConfig.AppURL = "https://test.example.com"
+	common.EnvConfig.EncryptionKey = []byte("0123456789abcdef0123456789abcdef")
+
+	db := testutils.NewDatabaseForTest(t)
+
+	appConfigService, err := service.NewAppConfigService(t.Context(), db)
+	require.NoError(t, err)
+
+	jwtService, err := service.NewJwtService(t.Context(), db, appConfigService)
+	require.NoError(t, err)
+
+	userService := service.NewUserService(db, jwtService, nil, nil, appConfigService, nil, nil, nil, nil)
+	apiKeyService, err := service.NewApiKeyService(t.Context(), db, nil)
+	require.NoError(t, err)
+
+	authMiddleware := NewAuthMiddleware(apiKeyService, userService, jwtService)
+
+	user := createUserForAuthMiddlewareTest(t, db)
+	jwtToken, err := jwtService.GenerateAccessToken(user)
+	require.NoError(t, err)
+
+	_, apiKeyToken, err := apiKeyService.CreateApiKey(t.Context(), user.ID, dto.ApiKeyCreateDto{
+		Name:      "Middleware API Key",
+		ExpiresAt: datatype.DateTime(time.Now().Add(24 * time.Hour)),
+	})
+	require.NoError(t, err)
+
+	router := gin.New()
+	router.Use(NewErrorHandlerMiddleware().Add())
+	router.GET("/api/protected", authMiddleware.WithAdminNotRequired().WithApiKeyAuthDisabled().Add(), func(c *gin.Context) {
+		c.Status(http.StatusNoContent)
+	})
+
+	t.Run("rejects API key auth when API key auth is disabled", func(t *testing.T) {
+		req := httptest.NewRequest(http.MethodGet, "/api/protected", nil)
+		req.Header.Set("X-API-Key", apiKeyToken)
+		recorder := httptest.NewRecorder()
+
+		router.ServeHTTP(recorder, req)
+
+		require.Equal(t, http.StatusForbidden, recorder.Code)
+
+		var body map[string]string
+		err := json.Unmarshal(recorder.Body.Bytes(), &body)
+		require.NoError(t, err)
+		require.Equal(t, "API key authentication is not allowed for this endpoint", body["error"])
+	})
+
+	t.Run("allows JWT auth when API key auth is disabled", func(t *testing.T) {
+		req := httptest.NewRequest(http.MethodGet, "/api/protected", nil)
+		req.Header.Set("Authorization", "Bearer "+jwtToken)
+		recorder := httptest.NewRecorder()
+
+		router.ServeHTTP(recorder, req)
+
+		require.Equal(t, http.StatusNoContent, recorder.Code)
+	})
+}
+
+func createUserForAuthMiddlewareTest(t *testing.T, db *gorm.DB) model.User {
+	t.Helper()
+
+	email := "auth@example.com"
+	user := model.User{
+		Username:    "auth-user",
+		Email:       &email,
+		FirstName:   "Auth",
+		LastName:    "User",
+		DisplayName: "Auth User",
+	}
+
+	err := db.Create(&user).Error
+	require.NoError(t, err)
+
+	return user
+}

backend/internal/service/api_key_service.go

@@ -77,6 +77,9 @@ func (s *ApiKeyService) CreateApiKey(ctx context.Context, userID string, input d
 		Create(&apiKey).
 		Error
 	if err != nil {
+		if errors.Is(err, gorm.ErrDuplicatedKey) {
+			return model.ApiKey{}, "", &common.AlreadyInUseError{Property: "API key name"}
+		}
 		return model.ApiKey{}, "", err
 	}