Skip to content

Commit

Permalink
fix(auth): malformed SSO cache didn't prompt reauth (#6164)
Browse files Browse the repository at this point in the history
## Problem:

When we loaded sso cache from disk, we would only invalidate (leading to
a reauth prompt) if the cache file was missing.

But if the cache file was present, though its content was malformed, we
would incorrectly treat it as recoverable by throwing instead of
returning undefined. Users would get stuck in a state where all future
api calls would fail, and they'd never get a prompt to reauth to fix
their SSO session.

## Solution:

If we detect a SyntaxError treat it as non-recoverable, meaning it will
trigger a reauth.

Also added some code to validate the content of the SSO cache we loaded
from disk to ensure it is what we expected.

Fixes #6140

---

- Treat all work as PUBLIC. Private `feature/x` branches will not be
squash-merged at release time.
- Your code changes must meet the guidelines in
[CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines).

License: I confirm that my contribution is made under the terms of the
Apache 2.0 license.

---------

Signed-off-by: nkomonen-amazon <[email protected]>
  • Loading branch information
nkomonen-amazon authored Dec 11, 2024
1 parent 0a40071 commit 2af8b45
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"type": "Bug Fix",
"description": "Auth: SSO session was bad, but no reauth prompt given"
}
9 changes: 8 additions & 1 deletion packages/core/src/auth/sso/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,12 @@ import { getLogger } from '../../shared/logger/logger'
import fs from '../../shared/fs/fs'
import { createDiskCache, KeyedCache, mapCache } from '../../shared/utilities/cacheUtils'
import { stripUndefined } from '../../shared/utilities/collectionUtils'
import { hasProps, selectFrom } from '../../shared/utilities/tsUtils'
import { getMissingProps, hasProps, selectFrom } from '../../shared/utilities/tsUtils'
import { SsoToken, ClientRegistration } from './model'
import { DevSettings } from '../../shared/settings'
import { onceChanged } from '../../shared/utilities/functionUtils'
import globals from '../../shared/extensionGlobals'
import { ToolkitError } from '../../shared/errors'

interface RegistrationKey {
readonly startUrl: string
Expand Down Expand Up @@ -92,6 +93,12 @@ export function getTokenCache(directory = getCacheDir()): KeyedCache<SsoAccess>

stripUndefined(token)

// Validate data is not missing.
const missingProps = getMissingProps(token, 'accessToken', 'refreshToken')
if (missingProps.length > 0) {
throw new ToolkitError(`SSO cache data unexpectedly missing props: ${JSON.stringify(missingProps)}`)
}

return {
token,
registration,
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/shared/regions/regionProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { AwsContext } from '../awsContext'
import { getIdeProperties, isAmazonQ, isCloud9 } from '../extensionUtilities'
import { ResourceFetcher } from '../resourcefetcher/resourcefetcher'
import { isSsoConnection } from '../../auth/connection'
import { Auth } from '../../auth'
import { Auth } from '../../auth/auth'

export const defaultRegion = 'us-east-1'
export const defaultPartition = 'aws'
Expand Down
13 changes: 13 additions & 0 deletions packages/core/src/shared/utilities/cacheUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,23 @@ export function createDiskCache<V, K>(
log('loaded', key)
return result
} catch (error) {
// Non-recoverable errors mean there is no usable data.
// Recoverable errors mean we can possibly use the data for something like
// an SSO token refresh, or to just retry.
// Returning undefined implies non-recoverable.

// -- Non-recoverable Errors --
if (isFileNotFoundError(error)) {
log('read failed (file not found)', key)
return
}
if (error instanceof SyntaxError) {
// file content was malformed or empty
log(`read failed (invalid JSON)`, key)
return
}

// -- Recoverable Errors --
log(`read failed ${error}`, key)
throw createDiskCacheError(error, 'LOAD', target, key)
}
Expand Down
4 changes: 3 additions & 1 deletion packages/core/src/test/credentials/sso/cache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import * as path from 'path'
import { makeTemporaryToolkitFolder, tryRemoveFolder } from '../../../shared/filesystemUtilities'
import { getRegistrationCache, getTokenCache } from '../../../auth/sso/cache'
import { fs } from '../../../shared'
import { SsoToken } from '../../../auth/sso/model'

describe('SSO Cache', function () {
const region = 'dummyRegion'
Expand All @@ -26,7 +27,8 @@ describe('SSO Cache', function () {
const validToken = {
accessToken: 'longstringofrandomcharacters',
expiresAt: new Date(Date.now() + hourInMs),
}
refreshToken: 'dummyRefreshToken',
} as SsoToken

beforeEach(async function () {
testDir = await makeTemporaryToolkitFolder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ describe('SsoAccessTokenProvider', function () {
return {
accessToken: 'dummyAccessToken',
expiresAt: new clock.Date(clock.Date.now() + timeDelta),
refreshToken: 'dummyRefreshToken',
...extras,
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"type": "Bug Fix",
"description": "Auth: SSO session was bad, but no reauth prompt given"
}

0 comments on commit 2af8b45

Please sign in to comment.