fix(security): require Bearer prefix for admin API tokens and document auth-disabled behavior

commit: d044953e1809907c0447ccd5aa334a98c0b9a1c6
author: bnema · 8mo ago · 2026-02-01 14:32 UTC
parent: 9da36c1
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

2 files · +19 -16

M internal/adapters/in/http/admin/middleware.go +14 -8


@@ -51,30 +51,36 @@ func AuthMiddleware(
51	51		}
52	52		}
53	53
54		-	// Check if auth is enabled
	54	+	// Check if auth is enabled.
	55	+	// DESIGN NOTE: When auth is disabled, the admin API is intentionally
	56	+	// accessible without credentials. This is the expected behavior for
	57	+	// local/development setups where Gordon is not exposed to the internet.
	58	+	// Users who disable auth accept this tradeoff explicitly in their config.
	59	+	// For production deployments, auth should always be enabled.
55	60		if !authSvc.IsEnabled() {
56		-	// Auth disabled, log warning and allow all requests
57	61		log.Warn().
58	62		Str("path", r.URL.Path).
59	63		Str("method", r.Method).
60	64		Str("remote_addr", r.RemoteAddr).
61		-	Msg("auth disabled - allowing unauthenticated admin API access")
	65	+	Msg("auth disabled - allowing unauthenticated admin API access (intended for local use only)")
62	66		next.ServeHTTP(w, r)
63	67		return
64	68		}
65	69
66		-	// Extract token from Authorization header
	70	+	// Extract token from Authorization header.
	71	+	// SECURITY: Require the standard "Bearer" prefix per RFC 6750
	72	+	// to prevent accidental token exposure in non-standard formats.
67	73		auth := r.Header.Get("Authorization")
68	74		if auth == "" {
69	75		sendUnauthorized(w, "missing authorization header")
70	76		return
71	77		}
72	78
73		-	// Support both "Bearer <token>" and direct token
74		-	token := auth
75		-	if strings.HasPrefix(auth, "Bearer ") {
76		-	token = strings.TrimPrefix(auth, "Bearer ")
	79	+	if !strings.HasPrefix(auth, "Bearer ") {
	80	+	sendUnauthorized(w, "authorization header must use Bearer scheme")
	81	+	return
77	82		}
	83	+	token := strings.TrimPrefix(auth, "Bearer ")
78	84
79	85		// Validate token
80	86		claims, err := authSvc.ValidateToken(ctx, token)

 				}
 			}
-			// Check if auth is enabled
+			// Check if auth is enabled.
+			// DESIGN NOTE: When auth is disabled, the admin API is intentionally
+			// accessible without credentials. This is the expected behavior for
+			// local/development setups where Gordon is not exposed to the internet.
+			// Users who disable auth accept this tradeoff explicitly in their config.
+			// For production deployments, auth should always be enabled.
 			if !authSvc.IsEnabled() {
-				// Auth disabled, log warning and allow all requests
 				log.Warn().
 					Str("path", r.URL.Path).
 					Str("method", r.Method).
 					Str("remote_addr", r.RemoteAddr).
-					Msg("auth disabled - allowing unauthenticated admin API access")
+					Msg("auth disabled - allowing unauthenticated admin API access (intended for local use only)")
 				next.ServeHTTP(w, r)
 				return
 			}
-			// Extract token from Authorization header
+			// Extract token from Authorization header.
+			// SECURITY: Require the standard "Bearer" prefix per RFC 6750
+			// to prevent accidental token exposure in non-standard formats.
 			auth := r.Header.Get("Authorization")
 			if auth == "" {
 				sendUnauthorized(w, "missing authorization header")
 				return
 			}
