fix(auth): filesystem error during SSO token refresh expires session #5549

nkomonen-amazon · web-flow · commit 55ac0ba22117 · 2024-09-05T13:55:50.000-07:00
## Problem:
When we get SSO cache errors, such as a failed write, we do not want to
invalidate the connection. Instead we want to gracefully handle this
similar
to network errors, since the connection can still be recovered on a
subsequent
token refresh.

A place that this happens is during SSO token refresh. Where if there is
a filesystem
error when writing the new token to disk, the connection will become
invalid.

## Solution:
Catch sso cache errors and wrap them in a high level `DiskCacheError` so
that we can easily determine an sso cache error during token refresh.
Then we can
know not to invalidate the connection when there is a cache error since
`isRecoverableError()` will
catch the error.

Additionally a message is shown to the user explaining how the sso token
refresh failed and guides
them to the logs in hopes that they can fix it themselves, or at least
give it to us when the report the error.
diff --git a/packages/amazonq/package.json b/packages/amazonq/package.json
@@ -110,6 +110,10 @@
                         "amazonQSessionConfigurationMessage": {
                             "type": "boolean",
                             "default": false
+                        },
+                        "ssoCacheError": {
+                            "type": "boolean",
+                            "default": false
                         }
                     },
                     "additionalProperties": false
diff --git a/packages/core/package.json b/packages/core/package.json
@@ -192,6 +192,10 @@
                         "codeCatalystConnectionExpired": {
                             "type": "boolean",
                             "default": false
+                        },
+                        "ssoCacheError": {
+                            "type": "boolean",
+                            "default": false
                         }
                     },
                     "additionalProperties": false
diff --git a/packages/core/src/auth/auth.ts b/packages/core/src/auth/auth.ts
@@ -13,7 +13,7 @@ import * as localizedText from '../shared/localizedText'
 import { Credentials } from '@aws-sdk/types'
 import { SsoAccessTokenProvider } from './sso/ssoAccessTokenProvider'
 import { Timeout } from '../shared/utilities/timeoutUtils'
-import { errorCode, isAwsError, isNetworkError, ToolkitError, UnknownError } from '../shared/errors'
+import { DiskCacheError, errorCode, isAwsError, isNetworkError, ToolkitError, UnknownError } from '../shared/errors'
 import { getCache } from './sso/cache'
 import { isNonNullable, Mutable } from '../shared/utilities/tsUtils'
 import { builderIdStartUrl, SsoToken, truncateStartUrl } from './sso/model'
