Skip to content

Commit

Permalink
Credentials: partial migration to SDK v3 #1766
Browse files Browse the repository at this point in the history
## Problem
SDK v2 has some known issues in regards to credentials from source_profile. SDK v3 does not fix them out-right, however, it does expose some functionality that allows us to fix the bugs.

## Solution
Migrate the credentials subsystem to use SDK v3, then add additional logic to handle source_profile resolution.
The new SDK credentials API allows us to 'pre-load' credentials when calling relevant APIs. We can use this functionality to load source profile credentials ourselves, letting the SDK treat them as static credentials. Any valid profile should now be usable as a base profile as well (e.g. processes, SSO, MFA, etc.)
  • Loading branch information
JadenSimon authored Aug 12, 2021
1 parent 740c4bb commit d1ce6de
Show file tree
Hide file tree
Showing 53 changed files with 4,734 additions and 2,647 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"type": "Bug Fix",
"description": "Credentials: correctly handle different `source_profile` combinations"
}
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,4 @@ src/shared/telemetry/clienttelemetry.d.ts

# Generated by tests
src/testFixtures/**/bin
src/testFixtures/**/obj
src/testFixtures/**/obj
9 changes: 4 additions & 5 deletions .gitpod.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
tasks:
- init: npm install
- init: npm install

vscode:
extensions:
- dbaeumer.vscode-eslint
- eg2.vscode-npm-script

extensions:
- dbaeumer.vscode-eslint
- eg2.vscode-npm-script
6,526 changes: 4,182 additions & 2,344 deletions package-lock.json

Large diffs are not rendered by default.

5 changes: 5 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2116,6 +2116,11 @@
"webpack-cli": "^4.7.2"
},
"dependencies": {
"@aws-sdk/client-sso": "^3.16.0",
"@aws-sdk/client-sso-oidc": "^3.16.0",
"@aws-sdk/credential-provider-ini": "^3.15.0",
"@aws-sdk/credential-provider-process": "^3.15.0",
"@aws-sdk/credential-provider-sso": "^3.16.0",
"adm-zip": "^0.4.13",
"amazon-states-language-service": "^1.6.4",
"async-lock": "^1.3.0",
Expand Down
12 changes: 10 additions & 2 deletions src/awsexplorer/awsExplorer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@ export class AwsExplorer implements vscode.TreeDataProvider<AWSTreeNodeBase>, Re
localize('AWS.explorerNode.addRegion', 'Add a region to {0} Explorer...', getIdeProperties().company),
'aws.showRegion',
undefined,
localize('AWS.explorerNode.addRegion.tooltip', 'Click here to add a region to {0} Explorer.', getIdeProperties().company)
localize(
'AWS.explorerNode.addRegion.tooltip',
'Click here to add a region to {0} Explorer.',
getIdeProperties().company
)
)

