From d8ab8dc7eeb57ecf6070307dbc33e841deb32251 Mon Sep 17 00:00:00 2001 From: Alex Ababii Date: Mon, 12 Oct 2026 16:36:56 +0200 Subject: [PATCH] feat(authguest): extracted restrict access logic into separate package, backoff and quota limiters --- services/auth-guest/pkg/command/server.go | 4 +- services/auth-guest/pkg/config/config.go | 47 +- .../pkg/config/defaults/defaultconfig.go | 71 +-- .../pkg/config/defaults/defaultconfig_test.go | 62 ++- services/auth-guest/pkg/server/http/errors.go | 21 +- services/auth-guest/pkg/server/http/renew.go | 2 +- .../auth-guest/pkg/server/http/renew_test.go | 14 +- .../auth-guest/pkg/server/http/verify_pin.go | 8 +- .../pkg/server/http/verify_pin_test.go | 59 ++- .../pkg/server/http/verify_token.go | 10 +- .../pkg/service/authguest/errors.go | 52 ++ .../pkg/service/authguest/options.go | 24 +- .../pkg/service/authguest/service.go | 294 ++++++----- .../pkg/service/authguest/service_test.go | 482 +++++++++++++----- .../pkg/service/guard/backoff_limiter.go | 65 +++ .../pkg/service/guard/backoff_limiter_test.go | 52 ++ .../auth-guest/pkg/service/guard/nonce.go | 78 +++ .../pkg/service/guard/nonce_test.go | 56 ++ .../pkg/service/guard/quota_limiter.go | 60 +++ .../pkg/service/guard/quota_limiter_test.go | 56 ++ .../pkg/service/storage/file_manager_test.go | 23 +- .../auth-guest/pkg/service/storage/storage.go | 38 +- 22 files changed, 1178 insertions(+), 400 deletions(-) create mode 100644 services/auth-guest/pkg/service/authguest/errors.go create mode 100644 services/auth-guest/pkg/service/guard/backoff_limiter.go create mode 100644 services/auth-guest/pkg/service/guard/backoff_limiter_test.go create mode 100644 services/auth-guest/pkg/service/guard/nonce.go create mode 100644 services/auth-guest/pkg/service/guard/nonce_test.go create mode 100644 services/auth-guest/pkg/service/guard/quota_limiter.go create mode 100644 services/auth-guest/pkg/service/guard/quota_limiter_test.go diff --git a/services/auth-guest/pkg/command/server.go b/services/auth-guest/pkg/command/server.go index aff76690c7..7a9b9b1323 100644 --- a/services/auth-guest/pkg/command/server.go +++ b/services/auth-guest/pkg/command/server.go @@ -104,7 +104,9 @@ func Server(cfg *config.Config) *cobra.Command { authguest.ServiceAccount(cfg.ServiceAccount), authguest.JWT(jwtService), authguest.EventsPublisher(evStream), - authguest.Challenge(cfg.Challenge), + authguest.Nonce(cfg.Nonce), + authguest.BackoffLimiter(cfg.BackoffLimiter), + authguest.QuotaLimiter(cfg.QuotaLimiter), ) if !cfg.HTTP.Disabled { diff --git a/services/auth-guest/pkg/config/config.go b/services/auth-guest/pkg/config/config.go index 063d6fc999..a7db5788a7 100644 --- a/services/auth-guest/pkg/config/config.go +++ b/services/auth-guest/pkg/config/config.go @@ -28,13 +28,14 @@ type Config struct { RevaGateway string `yaml:"reva_gateway" env:"OC_REVA_GATEWAY" desc:"CS3 gateway used to look up user metadata" introductionVersion:"%%NEXT%%"` GRPCClientTLS *shared.GRPCClientTLS `yaml:"grpc_client_tls"` - GRPC GRPCConfig `yaml:"grpc"` - HTTP HTTP `yaml:"http"` - Storage Storage `yaml:"storage"` - TokenManager *TokenManager `yaml:"token_manager"` - JWT JWT `yaml:"jwt"` - Challenge Challenge `yaml:"challenge"` - Renew Renew `yaml:"renew"` + GRPC GRPCConfig `yaml:"grpc"` + HTTP HTTP `yaml:"http"` + Storage Storage `yaml:"storage"` + TokenManager *TokenManager `yaml:"token_manager"` + JWT JWT `yaml:"jwt"` + Nonce Nonce `yaml:"nonce"` + BackoffLimiter BackoffLimiter `yaml:"backoff_limiter"` + QuotaLimiter QuotaLimiter `yaml:"quota_limiter"` ServiceAccount ServiceAccount `yaml:"service_account"` @@ -103,20 +104,28 @@ type JWT struct { TTL time.Duration `yaml:"ttl" env:"AUTH_GUEST_JWT_TTL" desc:"The lifetime of a redeemed guest session token." introductionVersion:"%%NEXT%%"` } -// Challenge defines the configuration of the browser-bound PIN challenge that -// is created on renewal and required to verify a PIN. -type Challenge struct { - CookieName string `yaml:"cookie_name" env:"AUTH_GUEST_CHALLENGE_COOKIE_NAME" desc:"The name of the cookie that binds a PIN challenge to the browser that requested the renewal." introductionVersion:"%%NEXT%%"` - TTL time.Duration `yaml:"ttl" env:"AUTH_GUEST_CHALLENGE_TTL" desc:"The lifetime of a PIN challenge. After it expires the PIN has to be renewed again. See the Environment Variable Types description for more details." introductionVersion:"%%NEXT%%"` - MaxAttempts int `yaml:"max_attempts" env:"AUTH_GUEST_CHALLENGE_MAX_ATTEMPTS" desc:"The maximum number of failed PIN verification attempts before the challenge and the PIN are invalidated." introductionVersion:"%%NEXT%%"` - BackoffBase time.Duration `yaml:"backoff_base" env:"AUTH_GUEST_CHALLENGE_BACKOFF_BASE" desc:"The base duration for the exponential backoff between failed PIN verification attempts. See the Environment Variable Types description for more details." introductionVersion:"%%NEXT%%"` - BackoffCap time.Duration `yaml:"backoff_cap" env:"AUTH_GUEST_CHALLENGE_BACKOFF_CAP" desc:"The maximum duration for the exponential backoff between failed PIN verification attempts. See the Environment Variable Types description for more details." introductionVersion:"%%NEXT%%"` +// Nonce defines the configuration of the browser-bound nonce that is +// created on renewal and required to verify a PIN. +type Nonce struct { + CookieName string `yaml:"cookie_name" env:"AUTH_GUEST_NONCE_COOKIE_NAME" desc:"The name of the cookie that binds a nonce to the browser that requested the renewal." introductionVersion:"%%NEXT%%"` + TTL time.Duration `yaml:"ttl" env:"AUTH_GUEST_NONCE_TTL" desc:"The lifetime of a nonce. After it expires the PIN has to be renewed again. See the Environment Variable Types description for more details." introductionVersion:"%%NEXT%%"` } -// Renew defines the configuration to throttle renewal requests per guest link. -type Renew struct { - Cooldown time.Duration `yaml:"cooldown" env:"AUTH_GUEST_RENEW_COOLDOWN" desc:"The minimum time between two renewal requests for the same guest link. See the Environment Variable Types description for more details." introductionVersion:"%%NEXT%%"` - DailyCap int `yaml:"daily_cap" env:"AUTH_GUEST_RENEW_DAILY_CAP" desc:"The maximum number of renewal requests for the same guest link within the renewal window of 24 hours." introductionVersion:"%%NEXT%%"` +// BackoffLimiter defines the configuration of a limiter that counts events, +// blocks after a maximum number of them and increases the wait between events +// exponentially. +type BackoffLimiter struct { + MaxAttempts int `yaml:"max_attempts" env:"AUTH_GUEST_BACKOFF_MAX_ATTEMPTS" desc:"The maximum number of failed attempts before the PIN is invalidated." introductionVersion:"%%NEXT%%"` + BackoffBase time.Duration `yaml:"backoff_base" env:"AUTH_GUEST_BACKOFF_BASE" desc:"The base duration for the exponential backoff between failed attempts. See the Environment Variable Types description for more details." introductionVersion:"%%NEXT%%"` + BackoffCap time.Duration `yaml:"backoff_cap" env:"AUTH_GUEST_BACKOFF_CAP" desc:"The maximum duration for the exponential backoff between failed attempts. See the Environment Variable Types description for more details." introductionVersion:"%%NEXT%%"` +} + +// QuotaLimiter defines the configuration of a limiter that allows a maximum +// number of events per window with a minimum cooldown between them. +type QuotaLimiter struct { + DailyCap int `yaml:"daily_cap" env:"AUTH_GUEST_QUOTA_DAILY_CAP" desc:"The maximum number of renewals for the same share within the window." introductionVersion:"%%NEXT%%"` + Cooldown time.Duration `yaml:"cooldown" env:"AUTH_GUEST_QUOTA_COOLDOWN" desc:"The minimum time between two renewals for the same share. See the Environment Variable Types description for more details." introductionVersion:"%%NEXT%%"` + Window time.Duration `yaml:"window" env:"AUTH_GUEST_QUOTA_WINDOW" desc:"The window over which the renewal quota is counted. See the Environment Variable Types description for more details." introductionVersion:"%%NEXT%%"` } const ( diff --git a/services/auth-guest/pkg/config/defaults/defaultconfig.go b/services/auth-guest/pkg/config/defaults/defaultconfig.go index 32e9acebbd..8b4a9db58b 100644 --- a/services/auth-guest/pkg/config/defaults/defaultconfig.go +++ b/services/auth-guest/pkg/config/defaults/defaultconfig.go @@ -13,16 +13,17 @@ import ( "github.com/opencloud-eu/opencloud/services/auth-guest/pkg/config" ) -// Default values for the PIN challenge and renewal throttling. +// Default values for the guard policies (nonce, backoff and quota limiters). const ( - defaultSessionCookieName = "__Host-oc_guest_session" - defaultChallengeCookieName = "__Host-oc_guest_challenge" - defaultChallengeTTL = 15 * time.Minute - defaultChallengeMaxAttempts = 5 - defaultChallengeBackoffBase = time.Second - defaultChallengeBackoffCap = 30 * time.Second - defaultRenewCooldown = time.Minute - defaultRenewDailyCap = 20 + defaultSessionCookieName = "__Host-oc_guest_session" + defaultNonceCookieName = "__Host-oc_guest_nonce" + defaultNonceTTL = 15 * time.Minute + defaultBackoffMaxAttempts = 5 + defaultBackoffBase = time.Second + defaultBackoffCap = 30 * time.Second + defaultQuotaCooldown = time.Minute + defaultQuotaWindow = 24 * time.Hour + defaultQuotaDailyCap = 20 ) // FullDefaultConfig returns the full default config @@ -75,16 +76,19 @@ func DefaultConfig() *config.Config { CookieName: defaultSessionCookieName, TTL: 24 * time.Hour, }, - Challenge: config.Challenge{ - CookieName: defaultChallengeCookieName, - TTL: defaultChallengeTTL, - MaxAttempts: defaultChallengeMaxAttempts, - BackoffBase: defaultChallengeBackoffBase, - BackoffCap: defaultChallengeBackoffCap, + Nonce: config.Nonce{ + CookieName: defaultNonceCookieName, + TTL: defaultNonceTTL, }, - Renew: config.Renew{ - Cooldown: defaultRenewCooldown, - DailyCap: defaultRenewDailyCap, + BackoffLimiter: config.BackoffLimiter{ + MaxAttempts: defaultBackoffMaxAttempts, + BackoffBase: defaultBackoffBase, + BackoffCap: defaultBackoffCap, + }, + QuotaLimiter: config.QuotaLimiter{ + DailyCap: defaultQuotaDailyCap, + Cooldown: defaultQuotaCooldown, + Window: defaultQuotaWindow, }, } } @@ -114,26 +118,29 @@ func EnsureDefaults(cfg *config.Config) { cfg.HTTP.TLS = cfg.Commons.HTTPServiceTLS } - if cfg.Challenge.CookieName == "" { - cfg.Challenge.CookieName = defaultChallengeCookieName + if cfg.Nonce.CookieName == "" { + cfg.Nonce.CookieName = defaultNonceCookieName } - if cfg.Challenge.TTL <= 0 { - cfg.Challenge.TTL = defaultChallengeTTL + if cfg.Nonce.TTL <= 0 { + cfg.Nonce.TTL = defaultNonceTTL } - if cfg.Challenge.MaxAttempts <= 0 { - cfg.Challenge.MaxAttempts = defaultChallengeMaxAttempts + if cfg.BackoffLimiter.MaxAttempts <= 0 { + cfg.BackoffLimiter.MaxAttempts = defaultBackoffMaxAttempts } - if cfg.Challenge.BackoffBase <= 0 { - cfg.Challenge.BackoffBase = defaultChallengeBackoffBase + if cfg.BackoffLimiter.BackoffBase <= 0 { + cfg.BackoffLimiter.BackoffBase = defaultBackoffBase } - if cfg.Challenge.BackoffCap <= 0 { - cfg.Challenge.BackoffCap = defaultChallengeBackoffCap + if cfg.BackoffLimiter.BackoffCap <= 0 { + cfg.BackoffLimiter.BackoffCap = defaultBackoffCap } - if cfg.Renew.Cooldown <= 0 { - cfg.Renew.Cooldown = defaultRenewCooldown + if cfg.QuotaLimiter.Cooldown <= 0 { + cfg.QuotaLimiter.Cooldown = defaultQuotaCooldown } - if cfg.Renew.DailyCap <= 0 { - cfg.Renew.DailyCap = defaultRenewDailyCap + if cfg.QuotaLimiter.Window <= 0 { + cfg.QuotaLimiter.Window = defaultQuotaWindow + } + if cfg.QuotaLimiter.DailyCap <= 0 { + cfg.QuotaLimiter.DailyCap = defaultQuotaDailyCap } } diff --git a/services/auth-guest/pkg/config/defaults/defaultconfig_test.go b/services/auth-guest/pkg/config/defaults/defaultconfig_test.go index 080f2b53d1..705b9b71da 100644 --- a/services/auth-guest/pkg/config/defaults/defaultconfig_test.go +++ b/services/auth-guest/pkg/config/defaults/defaultconfig_test.go @@ -11,54 +11,60 @@ import ( "github.com/stretchr/testify/assert" ) -func TestDefaultConfigChallengeAndRenew(t *testing.T) { +func TestDefaultConfigGuard(t *testing.T) { cfg := DefaultConfig() - assert.Equal(t, defaultChallengeCookieName, cfg.Challenge.CookieName) - assert.Equal(t, defaultChallengeTTL, cfg.Challenge.TTL) - assert.Equal(t, defaultChallengeMaxAttempts, cfg.Challenge.MaxAttempts) - assert.Equal(t, defaultChallengeBackoffBase, cfg.Challenge.BackoffBase) - assert.Equal(t, defaultChallengeBackoffCap, cfg.Challenge.BackoffCap) - assert.Equal(t, defaultRenewCooldown, cfg.Renew.Cooldown) - assert.Equal(t, defaultRenewDailyCap, cfg.Renew.DailyCap) + assert.Equal(t, defaultNonceCookieName, cfg.Nonce.CookieName) + assert.Equal(t, defaultNonceTTL, cfg.Nonce.TTL) + assert.Equal(t, defaultBackoffMaxAttempts, cfg.BackoffLimiter.MaxAttempts) + assert.Equal(t, defaultBackoffBase, cfg.BackoffLimiter.BackoffBase) + assert.Equal(t, defaultBackoffCap, cfg.BackoffLimiter.BackoffCap) + assert.Equal(t, defaultQuotaCooldown, cfg.QuotaLimiter.Cooldown) + assert.Equal(t, defaultQuotaWindow, cfg.QuotaLimiter.Window) + assert.Equal(t, defaultQuotaDailyCap, cfg.QuotaLimiter.DailyCap) } -func TestEnsureDefaultsFillsChallengeAndRenew(t *testing.T) { +func TestEnsureDefaultsFillsGuard(t *testing.T) { cfg := &config.Config{} EnsureDefaults(cfg) - assert.Equal(t, defaultChallengeCookieName, cfg.Challenge.CookieName) - assert.Equal(t, defaultChallengeTTL, cfg.Challenge.TTL) - assert.Equal(t, defaultChallengeMaxAttempts, cfg.Challenge.MaxAttempts) - assert.Equal(t, defaultChallengeBackoffBase, cfg.Challenge.BackoffBase) - assert.Equal(t, defaultChallengeBackoffCap, cfg.Challenge.BackoffCap) - assert.Equal(t, defaultRenewCooldown, cfg.Renew.Cooldown) - assert.Equal(t, defaultRenewDailyCap, cfg.Renew.DailyCap) + assert.Equal(t, defaultNonceCookieName, cfg.Nonce.CookieName) + assert.Equal(t, defaultNonceTTL, cfg.Nonce.TTL) + assert.Equal(t, defaultBackoffMaxAttempts, cfg.BackoffLimiter.MaxAttempts) + assert.Equal(t, defaultBackoffBase, cfg.BackoffLimiter.BackoffBase) + assert.Equal(t, defaultBackoffCap, cfg.BackoffLimiter.BackoffCap) + assert.Equal(t, defaultQuotaCooldown, cfg.QuotaLimiter.Cooldown) + assert.Equal(t, defaultQuotaWindow, cfg.QuotaLimiter.Window) + assert.Equal(t, defaultQuotaDailyCap, cfg.QuotaLimiter.DailyCap) } -func TestEnsureDefaultsKeepsCustomChallengeAndRenew(t *testing.T) { +func TestEnsureDefaultsKeepsCustomGuard(t *testing.T) { cfg := &config.Config{ - Challenge: config.Challenge{ - CookieName: "custom-challenge", - TTL: 2 * time.Minute, + Nonce: config.Nonce{ + CookieName: "custom-nonce", + TTL: 2 * time.Minute, + }, + BackoffLimiter: config.BackoffLimiter{ MaxAttempts: 3, BackoffBase: 2 * time.Second, BackoffCap: 10 * time.Second, }, - Renew: config.Renew{ + QuotaLimiter: config.QuotaLimiter{ Cooldown: 30 * time.Second, + Window: 12 * time.Hour, DailyCap: 5, }, } EnsureDefaults(cfg) - assert.Equal(t, "custom-challenge", cfg.Challenge.CookieName) - assert.Equal(t, 2*time.Minute, cfg.Challenge.TTL) - assert.Equal(t, 3, cfg.Challenge.MaxAttempts) - assert.Equal(t, 2*time.Second, cfg.Challenge.BackoffBase) - assert.Equal(t, 10*time.Second, cfg.Challenge.BackoffCap) - assert.Equal(t, 30*time.Second, cfg.Renew.Cooldown) - assert.Equal(t, 5, cfg.Renew.DailyCap) + assert.Equal(t, "custom-nonce", cfg.Nonce.CookieName) + assert.Equal(t, 2*time.Minute, cfg.Nonce.TTL) + assert.Equal(t, 3, cfg.BackoffLimiter.MaxAttempts) + assert.Equal(t, 2*time.Second, cfg.BackoffLimiter.BackoffBase) + assert.Equal(t, 10*time.Second, cfg.BackoffLimiter.BackoffCap) + assert.Equal(t, 30*time.Second, cfg.QuotaLimiter.Cooldown) + assert.Equal(t, 12*time.Hour, cfg.QuotaLimiter.Window) + assert.Equal(t, 5, cfg.QuotaLimiter.DailyCap) } diff --git a/services/auth-guest/pkg/server/http/errors.go b/services/auth-guest/pkg/server/http/errors.go index 520cf7294a..c09d2ed402 100644 --- a/services/auth-guest/pkg/server/http/errors.go +++ b/services/auth-guest/pkg/server/http/errors.go @@ -7,6 +7,8 @@ import ( "encoding/json" "errors" "net/http" + "strconv" + "time" "github.com/opencloud-eu/opencloud/services/auth-guest/pkg/service/authguest" "github.com/opencloud-eu/opencloud/services/auth-guest/pkg/service/jwt" @@ -18,6 +20,7 @@ type errorResponse struct { ErrorType string `json:"errorType"` Message string `json:"message"` PermissionID string `json:"permissionId"` + RetryAfter int `json:"retryAfter,omitempty"` } func writeError(w http.ResponseWriter, status int, body errorResponse) { @@ -59,10 +62,12 @@ func writeGuestError(w http.ResponseWriter, err error) { status, errorType = http.StatusUnauthorized, "pinInvalid" case errors.Is(ge.ErrorType, authguest.ErrPinExpired): status, errorType = http.StatusUnauthorized, "pinExpired" - case errors.Is(ge.ErrorType, authguest.ErrChallengeInvalid): - status, errorType = http.StatusUnauthorized, "challengeInvalid" - case errors.Is(ge.ErrorType, authguest.ErrChallengeExpired): - status, errorType = http.StatusUnauthorized, "challengeExpired" + case errors.Is(ge.ErrorType, authguest.ErrNonceInvalid): + status, errorType = http.StatusUnauthorized, "nonceInvalid" + case errors.Is(ge.ErrorType, authguest.ErrNonceExpired): + status, errorType = http.StatusUnauthorized, "nonceExpired" + case errors.Is(ge.ErrorType, authguest.ErrTooManyAttempts): + status, errorType = http.StatusTooManyRequests, "tooManyRequests" case errors.Is(ge.ErrorType, jwt.ErrInvalidSession): status, errorType = http.StatusUnauthorized, "sessionInvalid" case errors.Is(ge.ErrorType, authguest.ErrPermissionMismatch): @@ -74,5 +79,11 @@ func writeGuestError(w http.ResponseWriter, err error) { message = "An internal error occurred." } - writeError(w, status, errorResponse{ErrorType: errorType, Message: message, PermissionID: ge.ShareID}) + retryAfter := 0 + if ge.RetryAfter > 0 { + retryAfter = int((ge.RetryAfter + time.Second - 1) / time.Second) + w.Header().Set("Retry-After", strconv.Itoa(retryAfter)) + } + + writeError(w, status, errorResponse{ErrorType: errorType, Message: message, PermissionID: ge.ShareID, RetryAfter: retryAfter}) } diff --git a/services/auth-guest/pkg/server/http/renew.go b/services/auth-guest/pkg/server/http/renew.go index d15cd7c360..25b52d90c7 100644 --- a/services/auth-guest/pkg/server/http/renew.go +++ b/services/auth-guest/pkg/server/http/renew.go @@ -50,7 +50,7 @@ func RenewHandler(log log.Logger, s authguest.AuthGuest, cfg *config.Config) fun return } - setChallengeCookie(w, cfg, nonce) + setNonceCookie(w, cfg, nonce) w.WriteHeader(http.StatusOK) } diff --git a/services/auth-guest/pkg/server/http/renew_test.go b/services/auth-guest/pkg/server/http/renew_test.go index 20774da19b..46a05e7f81 100644 --- a/services/auth-guest/pkg/server/http/renew_test.go +++ b/services/auth-guest/pkg/server/http/renew_test.go @@ -24,7 +24,7 @@ import ( const testCookieName = "__Host-oc_guest_session" -const testChallengeCookieName = "__Host-oc_guest_challenge" +const testNonceCookieName = "__Host-oc_guest_nonce" func newRenewHandler(t *testing.T, svc authguest.AuthGuest) http.HandlerFunc { t.Helper() @@ -33,8 +33,8 @@ func newRenewHandler(t *testing.T, svc authguest.AuthGuest) http.HandlerFunc { CookieName: testCookieName, TTL: time.Hour, }, - Challenge: config.Challenge{ - CookieName: testChallengeCookieName, + Nonce: config.Nonce{ + CookieName: testNonceCookieName, TTL: time.Hour, }, } @@ -43,7 +43,7 @@ func newRenewHandler(t *testing.T, svc authguest.AuthGuest) http.HandlerFunc { func TestRenewHandlerWithLinkToken(t *testing.T) { svcMock := mocks.NewAuthGuest(t) - svcMock.On("Renew", mock.Anything, "share-1", "link-token", "").Return("challenge-nonce", nil) + svcMock.On("Renew", mock.Anything, "share-1", "link-token", "").Return("nonce-value", nil) body, err := json.Marshal(RenewRequest{PermissionID: "share-1", Token: "link-token"}) require.NoError(t, err) @@ -56,12 +56,12 @@ func TestRenewHandlerWithLinkToken(t *testing.T) { var cookie *http.Cookie for _, c := range rr.Result().Cookies() { - if c.Name == testChallengeCookieName { + if c.Name == testNonceCookieName { cookie = c } } require.NotNil(t, cookie) - assert.Equal(t, "challenge-nonce", cookie.Value) + assert.Equal(t, "nonce-value", cookie.Value) assert.True(t, cookie.HttpOnly) assert.True(t, cookie.Secure) assert.Equal(t, "/", cookie.Path) @@ -69,7 +69,7 @@ func TestRenewHandlerWithLinkToken(t *testing.T) { func TestRenewHandlerWithSessionCookie(t *testing.T) { svcMock := mocks.NewAuthGuest(t) - svcMock.On("Renew", mock.Anything, "share-1", "", "session-token").Return("challenge-nonce", nil) + svcMock.On("Renew", mock.Anything, "share-1", "", "session-token").Return("nonce-value", nil) body, err := json.Marshal(RenewRequest{PermissionID: "share-1"}) require.NoError(t, err) diff --git a/services/auth-guest/pkg/server/http/verify_pin.go b/services/auth-guest/pkg/server/http/verify_pin.go index 907e969ba2..78454044ef 100644 --- a/services/auth-guest/pkg/server/http/verify_pin.go +++ b/services/auth-guest/pkg/server/http/verify_pin.go @@ -36,10 +36,10 @@ func VerifyPinHandler(log log.Logger, s authguest.AuthGuest, cfg *config.Config) return } - cookie, err := r.Cookie(cfg.Challenge.CookieName) + cookie, err := r.Cookie(cfg.Nonce.CookieName) if err != nil || cookie.Value == "" { - log.Debug().Msg("the challenge cookie is missing from the request") - writeError(w, http.StatusBadRequest, errorResponse{ErrorType: "invalidRequest", Message: "The challenge cookie is missing from the request."}) + log.Debug().Msg("the nonce cookie is missing from the request") + writeError(w, http.StatusBadRequest, errorResponse{ErrorType: "invalidRequest", Message: "The nonce cookie is missing from the request."}) return } @@ -50,7 +50,7 @@ func VerifyPinHandler(log log.Logger, s authguest.AuthGuest, cfg *config.Config) return } - clearChallengeCookie(w, cfg) + clearNonceCookie(w, cfg) setSessionCookie(w, cfg, result.SessionToken) w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) diff --git a/services/auth-guest/pkg/server/http/verify_pin_test.go b/services/auth-guest/pkg/server/http/verify_pin_test.go index 1c4a32b2dd..9ca4f2d21b 100644 --- a/services/auth-guest/pkg/server/http/verify_pin_test.go +++ b/services/auth-guest/pkg/server/http/verify_pin_test.go @@ -28,8 +28,8 @@ func newVerifyPinHandler(t *testing.T, svc authguest.AuthGuest) http.HandlerFunc CookieName: "__Host-oc_guest_session", TTL: time.Hour, }, - Challenge: config.Challenge{ - CookieName: testChallengeCookieName, + Nonce: config.Nonce{ + CookieName: testNonceCookieName, TTL: time.Hour, }, } @@ -39,13 +39,13 @@ func newVerifyPinHandler(t *testing.T, svc authguest.AuthGuest) http.HandlerFunc func TestVerifyPinHandler(t *testing.T) { svcMock := mocks.NewAuthGuest(t) expiresAt := time.Date(2026, 10, 15, 10, 30, 0, 0, time.FixedZone("CEST", 2*60*60)) - svcMock.On("VerifyPin", mock.Anything, "share-1", "123456", "challenge-nonce").Return(&authguest.SessionResponse{SessionToken: "session-token", ExpiresAt: expiresAt, ShareID: "share-1"}, nil) + svcMock.On("VerifyPin", mock.Anything, "share-1", "123456", "nonce-value").Return(&authguest.SessionResponse{SessionToken: "session-token", ExpiresAt: expiresAt, ShareID: "share-1"}, nil) body, err := json.Marshal(VerifyPinRequest{Pin: "123456", PermissionID: "share-1"}) require.NoError(t, err) req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(string(body))) - req.AddCookie(&http.Cookie{Name: testChallengeCookieName, Value: "challenge-nonce"}) + req.AddCookie(&http.Cookie{Name: testNonceCookieName, Value: "nonce-value"}) rr := httptest.NewRecorder() newVerifyPinHandler(t, svcMock)(rr, req) @@ -64,14 +64,14 @@ func TestVerifyPinHandler(t *testing.T) { assert.True(t, cookie.Secure) assert.Equal(t, "/", cookie.Path) - var challengeCookie *http.Cookie + var nonceCookie *http.Cookie for _, c := range rr.Result().Cookies() { - if c.Name == testChallengeCookieName { - challengeCookie = c + if c.Name == testNonceCookieName { + nonceCookie = c } } - require.NotNil(t, challengeCookie) - assert.Equal(t, -1, challengeCookie.MaxAge) + require.NotNil(t, nonceCookie) + assert.Equal(t, -1, nonceCookie.MaxAge) var resp verifyPinResponse require.NoError(t, json.NewDecoder(rr.Body).Decode(&resp)) @@ -102,17 +102,17 @@ func TestVerifyPinHandlerErrorMapping(t *testing.T) { wantPermission: "share-1", }, { - name: "challenge invalid", - err: &authguest.GuestError{ErrorType: authguest.ErrChallengeInvalid, ShareID: "share-1"}, + name: "nonce invalid", + err: &authguest.GuestError{ErrorType: authguest.ErrNonceInvalid, ShareID: "share-1"}, wantStatus: http.StatusUnauthorized, - wantType: "challengeInvalid", + wantType: "nonceInvalid", wantPermission: "share-1", }, { - name: "challenge expired", - err: &authguest.GuestError{ErrorType: authguest.ErrChallengeExpired, ShareID: "share-1"}, + name: "nonce expired", + err: &authguest.GuestError{ErrorType: authguest.ErrNonceExpired, ShareID: "share-1"}, wantStatus: http.StatusUnauthorized, - wantType: "challengeExpired", + wantType: "nonceExpired", wantPermission: "share-1", }, { @@ -146,7 +146,7 @@ func TestVerifyPinHandlerErrorMapping(t *testing.T) { require.NoError(t, err) req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(string(body))) - req.AddCookie(&http.Cookie{Name: testChallengeCookieName, Value: "challenge-nonce"}) + req.AddCookie(&http.Cookie{Name: testNonceCookieName, Value: "nonce-value"}) rr := httptest.NewRecorder() newVerifyPinHandler(t, svcMock)(rr, req) @@ -161,7 +161,30 @@ func TestVerifyPinHandlerErrorMapping(t *testing.T) { } } -func TestVerifyPinHandlerMissingChallengeCookie(t *testing.T) { +func TestVerifyPinHandlerTooManyAttempts(t *testing.T) { + svcMock := mocks.NewAuthGuest(t) + svcMock.On("VerifyPin", mock.Anything, "share-1", "123456", mock.Anything). + Return(nil, &authguest.GuestError{ErrorType: authguest.ErrTooManyAttempts, ShareID: "share-1", RetryAfter: 1500 * time.Millisecond}) + + body, err := json.Marshal(VerifyPinRequest{Pin: "123456", PermissionID: "share-1"}) + require.NoError(t, err) + + req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(string(body))) + req.AddCookie(&http.Cookie{Name: testNonceCookieName, Value: "nonce-value"}) + + rr := httptest.NewRecorder() + newVerifyPinHandler(t, svcMock)(rr, req) + + assert.Equal(t, http.StatusTooManyRequests, rr.Code) + assert.Equal(t, "2", rr.Header().Get("Retry-After")) + + var resp errorResponse + require.NoError(t, json.NewDecoder(rr.Body).Decode(&resp)) + assert.Equal(t, "tooManyRequests", resp.ErrorType) + assert.Equal(t, 2, resp.RetryAfter) +} + +func TestVerifyPinHandlerMissingNonceCookie(t *testing.T) { tests := []struct { name string addCookie bool @@ -180,7 +203,7 @@ func TestVerifyPinHandlerMissingChallengeCookie(t *testing.T) { req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(string(body))) if tt.addCookie { - req.AddCookie(&http.Cookie{Name: testChallengeCookieName, Value: tt.cookieValue}) + req.AddCookie(&http.Cookie{Name: testNonceCookieName, Value: tt.cookieValue}) } rr := httptest.NewRecorder() diff --git a/services/auth-guest/pkg/server/http/verify_token.go b/services/auth-guest/pkg/server/http/verify_token.go index bfdb71363a..5aebdce8d5 100644 --- a/services/auth-guest/pkg/server/http/verify_token.go +++ b/services/auth-guest/pkg/server/http/verify_token.go @@ -72,21 +72,21 @@ func setSessionCookie(w http.ResponseWriter, cfg *config.Config, sessionToken st }) } -func setChallengeCookie(w http.ResponseWriter, cfg *config.Config, nonce string) { +func setNonceCookie(w http.ResponseWriter, cfg *config.Config, nonce string) { http.SetCookie(w, &http.Cookie{ - Name: cfg.Challenge.CookieName, + Name: cfg.Nonce.CookieName, Value: nonce, Path: "/", HttpOnly: true, Secure: true, SameSite: http.SameSiteLaxMode, - MaxAge: int(cfg.Challenge.TTL.Seconds()), + MaxAge: int(cfg.Nonce.TTL.Seconds()), }) } -func clearChallengeCookie(w http.ResponseWriter, cfg *config.Config) { +func clearNonceCookie(w http.ResponseWriter, cfg *config.Config) { http.SetCookie(w, &http.Cookie{ - Name: cfg.Challenge.CookieName, + Name: cfg.Nonce.CookieName, Value: "", Path: "/", HttpOnly: true, diff --git a/services/auth-guest/pkg/service/authguest/errors.go b/services/auth-guest/pkg/service/authguest/errors.go new file mode 100644 index 0000000000..71e4921c3a --- /dev/null +++ b/services/auth-guest/pkg/service/authguest/errors.go @@ -0,0 +1,52 @@ +// Copyright 2026 OpenCloud GmbH +// SPDX-License-Identifier: Apache-2.0 + +package authguest + +import ( + "errors" + "time" + + "github.com/opencloud-eu/opencloud/services/auth-guest/pkg/service/guard" + "github.com/opencloud-eu/opencloud/services/auth-guest/pkg/service/storage" +) + +var ( + ErrExpired = errors.New("token expired") + ErrAlreadyRedeemed = errors.New("token already redeemed") + ErrShareNotFound = errors.New("share not found") + ErrShareExpired = errors.New("share expired") + ErrEventsNotConfigured = errors.New("event publisher not configured") + ErrPinInvalid = errors.New("pin invalid") + ErrPinExpired = errors.New("pin expired") + ErrNonceInvalid = errors.New("nonce invalid") + ErrNonceExpired = errors.New("nonce expired") + ErrTooManyAttempts = errors.New("too many attempts") + ErrPermissionMismatch = errors.New("permissionId does not match the provided credentials") +) + +// GuestError wraps an auth-guest failure together with the share id. The HTTP +// transport inspects ErrorType to choose a status code and message. +type GuestError struct { + ErrorType error + ShareID string + RetryAfter time.Duration +} + +func (e *GuestError) Error() string { return e.ErrorType.Error() } + +func nonceError(err error) error { + if errors.Is(err, guard.ErrNonceExpired) { + return ErrNonceExpired + } + return ErrNonceInvalid +} + +// nonceStoreError wraps a store error as a GuestError, treating a missing +// record as an invalid nonce. +func nonceStoreError(err error, shareID string) *GuestError { + if errors.Is(err, storage.ErrNotFound) { + err = ErrNonceInvalid + } + return &GuestError{ErrorType: err, ShareID: shareID} +} diff --git a/services/auth-guest/pkg/service/authguest/options.go b/services/auth-guest/pkg/service/authguest/options.go index 4cc368200c..2e40c6e240 100644 --- a/services/auth-guest/pkg/service/authguest/options.go +++ b/services/auth-guest/pkg/service/authguest/options.go @@ -19,7 +19,9 @@ type Options struct { ServiceAccount config.ServiceAccount JWT *jwt.JwtService Publisher events.Publisher - Challenge config.Challenge + Nonce config.Nonce + BackoffLimiter config.BackoffLimiter + QuotaLimiter config.QuotaLimiter } // GatewaySelector adds a grpc client selector for the gateway service @@ -50,9 +52,23 @@ func EventsPublisher(p events.Publisher) Option { } } -// Challenge configures the PIN challenge for the auth-guest service -func Challenge(challenge config.Challenge) Option { +// Nonce configures the browser-bound nonce for the auth-guest service +func Nonce(n config.Nonce) Option { return func(o *Options) { - o.Challenge = challenge + o.Nonce = n + } +} + +// BackoffLimiter configures the failed-attempt backoff limiter for the auth-guest service +func BackoffLimiter(l config.BackoffLimiter) Option { + return func(o *Options) { + o.BackoffLimiter = l + } +} + +// QuotaLimiter configures the renewal quota limiter for the auth-guest service +func QuotaLimiter(l config.QuotaLimiter) Option { + return func(o *Options) { + o.QuotaLimiter = l } } diff --git a/services/auth-guest/pkg/service/authguest/service.go b/services/auth-guest/pkg/service/authguest/service.go index a8dda9c6c1..786b30d68a 100644 --- a/services/auth-guest/pkg/service/authguest/service.go +++ b/services/auth-guest/pkg/service/authguest/service.go @@ -5,9 +5,6 @@ package authguest import ( "context" - "crypto/rand" - "crypto/subtle" - "encoding/base64" "errors" "fmt" "time" @@ -18,6 +15,7 @@ import ( "github.com/opencloud-eu/opencloud/pkg/events" "github.com/opencloud-eu/opencloud/services/auth-guest/pkg/config" + "github.com/opencloud-eu/opencloud/services/auth-guest/pkg/service/guard" "github.com/opencloud-eu/opencloud/services/auth-guest/pkg/service/jwt" "github.com/opencloud-eu/opencloud/services/auth-guest/pkg/service/pin" "github.com/opencloud-eu/opencloud/services/auth-guest/pkg/service/storage" @@ -27,45 +25,11 @@ import ( "github.com/opencloud-eu/reva/v2/pkg/utils" ) -var ErrExpired = errors.New("token expired") -var ErrAlreadyRedeemed = errors.New("token already redeemed") -var ErrShareNotFound = errors.New("share not found") -var ErrShareExpired = errors.New("share expired") -var ErrEventsNotConfigured = errors.New("event publisher not configured") -var ErrPinInvalid = errors.New("pin invalid") -var ErrPinExpired = errors.New("pin expired") - -// ErrChallengeInvalid is returned when the browser-bound challenge is missing -// or does not match the stored challenge. -var ErrChallengeInvalid = errors.New("challenge invalid") - -// ErrChallengeExpired is returned when the browser-bound challenge has expired. -var ErrChallengeExpired = errors.New("challenge expired") -var ErrPermissionMismatch = errors.New("permissionId does not match the provided credentials") - -const guestLinkTokenTTL = 30 * time.Minute -const guestPinTTL = 30 * time.Minute - -const nonceLength = 32 - -// generateNonce returns a random value used to bind a PIN challenge to the -// browser that requested the renewal. -func generateNonce() (string, error) { - b := make([]byte, nonceLength) - if _, err := rand.Read(b); err != nil { - return "", fmt.Errorf("could not generate random nonce: %w", err) - } - return base64.RawURLEncoding.EncodeToString(b), nil -} - -// GuestError wraps an auth-guest failure together with the share id. The HTTP -// transport inspects ErrorType to choose a status code and message. -type GuestError struct { - ErrorType error - ShareID string -} - -func (e *GuestError) Error() string { return e.ErrorType.Error() } +const ( + guestLinkTokenTTL = 30 * time.Minute + guestPinTTL = 30 * time.Minute + renewMaxAttempts = 3 +) // SessionResponse is the result of a successful authentication: a session token, // its expiry and the share id it grants access to. @@ -94,7 +58,15 @@ type AuthGuestService struct { serviceAccount config.ServiceAccount jwtService *jwt.JwtService publisher revaevents.Publisher - challenge config.Challenge + guards guards +} + +// guards bundles the rate limiters and the nonce protecting the guest +// endpoints. Their policies are fixed at construction from the service config. +type guards struct { + backoffLimiter guard.BackoffLimiter + quotaLimiter guard.QuotaLimiter + nonce guard.Nonce } func NewAuthGuestService(tokenSvc *token.TokenService, store storage.Manager, opts ...Option) *AuthGuestService { @@ -110,7 +82,11 @@ func NewAuthGuestService(tokenSvc *token.TokenService, store storage.Manager, op serviceAccount: o.ServiceAccount, jwtService: o.JWT, publisher: o.Publisher, - challenge: o.Challenge, + guards: guards{ + backoffLimiter: guard.NewBackoffLimiter(o.BackoffLimiter), + quotaLimiter: guard.NewQuotaLimiter(o.QuotaLimiter), + nonce: guard.NewNonce(o.Nonce), + }, } } @@ -120,13 +96,8 @@ func (s *AuthGuestService) CreateToken(ctx context.Context, shareID string) (*to return nil, err } - if err := s.store.Add(storage.Record{ - ShareID: shareID, - ShareIDHash: tok.ShareIDHash, - SecretHash: tok.SecretHash(), - Expiry: time.Now().Add(guestLinkTokenTTL), - Redeemed: false, - }); err != nil { + rec := storage.NewRecord(shareID, tok.ShareIDHash, tok.SecretHash(), time.Now().Add(guestLinkTokenTTL)) + if err := s.store.Add(rec); err != nil { return nil, err } @@ -166,9 +137,9 @@ func (s *AuthGuestService) VerifyToken(ctx context.Context, tokenString string) return &SessionResponse{SessionToken: sessionToken, ExpiresAt: expiresAt, ShareID: rec.ShareID}, nil } -// Renew generates a new guest link token, PIN and challenge nonce for shareID, +// Renew generates a new guest link token, PIN and nonce for shareID, // authorized by the previous link token or a (possibly expired) session token. -// The returned nonce binds the PIN challenge to the requesting browser. +// The returned nonce binds the PIN to the requesting browser. func (s *AuthGuestService) Renew(ctx context.Context, shareID, tokenString, sessionToken string) (string, error) { if s.publisher == nil { return "", ErrEventsNotConfigured @@ -178,6 +149,19 @@ func (s *AuthGuestService) Renew(ctx context.Context, shareID, tokenString, sess return "", err } + hash := token.Hash(shareID) + now := time.Now() + + current, err := s.store.Get(hash) + if err != nil { + return "", &GuestError{ErrorType: err, ShareID: shareID} + } + + // Reject an active cooldown before doing the (remote) share validation. + if wait := s.guards.quotaLimiter.RetryAfter(¤t.QuotaLimiterState, now); wait > 0 { + return "", &GuestError{ErrorType: ErrTooManyAttempts, ShareID: shareID, RetryAfter: wait} + } + share, err := s.validateShare(ctx, shareID) if err != nil { return "", err @@ -198,32 +182,8 @@ func (s *AuthGuestService) Renew(ctx context.Context, shareID, tokenString, sess return "", err } - nonce, err := generateNonce() + nonce, seen, updated, err := s.rotateGuestToken(hash, tok, pinHash, now) if err != nil { - return "", err - } - - now := time.Now() - - current, err := s.store.Get(token.Hash(shareID)) - if err != nil { - return "", &GuestError{ErrorType: err, ShareID: shareID} - } - - updated, err := s.store.UpdateFrom(*current, func(rec *storage.Record) error { - rec.SecretHash = tok.SecretHash() - rec.PinHash = pinHash - rec.Expiry = now.Add(guestLinkTokenTTL) - rec.PinExpiry = now.Add(guestPinTTL) - rec.ChallengeExpiry = now.Add(s.challenge.TTL) - rec.ChallengeNonceHash = token.Hash(nonce) - rec.Redeemed = false - return nil - }) - if err != nil { - if errors.Is(err, storage.ErrConflict) { - err = ErrAlreadyRedeemed - } return "", &GuestError{ErrorType: err, ShareID: shareID} } @@ -236,8 +196,8 @@ func (s *AuthGuestService) Renew(ctx context.Context, shareID, tokenString, sess Pin: pinStr, Timestamp: now, }); err != nil { - if _, rbErr := s.store.UpdateFrom(*updated, func(rec *storage.Record) error { - *rec = *current + if _, rbErr := s.store.UpdateFrom(updated, func(rec *storage.Record) error { + *rec = seen return nil }); rbErr != nil { return "", fmt.Errorf("publishing guest token renewed event failed: %w (rollback failed: %v)", err, rbErr) @@ -248,57 +208,35 @@ func (s *AuthGuestService) Renew(ctx context.Context, shareID, tokenString, sess return nonce, nil } -// VerifyPin validates a PIN and the browser-bound challenge nonce and exchanges +// VerifyPin validates a PIN and the browser-bound nonce and exchanges // them for a session token and the share id. func (s *AuthGuestService) VerifyPin(ctx context.Context, shareID, pinValue, nonce string) (*SessionResponse, error) { - rec, err := s.store.Get(token.Hash(shareID)) - if err != nil { - if errors.Is(err, storage.ErrNotFound) { - err = ErrChallengeInvalid - } - return nil, &GuestError{ErrorType: err, ShareID: shareID} - } + hash := token.Hash(shareID) - // this check is here because the validateShare call is pretty costlly, and it's better to fail here in case the - // challenge is invalid or expired before making the remote call. - if err := verifyChallenge(rec, nonce, time.Now()); err != nil { - return nil, &GuestError{ErrorType: err, ShareID: shareID} + if err := s.precheckPin(shareID, nonce); err != nil { + return nil, err } if _, err := s.validateShare(ctx, shareID); err != nil { return nil, err } - err = s.store.Update(token.Hash(shareID), func(rec *storage.Record) error { - if err := verifyChallenge(rec, nonce, time.Now()); err != nil { + // verifyPinAttempt reports a wrong PIN as ok=false with a nil error, so the + // failed-attempt state is persisted by Update; real errors abort the write. + var denied bool + err := s.store.Update(hash, func(rec *storage.Record) error { + ok, err := s.verifyPinAttempt(rec, pinValue, nonce) + if err != nil { return err } - - if rec.PinHash == "" { - return ErrPinInvalid - } - - if !rec.PinExpiry.IsZero() && rec.PinExpiry.Before(time.Now()) { - return ErrPinExpired - } - - ok, err := pin.Verify(pinValue, rec.PinHash) - if err != nil || !ok { - return ErrPinInvalid - } - - rec.PinHash = "" - rec.PinExpiry = time.Time{} - rec.ChallengeExpiry = time.Time{} - rec.ChallengeNonceHash = "" - + denied = !ok return nil }) if err != nil { - if errors.Is(err, storage.ErrNotFound) { - err = ErrChallengeInvalid - } - return nil, &GuestError{ErrorType: err, ShareID: shareID} + return nil, nonceStoreError(err, shareID) + } + if denied { + return nil, &GuestError{ErrorType: ErrPinInvalid, ShareID: shareID} } sessionToken, expiresAt, err := s.jwtService.Sign(shareID) @@ -320,6 +258,114 @@ func (s *AuthGuestService) CleanupShare(shareID string) error { return nil } +// rotateGuestToken replaces the guest token's secret, PIN and nonce +// under the record lock. When the record is modified concurrently it re-reads +// and re-applies the rotation, up to renewMaxAttempts. It returns the issued +// nonce, the record state seen before the successful update (for rollback) and +// the updated record. +func (s *AuthGuestService) rotateGuestToken(hash string, tok *token.Token, pinHash string, now time.Time) (string, storage.Record, storage.Record, error) { + var nonce string + for attempt := 1; ; attempt++ { + rec, err := s.store.Get(hash) + if err != nil { + return "", storage.Record{}, storage.Record{}, err + } + + updated, err := s.store.UpdateFrom(*rec, func(record *storage.Record) error { + if s.guards.quotaLimiter.RetryAfter(&record.QuotaLimiterState, now) > 0 { + return ErrTooManyAttempts + } + + n, err := s.guards.nonce.Issue(&record.NonceState, now) + if err != nil { + return err + } + nonce = n + record.BackoffLimiterState = guard.BackoffLimiterState{} + s.guards.quotaLimiter.Record(&record.QuotaLimiterState, now) + + record.SecretHash = tok.SecretHash() + record.PinHash = pinHash + record.Expiry = now.Add(guestLinkTokenTTL) + record.PinExpiry = now.Add(guestPinTTL) + record.Redeemed = false + return nil + }) + if err == nil { + return nonce, *rec, *updated, nil + } + if !errors.Is(err, storage.ErrConflict) || attempt >= renewMaxAttempts { + return "", storage.Record{}, storage.Record{}, err + } + } +} + +// checkPinRequest verifies the browser-bound nonce and then the PIN +// failed-attempt backoff, returning the error to surface to the client. The +// nonce is checked first so a missing or mismatched nonce is not hidden behind +// rate limiting. +func (s *AuthGuestService) checkPinRequest(rec *storage.Record, nonce string, now time.Time) *GuestError { + if err := s.guards.nonce.Verify(&rec.NonceState, nonce, now); err != nil { + return &GuestError{ErrorType: nonceError(err), ShareID: rec.ShareID} + } + if wait := s.guards.backoffLimiter.RetryAfter(&rec.BackoffLimiterState, now); wait > 0 { + return &GuestError{ErrorType: ErrTooManyAttempts, ShareID: rec.ShareID, RetryAfter: wait} + } + return nil +} + +// precheckPin cheaply rejects a bad or blocked request before the (remote) +// share validation. The same checks run again under the record lock in +// verifyPinAttempt, which is what actually consumes the attempt. +func (s *AuthGuestService) precheckPin(shareID, nonce string) error { + rec, err := s.store.Get(token.Hash(shareID)) + if err != nil { + return nonceStoreError(err, shareID) + } + + ge := s.checkPinRequest(rec, nonce, time.Now()) + if ge != nil { + return ge + } + return nil +} + +// verifyPinAttempt verifies the PIN under the caller's lock and updates the +// record: a success clears the credential, a wrong PIN records the attempt and +// its backoff. Gate and expiry failures leave the record untouched and return an +// error. +func (s *AuthGuestService) verifyPinAttempt(rec *storage.Record, pinValue, nonce string) (ok bool, err error) { + now := time.Now() + + // The pre-check already surfaced Retry-After; the wait is dropped here + // because a store update cannot carry HTTP headers. + ge := s.checkPinRequest(rec, nonce, now) + if ge != nil { + return false, ge.ErrorType + } + + switch { + case rec.PinHash == "": + return false, ErrPinInvalid + case !rec.PinExpiry.IsZero() && rec.PinExpiry.Before(now): + return false, ErrPinExpired + } + + match, _ := pin.Verify(pinValue, rec.PinHash) + if !match && !s.guards.backoffLimiter.Record(&rec.BackoffLimiterState, now) { + // Wrong PIN, but attempts remain: keep the credential and its nonce. + return false, nil + } + + // Either the PIN was correct (credential consumed) or the attempts are + // exhausted (credential invalidated); both spend the single-use nonce. + rec.PinHash = "" + rec.PinExpiry = time.Time{} + s.guards.nonce.Clear(&rec.NonceState) + s.guards.backoffLimiter.Reset(&rec.BackoffLimiterState) + return match, nil +} + // verifyRenewCredentials checks that the previous link token or a (possibly // expired) session token belongs to shareID. func (s *AuthGuestService) verifyRenewCredentials(shareID, tokenString, sessionToken string) error { @@ -448,17 +494,3 @@ func (s *AuthGuestService) getShare(ctx context.Context, shareID string) (*colla return share, nil } - -// verifyChallenge validates the browser-bound challenge nonce against the record -func verifyChallenge(rec *storage.Record, nonce string, now time.Time) error { - if rec.ChallengeNonceHash == "" || - subtle.ConstantTimeCompare([]byte(token.Hash(nonce)), []byte(rec.ChallengeNonceHash)) != 1 { - return ErrChallengeInvalid - } - - if !rec.ChallengeExpiry.IsZero() && rec.ChallengeExpiry.Before(now) { - return ErrChallengeExpired - } - - return nil -} diff --git a/services/auth-guest/pkg/service/authguest/service_test.go b/services/auth-guest/pkg/service/authguest/service_test.go index 6914ad09ce..cc6231db02 100644 --- a/services/auth-guest/pkg/service/authguest/service_test.go +++ b/services/auth-guest/pkg/service/authguest/service_test.go @@ -19,6 +19,7 @@ import ( provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" ocEvents "github.com/opencloud-eu/opencloud/pkg/events" "github.com/opencloud-eu/opencloud/services/auth-guest/pkg/config" + "github.com/opencloud-eu/opencloud/services/auth-guest/pkg/service/guard" "github.com/opencloud-eu/opencloud/services/auth-guest/pkg/service/jwt" "github.com/opencloud-eu/opencloud/services/auth-guest/pkg/service/pin" "github.com/opencloud-eu/opencloud/services/auth-guest/pkg/service/storage" @@ -36,8 +37,6 @@ import ( const testShareID = "e0123456-7890-abcd-ef01-234567890abc" -const testNonce = "test-nonce" - type testPublisher struct { published []any err error @@ -170,6 +169,17 @@ func newAuthGuestService(t *testing.T, store storage.Manager, gwc *cs3mocks.Gate GatewaySelector(newGatewayTestSelector(gwc)), ServiceAccount(config.ServiceAccount{ServiceAccountID: "sa-id", ServiceAccountSecret: "sa-secret"}), JWT(jwt.NewJwtService("test-secret", time.Hour)), + Nonce(config.Nonce{TTL: time.Minute}), + BackoffLimiter(config.BackoffLimiter{ + MaxAttempts: 5, + BackoffBase: time.Second, + BackoffCap: 30 * time.Second, + }), + QuotaLimiter(config.QuotaLimiter{ + Cooldown: time.Minute, + Window: 24 * time.Hour, + DailyCap: 20, + }), ) } @@ -374,6 +384,14 @@ func TestVerifyTokenConflictIsAlreadyRedeemed(t *testing.T) { } func newRenewService(t *testing.T, store storage.Manager, gwc *cs3mocks.GatewayAPIClient, publisher revaevents.Publisher) *AuthGuestService { + t.Helper() + return newRenewServiceWithLimiter(t, store, gwc, publisher, config.QuotaLimiter{ + Window: 24 * time.Hour, + DailyCap: 100, + }) +} + +func newRenewServiceWithLimiter(t *testing.T, store storage.Manager, gwc *cs3mocks.GatewayAPIClient, publisher revaevents.Publisher, quota config.QuotaLimiter) *AuthGuestService { t.Helper() return NewAuthGuestService( token.NewTokenService(), @@ -382,6 +400,13 @@ func newRenewService(t *testing.T, store storage.Manager, gwc *cs3mocks.GatewayA ServiceAccount(config.ServiceAccount{ServiceAccountID: "sa-id", ServiceAccountSecret: "sa-secret"}), JWT(jwt.NewJwtService("test-secret", 24*time.Hour)), EventsPublisher(publisher), + Nonce(config.Nonce{TTL: time.Hour}), + BackoffLimiter(config.BackoffLimiter{ + MaxAttempts: 5, + BackoffBase: time.Second, + BackoffCap: 30 * time.Second, + }), + QuotaLimiter(quota), ) } @@ -392,6 +417,35 @@ func existingRecord() storage.Record { } } +// pinRecord returns a record holding a freshly issued nonce, together with the +// nonce to pass to VerifyPin. +func pinRecord(t *testing.T, pinHash string, pinExpiry time.Time, backoff guard.BackoffLimiterState, nonceExpiry time.Time) (storage.Record, string) { + t.Helper() + + var state guard.NonceState + nonce, err := guard.NewNonce(config.Nonce{TTL: time.Hour}).Issue(&state, time.Now()) + require.NoError(t, err) + if !nonceExpiry.IsZero() { + state.Expiry = nonceExpiry + } + + return storage.Record{ + ShareID: testShareID, + ShareIDHash: token.Hash(testShareID), + PinHash: pinHash, + PinExpiry: pinExpiry, + NonceState: state, + BackoffLimiterState: backoff, + }, nonce +} + +func validShareResponse() *collaboration.GetShareResponse { + return &collaboration.GetShareResponse{ + Status: &rpc.Status{Code: rpc.Code_CODE_OK}, + Share: newGuestShare(time.Now().Add(time.Hour)), + } +} + func TestRenewPersistsRecordAndPublishesEvent(t *testing.T) { store := storage.NewFileManager(t.TempDir()) tok, rec := newToken(t) @@ -417,7 +471,8 @@ func TestRenewPersistsRecordAndPublishesEvent(t *testing.T) { assert.True(t, strings.HasPrefix(replaced.PinHash, "$argon2id$")) assert.WithinDuration(t, time.Now().Add(guestLinkTokenTTL), replaced.Expiry, time.Minute) assert.WithinDuration(t, time.Now().Add(guestPinTTL), replaced.PinExpiry, time.Minute) - assert.Equal(t, token.Hash(nonce), replaced.ChallengeNonceHash) + assert.Equal(t, token.Hash(nonce), replaced.NonceState.Hash) + assert.NoError(t, guard.NewNonce(config.Nonce{TTL: time.Hour}).Verify(&replaced.NonceState, nonce, time.Now())) assert.False(t, replaced.Redeemed) require.Len(t, publisher.published, 1) @@ -480,6 +535,46 @@ func TestRenewLinkTokenIgnoresRedeemedAndExpiry(t *testing.T) { require.NoError(t, err) } +// conflictOnceStore returns ErrConflict on the first UpdateFrom, simulating a +// concurrent modification, then delegates to the wrapped Manager. +type conflictOnceStore struct { + storage.Manager + updates int +} + +func (s *conflictOnceStore) UpdateFrom(seen storage.Record, fn func(*storage.Record) error) (*storage.Record, error) { + s.updates++ + if s.updates == 1 { + return nil, storage.ErrConflict + } + return s.Manager.UpdateFrom(seen, fn) +} + +func TestRenewRetriesOnConflict(t *testing.T) { + store := storage.NewFileManager(t.TempDir()) + tok, rec := newToken(t) + require.NoError(t, store.Add(rec)) + + publisher := &testPublisher{} + gwc := newGatewayMock(validShareResponse()) + flaky := &conflictOnceStore{Manager: store} + s := newRenewService(t, flaky, gwc, publisher) + + nonce, err := s.Renew(context.Background(), testShareID, tok, "") + require.NoError(t, err) + assert.NotEmpty(t, nonce) + assert.Equal(t, 2, flaky.updates) + + renewed, err := store.Get(token.Hash(testShareID)) + require.NoError(t, err) + assert.Equal(t, rec.Revision+1, renewed.Revision) + assert.NoError(t, guard.NewNonce(config.Nonce{TTL: time.Hour}).Verify(&renewed.NonceState, nonce, time.Now())) + require.Len(t, publisher.published, 1) + + // The remote share is validated only once; the retry re-reads the record only. + gwc.AssertNumberOfCalls(t, "GetShare", 1) +} + func TestRenewInvalidCredentials(t *testing.T) { store := storage.NewFileManager(t.TempDir()) s := newRenewService(t, store, newGatewayMock(&collaboration.GetShareResponse{}), &testPublisher{}) @@ -639,25 +734,94 @@ func TestRenewShareExpired(t *testing.T) { assert.Equal(t, testShareID, ge.ShareID) } +func TestRenewRecordsCooldown(t *testing.T) { + store := storage.NewFileManager(t.TempDir()) + tok, rec := newToken(t) + require.NoError(t, store.Add(rec)) + + s := newRenewServiceWithLimiter(t, store, newGatewayMock(validShareResponse()), &testPublisher{}, config.QuotaLimiter{ + Cooldown: time.Minute, + Window: 24 * time.Hour, + DailyCap: 20, + }) + + _, err := s.Renew(context.Background(), testShareID, tok, "") + require.NoError(t, err) + + got, err := store.Get(token.Hash(testShareID)) + require.NoError(t, err) + assert.Equal(t, 1, got.QuotaLimiterState.Count) + assert.False(t, got.QuotaLimiterState.WindowStart.IsZero()) + assert.WithinDuration(t, time.Now().Add(time.Minute), got.QuotaLimiterState.NotBefore, time.Second) +} + +func TestRenewBlockedDuringCooldown(t *testing.T) { + store := storage.NewFileManager(t.TempDir()) + tok, rec := newToken(t) + rec.QuotaLimiterState = guard.QuotaLimiterState{ + Count: 1, + NotBefore: time.Now().Add(time.Minute), + WindowStart: time.Now(), + } + require.NoError(t, store.Add(rec)) + + gwc := newGatewayMock(validShareResponse()) + s := newRenewServiceWithLimiter(t, store, gwc, &testPublisher{}, config.QuotaLimiter{ + Cooldown: time.Minute, + Window: 24 * time.Hour, + DailyCap: 20, + }) + + _, err := s.Renew(context.Background(), testShareID, tok, "") + require.Error(t, err) + + var ge *GuestError + require.ErrorAs(t, err, &ge) + assert.ErrorIs(t, ge.ErrorType, ErrTooManyAttempts) + assert.Greater(t, ge.RetryAfter, time.Duration(0)) + + gwc.AssertNotCalled(t, "GetShare", mock.Anything, mock.Anything) +} + +func TestRenewDailyCap(t *testing.T) { + store := storage.NewFileManager(t.TempDir()) + tok, rec := newToken(t) + rec.QuotaLimiterState = guard.QuotaLimiterState{ + Count: 20, + NotBefore: time.Now().Add(time.Minute), + WindowStart: time.Now(), + } + require.NoError(t, store.Add(rec)) + + gwc := newGatewayMock(validShareResponse()) + s := newRenewServiceWithLimiter(t, store, gwc, &testPublisher{}, config.QuotaLimiter{ + Cooldown: time.Minute, + Window: 24 * time.Hour, + DailyCap: 20, + }) + + _, err := s.Renew(context.Background(), testShareID, tok, "") + require.Error(t, err) + + var ge *GuestError + require.ErrorAs(t, err, &ge) + assert.ErrorIs(t, ge.ErrorType, ErrTooManyAttempts) + assert.Greater(t, ge.RetryAfter, time.Duration(0)) + + gwc.AssertNotCalled(t, "GetShare", mock.Anything, mock.Anything) +} + func TestVerifyPinSuccess(t *testing.T) { pinHash, err := pin.Hash("123456") require.NoError(t, err) + rec, nonce := pinRecord(t, pinHash, time.Now().Add(time.Minute), guard.BackoffLimiterState{}, time.Time{}) store := storage.NewFileManager(t.TempDir()) - require.NoError(t, store.Add(storage.Record{ - ShareID: testShareID, - ShareIDHash: token.Hash(testShareID), - PinHash: pinHash, - PinExpiry: time.Now().Add(time.Minute), - ChallengeNonceHash: token.Hash(testNonce), - })) + require.NoError(t, store.Add(rec)) - s := newAuthGuestService(t, store, newGatewayMock(&collaboration.GetShareResponse{ - Status: &rpc.Status{Code: rpc.Code_CODE_OK}, - Share: newGuestShare(time.Now().Add(time.Hour)), - })) + s := newAuthGuestService(t, store, newGatewayMock(validShareResponse())) - result, err := s.VerifyPin(context.Background(), testShareID, "123456", testNonce) + result, err := s.VerifyPin(context.Background(), testShareID, "123456", nonce) require.NoError(t, err) assert.NotEmpty(t, result.SessionToken) assert.WithinDuration(t, time.Now().Add(time.Hour), result.ExpiresAt, 2*time.Second) @@ -667,80 +831,67 @@ func TestVerifyPinSuccess(t *testing.T) { require.NoError(t, err) assert.Empty(t, got.PinHash) assert.True(t, got.PinExpiry.IsZero()) - assert.True(t, got.ChallengeExpiry.IsZero()) - assert.Empty(t, got.ChallengeNonceHash) + assert.Empty(t, got.NonceState.Hash) } func TestVerifyPinErrors(t *testing.T) { pinHash, err := pin.Hash("123456") require.NoError(t, err) - validRecord := storage.Record{ - ShareID: testShareID, - ShareIDHash: token.Hash(testShareID), - PinHash: pinHash, - PinExpiry: time.Now().Add(time.Minute), - ChallengeNonceHash: token.Hash(testNonce), + validShare := validShareResponse() + shareNotFound := &collaboration.GetShareResponse{ + Status: &rpc.Status{Code: rpc.Code_CODE_NOT_FOUND}, } - expiredRecord := storage.Record{ - ShareID: testShareID, - ShareIDHash: token.Hash(testShareID), - PinHash: pinHash, - PinExpiry: time.Now().Add(-time.Minute), - ChallengeNonceHash: token.Hash(testNonce), - } - challengeExpiredRecord := storage.Record{ - ShareID: testShareID, - ShareIDHash: token.Hash(testShareID), - PinHash: pinHash, - PinExpiry: time.Now().Add(time.Minute), - ChallengeExpiry: time.Now().Add(-time.Minute), - ChallengeNonceHash: token.Hash(testNonce), - } - noPinRecord := storage.Record{ - ShareID: testShareID, - ShareIDHash: token.Hash(testShareID), - ChallengeNonceHash: token.Hash(testNonce), + shareExpired := &collaboration.GetShareResponse{ + Status: &rpc.Status{Code: rpc.Code_CODE_OK}, + Share: newGuestShare(time.Now().Add(-time.Hour)), } - validShare := &collaboration.GetShareResponse{ - Status: &rpc.Status{Code: rpc.Code_CODE_OK}, - Share: newGuestShare(time.Now().Add(time.Hour)), + freshRecord := func(t *testing.T) (storage.Record, string) { + return pinRecord(t, pinHash, time.Now().Add(time.Minute), guard.BackoffLimiterState{}, time.Time{}) } tests := []struct { name string pin string - rec storage.Record + build func(t *testing.T) (storage.Record, string) missing bool response *collaboration.GetShareResponse wantErr error }{ { - name: "invalid pin", - pin: "000000", - rec: validRecord, + name: "invalid pin", + pin: "000000", + build: func(t *testing.T) (storage.Record, string) { + return freshRecord(t) + }, response: validShare, wantErr: ErrPinInvalid, }, { - name: "expired pin", - pin: "123456", - rec: expiredRecord, + name: "expired pin", + pin: "123456", + build: func(t *testing.T) (storage.Record, string) { + return pinRecord(t, pinHash, time.Now().Add(-time.Minute), guard.BackoffLimiterState{}, time.Time{}) + }, response: validShare, wantErr: ErrPinExpired, }, { - name: "expired challenge", - pin: "123456", - rec: challengeExpiredRecord, + name: "expired nonce", + pin: "123456", + build: func(t *testing.T) (storage.Record, string) { + return pinRecord(t, pinHash, time.Now().Add(time.Minute), guard.BackoffLimiterState{}, time.Now().Add(-time.Minute)) + }, response: validShare, - wantErr: ErrChallengeExpired, + wantErr: ErrNonceExpired, }, { - name: "no pin set", - pin: "123456", - rec: noPinRecord, + name: "no pin set", + pin: "123456", + build: func(t *testing.T) (storage.Record, string) { + return pinRecord(t, "", time.Time{}, guard.BackoffLimiterState{}, time.Time{}) + }, response: validShare, wantErr: ErrPinInvalid, }, @@ -749,39 +900,46 @@ func TestVerifyPinErrors(t *testing.T) { pin: "123456", missing: true, response: validShare, - wantErr: ErrChallengeInvalid, + wantErr: ErrNonceInvalid, }, { name: "share not found", pin: "123456", - rec: validRecord, - response: &collaboration.GetShareResponse{ - Status: &rpc.Status{Code: rpc.Code_CODE_NOT_FOUND}, + build: func(t *testing.T) (storage.Record, string) { + return freshRecord(t) }, - wantErr: ErrShareNotFound, + response: shareNotFound, + wantErr: ErrShareNotFound, }, { name: "share expired", pin: "123456", - rec: validRecord, - response: &collaboration.GetShareResponse{ - Status: &rpc.Status{Code: rpc.Code_CODE_OK}, - Share: newGuestShare(time.Now().Add(-time.Hour)), + build: func(t *testing.T) (storage.Record, string) { + return freshRecord(t) }, - wantErr: ErrShareExpired, + response: shareExpired, + wantErr: ErrShareExpired, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + var ( + rec storage.Record + nonce string + ) + if !tt.missing { + rec, nonce = tt.build(t) + } + store := storage.NewFileManager(t.TempDir()) if !tt.missing { - require.NoError(t, store.Add(tt.rec)) + require.NoError(t, store.Add(rec)) } s := newAuthGuestService(t, store, newGatewayMock(tt.response)) - _, err := s.VerifyPin(context.Background(), testShareID, tt.pin, testNonce) + _, err := s.VerifyPin(context.Background(), testShareID, tt.pin, nonce) require.Error(t, err) var ge *GuestError @@ -792,9 +950,9 @@ func TestVerifyPinErrors(t *testing.T) { if !tt.missing { got, err := store.Get(token.Hash(testShareID)) require.NoError(t, err) - assert.Equal(t, tt.rec.PinHash, got.PinHash) - assert.WithinDuration(t, tt.rec.PinExpiry, got.PinExpiry, time.Second) - assert.Equal(t, tt.rec.Redeemed, got.Redeemed) + assert.Equal(t, rec.PinHash, got.PinHash) + assert.WithinDuration(t, rec.PinExpiry, got.PinExpiry, time.Second) + assert.Equal(t, rec.Redeemed, got.Redeemed) } }) } @@ -804,14 +962,6 @@ func TestVerifyPinNonceMismatch(t *testing.T) { pinHash, err := pin.Hash("123456") require.NoError(t, err) - rec := storage.Record{ - ShareID: testShareID, - ShareIDHash: token.Hash(testShareID), - PinHash: pinHash, - PinExpiry: time.Now().Add(time.Minute), - ChallengeNonceHash: token.Hash(testNonce), - } - tests := []struct { name string nonce string @@ -822,62 +972,58 @@ func TestVerifyPinNonceMismatch(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + rec, _ := pinRecord(t, pinHash, time.Now().Add(time.Minute), guard.BackoffLimiterState{}, time.Time{}) + store := storage.NewFileManager(t.TempDir()) require.NoError(t, store.Add(rec)) - s := newAuthGuestService(t, store, newGatewayMock(&collaboration.GetShareResponse{ - Status: &rpc.Status{Code: rpc.Code_CODE_OK}, - Share: newGuestShare(time.Now().Add(time.Hour)), - })) + s := newAuthGuestService(t, store, newGatewayMock(validShareResponse())) _, err := s.VerifyPin(context.Background(), testShareID, "123456", tt.nonce) require.Error(t, err) var ge *GuestError require.ErrorAs(t, err, &ge) - assert.ErrorIs(t, ge.ErrorType, ErrChallengeInvalid) + assert.ErrorIs(t, ge.ErrorType, ErrNonceInvalid) got, err := store.Get(token.Hash(testShareID)) require.NoError(t, err) assert.Equal(t, rec.PinHash, got.PinHash) - assert.Equal(t, rec.ChallengeNonceHash, got.ChallengeNonceHash) + assert.NotEmpty(t, got.NonceState.Hash) }) } } -func TestVerifyPinChallengeRejectedBeforeGateway(t *testing.T) { +func TestVerifyPinNonceRejectedBeforeGateway(t *testing.T) { pinHash, err := pin.Hash("123456") require.NoError(t, err) tests := []struct { - name string - nonce string - challengeExpiry time.Time - wantErr error + name string + override string + expiry time.Time + wantErr error }{ - {name: "wrong nonce", nonce: "wrong-nonce", wantErr: ErrChallengeInvalid}, - {name: "expired challenge", nonce: testNonce, challengeExpiry: time.Now().Add(-time.Minute), wantErr: ErrChallengeExpired}, + {name: "wrong nonce", override: "wrong-nonce", wantErr: ErrNonceInvalid}, + {name: "expired nonce", expiry: time.Now().Add(-time.Minute), wantErr: ErrNonceExpired}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - store := storage.NewFileManager(t.TempDir()) - require.NoError(t, store.Add(storage.Record{ - ShareID: testShareID, - ShareIDHash: token.Hash(testShareID), - PinHash: pinHash, - PinExpiry: time.Now().Add(time.Minute), - ChallengeNonceHash: token.Hash(testNonce), - ChallengeExpiry: tt.challengeExpiry, - })) + rec, issued := pinRecord(t, pinHash, time.Now().Add(time.Minute), guard.BackoffLimiterState{}, tt.expiry) - gwc := newGatewayMock(&collaboration.GetShareResponse{ - Status: &rpc.Status{Code: rpc.Code_CODE_OK}, - Share: newGuestShare(time.Now().Add(time.Hour)), - }) + store := storage.NewFileManager(t.TempDir()) + require.NoError(t, store.Add(rec)) + + gwc := newGatewayMock(validShareResponse()) s := newAuthGuestService(t, store, gwc) - _, err := s.VerifyPin(context.Background(), testShareID, "123456", tt.nonce) + nonce := tt.override + if nonce == "" { + nonce = issued + } + + _, err := s.VerifyPin(context.Background(), testShareID, "123456", nonce) require.Error(t, err) var ge *GuestError @@ -890,23 +1036,117 @@ func TestVerifyPinChallengeRejectedBeforeGateway(t *testing.T) { } } +func TestVerifyPinFailedAttemptSetsBackoff(t *testing.T) { + pinHash, err := pin.Hash("123456") + require.NoError(t, err) + + rec, nonce := pinRecord(t, pinHash, time.Now().Add(time.Minute), guard.BackoffLimiterState{}, time.Time{}) + store := storage.NewFileManager(t.TempDir()) + require.NoError(t, store.Add(rec)) + + s := newAuthGuestService(t, store, newGatewayMock(validShareResponse())) + + _, err = s.VerifyPin(context.Background(), testShareID, "000000", nonce) + require.Error(t, err) + + var ge *GuestError + require.ErrorAs(t, err, &ge) + assert.ErrorIs(t, ge.ErrorType, ErrPinInvalid) + + got, err := store.Get(token.Hash(testShareID)) + require.NoError(t, err) + assert.Equal(t, 1, got.BackoffLimiterState.Count) + assert.WithinDuration(t, time.Now().Add(time.Second), got.BackoffLimiterState.NotBefore, time.Second) + assert.NotEmpty(t, got.PinHash, "PIN must not be invalidated before MaxAttempts") +} + +func TestVerifyPinBlockedDuringBackoff(t *testing.T) { + pinHash, err := pin.Hash("123456") + require.NoError(t, err) + + rec, nonce := pinRecord(t, pinHash, time.Now().Add(time.Minute), guard.BackoffLimiterState{ + Count: 1, + NotBefore: time.Now().Add(time.Minute), + }, time.Time{}) + + store := storage.NewFileManager(t.TempDir()) + require.NoError(t, store.Add(rec)) + + gwc := newGatewayMock(validShareResponse()) + s := newAuthGuestService(t, store, gwc) + + _, err = s.VerifyPin(context.Background(), testShareID, "123456", nonce) + require.Error(t, err) + + var ge *GuestError + require.ErrorAs(t, err, &ge) + assert.ErrorIs(t, ge.ErrorType, ErrTooManyAttempts) + assert.Greater(t, ge.RetryAfter, time.Duration(0)) + + gwc.AssertNotCalled(t, "GetShare", mock.Anything, mock.Anything) +} + +func TestVerifyPinInvalidatedAfterMaxAttempts(t *testing.T) { + pinHash, err := pin.Hash("123456") + require.NoError(t, err) + + rec, nonce := pinRecord(t, pinHash, time.Now().Add(time.Minute), guard.BackoffLimiterState{ + Count: 4, + NotBefore: time.Now().Add(-time.Second), + }, time.Time{}) + + store := storage.NewFileManager(t.TempDir()) + require.NoError(t, store.Add(rec)) + + s := newAuthGuestService(t, store, newGatewayMock(validShareResponse())) + + _, err = s.VerifyPin(context.Background(), testShareID, "000000", nonce) + require.Error(t, err) + + var ge *GuestError + require.ErrorAs(t, err, &ge) + assert.ErrorIs(t, ge.ErrorType, ErrPinInvalid) + + got, err := store.Get(token.Hash(testShareID)) + require.NoError(t, err) + assert.Empty(t, got.PinHash) + assert.Empty(t, got.NonceState.Hash) + + // The cleared nonce rejects the next attempt. + _, err = s.VerifyPin(context.Background(), testShareID, "123456", nonce) + require.Error(t, err) + + var ge2 *GuestError + require.ErrorAs(t, err, &ge2) + assert.ErrorIs(t, ge2.ErrorType, ErrNonceInvalid) +} + +func TestRenewResetsPinAttempts(t *testing.T) { + store := storage.NewFileManager(t.TempDir()) + tok, rec := newToken(t) + rec.BackoffLimiterState = guard.BackoffLimiterState{Count: 3, NotBefore: time.Now().Add(time.Minute)} + require.NoError(t, store.Add(rec)) + + s := newRenewService(t, store, newGatewayMock(validShareResponse()), &testPublisher{}) + + _, err := s.Renew(context.Background(), testShareID, tok, "") + require.NoError(t, err) + + got, err := store.Get(token.Hash(testShareID)) + require.NoError(t, err) + assert.Zero(t, got.BackoffLimiterState.Count) + assert.True(t, got.BackoffLimiterState.NotBefore.IsZero()) +} + func TestVerifyPinSingleUseConcurrent(t *testing.T) { pinHash, err := pin.Hash("123456") require.NoError(t, err) + rec, nonce := pinRecord(t, pinHash, time.Now().Add(time.Minute), guard.BackoffLimiterState{}, time.Time{}) store := storage.NewFileManager(t.TempDir()) - require.NoError(t, store.Add(storage.Record{ - ShareID: testShareID, - ShareIDHash: token.Hash(testShareID), - PinHash: pinHash, - PinExpiry: time.Now().Add(time.Minute), - ChallengeNonceHash: token.Hash(testNonce), - })) + require.NoError(t, store.Add(rec)) - s := newAuthGuestService(t, store, newGatewayMock(&collaboration.GetShareResponse{ - Status: &rpc.Status{Code: rpc.Code_CODE_OK}, - Share: newGuestShare(time.Now().Add(time.Hour)), - })) + s := newAuthGuestService(t, store, newGatewayMock(validShareResponse())) const workers = 10 var ( @@ -917,7 +1157,7 @@ func TestVerifyPinSingleUseConcurrent(t *testing.T) { wg.Add(1) go func() { defer wg.Done() - if _, err := s.VerifyPin(context.Background(), testShareID, "123456", testNonce); err == nil { + if _, err := s.VerifyPin(context.Background(), testShareID, "123456", nonce); err == nil { success.Add(1) } }() diff --git a/services/auth-guest/pkg/service/guard/backoff_limiter.go b/services/auth-guest/pkg/service/guard/backoff_limiter.go new file mode 100644 index 0000000000..3b2c8df5d1 --- /dev/null +++ b/services/auth-guest/pkg/service/guard/backoff_limiter.go @@ -0,0 +1,65 @@ +// Copyright 2026 OpenCloud GmbH +// SPDX-License-Identifier: Apache-2.0 + +// Package guard provides generic building blocks for restricting access to +// endpoints: a browser-bound nonce, a backoff limiter and a quota limiter. The +// state is designed to be persisted per guarded resource; the policies come +// from the service config package, so guard depends on it. +package guard + +import ( + "time" + + "github.com/opencloud-eu/opencloud/services/auth-guest/pkg/config" +) + +const backoffFactor = 2 + +// BackoffLimiterState is the persisted state of a BackoffLimiter. +type BackoffLimiterState struct { + Count int `json:"count,omitempty"` + NotBefore time.Time `json:"notbefore,omitzero"` +} + +// BackoffLimiter counts events, blocks once the configured maximum is reached +// and grows the wait between events exponentially. +type BackoffLimiter struct { + maxAttempts int + base time.Duration + cap time.Duration +} + +// NewBackoffLimiter returns a BackoffLimiter configured from cfg. +func NewBackoffLimiter(cfg config.BackoffLimiter) BackoffLimiter { + return BackoffLimiter{maxAttempts: cfg.MaxAttempts, base: cfg.BackoffBase, cap: cfg.BackoffCap} +} + +// RetryAfter returns the remaining wait before the next event is allowed, or 0. +func (l BackoffLimiter) RetryAfter(s *BackoffLimiterState, now time.Time) time.Duration { + if s.NotBefore.After(now) { + return s.NotBefore.Sub(now) + } + return 0 +} + +// Record records an event and reports whether the limit is exhausted. +func (l BackoffLimiter) Record(s *BackoffLimiterState, now time.Time) (exhausted bool) { + s.Count++ + s.NotBefore = now.Add(delay(l.base, l.cap, s.Count)) + return s.Count >= l.maxAttempts +} + +// Reset clears the persisted state. +func (l BackoffLimiter) Reset(s *BackoffLimiterState) { + *s = BackoffLimiterState{} +} + +// delay returns the wait before the next attempt after the given number of +// consecutive failures: min(base*2^(failures-1), maxWait). +func delay(base, maxWait time.Duration, failures int) time.Duration { + wait := min(base, maxWait) + for i := 1; i < failures; i++ { + wait = min(wait*backoffFactor, maxWait) + } + return wait +} diff --git a/services/auth-guest/pkg/service/guard/backoff_limiter_test.go b/services/auth-guest/pkg/service/guard/backoff_limiter_test.go new file mode 100644 index 0000000000..df6a8aa2fb --- /dev/null +++ b/services/auth-guest/pkg/service/guard/backoff_limiter_test.go @@ -0,0 +1,52 @@ +// Copyright 2026 OpenCloud GmbH +// SPDX-License-Identifier: Apache-2.0 + +package guard + +import ( + "testing" + "time" + + "github.com/opencloud-eu/opencloud/services/auth-guest/pkg/config" + "github.com/stretchr/testify/assert" +) + +func TestBackoffLimiterExponentialBackoff(t *testing.T) { + now := time.Now() + limiter := NewBackoffLimiter(config.BackoffLimiter{MaxAttempts: 5, BackoffBase: time.Second, BackoffCap: 30 * time.Second}) + var s BackoffLimiterState + want := []time.Duration{time.Second, 2 * time.Second, 4 * time.Second, 8 * time.Second, 16 * time.Second} + + for i, w := range want { + exhausted := limiter.Record(&s, now) + assert.Equal(t, i+1, s.Count, "failure %d", i+1) + assert.WithinDuration(t, now.Add(w), s.NotBefore, time.Millisecond, "failure %d", i+1) + assert.Equal(t, i+1 >= 5, exhausted, "failure %d", i+1) + assert.Equal(t, w, limiter.RetryAfter(&s, now), "retry after %d", i+1) + assert.Zero(t, limiter.RetryAfter(&s, now.Add(w)), "retry after elapsed %d", i+1) + } +} + +func TestBackoffLimiterCap(t *testing.T) { + now := time.Now() + limiter := NewBackoffLimiter(config.BackoffLimiter{MaxAttempts: 10, BackoffBase: time.Second, BackoffCap: 4 * time.Second}) + var s BackoffLimiterState + want := []time.Duration{time.Second, 2 * time.Second, 4 * time.Second, 4 * time.Second, 4 * time.Second} + + for i, w := range want { + limiter.Record(&s, now) + assert.WithinDuration(t, now.Add(w), s.NotBefore, time.Millisecond, "attempt %d", i+1) + } +} + +func TestBackoffLimiterReset(t *testing.T) { + limiter := NewBackoffLimiter(config.BackoffLimiter{MaxAttempts: 5, BackoffBase: time.Second, BackoffCap: 30 * time.Second}) + s := BackoffLimiterState{ + Count: 3, + NotBefore: time.Now().Add(time.Minute), + } + + limiter.Reset(&s) + assert.Zero(t, s.Count) + assert.True(t, s.NotBefore.IsZero()) +} diff --git a/services/auth-guest/pkg/service/guard/nonce.go b/services/auth-guest/pkg/service/guard/nonce.go new file mode 100644 index 0000000000..069e4fc07e --- /dev/null +++ b/services/auth-guest/pkg/service/guard/nonce.go @@ -0,0 +1,78 @@ +// Copyright 2026 OpenCloud GmbH +// SPDX-License-Identifier: Apache-2.0 + +package guard + +import ( + "crypto/rand" + "crypto/sha256" + "crypto/subtle" + "encoding/base64" + "errors" + "time" + + "github.com/opencloud-eu/opencloud/services/auth-guest/pkg/config" +) + +// Nonce verification errors. +var ( + ErrNonceMissing = errors.New("nonce missing") + ErrNonceMismatch = errors.New("nonce mismatch") + ErrNonceExpired = errors.New("nonce expired") +) + +const nonceLength = 32 + +// NonceState is the persisted browser-bound nonce. +type NonceState struct { + Hash string `json:"hash,omitempty"` + Expiry time.Time `json:"expiry,omitzero"` +} + +// Nonce issues and verifies browser-bound nonces. +type Nonce struct { + ttl time.Duration +} + +// NewNonce returns a Nonce configured from cfg. +func NewNonce(cfg config.Nonce) Nonce { + return Nonce{ttl: cfg.TTL} +} + +// Issue generates a nonce, stores its hash and expiry, and returns the raw +// nonce to be handed to the client. +func (n Nonce) Issue(s *NonceState, now time.Time) (string, error) { + b := make([]byte, nonceLength) + _, err := rand.Read(b) + if err != nil { + return "", err + } + value := base64.RawURLEncoding.EncodeToString(b) + s.Hash = hash(value) + s.Expiry = now.Add(n.ttl) + return value, nil +} + +// Verify checks the nonce against the stored hash and expiry. +func (n Nonce) Verify(s *NonceState, value string, now time.Time) error { + if s.Hash == "" { + return ErrNonceMissing + } + if subtle.ConstantTimeCompare([]byte(hash(value)), []byte(s.Hash)) != 1 { + return ErrNonceMismatch + } + if !s.Expiry.IsZero() && s.Expiry.Before(now) { + return ErrNonceExpired + } + return nil +} + +// Clear removes the stored nonce. +func (n Nonce) Clear(s *NonceState) { + *s = NonceState{} +} + +func hash(nonce string) string { + sum := sha256.Sum256([]byte(nonce)) + return base64.RawURLEncoding.EncodeToString(sum[:]) +} diff --git a/services/auth-guest/pkg/service/guard/nonce_test.go b/services/auth-guest/pkg/service/guard/nonce_test.go new file mode 100644 index 0000000000..fd156aa99c --- /dev/null +++ b/services/auth-guest/pkg/service/guard/nonce_test.go @@ -0,0 +1,56 @@ +// Copyright 2026 OpenCloud GmbH +// SPDX-License-Identifier: Apache-2.0 + +package guard + +import ( + "testing" + "time" + + "github.com/opencloud-eu/opencloud/services/auth-guest/pkg/config" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNonceIssueAndVerify(t *testing.T) { + now := time.Now() + n := NewNonce(config.Nonce{TTL: time.Minute}) + var s NonceState + + nonce, err := n.Issue(&s, now) + require.NoError(t, err) + assert.NotEmpty(t, nonce) + assert.NotEmpty(t, s.Hash) + + require.NoError(t, n.Verify(&s, nonce, now)) + assert.ErrorIs(t, n.Verify(&s, "other", now), ErrNonceMismatch) +} + +func TestNonceVerifyMissing(t *testing.T) { + n := NewNonce(config.Nonce{TTL: time.Minute}) + var s NonceState + assert.ErrorIs(t, n.Verify(&s, "whatever", time.Now()), ErrNonceMissing) +} + +func TestNonceVerifyExpired(t *testing.T) { + now := time.Now() + n := NewNonce(config.Nonce{TTL: time.Minute}) + var s NonceState + + nonce, err := n.Issue(&s, now) + require.NoError(t, err) + assert.ErrorIs(t, n.Verify(&s, nonce, now.Add(2*time.Minute)), ErrNonceExpired) +} + +func TestNonceClear(t *testing.T) { + now := time.Now() + n := NewNonce(config.Nonce{TTL: time.Minute}) + var s NonceState + + nonce, err := n.Issue(&s, now) + require.NoError(t, err) + + n.Clear(&s) + assert.Empty(t, s.Hash) + assert.ErrorIs(t, n.Verify(&s, nonce, now), ErrNonceMissing) +} diff --git a/services/auth-guest/pkg/service/guard/quota_limiter.go b/services/auth-guest/pkg/service/guard/quota_limiter.go new file mode 100644 index 0000000000..c5a1ae443c --- /dev/null +++ b/services/auth-guest/pkg/service/guard/quota_limiter.go @@ -0,0 +1,60 @@ +// Copyright 2026 OpenCloud GmbH +// SPDX-License-Identifier: Apache-2.0 + +package guard + +import ( + "time" + + "github.com/opencloud-eu/opencloud/services/auth-guest/pkg/config" +) + +// QuotaLimiterState is the persisted state of a QuotaLimiter. +type QuotaLimiterState struct { + Count int `json:"count,omitempty"` + NotBefore time.Time `json:"notbefore,omitzero"` + WindowStart time.Time `json:"windowstart,omitzero"` +} + +// QuotaLimiter allows a maximum number of events per window, with a minimum +// cooldown between them. +type QuotaLimiter struct { + max int + cooldown time.Duration + window time.Duration +} + +// NewQuotaLimiter returns a QuotaLimiter configured from cfg. +func NewQuotaLimiter(cfg config.QuotaLimiter) QuotaLimiter { + return QuotaLimiter{max: cfg.DailyCap, cooldown: cfg.Cooldown, window: cfg.Window} +} + +// RetryAfter returns the remaining wait before the next event is allowed, or 0. +func (l QuotaLimiter) RetryAfter(s *QuotaLimiterState, now time.Time) time.Duration { + var wait time.Duration + if l.window > 0 && now.Sub(s.WindowStart) < l.window && s.Count >= l.max { + wait = l.window - now.Sub(s.WindowStart) + } + if s.NotBefore.After(now) { + if w := s.NotBefore.Sub(now); w > wait { + wait = w + } + } + return wait +} + +// Record records an event and reports whether the limit is exhausted. +func (l QuotaLimiter) Record(s *QuotaLimiterState, now time.Time) (exhausted bool) { + if l.window > 0 && (s.WindowStart.IsZero() || now.Sub(s.WindowStart) >= l.window) { + s.WindowStart = now + s.Count = 0 + } + s.Count++ + s.NotBefore = now.Add(l.cooldown) + return s.Count >= l.max +} + +// Reset clears the persisted state. +func (l QuotaLimiter) Reset(s *QuotaLimiterState) { + *s = QuotaLimiterState{} +} diff --git a/services/auth-guest/pkg/service/guard/quota_limiter_test.go b/services/auth-guest/pkg/service/guard/quota_limiter_test.go new file mode 100644 index 0000000000..3e446fb28a --- /dev/null +++ b/services/auth-guest/pkg/service/guard/quota_limiter_test.go @@ -0,0 +1,56 @@ +// Copyright 2026 OpenCloud GmbH +// SPDX-License-Identifier: Apache-2.0 + +package guard + +import ( + "testing" + "time" + + "github.com/opencloud-eu/opencloud/services/auth-guest/pkg/config" + "github.com/stretchr/testify/assert" +) + +func TestQuotaLimiterCooldownAndWindow(t *testing.T) { + now := time.Now() + limiter := NewQuotaLimiter(config.QuotaLimiter{DailyCap: 2, Cooldown: time.Minute, Window: time.Hour}) + var s QuotaLimiterState + + assert.Zero(t, limiter.RetryAfter(&s, now)) + + limiter.Record(&s, now) + assert.Equal(t, time.Minute, limiter.RetryAfter(&s, now)) + assert.Zero(t, limiter.RetryAfter(&s, now.Add(time.Minute))) + + limiter.Record(&s, now) + assert.Equal(t, time.Hour, limiter.RetryAfter(&s, now)) + + assert.Zero(t, limiter.RetryAfter(&s, now.Add(2*time.Hour))) + limiter.Record(&s, now.Add(2*time.Hour)) + assert.Equal(t, 1, s.Count) +} + +func TestQuotaLimiterCooldownOutlastsWindow(t *testing.T) { + now := time.Now() + limiter := NewQuotaLimiter(config.QuotaLimiter{DailyCap: 2, Cooldown: 2 * time.Hour, Window: time.Hour}) + var s QuotaLimiterState + + limiter.Record(&s, now) + limiter.Record(&s, now) + + assert.Equal(t, 2*time.Hour, limiter.RetryAfter(&s, now)) +} + +func TestQuotaLimiterReset(t *testing.T) { + limiter := NewQuotaLimiter(config.QuotaLimiter{DailyCap: 2, Cooldown: time.Minute, Window: time.Hour}) + s := QuotaLimiterState{ + Count: 3, + NotBefore: time.Now().Add(time.Minute), + WindowStart: time.Now(), + } + + limiter.Reset(&s) + assert.Zero(t, s.Count) + assert.True(t, s.NotBefore.IsZero()) + assert.True(t, s.WindowStart.IsZero()) +} diff --git a/services/auth-guest/pkg/service/storage/file_manager_test.go b/services/auth-guest/pkg/service/storage/file_manager_test.go index 3b15894954..eb29d5a2eb 100644 --- a/services/auth-guest/pkg/service/storage/file_manager_test.go +++ b/services/auth-guest/pkg/service/storage/file_manager_test.go @@ -11,6 +11,7 @@ import ( "testing" "time" + "github.com/opencloud-eu/opencloud/services/auth-guest/pkg/service/guard" "github.com/opencloud-eu/opencloud/services/auth-guest/pkg/service/token" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -40,20 +41,26 @@ func TestFileManagerAddGet(t *testing.T) { assert.Equal(t, rec, *got) } -func TestFileManagerChallengeAndRenewFieldsRoundTrip(t *testing.T) { +func TestFileManagerGuardsRoundTrip(t *testing.T) { dir := t.TempDir() s := NewFileManager(dir) rec := newRecord("e0123456-7890-abcd-ef01-234567890abc") rec.PinHash = "pinhash" rec.PinExpiry = time.Date(2026, 2, 3, 4, 5, 6, 0, time.UTC) - rec.ChallengeNonceHash = "noncehash" - rec.ChallengeExpiry = time.Date(2026, 2, 3, 4, 5, 9, 0, time.UTC) - rec.PinFailures = 2 - rec.PinNotBefore = time.Date(2026, 2, 3, 4, 5, 7, 0, time.UTC) - rec.LastRenewAt = time.Date(2026, 2, 3, 4, 5, 8, 0, time.UTC) - rec.RenewCount = 3 - rec.RenewWindowStart = time.Date(2026, 2, 3, 0, 0, 0, 0, time.UTC) + rec.NonceState = guard.NonceState{ + Hash: "noncehash", + Expiry: time.Date(2026, 2, 3, 4, 5, 9, 0, time.UTC), + } + rec.BackoffLimiterState = guard.BackoffLimiterState{ + Count: 2, + NotBefore: time.Date(2026, 2, 3, 4, 5, 7, 0, time.UTC), + } + rec.QuotaLimiterState = guard.QuotaLimiterState{ + Count: 3, + NotBefore: time.Date(2026, 2, 3, 4, 5, 8, 0, time.UTC), + WindowStart: time.Date(2026, 2, 3, 0, 0, 0, 0, time.UTC), + } require.NoError(t, s.Add(rec)) diff --git a/services/auth-guest/pkg/service/storage/storage.go b/services/auth-guest/pkg/service/storage/storage.go index 7c1da3789e..38f39d79ec 100644 --- a/services/auth-guest/pkg/service/storage/storage.go +++ b/services/auth-guest/pkg/service/storage/storage.go @@ -6,6 +6,8 @@ package storage import ( "errors" "time" + + "github.com/opencloud-eu/opencloud/services/auth-guest/pkg/service/guard" ) var ErrNotFound = errors.New("record not found") @@ -14,23 +16,27 @@ var ErrConflict = errors.New("record was modified concurrently") // Record holds the data persisted for a guest share token. type Record struct { - ShareID string `json:"shareid"` - ShareIDHash string `json:"shareidhash"` - SecretHash string `json:"secrethash"` - PinHash string `json:"pinhash"` - Expiry time.Time `json:"expiry,omitzero"` - PinExpiry time.Time `json:"pinexpiry,omitzero"` - Redeemed bool `json:"redeemed"` - Revision uint64 `json:"revision"` + ShareID string `json:"shareid"` + ShareIDHash string `json:"shareidhash"` + SecretHash string `json:"secrethash"` + PinHash string `json:"pinhash"` + Expiry time.Time `json:"expiry,omitzero"` + PinExpiry time.Time `json:"pinexpiry,omitzero"` + Redeemed bool `json:"redeemed"` + Revision uint64 `json:"revision"` + NonceState guard.NonceState `json:"nonce,omitzero"` + BackoffLimiterState guard.BackoffLimiterState `json:"backoff_limiter,omitzero"` + QuotaLimiterState guard.QuotaLimiterState `json:"quota_limiter,omitzero"` +} - // The following fields are needed for rate limiting. - ChallengeNonceHash string `json:"challengenoncehash,omitempty"` - ChallengeExpiry time.Time `json:"challengeexpiry,omitzero"` - PinFailures int `json:"pinfailures,omitempty"` - PinNotBefore time.Time `json:"pinnotbefore,omitzero"` - LastRenewAt time.Time `json:"lastrenewat,omitzero"` - RenewCount int `json:"renewcount,omitempty"` - RenewWindowStart time.Time `json:"renewwindowstart,omitzero"` +// NewRecord returns a record for the given token. +func NewRecord(shareID, shareIDHash, secretHash string, expiry time.Time) Record { + return Record{ + ShareID: shareID, + ShareIDHash: shareIDHash, + SecretHash: secretHash, + Expiry: expiry, + } } type Manager interface {