Skip to content

Commit ffc1931

Browse files
fix: perform best effort cleanup for concurrently created refresh tokens
Signed-off-by: Danil Grigorev <daniil.grigorev.dev@gmail.com>
1 parent 5d04dfb commit ffc1931

2 files changed

Lines changed: 51 additions & 2 deletions

File tree

server/handlers.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1769,11 +1769,13 @@ func (s *Server) handlePasswordGrant(w http.ResponseWriter, r *http.Request, cli
17691769
return
17701770
}
17711771
} else {
1772+
inconsitentDB := false
17721773
if oldTokenRef, ok := session.Refresh[tokenRef.ClientID]; ok {
17731774
// Delete old refresh token from storage.
17741775
if err := s.storage.DeleteRefresh(ctx, oldTokenRef.ID); err != nil {
17751776
if err == storage.ErrNotFound {
17761777
s.logger.Warn("database inconsistent, refresh token missing", "token_id", oldTokenRef.ID)
1778+
inconsitentDB = true
17771779
} else {
17781780
s.logger.ErrorContext(r.Context(), "failed to delete refresh token", "err", err)
17791781
s.tokenErrHelper(w, errServerError, "", http.StatusInternalServerError)
@@ -1785,6 +1787,12 @@ func (s *Server) handlePasswordGrant(w http.ResponseWriter, r *http.Request, cli
17851787

17861788
// Update existing OfflineSession obj with new RefreshTokenRef.
17871789
if err := s.storage.UpdateOfflineSessions(ctx, session.UserID, session.ConnID, func(old storage.OfflineSessions) (storage.OfflineSessions, error) {
1790+
// Perform removal of the old concurrent/uncached refresh token from a freshly fetched instance
1791+
if inconsitentDB && old.Refresh[tokenRef.ClientID] != nil {
1792+
if err := s.storage.DeleteRefresh(ctx, old.Refresh[tokenRef.ClientID].ID); err != nil {
1793+
s.logger.Warn("failover cleanup failed, database inconsistent, refresh token missing", "token_id", old.Refresh[tokenRef.ClientID].ID, "err", err)
1794+
}
1795+
}
17881796
old.Refresh[tokenRef.ClientID] = &tokenRef
17891797
old.ConnectorData = identity.ConnectorData
17901798
return old, nil

storage/kubernetes/storage.go

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -395,8 +395,17 @@ func (cli *client) ListClients(ctx context.Context) ([]storage.Client, error) {
395395
return nil, errors.New("not implemented")
396396
}
397397

398-
func (cli *client) ListRefreshTokens(ctx context.Context) ([]storage.RefreshToken, error) {
399-
return nil, errors.New("not implemented")
398+
func (cli *client) ListRefreshTokens(ctx context.Context) (tokens []storage.RefreshToken, err error) {
399+
var refreshTokenList RefreshList
400+
if err = cli.list(resourceRefreshToken, &refreshTokenList); err != nil {
401+
return tokens, fmt.Errorf("failed to list passwords: %v", err)
402+
}
403+
404+
for _, token := range refreshTokenList.RefreshTokens {
405+
tokens = append(tokens, toStorageRefreshToken(token))
406+
}
407+
408+
return
400409
}
401410

402411
func (cli *client) ListPasswords(ctx context.Context) (passwords []storage.Password, err error) {
@@ -679,6 +688,25 @@ func (cli *client) GarbageCollect(ctx context.Context, now time.Time) (result st
679688
}
680689
}
681690

691+
refreshTokens, err := cli.ListRefreshTokens(ctx)
692+
if err != nil {
693+
return result, err
694+
}
695+
696+
for _, refreshToken := range refreshTokens {
697+
matches, err := cli.refreshTokenMatchesOfflineSession(ctx, refreshToken)
698+
if err != nil {
699+
return result, err
700+
}
701+
if matches {
702+
continue
703+
}
704+
if err := cli.DeleteRefresh(ctx, refreshToken.ID); err != nil && err != storage.ErrNotFound {
705+
cli.logger.Error("failed to delete orphan refresh token", "err", err)
706+
delErr = fmt.Errorf("failed to delete orphan refresh token: %v", err)
707+
}
708+
}
709+
682710
var deviceTokens DeviceTokenList
683711
if err := cli.listN(resourceDeviceToken, &deviceTokens, gcResultLimit); err != nil {
684712
return result, fmt.Errorf("failed to list device tokens: %v", err)
@@ -716,6 +744,19 @@ func (cli *client) GarbageCollect(ctx context.Context, now time.Time) (result st
716744
return result, delErr
717745
}
718746

747+
func (cli *client) refreshTokenMatchesOfflineSession(ctx context.Context, refreshToken storage.RefreshToken) (bool, error) {
748+
offlineSessions, err := cli.GetOfflineSessions(ctx, refreshToken.Claims.UserID, refreshToken.ConnectorID)
749+
if err != nil {
750+
if err == storage.ErrNotFound {
751+
return false, nil
752+
}
753+
return false, err
754+
}
755+
756+
ref := offlineSessions.Refresh[refreshToken.ClientID]
757+
return ref != nil && ref.ID == refreshToken.ID, nil
758+
}
759+
719760
func (cli *client) CreateDeviceRequest(ctx context.Context, d storage.DeviceRequest) error {
720761
return cli.post(resourceDeviceRequest, cli.fromStorageDeviceRequest(d))
721762
}

0 commit comments

Comments
 (0)