Recover OAuth credentials marked invalid instead of requiring a manual re-login

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### Bug Fixes
 
+- **Authentication**: OAuth credentials that were previously marked invalid are now re-validated by attempting a token refresh instead of being skipped forever. A credential invalidated by a transient token-endpoint error now self-heals on the next refresh, so users no longer see a persistent "There was an error connecting to ... Please log in again." message in every new window despite still being signed in.
 - **RovoDev**: Hid stack traces, stderr, and log details from external users while preserving them for Atlassian users.
 - **RovoDev (BBY)**: Fixed `ROVODEV_REBRAND_JCA` env var handling so the "Jira Coding Agent" rebrand works correctly in webviews.
 - **Notifications**: Fixed `atlassianNotificationNotifier` to correctly flush all promise levels, resolving a test reliability issue.

src/atlclients/authStore.test.ts

@@ -1169,8 +1169,7 @@ describe('CredentialManager', () => {
             expect(Logger.debug).toHaveBeenCalledWith(expect.stringContaining('permanent previous failure'));
         });
 
-        it('should skip refreshAccessToken when OAuth credentials are already invalid', async () => {
-            const Logger = require('../logger').Logger;
+        it('should attempt a recovery refresh when OAuth credentials are marked invalid', async () => {
             const site = { ...mockJiraSite, isCloud: true };
             const invalidOAuthInfo: OAuthInfo = {
                 ...mockOAuthInfo,
@@ -1182,46 +1181,82 @@ describe('CredentialManager', () => {
             const saveAuthInfoSpy = jest.spyOn(credentialManager as any, 'saveAuthInfo');
             saveAuthInfoSpy.mockResolvedValue(true);
 
-            Logger.debug.mockClear();
-            mockRefresher.getNewTokens.mockClear();
+            mockRefresher.getNewTokens.mockResolvedValue({
+                tokens: {
+                    accessToken: 'new-access-token',
+                    refreshToken: 'new-refresh-token',
+                    expiration: Date.now() + 3600000,
+                    receivedAt: Date.now(),
+                },
+                shouldSlowDown: false,
+                shouldInvalidate: false,
+            });
 
-            const result = await (credentialManager as any).refreshAccessToken(site);
+            await (credentialManager as any).refreshAccessToken(site);
 
-            expect(result).toBeUndefined();
-            expect(mockRefresher.getNewTokens).not.toHaveBeenCalled();
-            expect(saveAuthInfoSpy).not.toHaveBeenCalled();
-            expect(Logger.debug).toHaveBeenCalledWith(expect.stringContaining('credentials are invalid'));
+            expect(mockRefresher.getNewTokens).toHaveBeenCalled();
+            expect(saveAuthInfoSpy).toHaveBeenCalledWith(
+                site,
+                expect.objectContaining({
+                    state: AuthInfoState.Valid,
+                    access: 'new-access-token',
+                }),
+            );
+        });
+
+        it('should keep credentials invalid when the recovery refresh fails permanently', async () => {
+            const site = { ...mockJiraSite, isCloud: true };
+            const invalidOAuthInfo: OAuthInfo = {
+                ...mockOAuthInfo,
+                state: AuthInfoState.Invalid,
+            };
+
+            const getAuthInfoSpy = jest.spyOn(credentialManager as any, 'getAuthInfoForProductAndCredentialId');
+            getAuthInfoSpy.mockResolvedValue(invalidOAuthInfo);
+            const saveAuthInfoSpy = jest.spyOn(credentialManager as any, 'saveAuthInfo');
+            saveAuthInfoSpy.mockResolvedValue(true);
 
+            mockRefresher.getNewTokens.mockResolvedValue({
+                tokens: undefined,
+                shouldInvalidate: true,
+                shouldSlowDown: false,
+            });
+
+            await (credentialManager as any).refreshAccessToken(site);
+
+            expect(saveAuthInfoSpy).toHaveBeenCalledWith(
+                site,
+                expect.objectContaining({ state: AuthInfoState.Invalid }),
+            );
             const failedRefresh = (credentialManager as any)._failedRefreshCache.get(site.credentialId);
             expect(failedRefresh?.permanentFailure).toBe(true);
         });
 
-        it('should skip token refresh when credentials state is Invalid', async () => {
-            const Logger = require('../logger').Logger;
+        it('should not retry a recovery refresh after a permanent failure in the same session', async () => {
             const site = { ...mockJiraSite, isCloud: true };
-            const invalidOAuthInfo = {
+            const invalidOAuthInfo: OAuthInfo = {
                 ...mockOAuthInfo,
                 state: AuthInfoState.Invalid,
             };
 
-            const getAuthInfoForProductSpy = jest.spyOn(
-                credentialManager as any,
-                'getAuthInfoForProductAndCredentialId',
-            );
-            getAuthInfoForProductSpy.mockResolvedValue(invalidOAuthInfo);
-            const softRefreshSpy = jest.spyOn(credentialManager as any, 'softRefreshOAuth');
-            softRefreshSpy.mockImplementation(async (site: DetailedSiteInfo, authInfo: AuthInfo) => {
-                if (!authInfo || authInfo.state === AuthInfoState.Invalid) {
-                    Logger.debug(`Skipping token refresh for ${site.baseApiUrl}; credentials are invalid.`);
-                    return authInfo;
-                }
-                return authInfo;
+            const getAuthInfoSpy = jest.spyOn(credentialManager as any, 'getAuthInfoForProductAndCredentialId');
+            getAuthInfoSpy.mockResolvedValue(invalidOAuthInfo);
+            const saveAuthInfoSpy = jest.spyOn(credentialManager as any, 'saveAuthInfo');
+            saveAuthInfoSpy.mockResolvedValue(true);
+
+            mockRefresher.getNewTokens.mockResolvedValue({
+                tokens: undefined,
+                shouldInvalidate: true,
+                shouldSlowDown: false,
             });
 
-            const result = await credentialManager.getAuthInfo(site);
+            await (credentialManager as any).refreshAccessToken(site);
 
-            expect(Logger.debug).toHaveBeenCalledWith(expect.stringContaining('credentials are invalid'));
-            expect(result?.state).toBe(AuthInfoState.Invalid);
+            mockRefresher.getNewTokens.mockClear();
+            const result = await (credentialManager as any).refreshAccessToken(site);
+
+            expect(result).toBeUndefined();
+            expect(mockRefresher.getNewTokens).not.toHaveBeenCalled();
         });
     });
 });

src/atlclients/authStore.ts

@@ -394,14 +394,13 @@ export class CredentialManager implements Disposable {
             return authInfo; // not an OAuth info, no need to refresh
         }
 
-        if (credentials.state === AuthInfoState.Invalid) {
-            Logger.debug(`Skipping token refresh for ${site.baseApiUrl}; credentials are invalid.`);
-            return credentials;
-        }
-
         const GRACE_PERIOD = 30 * Time.MINUTES;
 
-        if (credentials.expirationDate) {
+        if (credentials.state === AuthInfoState.Invalid) {
+            // Credentials may have been invalidated spuriously (e.g. a transient 401 from the token
+            // endpoint), so attempt a refresh anyway; a successful one marks them Valid again.
+            Logger.debug(`Credentials for ${site.baseApiUrl} are marked invalid; attempting to recover.`);
+        } else if (credentials.expirationDate) {
             const diff = credentials.expirationDate - Date.now();
             Logger.debug(
                 `${Math.floor(diff / 1000)} seconds remaining for ${site.name} refresh token. ${diff > GRACE_PERIOD ? 'No refresh needed yet.' : 'refreshing...'}`,
@@ -567,13 +566,9 @@ export class CredentialManager implements Disposable {
         }
 
         if (credentials.state === AuthInfoState.Invalid) {
-            Logger.debug(`Skipping token refresh for credentialID: ${site.credentialId}; credentials are invalid.`);
-            this._failedRefreshCache.set(site.credentialId, {
-                attemptsCount: this._failedRefreshCache.get(site.credentialId)?.attemptsCount ?? 0,
-                lastAttemptAt: new Date(),
-                permanentFailure: true,
-            });
-            return undefined;
+            Logger.debug(
+                `Credentials for credentialID: ${site.credentialId} are marked invalid; attempting to recover.`,
+            );
         }
 
         const failedRefresh = this._failedRefreshCache.get(site.credentialId);
@@ -618,6 +613,8 @@ export class CredentialManager implements Disposable {
                     credentials.refresh = newTokens.refreshToken;
                     credentials.iat = newTokens.iat ?? 0;
                 }
+                // A successful refresh proves the credentials work, even if they were marked invalid before
+                credentials.state = AuthInfoState.Valid;
 
                 await this.saveAuthInfo(site, credentials);
                 if (this._failedRefreshCache.has(site.credentialId)) {