-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Add Login UI MFA Flow #142
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feat/mfa---usercontroller-mfa-integration-device-trust-cookie-management-audit-wiring-and-2fa-rate-limiting---cu-86ba2zc6p
Are you sure you want to change the base?
Changes from all commits
3481ac9
7ef8b52
603cae8
2c46548
ea8c4ed
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| import React from "react"; | ||
| import Grid from "@material-ui/core/Grid"; | ||
| import Button from "@material-ui/core/Button"; | ||
| import styles from "../login.module.scss"; | ||
|
|
||
| const EmailErrorActions = ({ | ||
| emitOtpAction, | ||
| createAccountAction, | ||
| onValidateEmail, | ||
| disableInput, | ||
| }) => { | ||
| return ( | ||
| <Grid container spacing={1}> | ||
| <Grid | ||
| container | ||
| item | ||
| spacing={1} | ||
| justifyContent="center" | ||
| alignItems="center" | ||
| > | ||
| <Grid item> | ||
| <Button | ||
| variant="contained" | ||
| onClick={emitOtpAction} | ||
| type="button" | ||
| className={styles.secondary_btn} | ||
| color="primary" | ||
| > | ||
| Email me a one time use code | ||
| </Button> | ||
| </Grid> | ||
| <Grid item> | ||
| <Button | ||
| variant="contained" | ||
| href={createAccountAction} | ||
| type="button" | ||
| target="_self" | ||
| className={styles.secondary_btn} | ||
| color="primary" | ||
| > | ||
| Register and set a password | ||
| </Button> | ||
| </Grid> | ||
| <Grid item> | ||
| <Button | ||
| variant="text" | ||
| onClick={onValidateEmail} | ||
| disabled={disableInput} | ||
| className={styles.secondary_btn} | ||
| color="primary" | ||
| > | ||
| Adjust email above and try again | ||
| </Button> | ||
| </Grid> | ||
| </Grid> | ||
| </Grid> | ||
| ); | ||
| }; | ||
|
|
||
| export default EmailErrorActions; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| import React from "react"; | ||
| import Paper from "@material-ui/core/Paper"; | ||
| import TextField from "@material-ui/core/TextField"; | ||
| import Button from "@material-ui/core/Button"; | ||
| import styles from "../login.module.scss"; | ||
| import HTMLRender from "../../shared/HTMLRender"; | ||
|
|
||
| const EmailInputForm = ({ | ||
| value, | ||
| onValidateEmail, | ||
| onHandleUserNameChange, | ||
| disableInput, | ||
| emailError, | ||
| }) => { | ||
| return ( | ||
| <> | ||
| <Paper | ||
| elevation={0} | ||
| component="form" | ||
| target="_self" | ||
| className={styles.paper_root} | ||
| onSubmit={onValidateEmail} | ||
| > | ||
| <TextField | ||
| id="email" | ||
| name="email" | ||
| value={value} | ||
| autoComplete="email" | ||
| variant="outlined" | ||
| margin="normal" | ||
| required | ||
| fullWidth | ||
| disabled={disableInput} | ||
| label="Email Address" | ||
| autoFocus={true} | ||
| onChange={onHandleUserNameChange} | ||
| error={emailError != ""} | ||
| /> | ||
| {emailError == "" && ( | ||
| <Button | ||
| variant="contained" | ||
| color="primary" | ||
| title="Continue" | ||
| className={styles.apply_button} | ||
| disabled={disableInput} | ||
| onClick={onValidateEmail} | ||
| > | ||
| > | ||
| </Button> | ||
| )} | ||
| </Paper> | ||
| {emailError != "" && ( | ||
| <HTMLRender component="p" className={styles.error_label}> | ||
| {emailError} | ||
| </HTMLRender> | ||
| )} | ||
| </> | ||
| ); | ||
| }; | ||
|
|
||
| export default EmailInputForm; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| import React from "react"; | ||
| import Grid from "@material-ui/core/Grid"; | ||
| import Button from "@material-ui/core/Button"; | ||
| import Link from "@material-ui/core/Link"; | ||
| import styles from "../login.module.scss"; | ||
|
|
||
| const ExistingAccountActions = ({ | ||
| emitOtpAction, | ||
| forgotPasswordAction, | ||
| userName, | ||
| disableInput, | ||
| }) => { | ||
| let forgotPasswordActionHref = forgotPasswordAction; | ||
|
|
||
| if (userName) { | ||
| forgotPasswordActionHref = `${forgotPasswordAction}?email=${encodeURIComponent(userName)}`; | ||
| } | ||
|
|
||
| return ( | ||
| <Grid container spacing={1} style={{ marginTop: "30px" }}> | ||
| <Grid item xs={12}> | ||
| <Button | ||
| variant="contained" | ||
| onClick={emitOtpAction} | ||
| type="button" | ||
| disabled={disableInput} | ||
| className={styles.secondary_btn} | ||
| color="primary" | ||
| > | ||
| Sign in by emailing me a single-use code | ||
| </Button> | ||
| </Grid> | ||
| <Grid item xs={12}> | ||
| <Link | ||
| disabled={disableInput} | ||
| href={forgotPasswordActionHref} | ||
| target="_self" | ||
|
Comment on lines
+34
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor 🧩 Analysis chain🏁 Script executed: # First, let's find and check the package.json to determine MUI version
cd /
fd -t f "package.json" -x head -50 {} \; 2>/dev/null | head -100Repository: OpenStackweb/openstackid Length of output: 2974 🏁 Script executed: # Find the specific file mentioned in the review
fd -t f "existing_account_actions.js"Repository: OpenStackweb/openstackid Length of output: 220 🏁 Script executed: # Check for package.json at repo root
cat package.json 2>/dev/null | grep -A5 -B5 "`@material-ui`"Repository: OpenStackweb/openstackid Length of output: 698 🏁 Script executed: # Read the existing_account_actions.js file to see the full context
cat -n resources/js/login/components/existing_account_actions.jsRepository: OpenStackweb/openstackid Length of output: 1678 🏁 Script executed: # Search for other usages of Link component in the codebase to understand pattern
rg "from.*`@material-ui/core`" resources/js/login/components/existing_account_actions.js -A 10Repository: OpenStackweb/openstackid Length of output: 506 🏁 Script executed: # Check what disableInput is and how it's defined
rg "disableInput" resources/js/login/components/existing_account_actions.js -B 2 -A 2Repository: OpenStackweb/openstackid Length of output: 585 🌐 Web query:
💡 Result: In Material-UI (MUI) v4, the Link component does not have a built-in "disabled" state or default styling for a disabled appearance [1][2]. While you can pass a disabled prop to the Link component—as it is built on top of the Typography component and supports spreading arbitrary props—the Link component itself does not natively interpret or style this prop [3][1][4]. Consequently, simply adding the disabled prop to a Link will not prevent user interaction, nor will it change the component's appearance [1][2]. To achieve a disabled effect in MUI v4, you must implement custom logic and styling [1]: 1. Accessibility and Interaction: If you need to stop interaction, you should manually disable the link's functionality (e.g., by preventing the onClick handler) and manage its focusability, such as by setting tabIndex={-1} [1]. For accessibility, it is recommended to add the aria-disabled="true" attribute [1]. 2. Styling: Since there is no default "disabled" class, you need to apply custom CSS to visually indicate that the link is disabled (e.g., changing the text color, setting cursor to default or not-allowed, and disabling text-decoration) [1]. 3. Using component="button": If you change the underlying element to a button using component="button", the standard HTML disabled attribute will function; however, you will still need to define custom styles to achieve the desired disabled visual appearance, as MUI v4 does not provide default disabled styling for the Link component even when rendered as a button [1][2]. Citations:
🌐 Web query:
💡 Result: In MUI v4.11, setting the disabled prop on MuiLink only prevents navigation when the rendered root element is a button (component="button"); otherwise it won’t reliably stop link navigation because the underlying element is an . What the code shows (v4.11.4): - MuiLink defaults component="a" (anchor). [1] - The Link implementation applies a “button” class only when component === 'button'. [1] So for disabled navigation you must do one of these: 1) Render it as a button and use disabled Use component="button". This makes disabled behave like a real disabled button: the browser won’t trigger navigation for href-based routing and MUI applies disabled semantics/styles for the button variant. - Example pattern: ... [2] 2) Keep it as an anchor () and block navigation yourself If you must use href with the default component="a", disabled does not inherently stop the default anchor click. A workaround is to intercept the click and call preventDefault when you want “disabled navigation”. (This is essentially the pattern described as a working workaround.) [2] 3) For accessibility If you implement “disabled-like” behavior via preventDefault, also add aria-disabled and remove from tab order as needed (as suggested in the workaround). [2] Practical takeaway - If you’re using MuiLink for navigation and want a true “disabled” state in MUI v4.11: set component="button" (and provide onClick only if needed) rather than relying on disabled while keeping it as an . [2][1] - If you keep href and the element: you need an onClick handler to preventDefault; disabled alone won’t be sufficient. [2] Citations:
Passing Suggested fix <Grid item xs={12}>
<Link
- disabled={disableInput}
href={forgotPasswordActionHref}
+ onClick={disableInput ? (ev) => ev.preventDefault() : undefined}
+ aria-disabled={disableInput}
+ className={disableInput ? styles.disabled_link : undefined}
target="_self"
variant="body2"
>
Reset your password
</Link>
</Grid>🤖 Prompt for AI Agents |
||
| variant="body2" | ||
| > | ||
| Reset your password | ||
| </Link> | ||
| </Grid> | ||
| </Grid> | ||
| ); | ||
| }; | ||
|
|
||
| export default ExistingAccountActions; | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,77 @@ | ||||||||||||||||||||
| import React, { useMemo } from "react"; | ||||||||||||||||||||
| import Link from "@material-ui/core/Link"; | ||||||||||||||||||||
| import styles from "../login.module.scss"; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const HelpLinks = ({ | ||||||||||||||||||||
| userName, | ||||||||||||||||||||
| showEmitOtpAction, | ||||||||||||||||||||
| forgotPasswordAction, | ||||||||||||||||||||
| showForgotPasswordAction, | ||||||||||||||||||||
| showVerifyEmailAction, | ||||||||||||||||||||
| verifyEmailAction, | ||||||||||||||||||||
| showHelpAction, | ||||||||||||||||||||
| helpAction, | ||||||||||||||||||||
| appName, | ||||||||||||||||||||
| emitOtpAction, | ||||||||||||||||||||
| }) => { | ||||||||||||||||||||
| const actions = useMemo(() => { | ||||||||||||||||||||
| let forgotPasswordActionHref = forgotPasswordAction; | ||||||||||||||||||||
| if (userName) { | ||||||||||||||||||||
| forgotPasswordActionHref = `${forgotPasswordAction}?email=${encodeURIComponent(userName)}`; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
Comment on lines
+18
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Handle existing query params when appending Current construction always uses Suggested fix let forgotPasswordActionHref = forgotPasswordAction;
if (userName) {
- forgotPasswordActionHref = `${forgotPasswordAction}?email=${encodeURIComponent(userName)}`;
+ const separator = forgotPasswordAction.includes("?") ? "&" : "?";
+ forgotPasswordActionHref = `${forgotPasswordAction}${separator}email=${encodeURIComponent(userName)}`;
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||
|
|
||||||||||||||||||||
| return [ | ||||||||||||||||||||
| { | ||||||||||||||||||||
| show: showEmitOtpAction, | ||||||||||||||||||||
| href: "#", | ||||||||||||||||||||
| onClick: emitOtpAction, | ||||||||||||||||||||
| label: "Get A Single-use Code emailed to you", | ||||||||||||||||||||
| }, | ||||||||||||||||||||
| { | ||||||||||||||||||||
| show: showForgotPasswordAction, | ||||||||||||||||||||
| href: forgotPasswordActionHref, | ||||||||||||||||||||
| label: "Reset your password", | ||||||||||||||||||||
| }, | ||||||||||||||||||||
| { | ||||||||||||||||||||
| show: showVerifyEmailAction, | ||||||||||||||||||||
| href: verifyEmailAction, | ||||||||||||||||||||
| label: `Verify ${appName}`, | ||||||||||||||||||||
| }, | ||||||||||||||||||||
| { | ||||||||||||||||||||
| show: showHelpAction, | ||||||||||||||||||||
| href: helpAction, | ||||||||||||||||||||
| label: "Having trouble?", | ||||||||||||||||||||
| }, | ||||||||||||||||||||
| ].filter((action) => action.show); | ||||||||||||||||||||
| }, [ | ||||||||||||||||||||
| showEmitOtpAction, | ||||||||||||||||||||
| showForgotPasswordAction, | ||||||||||||||||||||
| showVerifyEmailAction, | ||||||||||||||||||||
| showHelpAction, | ||||||||||||||||||||
| userName, | ||||||||||||||||||||
| forgotPasswordAction, | ||||||||||||||||||||
| verifyEmailAction, | ||||||||||||||||||||
| helpAction, | ||||||||||||||||||||
| appName, | ||||||||||||||||||||
| emitOtpAction, | ||||||||||||||||||||
| ]); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| return ( | ||||||||||||||||||||
| <> | ||||||||||||||||||||
| <hr className={styles.separator} /> | ||||||||||||||||||||
| {actions.map((action, index) => ( | ||||||||||||||||||||
| <Link | ||||||||||||||||||||
| key={index} | ||||||||||||||||||||
| href={action.href} | ||||||||||||||||||||
| onClick={action.onClick} | ||||||||||||||||||||
| variant="body2" | ||||||||||||||||||||
| target="_self" | ||||||||||||||||||||
| > | ||||||||||||||||||||
| {action.label} | ||||||||||||||||||||
| </Link> | ||||||||||||||||||||
| ))} | ||||||||||||||||||||
| </> | ||||||||||||||||||||
| ); | ||||||||||||||||||||
| }; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| export default HelpLinks; | ||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| import React from "react"; | ||
| import Link from "@material-ui/core/Link"; | ||
| import styles from "../login.module.scss"; | ||
|
|
||
| const OTPHelpLinks = ({ emitOtpAction }) => { | ||
| return ( | ||
| <> | ||
| <hr className={styles.separator} /> | ||
| <p className={styles.otp_p}>Didn't receive it ?</p> | ||
| <p className={styles.otp_p}> | ||
| Check your spam folder or{" "} | ||
| <Link href="#" onClick={emitOtpAction} variant="body2" target="_self"> | ||
| resend email. | ||
| </Link> | ||
| </p> | ||
| </> | ||
| ); | ||
| }; | ||
|
|
||
| export default OTPHelpLinks; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Disable all recovery actions while input is locked.
Two buttons in this block ignore
disableInput, so users can still trigger actions while the UI is in a disabled state.Suggested fix
<Grid item> <Button variant="contained" onClick={emitOtpAction} + disabled={disableInput} type="button" className={styles.secondary_btn} color="primary" > Email me a one time use code </Button> </Grid> <Grid item> <Button variant="contained" href={createAccountAction} + disabled={disableInput} type="button" target="_self" className={styles.secondary_btn} color="primary" > Register and set a password </Button> </Grid>📝 Committable suggestion
🤖 Prompt for AI Agents