public constructor(
Expand All @@ -51,7 +55,11 @@ export class AwsExplorer implements vscode.TreeDataProvider<AWSTreeNodeBase>, Re
this.extContext.subscriptions.push(
this.awsContext.onDidChangeContext(e => {
if (!e.accountId) {
this.ROOT_NODE_SIGN_IN.label = localize('AWS.explorerNode.signIn', 'Connect to {0}...', getIdeProperties().company)
this.ROOT_NODE_SIGN_IN.label = localize(
'AWS.explorerNode.signIn',
'Connect to {0}...',
getIdeProperties().company
)
this.ROOT_NODE_SIGN_IN.tooltip = localize(
'AWS.explorerNode.signIn.tooltip',
'Click here to select credentials for the {0} Toolkit',
Expand Down
6 changes: 5 additions & 1 deletion src/awsexplorer/commands/copyArn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,11 @@ export async function copyArnCommand(
const logsItem = localize('AWS.generic.message.viewLogs', 'View Logs...')
window
.showErrorMessage(
localize('AWS.explorerNode.noArnFound', 'Could not find an ARN for selected {0} Explorer node', getIdeProperties().company),
localize(
'AWS.explorerNode.noArnFound',
'Could not find an ARN for selected {0} Explorer node',
getIdeProperties().company
),
logsItem
)
.then(selection => {
Expand Down
12 changes: 10 additions & 2 deletions src/credentials/awsCredentialsStatusBarItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,22 @@ export async function initializeAwsCredentialsStatusBarItem(
}

// Resolves when the status bar reaches its final state
export async function updateCredentialsStatusBarItem(statusBarItem: vscode.StatusBarItem, credentialsId?: string): Promise<void> {
export async function updateCredentialsStatusBarItem(
statusBarItem: vscode.StatusBarItem,
credentialsId?: string
): Promise<void> {
clearTimeout(timeoutID)

// Shows confirmation text in the status bar message
let delay = 0
if (credentialsId) {
delay = STATUSBAR_CONNECTED_DELAY
statusBarItem.text = localize('AWS.credentials.statusbar.text', '{0}: {1}', getIdeProperties().company, STATUSBAR_TEXT_CONNECTED)
statusBarItem.text = localize(
'AWS.credentials.statusbar.text',
'{0}: {1}',
getIdeProperties().company,
STATUSBAR_TEXT_CONNECTED
)
}

return new Promise<void>(
Expand Down
10 changes: 4 additions & 6 deletions src/credentials/credentialsCreator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,8 @@ const ERROR_MESSAGE_USER_CANCELLED = localize(
*/
export async function getMfaTokenFromUser(
mfaSerial: string,
profileName: string,
callback: (err?: Error, token?: string) => void
): Promise<void> {
profileName: string
): Promise<string> {
try {
const inputBox = createInputBox({
options: {
Expand All @@ -45,9 +44,8 @@ export async function getMfaTokenFromUser(
throw new Error(ERROR_MESSAGE_USER_CANCELLED)
}

callback(undefined, token)
return token
} catch (err) {
const error = err as Error
callback(error)
throw err as Error
}
}
21 changes: 16 additions & 5 deletions src/credentials/credentialsStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import * as AWS from 'aws-sdk'
import * as AWS from '@aws-sdk/types'
import { getLogger } from '../shared/logger/logger'
import { asString, CredentialsProvider, CredentialsId } from './providers/credentials'
import { CredentialsProviderManager } from './providers/credentialsProviderManager'
Expand All @@ -23,14 +23,25 @@ export class CredentialsStore {
this.credentialsCache = {}
}

/**
* Checks if the stored credentials are valid. Non-existent or expired credentials returns false.
*
* If the expiration property does not exist, it is assumed to never expire.
*/
public isValid(key: string): boolean {
if (this.credentialsCache[key]) {
const expiration = this.credentialsCache[key].credentials.expiration
return expiration !== undefined ? expiration >= new Date() : true
}

return false
}

/**
* Returns undefined if the specified credentials are expired or not found.
*/
public async getCredentials(credentials: CredentialsId): Promise<CachedCredentials | undefined> {
if (
this.credentialsCache[asString(credentials)] &&
!this.credentialsCache[asString(credentials)].credentials.expired
) {
if (this.isValid(asString(credentials))) {
return this.credentialsCache[asString(credentials)]
} else {
return undefined
Expand Down
17 changes: 8 additions & 9 deletions src/credentials/credentialsUtilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import * as nls from 'vscode-nls'
const localize = nls.loadMessageBundle()

import * as vscode from 'vscode'
import { Credentials } from 'aws-sdk'
import { Credentials } from '@aws-sdk/types'
import { credentialHelpUrl } from '../shared/constants'
import { Profile } from '../shared/credentials/credentialsFile'
import { isCloud9 } from '../shared/extensionUtilities'
Expand Down Expand Up @@ -64,16 +64,16 @@ export function hasProfileProperty(profile: Profile, propertyName: string): bool
* User cancellation or timeout expiration will cause rejection.
*
* @param profile Profile name to display for the progress message
* @param provider This can be a Promise that returns Credentials, or void if using 'refresh'
* @param provider A promise that resolves in Credentials
* @param timeout How long to wait for resolution without user intervention (default: 5 minutes)
*
* @returns The resolved Credentials or undefined if the the provider was a 'refresh' Promise
*/
export async function resolveProviderWithCancel<T extends AWS.Credentials | void>(
export async function resolveProviderWithCancel(
profile: string,
provider: Promise<T>,
provider: Promise<Credentials>,
timeout: Timeout | number = CREDENTIALS_TIMEOUT
): Promise<T> {
): Promise<Credentials> {
if (typeof timeout === 'number') {
timeout = new Timeout(timeout)
}
Expand All @@ -88,14 +88,13 @@ export async function resolveProviderWithCancel<T extends AWS.Credentials | void
}
}, CREDENTIALS_PROGRESS_DELAY)

await waitTimeout(provider, timeout, {
return (await waitTimeout(provider, timeout, {
onCancel: () => {
throw new Error(`Request to get credentials for "${profile}" cancelled`)
},
onExpire: () => {
throw new Error(`Request to get credentials for "${profile}" expired`)
},
})

return provider
allowUndefined: false,
})) as Credentials
}
1 change: 1 addition & 0 deletions src/credentials/providers/credentials.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import * as AWS from '@aws-sdk/types'
import * as telemetry from '../../shared/telemetry/telemetry.gen'

const CREDENTIALS_PROVIDER_ID_SEPARATOR = ':'
Expand Down
5 changes: 3 additions & 2 deletions src/credentials/providers/credentialsProviderFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { CredentialsProvider, CredentialsProviderType , CredentialsId, isEqual } from './credentials'
import { CredentialsProvider, CredentialsProviderType, CredentialsId, isEqual } from './credentials'

/**
* Responsible for producing CredentialsProvider objects for a Credential Type
Expand All @@ -19,7 +19,8 @@ export interface CredentialsProviderFactory {
}

export abstract class BaseCredentialsProviderFactory<T extends CredentialsProvider>
implements CredentialsProviderFactory {
implements CredentialsProviderFactory
{
protected providers: T[] = []

public getProviderType(): CredentialsProviderType | undefined {
Expand Down
7 changes: 4 additions & 3 deletions src/credentials/providers/ec2CredentialsProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { Credentials, EC2MetadataCredentials } from 'aws-sdk'
import { Credentials } from '@aws-sdk/types'
import { fromInstanceMetadata } from '@aws-sdk/credential-provider-imds'
import { DefaultEc2MetadataClient } from '../../shared/clients/ec2MetadataClient'
import { Ec2MetadataClient } from '../../shared/clients/ec2MetadataClient'
import { getLogger } from '../../shared/logger'
Expand All @@ -17,7 +18,7 @@ import { CredentialsId, CredentialsProvider, CredentialsProviderType } from './c
* @see CredentialsProviderType
*/
export class Ec2CredentialsProvider implements CredentialsProvider {
private credentials: EC2MetadataCredentials | undefined
private credentials: Credentials | undefined
private region: string | undefined
private available: boolean | undefined

Expand Down Expand Up @@ -86,7 +87,7 @@ export class Ec2CredentialsProvider implements CredentialsProvider {

public async getCredentials(): Promise<Credentials> {
if (!this.credentials) {
this.credentials = new EC2MetadataCredentials()
this.credentials = await fromInstanceMetadata()()
}
return this.credentials
}
Expand Down
7 changes: 4 additions & 3 deletions src/credentials/providers/ecsCredentialsProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { Credentials, ECSCredentials } from 'aws-sdk'
import { Credentials } from '@aws-sdk/types'
import { fromContainerMetadata } from '@aws-sdk/credential-provider-imds'
import { EnvironmentVariables } from '../../shared/environmentVariables'
import { CredentialType } from '../../shared/telemetry/telemetry.gen'
import { getStringHash } from '../../shared/utilities/textUtilities'
Expand All @@ -15,7 +16,7 @@ import { CredentialsId, CredentialsProvider, CredentialsProviderType } from './c
* @see CredentialsProviderType
*/
export class EcsCredentialsProvider implements CredentialsProvider {
private credentials: ECSCredentials | undefined
private credentials: Credentials | undefined

public async isAvailable(): Promise<boolean> {
const env = process.env as EnvironmentVariables
Expand Down Expand Up @@ -56,7 +57,7 @@ export class EcsCredentialsProvider implements CredentialsProvider {

public async getCredentials(): Promise<Credentials> {
if (!this.credentials) {
this.credentials = new ECSCredentials()
this.credentials = await fromContainerMetadata()()
}
return this.credentials
}
Expand Down
9 changes: 4 additions & 5 deletions src/credentials/providers/envVarsCredentialsProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { Credentials, EnvironmentCredentials } from 'aws-sdk'
import { Credentials } from '@aws-sdk/types'
import { fromEnv } from '@aws-sdk/credential-provider-env'
import { EnvironmentVariables } from '../../shared/environmentVariables'
import { CredentialType } from '../../shared/telemetry/telemetry.gen'
import { getStringHash } from '../../shared/utilities/textUtilities'
Expand All @@ -15,9 +16,7 @@ import { CredentialsId, CredentialsProvider, CredentialsProviderType } from './c
* @see CredentialsProviderType
*/
export class EnvVarsCredentialsProvider implements CredentialsProvider {
public static readonly AWS_ENV_VAR_PREFIX: string = 'AWS'

private credentials: EnvironmentCredentials | undefined
private credentials: Credentials | undefined

public async isAvailable(): Promise<boolean> {
const env = process.env as EnvironmentVariables
Expand Down Expand Up @@ -58,7 +57,7 @@ export class EnvVarsCredentialsProvider implements CredentialsProvider {

public async getCredentials(): Promise<Credentials> {
if (!this.credentials) {
this.credentials = new EnvironmentCredentials(EnvVarsCredentialsProvider.AWS_ENV_VAR_PREFIX)
this.credentials = await fromEnv()()
}
return this.credentials
}
Expand Down
Loading

0 comments on commit d1ce6de

Please sign in to comment.