Skip to content

Commit b2b334e

Browse files
authored
chore: fast fail if loginWith phone is enabled but sms mfa is not (#1523)
1 parent 360d975 commit b2b334e

File tree

3 files changed

+72
-11
lines changed

3 files changed

+72
-11
lines changed

.changeset/wise-dragons-marry.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@aws-amplify/auth-construct': patch
3+
---
4+
5+
chore: fast fail if loginWith phone is enabled but sms mfa is not

packages/auth-construct/src/construct.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -515,6 +515,41 @@ void describe('Auth construct', () => {
515515
);
516516
});
517517

518+
void it('throws error if sms MFA is not enabled with phone login', () => {
519+
assert.throws(
520+
() =>
521+
new AmplifyAuth(new Stack(new App()), 'test', {
522+
loginWith: {
523+
phone: true,
524+
},
525+
multifactor: {
526+
mode: 'OPTIONAL',
527+
totp: true,
528+
},
529+
}),
530+
{
531+
message:
532+
'Invalid MFA settings. SMS must be enabled in multiFactor if loginWith phone is enabled',
533+
}
534+
);
535+
assert.throws(
536+
() =>
537+
new AmplifyAuth(new Stack(new App()), 'test', {
538+
loginWith: {
539+
phone: true,
540+
},
541+
multifactor: {
542+
mode: 'REQUIRED',
543+
totp: true,
544+
},
545+
}),
546+
{
547+
message:
548+
'Invalid MFA settings. SMS must be enabled in multiFactor if loginWith phone is enabled',
549+
}
550+
);
551+
});
552+
518553
void it('requires email attribute if email is enabled', () => {
519554
const app = new App();
520555
const stack = new Stack(app);

packages/auth-construct/src/construct.ts

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
CfnUserPoolClient,
1212
CfnUserPoolGroup,
1313
Mfa,
14+
MfaSecondFactor,
1415
OAuthScope,
1516
OidcAttributeRequestMethod,
1617
ProviderAttribute,
@@ -375,6 +376,16 @@ export class AmplifyAuth
375376
smsMessage: smsMessage,
376377
};
377378
}
379+
const mfaType = this.getMFAType(props.multifactor);
380+
const mfaMode = this.getMFAMode(props.multifactor);
381+
382+
// If phone login is enabled along with MFA, cognito requires that mfa SMS type to be enabled.
383+
if (phoneEnabled && mfaMode && mfaMode !== 'OFF' && !mfaType?.sms) {
384+
throw Error(
385+
'Invalid MFA settings. SMS must be enabled in multiFactor if loginWith phone is enabled'
386+
);
387+
}
388+
378389
const userPoolProps: UserPoolProps = {
379390
signInCaseSensitive: DEFAULTS.SIGN_IN_CASE_SENSITIVE,
380391
signInAliases: {
@@ -397,16 +408,9 @@ export class AmplifyAuth
397408
...(props.userAttributes ? props.userAttributes : {}),
398409
},
399410
selfSignUpEnabled: DEFAULTS.ALLOW_SELF_SIGN_UP,
400-
mfa: this.getMFAMode(props.multifactor),
411+
mfa: mfaMode,
401412
mfaMessage: this.getMFAMessage(props.multifactor),
402-
mfaSecondFactor:
403-
typeof props.multifactor === 'object' &&
404-
props.multifactor.mode !== 'OFF'
405-
? {
406-
sms: props.multifactor.sms ? true : false,
407-
otp: props.multifactor.totp ? true : false,
408-
}
409-
: undefined,
413+
mfaSecondFactor: mfaType,
410414
accountRecovery: this.getAccountRecoverySetting(
411415
emailEnabled,
412416
phoneEnabled,
@@ -542,10 +546,10 @@ export class AmplifyAuth
542546
};
543547

544548
/**
545-
* Convert user friendly Mfa mode to cognito Mfa type.
549+
* Convert user friendly Mfa mode to cognito Mfa Mode.
546550
* This eliminates the need for users to import cognito.Mfa.
547551
* @param mfa - MFA settings
548-
* @returns cognito MFA enforcement type
552+
* @returns cognito MFA enforcement mode
549553
*/
550554
private getMFAMode = (mfa: AuthProps['multifactor']): Mfa | undefined => {
551555
if (mfa) {
@@ -561,6 +565,23 @@ export class AmplifyAuth
561565
return undefined;
562566
};
563567

568+
/**
569+
* Convert user friendly Mfa type to cognito Mfa type.
570+
* This eliminates the need for users to import cognito.Mfa.
571+
* @param mfa - MFA settings
572+
* @returns cognito MFA type (sms or totp)
573+
*/
574+
private getMFAType = (
575+
mfa: AuthProps['multifactor']
576+
): MfaSecondFactor | undefined => {
577+
return typeof mfa === 'object' && mfa.mode !== 'OFF'
578+
? {
579+
sms: mfa.sms ? true : false,
580+
otp: mfa.totp ? true : false,
581+
}
582+
: undefined;
583+
};
584+
564585
/**
565586
* Convert user friendly account recovery method to cognito AccountRecover enum.
566587
* This eliminates the need for users to import cognito.AccountRecovery.

0 commit comments

Comments
 (0)