From 26c323b5a62a32036c923a3442670509754f64de Mon Sep 17 00:00:00 2001
From: Misi <mgyongyosi@users.noreply.github.com>
Date: Wed, 18 Dec 2024 15:30:39 +0100
Subject: [PATCH] Chore: Do not return the token on refresh error (#98171)

* Small improvements

* Remove unused error
---
 pkg/services/oauthtoken/oauth_token.go | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/pkg/services/oauthtoken/oauth_token.go b/pkg/services/oauthtoken/oauth_token.go
index 62708b0ff98..c54959c8e79 100644
--- a/pkg/services/oauthtoken/oauth_token.go
+++ b/pkg/services/oauthtoken/oauth_token.go
@@ -34,7 +34,6 @@ var (
 	ExpiryDelta            = 10 * time.Second
 	ErrNoRefreshTokenFound = errors.New("no refresh token found")
 	ErrNotAnOAuthProvider  = errors.New("not an oauth provider")
-	ErrCouldntRefreshToken = errors.New("could not refresh token")
 	ErrRetriesExhausted    = errors.New("retries exhausted")
 )
 
@@ -151,6 +150,8 @@ func (o *Service) GetCurrentOAuthToken(ctx context.Context, usr identity.Request
 			return persistedToken
 		}
 
+		ctxLogger.Error("Failed to refresh OAuth token", "error", err)
+
 		return nil
 	}
 
@@ -353,7 +354,6 @@ func (o *Service) InvalidateOAuthTokens(ctx context.Context, usr identity.Reques
 		}
 	}
 
-	// TODO: Should this run regardless of the feature flag?
 	return o.AuthInfoService.UpdateAuthInfo(ctx, &login.UpdateAuthInfoCommand{
 		UserId:     userID,
 		AuthModule: usr.GetAuthenticatedBy(),
@@ -394,13 +394,13 @@ func (o *Service) tryGetOrRefreshOAuthToken(ctx context.Context, persistedToken
 	connect, err := o.SocialService.GetConnector(authProvider)
 	if err != nil {
 		ctxLogger.Error("Failed to get oauth connector", "provider", authProvider, "error", err)
-		return persistedToken, err
+		return nil, err
 	}
 
 	client, err := o.SocialService.GetOAuthHttpClient(authProvider)
 	if err != nil {
 		ctxLogger.Error("Failed to get oauth http client", "provider", authProvider, "error", err)
-		return persistedToken, err
+		return nil, err
 	}
 	ctx = context.WithValue(ctx, oauth2.HTTPClient, client)
 
@@ -411,6 +411,7 @@ func (o *Service) tryGetOrRefreshOAuthToken(ctx context.Context, persistedToken
 	o.tokenRefreshDuration.WithLabelValues(authProvider, fmt.Sprintf("%t", err == nil)).Observe(duration.Seconds())
 
 	if err != nil {
+		span.SetAttributes(attribute.Bool("token_refreshed", false))
 		ctxLogger.Error("Failed to retrieve oauth access token",
 			"provider", usr.GetAuthenticatedBy(), "error", err)
 
@@ -422,6 +423,8 @@ func (o *Service) tryGetOrRefreshOAuthToken(ctx context.Context, persistedToken
 		return nil, err
 	}
 
+	span.SetAttributes(attribute.Bool("token_refreshed", true))
+
 	// If the tokens are not the same, update the entry in the DB
 	if !tokensEq(persistedToken, token) {
 		updateAuthCommand := &login.UpdateAuthInfoCommand{
@@ -443,7 +446,7 @@ func (o *Service) tryGetOrRefreshOAuthToken(ctx context.Context, persistedToken
 		if !o.features.IsEnabledGlobally(featuremgmt.FlagImprovedExternalSessionHandling) {
 			if err := o.AuthInfoService.UpdateAuthInfo(ctx, updateAuthCommand); err != nil {
 				ctxLogger.Error("Failed to update auth info during token refresh", "authID", usr.GetAuthID(), "error", err)
-				return token, err
+				return nil, err
 			}
 		}
 
@@ -451,7 +454,7 @@ func (o *Service) tryGetOrRefreshOAuthToken(ctx context.Context, persistedToken
 			Token: token,
 		}); err != nil {
 			ctxLogger.Error("Failed to update external session during token refresh", "error", err)
-			return token, err
+			return nil, err
 		}
 
 		ctxLogger.Debug("Updated oauth info for user")