Skip to content

Commit a04603b

Browse files
authored
fix(auth): static credentials without an expiration are not coherent with the source #3407
Problem: It's possible to load temporary credentials without an explicit expiration time. Cloud9 writes credentials in this way. Without an expiration time, we assume they are good forever even when the underlying source changes. Solution: Fetch a new provider on every call. CredentialsProviderManager calls fs.stat every time which might have performance implications. This really only affects CW on C9, need to test to see if it's noticeable.
1 parent 6b8ac98 commit a04603b

File tree

5 files changed

+83
-21
lines changed

5 files changed

+83
-21
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: changes to the AWS shared configuration files are not always respected when fetching credentials"
4+
}

src/credentials/auth.ts

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -296,14 +296,14 @@ async function loadIamProfilesIntoStore(store: ProfileStore, manager: Credential
296296
continue
297297
}
298298

299-
if (providers[id] === undefined) {
300-
await store.deleteProfile(id)
301-
} else if (profile.subtype === 'linked') {
299+
if (profile.subtype === 'linked') {
302300
const source = store.getProfile(profile.ssoSession)
303301
if (source === undefined || source.type !== 'sso') {
304302
await store.deleteProfile(id)
305303
manager.removeProvider(fromString(id))
306304
}
305+
} else if (providers[id] === undefined) {
306+
await store.deleteProfile(id)
307307
}
308308
}
309309

@@ -430,7 +430,7 @@ export class Auth implements AuthService, ConnectionManager {
430430
const provider = await this.getCredentialsProvider(id, profile)
431431
await this.authenticate(id, () => this.createCachedCredentials(provider))
432432

433-
return this.getIamConnection(id, provider)
433+
return this.getIamConnection(id, profile)
434434
}
435435
}
436436

@@ -444,9 +444,7 @@ export class Auth implements AuthService, ConnectionManager {
444444

445445
const validated = await this.validateConnection(id, profile)
446446
const conn =
447-
validated.type === 'sso'
448-
? this.getSsoConnection(id, validated)
449-
: this.getIamConnection(id, await this.getCredentialsProvider(id, validated))
447+
validated.type === 'sso' ? this.getSsoConnection(id, validated) : this.getIamConnection(id, validated)
450448

451449
this.#activeConnection = conn
452450
this.#onDidChangeActiveConnection.fire(conn)
@@ -685,7 +683,7 @@ export class Auth implements AuthService, ConnectionManager {
685683
if (profile.type === 'sso') {
686684
return this.getSsoConnection(id, profile)
687685
} else {
688-
return this.getIamConnection(id, await this.getCredentialsProvider(id, profile))
686+
return this.getIamConnection(id, profile)
689687
}
690688
}
691689

@@ -699,6 +697,10 @@ export class Auth implements AuthService, ConnectionManager {
699697
return provider
700698
}
701699

700+
return this.getSsoLinkedCredentialsProvider(id, profile)
701+
}
702+
703+
private getSsoLinkedCredentialsProvider(id: Connection['id'], profile: LinkedIamProfile) {
702704
const sourceProfile = this.store.getProfile(profile.ssoSession)
703705
if (sourceProfile === undefined) {
704706
throw new Error(`Source profile for "${id}" no longer exists`)
@@ -760,16 +762,17 @@ export class Auth implements AuthService, ConnectionManager {
760762
)
761763
}
762764

763-
private getIamConnection(id: Connection['id'], provider: CredentialsProvider): IamConnection & StatefulConnection {
764-
const profile = this.store.getProfileOrThrow(id)
765-
765+
private getIamConnection(
766+
id: Connection['id'],
767+
profile: StoredProfile<IamProfile>
768+
): IamConnection & StatefulConnection {
766769
return {
767770
id,
768771
type: 'iam',
769772
state: profile.metadata.connectionState,
770773
label:
771774
profile.metadata.label ?? (profile.type === 'iam' && profile.subtype === 'linked' ? profile.name : id),
772-
getCredentials: () => this.getCredentials(id, provider),
775+
getCredentials: async () => this.getCredentials(id, await this.getCredentialsProvider(id, profile)),
773776
}
774777
}
775778

src/test/credentials/auth.test.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,11 @@ import { captureEventOnce } from '../testUtil'
2020
import { createBuilderIdProfile, createSsoProfile, createTestAuth } from './testUtil'
2121
import { toCollection } from '../../shared/utilities/asyncCollection'
2222
import globals from '../../shared/extensionGlobals'
23+
import { SystemUtilities } from '../../shared/systemUtilities'
24+
import { makeTemporaryToolkitFolder } from '../../shared/filesystemUtilities'
25+
import { SharedCredentialsProviderFactory } from '../../credentials/providers/sharedCredentialsProviderFactory'
26+
import { UserCredentialsUtils } from '../../shared/credentials/userCredentialsUtils'
27+
import { getCredentialsFilename } from '../../credentials/sharedCredentialsFile'
2328

2429
const ssoProfile = createSsoProfile()
2530
const scopedSsoProfile = createSsoProfile({ scopes: ['foo'] })
@@ -360,6 +365,50 @@ describe('Auth', function () {
360365
})
361366
})
362367

