Skip to content
This repository was archived by the owner on Mar 19, 2025. It is now read-only.

Commit b7b2156

Browse files
author
Damion Vega
authored
Merge pull request #104 from userfront/feature/dev-1046-bug-in-mfa-setup
Do not send user email when setting up TOTP as 2nd factor
2 parents a04deae + 42226a4 commit b7b2156

8 files changed

Lines changed: 272 additions & 51 deletions

File tree

README.md

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ Userfront.init("myTenantId");
9393
| `<PasswordResetForm />` | `<password-reset-form></password-reset-form>` |
9494
| `<LogoutButton />` | `<logout-button></logout-button>` |
9595

96-
Note: when using them in plain HTML, Web Components are not self-closing and must have the full closing tag.
96+
**Note**: when using them in plain HTML, Web Components are not self-closing and must have the full closing tag.
9797
When using in Vue, they can be written in self-closing form: `<signup-form />`.
9898

9999
The Vue components are `<kebab-case>` because they are Web Components under the hood, and Web Components are required to be in kebab-case.
@@ -104,13 +104,23 @@ In React, props are `camelCase`. In Vue and Web Components, props are `kebab-cas
104104

105105
All props are optional.
106106

107-
- `tenantId` / `tenant-id`: your tenant ID, from the Userfront dashboard. If you call `Userfront.init("yourTenantId")` before using the components, it's not necessary to provide this prop.
108-
- `compact`: if `true` and username/password is an allowed factor in your tenant's [authentication flow](https://userfront.com/dashboard/authentication), show a "Password" button. If `false`, show the username and password fields directly in the form's sign-on method selection view.
107+
- `tenantId` / `tenant-id`: your workspace ID
108+
- This prop is not necessary if you call `Userfront.init("workspace_id")` before using the components.
109+
- Your workspace ID can be found on the [**Overview** page in your Userfront dashboard](https://userfront.com/dashboard).
110+
- `compact`:
109111
- Default: `false`
110-
- `redirect`: controls if and where the form should redirect **after** sign-on. If `false`, the form does not redirect. If set to a path, redirects to that path. If empty, redirects [as configured in your Userfront dashboard](https://userfront.com/dashboard/paths).
112+
- `true`: hide the email & password inputs in favor of a "Username and password" button. Clicking this button will display the necessary inputs.
113+
- **Note**: The **Password** factor must be an enabled factor in your workspace's authentication flow configured on the [**Authentication** page in your Userfront dashboard](https://userfront.com/dashboard/authentication).
114+
- `false`: show the username and password fields directly in the form's sign-on method selection view.
115+
- `redirect`: controls if and where the form should redirect **after** sign-on.
111116
- Default: `undefined`
112-
- `redirectOnLoadIfLoggedIn` / `redirect-on-load-if-logged-in`: if `true` and the user is already logged in when they load the form, redirects per the `redirect` parameter. If `false`, do not redirect if the user is already logged in when they load the form.
117+
- `false`: the form does not redirect.
118+
- If set to a path, redirect to that path.
119+
- If empty, redirect to the path configured on the [**Paths & routing** page in your Userfront dashboard](https://userfront.com/dashboard/paths).
120+
- `redirectOnLoadIfLoggedIn` / `redirect-on-load-if-logged-in`:
113121
- Default: `false`
122+
- `true`: redirects per the `redirect` parameter if the user is already logged in when the form is loaded.
123+
- `false`: do not redirect if the user is already logged in when the form is loaded.
114124

115125
## Development
116126

@@ -129,24 +139,16 @@ The repo is configured as an npm workspace to enable sharing of libraries and dy
129139

130140
1. Clone this repo.
131141
2. Install:
132-
133-
- `npm install`
134-
135-
4. Run dev servers:
136-
137-
- `npm run dev`
138-
139-
This will run the live dev servers for both the package and the site.
140-
141-
5. Run unit test:
142-
143-
- `npm run test`
144-
145-
6. Run Storybook:
146-
147-
- `npm run storybook -w package`
148-
- Find the link to the local Storybook server in the output.
149-
- Storybook should hot reload on changes to the package. Each UI state has its own component, so changes should show immediately and shouldn't require reloading the page.
142+
- `npm install`
143+
3. Run dev servers:
144+
- `npm run dev`
145+
- This will run the live dev servers for both the package and the site.
146+
4. Run unit test:
147+
- `npm run test`
148+
5. Run Storybook:
149+
- `npm run storybook -w package`
150+
- Find the link to the local Storybook server in the output.
151+
- Storybook should hot reload on changes to the package. Each UI state has its own component, so changes should show immediately and shouldn't require reloading the page.
150152

151153
### Architecture
152154

package-lock.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package/package-lock.json

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

package/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@userfront/toolkit",
3-
"version": "1.1.0-alpha.2",
3+
"version": "1.1.0-alpha.3",
44
"description": "Bindings and components for authentication with Userfront with React, Vue, other frameworks, and plain JS + HTML",
55
"type": "module",
66
"directories": {

package/src/models/views/setUpTotp.ts

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { callUserfront } from "../../services/userfront";
2+
import { hasValue } from "../config/utils";
23
import {
34
AuthContext,
45
AuthMachineConfig,
@@ -51,19 +52,32 @@ const setUpTotpConfig: AuthMachineConfig = {
5152
entry: "clearError",
5253
invoke: {
5354
// Set the code and call the API method
54-
src: (context: AuthContext<any>, event: AuthMachineEvent) =>
55-
callUserfront({
55+
src: (context: AuthContext<any>, event: AuthMachineEvent) => {
56+
const arg: Record<string, any> = {
57+
method: "totp",
58+
};
59+
60+
if (hasValue((<TotpCodeSubmitEvent>event).totpCode)) {
61+
arg.totpCode = (<TotpCodeSubmitEvent>event).totpCode;
62+
}
63+
64+
// API only requires email/emailOrUsername when logging in via first factor
65+
if (!context.isSecondFactor) {
66+
if (hasValue(context.user.email)) {
67+
arg.email = context.user.email;
68+
} else if (hasValue(context.user.emailOrUsername)) {
69+
arg.emailOrUsername = context.user.emailOrUsername;
70+
}
71+
72+
arg.redirect = false;
73+
}
74+
75+
return callUserfront({
5676
// Should ALWAYS be Userfront.login here
5777
method: "login",
58-
args: [
59-
{
60-
method: "totp",
61-
totpCode: (<TotpCodeSubmitEvent>event).totpCode,
62-
email: context.user.email,
63-
redirect: false,
64-
},
65-
],
66-
}),
78+
args: [arg],
79+
});
80+
},
6781
// On error, show the error message and return to the form
6882
onError: {
6983
actions: "setErrorFromApiError",
@@ -98,8 +112,7 @@ const setUpTotpConfig: AuthMachineConfig = {
98112
],
99113
},
100114
},
101-
// Show an error message only - if there's a problem getting
102-
// the QR code.
115+
// Show an error message — only if there's a problem getting the QR code
103116
showErrorMessage: {
104117
on: {
105118
retry: "getQrCode",

package/src/views/LogInWithPassword.jsx

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,20 +33,14 @@ const LogInWithPassword = ({ onEvent, allowBack, error }) => {
3333
});
3434
}
3535
};
36+
3637
return (
3738
<form onSubmit={handleSubmit} className="userfront-form">
3839
<div className="userfront-form-row">
3940
<Input.EmailOrUsername showError={emailOrUsernameError} />
4041
</div>
4142
<div className="userfront-form-row">
42-
<Input.Password showError={passwordError} />
43-
<span
44-
className="userfront-secondary-text"
45-
id="userfront-password-rules"
46-
>
47-
At least 16 characters OR at least 8 characters including a number and
48-
a letter.
49-
</span>
43+
<Input.Password label="Password" showError={passwordError} />
5044
</div>
5145
<ErrorMessage error={error} />
5246
<div className="userfront-button-row">
Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,211 @@
1+
import { describe, it, expect } from "vitest";
2+
import setUpTotpConfig from "../../../../src/models/views/setUpTotp";
3+
import { createMachine, interpret } from "xstate";
4+
import { createTestMachine, createTestModel } from "@xstate/test";
5+
import * as actions from "../../../../src/models/config/actions";
6+
import * as guards from "../../../../src/models/config/guards";
7+
import { useMockUserfront, addGlobalStates } from "../../../utils";
8+
import { defaultAuthContext } from "../../../../src/models/forms/universal";
9+
10+
const machineOptions = {
11+
actions,
12+
guards,
13+
};
14+
15+
const setUpTotpCodeMachine = createMachine(
16+
addGlobalStates(setUpTotpConfig),
17+
<any>machineOptions
18+
).withContext({
19+
config: defaultAuthContext.config,
20+
user: {
21+
email: "",
22+
},
23+
action: "setup",
24+
isSecondFactor: true,
25+
activeFactor: {
26+
channel: "authenticator",
27+
strategy: "totp",
28+
isConfiguredByUser: true,
29+
},
30+
allowBack: true,
31+
});
32+
33+
const testMachine = createTestMachine({
34+
initial: "gettingQrCode",
35+
states: {
36+
gettingQrCode: {
37+
on: {
38+
succeedGettingQrCode: "showingQrCode",
39+
},
40+
},
41+
showingQrCode: {
42+
on: {
43+
submitQrCode: "confirmingQrCode",
44+
back: "returnedToFactorSelection",
45+
},
46+
},
47+
showingQrCodeWithError: {
48+
on: {
49+
submitQrCode: "confirmingQrCode",
50+
back: "returnedToFactorSelection",
51+
},
52+
},
53+
confirmingQrCode: {
54+
on: {
55+
failConfirmingCode: "showingQrCodeWithError",
56+
succeedConfirmingCode: "showingBackupCodes",
57+
},
58+
},
59+
showingBackupCodes: {
60+
on: {
61+
finish: "showingComplete",
62+
},
63+
},
64+
showingComplete: {},
65+
returnedToFactorSelection: {},
66+
},
67+
});
68+
69+
const testModel = createTestModel(testMachine);
70+
71+
/**
72+
* Tests for setting up TOTP as a second factor to resolve reported bug:
73+
* https://linear.app/userfront/issue/DEV-1046/bug-in-mfa-setup
74+
*
75+
* NOTE: Mock does not detect calls for store.user.getTotp(), so we are not
76+
* able to resolve/reject like we do with the other `Userfront` methods.
77+
* The userfront mock has been modified below to resolve the API's response.
78+
*/
79+
describe("model-based: models/mfa/setUpTotpCode", () => {
80+
testModel.getPaths().forEach((path) => {
81+
it(path.description, async () => {
82+
const qrCode = "data:image/png;base64,testqrcode==";
83+
const backupCodes = [
84+
"60bb6-9393a",
85+
"1b8ef-e3e4b",
86+
"1488f-7cd2e",
87+
"3169e-fa7e3",
88+
];
89+
// Mock must be initialized before starting machine
90+
const mockUserfront = useMockUserfront({
91+
user: {
92+
getTotp: () => {
93+
return new Promise((resolve) => {
94+
resolve({
95+
totpSecret: "testtotpsecret",
96+
qrCode,
97+
backupCodes,
98+
});
99+
});
100+
},
101+
},
102+
});
103+
104+
const setUpTotpCodeService = interpret(setUpTotpCodeMachine);
105+
setUpTotpCodeService.start();
106+
107+
const expected = {
108+
totpCode: "123456",
109+
// Requests
110+
getQrCodeReq: {
111+
success: {
112+
qrCode,
113+
backupCodes,
114+
},
115+
},
116+
confirmQrCodeReq: {
117+
error: {
118+
message: "Confirm code error",
119+
error: {
120+
type: "ConfirmCodeError",
121+
},
122+
},
123+
success: {
124+
isMfaRequired: false,
125+
},
126+
},
127+
};
128+
129+
await path.test({
130+
states: {
131+
gettingQrCode: () => {
132+
const state = setUpTotpCodeService.getSnapshot();
133+
expect(state.value).toEqual("getQrCode");
134+
expect(state.context.error).toBeFalsy();
135+
},
136+
showingQrCode: () => {
137+
const state = setUpTotpCodeService.getSnapshot();
138+
expect(state.value).toEqual("showQrCode");
139+
expect(state.context.error).toBeFalsy();
140+
expect(state.context.view).toEqual({
141+
qrCode: expected.getQrCodeReq.success.qrCode,
142+
backupCodes: expected.getQrCodeReq.success.backupCodes,
143+
});
144+
},
145+
confirmingQrCode: () => {
146+
const state = setUpTotpCodeService.getSnapshot();
147+
expect(state.value).toEqual("confirmTotpCode");
148+
expect(state.context.error).toBeFalsy();
149+
expect(mockUserfront.lastCall?.method).toEqual("login");
150+
151+
const arg = mockUserfront.lastCall?.args[0];
152+
expect(arg).not.toHaveProperty("email");
153+
expect(arg.method).toEqual("totp");
154+
expect(arg.totpCode).toEqual(expected.totpCode);
155+
},
156+
showingQrCodeWithError: () => {
157+
const state = setUpTotpCodeService.getSnapshot();
158+
expect(state.value).toEqual("showQrCode");
159+
expect(state.context.error).toEqual(
160+
expected.confirmQrCodeReq.error
161+
);
162+
},
163+
showingBackupCodes: () => {
164+
const state = setUpTotpCodeService.getSnapshot();
165+
expect(state.value).toEqual("showBackupCodes");
166+
expect(state.context.error).toBeFalsy();
167+
},
168+
showingComplete: () => {
169+
const state = setUpTotpCodeService.getSnapshot();
170+
expect(state.value).toEqual("showBackupCodes");
171+
expect(state.context.error).toBeFalsy();
172+
},
173+
returnedToFactorSelection: () => {
174+
const state = setUpTotpCodeService.getSnapshot();
175+
expect(state.value).toEqual("backToFactors");
176+
expect(state.context.error).toBeFalsy();
177+
},
178+
},
179+
events: {
180+
succeedGettingQrCode: async () => {
181+
// Mock does not detect call for store.user.getTotp(); do not
182+
// resolve the getQrCode response here
183+
},
184+
submitQrCode: () => {
185+
const { totpCode } = expected;
186+
setUpTotpCodeService.send("submit", { totpCode });
187+
},
188+
back: () => {
189+
setUpTotpCodeService.send("back");
190+
},
191+
failConfirmingCode: async () => {
192+
try {
193+
await mockUserfront.reject(expected.confirmQrCodeReq.error);
194+
} catch (error) {
195+
await Promise.resolve();
196+
return;
197+
}
198+
},
199+
succeedConfirmingCode: async () => {
200+
try {
201+
await mockUserfront.resolve(expected.confirmQrCodeReq.success);
202+
} catch (error) {
203+
await Promise.resolve();
204+
return;
205+
}
206+
},
207+
},
208+
});
209+
});
210+
});
211+
});

0 commit comments

Comments
 (0)