@@ -652,7 +652,7 @@ export class Auth implements AuthService, ConnectionManager {
     }
 
     private async handleSsoTokenError(id: Connection['id'], err: unknown) {
-        this.throwOnNetworkError(err)
+        this.throwOnRecoverableError(err)
 
         this.#validationErrors.set(id, UnknownError.cast(err))
         getLogger().info(`auth: Handling validation error of connection: ${id}`)
@@ -830,7 +830,7 @@ export class Auth implements AuthService, ConnectionManager {
     @withTelemetryContext({ name: '_getToken', class: authClassName })
     private async _getToken(id: Connection['id'], provider: SsoAccessTokenProvider): Promise<SsoToken> {
         const token = await provider.getToken().catch((err) => {
-            this.throwOnNetworkError(err)
+            this.throwOnRecoverableError(err)
 
             this.#validationErrors.set(id, err)
         })
@@ -842,16 +842,25 @@ export class Auth implements AuthService, ConnectionManager {
      * Auth processes can fail due to reasons outside of authentication actually being expired.
      * We do not want to intepret these failures as invalid/expired auth tokens.
      *
-     * We use this to check if the given error is unanticipated. If it is we throw,
-     * expecting the caller to not change the state of the connection.
+     * We use this to check if the given error is recoverable, meaning retrying getToken at
+     * a later time could succeed. If it is recoverable, we throw, expecting the caller to not
+     * change the state of the connection.
      */
-    private throwOnNetworkError(e: unknown) {
+    private throwOnRecoverableError(e: unknown) {
         if (isNetworkError(e)) {
             throw new ToolkitError('Failed to update connection due to networking issues', {
                 cause: e,
                 code: e.code,
             })
         }
+
+        const possibleCacheError = DiskCacheError.instanceIf(e)
+        if (possibleCacheError instanceof DiskCacheError) {
+            throw new ToolkitError('Failed to update connection due to file system operation failures', {
+                cause: possibleCacheError,
+                code: possibleCacheError.code,
+            })
+        }
     }
 
     private readonly getCredentials = keyedDebounce(this._getCredentials.bind(this))
diff --git a/packages/core/src/auth/sso/ssoAccessTokenProvider.ts b/packages/core/src/auth/sso/ssoAccessTokenProvider.ts
@@ -20,7 +20,9 @@ import { hasProps, hasStringProps, RequiredProps, selectFrom } from '../../share
 import { OidcClient } from './clients'
 import { loadOr } from '../../shared/utilities/cacheUtils'
 import {
+    DiskCacheError,
     ToolkitError,
+    getErrorMsg,
     getRequestId,
     getTelemetryReason,
     getTelemetryReasonDesc,
@@ -33,16 +35,18 @@ import { AwsLoginWithBrowser, AwsRefreshCredentials, telemetry } from '../../sha
 import { indent, toBase64URL } from '../../shared/utilities/textUtilities'
 import { AuthSSOServer } from './server'
 import { CancellationError, sleep } from '../../shared/utilities/timeoutUtils'
-import { getIdeProperties, isCloud9 } from '../../shared/extensionUtilities'
+import { getIdeProperties, isAmazonQ, isCloud9 } from '../../shared/extensionUtilities'
 import { randomBytes, createHash } from 'crypto'
 import { localize } from '../../shared/utilities/vsCodeUtils'
 import { randomUUID } from '../../shared/crypto'
 import { isRemoteWorkspace, isWebWorkspace } from '../../shared/vscode/env'
 import { showInputBox } from '../../shared/ui/inputPrompter'
-import { DevSettings } from '../../shared/settings'
+import { AmazonQPromptSettings, DevSettings, PromptSettings, ToolkitPromptSettings } from '../../shared/settings'
 import { onceChanged } from '../../shared/utilities/functionUtils'
 import { NestedMap } from '../../shared/utilities/map'
 import { asStringifiedStack } from '../../shared/telemetry/spans'
+import { showViewLogsMessage } from '../../shared/utilities/messages'
+import _ from 'lodash'
 
 export const authenticationPath = 'sso/authenticated'
 
@@ -188,6 +192,25 @@ export abstract class SsoAccessTokenProvider {
 
             return refreshed
         } catch (err) {
+            const possibleCacheError = DiskCacheError.instanceIf(err)
+            if (possibleCacheError instanceof DiskCacheError) {
+                /**
+                 * Background:
+                 * - During token refresh when saving to our filesystem/cache there are sometimes file system
+                 *   related errors.
+                 * - When these errors ocurr it will cause the token refresh process to fail, and the users SSO
+                 *   connection to become invalid.
+                 * - Because these filesystem errors do not indicate the SSO session is actually stale,
+                 *   we want to catch these filesystem errors and not invalidate the users SSO connection since a
+                 *   subsequent attempt to refresh may succeed.
+                 * - To give the user a chance to resolve their filesystem related issue, we want to point them
+                 *   to the logs where the error was logged. Hopefully they can use this information to fix the issue,
+                 *   or at least hint for them to provide the logs in a bug report.
+                 */
+                void DiskCacheErrorMessage.instance.showMessageThrottled(possibleCacheError)
+                throw possibleCacheError
+            }
+
             if (!isNetworkError(err)) {
                 const reason = getTelemetryReason(err)
                 telemetry.aws_refreshCredentials.emit({
@@ -761,3 +784,53 @@ type ReAuthStateValue = {
     // the latest reason for why the connection was moved in to a "needs reauth" state
     reAuthReason?: string
 }
+
+/**
+ * Singleton class that manages showing the user a message during
+ * failures that ocurr during SSO disk cache operations.
+ *
+ * Background:
+ * - We need this {@link DiskCacheErrorMessage} specifically as a singleton since we want to ensure
+ *   that only 1 instance of this message appears at a time. There are cases where it shows multiple messages
+ *   if not used as a singleton.
+ */
+class DiskCacheErrorMessage {
+    static #instance: DiskCacheErrorMessage
+    static get instance() {
+        return (this.#instance ??= new DiskCacheErrorMessage())
+    }
+
+    /**
+     * Show a `"don't show again"`-able message which tells the user about a file system related error
+     * with the sso cache.
+     */
+    public showMessageThrottled(error: Error) {
+        return this._showMessageThrottled(error)
+    }
+    private _showMessageThrottled = _.throttle(async (error: Error) => this._showMessage(error), 60_000, {
+        leading: true,
+    })
+    private async _showMessage(error: Error) {
+        const dontShow = 'Never warn again'
+
+        const promptSettings: PromptSettings = isAmazonQ()
+            ? AmazonQPromptSettings.instance
+            : ToolkitPromptSettings.instance
+
+        // We know 'ssoCacheError' is in all extension prompt settings
+        if (await promptSettings.isPromptEnabled('ssoCacheError')) {
+            const result = await showMessage()
+            if (result === dontShow) {
+                await promptSettings.disablePrompt('ssoCacheError')
+            }
+        }
+
+        function showMessage() {
+            return showViewLogsMessage(
+                `File system related error during SSO cache operation:\n"${getErrorMsg(error, true)}"`,
+                'error',
+                [dontShow]
+            )
+        }
+    }
+}
diff --git a/packages/core/src/shared/errors.ts b/packages/core/src/shared/errors.ts
@@ -880,7 +880,7 @@ function isEbusyError(err: Error) {
 }
 
 /** Helper function to assert given error has the expected properties */
-function isError(err: Error, id: string, messageIncludes: string = '') {
+export function isError(err: Error, id: string, messageIncludes: string = '') {
     // It is not always clear if the error has the expected value in the `name` or `code` field
     // so this checks both.
     return (err.name === id || (err as any).code === id) && err.message.includes(messageIncludes)
@@ -984,6 +984,65 @@ export class AwsClientResponseError extends Error {
     }
 }
 
+/**
+ * Represents a generalized error that happened during a disk cache operation.
+ *
+ * For example, when SSO refreshes a token a disk cache error can occur when it
+ * attempts to read/write the disk cache. These errors can be recoverable and do not
+ * imply that the SSO session is stale. So by wrapping those errors in an instance of
+ * this class it will help to distinguish them.
+ */
+export class DiskCacheError extends Error {
+    #code: string | undefined
+
+    /** Use {@link DiskCacheError.instanceIf()} to create an instance */
+    protected constructor(message: string, options?: { code?: string }) {
+        super(message)
+        this.#code = options?.code
+    }
+
+    /**
+     * We are seeing these errors in telemetry, but they have no error message attached.
+     * The best we can do is assume these are filesystem errors in the context that we see them.
+     */
+    private static fileSystemErrorsWithoutMessage = ['EACCES', 'EBADF']
+
+    /**
+     * Return an instance of {@link DiskCacheError} that consists of the properties from the
+     * given error IF certain conditions are met. Otherwise, the original error is returned.
+     *
+     * - Also returns the original error if it is already a {@link DiskCacheError}.
+     */
+    public static instanceIf<T>(err: T): DiskCacheError | T {
+        if (!(err instanceof Error) || err instanceof DiskCacheError) {
+            return err
+        }
+
+        const errorId = (err as any).code ?? err.name
+
+        if (DiskCacheError.fileSystemErrorsWithoutMessage.includes(errorId)) {
+            // The error message will probably be blank
+            const message = err.message ? err.message : 'No msg'
+            return new DiskCacheError(message, { code: errorId })
+        }
+
+        // These are errors we were seeing in telemetry
+        if (
+            isError(err, 'ENOSPC', 'no space left on device') ||
+            isError(err, 'EPERM', 'operation not permitted') ||
+            isError(err, 'EBUSY', 'resource busy or locked')
+        ) {
+            return new DiskCacheError(err.message, { code: errorId })
+        }
+
+        return err // the error does no meet the conditions to be a DiskCacheError
+    }
+
+    public get code() {
+        return this.#code
+    }
+}
+
 /**
  * Run a function and swallow any errors that are not specified by `shouldThrow`
  */
diff --git a/packages/core/src/shared/settings-amazonq.gen.ts b/packages/core/src/shared/settings-amazonq.gen.ts
@@ -16,7 +16,8 @@ export const amazonqSettings = {
         "codeWhispererNewWelcomeMessage": {},
         "codeWhispererConnectionExpired": {},
         "amazonQWelcomePage": {},
-        "amazonQSessionConfigurationMessage": {}
+        "amazonQSessionConfigurationMessage": {},
+        "ssoCacheError": {}
     },
     "amazonQ.showInlineCodeSuggestionsWithCodeReferences": {},
     "amazonQ.importRecommendationForInlineCodeSuggestions": {},
diff --git a/packages/core/src/shared/settings-toolkit.gen.ts b/packages/core/src/shared/settings-toolkit.gen.ts
@@ -36,7 +36,8 @@ export const toolkitSettings = {
         "createCredentialsProfile": {},
         "samcliConfirmDevStack": {},
         "remoteConnected": {},
-        "codeCatalystConnectionExpired": {}
+        "codeCatalystConnectionExpired": {},
+        "ssoCacheError": {}
     },
     "aws.experiments": {
         "jsonResourceModification": {}
diff --git a/packages/core/src/shared/settings.ts b/packages/core/src/shared/settings.ts
@@ -620,10 +620,13 @@ export function fromExtensionManifest<T extends TypeDescriptor & Partial<Section
  */
 export const toolkitPrompts = settingsProps['aws.suppressPrompts']
 type toolkitPromptName = keyof typeof toolkitPrompts
-export class ToolkitPromptSettings extends Settings.define(
-    'aws.suppressPrompts',
-    toRecord(keys(toolkitPrompts), () => Boolean)
-) {
+export class ToolkitPromptSettings
+    extends Settings.define(
+        'aws.suppressPrompts',
+        toRecord(keys(toolkitPrompts), () => Boolean)
+    )
+    implements PromptSettings
+{
     public async isPromptEnabled(promptName: toolkitPromptName): Promise<boolean> {
         try {
             return !this._getOrThrow(promptName, false)
@@ -650,10 +653,13 @@ export class ToolkitPromptSettings extends Settings.define(
 
 export const amazonQPrompts = settingsProps['amazonQ.suppressPrompts']
 type amazonQPromptName = keyof typeof amazonQPrompts
-export class AmazonQPromptSettings extends Settings.define(
-    'amazonQ.suppressPrompts',
-    toRecord(keys(amazonQPrompts), () => Boolean)
-) {
+export class AmazonQPromptSettings
+    extends Settings.define(
+        'amazonQ.suppressPrompts',
+        toRecord(keys(amazonQPrompts), () => Boolean)
+    )
+    implements PromptSettings
+{
     public async isPromptEnabled(promptName: amazonQPromptName): Promise<boolean> {
         try {
             return !this._getOrThrow(promptName, false)
@@ -678,6 +684,18 @@ export class AmazonQPromptSettings extends Settings.define(
     }
 }
 
+/**
+ * Use cautiously as this is misleading. Ideally we create a type
+ * which is the intersection of the types (only the values that occur
+ * in each are selected), but idk how to do that.
+ */
+type AllPromptNames = amazonQPromptName | toolkitPromptName
+
+export interface PromptSettings {
+    isPromptEnabled(promptName: AllPromptNames): Promise<boolean>
+    disablePrompt(promptName: AllPromptNames): Promise<void>
+}
+
 const experiments = settingsProps['aws.experiments']
 type ExperimentName = keyof typeof experiments
 
diff --git a/packages/core/src/shared/utilities/cacheUtils.ts b/packages/core/src/shared/utilities/cacheUtils.ts
diff --git a/packages/core/src/test/shared/errors.test.ts b/packages/core/src/test/shared/errors.test.ts
diff --git a/packages/toolkit/package.json b/packages/toolkit/package.json