368+
describe('Shared ini files', function () {
369+
let tmpDir: string
370+
371+
beforeEach(async function () {
372+
tmpDir = await makeTemporaryToolkitFolder()
373+
sinon.stub(SystemUtilities, 'getHomeDirectory').returns(tmpDir)
374+
sinon.stub(globals.loginManager, 'validateCredentials').resolves('123')
375+
auth.credentialsManager.addProviderFactory(new SharedCredentialsProviderFactory())
376+
})
377+
378+
afterEach(async function () {
379+
sinon.restore()
380+
await SystemUtilities.delete(tmpDir, { recursive: true })
381+
})
382+
383+
it('does not cache if the credentials file changes', async function () {
384+
const initialCreds = {
385+
profileName: 'default',
386+
accessKey: 'x',
387+
secretKey: 'x',
388+
}
389+
390+
await UserCredentialsUtils.generateCredentialsFile(initialCreds)
391+
392+
const conn = await auth.getConnection({ id: 'profile:default' })
393+
assert.ok(conn?.type === 'iam', 'Expected an IAM connection')
394+
assert.deepStrictEqual(await conn.getCredentials(), {
395+
accessKeyId: initialCreds.accessKey,
396+
secretAccessKey: initialCreds.secretKey,
397+
sessionToken: undefined,
398+
})
399+
400+
await SystemUtilities.delete(getCredentialsFilename())
401+
402+
const newCreds = { ...initialCreds, accessKey: 'y', secretKey: 'y' }
403+
await UserCredentialsUtils.generateCredentialsFile(newCreds)
404+
assert.deepStrictEqual(await conn.getCredentials(), {
405+
accessKeyId: newCreds.accessKey,
406+
secretAccessKey: newCreds.secretKey,
407+
sessionToken: undefined,
408+
})
409+
})
410+
})
411+
363412
describe('AuthNode', function () {
364413
it('shows a message to create a connection if no connections exist', async function () {
365414
const node = new AuthNode(auth)

src/test/credentials/provider/sharedCredentialsProvider.test.ts

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55

6-
import * as vscode from 'vscode'
76
import * as assert from 'assert'
87
import * as FakeTimers from '@sinonjs/fake-timers'
98
import * as sinon from 'sinon'
@@ -12,20 +11,13 @@ import { stripUndefined } from '../../../shared/utilities/collectionUtils'
1211
import * as process from '@aws-sdk/credential-provider-process'
1312
import { ParsedIniData } from '@aws-sdk/shared-ini-file-loader'
1413
import { installFakeClock } from '../../testUtil'
15-
import { parseIni, mergeAndValidateSections } from '../../../credentials/sharedCredentials'
1614
import { SsoClient } from '../../../credentials/sso/clients'
1715
import { stub } from '../../utilities/stubber'
1816
import { SsoAccessTokenProvider } from '../../../credentials/sso/ssoAccessTokenProvider'
17+
import { createTestSections } from '../testUtil'
1918

2019
const missingPropertiesFragment = 'missing properties'
2120

22-
async function createTestSections(ini: string) {
23-
const doc = await vscode.workspace.openTextDocument({ content: ini })
24-
const sections = parseIni(doc.getText(), doc.uri)
25-
26-
return mergeAndValidateSections(sections).sections
27-
}
28-
2921
describe('SharedCredentialsProvider', async function () {
3022
let clock: FakeTimers.InstalledClock
3123
let sandbox: sinon.SinonSandbox

src/test/credentials/testUtil.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
*/
55

66
import * as assert from 'assert'
7+
import * as vscode from 'vscode'
78
import { Auth, Connection, ProfileStore, SsoConnection, SsoProfile } from '../../credentials/auth'
89
import { CredentialsProviderManager } from '../../credentials/providers/credentialsProviderManager'
910
import { SsoClient } from '../../credentials/sso/clients'
@@ -14,6 +15,8 @@ import { captureEvent, EventCapturer } from '../testUtil'
1415
import { stub } from '../utilities/stubber'
1516
import globals from '../../shared/extensionGlobals'
1617
import { fromString } from '../../credentials/providers/credentials'
18+
import { mergeAndValidateSections, parseIni } from '../../credentials/sharedCredentials'
19+
import { SharedCredentialsProvider } from '../../credentials/providers/sharedCredentialsProvider'
1720

1821
export function createSsoProfile(props?: Partial<Omit<SsoProfile, 'type'>>): SsoProfile {
1922
return {
@@ -28,6 +31,17 @@ export function createBuilderIdProfile(props?: Partial<Omit<SsoProfile, 'type' |
2831
return createSsoProfile({ startUrl: builderIdStartUrl, ...props })
2932
}
3033

34+
export async function createTestSections(ini: string) {
35+
const doc = await vscode.workspace.openTextDocument({ content: ini })
36+
const sections = parseIni(doc.getText(), doc.uri)
37+
38+
return mergeAndValidateSections(sections).sections
39+
}
40+
41+
export async function createSharedCredentialsProvider(name: string, ini: string) {
42+
return new SharedCredentialsProvider(name, await createTestSections(ini))
43+
}
44+
3145
function createTestTokenProvider() {
3246
let token: SsoToken | undefined
3347
let counter = 0

0 commit comments

Comments
 (0)