-			// Support both "Bearer <token>" and direct token
+			if !strings.HasPrefix(auth, "Bearer ") {
-			token := auth
+				sendUnauthorized(w, "authorization header must use Bearer scheme")
-			if strings.HasPrefix(auth, "Bearer ") {
+				return
-				token = strings.TrimPrefix(auth, "Bearer ")
 			}
+			token := strings.TrimPrefix(auth, "Bearer ")
 			// Validate token
 			claims, err := authSvc.ValidateToken(ctx, token)

M internal/adapters/in/http/admin/middleware_test.go +5 -8


@@ -261,7 +261,7 @@ func TestAuthMiddleware_Success(t *testing.T) {
261	261		assert.Equal(t, "admin", GetSubject(capturedCtx))
262	262		}
263	263
264		-	func TestAuthMiddleware_DirectToken(t *testing.T) {
	264	+	func TestAuthMiddleware_DirectToken_Rejected(t *testing.T) {
265	265		authSvc := inmocks.NewMockAuthService(t)
266	266		globalLimiter := outmocks.NewMockRateLimiter(t)
267	267		ipLimiter := outmocks.NewMockRateLimiter(t)

@@ -269,11 +269,6 @@ func TestAuthMiddleware_DirectToken(t *testing.T) {
269	269		globalLimiter.EXPECT().Allow(context.Background(), "global").Return(true)
270	270		ipLimiter.EXPECT().Allow(context.Background(), "ip:192.168.1.100").Return(true)
271	271		authSvc.EXPECT().IsEnabled().Return(true)
272		-	// Token without "Bearer " prefix
273		-	authSvc.EXPECT().ValidateToken(mock.Anything, "direct-token").Return(&domain.TokenClaims{
274		-	Subject: "admin",
275		-	Scopes: []string{"admin:routes:read"},
276		-	}, nil)
277	272
278	273		handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
279	274		w.WriteHeader(http.StatusOK)

@@ -282,13 +277,15 @@ func TestAuthMiddleware_DirectToken(t *testing.T) {
282	277		mw := AuthMiddleware(authSvc, globalLimiter, ipLimiter, nil, adminTestLogger())
283	278		wrappedHandler := mw(handler)
284	279
	280	+	// Token without "Bearer " prefix should be rejected (RFC 6750)
285	281		req := httptest.NewRequest(http.MethodGet, "/admin/status", nil)
286	282		req.RemoteAddr = "192.168.1.100:12345"
287		-	req.Header.Set("Authorization", "direct-token") // No "Bearer " prefix
	283	+	req.Header.Set("Authorization", "direct-token")
288	284		rec := httptest.NewRecorder()
289	285		wrappedHandler.ServeHTTP(rec, req)
290	286
291		-	assert.Equal(t, http.StatusOK, rec.Code)
	287	+	assert.Equal(t, http.StatusUnauthorized, rec.Code)
	288	+	assert.Contains(t, rec.Body.String(), "Bearer scheme")
292	289		}
293	290
294	291		func TestRequireScope(t *testing.T) {

 	assert.Equal(t, "admin", GetSubject(capturedCtx))
 }
-func TestAuthMiddleware_DirectToken(t *testing.T) {
+func TestAuthMiddleware_DirectToken_Rejected(t *testing.T) {
 	authSvc := inmocks.NewMockAuthService(t)
 	globalLimiter := outmocks.NewMockRateLimiter(t)
 	ipLimiter := outmocks.NewMockRateLimiter(t)
 	globalLimiter.EXPECT().Allow(context.Background(), "global").Return(true)
 	ipLimiter.EXPECT().Allow(context.Background(), "ip:192.168.1.100").Return(true)
 	authSvc.EXPECT().IsEnabled().Return(true)
-	// Token without "Bearer " prefix
-	authSvc.EXPECT().ValidateToken(mock.Anything, "direct-token").Return(&domain.TokenClaims{
-		Subject: "admin",
-		Scopes:  []string{"admin:routes:read"},
-	}, nil)
 	handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		w.WriteHeader(http.StatusOK)
 	mw := AuthMiddleware(authSvc, globalLimiter, ipLimiter, nil, adminTestLogger())
 	wrappedHandler := mw(handler)
+	// Token without "Bearer " prefix should be rejected (RFC 6750)
 	req := httptest.NewRequest(http.MethodGet, "/admin/status", nil)
 	req.RemoteAddr = "192.168.1.100:12345"
-	req.Header.Set("Authorization", "direct-token") // No "Bearer " prefix
+	req.Header.Set("Authorization", "direct-token")
 	rec := httptest.NewRecorder()
 	wrappedHandler.ServeHTTP(rec, req)
-	assert.Equal(t, http.StatusOK, rec.Code)
+	assert.Equal(t, http.StatusUnauthorized, rec.Code)
+	assert.Contains(t, rec.Body.String(), "Bearer scheme")
 }
 func TestRequireScope(t *testing.T) {