Skip to content

Commit d452163

Browse files
authored
fix(toolkit-lib): MFA token cannot be provided through IoHost (#508)
Relates to #396 Considering this a bug fix, since it is currently not possible to integrate an IoHost with this. While this does affect the CLI, the DX is virtually unchanged. I run the auth test suite to ensure everything is still working as expected. Before: <img width="589" alt="before" src="https://github.com/user-attachments/assets/cb5d7d39-ed81-4bef-b859-3eb26d964f59" /> After: <img width="619" alt="after" src="https://github.com/user-attachments/assets/bf410b53-44c5-4657-ab6b-890ca63fb508" /> --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
1 parent 1445ba1 commit d452163

File tree

12 files changed

+86
-21
lines changed

12 files changed

+86
-21
lines changed

.github/workflows/integ.yml

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.projenrc.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1706,6 +1706,7 @@ new CdkCliIntegTestsWorkflow(repo, {
17061706
cdkAliasPackage.name,
17071707
cliInteg.name,
17081708
toolkitLib.name,
1709+
cliPluginContract.name,
17091710
],
17101711

17111712
allowUpstreamVersions: [

packages/@aws-cdk/toolkit-lib/docs/message-registry.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,3 +102,4 @@ group: Documents
102102
| `CDK_SDK_I0000` | An SDK debug message. | `debug` | n/a |
103103
| `CDK_SDK_W0000` | An SDK warning message. | `warn` | n/a |
104104
| `CDK_SDK_I0100` | An SDK trace. SDK traces are emitted as traces to the IoHost, but contain the original SDK logging level. | `trace` | {@link SdkTrace} |
105+
| `CDK_SDK_I1100` | Get an MFA token for an MFA device. | `info` | {@link MfaTokenRequest} |

packages/@aws-cdk/toolkit-lib/lib/api/aws-auth/awscli-compatible.ts

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import { createCredentialChain, fromEnv, fromIni, fromNodeProviderChain } from '
44
import { MetadataService } from '@aws-sdk/ec2-metadata-service';
55
import type { NodeHttpHandlerOptions } from '@smithy/node-http-handler';
66
import { loadSharedConfigFiles } from '@smithy/shared-ini-file-loader';
7-
import * as promptly from 'promptly';
87
import { makeCachingProvider } from './provider-caching';
98
import { ProxyAgentProvider } from './proxy-agent';
109
import type { ISdkLogger } from './sdk-logger';
@@ -219,18 +218,18 @@ export class AwsCliCompatible {
219218
}
220219

221220
/**
222-
* Ask user for MFA token for given serial
221+
* Ask user for MFA token for given MFA device
223222
*
224223
* Result is send to callback function for SDK to authorize the request
225224
*/
226-
private async tokenCodeFn(serialArn: string): Promise<string> {
225+
private async tokenCodeFn(deviceArn: string): Promise<string> {
227226
const debugFn = (msg: string, ...args: any[]) => this.ioHelper.notify(IO.DEFAULT_SDK_DEBUG.msg(format(msg, ...args)));
228-
await debugFn('Require MFA token for serial ARN', serialArn);
227+
await debugFn('Require MFA token from MFA device with ARN', deviceArn);
229228
try {
230-
const token: string = await promptly.prompt(`MFA token for ${serialArn}: `, {
231-
trim: true,
232-
default: '',
233-
});
229+
const token: string = await this.ioHelper.requestResponse(IO.CDK_SDK_I1100.req(`MFA token for ${deviceArn}`, {
230+
deviceArn,
231+
}, ''));
232+
234233
await debugFn('Successfully got MFA token from user');
235234
return token;
236235
} catch (err: any) {

packages/@aws-cdk/toolkit-lib/lib/api/io/private/message-maker.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,11 @@ export interface IoRequestMaker<T, U> extends MessageInfo {
113113
/**
114114
* Create a message for this code, with or without payload.
115115
*/
116-
req: [T] extends [AbsentData] ? (message: string) => ActionLessMessage<AbsentData> : (message: string, data: T) => ActionLessRequest<T, U>;
116+
req: [T] extends [AbsentData]
117+
? (message: string) => ActionLessMessage<AbsentData>
118+
: [U] extends [boolean]
119+
? (message: string, data: T) => ActionLessRequest<T, U>
120+
: (message: string, data: T, defaultResponse: U) => ActionLessRequest<T, U>;
117121
}
118122

119123
/**
@@ -143,3 +147,24 @@ export const confirm = <T extends object = ImpossibleType>(details: Required<Omi
143147
...details,
144148
defaultResponse: true,
145149
});
150+
151+
/**
152+
* An open ended question with a string answer, typically provided on-demand by a user.
153+
*/
154+
export function question<T>(details: CodeInfo): IoRequestMaker<T, string> {
155+
const level = 'info';
156+
const maker = (text: string, data: T, defaultResponse: string) => ({
157+
time: new Date(),
158+
level,
159+
code: details.code,
160+
message: text,
161+
data,
162+
defaultResponse,
163+
} as ActionLessRequest<T, string>);
164+
165+
return {
166+
...details,
167+
level,
168+
req: maker as any,
169+
};
170+
}

packages/@aws-cdk/toolkit-lib/lib/api/io/private/messages.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import type { StackDetailsPayload } from '../../../payloads/list';
1111
import type { CloudWatchLogEvent, CloudWatchLogMonitorControlEvent } from '../../../payloads/logs-monitor';
1212
import type { RefactorResult } from '../../../payloads/refactor';
1313
import type { StackRollbackProgress } from '../../../payloads/rollback';
14-
import type { SdkTrace } from '../../../payloads/sdk-trace';
14+
import type { MfaTokenRequest, SdkTrace } from '../../../payloads/sdk';
1515
import type { StackActivity, StackMonitoringControlEvent } from '../../../payloads/stack-activity';
1616
import type { StackSelectionDetails } from '../../../payloads/synth';
1717
import type { AssemblyData, ConfirmationRequest, ContextProviderMessageSource, Duration, ErrorPayload, StackAndAssemblyData } from '../../../payloads/types';
@@ -517,6 +517,11 @@ export const IO = {
517517
description: 'An SDK trace. SDK traces are emitted as traces to the IoHost, but contain the original SDK logging level.',
518518
interface: 'SdkTrace',
519519
}),
520+
CDK_SDK_I1100: make.question<MfaTokenRequest>({
521+
code: 'CDK_SDK_I1100',
522+
description: 'Get an MFA token for an MFA device.',
523+
interface: 'MfaTokenRequest',
524+
}),
520525
};
521526

522527
//////////////////////////////////////////////////////////////////////////////////////////

packages/@aws-cdk/toolkit-lib/lib/payloads/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ export * from './bootstrap-environment-progress';
22
export * from './deploy';
33
export * from './destroy';
44
export * from './list';
5-
export * from './sdk-trace';
5+
export * from './sdk';
66
export * from './context';
77
export * from './rollback';
88
export * from './stack-activity';

packages/@aws-cdk/toolkit-lib/lib/payloads/sdk-trace.ts renamed to packages/@aws-cdk/toolkit-lib/lib/payloads/sdk.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import type { DataRequest } from './types';
2+
13
/**
24
* An SDK logging trace.
35
*
@@ -19,3 +21,13 @@ export interface SdkTrace {
1921
*/
2022
readonly content: any;
2123
}
24+
25+
/**
26+
* Get an MFA token for an MFA device.
27+
*/
28+
export interface MfaTokenRequest extends DataRequest {
29+
/**
30+
* The ARN of the MFA device a token is required for.
31+
*/
32+
readonly deviceArn: string;
33+
}

packages/@aws-cdk/toolkit-lib/lib/payloads/types.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,18 @@ export interface ConfirmationRequest {
9999
readonly concurrency?: number;
100100
}
101101

102+
/**
103+
* A generic request for data
104+
*/
105+
export interface DataRequest {
106+
/**
107+
* An optional description of the expected response
108+
* Provides additional details on what the response can be.
109+
* This can be treated as a direct instruction to end-users when prompting for input.
110+
*/
111+
responseDescription?: string;
112+
}
113+
102114
export interface ContextProviderMessageSource {
103115
/**
104116
* The name of the context provider sending the message

packages/@aws-cdk/toolkit-lib/test/_helpers/test-io-host.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ import type { IIoHost, IoMessage, IoMessageLevel, IoRequest } from '../../lib/ap
22
import type { IoHelper } from '../../lib/api/io/private';
33
import { asIoHelper, isMessageRelevantForLevel } from '../../lib/api/io/private';
44

5+
type MessageMock = jest.Mock<void, [IoMessage<any>]>;
6+
type RequestMock<U extends any> = jest.Mock<U, [IoRequest<any, U>]>;
7+
58
/**
69
* An implementation of `IIoHost` that records messages,
710
* lets you assert on what was logged and can be spied on.
@@ -22,8 +25,8 @@ import { asIoHelper, isMessageRelevantForLevel } from '../../lib/api/io/private'
2225
*/
2326
export class TestIoHost implements IIoHost {
2427
public messages: Array<IoMessage<unknown>> = [];
25-
public readonly notifySpy: jest.Mock<any, any, any>;
26-
public readonly requestSpy: jest.Mock<any, any, any>;
28+
public readonly notifySpy: MessageMock;
29+
public readonly requestSpy: RequestMock<any>;
2730

2831
constructor(public level: IoMessageLevel = 'info') {
2932
this.notifySpy = jest.fn();
@@ -49,10 +52,11 @@ export class TestIoHost implements IIoHost {
4952
}
5053

5154
public async requestResponse<T, U>(msg: IoRequest<T, U>): Promise<U> {
55+
let spyResponse;
5256
if (isMessageRelevantForLevel(msg, this.level)) {
53-
this.requestSpy(msg);
57+
spyResponse = await this.requestSpy(msg);
5458
}
55-
return msg.defaultResponse;
59+
return spyResponse ?? msg.defaultResponse;
5660
}
5761

5862
public expectMessage(m: { containing: string; level?: IoMessageLevel }) {

packages/@aws-cdk/toolkit-lib/test/api/aws-auth/sdk-provider.test.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
import * as os from 'os';
1616
import * as cxapi from '@aws-cdk/cx-api';
1717
import * as fromEnv from '@aws-sdk/credential-provider-env';
18-
import * as promptly from 'promptly';
1918
import * as uuid from 'uuid';
2019
import type { RegisterRoleOptions, RegisterUserOptions } from './fake-sts';
2120
import { FakeSts } from './fake-sts';
@@ -335,7 +334,7 @@ describe('with intercepted network calls', () => {
335334

336335
test.each(providersForMfa)('mfa_serial in profile will ask user for token', async (metaProvider: () => Promise<SdkProvider>) => {
337336
// GIVEN
338-
const mockPrompt = jest.spyOn(promptly, 'prompt').mockResolvedValue('1234');
337+
ioHost.requestSpy.mockImplementation((msg) => msg.code === 'CDK_SDK_I1100' ? '1234' : undefined);
339338

340339
prepareCreds({
341340
fakeSts,
@@ -362,7 +361,10 @@ describe('with intercepted network calls', () => {
362361

363362
// Make sure the MFA mock was called during this test, only once
364363
// (Credentials need to remain cached)
365-
expect(mockPrompt).toHaveBeenCalledTimes(1);
364+
expect(ioHost.requestSpy).toHaveBeenCalledTimes(1);
365+
expect(ioHost.requestSpy).toHaveBeenCalledWith(expect.objectContaining({
366+
code: 'CDK_SDK_I1100',
367+
}));
366368
});
367369
});
368370

packages/aws-cdk/lib/cli/io-host/cli-io-host.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -357,10 +357,12 @@ export class CliIoHost implements IIoHost {
357357
const data: {
358358
motivation?: string;
359359
concurrency?: number;
360+
responseDescription?: string;
360361
} = msg.data ?? {};
361362

362363
const motivation = data.motivation ?? 'User input is needed';
363364
const concurrency = data.concurrency ?? 0;
365+
const responseDescription = data.responseDescription;
364366

365367
// only talk to user if STDIN is a terminal (otherwise, fail)
366368
if (!this.isTTY) {
@@ -392,8 +394,10 @@ export class CliIoHost implements IIoHost {
392394

393395
// Asking for a specific value
394396
const prompt = extractPromptInfo(msg);
395-
const answer = await promptly.prompt(`${chalk.cyan(msg.message)} (${prompt.default})`, {
397+
const desc = responseDescription ?? prompt.default;
398+
const answer = await promptly.prompt(`${chalk.cyan(msg.message)}${desc ? ` (${desc})` : ''}`, {
396399
default: prompt.default,
400+
trim: true,
397401
});
398402
return prompt.convertAnswer(answer);
399403
});

0 commit comments

Comments
 (0)