Skip to content

Commit

Permalink
fix: remove rate limiting implementation
Browse files Browse the repository at this point in the history
  • Loading branch information
nsklikas committed Oct 18, 2024
1 parent fe756f5 commit 6e7398f
Show file tree
Hide file tree
Showing 9 changed files with 13 additions and 127 deletions.
6 changes: 2 additions & 4 deletions compose/compose_strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package compose
import (
"context"

"github.com/coocood/freecache"
"github.com/ory/fosite"
"github.com/ory/fosite/handler/oauth2"
"github.com/ory/fosite/handler/openid"
Expand Down Expand Up @@ -55,8 +54,7 @@ func NewOpenIDConnectStrategy(keyGetter func(context.Context) (interface{}, erro
// Create a new device strategy
func NewDeviceStrategy(config fosite.Configurator) *rfc8628.DefaultDeviceStrategy {
return &rfc8628.DefaultDeviceStrategy{
Enigma: &hmac.HMACStrategy{Config: config},
RateLimiterCache: freecache.NewCache(1024 * 1024),
Config: config,
Enigma: &hmac.HMACStrategy{Config: config},
Config: config,
}
}
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ replace github.com/gorilla/sessions => github.com/gorilla/sessions v1.2.1

require (
github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2
github.com/coocood/freecache v1.2.4
github.com/cristalhq/jwt/v4 v4.0.2
github.com/dgraph-io/ristretto v0.1.1
github.com/go-jose/go-jose/v3 v3.0.3
Expand Down
4 changes: 0 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ github.com/cenkalti/backoff/v4 v4.2.1 h1:y4OZtCnogmCPw98Zjyt5a6+QwPLGkiQsYW5oUqy
github.com/cenkalti/backoff/v4 v4.2.1/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE=
github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
github.com/cespare/xxhash/v2 v2.1.1/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
github.com/cespare/xxhash/v2 v2.1.2/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
github.com/cespare/xxhash/v2 v2.2.0 h1:DC2CZ1Ep5Y4k3ZQ899DldepgrayRUGE6BBZ/cd9Cj44=
github.com/cespare/xxhash/v2 v2.2.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
github.com/chzyer/logex v1.1.10/go.mod h1:+Ywpsq7O8HXn0nuIou7OrIPyXbp3wmkHB+jjWRnGsAI=
Expand All @@ -57,8 +56,6 @@ github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGX
github.com/cncf/udpa/go v0.0.0-20200629203442-efcf912fb354/go.mod h1:WmhPx2Nbnhtbo57+VJT5O0JRkEi1Wbu0z5j0R8u5Hbk=
github.com/cncf/udpa/go v0.0.0-20201120205902-5459f2c99403/go.mod h1:WmhPx2Nbnhtbo57+VJT5O0JRkEi1Wbu0z5j0R8u5Hbk=
github.com/cockroachdb/apd v1.1.0/go.mod h1:8Sl8LxpKi29FqWXR16WEFZRNSz3SoPzUzeMeY4+DwBQ=
github.com/coocood/freecache v1.2.4 h1:UdR6Yz/X1HW4fZOuH0Z94KwG851GWOSknua5VUbb/5M=
github.com/coocood/freecache v1.2.4/go.mod h1:RBUWa/Cy+OHdfTGFEhEuE1pMCMX51Ncizj7rthiQ3vk=
github.com/coreos/go-systemd v0.0.0-20190321100706-95778dfbb74e/go.mod h1:F5haX7vjVVG0kc13fIWeqUViNPyEJxv/OmvnBo0Yme4=
github.com/coreos/go-systemd v0.0.0-20190719114852-fd7a80b32e1f/go.mod h1:F5haX7vjVVG0kc13fIWeqUViNPyEJxv/OmvnBo0Yme4=
github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
Expand Down Expand Up @@ -375,7 +372,6 @@ github.com/stretchr/objx v0.2.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoH
github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY=
github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA=
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
Expand Down
7 changes: 2 additions & 5 deletions handler/openid/flow_device_auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ import (

"github.com/stretchr/testify/require"

"github.com/coocood/freecache"

"github.com/ory/fosite"
"github.com/ory/fosite/handler/rfc8628"
"github.com/ory/fosite/token/hmac"
Expand All @@ -42,9 +40,8 @@ func TestDeviceAuth_HandleDeviceEndpointRequest(t *testing.T) {
h := OpenIDConnectDeviceHandler{
OpenIDConnectRequestStorage: store,
DeviceCodeStrategy: &rfc8628.DefaultDeviceStrategy{
Enigma: &hmac.HMACStrategy{Config: &fosite.Config{GlobalSecret: []byte("foobar")}},
RateLimiterCache: freecache.NewCache(1024 * 1024),
Config: config,
Enigma: &hmac.HMACStrategy{Config: &fosite.Config{GlobalSecret: []byte("foobar")}},
Config: config,
},
Config: config,
IDTokenHandleHelper: &IDTokenHandleHelper{
Expand Down
6 changes: 2 additions & 4 deletions handler/openid/flow_device_token_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (

"github.com/stretchr/testify/require"

"github.com/coocood/freecache"
"github.com/ory/fosite/handler/rfc8628"
"github.com/ory/fosite/internal"
"github.com/ory/fosite/token/hmac"
Expand Down Expand Up @@ -59,9 +58,8 @@ func TestDeviceToken_PopulateTokenEndpointResponse(t *testing.T) {
h := OpenIDConnectDeviceHandler{
OpenIDConnectRequestStorage: store,
DeviceCodeStrategy: &rfc8628.DefaultDeviceStrategy{
Enigma: &hmac.HMACStrategy{Config: &fosite.Config{GlobalSecret: []byte("foobar")}},
RateLimiterCache: freecache.NewCache(1024 * 1024),
Config: config,
Enigma: &hmac.HMACStrategy{Config: &fosite.Config{GlobalSecret: []byte("foobar")}},
Config: config,
},
Config: config,
IDTokenHandleHelper: &IDTokenHandleHelper{
Expand Down
74 changes: 3 additions & 71 deletions handler/rfc8628/strategy_hmacsha.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,9 @@ package rfc8628

import (
"context"
"encoding/json"
"strings"
"time"

"github.com/coocood/freecache"
"github.com/mohae/deepcopy"

"github.com/ory/x/errorsx"
Expand Down Expand Up @@ -101,9 +99,8 @@ func (s *DefaultDeviceFlowSession) SetBrowserFlowCompleted(flag bool) {

// DefaultDeviceStrategy implements the default device strategy
type DefaultDeviceStrategy struct {
Enigma *enigma.HMACStrategy
RateLimiterCache *freecache.Cache
Config interface {
Enigma *enigma.HMACStrategy
Config interface {
fosite.DeviceProvider
fosite.DeviceAndUserCodeLifespanProvider
}
Expand Down Expand Up @@ -173,70 +170,5 @@ func (h *DefaultDeviceStrategy) ValidateDeviceCode(ctx context.Context, r fosite

// ShouldRateLimit is used to decide whether a request should be rate-limited
func (h *DefaultDeviceStrategy) ShouldRateLimit(context context.Context, code string) (bool, error) {
key := code + "_limiter"

keyBytes := []byte(key)
object, err := h.RateLimiterCache.Get(keyBytes)
// This code is not in the cache, so we just add it
if err != nil {
timer := new(expirationTimer)
timer.Counter = 1
timer.NotUntil = h.getNotUntil(context, 1)
exp, err := h.serializeExpiration(timer)
if err != nil {
return false, errorsx.WithStack(fosite.ErrServerError.WithHintf("Failed to serialize expiration struct %s", err))
}
// Set the expiration time as value, and use the lifespan of the device code as TTL.
h.RateLimiterCache.Set(keyBytes, exp, int(h.Config.GetDeviceAndUserCodeLifespan(context).Seconds()))
return false, nil
}

expiration, err := h.deserializeExpiration(object)
if err != nil {
return false, errorsx.WithStack(fosite.ErrServerError.WithHintf("Failed to store to rate limit cache: %s", err))
}

// The code is valid and enough time has passed since the last call.
if time.Now().After(expiration.NotUntil) {
expiration.NotUntil = h.getNotUntil(context, expiration.Counter)
exp, err := h.serializeExpiration(expiration)
if err != nil {
return false, errorsx.WithStack(fosite.ErrServerError.WithHintf("Failed to serialize expiration struct %s", err))
}
h.RateLimiterCache.Set(keyBytes, exp, int(h.Config.GetDeviceAndUserCodeLifespan(context).Seconds()))
return false, nil
}

// The token calls were made too fast, we need to double the interval period
expiration.NotUntil = h.getNotUntil(context, expiration.Counter+1)
expiration.Counter += 1
exp, err := h.serializeExpiration(expiration)
if err != nil {
return false, errorsx.WithStack(fosite.ErrServerError.WithHintf("Failed to serialize expiration struct %s", err))
}
h.RateLimiterCache.Set(keyBytes, exp, int(h.Config.GetDeviceAndUserCodeLifespan(context).Seconds()))

return true, nil
}

func (h *DefaultDeviceStrategy) getNotUntil(context context.Context, multiplier int) time.Time {
duration := h.Config.GetDeviceAuthTokenPollingInterval(context)
expiration := time.Now().Add(duration * time.Duration(multiplier)).Add(-POLLING_RATE_LIMITING_LEEWAY)
return expiration
}

type expirationTimer struct {
NotUntil time.Time
Counter int
}

func (h *DefaultDeviceStrategy) serializeExpiration(exp *expirationTimer) ([]byte, error) {
b, err := json.Marshal(exp)
return b, err
}

func (h *DefaultDeviceStrategy) deserializeExpiration(b []byte) (*expirationTimer, error) {
timer := new(expirationTimer)
err := json.Unmarshal(b, timer)
return timer, err
return false, nil
}
34 changes: 1 addition & 33 deletions handler/rfc8628/strategy_hmacsha_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"testing"
"time"

"github.com/coocood/freecache"
"github.com/stretchr/testify/assert"

"github.com/ory/fosite"
Expand All @@ -20,8 +19,7 @@ import (
)

var hmacshaStrategy = DefaultDeviceStrategy{
Enigma: &hmac.HMACStrategy{Config: &fosite.Config{GlobalSecret: []byte("foobarfoobarfoobarfoobarfoobarfoobarfoobarfoobar")}},
RateLimiterCache: freecache.NewCache(16384 * 64),
Enigma: &hmac.HMACStrategy{Config: &fosite.Config{GlobalSecret: []byte("foobarfoobarfoobarfoobarfoobarfoobarfoobarfoobar")}},
Config: &fosite.Config{
AccessTokenLifespan: time.Minute * 24,
AuthorizeCodeLifespan: time.Minute * 24,
Expand Down Expand Up @@ -110,33 +108,3 @@ func TestHMACDeviceCode(t *testing.T) {
})
}
}

func TestRateLimit(t *testing.T) {
t.Run("ratelimit no-wait", func(t *testing.T) {
hmacshaStrategy.RateLimiterCache.Clear()
b, err := hmacshaStrategy.ShouldRateLimit(context.TODO(), "AAA")
assert.NoError(t, err)
assert.False(t, b)
b, err = hmacshaStrategy.ShouldRateLimit(context.TODO(), "AAA")
assert.NoError(t, err)
assert.True(t, b)
})

t.Run("ratelimit wait", func(t *testing.T) {
hmacshaStrategy.RateLimiterCache.Clear()
b, err := hmacshaStrategy.ShouldRateLimit(context.TODO(), "AAA")
assert.NoError(t, err)
assert.False(t, b)
time.Sleep(500 * time.Millisecond)
b, err = hmacshaStrategy.ShouldRateLimit(context.TODO(), "AAA")
assert.NoError(t, err)
assert.False(t, b)
time.Sleep(500 * time.Millisecond)
b, err = hmacshaStrategy.ShouldRateLimit(context.TODO(), "AAA")
assert.NoError(t, err)
assert.False(t, b)
b, err = hmacshaStrategy.ShouldRateLimit(context.TODO(), "AAA")
assert.NoError(t, err)
assert.True(t, b)
})
}
2 changes: 1 addition & 1 deletion handler/rfc8628/token_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ func (c DeviceCodeTokenEndpointHandler) validateCode(ctx context.Context, reques
return err
}
if shouldRateLimit {
return errorsx.WithStack(fosite.ErrPollingRateLimited)
return errorsx.WithStack(fosite.ErrSlowDown)
}
return nil
}
Expand Down
6 changes: 2 additions & 4 deletions handler/rfc8628/token_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"testing"
"time"

"github.com/coocood/freecache"
"github.com/pkg/errors"

"github.com/ory/fosite/internal"
Expand All @@ -34,8 +33,7 @@ var hmacshaStrategy = oauth2.NewHMACSHAStrategy(
)

var RFC8628HMACSHAStrategy = DefaultDeviceStrategy{
Enigma: &hmac.HMACStrategy{Config: &fosite.Config{GlobalSecret: []byte("foobarfoobarfoobarfoobarfoobarfoobarfoobarfoobar")}},
RateLimiterCache: freecache.NewCache(1024 * 1024),
Enigma: &hmac.HMACStrategy{Config: &fosite.Config{GlobalSecret: []byte("foobarfoobarfoobarfoobarfoobarfoobarfoobarfoobar")}},
Config: &fosite.Config{
DeviceAndUserCodeLifespan: time.Minute * 30,
},
Expand Down Expand Up @@ -350,7 +348,7 @@ func TestDeviceUserCode_HandleTokenEndpointRequest_RateLimiting(t *testing.T) {
err = h.HandleTokenEndpointRequest(context.Background(), areq)
require.NoError(t, err, "%+v", err)
err = h.HandleTokenEndpointRequest(context.Background(), areq)
require.Error(t, fosite.ErrPollingRateLimited, err)
require.Error(t, fosite.ErrSlowDown, err)
time.Sleep(10 * time.Second)
err = h.HandleTokenEndpointRequest(context.Background(), areq)
require.NoError(t, err, "%+v", err)
Expand Down

0 comments on commit 6e7398f

Please sign in to comment.