Skip to content

Commit d1ead62

Browse files
authored
Merge #3362 nkomonen-amazon/removeOldSsoCacheSimpler
2 parents 6cfe24a + c7913f2 commit d1ead62

File tree

3 files changed

+178
-28
lines changed

3 files changed

+178
-28
lines changed

src/credentials/sso/cache.ts

Lines changed: 86 additions & 26 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,32 +34,57 @@ 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

43-
export function getRegistrationCache(directory = getCacheDir()): KeyedCache<ClientRegistration, RegistrationKey> {
44-
const hashScopes = (scopes: string[]) => {
45-
const shasum = crypto.createHash('sha256')
46-
scopes.forEach(s => shasum.update(s))
47-
return shasum.digest('hex')
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
4854
}
4955

50-
const getTarget = (key: RegistrationKey) => {
51-
const suffix = `${key.region}${key.scopes && key.scopes.length > 0 ? `-${hashScopes(key.scopes)}` : ''}`
52-
return path.join(directory, `aws-toolkit-vscode-client-id-${suffix}.json`)
53-
}
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+
}
5476

77+
export function getRegistrationCache(directory = getCacheDir()): KeyedCache<ClientRegistration, RegistrationKey> {
5578
// Compatability for older Toolkit versions (format on disk is unchanged)
5679
type StoredRegistration = Omit<ClientRegistration, 'expiresAt'> & { readonly expiresAt: string }
5780
const read = (data: StoredRegistration) => ({ ...data, expiresAt: new Date(data.expiresAt) })
5881
const write = (data: ClientRegistration) => ({ ...data, expiresAt: data.expiresAt.toISOString() })
5982

6083
const logger = (message: string) => getLogger().debug(`SSO registration cache: ${message}`)
61-
const cache: KeyedCache<StoredRegistration, RegistrationKey> = createDiskCache(getTarget, logger)
84+
const cache: KeyedCache<StoredRegistration, RegistrationKey> = createDiskCache(
85+
(registrationKey: RegistrationKey) => getRegistrationCacheFile(directory, registrationKey),
86+
logger
87+
)
6288

6389
return mapCache(cache, read, write)
6490
}
@@ -112,24 +138,58 @@ export function getTokenCache(directory = getCacheDir()): KeyedCache<SsoAccess>
112138
}
113139
}
114140

115-
const getTarget = (ssoUrl: string) => {
116-
const encoded = encodeURI(ssoUrl)
117-
// Per the spec: 'SSO Login Token Flow' the access token must be
118-
// cached as the SHA1 hash of the bytes of the UTF-8 encoded
119-
// startUrl value with ".json" appended to the end.
141+
const logger = (message: string) => getLogger().debug(`SSO token cache: ${message}`)
142+
const cache = createDiskCache<StoredToken, string>((ssoUrl: string) => getTokenCacheFile(directory, ssoUrl), logger)
120143

121-
const shasum = crypto.createHash('sha1')
122-
// Suppress warning because:
123-
// 1. SHA1 is prescribed by the AWS SSO spec
124-
// 2. the hashed startUrl value is not a secret
125-
shasum.update(encoded) // lgtm[js/weak-cryptographic-algorithm]
126-
const hashedUrl = shasum.digest('hex') // lgtm[js/weak-cryptographic-algorithm]
144+
return mapCache(cache, read, write)
145+
}
127146

128-
return path.join(directory, `${hashedUrl}.json`)
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
129167
}
168+
}
130169

131-
const logger = (message: string) => getLogger().debug(`SSO token cache: ${message}`)
132-
const cache = createDiskCache<StoredToken, string>(getTarget, logger)
170+
export function getTokenCacheFile(ssoCacheDir: string, ssoUrl: string): string {
171+
const encoded = encodeURI(ssoUrl)
172+
// Per the spec: 'SSO Login Token Flow' the access token must be
173+
// cached as the SHA1 hash of the bytes of the UTF-8 encoded
174+
// startUrl value with ".json" appended to the end.
133175

134-
return mapCache(cache, read, write)
176+
const shasum = crypto.createHash('sha1')
177+
// Suppress warning because:
178+
// 1. SHA1 is prescribed by the AWS SSO spec
179+
// 2. the hashed startUrl value is not a secret
180+
shasum.update(encoded) // lgtm[js/weak-cryptographic-algorithm]
181+
const hashedUrl = shasum.digest('hex') // lgtm[js/weak-cryptographic-algorithm]
182+
183+
return path.join(ssoCacheDir, `${hashedUrl}.json`)
184+
}
185+
186+
export function getRegistrationCacheFile(ssoCacheDir: string, key: RegistrationKey): string {
187+
const hashScopes = (scopes: string[]) => {
188+
const shasum = crypto.createHash('sha256')
189+
scopes.forEach(s => shasum.update(s))
190+
return shasum.digest('hex')
191+
}
192+
193+
const suffix = `${key.region}${key.scopes && key.scopes.length > 0 ? `-${hashScopes(key.scopes)}` : ''}`
194+
return path.join(ssoCacheDir, `aws-toolkit-vscode-client-id-${suffix}.json`)
135195
}

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)