Skip to content

Commit

Permalink
fix(cli): short-lived credentials are not refreshed
Browse files Browse the repository at this point in the history
The CLI immediately invokes credential *providers* to produce
*credentials*, and passes the credentials around instead of the
providers. This means that if short-lived credentials expire (like
session credentials from roles), there is no way to refresh them.

CLI calls will start to fail if that happens.

To fix this, instead of resolving providers to credentials, pass
providers around instead.

Implications for auth plugins
-------------

This widens the plugin protocol: the new plugin protocol *forced* a
translation to V3 credentials, and had no way to return V3 providers.
While it is now possible to return V3 Credential Providers from the
plugin protocol, plugin writers cannot easily take advantage of that
protocol because there have been ~8 CLI releases that only support V3
credentials and will fail at runtime of V3 providers are returned.

To support this, pass a new options argument into `getProvider()`:
this will indicate whether V3 Providers are supported or not. Plugins
can return a provider if the CLI indicates that it supports V3
providers, and avoid doing that if the CLI indicates it won't. That way,
plugins can be rewritten to take advantage of returning V3 providers
without crashing on CLI versions `2.167.0..(this releases)`.

This also affects #32111 in which the plugin contract is being moved.
  • Loading branch information
rix0rrr committed Dec 2, 2024
1 parent 24adca3 commit c4b32dc
Show file tree
Hide file tree
Showing 5 changed files with 192 additions and 40 deletions.
10 changes: 10 additions & 0 deletions packages/aws-cdk/lib/api/aws-auth/cached.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,13 @@ export function cached<A extends object, B>(obj: A, sym: symbol, fn: () => B): B
}
return (obj as any)[sym];
}

/**
* Like 'cached', but async
*/
export async function cachedAsync<A extends object, B>(obj: A, sym: symbol, fn: () => Promise<B>): Promise<B> {
if (!(sym in obj)) {
(obj as any)[sym] = await fn();
}
return (obj as any)[sym];
}
91 changes: 69 additions & 22 deletions packages/aws-cdk/lib/api/aws-auth/credential-plugins.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { AwsCredentialIdentity } from '@smithy/types';
import { inspect } from 'util';
import type { AwsCredentialIdentityProvider } from '@smithy/types';
import { debug, warning } from '../../logging';
import { CredentialProviderSource, Mode, PluginHost } from '../plugin';
import { CredentialProviderSource, PluginProviderResult, Mode, PluginHost, SDKv2CompatibleCredentials, SDKv3CompatibleCredentialProvider, SDKv3CompatibleCredentials } from '../plugin';

