Skip to content

Commit 7337e21

Browse files
cemalkiliccemalkilic
authored
fix: return JSON response for unmatched routes instead of plain text (#2457)
## What kind of change does this PR introduce? Fix ## Summary Routes behind feature flags (e.g. OAuthServer, CustomOAuth) return chi's default plain text "404 page not found" when disabled. Register custom NotFound and MethodNotAllowed handlers on the router so all unmatched routes return structured JSON errors consistent with the rest of the API. Fixes inconsistent `content-type: text/plain` responses when feature-gated routes (OAuthServer, CustomOAuth) are disabled ## Test plan - [x] Hit a disabled route (e.g. `/oauth/clients/register` with `OAuthServer.Enabled=false`) and verify JSON 404 response with `error_code: "route_not_found"`
1 parent 2af904a commit 7337e21

6 files changed

Lines changed: 142 additions & 66 deletions

File tree

‎internal/api/api.go‎

Lines changed: 50 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -200,9 +200,7 @@ func NewAPIWithVersion(globalConfig *conf.GlobalConfiguration, db *storage.Conne
200200
// Both OIDC Discovery and OAuth Authorization Server Metadata use the same unified handler
201201
// OIDC Discovery is an extension of RFC 8414, so one response satisfies both specs
202202
r.Get("/.well-known/openid-configuration", api.WellKnownOpenID)
203-
if globalConfig.OAuthServer.Enabled {
204-
r.Get("/.well-known/oauth-authorization-server", api.WellKnownOpenID)
205-
}
203+
r.With(api.requireOAuthServerEnabled).Get("/.well-known/oauth-authorization-server", api.WellKnownOpenID)
206204

207205
r.Route("/callback", func(r *router) {
208206
r.Use(api.isValidExternalHost)
@@ -285,13 +283,12 @@ func NewAPIWithVersion(globalConfig *conf.GlobalConfiguration, db *storage.Conne
285283
r.Delete("/{identity_id}", api.DeleteIdentity)
286284
})
287285

288-
// OAuth grant management endpoints (only if OAuth server is enabled)
289-
if globalConfig.OAuthServer.Enabled {
290-
r.Route("/oauth/grants", func(r *router) {
291-
r.Get("/", api.oauthServer.UserListOAuthGrants)
292-
r.Delete("/", api.oauthServer.UserRevokeOAuthGrant)
293-
})
294-
}
286+
// OAuth grant management endpoints
287+
r.Route("/oauth/grants", func(r *router) {
288+
r.Use(api.requireOAuthServerEnabled)
289+
r.Get("/", api.oauthServer.UserListOAuthGrants)
290+
r.Delete("/", api.oauthServer.UserRevokeOAuthGrant)
291+
})
295292
})
296293

297294
r.With(api.requireAuthentication).Route("/factors", func(r *router) {
@@ -397,60 +394,57 @@ func NewAPIWithVersion(globalConfig *conf.GlobalConfiguration, db *storage.Conne
397394
})
398395

399396
// Admin only oauth client management endpoints
400-
if globalConfig.OAuthServer.Enabled {
401-
r.Route("/oauth", func(r *router) {
402-
r.Route("/clients", func(r *router) {
403-
// Manual client registration
404-
r.Post("/", api.oauthServer.AdminOAuthServerClientRegister)
405-
406-
r.Get("/", api.oauthServer.OAuthServerClientList)
407-
408-
r.Route("/{client_id}", func(r *router) {
409-
r.Use(api.oauthServer.LoadOAuthServerClient)
410-
r.Get("/", api.oauthServer.OAuthServerClientGet)
411-
r.Put("/", api.oauthServer.OAuthServerClientUpdate)
412-
r.Delete("/", api.oauthServer.OAuthServerClientDelete)
413-
r.Post("/regenerate_secret", api.oauthServer.OAuthServerClientRegenerateSecret)
414-
})
397+
r.Route("/oauth", func(r *router) {
398+
r.Use(api.requireOAuthServerEnabled)
399+
r.Route("/clients", func(r *router) {
400+
// Manual client registration
401+
r.Post("/", api.oauthServer.AdminOAuthServerClientRegister)
402+
403+
r.Get("/", api.oauthServer.OAuthServerClientList)
404+
405+
r.Route("/{client_id}", func(r *router) {
406+
r.Use(api.oauthServer.LoadOAuthServerClient)
407+
r.Get("/", api.oauthServer.OAuthServerClientGet)
408+
r.Put("/", api.oauthServer.OAuthServerClientUpdate)
409+
r.Delete("/", api.oauthServer.OAuthServerClientDelete)
410+
r.Post("/regenerate_secret", api.oauthServer.OAuthServerClientRegenerateSecret)
415411
})
416412
})
417-
}
413+
})
418414

419415
// Custom OAuth/OIDC provider management endpoints
420-
if globalConfig.CustomOAuth.Enabled {
421-
r.Route("/custom-providers", func(r *router) {
422-
// supports both OAuth2 and OIDC via provider_type)
423-
r.Get("/", api.adminCustomOAuthProvidersList) // Optional ?type=oauth2 or ?type=oidc filter
424-
r.Post("/", api.adminCustomOAuthProviderCreate) // provider_type in request body
425-
426-
r.Route("/{identifier}", func(r *router) {
427-
r.Get("/", api.adminCustomOAuthProviderGet)
428-
r.Put("/", api.adminCustomOAuthProviderUpdate)
429-
r.Delete("/", api.adminCustomOAuthProviderDelete)
430-
})
416+
r.Route("/custom-providers", func(r *router) {
417+
r.Use(api.requireCustomOAuthEnabled)
418+
// supports both OAuth2 and OIDC via provider_type)
419+
r.Get("/", api.adminCustomOAuthProvidersList) // Optional ?type=oauth2 or ?type=oidc filter
420+
r.Post("/", api.adminCustomOAuthProviderCreate) // provider_type in request body
421+
422+
r.Route("/{identifier}", func(r *router) {
423+
r.Get("/", api.adminCustomOAuthProviderGet)
424+
r.Put("/", api.adminCustomOAuthProviderUpdate)
425+
r.Delete("/", api.adminCustomOAuthProviderDelete)
431426
})
432-
}
427+
})
433428
})
434429

435430
// OAuth Dynamic Client Registration endpoint (public, rate limited)
436-
if globalConfig.OAuthServer.Enabled {
437-
r.Route("/oauth", func(r *router) {
438-
r.With(api.limitHandler(api.limiterOpts.OAuthClientRegister)).
439-
Post("/clients/register", api.oauthServer.OAuthServerClientDynamicRegister)
440-
441-
// OAuth Token endpoint (public, with client authentication)
442-
r.With(api.requireOAuthClientAuth).Post("/token", api.oauthServer.OAuthToken)
443-
444-
// OIDC UserInfo endpoint (requires user authentication via Bearer token)
445-
r.With(api.requireAuthentication).Get("/userinfo", api.oauthServer.OAuthUserInfo)
446-
447-
// OAuth 2.1 Authorization endpoints
448-
// `/authorize` to initiate OAuth2 authorization code flow where Supabase Auth is the OAuth2 provider
449-
r.Get("/authorize", api.oauthServer.OAuthServerAuthorize)
450-
r.With(api.requireAuthentication).Get("/authorizations/{authorization_id}", api.oauthServer.OAuthServerGetAuthorization)
451-
r.With(api.requireAuthentication).Post("/authorizations/{authorization_id}/consent", api.oauthServer.OAuthServerConsent)
452-
})
453-
}
431+
r.Route("/oauth", func(r *router) {
432+
r.Use(api.requireOAuthServerEnabled)
433+
r.With(api.limitHandler(api.limiterOpts.OAuthClientRegister)).
434+
Post("/clients/register", api.oauthServer.OAuthServerClientDynamicRegister)
435+
436+
// OAuth Token endpoint (public, with client authentication)
437+
r.With(api.requireOAuthClientAuth).Post("/token", api.oauthServer.OAuthToken)
438+
439+
// OIDC UserInfo endpoint (requires user authentication via Bearer token)
440+
r.With(api.requireAuthentication).Get("/userinfo", api.oauthServer.OAuthUserInfo)
441+
442+
// OAuth 2.1 Authorization endpoints
443+
// `/authorize` to initiate OAuth2 authorization code flow where Supabase Auth is the OAuth2 provider
444+
r.Get("/authorize", api.oauthServer.OAuthServerAuthorize)
445+
r.With(api.requireAuthentication).Get("/authorizations/{authorization_id}", api.oauthServer.OAuthServerGetAuthorization)
446+
r.With(api.requireAuthentication).Post("/authorizations/{authorization_id}/consent", api.oauthServer.OAuthServerConsent)
447+
})
454448
})
455449

456450
corsHandler := cors.New(cors.Options{

‎internal/api/api_test.go‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,18 @@
11
package api
22

33
import (
4+
"encoding/json"
5+
"net/http"
6+
"net/http/httptest"
47
"testing"
58

69
"github.com/stretchr/testify/require"
710
"github.com/supabase/auth/internal/conf"
811
"github.com/supabase/auth/internal/crypto"
912
"github.com/supabase/auth/internal/storage"
1013
"github.com/supabase/auth/internal/storage/test"
14+
15+
"github.com/supabase/auth/internal/api/apierrors"
1116
)
1217

1318
const (
@@ -77,6 +82,66 @@ func TestOAuthServerDisabledByDefault(t *testing.T) {
7782
require.Nil(t, api.oauthServer)
7883
}
7984

85+
func TestDisabledFeatureReturnsJSON(t *testing.T) {
86+
cases := []struct {
87+
name string
88+
method string
89+
path string
90+
errorCode string
91+
setup func(*conf.GlobalConfiguration)
92+
}{
93+
{
94+
name: "OAuthServer disabled returns JSON on /oauth/token",
95+
method: http.MethodPost,
96+
path: "http://localhost/oauth/token",
97+
errorCode: apierrors.ErrorCodeFeatureDisabled,
98+
setup: func(c *conf.GlobalConfiguration) {
99+
c.OAuthServer.Enabled = false
100+
},
101+
},
102+
{
103+
name: "OAuthServer disabled returns JSON on /oauth/clients/register",
104+
method: http.MethodPost,
105+
path: "http://localhost/oauth/clients/register",
106+
errorCode: apierrors.ErrorCodeFeatureDisabled,
107+
setup: func(c *conf.GlobalConfiguration) {
108+
c.OAuthServer.Enabled = false
109+
},
110+
},
111+
{
112+
name: "OAuthServer disabled returns JSON on /.well-known/oauth-authorization-server",
113+
method: http.MethodGet,
114+
path: "http://localhost/.well-known/oauth-authorization-server",
115+
errorCode: apierrors.ErrorCodeFeatureDisabled,
116+
setup: func(c *conf.GlobalConfiguration) {
117+
c.OAuthServer.Enabled = false
118+
},
119+
},
120+
}
121+
122+
for _, tc := range cases {
123+
t.Run(tc.name, func(t *testing.T) {
124+
api, _, err := setupAPIForTestWithCallback(func(config *conf.GlobalConfiguration, conn *storage.Connection) {
125+
if config != nil {
126+
tc.setup(config)
127+
}
128+
})
129+
require.NoError(t, err)
130+
131+
req := httptest.NewRequest(tc.method, tc.path, nil)
132+
w := httptest.NewRecorder()
133+
api.handler.ServeHTTP(w, req)
134+
135+
require.Equal(t, http.StatusNotFound, w.Code)
136+
require.Contains(t, w.Header().Get("Content-Type"), "application/json")
137+
138+
var body map[string]interface{}
139+
require.NoError(t, json.NewDecoder(w.Body).Decode(&body))
140+
require.Equal(t, tc.errorCode, body["error_code"])
141+
})
142+
}
143+
}
144+
80145
func TestOAuthServerCanBeEnabled(t *testing.T) {
81146
api, _, err := setupAPIForTestWithCallback(func(config *conf.GlobalConfiguration, conn *storage.Connection) {
82147
if config != nil {

‎internal/api/custom_oauth_admin.go‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -142,11 +142,6 @@ func (a *API) adminCustomOAuthProviderCreate(w http.ResponseWriter, r *http.Requ
142142
db := a.db.WithContext(ctx)
143143
config := a.config
144144

145-
// Check if custom OAuth is enabled
146-
if !config.CustomOAuth.Enabled {
147-
return apierrors.NewBadRequestError(apierrors.ErrorCodeFeatureDisabled, "Custom OAuth/OIDC providers are not enabled")
148-
}
149-
150145
// Parse request parameters
151146
params := &AdminCustomOAuthProviderParams{}
152147
if err := retrieveRequestParams(r, params); err != nil {

‎internal/api/custom_oauth_admin_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -409,11 +409,11 @@ func (ts *CustomOAuthAdminTestSuite) TestCreateProviderFeatureDisabled() {
409409
ts.Config.CustomOAuth.Enabled = false
410410

411411
payload := ts.createTestOAuth2Payload("test")
412-
w := ts.createProvider(payload, http.StatusBadRequest)
412+
w := ts.createProvider(payload, http.StatusNotFound)
413413

414414
var apiErr apierrors.HTTPError
415415
json.NewDecoder(w.Body).Decode(&apiErr)
416-
assert.Contains(ts.T(), apiErr.Message, "not enabled")
416+
assert.Contains(ts.T(), apiErr.Message, "disabled")
417417
}
418418

419419
func (ts *CustomOAuthAdminTestSuite) TestCreateProviderDuplicateIdentifier() {

‎internal/api/middleware.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,22 @@ func (a *API) requireManualLinkingEnabled(w http.ResponseWriter, req *http.Reque
371371
return ctx, nil
372372
}
373373

374+
func (a *API) requireOAuthServerEnabled(w http.ResponseWriter, req *http.Request) (context.Context, error) {
375+
ctx := req.Context()
376+
if !a.config.OAuthServer.Enabled {
377+
return nil, apierrors.NewNotFoundError(apierrors.ErrorCodeFeatureDisabled, "OAuth server is disabled")
378+
}
379+
return ctx, nil
380+
}
381+
382+
func (a *API) requireCustomOAuthEnabled(w http.ResponseWriter, req *http.Request) (context.Context, error) {
383+
ctx := req.Context()
384+
if !a.config.CustomOAuth.Enabled {
385+
return nil, apierrors.NewNotFoundError(apierrors.ErrorCodeFeatureDisabled, "Custom OAuth providers are disabled")
386+
}
387+
return ctx, nil
388+
}
389+
374390
func (a *API) requirePasskeyEnabled(w http.ResponseWriter, req *http.Request) (context.Context, error) {
375391
ctx := req.Context()
376392
if !a.config.Passkey.Enabled {

‎internal/api/middleware_test.go‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -379,9 +379,15 @@ func (ts *MiddlewareTestSuite) TestTimeoutMiddleware() {
379379
timeoutHandler := timeoutMiddleware(ts.Config.API.MaxRequestDuration)
380380

381381
slowHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
382-
// Sleep for 1 second to simulate a slow handler which should trigger the timeout
383-
time.Sleep(1 * time.Second)
384-
ts.API.handler.ServeHTTP(w, r)
382+
// Simulate a slow handler which should trigger the timeout.
383+
// Use context-aware wait so the goroutine exits when the timeout fires,
384+
// avoiding a data race with test cleanup.
385+
select {
386+
case <-time.After(1 * time.Second):
387+
ts.API.handler.ServeHTTP(w, r)
388+
case <-r.Context().Done():
389+
return
390+
}
385391
})
386392
timeoutHandler(slowHandler).ServeHTTP(w, req)
387393
assert.Equal(ts.T(), http.StatusGatewayTimeout, w.Code)

0 commit comments

Comments
 (0)