Skip to content

Commit af51749

Browse files
committed
Revert "feat: Add Sb-Forwarded-For header and IP-based rate limiting (#2295)"
This reverts commit e8f679b.
1 parent e5d11c2 commit af51749

9 files changed

Lines changed: 7 additions & 613 deletions

File tree

README.md

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -888,12 +888,6 @@ Enforce reauthentication on password update.
888888

889889
Use this to enable/disable anonymous sign-ins.
890890

891-
### IP address forwarding
892-
893-
`GOTRUE_SECURITY_SB_FORWARDED_FOR_ENABLED` - `bool`
894-
895-
Enable IP address forwarding using the `Sb-Forwarded-For` HTTP request header. When enabled, Auth will parse the first value of this header as an IP address and use it for IP address tracking and rate limiting. Make sure this header is fully trusted before enabling this feature by only passing it from trustworthy clients or proxies.
896-
897891
## Endpoints
898892

899893
Auth exposes the following endpoints:

internal/api/api.go

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import (
1919
"github.com/supabase/auth/internal/mailer/templatemailer"
2020
"github.com/supabase/auth/internal/models"
2121
"github.com/supabase/auth/internal/observability"
22-
"github.com/supabase/auth/internal/sbff"
2322
"github.com/supabase/auth/internal/storage"
2423
"github.com/supabase/auth/internal/tokens"
2524
"github.com/supabase/auth/internal/utilities"
@@ -153,17 +152,8 @@ func NewAPIWithVersion(globalConfig *conf.GlobalConfiguration, db *storage.Conne
153152
r := newRouter()
154153
r.UseBypass(observability.AddRequestID(globalConfig))
155154
r.UseBypass(logger)
156-
r.UseBypass(recoverer)
157-
r.UseBypass(
158-
sbff.Middleware(
159-
&globalConfig.Security,
160-
func(r *http.Request, err error) {
161-
log := observability.GetLogEntry(r).Entry
162-
log.WithField("error", err.Error()).Warn("error processing Sb-Forwarded-For")
163-
},
164-
),
165-
)
166155
r.UseBypass(xffmw.Handler)
156+
r.UseBypass(recoverer)
167157

168158
if globalConfig.API.MaxRequestDuration > 0 {
169159
r.UseBypass(timeoutMiddleware(globalConfig.API.MaxRequestDuration))

internal/api/middleware.go

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"github.com/supabase/auth/internal/api/shared"
2121
"github.com/supabase/auth/internal/models"
2222
"github.com/supabase/auth/internal/observability"
23-
"github.com/supabase/auth/internal/sbff"
2423
"github.com/supabase/auth/internal/security"
2524
"github.com/supabase/auth/internal/utilities"
2625

@@ -62,7 +61,7 @@ func (f *FunctionHooks) UnmarshalJSON(b []byte) error {
6261

6362
var emailRateLimitCounter = observability.ObtainMetricCounter("gotrue_email_rate_limit_counter", "Number of times an email rate limit has been triggered")
6463

65-
func (a *API) performRateLimitingWithHeader(lmt *limiter.Limiter, req *http.Request) error {
64+
func (a *API) performRateLimiting(lmt *limiter.Limiter, req *http.Request) error {
6665
limitHeader := a.config.RateLimitHeader
6766

6867
// If no rate limit header was set, ignore rate limiting
@@ -113,18 +112,6 @@ func (a *API) performRateLimitingWithHeader(lmt *limiter.Limiter, req *http.Requ
113112
return nil
114113
}
115114

116-
func (a *API) performRateLimiting(lmt *limiter.Limiter, req *http.Request) error {
117-
if sbffAddr, ok := sbff.GetIPAddress(req); ok {
118-
if err := tollbooth.LimitByKeys(lmt, []string{sbffAddr}); err != nil {
119-
return apierrors.NewTooManyRequestsError(apierrors.ErrorCodeOverRequestRateLimit, "Request rate limit reached")
120-
}
121-
122-
return nil
123-
}
124-
125-
return a.performRateLimitingWithHeader(lmt, req)
126-
}
127-
128115
func (a *API) limitHandler(lmt *limiter.Limiter) middlewareHandler {
129116
return func(w http.ResponseWriter, req *http.Request) (context.Context, error) {
130117
return req.Context(), a.performRateLimiting(lmt, req)

internal/api/middleware_test.go

Lines changed: 1 addition & 161 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import (
1919
"github.com/stretchr/testify/suite"
2020
"github.com/supabase/auth/internal/api/apierrors"
2121
"github.com/supabase/auth/internal/conf"
22-
"github.com/supabase/auth/internal/sbff"
2322
"github.com/supabase/auth/internal/storage"
2423
)
2524

@@ -416,166 +415,7 @@ func TestTimeoutResponseWriter(t *testing.T) {
416415
require.Equal(t, w1.Result(), w2.Result())
417416
}
418417

419-
func (ts *MiddlewareTestSuite) TestPerformRateLimitingWithSBFF() {
420-
origRateLimitHeader := ts.Config.RateLimitHeader
421-
origSBFFEnabled := ts.Config.Security.SbForwardedForEnabled
422-
423-
defer func() {
424-
ts.Config.RateLimitHeader = origRateLimitHeader
425-
ts.Config.Security.SbForwardedForEnabled = origSBFFEnabled
426-
}()
427-
428-
ts.Config.RateLimitHeader = "X-Test-Perform-Rate-Limiting"
429-
ts.Config.Security.SbForwardedForEnabled = true
430-
431-
type headerSet struct {
432-
rateLimiting string
433-
sbForwardedFor string
434-
}
435-
436-
testCases := []struct {
437-
name string
438-
headerValues []headerSet
439-
expErr error
440-
}{
441-
{
442-
name: "multiple SBFF values, single rate limiting value",
443-
headerValues: []headerSet{
444-
{
445-
sbForwardedFor: "192.168.1.100",
446-
rateLimiting: "60.60.60.60",
447-
},
448-
{
449-
sbForwardedFor: "192.168.1.200",
450-
rateLimiting: "60.60.60.60",
451-
},
452-
},
453-
expErr: nil,
454-
},
455-
{
456-
name: "single SBFF value, multiple rate limiting values",
457-
headerValues: []headerSet{
458-
{
459-
sbForwardedFor: "192.168.1.100",
460-
rateLimiting: "60.60.60.60",
461-
},
462-
{
463-
sbForwardedFor: "192.168.1.100",
464-
rateLimiting: "70.70.70.70",
465-
},
466-
},
467-
expErr: apierrors.NewTooManyRequestsError(
468-
apierrors.ErrorCodeOverRequestRateLimit,
469-
"Request rate limit reached",
470-
),
471-
},
472-
{
473-
name: "no SBFF value, multiple rate limiting values",
474-
headerValues: []headerSet{
475-
{
476-
sbForwardedFor: "",
477-
rateLimiting: "60.60.60.60",
478-
},
479-
{
480-
sbForwardedFor: "",
481-
rateLimiting: "70.70.70.70",
482-
},
483-
},
484-
expErr: nil,
485-
},
486-
{
487-
name: "no SBFF value, single rate limiting value",
488-
headerValues: []headerSet{
489-
{
490-
sbForwardedFor: "",
491-
rateLimiting: "60.60.60.60",
492-
},
493-
{
494-
sbForwardedFor: "",
495-
rateLimiting: "60.60.60.60",
496-
},
497-
},
498-
expErr: apierrors.NewTooManyRequestsError(
499-
apierrors.ErrorCodeOverRequestRateLimit,
500-
"Request rate limit reached",
501-
),
502-
},
503-
{
504-
name: "invalid SBFF value, multiple rate limiting values",
505-
headerValues: []headerSet{
506-
{
507-
sbForwardedFor: "invalid",
508-
rateLimiting: "60.60.60.60",
509-
},
510-
{
511-
sbForwardedFor: "invalid",
512-
rateLimiting: "70.70.70.70",
513-
},
514-
},
515-
expErr: nil,
516-
},
517-
{
518-
name: "invalid SBFF value, single rate limiting value",
519-
headerValues: []headerSet{
520-
{
521-
sbForwardedFor: "invalid",
522-
rateLimiting: "60.60.60.60",
523-
},
524-
{
525-
sbForwardedFor: "invalid",
526-
rateLimiting: "60.60.60.60",
527-
},
528-
},
529-
expErr: apierrors.NewTooManyRequestsError(
530-
apierrors.ErrorCodeOverRequestRateLimit,
531-
"Request rate limit reached",
532-
),
533-
},
534-
}
535-
536-
// This test uses the SBFF middleware to inject the Sb-Forwarded-For IP address value, then
537-
// wraps a handler that calls performRateLimiting and stores the error value.
538-
for _, tc := range testCases {
539-
lmt := tollbooth.NewLimiter(
540-
1,
541-
&limiter.ExpirableOptions{
542-
DefaultExpirationTTL: time.Hour,
543-
},
544-
)
545-
546-
var obsErr error
547-
548-
var handler http.HandlerFunc = func(rw http.ResponseWriter, r *http.Request) {
549-
obsErr = ts.API.performRateLimiting(lmt, r)
550-
}
551-
552-
errCallback := func(r *http.Request, err error) {
553-
}
554-
555-
middleware := sbff.Middleware(&ts.Config.Security, errCallback)
556-
557-
wrappedHandler := middleware(handler)
558-
559-
for _, h := range tc.headerValues {
560-
r := httptest.NewRequest(http.MethodGet, "http://localhost/", nil)
561-
562-
if h.rateLimiting != "" {
563-
r.Header.Set(ts.Config.RateLimitHeader, h.rateLimiting)
564-
}
565-
566-
if h.sbForwardedFor != "" {
567-
r.Header.Set(sbff.HeaderName, h.sbForwardedFor)
568-
}
569-
570-
wrappedHandler.ServeHTTP(nil, r)
571-
}
572-
573-
require.ErrorIs(ts.T(), obsErr, tc.expErr)
574-
}
575-
576-
}
577-
578-
func (ts *MiddlewareTestSuite) TestPerformRateLimitingWithHeader() {
418+
func (ts *MiddlewareTestSuite) TestPerformRateLimiting() {
579419
ts.Config.RateLimitHeader = "X-Test-Perform-Rate-Limiting"
580420

581421
tests := []struct {

internal/conf/configuration.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -731,7 +731,6 @@ type SecurityConfiguration struct {
731731
RefreshTokenAllowReuse bool `json:"refresh_token_allow_reuse" split_words:"true"`
732732
UpdatePasswordRequireReauthentication bool `json:"update_password_require_reauthentication" split_words:"true"`
733733
ManualLinkingEnabled bool `json:"manual_linking_enabled" split_words:"true" default:"false"`
734-
SbForwardedForEnabled bool `json:"sb_forwarded_for_enabled" split_words:"true" default:"false"`
735734

736735
DBEncryption DatabaseEncryptionConfiguration `json:"database_encryption" split_words:"true"`
737736
}

internal/sbff/sbff.go

Lines changed: 0 additions & 94 deletions
This file was deleted.

0 commit comments

Comments
 (0)