/**
* Cache for credential providers.
Expand All @@ -15,9 +16,9 @@ import { CredentialProviderSource, Mode, PluginHost } from '../plugin';
* for the given account.
*/
export class CredentialPlugins {
private readonly cache: { [key: string]: PluginCredentials | undefined } = {};
private readonly cache: { [key: string]: PluginCredentialsFetchResult | undefined } = {};

public async fetchCredentialsFor(awsAccountId: string, mode: Mode): Promise<PluginCredentials | undefined> {
public async fetchCredentialsFor(awsAccountId: string, mode: Mode): Promise<PluginCredentialsFetchResult | undefined> {
const key = `${awsAccountId}-${mode}`;
if (!(key in this.cache)) {
this.cache[key] = await this.lookupCredentials(awsAccountId, mode);
Expand All @@ -29,7 +30,7 @@ export class CredentialPlugins {
return PluginHost.instance.credentialProviderSources.map((s) => s.name);
}

private async lookupCredentials(awsAccountId: string, mode: Mode): Promise<PluginCredentials | undefined> {
private async lookupCredentials(awsAccountId: string, mode: Mode): Promise<PluginCredentialsFetchResult | undefined> {
const triedSources: CredentialProviderSource[] = [];
// Otherwise, inspect the various credential sources we have
for (const source of PluginHost.instance.credentialProviderSources) {
Expand Down Expand Up @@ -59,28 +60,74 @@ export class CredentialPlugins {
continue;
}
debug(`Using ${source.name} credentials for account ${awsAccountId}`);
const providerOrCreds = await source.getProvider(awsAccountId, mode);
const providerOrCreds = await source.getProvider(awsAccountId, mode, {
supportsV3Providers: true,
});

// Backwards compatibility: if the plugin returns a ProviderChain, resolve that chain.
// Otherwise it must have returned credentials.
const credentials = (providerOrCreds as any).resolvePromise
? await (providerOrCreds as any).resolvePromise()
: providerOrCreds;

// Another layer of backwards compatibility: in SDK v2, the credentials object
// is both a container and a provider. So we need to force the refresh using getPromise.
// In SDK v3, these two responsibilities are separate, and the getPromise doesn't exist.
if ((credentials as any).getPromise) {
await (credentials as any).getPromise();
}

return { credentials, pluginName: source.name };
return {
credentials: v3ProviderFromPluginResult(providerOrCreds),
pluginName: source.name,
};
}
return undefined;
}
}

export interface PluginCredentials {
readonly credentials: AwsCredentialIdentity;
/**
* Result from trying to fetch credentials from the Plugin host
*/
export interface PluginCredentialsFetchResult {
/**
* SDK-v3 compatible credential provider
*/
readonly credentials: AwsCredentialIdentityProvider;

/**
* Name of plugin that successfully provided credentials
*/
readonly pluginName: string;
}

/**
* Converts whatever the plugin gave us into an SDKv3-compatible credential provider.
*/
function v3ProviderFromPluginResult(x: PluginProviderResult): AwsCredentialIdentityProvider {
if (isV3Provider(x)) {
return x;
} else if (isV3Credentials(x)) {
return () => Promise.resolve(x);
} else if (isV2Credentials(x)) {
return v3ProviderFromV2Credentials(x);
} else {
throw new Error(`Unrecognized credential provider result: ${inspect(x)}`);
}
}

/**
* Converts a V2 credential into a V3-compatible provider
*/
function v3ProviderFromV2Credentials(x: SDKv2CompatibleCredentials): AwsCredentialIdentityProvider {
return async () => {
// Get will fetch or refresh as necessary
await x.getPromise();

return {
accessKeyId: x.accessKeyId,
secretAccessKey: x.secretAccessKey,
sessionToken: x.sessionToken,
expiration: x.expireTime,
};
};
}

function isV3Provider(x: PluginProviderResult): x is SDKv3CompatibleCredentialProvider {
return typeof x === 'function';
}

function isV2Credentials(x: PluginProviderResult): x is SDKv2CompatibleCredentials {
return !!(x && typeof x === 'object' && x.accessKeyId && (x as SDKv2CompatibleCredentials).getPromise);
}

function isV3Credentials(x: PluginProviderResult): x is SDKv3CompatibleCredentials {
return !!(x && typeof x === 'object' && x.accessKeyId && !isV2Credentials(x));
}
14 changes: 7 additions & 7 deletions packages/aws-cdk/lib/api/aws-auth/sdk-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ export class SdkProvider {
throw new Error('Unable to resolve AWS credentials (setup with "aws configure")');
}

return await new SDK(credentials, this.defaultRegion, this.requestHandler, this.logger).currentAccount();
return await new SDK(this.defaultCredentials, this.defaultRegion, this.requestHandler, this.logger).currentAccount();
} catch (e: any) {
// Treat 'ExpiredToken' specially. This is a common situation that people may find themselves in, and
// they are complaining about if we fail 'cdk synth' on them. We loudly complain in order to show that
Expand Down Expand Up @@ -307,7 +307,7 @@ export class SdkProvider {
if (defaultAccountId === accountId) {
return {
source: 'correctDefault',
credentials: await this.defaultCredentials(),
credentials: await this.defaultCredentialProvider,
};
}

Expand All @@ -322,7 +322,7 @@ export class SdkProvider {
return {
source: 'incorrectDefault',
accountId: defaultAccountId,
credentials: await this.defaultCredentials(),
credentials: await this.defaultCredentialProvider,
unusedPlugins: this.plugins.availablePluginNames,
};
}
Expand Down Expand Up @@ -380,7 +380,7 @@ export class SdkProvider {
customUserAgent: 'aws-cdk',
logger: this.logger,
},
})();
});

return new SDK(credentials, region, this.requestHandler, this.logger);
} catch (err: any) {
Expand Down Expand Up @@ -457,11 +457,11 @@ export interface CredentialsOptions {
* Result of obtaining base credentials
*/
type ObtainBaseCredentialsResult =
| { source: 'correctDefault'; credentials: AwsCredentialIdentity }
| { source: 'plugin'; pluginName: string; credentials: AwsCredentialIdentity }
| { source: 'correctDefault'; credentials: AwsCredentialIdentityProvider }
| { source: 'plugin'; pluginName: string; credentials: AwsCredentialIdentityProvider }
| {
source: 'incorrectDefault';
credentials: AwsCredentialIdentity;
credentials: AwsCredentialIdentityProvider;
accountId: string;
unusedPlugins: string[];
}
Expand Down
19 changes: 10 additions & 9 deletions packages/aws-cdk/lib/api/aws-auth/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -311,11 +311,11 @@ import { GetCallerIdentityCommand, STSClient } from '@aws-sdk/client-sts';
import { Upload } from '@aws-sdk/lib-storage';
import { getEndpointFromInstructions } from '@smithy/middleware-endpoint';
import type { NodeHttpHandlerOptions } from '@smithy/node-http-handler';
import { AwsCredentialIdentity, Logger } from '@smithy/types';
import { AwsCredentialIdentityProvider, Logger } from '@smithy/types';
import { ConfiguredRetryStrategy } from '@smithy/util-retry';
import { WaiterResult } from '@smithy/util-waiter';
import { AccountAccessKeyCache } from './account-cache';
import { cached } from './cached';
import { cachedAsync } from './cached';
import { Account } from './sdk-provider';
import { defaultCliUserAgent } from './user-agent';
import { debug } from '../../logging';
Expand Down Expand Up @@ -350,7 +350,7 @@ export interface SdkOptions {

export interface ConfigurationOptions {
region: string;
credentials: AwsCredentialIdentity;
credentials: AwsCredentialIdentityProvider;
requestHandler: NodeHttpHandlerOptions;
retryStrategy: ConfiguredRetryStrategy;
customUserAgent: string;
Expand Down Expand Up @@ -542,14 +542,14 @@ export class SDK {
private _credentialsValidated = false;

constructor(
private readonly _credentials: AwsCredentialIdentity,
private readonly credProvider: AwsCredentialIdentityProvider,
region: string,
requestHandler: NodeHttpHandlerOptions,
logger?: Logger,
) {
this.config = {
region,
credentials: _credentials,
credentials: credProvider,
requestHandler,
retryStrategy: new ConfiguredRetryStrategy(7, (attempt) => 300 * (2 ** attempt)),
customUserAgent: defaultCliUserAgent(),
Expand Down Expand Up @@ -943,8 +943,9 @@ export class SDK {
}

public async currentAccount(): Promise<Account> {
return cached(this, CURRENT_ACCOUNT_KEY, () =>
SDK.accountCache.fetch(this._credentials.accessKeyId, async () => {
return cachedAsync(this, CURRENT_ACCOUNT_KEY, async () => {
const creds = await this.credProvider();
return SDK.accountCache.fetch(creds.accessKeyId, async () => {
// if we don't have one, resolve from STS and store in cache.
debug('Looking up default account ID from STS');
const client = new STSClient({
Expand All @@ -963,8 +964,8 @@ export class SDK {
// Save another STS call later if this one already succeeded
this._credentialsValidated = true;
return { accountId, partition };
}),
);
});
});
}

/**
Expand Down
98 changes: 96 additions & 2 deletions packages/aws-cdk/lib/api/plugin/credential-provider-source.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import type { AwsCredentialIdentity } from '@smithy/types';
export enum Mode {
ForReading,
ForWriting,
Expand All @@ -25,6 +24,101 @@ export interface CredentialProviderSource {
* Construct a credential provider for the given account and the given access mode
*
* Guaranteed to be called only if canProvideCredentails() returned true at some point.
*
* While it is possible for the plugin to return a static set of credentials, it is
* recommended to return a provider.
*/
getProvider(accountId: string, mode: Mode, options?: PluginProviderOptions): Promise<PluginProviderResult>;
}

/**
* Options for the `getProvider()` function of a CredentialProviderSource
*/
export interface PluginProviderOptions {
/**
* Whether or not this implementation of the CLI will recognize the `SDKv3CompatibleCredentialProvider` return variant
*
* Unless otherwise indicated, the CLI version will only support SDKv3
* credentials, not SDKv3 providers. You should avoid returning types that the
* consuming CLI will not understand, because it will most likely crash.
*
* @default false
*/
getProvider(accountId: string, mode: Mode): Promise<AwsCredentialIdentity>;
readonly supportsV3Providers?: boolean;
}

export type PluginProviderResult = SDKv2CompatibleCredentials | SDKv3CompatibleCredentialProvider | SDKv3CompatibleCredentials;

/**
* SDKv2-compatible credential provider.
*
* Based on the `Credentials` class in SDKv2. This object is a set of credentials
* and a credential provider in one (it is a set of credentials that remember
* where they came from and can refresh themselves).
*/
export interface SDKv2CompatibleCredentials {
/**
* AWS access key ID.
*/
accessKeyId: string;
/**
* Whether the credentials have been expired and require a refresh.
* Used in conjunction with expireTime.
*/
expired: boolean;
/**
* Time when credentials should be considered expired.
* Used in conjunction with expired.
*/
expireTime: Date;
/**
* AWS secret access key.
*/
secretAccessKey: string;
/**
* AWS session token.
*/
sessionToken: string;

/**
* Gets the existing credentials, refreshing them if necessary, and returns
* a promise that will be fulfilled immediately (if no refresh is necessary)
* or when the refresh has completed.
*/
getPromise(): Promise<void>;
}

/**
* Provider for credentials
*
* Based on the `AwsCredentialIdentityProvider` type from SDKv3. This type
* is only a credential factory. It may or may not be cached; that is,
* calling the provider twice may do 2 API requests, or it may do one
* if the result from the first call can be reused.
*/
export type SDKv3CompatibleCredentialProvider = (identityProperties?: Record<string, any>) => Promise<SDKv3CompatibleCredentials>;

/**
* Based on the `AwsCredentialIdentity` type from SDKv3.
*
* This is a static set of credentials.
*/
export interface SDKv3CompatibleCredentials {
/**
* AWS access key ID
*/
readonly accessKeyId: string;
/**
* AWS secret access key
*/
readonly secretAccessKey: string;
/**
* A security or session token to use with these credentials. Usually
* present for temporary credentials.
*/
readonly sessionToken?: string;
/**
* A `Date` when the identity or credential will no longer be accepted.
*/
readonly expiration?: Date;
}

0 comments on commit c4b32dc

Please sign in to comment.