shishantbiswas/coder
1
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2026-06-02 20:48:20 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: trim whitespace from API tokens (#19814)

Browse Source
This commit is contained in:
Thomas Kosiewski
2025-09-15 10:02:10 +02:00
committed by GitHub
parent 6a9b896f5b
commit d238480c7a
3 changed files with 13 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files
coderd/httpmw/apikey.go
+3 -2
View File
@@ -730,13 +730,14 @@ func APITokenFromRequest(r *http.Request) string {
// Check Authorization: Bearer <token> header (case-insensitive per RFC 6750)
authHeader := r.Header.Get("Authorization")
if strings.HasPrefix(strings.ToLower(authHeader), "bearer ") {
return authHeader[7:] // Skip "Bearer " (7 characters)
// Skip "Bearer " (7 characters) and trim surrounding whitespace
return strings.TrimSpace(authHeader[7:])
}
// Check access_token query parameter
accessToken := r.URL.Query().Get("access_token")
if accessToken != "" {
return accessToken
return strings.TrimSpace(accessToken)
}
return ""
coderd/httpmw/rfc6750_extended_test.go
+8 -24
View File
@@ -20,6 +20,8 @@ import (
)
// TestOAuth2BearerTokenSecurityBoundaries tests RFC 6750 security boundaries
//
//nolint:tparallel,paralleltest // Subtests share a DB; run sequentially to avoid Windows DB cleanup flake.
func TestOAuth2BearerTokenSecurityBoundaries(t *testing.T) {
t.Parallel()
@@ -41,8 +43,6 @@ func TestOAuth2BearerTokenSecurityBoundaries(t *testing.T) {
})
t.Run("TokenIsolation", func(t *testing.T) {
t.Parallel()
// Create middleware
middleware := httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{
DB: db,
@@ -78,8 +78,6 @@ func TestOAuth2BearerTokenSecurityBoundaries(t *testing.T) {
})
t.Run("CrossTokenAttempts", func(t *testing.T) {
t.Parallel()
middleware := httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{
DB: db,
})
@@ -101,8 +99,6 @@ func TestOAuth2BearerTokenSecurityBoundaries(t *testing.T) {
})
t.Run("TimingAttackResistance", func(t *testing.T) {
t.Parallel()
middleware := httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{
DB: db,
})
@@ -150,6 +146,8 @@ func TestOAuth2BearerTokenSecurityBoundaries(t *testing.T) {
}
// TestOAuth2BearerTokenMalformedHeaders tests handling of malformed Bearer headers per RFC 6750
//
//nolint:tparallel,paralleltest // Subtests share a DB; run sequentially to avoid Windows DB cleanup flake.
func TestOAuth2BearerTokenMalformedHeaders(t *testing.T) {
t.Parallel()
@@ -215,8 +213,6 @@ func TestOAuth2BearerTokenMalformedHeaders(t *testing.T) {
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
t.Parallel()
req := httptest.NewRequest("GET", "/test", nil)
req.Header.Set("Authorization", test.authHeader)
rec := httptest.NewRecorder()
@@ -234,6 +230,8 @@ func TestOAuth2BearerTokenMalformedHeaders(t *testing.T) {
}
// TestOAuth2BearerTokenPrecedence tests token extraction precedence per RFC 6750
//
//nolint:tparallel,paralleltest // Subtests share a DB; run sequentially to avoid Windows DB cleanup flake.
func TestOAuth2BearerTokenPrecedence(t *testing.T) {
t.Parallel()
@@ -257,8 +255,6 @@ func TestOAuth2BearerTokenPrecedence(t *testing.T) {
}))
t.Run("CookieTakesPrecedenceOverBearer", func(t *testing.T) {
t.Parallel()
req := httptest.NewRequest("GET", "/test", nil)
// Set both cookie and Bearer header - cookie should take precedence
req.AddCookie(&http.Cookie{
@@ -274,8 +270,6 @@ func TestOAuth2BearerTokenPrecedence(t *testing.T) {
})
t.Run("QueryParameterTakesPrecedenceOverBearer", func(t *testing.T) {
t.Parallel()
// Set both query parameter and Bearer header - query should take precedence
u, _ := url.Parse("/test")
q := u.Query()
@@ -292,8 +286,6 @@ func TestOAuth2BearerTokenPrecedence(t *testing.T) {
})
t.Run("BearerHeaderFallback", func(t *testing.T) {
t.Parallel()
// Only set Bearer header - should be used as fallback
req := httptest.NewRequest("GET", "/test", nil)
req.Header.Set("Authorization", "Bearer "+validToken)
@@ -305,8 +297,6 @@ func TestOAuth2BearerTokenPrecedence(t *testing.T) {
})
t.Run("AccessTokenQueryParameterFallback", func(t *testing.T) {
t.Parallel()
// Only set access_token query parameter - should be used as fallback
u, _ := url.Parse("/test")
q := u.Query()
@@ -322,8 +312,6 @@ func TestOAuth2BearerTokenPrecedence(t *testing.T) {
})
t.Run("MultipleAuthMethodsShouldNotConflict", func(t *testing.T) {
t.Parallel()
// RFC 6750 says clients shouldn't send tokens in multiple ways,
// but if they do, we should handle it gracefully by using precedence
u, _ := url.Parse("/test")
@@ -348,6 +336,8 @@ func TestOAuth2BearerTokenPrecedence(t *testing.T) {
}
// TestOAuth2WWWAuthenticateCompliance tests WWW-Authenticate header compliance with RFC 6750
//
//nolint:tparallel,paralleltest // Subtests share a DB; run sequentially to avoid Windows DB cleanup flake.
func TestOAuth2WWWAuthenticateCompliance(t *testing.T) {
t.Parallel()
@@ -363,8 +353,6 @@ func TestOAuth2WWWAuthenticateCompliance(t *testing.T) {
}))
t.Run("UnauthorizedResponse", func(t *testing.T) {
t.Parallel()
req := httptest.NewRequest("GET", "/test", nil)
req.Header.Set("Authorization", "Bearer invalid-token")
rec := httptest.NewRecorder()
@@ -383,8 +371,6 @@ func TestOAuth2WWWAuthenticateCompliance(t *testing.T) {
})
t.Run("ExpiredTokenResponse", func(t *testing.T) {
t.Parallel()
// Create an expired API key
_, expiredToken := dbgen.APIKey(t, db, database.APIKey{
UserID: user.ID,
@@ -406,8 +392,6 @@ func TestOAuth2WWWAuthenticateCompliance(t *testing.T) {
})
t.Run("InsufficientScopeResponse", func(t *testing.T) {
t.Parallel()
// For this test, we'll test with an invalid token to trigger the middleware's
// error handling which does set WWW-Authenticate headers for 403 responses
// In practice, insufficient scope errors would be handled by RBAC middleware
coderd/httpmw/rfc6750_test.go
+2 -12
View File
@@ -19,6 +19,8 @@ import (
// TestRFC6750BearerTokenAuthentication tests that RFC 6750 bearer tokens work correctly
// for authentication, including both Authorization header and access_token query parameter methods.
//
//nolint:tparallel,paralleltest // Subtests share a DB; run sequentially to avoid Windows DB cleanup flake.
func TestRFC6750BearerTokenAuthentication(t *testing.T) {
t.Parallel()
@@ -44,8 +46,6 @@ func TestRFC6750BearerTokenAuthentication(t *testing.T) {
})
t.Run("AuthorizationBearerHeader", func(t *testing.T) {
t.Parallel()
req := httptest.NewRequest("GET", "/test", nil)
req.Header.Set("Authorization", "Bearer "+token)
@@ -57,8 +57,6 @@ func TestRFC6750BearerTokenAuthentication(t *testing.T) {
})
t.Run("AccessTokenQueryParameter", func(t *testing.T) {
t.Parallel()
req := httptest.NewRequest("GET", "/test?access_token="+url.QueryEscape(token), nil)
rw := httptest.NewRecorder()
@@ -69,8 +67,6 @@ func TestRFC6750BearerTokenAuthentication(t *testing.T) {
})
t.Run("BearerTokenPriorityAfterCustomMethods", func(t *testing.T) {
t.Parallel()
// Create a different token for custom header
customKey, customToken := dbgen.APIKey(t, db, database.APIKey{
UserID: user.ID,
@@ -98,8 +94,6 @@ func TestRFC6750BearerTokenAuthentication(t *testing.T) {
})
t.Run("InvalidBearerToken", func(t *testing.T) {
t.Parallel()
req := httptest.NewRequest("GET", "/test", nil)
req.Header.Set("Authorization", "Bearer invalid-token")
@@ -118,8 +112,6 @@ func TestRFC6750BearerTokenAuthentication(t *testing.T) {
})
t.Run("ExpiredBearerToken", func(t *testing.T) {
t.Parallel()
// Create an expired token
_, expiredToken := dbgen.APIKey(t, db, database.APIKey{
UserID: user.ID,
@@ -144,8 +136,6 @@ func TestRFC6750BearerTokenAuthentication(t *testing.T) {
})
t.Run("MissingBearerToken", func(t *testing.T) {
t.Parallel()
req := httptest.NewRequest("GET", "/test", nil)
// No authentication provided