Skip to content

Commit c7913f2

Browse files
fix(auth): invalid sso tokens being loaded
Problem: At a certain point in time sso tokens were became invalid. We stored those tokens in cache and eventually they will throw errors to the user when the are attempted to be refreshed. Solution: When the cache is being setup delete any invalid files. Invalid files are files that were created before a certain date. Signed-off-by: Nikolas Komonen <[email protected]>
1 parent 188a185 commit c7913f2

File tree

3 files changed

+152
-5
lines changed

3 files changed

+152
-5
lines changed

src/credentials/sso/cache.ts

Lines changed: 60 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { hasProps, selectFrom } from '../../shared/utilities/tsUtils'
1212
import { SsoToken, ClientRegistration } from './model'
1313
import { SystemUtilities } from '../../shared/systemUtilities'
1414
import { DevSettings } from '../../shared/settings'
15+
import { statSync, Stats, readdirSync, unlinkSync } from 'fs'
1516

1617
interface RegistrationKey {
1718
readonly region: string
@@ -33,13 +34,46 @@ export interface SsoCache {
3334
const defaultCacheDir = path.join(SystemUtilities.getHomeDirectory(), '.aws', 'sso', 'cache')
3435
export const getCacheDir = () => DevSettings.instance.get('ssoCacheDirectory', defaultCacheDir)
3536

36-
export function getCache(directory = getCacheDir()): SsoCache {
37+
export function getCache(directory = getCacheDir(), statFunc = getFileStats): SsoCache {
38+
try {
39+
deleteOldFiles(directory, statFunc)
40+
} catch (e) {
41+
getLogger().warn('auth: error deleting old files in sso cache: %s', e)
42+
}
43+
3744
return {
3845
token: getTokenCache(directory),
3946
registration: getRegistrationCache(directory),
4047
}
4148
}
4249

50+
function deleteOldFiles(directory: string, statFunc: typeof getFileStats) {
51+
if (!isDirSafeToDeleteFrom(directory)) {
52+
getLogger().warn(`Skipped deleting files in directory: ${path.resolve(directory)}`)
53+
return
54+
}
55+
56+
const fileNames = readdirSync(directory)
57+
fileNames.forEach(fileName => {
58+
const filePath = path.join(directory, fileName)
59+
if (path.extname(filePath) === '.json' && isOldFile(filePath, statFunc)) {
60+
unlinkSync(filePath)
61+
getLogger().warn(`auth: removed old cache file: ${filePath}`)
62+
}
63+
})
64+
}
65+
66+
export function isDirSafeToDeleteFrom(dirPath: string): boolean {
67+
const resolvedPath = path.resolve(dirPath)
68+
const isRoot = resolvedPath === path.resolve('/')
69+
const isCwd = resolvedPath === path.resolve('.')
70+
const isAbsolute = path.isAbsolute(dirPath)
71+
const pathDepth = resolvedPath.split(path.sep).length
72+
73+
const isSafe = !isRoot && !isCwd && isAbsolute && pathDepth >= 5
74+
return isSafe
75+
}
76+
4377
export function getRegistrationCache(directory = getCacheDir()): KeyedCache<ClientRegistration, RegistrationKey> {
4478
// Compatability for older Toolkit versions (format on disk is unchanged)
4579
type StoredRegistration = Omit<ClientRegistration, 'expiresAt'> & { readonly expiresAt: string }
@@ -110,7 +144,30 @@ export function getTokenCache(directory = getCacheDir()): KeyedCache<SsoAccess>
110144
return mapCache(cache, read, write)
111145
}
112146

113-
function getTokenCacheFile(ssoCacheDir: string, ssoUrl: string) {
147+
function getFileStats(file: string): Stats {
148+
return statSync(file)
149+
}
150+
151+
const firstValidDate = new Date(2023, 3, 14) // April 14, 2023
152+
153+
/**
154+
* @returns true if file is older than the first valid date
155+
*/
156+
function isOldFile(file: string, statFunc: typeof getFileStats): boolean {
157+
try {
158+
const statResult = statFunc(file)
159+
// Depending on the Windows filesystem, birthtime may be 0, so we fall back to ctime (last time metadata was changed)
160+
// https://nodejs.org/api/fs.html#stat-time-values
161+
return statResult.birthtimeMs !== 0
162+
? statResult.birthtimeMs < firstValidDate.getTime()
163+
: statResult.ctime < firstValidDate
164+
} catch (err) {
165+
getLogger().debug(`SSO cache file age not be verified: ${file}: ${err}`)
166+
return false // Assume it is no old since we cannot validate
167+
}
168+
}
169+
170+
export function getTokenCacheFile(ssoCacheDir: string, ssoUrl: string): string {
114171
const encoded = encodeURI(ssoUrl)
115172
// Per the spec: 'SSO Login Token Flow' the access token must be
116173
// cached as the SHA1 hash of the bytes of the UTF-8 encoded
@@ -126,7 +183,7 @@ function getTokenCacheFile(ssoCacheDir: string, ssoUrl: string) {
126183
return path.join(ssoCacheDir, `${hashedUrl}.json`)
127184
}
128185

129-
const getRegistrationCacheFile = (ssoCacheDir: string, key: RegistrationKey) => {
186+
export function getRegistrationCacheFile(ssoCacheDir: string, key: RegistrationKey): string {
130187
const hashScopes = (scopes: string[]) => {
131188
const shasum = crypto.createHash('sha256')
132189
scopes.forEach(s => shasum.update(s))

src/test/credentials/sso/ssoAccessTokenProvider.test.ts

Lines changed: 84 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,12 @@ import * as FakeTimers from '@sinonjs/fake-timers'
88
import * as sinon from 'sinon'
99
import { SsoAccessTokenProvider } from '../../../credentials/sso/ssoAccessTokenProvider'
1010
import { installFakeClock } from '../../testUtil'
11-
import { getCache } from '../../../credentials/sso/cache'
11+
import {
12+
getCache,
13+
getTokenCacheFile,
14+
isDirSafeToDeleteFrom,
15+
getRegistrationCacheFile,
16+
} from '../../../credentials/sso/cache'
1217

1318
import { instance, mock, when, anything, reset } from '../../utilities/mockito'
1419
import { makeTemporaryToolkitFolder, tryRemoveFolder } from '../../../shared/filesystemUtilities'
@@ -25,6 +30,8 @@ import { getOpenExternalStub } from '../../globalSetup.test'
2530
import { getTestWindow } from '../../shared/vscode/window'
2631
import { SeverityLevel } from '../../shared/vscode/message'
2732
import { ToolkitError } from '../../../shared/errors'
33+
import * as fs from 'fs'
34+
import * as path from 'path'
2835

2936
const hourInMs = 3600000
3037

@@ -66,6 +73,13 @@ describe('SsoAccessTokenProvider', function () {
6673
}
6774
}
6875

76+
async function makeTemporaryTokenCacheFolder() {
77+
const root = await makeTemporaryToolkitFolder()
78+
const cacheDir = path.join(root, '.aws', 'sso', 'cache')
79+
fs.mkdirSync(cacheDir, { recursive: true })
80+
return cacheDir
81+
}
82+
6983
before(function () {
7084
clock = installFakeClock()
7185
})
@@ -75,7 +89,7 @@ describe('SsoAccessTokenProvider', function () {
7589
})
7690

7791
beforeEach(async function () {
78-
tempDir = await makeTemporaryToolkitFolder()
92+
tempDir = await makeTemporaryTokenCacheFolder()
7993
cache = getCache(tempDir)
8094
sut = new SsoAccessTokenProvider({ region, startUrl }, cache, instance(oidcClient))
8195
})
@@ -115,6 +129,74 @@ describe('SsoAccessTokenProvider', function () {
115129
assert.strictEqual(await cache.token.load(startUrl), undefined)
116130
})
117131

132+
describe('does not return old tokens', function () {
133+
// A file is old when the creation time is under a certain date
134+
const oldTime: Date = new Date(2023, 3, 10) // April 10, 2023
135+
const nonOldTime: Date = new Date(2023, 3, 15) // April 15, 2023
136+
137+
function oldBirthtime(file: fs.PathLike): fs.Stats {
138+
return { birthtimeMs: oldTime.getTime(), birthtime: oldTime } as fs.Stats
139+
}
140+
141+
/** Windows edge case where birthtime does not exist, instead check ctime */
142+
function noBirthtimeOldCTime(file: fs.PathLike): fs.Stats {
143+
return { birthtimeMs: 0, ctime: oldTime } as fs.Stats
144+
}
145+
146+
const oldStatsFuncs = [oldBirthtime, noBirthtimeOldCTime]
147+
148+
oldStatsFuncs.forEach(invalidStatsFunc => {
149+
it(`deletes old invalid tokens when ${invalidStatsFunc.name} then returns undefined`, async function () {
150+
await cache.token.save(startUrl, { region, startUrl, token: createToken(hourInMs) })
151+
const tokenCacheFile = getTokenCacheFile(tempDir, startUrl)
152+
assert.strictEqual(fs.existsSync(tokenCacheFile), true)
153+
154+
// Set the func which returns Stats that are always 'invalid'
155+
cache = getCache(tempDir, invalidStatsFunc)
156+
157+
assert.strictEqual(await cache.token.load(startUrl), undefined)
158+
assert.strictEqual(fs.existsSync(tokenCacheFile), false)
159+
})
160+
161+
it(`deletes old invalid registrations when ${invalidStatsFunc.name} then returns undefined`, async function () {
162+
const registrationKey = { region }
163+
await cache.registration.save(registrationKey, createRegistration(hourInMs))
164+
const registrationCacheFile = getRegistrationCacheFile(tempDir, registrationKey)
165+
assert.strictEqual(fs.existsSync(registrationCacheFile), true)
166+
167+
// Set the func which returns Stats that are always 'invalid'
168+
cache = getCache(tempDir, invalidStatsFunc)
169+
assert.strictEqual(await cache.token.load(startUrl), undefined)
170+
assert.strictEqual(fs.existsSync(registrationCacheFile), false)
171+
})
172+
})
173+
174+
function nonOldBirthtime(file: fs.PathLike): fs.Stats {
175+
return { birthtimeMs: nonOldTime.getTime() } as fs.Stats
176+
}
177+
178+
it(`returns token from non-old file`, async function () {
179+
const token = createToken(hourInMs)
180+
await cache.token.save(startUrl, { region, startUrl, token })
181+
const tokenCacheFile = getTokenCacheFile(tempDir, startUrl)
182+
assert.strictEqual(fs.existsSync(tokenCacheFile), true)
183+
184+
cache = getCache(tempDir, nonOldBirthtime)
185+
186+
assert.deepStrictEqual((await cache.token.load(startUrl))!.token, token)
187+
assert.strictEqual(fs.existsSync(tokenCacheFile), true)
188+
})
189+
190+
it('isDirSafeToDeleteFrom()', function () {
191+
assert.ok(!isDirSafeToDeleteFrom('.'))
192+
assert.ok(!isDirSafeToDeleteFrom('/'))
193+
assert.ok(!isDirSafeToDeleteFrom('not/an/absolute/path'))
194+
assert.ok(!isDirSafeToDeleteFrom('/a/b/c')) // Too shallow
195+
196+
assert.ok(isDirSafeToDeleteFrom('/a/b/c/d'))
197+
})
198+
})
199+
118200
it('returns `undefined` for expired tokens that cannot be refreshed', async function () {
119201
const expiredToken = createToken(-hourInMs)
120202
await cache.token.save(startUrl, { region, startUrl, token: expiredToken })

src/test/techdebt.test.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,4 +42,12 @@ describe('tech debt', function () {
4242
'with node16+, we can use crypto.randomUUID and remove the "uuid" dependency'
4343
)
4444
})
45+
46+
it('temporary code removed', function () {
47+
// Reason: https://sim.amazon.com/issues/IDE-10559
48+
// At this point in time most users who would have run in to this error
49+
// will have already had their old files deleted.
50+
const august2023 = new Date(2023, 7, 1)
51+
assert.ok(Date.now() < august2023.getTime(), 'remove `deleteOldFiles()` in `cache.ts`')
52+
})
4553
})

0 commit comments

Comments
 (0)