Skip to content

Commit ecd0eea

Browse files
authored
fix(auth): improve IdC connection lifespan #3501
Problem: Networking issues cause connections to appear invalidated because we can't refresh the access token. But the refresh token may still be valid. Solution: Assume that the connection is still valid if we see a networking issue. Callers will receive an exception when attempting to fetch a token without an internet connection.
1 parent cde05a5 commit ecd0eea

File tree

5 files changed

+55
-17
lines changed

5 files changed

+55
-17
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"type": "Bug Fix",
3+
"description": "auth: IAM Identity Center connections expire sooner than expected"
4+
}

src/credentials/auth.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import { Commands } from '../shared/vscode/commands2'
1818
import { createQuickPick, DataQuickPickItem, showQuickPick } from '../shared/ui/pickerPrompter'
1919
import { isValidResponse } from '../shared/wizards/wizard'
2020
import { CancellationError, Timeout } from '../shared/utilities/timeoutUtils'
21-
import { errorCode, formatError, isAwsError, ToolkitError, UnknownError } from '../shared/errors'
21+
import { errorCode, formatError, isAwsError, isNetworkError, ToolkitError, UnknownError } from '../shared/errors'
2222
import { getCache } from './sso/cache'
2323
import { createFactoryFunction, isNonNullable, Mutable } from '../shared/utilities/tsUtils'
2424
import { builderIdStartUrl, SsoToken } from './sso/model'
@@ -877,6 +877,13 @@ export class Auth implements AuthService, ConnectionManager {
877877
private readonly getToken = keyedDebounce(this._getToken.bind(this))
878878
private async _getToken(id: Connection['id'], provider: SsoAccessTokenProvider): Promise<SsoToken> {
879879
const token = await provider.getToken().catch(err => {
880+
// Bubble-up networking issues so we don't treat the session as invalid
881+
if (isNetworkError(err)) {
882+
throw new ToolkitError('Failed to refresh connection due to networking issues', {
883+
cause: err,
884+
})
885+
}
886+
880887
this.#validationErrors.set(id, err)
881888
})
882889

src/credentials/sso/ssoAccessTokenProvider.ts

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,14 @@ import { hasProps, hasStringProps, RequiredProps, selectFrom } from '../../share
1515
import { CancellationError, sleep } from '../../shared/utilities/timeoutUtils'
1616
import { OidcClient } from './clients'
1717
import { loadOr } from '../../shared/utilities/cacheUtils'
18-
import { getRequestId, getTelemetryReason, getTelemetryResult, isClientFault, ToolkitError } from '../../shared/errors'
18+
import {
19+
getRequestId,
20+
getTelemetryReason,
21+
getTelemetryResult,
22+
isClientFault,
23+
isNetworkError,
24+
ToolkitError,
25+
} from '../../shared/errors'
1926
import { getLogger } from '../../shared/logger'
2027
import { AwsRefreshCredentials, telemetry } from '../../shared/telemetry/telemetry'
2128
import { getIdeProperties, isCloud9 } from '../../shared/extensionUtilities'
@@ -123,17 +130,19 @@ export class SsoAccessTokenProvider {
123130

124131
return refreshed
125132
} catch (err) {
126-
telemetry.aws_refreshCredentials.emit({
127-
result: getTelemetryResult(err),
128-
reason: getTelemetryReason(err),
129-
requestId: getRequestId(err),
130-
sessionDuration: getSessionDuration(this.tokenCacheKey),
131-
credentialType: 'bearerToken',
132-
credentialSourceId: this.profile.startUrl === builderIdStartUrl ? 'awsId' : 'iamIdentityCenter',
133-
} as AwsRefreshCredentials)
134-
135-
if (err instanceof SSOOIDCServiceException && isClientFault(err)) {
136-
await this.cache.token.clear(this.tokenCacheKey)
133+
if (!isNetworkError(err)) {
134+
telemetry.aws_refreshCredentials.emit({
135+
result: getTelemetryResult(err),
136+
reason: getTelemetryReason(err),
137+
requestId: getRequestId(err),
138+
sessionDuration: getSessionDuration(this.tokenCacheKey),
139+
credentialType: 'bearerToken',
140+
credentialSourceId: this.profile.startUrl === builderIdStartUrl ? 'awsId' : 'iamIdentityCenter',
141+
} as AwsRefreshCredentials)
142+
143+
if (err instanceof SSOOIDCServiceException && isClientFault(err)) {
144+
await this.cache.token.clear(this.tokenCacheKey)
145+
}
137146
}
138147

139148
throw err

src/shared/errors.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { ServiceException } from '@aws-sdk/smithy-client'
99
import { isThrottlingError, isTransientError } from '@aws-sdk/service-error-classification'
1010
import { Result } from './telemetry/telemetry'
1111
import { CancellationError } from './utilities/timeoutUtils'
12-
import { hasKey, isNonNullable } from './utilities/tsUtils'
12+
import { isNonNullable } from './utilities/tsUtils'
1313
import type * as fs from 'fs'
1414
import type * as os from 'os'
1515

@@ -371,7 +371,7 @@ export function isAwsError(error: unknown): error is AWSError {
371371
return error instanceof Error && hasCode(error) && hasTime(error)
372372
}
373373

374-
function hasCode(error: Error): error is typeof error & { code: string } {
374+
function hasCode<T>(error: T): error is T & { code: string } {
375375
return typeof (error as { code?: unknown }).code === 'string'
376376
}
377377

@@ -403,7 +403,7 @@ export function getRequestId(error: unknown): string | undefined {
403403
export function isFileNotFoundError(err: unknown): boolean {
404404
if (err instanceof vscode.FileSystemError) {
405405
return err.code === vscode.FileSystemError.FileNotFound().code
406-
} else if (isNonNullable(err) && typeof err === 'object' && hasKey(err, 'code')) {
406+
} else if (hasCode(err)) {
407407
return err.code === 'ENOENT'
408408
}
409409

@@ -417,7 +417,7 @@ export function isNoPermissionsError(err: unknown): boolean {
417417
// The code _should_ be `NoPermissions`, maybe this is a bug?
418418
(err.code === 'Unknown' && err.message.includes('EACCES: permission denied'))
419419
)
420-
} else if (isNonNullable(err) && typeof err === 'object' && hasKey(err, 'code')) {
420+
} else if (hasCode(err)) {
421421
return err.code === 'EACCES'
422422
}
423423

@@ -494,3 +494,11 @@ export class PermissionsError extends ToolkitError {
494494
this.actual = actual
495495
}
496496
}
497+
498+
export function isNetworkError(err?: unknown) {
499+
if (!hasCode(err)) {
500+
return false
501+
}
502+
503+
return ['ENOTFOUND', 'EAI_AGAIN', 'ECONNRESET', 'ECONNREFUSED', 'ETIMEDOUT'].includes(err.code)
504+
}

src/test/credentials/auth.test.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,16 @@ describe('Auth', function () {
236236
assert.ok(err2 instanceof ToolkitError)
237237
assert.strictEqual(err2.cause, err1)
238238
})
239+
240+
it('bubbles up networking issues instead of invalidating the connection', async function () {
241+
const expected = new ToolkitError('test', { code: 'ETIMEDOUT' })
242+
const conn = await auth.createConnection(ssoProfile)
243+
auth.getTestTokenProvider(conn)?.getToken.rejects(expected)
244+
const actual = await conn.getToken().catch(e => e)
245+
assert.ok(actual instanceof ToolkitError)
246+
assert.strictEqual(actual.cause, expected)
247+
assert.strictEqual(auth.getConnectionState(conn), 'valid')
248+
})
239249
})
240250

241251
describe('Linked Connections', function () {

0 commit comments

Comments
 (0)