Skip to content

Commit 510e68b

Browse files
authored
fix: session upgrade percentage should be based on session, not request (#2371)
In #2356 a session would be upgraded to v2 refresh tokens based on the number of requests for that session. If you set a percentage value of 10% and there's 100 refresh token requests per session, all sessions would be upgraded within 1 day. This is rectified here by converting the session ID to a value in the `[0, 100)` range making sure that a random selection of sessions would be upgraded consistently.
1 parent f1fabc4 commit 510e68b

1 file changed

Lines changed: 10 additions & 1 deletion

File tree

internal/tokens/service.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"encoding/json"
66
"fmt"
7+
"hash/crc32"
78
mathRand "math/rand"
89
"net/http"
910
"net/url"
@@ -414,7 +415,15 @@ func (s *Service) RefreshTokenGrant(ctx context.Context, db *storage.Connection,
414415

415416
issuedToken = newToken.Token
416417

417-
shouldUpgrade := config.Security.RefreshTokenAlgorithmVersion == 2 && (config.Security.RefreshTokenUpgradePercentage >= 100 || mathRand.Intn(100) < config.Security.RefreshTokenUpgradePercentage) // #nosec
418+
shouldUpgrade := config.Security.RefreshTokenAlgorithmVersion == 2 && config.Security.RefreshTokenUpgradePercentage > 0
419+
420+
if shouldUpgrade && config.Security.RefreshTokenUpgradePercentage < 100 {
421+
// convert the session ID to a number in the range [0, 100) and check whether it should be upgraded
422+
// we don't want a % of refresh token requests, but a % of sessions here!
423+
424+
sessionRand := mathRand.New(mathRand.NewSource(int64(crc32.ChecksumIEEE(session.ID.Bytes())))) // #nosec
425+
shouldUpgrade = sessionRand.Intn(100) < config.Security.RefreshTokenUpgradePercentage // #nosec
426+
}
418427

419428
if shouldUpgrade {
420429
// got v1 refresh token that should be upgraded to v2

0 commit comments

Comments
 (0)