From 3dca98db92f649aa4e61af8ddf93afe4bb136f31 Mon Sep 17 00:00:00 2001 From: silverwind Date: Thu, 7 May 2026 22:54:55 +0200 Subject: [PATCH] refactor(oauth2): apply review feedback to refresh error handling - use errors.As so wrapped *oauth2.RetrieveError still matches - early-return on non-invalid_grant errors instead of if/else - log UserID instead of ExternalID to avoid leaking provider-side identifiers (which can be username/email) into INFO logs Co-Authored-By: Claude (Opus 4.7) --- services/auth/source/oauth2/source_sync.go | 25 +++++++++++----------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/services/auth/source/oauth2/source_sync.go b/services/auth/source/oauth2/source_sync.go index 6c14f99775..171d1c9ea7 100644 --- a/services/auth/source/oauth2/source_sync.go +++ b/services/auth/source/oauth2/source_sync.go @@ -5,6 +5,7 @@ package oauth2 import ( "context" + "errors" "time" user_model "code.gitea.io/gitea/models/user" @@ -49,20 +50,20 @@ func (source *Source) refresh(ctx context.Context, provider goth.Provider, u *us token, err := provider.RefreshToken(u.RefreshToken) if err != nil { - if retrieveErr, ok := err.(*oauth2.RetrieveError); ok && retrieveErr.ErrorCode == "invalid_grant" { - log.Info("SyncExternalUsers[%s] dropping invalid refresh token for external login %s", source.AuthSource.Name, u.ExternalID) - - // Refresh tokens can expire or be revoked independently from the - // upstream account state. Keep the local user active and only clear - // the cached tokens until the next successful OAuth sign-in updates them. - u.AccessToken = "" - u.RefreshToken = "" - u.ExpiresAt = time.Time{} - - return user_model.UpdateExternalUserByExternalID(ctx, u) - } else { + var retrieveErr *oauth2.RetrieveError + if !errors.As(err, &retrieveErr) || retrieveErr.ErrorCode != "invalid_grant" { return err } + log.Info("SyncExternalUsers[%s] dropping invalid refresh token for user %d", source.AuthSource.Name, u.UserID) + + // Refresh tokens can expire or be revoked independently from the + // upstream account state. Keep the local user active and only clear + // the cached tokens until the next successful OAuth sign-in updates them. + u.AccessToken = "" + u.RefreshToken = "" + u.ExpiresAt = time.Time{} + + return user_model.UpdateExternalUserByExternalID(ctx, u) } // Otherwise, update the tokens