Skip to content

Commit

Permalink
Reuse useReAuthenticate in useMfa.
Browse files Browse the repository at this point in the history
  • Loading branch information
Joerger committed Dec 6, 2024
1 parent 7b97b6b commit 5911b85
Show file tree
Hide file tree
Showing 8 changed files with 106 additions and 201 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export default function DocumentKubeExec({ doc, visible }: Props) {
useEffect(() => {
// when switching tabs or closing tabs, focus on visible terminal
terminalRef.current?.focus();
}, [visible, mfa.requested]);
}, [visible, mfa.challenge]);
const theme = useTheme();

const terminal = (
Expand All @@ -63,7 +63,7 @@ export default function DocumentKubeExec({ doc, visible }: Props) {
<Indicator />
</Box>
)}
{mfa.requested && <AuthnDialog mfa={mfa} onCancel={closeDocument} />}
{mfa.challenge && <AuthnDialog mfa={mfa} onCancel={closeDocument} />}

{status === 'waiting-for-exec-data' && (
<KubeExecData onExec={sendKubeExecData} onClose={closeDocument} />
Expand Down
6 changes: 3 additions & 3 deletions web/packages/teleport/src/Console/DocumentSsh/DocumentSsh.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ function DocumentSsh({ doc, visible }: PropTypes) {
getDownloader,
getUploader,
fileTransferRequests,
} = useFileTransfer(tty, session, doc, mfa.addMfaToScpUrls);
} = useFileTransfer(tty, session, doc, mfa.mfaRequired);
const theme = useTheme();

function handleCloseFileTransfer() {
Expand All @@ -72,7 +72,7 @@ function DocumentSsh({ doc, visible }: PropTypes) {
useEffect(() => {
// when switching tabs or closing tabs, focus on visible terminal
terminalRef.current?.focus();
}, [visible, mfa.requested]);
}, [visible, mfa.challenge]);

const onSearchClose = useCallback(() => {
setShowSearch(false);
Expand Down Expand Up @@ -136,7 +136,7 @@ function DocumentSsh({ doc, visible }: PropTypes) {
<Indicator />
</Box>
)}
{mfa.requested && <AuthnDialog mfa={mfa} onCancel={closeDocument} />}
{mfa.challenge && <AuthnDialog mfa={mfa} onCancel={closeDocument} />}
{status === 'initialized' && terminal}
</Document>
);
Expand Down
8 changes: 4 additions & 4 deletions web/packages/teleport/src/DesktopSession/DesktopSession.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -186,9 +186,9 @@ const MfaDialog = ({ mfa }: { mfa: MfaState }) => {
<AuthnDialog
mfa={mfa}
onCancel={() => {
mfa.setErrorText(
'This session requires multi factor authentication to continue. Please hit "Retry" and follow the prompts given by your browser to complete authentication.'
);
mfa.submitAttempt.status = 'failed';
mfa.submitAttempt.statusText =
'This session requires multi factor authentication to continue. Please hit "Retry" and follow the prompts given by your browser to complete authentication.';
}}
/>
);
Expand Down Expand Up @@ -294,7 +294,7 @@ const nextScreenState = (

// Otherwise, calculate a new screen state.
const showAnotherSessionActive = showAnotherSessionActiveDialog;
const showMfa = webauthn.requested;
const showMfa = webauthn.challenge;
const showAlert =
fetchAttempt.status === 'failed' || // Fetch attempt failed
tdpConnection.status === 'failed' || // TDP connection failed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,20 @@ export const LoadedWithMultipleOptions = () => {
...defaultProps,
mfa: {
...defaultProps.mfa,
ssoChallenge: {
redirectUrl: 'hi',
requestId: '123',
channelId: '123',
device: {
connectorId: '123',
connectorType: 'saml',
displayName: 'Okta',
challenge: {
ssoChallenge: {
redirectUrl: 'hi',
requestId: '123',
channelId: '123',
device: {
connectorId: '123',
connectorType: 'saml',
displayName: 'Okta',
},
},
webauthnPublicKey: {
challenge: new ArrayBuffer(1),
},
},
webauthnPublicKey: {
challenge: new ArrayBuffer(1),
},
},
};
Expand All @@ -54,8 +56,10 @@ export const LoadedWithSingleOption = () => {
...defaultProps,
mfa: {
...defaultProps.mfa,
webauthnPublicKey: {
challenge: new ArrayBuffer(1),
challenge: {
webauthnPublicKey: {
challenge: new ArrayBuffer(1),
},
},
},
};
Expand All @@ -67,7 +71,10 @@ export const Error = () => {
...defaultProps,
mfa: {
...defaultProps.mfa,
errorText: 'Something went wrong',
submitAttempt: {
status: 'failed',
statusText: 'Something went wrong',
},
},
};
return <AuthnDialog {...props} />;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@
*/

import React from 'react';
import { render, screen, fireEvent } from 'design/utils/testing';
import { render, screen, fireEvent, waitFor } from 'design/utils/testing';

import { makeDefaultMfaState, MfaState } from 'teleport/lib/useMfa';

import { SSOChallenge } from 'teleport/services/mfa';
import { SsoChallenge } from 'teleport/services/mfa';

import AuthnDialog from './AuthnDialog';

const mockSsoChallenge: SSOChallenge = {
const mockSsoChallenge: SsoChallenge = {
redirectUrl: 'url',
requestId: '123',
device: {
Expand All @@ -52,7 +52,11 @@ describe('AuthnDialog', () => {
});

test('renders single option dialog', () => {
const mfa = makeMockState({ ssoChallenge: mockSsoChallenge });
const mfa = makeMockState({
challenge: {
ssoChallenge: mockSsoChallenge,
},
});
render(<AuthnDialog mfa={mfa} onCancel={mockOnCancel} />);

expect(screen.getByText('Verify Your Identity')).toBeInTheDocument();
Expand All @@ -65,9 +69,11 @@ describe('AuthnDialog', () => {

test('renders multi option dialog', () => {
const mfa = makeMockState({
ssoChallenge: mockSsoChallenge,
webauthnPublicKey: {
challenge: new ArrayBuffer(1),
challenge: {
ssoChallenge: mockSsoChallenge,
webauthnPublicKey: {
challenge: new ArrayBuffer(1),
},
},
});
render(<AuthnDialog mfa={mfa} onCancel={mockOnCancel} />);
Expand All @@ -84,7 +90,10 @@ describe('AuthnDialog', () => {

test('displays error text when provided', () => {
const errorText = 'Authentication failed';
const mfa = makeMockState({ errorText });
const mfa = makeMockState({
challenge: {},
submitAttempt: { status: 'failed', statusText: errorText },
});
render(<AuthnDialog mfa={mfa} onCancel={mockOnCancel} />);

expect(screen.getByTestId('danger-alert')).toBeInTheDocument();
Expand All @@ -93,23 +102,27 @@ describe('AuthnDialog', () => {

test('sso button renders with callback', async () => {
const mfa = makeMockState({
ssoChallenge: mockSsoChallenge,
onSsoAuthenticate: jest.fn(),
challenge: {
ssoChallenge: mockSsoChallenge,
},
submitWithMfa: jest.fn(),
});
render(<AuthnDialog mfa={mfa} onCancel={mockOnCancel} />);
const ssoButton = screen.getByText('Okta');
fireEvent.click(ssoButton);
expect(mfa.onSsoAuthenticate).toHaveBeenCalledTimes(1);
expect(mfa.submitWithMfa).toHaveBeenCalledTimes(1);
});

test('webauthn button renders with callback', async () => {
const mfa = makeMockState({
webauthnPublicKey: { challenge: new ArrayBuffer(0) },
onWebauthnAuthenticate: jest.fn(),
challenge: {
webauthnPublicKey: { challenge: new ArrayBuffer(0) },
},
submitWithMfa: jest.fn(),
});
render(<AuthnDialog mfa={mfa} onCancel={mockOnCancel} />);
const webauthn = screen.getByText('Passkey or MFA Device');
fireEvent.click(webauthn);
expect(mfa.onWebauthnAuthenticate).toHaveBeenCalledTimes(1);
expect(mfa.submitWithMfa).toHaveBeenCalledTimes(1);
});
});
25 changes: 13 additions & 12 deletions web/packages/teleport/src/components/AuthnDialog/AuthnDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ import { SSOIcon } from 'shared/components/ButtonSso/ButtonSso';
import { MfaState } from 'teleport/lib/useMfa';

export default function AuthnDialog({ mfa, onCancel }: Props) {
let hasMultipleOptions = mfa.ssoChallenge && mfa.webauthnPublicKey;
let hasMultipleOptions =
mfa.challenge.ssoChallenge && mfa.challenge.webauthnPublicKey;

return (
<Dialog dialogCss={() => ({ width: '400px' })} open={true}>
Expand All @@ -40,9 +41,9 @@ export default function AuthnDialog({ mfa, onCancel }: Props) {
</ButtonIcon>
</Flex>
<DialogContent mb={5}>
{mfa.errorText && (
{mfa.submitAttempt.status === 'failed' && (
<Danger data-testid="danger-alert" mt={2} width="100%">
{mfa.errorText}
{mfa.submitAttempt.statusText}
</Danger>
)}
<Text color="text.slightlyMuted">
Expand All @@ -52,28 +53,28 @@ export default function AuthnDialog({ mfa, onCancel }: Props) {
</Text>
</DialogContent>
<Flex textAlign="center" width="100%" flexDirection="column" gap={2}>
{mfa.ssoChallenge && (
{mfa.challenge.ssoChallenge && (
<ButtonSecondary
size="extra-large"
onClick={mfa.onSsoAuthenticate}
onClick={() => mfa.submitWithMfa('sso')}
gap={2}
block
>
<SSOIcon
type={guessProviderType(
mfa.ssoChallenge.device.displayName ||
mfa.ssoChallenge.device.connectorId,
mfa.ssoChallenge.device.connectorType
mfa.challenge.ssoChallenge.device.displayName ||
mfa.challenge.ssoChallenge.device.connectorId,
mfa.challenge.ssoChallenge.device.connectorType
)}
/>
{mfa.ssoChallenge.device.displayName ||
mfa.ssoChallenge.device.connectorId}
{mfa.challenge.ssoChallenge.device.displayName ||
mfa.challenge.ssoChallenge.device.connectorId}
</ButtonSecondary>
)}
{mfa.webauthnPublicKey && (
{mfa.challenge.webauthnPublicKey && (
<ButtonSecondary
size="extra-large"
onClick={mfa.onWebauthnAuthenticate}
onClick={() => mfa.submitWithMfa('webauthn')}
gap={2}
block
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,16 @@ import {
// token, and after successfully obtaining the token, the function
// `onAuthenticated` will be called with this token.
export default function useReAuthenticate(props: ReauthProps): ReauthState {
if (!props.getMfaChallenge) {
props.getMfaChallenge = () =>
auth.getMfaChallenge({ scope: props.challengeScope });
}

// Note that attempt state "success" is not used or required.
// After the user submits, the control is passed back
// to the caller who is responsible for rendering the `ReAuthenticate`
// component.
const { attempt, setAttempt } = useAttempt('');

const [challenge, setMfaChallenge] = useState<MfaAuthenticateChallenge>(null);

// Provide a custom error handler to catch a webauthn frontend error that occurs
Expand Down Expand Up @@ -79,7 +83,7 @@ export default function useReAuthenticate(props: ReauthProps): ReauthState {
return challenge;
}

return auth.getMfaChallenge({ scope: props.challengeScope }).then(chal => {
return props.getMfaChallenge().then(chal => {
setMfaChallenge(chal);
return chal;
});
Expand Down Expand Up @@ -133,6 +137,7 @@ export default function useReAuthenticate(props: ReauthProps): ReauthState {

export type ReauthProps = {
challengeScope?: MfaChallengeScope;
getMfaChallenge?(): Promise<MfaAuthenticateChallenge>;
onMfaResponse?(res: MfaChallengeResponse): void;
// TODO(Joerger): Remove in favor of onMfaResponse, make onMfaResponse required.
onAuthenticated?(privilegeTokenId: string): void;
Expand Down
Loading

0 comments on commit 5911b85

Please sign in to comment.