-
Notifications
You must be signed in to change notification settings - Fork 281
(feat) Add support for TOTP 2FA to O3 #1385
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: main
Are you sure you want to change the base?
Changes from all commits
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,57 @@ | ||
| import React, { useState } from 'react'; | ||
| import { TextInput, Button, InlineNotification } from '@carbon/react'; | ||
|
|
||
| const LoginWithSecret: React.FC = () => { | ||
| const [username, setUsername] = useState(''); | ||
| const [question, setQuestion] = useState(''); | ||
| const [answer, setAnswer] = useState(''); | ||
| const [error, setError] = useState<string | null>(null); | ||
|
|
||
| const handleSubmit = (e: React.FormEvent) => { | ||
| e.preventDefault(); | ||
| // TODO: Implement secret question authentication logic | ||
| setError('Not implemented yet.'); | ||
| }; | ||
|
|
||
| return ( | ||
| <form onSubmit={handleSubmit} style={{ maxWidth: 400, margin: '0 auto' }}> | ||
| <h2>Login with Secret Question</h2> | ||
|
Member
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. This should be translated. |
||
| <TextInput | ||
|
Member
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. It doesn't seem like we should need to prompt for the username here, since presumably they've already filled this in. |
||
| id="username" | ||
| labelText="Username" | ||
| value={username} | ||
| onChange={(e) => setUsername(e.target.value)} | ||
| required | ||
| /> | ||
|
Member
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. We also should not prompt the user for their Secret Question here - this is something that is stored in their user profile, not something they enter in. So ideally we would retrieve this via REST and (if needed) pass it back to the backend as a hidden input. The user should not need to enter or remember this question text. |
||
| <TextInput | ||
| id="question" | ||
| labelText="Secret Question" | ||
| value={question} | ||
| onChange={(e) => setQuestion(e.target.value)} | ||
| required | ||
| /> | ||
| <TextInput | ||
| id="answer" | ||
| labelText="Answer" | ||
|
Member
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. I would think that the label text for the answer should just be the value of the secret question, no? If we do want to have text called "Answer", then this should be translated. |
||
| type="password" | ||
| value={answer} | ||
| onChange={(e) => setAnswer(e.target.value)} | ||
| required | ||
| /> | ||
| <Button type="submit" style={{ marginTop: 16 }}> | ||
| Login | ||
|
Member
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. This should be translated and css externalized |
||
| </Button> | ||
| {error && ( | ||
| <InlineNotification | ||
| kind="error" | ||
| title="Error" | ||
| subtitle={error} | ||
| onCloseButtonClick={() => setError(null)} | ||
| style={{ marginTop: 16 }} | ||
|
Member
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. This should be translated and css externalized |
||
| /> | ||
| )} | ||
| </form> | ||
| ); | ||
| }; | ||
|
|
||
| export default LoginWithSecret; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,154 @@ | ||
| import React from 'react'; | ||
| import userEvent from '@testing-library/user-event'; | ||
| import { render, screen, waitFor } from '@testing-library/react'; | ||
| import { openmrsFetch, sessionEndpoint } from '@openmrs/esm-framework'; | ||
| import LoginWithTotp from './login-with-totp.component'; | ||
|
|
||
| const mockOpenmrsFetch = jest.mocked(openmrsFetch); | ||
|
|
||
| describe('LoginWithTotp', () => { | ||
| beforeEach(() => { | ||
| mockOpenmrsFetch.mockClear(); | ||
| }); | ||
|
|
||
| it('should render the TOTP login form', () => { | ||
| render(<LoginWithTotp />); | ||
|
|
||
| expect(screen.getByText(/MFA Code/i)).toBeInTheDocument(); | ||
| expect(screen.getByRole('button', { name: /Verify/i })).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('should handle successful TOTP verification', async () => { | ||
| const mockTotpCode = '123456'; | ||
| const mockResponse = { | ||
| data: { | ||
| authenticated: true, | ||
| sessionLocation: { uuid: 'location-uuid' }, | ||
| }, | ||
| headers: new Headers(), | ||
| ok: true, | ||
| redirected: false, | ||
| status: 200, | ||
| statusText: 'OK', | ||
| type: 'cors' as ResponseType, | ||
| url: sessionEndpoint, | ||
| body: null, | ||
| bodyUsed: false, | ||
| bytes: () => Promise.resolve(new Uint8Array(0)), | ||
| clone: () => mockResponse, | ||
| arrayBuffer: () => Promise.resolve(new ArrayBuffer(0)), | ||
| blob: () => Promise.resolve(new Blob()), | ||
| formData: () => Promise.resolve(new FormData()), | ||
| json: () => Promise.resolve(mockResponse.data), | ||
| text: () => Promise.resolve(JSON.stringify(mockResponse.data)), | ||
| }; | ||
| mockOpenmrsFetch.mockResolvedValue(mockResponse); | ||
|
|
||
| const user = userEvent.setup(); | ||
| render(<LoginWithTotp />); | ||
|
|
||
| // Enter TOTP code | ||
| await user.type(screen.getByLabelText(/MFA Code/i), mockTotpCode); | ||
|
|
||
| // Submit form | ||
| await user.click(screen.getByRole('button', { name: /Verify/i })); | ||
|
|
||
| // Verify API call | ||
| expect(mockOpenmrsFetch).toHaveBeenCalledWith( | ||
| expect.stringContaining(sessionEndpoint), | ||
| expect.objectContaining({ | ||
| method: 'POST', | ||
| body: { | ||
| redirect: '/openmrs/spa/home', | ||
| }, | ||
| }), | ||
| ); | ||
|
|
||
| // Check URL parameters | ||
| const url = mockOpenmrsFetch.mock.calls[0][0]; | ||
| expect(url).toContain(`code=${mockTotpCode}`); | ||
| }); | ||
|
|
||
| it('should handle failed TOTP verification', async () => { | ||
| const mockTotpCode = '123456'; | ||
| const mockResponse = { | ||
| data: { | ||
| authenticated: false, | ||
| }, | ||
| headers: new Headers(), | ||
| ok: true, | ||
| redirected: false, | ||
| status: 200, | ||
| statusText: 'OK', | ||
| type: 'cors' as ResponseType, | ||
| url: sessionEndpoint, | ||
| body: null, | ||
| bodyUsed: false, | ||
| bytes: () => Promise.resolve(new Uint8Array(0)), | ||
| clone: () => mockResponse, | ||
| arrayBuffer: () => Promise.resolve(new ArrayBuffer(0)), | ||
| blob: () => Promise.resolve(new Blob()), | ||
| formData: () => Promise.resolve(new FormData()), | ||
| json: () => Promise.resolve(mockResponse.data), | ||
| text: () => Promise.resolve(JSON.stringify(mockResponse.data)), | ||
| }; | ||
| mockOpenmrsFetch.mockResolvedValue(mockResponse); | ||
|
|
||
| const user = userEvent.setup(); | ||
| render(<LoginWithTotp />); | ||
|
|
||
| // Enter TOTP code | ||
| await user.type(screen.getByLabelText(/MFA Code/i), mockTotpCode); | ||
|
|
||
| // Submit form | ||
| await user.click(screen.getByRole('button', { name: /Verify/i })); | ||
|
|
||
| // Check error message | ||
| await waitFor(() => { | ||
| expect(screen.getByText(/Invalid MFA code/i)).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| // Verify input is cleared | ||
| expect(screen.getByLabelText(/MFA Code/i)).toHaveValue(''); | ||
| }); | ||
|
|
||
| it('should handle API errors', async () => { | ||
| const mockTotpCode = '123456'; | ||
| mockOpenmrsFetch.mockRejectedValue(new Error('API Error')); | ||
|
|
||
| const user = userEvent.setup(); | ||
| render(<LoginWithTotp />); | ||
|
|
||
| // Enter TOTP code | ||
| await user.type(screen.getByLabelText(/MFA Code/i), mockTotpCode); | ||
|
|
||
| // Submit form | ||
| await user.click(screen.getByRole('button', { name: /Verify/i })); | ||
|
|
||
| // Check error message | ||
| await waitFor(() => { | ||
| expect(screen.getByText(/Failed to verify MFA code/i)).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| // Verify input is cleared | ||
| expect(screen.getByLabelText(/MFA Code/i)).toHaveValue(''); | ||
| }); | ||
|
|
||
| it('should show loading state during verification', async () => { | ||
| const mockTotpCode = '123456'; | ||
| mockOpenmrsFetch.mockImplementation(() => new Promise(() => {})); // Never resolves | ||
|
|
||
| const user = userEvent.setup(); | ||
| render(<LoginWithTotp />); | ||
|
|
||
| // Enter TOTP code | ||
| await user.type(screen.getByLabelText(/MFA Code/i), mockTotpCode); | ||
|
|
||
| // Submit form | ||
| await user.click(screen.getByRole('button', { name: /Verify/i })); | ||
|
|
||
| // Check loading state | ||
| expect(screen.getByText(/Verifying/i)).toBeInTheDocument(); | ||
| expect(screen.getByRole('button')).toBeDisabled(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| import React, { useState } from 'react'; | ||
| import { TextInput, Button, InlineLoading, InlineNotification, Tile } from '@carbon/react'; | ||
| import { openmrsFetch, sessionEndpoint, ArrowRightIcon, getCoreTranslation } from '@openmrs/esm-framework'; | ||
| import { useNavigate } from 'react-router-dom'; | ||
| import { useTranslation } from 'react-i18next'; | ||
| import Logo from '../logo.component'; | ||
| import Footer from '../footer.component'; | ||
| import styles from './login.scss'; | ||
|
|
||
| const LoginWithTotp: React.FC = () => { | ||
| const [totp, setTotp] = useState(''); | ||
| const [error, setError] = useState<string | null>(null); | ||
| const [isSubmitting, setIsSubmitting] = useState(false); | ||
| const navigate = useNavigate(); | ||
| const { t } = useTranslation(); | ||
|
|
||
| const handleSubmit = async (e: React.FormEvent) => { | ||
| e.preventDefault(); | ||
| setError(null); | ||
| setIsSubmitting(true); | ||
|
|
||
| try { | ||
| const searchParams = new URLSearchParams(); | ||
| searchParams.append('code', totp); | ||
| searchParams.append('redirect', 'spa/home'); | ||
| const url = `${sessionEndpoint}?${searchParams.toString()}`; | ||
|
|
||
| const response = await openmrsFetch(url); | ||
| const session = response.data; | ||
| const authenticated = session?.authenticated; | ||
| if (authenticated) { | ||
| if (session.sessionLocation) { | ||
| setTimeout(() => navigate('/openmrs/spa/home'), 0); | ||
|
Member
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.
|
||
| } else { | ||
| setTimeout(() => navigate('/login/location'), 0); | ||
| } | ||
| } else { | ||
| setError('Invalid MFA code'); | ||
|
Member
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. This should be translated |
||
| setTotp(''); | ||
| } | ||
| } catch (error) { | ||
| setError(error instanceof Error ? error.message : 'Failed to verify MFA code'); | ||
|
Member
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. This should be translated |
||
| setTotp(''); | ||
| } finally { | ||
| setIsSubmitting(false); | ||
| } | ||
| }; | ||
|
|
||
| return ( | ||
| <div className={styles.container}> | ||
| <Tile className={styles.loginCard}> | ||
| {error && ( | ||
| <div className={styles.errorMessage}> | ||
| <InlineNotification | ||
| kind="error" | ||
| subtitle={t(error)} | ||
| title={getCoreTranslation('error')} | ||
| onClick={() => setError(null)} | ||
| /> | ||
| </div> | ||
| )} | ||
| <div className={styles.center}> | ||
| <Logo t={t} /> | ||
| </div> | ||
| <form onSubmit={handleSubmit}> | ||
| <div className={styles.inputGroup}> | ||
|
Member
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. My guess is that we may want some kind of instructions / help text here that can remind a user how to log in with TOTP. |
||
| <TextInput | ||
| id="totp" | ||
| labelText={t('mfaCode', 'MFA Code')} | ||
| value={totp} | ||
| onChange={(e) => setTotp(e.target.value)} | ||
| required | ||
| maxLength={6} | ||
| autoFocus | ||
| /> | ||
| <Button | ||
| type="submit" | ||
| className={styles.continueButton} | ||
| renderIcon={(props) => <ArrowRightIcon size={24} {...props} />} | ||
| iconDescription={t('loginButtonIconDescription', 'Log in button')} | ||
| disabled={isSubmitting} | ||
| > | ||
| {isSubmitting ? ( | ||
| <InlineLoading className={styles.loader} description={t('loggingIn', 'Logging in') + '...'} /> | ||
| ) : ( | ||
| t('login', 'Verify') | ||
| )} | ||
| </Button> | ||
| </div> | ||
| </form> | ||
| </Tile> | ||
| <Footer /> | ||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
| export default LoginWithTotp; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| import { HeaderGlobalAction } from '@carbon/react'; | ||
| import { navigate, useSession } from '@openmrs/esm-framework'; | ||
| import React from 'react'; | ||
| import { useTranslation } from 'react-i18next'; | ||
| import styles from './totp-setup-link.scss'; | ||
| import { TwoFactorAuthentication } from '@carbon/react/icons'; | ||
|
|
||
| const TotpSetupLink: React.FC = () => { | ||
| const { t } = useTranslation(); | ||
| const session = useSession(); | ||
|
|
||
| const setupTotp = () => { | ||
| navigate({ | ||
| to: `\${openmrsSpaBase}/totp-setup?returnToUrl=${window.location.pathname}`, | ||
| }); | ||
| }; | ||
|
|
||
| return ( | ||
| <HeaderGlobalAction aria-label={t('setupMfa', 'Setup 2FA')} className={styles.setupMfaButton} onClick={setupTotp}> | ||
| <TwoFactorAuthentication size={20} /> | ||
| <span className={styles.setupMfaText}>{t('setupMfa', 'Setup 2FA')}</span> | ||
| </HeaderGlobalAction> | ||
|
Member
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. I guess this is fine for now, but doesn't seem ideal: |
||
| ); | ||
| }; | ||
|
|
||
| export default TotpSetupLink; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| @use '@carbon/layout'; | ||
|
|
||
| .setupMfaButton { | ||
| width: fit-content; | ||
| background-color: transparent; | ||
| color: white; | ||
| font-size: 14px; | ||
| padding: layout.$spacing-04 !important; // this gets unset in rtl language without !important | ||
|
|
||
| &:hover { | ||
| color: white; | ||
| } | ||
| } | ||
|
|
||
| .setupMfaText { | ||
| padding-inline-start: layout.$spacing-03; | ||
| } |
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.
As much as possible, we prefer to keep styles in
scssfiles and pass in a className, rather than using thestyleattribute.