fix(oauth): clear only the recorded revocation on in-place relinks, with no Redis on the sign-in path

Author: waleedlatif1

apps/sim/lib/auth/auth.ts

@@ -148,7 +148,7 @@ import { validateSignupEmailMx } from '@/lib/messaging/email/validation.server'
 import { isEmailVerificationEffectivelyEnabled } from '@/lib/messaging/email/verification'
 import { scheduleLifecycleEmail } from '@/lib/messaging/lifecycle'
 import { APP_ENTRY_PATH } from '@/lib/navigation/paths'
-import { clearOAuthRefreshFailure } from '@/lib/oauth/credential-service'
+import { clearRecordedRevocation } from '@/lib/oauth/credential-service'
 import {
   getMicrosoftRefreshTokenExpiry,
   isMicrosoftProvider,
@@ -711,9 +711,9 @@ export const auth = betterAuth({
         after: async (account, context) => {
           const path = context?.path
           if (!path?.startsWith('/oauth2/callback/') && !path?.startsWith('/callback/')) return
-          // Fails the callback like the draft hooks do, so a reconnect never reports success
-          // while the old revocation still blocks the credential.
-          await clearOAuthRefreshFailure(account.id)
+          // Fails the callback only if this statement fails, so a reconnect never reports
+          // success while the old revocation still blocks the credential.
+          await clearRecordedRevocation(account.id)
         },
       },
     },

apps/sim/lib/oauth/credential-revocation.integration.ts

@@ -26,6 +26,7 @@ import {
   OAUTH_CREDENTIAL_REVOKED,
 } from '@/lib/oauth/credential-revoked'
 import {
+  clearRecordedRevocation,
   getCredentialTerminalRefreshError,
   refreshTokenIfNeeded,
 } from '@/lib/oauth/credential-service'
@@ -307,7 +308,20 @@ describe('OAuth refresh-token revocation against PostgreSQL and Redis', () => {
     }
   })
 
-  it('resumes refreshing after a reconnect that kept the old refresh token', async () => {
+  it.each([
+    {
+      path: 'a credential draft reconnect',
+      reconnect: () =>
+        handleReconnectCredential({
+          draft: { credentialId },
+          newAccountId: accountId,
+          workspaceId,
+          userId,
+          now: new Date(),
+        }),
+    },
+    { path: 'an in-place relink', reconnect: () => clearRecordedRevocation(accountId) },
+  ])('resumes refreshing after $path that kept the old refresh token', async ({ reconnect }) => {
     await expect(resolveToken()).rejects.toBeInstanceOf(CredentialRevokedError)
 
     // A relink with no new refresh token from the provider rewrites only the access token.
@@ -319,13 +333,7 @@ describe('OAuth refresh-token revocation against PostgreSQL and Redis', () => {
         updatedAt: new Date(),
       })
       .where(eq(account.id, accountId))
-    await handleReconnectCredential({
-      draft: { credentialId },
-      newAccountId: accountId,
-      workspaceId,
-      userId,
-      now: new Date(),
-    })
+    await reconnect()
     await db
       .update(account)
       .set({ accessTokenExpiresAt: new Date(Date.now() - 60_000) })

apps/sim/lib/oauth/credential-service.ts

@@ -1075,11 +1075,23 @@ export async function getCredentialTerminalRefreshError(
   return errorCode ? { errorCode, providerId: row.providerId } : null
 }
 
+/**
+ * Clears the revocation recorded on an account's own row after its owner reauthorized it. A
+ * reauthorization can keep the old refresh token when the provider issues none, which the
+ * revocation's token fingerprint cannot tell from no reauthorization at all. One conditional
+ * statement with no Redis, so the sign-in path gains no dependency and most logins write nothing.
+ */
+export async function clearRecordedRevocation(accountId: string): Promise<void> {
+  await db
+    .update(account)
+    .set(CLEARED_REFRESH_REVOCATION)
+    .where(and(eq(account.id, accountId), isNotNull(account.refreshRevokedTokenHash)))
+}
+
 /**
  * Clears every refresh failure recorded against an account after its owner reauthorized it: the
- * revocation on its chain's rows and the Redis terminal-error flag. A reauthorization can keep
- * the old refresh token when the provider issues none, which the revocation's token fingerprint
- * cannot tell from no reauthorization at all.
+ * revocation on its chain's rows (every row of a Slack installation) and the Redis terminal-error
+ * flag.
  */
 export async function clearOAuthRefreshFailure(accountId: string): Promise<void> {
   const [row] = await db
@@ -1096,12 +1108,12 @@ export async function clearOAuthRefreshFailure(accountId: string): Promise<void>
     ? extractSlackTeamId(row.providerAccountId)
     : null
   await Promise.all([
-    db
-      .update(account)
-      .set(CLEARED_REFRESH_REVOCATION)
-      .where(
-        and(chainRowsFilter(accountId, slackTeamId), isNotNull(account.refreshRevokedTokenHash))
-      ),
+    slackTeamId
+      ? db
+          .update(account)
+          .set(CLEARED_REFRESH_REVOCATION)
+          .where(and(installationFilter(slackTeamId), isNotNull(account.refreshRevokedTokenHash)))
+      : clearRecordedRevocation(accountId),
     clearOAuthRefreshDeadFlag(
       refreshCoordinationScope(accountId, row.providerId, row.providerAccountId, row.idToken)
     ),