From 8cf3345aeb8a7c81c35eed6fb99ae1e813bafd59 Mon Sep 17 00:00:00 2001 From: joerger Date: Tue, 17 Dec 2024 17:32:09 -0800 Subject: [PATCH 01/16] Add global MFA context for admin MFA. --- .../teleport/src/MFAContext/MFAContext.tsx | 33 +++++++++++++++++++ web/packages/teleport/src/Teleport.tsx | 26 +++++++++------ 2 files changed, 49 insertions(+), 10 deletions(-) create mode 100644 web/packages/teleport/src/MFAContext/MFAContext.tsx diff --git a/web/packages/teleport/src/MFAContext/MFAContext.tsx b/web/packages/teleport/src/MFAContext/MFAContext.tsx new file mode 100644 index 0000000000000..390f7847ef33d --- /dev/null +++ b/web/packages/teleport/src/MFAContext/MFAContext.tsx @@ -0,0 +1,33 @@ +import { PropsWithChildren, createContext, useContext } from 'react'; + +import AuthnDialog from 'teleport/components/AuthnDialog'; +import { MfaState, useMfa } from 'teleport/lib/useMfa'; +import { MfaChallengeScope } from 'teleport/services/auth/auth'; + +export interface MFAContextValue { + adminMfa: MfaState; +} + +export const MFAContext = createContext(null); + +export const useMfaContext = () => { + return useContext(MFAContext); +}; + +export const MFAContextProvider = ({ children }: PropsWithChildren) => { + const adminMfa = useMfa({ + req: { + scope: MfaChallengeScope.ADMIN_ACTION, + isMfaRequiredRequest: { + admin_action: {}, + }, + }, + }); + + return ( + + + {children} + + ); +}; diff --git a/web/packages/teleport/src/Teleport.tsx b/web/packages/teleport/src/Teleport.tsx index 0c1e35918c24e..5a87e464659b1 100644 --- a/web/packages/teleport/src/Teleport.tsx +++ b/web/packages/teleport/src/Teleport.tsx @@ -38,12 +38,16 @@ import { LoginClose } from './Login/LoginClose'; import { LoginFailedComponent as LoginFailed } from './Login/LoginFailed'; import { LoginSuccess } from './Login/LoginSuccess'; import { LoginTerminalRedirect } from './Login/LoginTerminalRedirect'; -import { Main } from './Main'; -import { Player } from './Player'; import { SingleLogoutFailed } from './SingleLogoutFailed'; +import { Welcome } from './Welcome'; + +import { Player } from './Player'; + +import { MFAContextProvider } from './MFAContext/MFAContext'; + +import { Main } from './Main'; import TeleportContext from './teleportContext'; import TeleportContextProvider from './TeleportContextProvider'; -import { Welcome } from './Welcome'; const Teleport: React.FC = props => { const { ctx, history } = props; @@ -86,13 +90,15 @@ const Teleport: React.FC = props => { - - - {createPrivateRoutes()} - + + + + {createPrivateRoutes()} + + From f7a93e3e7c95d84992875a4db505e98c7e89dd6a Mon Sep 17 00:00:00 2001 From: joerger Date: Tue, 17 Dec 2024 17:49:36 -0800 Subject: [PATCH 02/16] Use global mfa context for fetch tokens. --- .../teleport/src/JoinTokens/JoinTokens.tsx | 7 +++++- .../teleport/src/MFAContext/MFAContext.tsx | 25 ++++++++++++++++--- web/packages/teleport/src/services/api/api.ts | 12 +++++++-- .../src/services/joinToken/joinToken.ts | 9 +++++-- 4 files changed, 44 insertions(+), 9 deletions(-) diff --git a/web/packages/teleport/src/JoinTokens/JoinTokens.tsx b/web/packages/teleport/src/JoinTokens/JoinTokens.tsx index 909f2c885605d..7b98a0e8c0886 100644 --- a/web/packages/teleport/src/JoinTokens/JoinTokens.tsx +++ b/web/packages/teleport/src/JoinTokens/JoinTokens.tsx @@ -54,6 +54,7 @@ import { } from 'teleport/components/Layout'; import ResourceEditor from 'teleport/components/ResourceEditor'; import useResources from 'teleport/components/useResources'; +import { useMfaContext } from 'teleport/MFAContext/MFAContext'; import { JoinToken } from 'teleport/services/joinToken'; import { KindJoinToken, Resource } from 'teleport/services/resources'; @@ -70,11 +71,15 @@ function makeTokenResource(token: JoinToken): Resource { export const JoinTokens = () => { const ctx = useTeleport(); + const mfa = useMfaContext(); const [creatingToken, setCreatingToken] = useState(false); const [editingToken, setEditingToken] = useState(null); const [tokenToDelete, setTokenToDelete] = useState(null); const [joinTokensAttempt, runJoinTokensAttempt, setJoinTokensAttempt] = - useAsync(async () => await ctx.joinTokenService.fetchJoinTokens()); + useAsync(async () => { + const mfaResponse = await mfa.getAdminActionMfaResponse(); + return ctx.joinTokenService.fetchJoinTokens(null, mfaResponse); + }); const resources = useResources( joinTokensAttempt.data?.items.map(makeTokenResource) || [], diff --git a/web/packages/teleport/src/MFAContext/MFAContext.tsx b/web/packages/teleport/src/MFAContext/MFAContext.tsx index 390f7847ef33d..6d2cdbd85584c 100644 --- a/web/packages/teleport/src/MFAContext/MFAContext.tsx +++ b/web/packages/teleport/src/MFAContext/MFAContext.tsx @@ -1,11 +1,18 @@ -import { PropsWithChildren, createContext, useContext } from 'react'; +import { + PropsWithChildren, + createContext, + useCallback, + useContext, + useRef, +} from 'react'; import AuthnDialog from 'teleport/components/AuthnDialog'; -import { MfaState, useMfa } from 'teleport/lib/useMfa'; +import { useMfa } from 'teleport/lib/useMfa'; import { MfaChallengeScope } from 'teleport/services/auth/auth'; +import { MfaChallengeResponse } from 'teleport/services/mfa'; export interface MFAContextValue { - adminMfa: MfaState; + getAdminActionMfaResponse(reusable?: boolean): Promise; } export const MFAContext = createContext(null); @@ -15,17 +22,27 @@ export const useMfaContext = () => { }; export const MFAContextProvider = ({ children }: PropsWithChildren) => { + const allowReuse = useRef(false); const adminMfa = useMfa({ req: { scope: MfaChallengeScope.ADMIN_ACTION, + allowReuse: allowReuse.current, isMfaRequiredRequest: { admin_action: {}, }, }, }); + const getAdminActionMfaResponse = useCallback( + async (reusable: boolean = false) => { + allowReuse.current = reusable; + return adminMfa.getChallengeResponse(); + }, + [adminMfa, allowReuse] + ); + return ( - + {children} diff --git a/web/packages/teleport/src/services/api/api.ts b/web/packages/teleport/src/services/api/api.ts index 5b19aef0bf580..4260b9a4c459c 100644 --- a/web/packages/teleport/src/services/api/api.ts +++ b/web/packages/teleport/src/services/api/api.ts @@ -28,8 +28,16 @@ import parseError, { ApiError } from './parseError'; export const MFA_HEADER = 'Teleport-Mfa-Response'; const api = { - get(url: string, abortSignal?: AbortSignal) { - return api.fetchJsonWithMfaAuthnRetry(url, { signal: abortSignal }); + get( + url: string, + abortSignal?: AbortSignal, + mfaResponse?: MfaChallengeResponse + ) { + return api.fetchJsonWithMfaAuthnRetry( + url, + { signal: abortSignal }, + mfaResponse + ); }, post(url, data?, abortSignal?, mfaResponse?: MfaChallengeResponse) { diff --git a/web/packages/teleport/src/services/joinToken/joinToken.ts b/web/packages/teleport/src/services/joinToken/joinToken.ts index fe564b4440dae..8e3ad6ba2d862 100644 --- a/web/packages/teleport/src/services/joinToken/joinToken.ts +++ b/web/packages/teleport/src/services/joinToken/joinToken.ts @@ -20,6 +20,8 @@ import cfg from 'teleport/config'; import api from 'teleport/services/api'; import { makeLabelMapOfStrArrs } from '../agents/make'; +import { MfaChallengeResponse } from '../mfa'; + import makeJoinToken from './makeJoinToken'; import { JoinRule, JoinToken, JoinTokenRequest } from './types'; @@ -69,8 +71,11 @@ class JoinTokenService { return api.post(cfg.getJoinTokensUrl(), req).then(makeJoinToken); } - fetchJoinTokens(signal: AbortSignal = null): Promise<{ items: JoinToken[] }> { - return api.get(cfg.getJoinTokensUrl(), signal).then(resp => { + fetchJoinTokens( + signal: AbortSignal = null, + mfaResponse?: MfaChallengeResponse + ): Promise<{ items: JoinToken[] }> { + return api.get(cfg.getJoinTokensUrl(), signal, mfaResponse).then(resp => { return { items: resp.items?.map(makeJoinToken) || [], }; From ae9b0de0df2f47a73685fa0db63e9c4f6cd31768 Mon Sep 17 00:00:00 2001 From: joerger Date: Tue, 17 Dec 2024 19:52:25 -0800 Subject: [PATCH 03/16] Set MFA context for services from MFA Context provider. --- .../teleport/src/JoinTokens/JoinTokens.tsx | 6 +-- .../teleport/src/MFAContext/MFAContext.tsx | 24 +++++------- .../src/services/joinToken/joinToken.ts | 39 +++++++++++++------ 3 files changed, 39 insertions(+), 30 deletions(-) diff --git a/web/packages/teleport/src/JoinTokens/JoinTokens.tsx b/web/packages/teleport/src/JoinTokens/JoinTokens.tsx index 7b98a0e8c0886..8af5200a07681 100644 --- a/web/packages/teleport/src/JoinTokens/JoinTokens.tsx +++ b/web/packages/teleport/src/JoinTokens/JoinTokens.tsx @@ -54,7 +54,6 @@ import { } from 'teleport/components/Layout'; import ResourceEditor from 'teleport/components/ResourceEditor'; import useResources from 'teleport/components/useResources'; -import { useMfaContext } from 'teleport/MFAContext/MFAContext'; import { JoinToken } from 'teleport/services/joinToken'; import { KindJoinToken, Resource } from 'teleport/services/resources'; @@ -71,14 +70,13 @@ function makeTokenResource(token: JoinToken): Resource { export const JoinTokens = () => { const ctx = useTeleport(); - const mfa = useMfaContext(); + const [creatingToken, setCreatingToken] = useState(false); const [editingToken, setEditingToken] = useState(null); const [tokenToDelete, setTokenToDelete] = useState(null); const [joinTokensAttempt, runJoinTokensAttempt, setJoinTokensAttempt] = useAsync(async () => { - const mfaResponse = await mfa.getAdminActionMfaResponse(); - return ctx.joinTokenService.fetchJoinTokens(null, mfaResponse); + return ctx.joinTokenService.fetchJoinTokens(null); }); const resources = useResources( diff --git a/web/packages/teleport/src/MFAContext/MFAContext.tsx b/web/packages/teleport/src/MFAContext/MFAContext.tsx index 6d2cdbd85584c..f54667eb31bc3 100644 --- a/web/packages/teleport/src/MFAContext/MFAContext.tsx +++ b/web/packages/teleport/src/MFAContext/MFAContext.tsx @@ -1,26 +1,17 @@ -import { - PropsWithChildren, - createContext, - useCallback, - useContext, - useRef, -} from 'react'; - +import { PropsWithChildren, createContext, useCallback, useRef } from 'react'; import AuthnDialog from 'teleport/components/AuthnDialog'; import { useMfa } from 'teleport/lib/useMfa'; import { MfaChallengeScope } from 'teleport/services/auth/auth'; import { MfaChallengeResponse } from 'teleport/services/mfa'; +import { useTeleport } from '..'; + export interface MFAContextValue { getAdminActionMfaResponse(reusable?: boolean): Promise; } export const MFAContext = createContext(null); -export const useMfaContext = () => { - return useContext(MFAContext); -}; - export const MFAContextProvider = ({ children }: PropsWithChildren) => { const allowReuse = useRef(false); const adminMfa = useMfa({ @@ -36,13 +27,18 @@ export const MFAContextProvider = ({ children }: PropsWithChildren) => { const getAdminActionMfaResponse = useCallback( async (reusable: boolean = false) => { allowReuse.current = reusable; - return adminMfa.getChallengeResponse(); + return (await adminMfa.getChallengeResponse()) || {}; // return an empty challenge to prevent mfa retry. }, [adminMfa, allowReuse] ); + const mfaCtx = { getAdminActionMfaResponse }; + + const ctx = useTeleport(); + ctx.joinTokenService.setMfaContext(mfaCtx); + return ( - + {children} diff --git a/web/packages/teleport/src/services/joinToken/joinToken.ts b/web/packages/teleport/src/services/joinToken/joinToken.ts index 8e3ad6ba2d862..0669ee952e1f2 100644 --- a/web/packages/teleport/src/services/joinToken/joinToken.ts +++ b/web/packages/teleport/src/services/joinToken/joinToken.ts @@ -17,10 +17,10 @@ */ import cfg from 'teleport/config'; +import { MFAContextValue } from 'teleport/MFAContext/MFAContext'; import api from 'teleport/services/api'; import { makeLabelMapOfStrArrs } from '../agents/make'; -import { MfaChallengeResponse } from '../mfa'; import makeJoinToken from './makeJoinToken'; import { JoinRule, JoinToken, JoinTokenRequest } from './types'; @@ -28,11 +28,18 @@ import { JoinRule, JoinToken, JoinTokenRequest } from './types'; const TeleportTokenNameHeader = 'X-Teleport-TokenName'; class JoinTokenService { + // MFA context is set late by the MFA Context provider. + mfa: MFAContextValue; + setMfaContext(mfa: MFAContextValue) { + this.mfa = mfa; + } + // TODO (avatus) refactor this code to eventually use `createJoinToken` - fetchJoinToken( + async fetchJoinToken( req: JoinTokenRequest, signal: AbortSignal = null ): Promise { + const mfaResponse = await this.mfa.getAdminActionMfaResponse(); return api .post( cfg.getJoinTokenUrl(), @@ -44,15 +51,17 @@ class JoinTokenService { req.suggestedAgentMatcherLabels ), }, - signal + signal, + mfaResponse ) .then(makeJoinToken); } - upsertJoinTokenYAML( + async upsertJoinTokenYAML( req: JoinTokenRequest, tokenName: string ): Promise { + const mfaResponse = await this.mfa.getAdminActionMfaResponse(); return api .putWithHeaders( cfg.getJoinTokenYamlUrl(), @@ -62,19 +71,23 @@ class JoinTokenService { { [TeleportTokenNameHeader]: tokenName, 'Content-Type': 'application/json', - } + }, + mfaResponse ) .then(makeJoinToken); } - createJoinToken(req: JoinTokenRequest): Promise { - return api.post(cfg.getJoinTokensUrl(), req).then(makeJoinToken); + async createJoinToken(req: JoinTokenRequest): Promise { + const mfaResponse = await this.mfa.getAdminActionMfaResponse(); + return api + .post(cfg.getJoinTokensUrl(), req, mfaResponse) + .then(makeJoinToken); } - fetchJoinTokens( - signal: AbortSignal = null, - mfaResponse?: MfaChallengeResponse + async fetchJoinTokens( + signal: AbortSignal = null ): Promise<{ items: JoinToken[] }> { + const mfaResponse = await this.mfa.getAdminActionMfaResponse(); return api.get(cfg.getJoinTokensUrl(), signal, mfaResponse).then(resp => { return { items: resp.items?.map(makeJoinToken) || [], @@ -82,11 +95,13 @@ class JoinTokenService { }); } - deleteJoinToken(id: string, signal: AbortSignal = null) { + async deleteJoinToken(id: string, signal: AbortSignal = null) { + const mfaResponse = await this.mfa.getAdminActionMfaResponse(); return api.deleteWithHeaders( cfg.getJoinTokensUrl(), { [TeleportTokenNameHeader]: id }, - signal + signal, + mfaResponse ); } } From 821b7787a82c71e6b52f052f83a0386b27850299 Mon Sep 17 00:00:00 2001 From: joerger Date: Tue, 17 Dec 2024 20:02:10 -0800 Subject: [PATCH 04/16] Use mfa context in api calls. --- .../teleport/src/MFAContext/MFAContext.tsx | 12 +++++++----- web/packages/teleport/src/Teleport.tsx | 6 +++--- web/packages/teleport/src/services/api/api.ts | 16 +++++++++++----- .../teleport/src/services/joinToken/joinToken.ts | 6 +++--- 4 files changed, 24 insertions(+), 16 deletions(-) diff --git a/web/packages/teleport/src/MFAContext/MFAContext.tsx b/web/packages/teleport/src/MFAContext/MFAContext.tsx index f54667eb31bc3..ddd478f207fba 100644 --- a/web/packages/teleport/src/MFAContext/MFAContext.tsx +++ b/web/packages/teleport/src/MFAContext/MFAContext.tsx @@ -1,18 +1,19 @@ import { PropsWithChildren, createContext, useCallback, useRef } from 'react'; import AuthnDialog from 'teleport/components/AuthnDialog'; import { useMfa } from 'teleport/lib/useMfa'; +import api from 'teleport/services/api'; import { MfaChallengeScope } from 'teleport/services/auth/auth'; import { MfaChallengeResponse } from 'teleport/services/mfa'; import { useTeleport } from '..'; -export interface MFAContextValue { +export interface MfaContextValue { getAdminActionMfaResponse(reusable?: boolean): Promise; } -export const MFAContext = createContext(null); +export const MfaContext = createContext(null); -export const MFAContextProvider = ({ children }: PropsWithChildren) => { +export const MfaContextProvider = ({ children }: PropsWithChildren) => { const allowReuse = useRef(false); const adminMfa = useMfa({ req: { @@ -36,11 +37,12 @@ export const MFAContextProvider = ({ children }: PropsWithChildren) => { const ctx = useTeleport(); ctx.joinTokenService.setMfaContext(mfaCtx); + api.setMfaContext(mfaCtx); return ( - + {children} - + ); }; diff --git a/web/packages/teleport/src/Teleport.tsx b/web/packages/teleport/src/Teleport.tsx index 5a87e464659b1..f192b6948bb39 100644 --- a/web/packages/teleport/src/Teleport.tsx +++ b/web/packages/teleport/src/Teleport.tsx @@ -43,7 +43,7 @@ import { Welcome } from './Welcome'; import { Player } from './Player'; -import { MFAContextProvider } from './MFAContext/MFAContext'; +import { MfaContextProvider } from './MFAContext/MFAContext'; import { Main } from './Main'; import TeleportContext from './teleportContext'; @@ -90,7 +90,7 @@ const Teleport: React.FC = props => { - + = props => { /> {createPrivateRoutes()} - + diff --git a/web/packages/teleport/src/services/api/api.ts b/web/packages/teleport/src/services/api/api.ts index 4260b9a4c459c..e13f07e2adece 100644 --- a/web/packages/teleport/src/services/api/api.ts +++ b/web/packages/teleport/src/services/api/api.ts @@ -18,16 +18,25 @@ import 'whatwg-fetch'; -import auth, { MfaChallengeScope } from 'teleport/services/auth/auth'; import websession from 'teleport/services/websession'; +import 'whatwg-fetch'; + +import { MfaContextValue } from 'teleport/MFAContext/MFAContext'; import { MfaChallengeResponse } from '../mfa'; import { storageService } from '../storageService'; + import parseError, { ApiError } from './parseError'; export const MFA_HEADER = 'Teleport-Mfa-Response'; +let mfaContext: MfaContextValue; + const api = { + setMfaContext(mfa: MfaContextValue) { + mfaContext = mfa; + }, + get( url: string, abortSignal?: AbortSignal, @@ -189,10 +198,7 @@ const api = { let mfaResponseForRetry; try { - const challenge = await auth.getMfaChallenge({ - scope: MfaChallengeScope.ADMIN_ACTION, - }); - mfaResponseForRetry = await auth.getMfaChallengeResponse(challenge); + mfaResponseForRetry = await mfaContext.getAdminActionMfaResponse(); } catch { throw new Error( 'Failed to fetch MFA challenge. Please connect a registered hardware key and try again. If you do not have a hardware key registered, you can add one from your account settings page.' diff --git a/web/packages/teleport/src/services/joinToken/joinToken.ts b/web/packages/teleport/src/services/joinToken/joinToken.ts index 0669ee952e1f2..e6bc2e34b126d 100644 --- a/web/packages/teleport/src/services/joinToken/joinToken.ts +++ b/web/packages/teleport/src/services/joinToken/joinToken.ts @@ -17,7 +17,7 @@ */ import cfg from 'teleport/config'; -import { MFAContextValue } from 'teleport/MFAContext/MFAContext'; +import { MfaContextValue } from 'teleport/MFAContext/MFAContext'; import api from 'teleport/services/api'; import { makeLabelMapOfStrArrs } from '../agents/make'; @@ -29,8 +29,8 @@ const TeleportTokenNameHeader = 'X-Teleport-TokenName'; class JoinTokenService { // MFA context is set late by the MFA Context provider. - mfa: MFAContextValue; - setMfaContext(mfa: MFAContextValue) { + mfa: MfaContextValue; + setMfaContext(mfa: MfaContextValue) { this.mfa = mfa; } From 900d8752682187d72f02fec7d993ee9442b6e8fb Mon Sep 17 00:00:00 2001 From: joerger Date: Wed, 18 Dec 2024 11:31:58 -0800 Subject: [PATCH 05/16] Use global mfa context for all admin actions with explicit MFA checks. --- .../teleport/src/MFAContext/MFAContext.tsx | 15 +++--- web/packages/teleport/src/Users/useUsers.ts | 2 +- web/packages/teleport/src/services/api/api.ts | 11 +---- .../teleport/src/services/auth/auth.ts | 48 ++++++------------- .../src/services/integrations/integrations.ts | 19 ++------ .../src/services/joinToken/joinToken.ts | 18 +++---- 6 files changed, 34 insertions(+), 79 deletions(-) diff --git a/web/packages/teleport/src/MFAContext/MFAContext.tsx b/web/packages/teleport/src/MFAContext/MFAContext.tsx index ddd478f207fba..daac6287bad00 100644 --- a/web/packages/teleport/src/MFAContext/MFAContext.tsx +++ b/web/packages/teleport/src/MFAContext/MFAContext.tsx @@ -1,12 +1,9 @@ import { PropsWithChildren, createContext, useCallback, useRef } from 'react'; import AuthnDialog from 'teleport/components/AuthnDialog'; import { useMfa } from 'teleport/lib/useMfa'; -import api from 'teleport/services/api'; -import { MfaChallengeScope } from 'teleport/services/auth/auth'; +import auth, { MfaChallengeScope } from 'teleport/services/auth/auth'; import { MfaChallengeResponse } from 'teleport/services/mfa'; -import { useTeleport } from '..'; - export interface MfaContextValue { getAdminActionMfaResponse(reusable?: boolean): Promise; } @@ -33,11 +30,11 @@ export const MfaContextProvider = ({ children }: PropsWithChildren) => { [adminMfa, allowReuse] ); - const mfaCtx = { getAdminActionMfaResponse }; - - const ctx = useTeleport(); - ctx.joinTokenService.setMfaContext(mfaCtx); - api.setMfaContext(mfaCtx); + const mfaCtx = { + getAdminActionMfaResponse, + useMfa, + }; + auth.setMfaContext(mfaCtx); return ( diff --git a/web/packages/teleport/src/Users/useUsers.ts b/web/packages/teleport/src/Users/useUsers.ts index 48dc99dd11637..404ea58fe252b 100644 --- a/web/packages/teleport/src/Users/useUsers.ts +++ b/web/packages/teleport/src/Users/useUsers.ts @@ -88,7 +88,7 @@ export default function useUsers({ } async function onCreate(u: User) { - const mfaResponse = await auth.getMfaChallengeResponseForAdminAction(true); + const mfaResponse = await auth.getAdminActionMfaResponse(true); return ctx.userService .createUser(u, ExcludeUserField.Traits, mfaResponse) .then(result => setUsers([result, ...users])) diff --git a/web/packages/teleport/src/services/api/api.ts b/web/packages/teleport/src/services/api/api.ts index e13f07e2adece..07bb68f068905 100644 --- a/web/packages/teleport/src/services/api/api.ts +++ b/web/packages/teleport/src/services/api/api.ts @@ -21,22 +21,15 @@ import 'whatwg-fetch'; import websession from 'teleport/services/websession'; import 'whatwg-fetch'; -import { MfaContextValue } from 'teleport/MFAContext/MFAContext'; - import { MfaChallengeResponse } from '../mfa'; import { storageService } from '../storageService'; +import auth from '../auth/auth'; import parseError, { ApiError } from './parseError'; export const MFA_HEADER = 'Teleport-Mfa-Response'; -let mfaContext: MfaContextValue; - const api = { - setMfaContext(mfa: MfaContextValue) { - mfaContext = mfa; - }, - get( url: string, abortSignal?: AbortSignal, @@ -198,7 +191,7 @@ const api = { let mfaResponseForRetry; try { - mfaResponseForRetry = await mfaContext.getAdminActionMfaResponse(); + mfaResponseForRetry = await auth.getAdminActionMfaResponse(); } catch { throw new Error( 'Failed to fetch MFA challenge. Please connect a registered hardware key and try again. If you do not have a hardware key registered, you can add one from your account settings page.' diff --git a/web/packages/teleport/src/services/auth/auth.ts b/web/packages/teleport/src/services/auth/auth.ts index 100259d6dfc20..2b3f4d7a0cde1 100644 --- a/web/packages/teleport/src/services/auth/auth.ts +++ b/web/packages/teleport/src/services/auth/auth.ts @@ -25,6 +25,8 @@ import { MfaChallengeResponse, SsoChallenge, } from 'teleport/services/mfa'; + +import { MfaContextValue } from 'teleport/MFAContext/MFAContext'; import { CaptureEvent, userEventService } from 'teleport/services/userEvent'; import { @@ -33,6 +35,7 @@ import { parseMfaChallengeJson, parseMfaRegistrationChallengeJson, } from '../mfa/makeMfa'; + import { makeChangedUserAuthn } from './make'; import makePasswordToken from './makePasswordToken'; import { @@ -44,7 +47,13 @@ import { UserCredentials, } from './types'; +let mfaContext: MfaContextValue; + const auth = { + setMfaContext(mfa: MfaContextValue) { + mfaContext = mfa; + }, + checkWebauthnSupport() { if (window.PublicKeyCredential) { return Promise.resolve(); @@ -277,24 +286,9 @@ const auth = { // If is_mfa_required_req is provided and it is found that MFA is not required, returns null instead. async getMfaChallengeResponse( challenge: MfaAuthenticateChallenge, - mfaType?: DeviceType, + mfaType: DeviceType, totpCode?: string ): Promise { - if (!challenge) return; - - // TODO(Joerger): If mfaType is not provided by a parent component, use some global context - // to display a component, similar to the one used in useMfa. For now we just default to - // whichever method we can succeed with first. - if (!mfaType) { - if (totpCode) { - mfaType = 'totp'; - } else if (challenge.webauthnPublicKey) { - mfaType = 'webauthn'; - } else if (challenge.ssoChallenge) { - mfaType = 'sso'; - } - } - if (mfaType === 'webauthn') { return auth.getWebAuthnChallengeResponse(challenge.webauthnPublicKey); } @@ -389,7 +383,7 @@ const auth = { createPrivilegeTokenWithWebauthn() { return auth .getMfaChallenge({ scope: MfaChallengeScope.MANAGE_DEVICES }) - .then(auth.getMfaChallengeResponse) + .then(chal => auth.getMfaChallengeResponse(chal, 'webauthn')) .then(mfaResp => auth.createPrivilegeToken(mfaResp)); }, @@ -442,27 +436,13 @@ const auth = { .then(res => res?.webauthn_response); }, - getMfaChallengeResponseForAdminAction(allowReuse?: boolean) { - // If the client is checking if MFA is required for an admin action, - // but we know admin action MFA is not enforced, return early. - if (!cfg.isAdminActionMfaEnforced()) { - return; - } - - return auth - .getMfaChallenge({ - scope: MfaChallengeScope.ADMIN_ACTION, - allowReuse: allowReuse, - isMfaRequiredRequest: { - admin_action: {}, - }, - }) - .then(auth.getMfaChallengeResponse); + getAdminActionMfaResponse(allowReuse?: boolean) { + return mfaContext.getAdminActionMfaResponse(allowReuse); }, // TODO(Joerger): Delete in favor of getMfaChallengeResponseForAdminAction once /e is updated. getWebauthnResponseForAdminAction(allowReuse?: boolean) { - return auth.getMfaChallengeResponseForAdminAction(allowReuse); + return mfaContext.getAdminActionMfaResponse(allowReuse); }, }; diff --git a/web/packages/teleport/src/services/integrations/integrations.ts b/web/packages/teleport/src/services/integrations/integrations.ts index 7b1ffa0b1724d..ff78b8a90f2d4 100644 --- a/web/packages/teleport/src/services/integrations/integrations.ts +++ b/web/packages/teleport/src/services/integrations/integrations.ts @@ -21,8 +21,9 @@ import api from 'teleport/services/api'; import { App } from '../apps'; import makeApp from '../apps/makeApps'; -import auth, { MfaChallengeScope } from '../auth/auth'; +import auth from '../auth/auth'; import makeNode from '../nodes/makeNode'; + import { AwsDatabaseVpcsResponse, AwsOidcDeployDatabaseServicesRequest, @@ -271,16 +272,7 @@ export const integrationService = { integrationName, req: AwsOidcDeployServiceRequest ): Promise { - const challenge = await auth.getMfaChallenge({ - scope: MfaChallengeScope.ADMIN_ACTION, - allowReuse: true, - isMfaRequiredRequest: { - admin_action: {}, - }, - }); - - const response = await auth.getMfaChallengeResponse(challenge); - + const response = await auth.getAdminActionMfaResponse(true); return api .post( cfg.getAwsDeployTeleportServiceUrl(integrationName), @@ -301,8 +293,7 @@ export const integrationService = { integrationName, req: AwsOidcDeployDatabaseServicesRequest ): Promise { - const mfaResponse = await auth.getMfaChallengeResponseForAdminAction(true); - + const mfaResponse = await auth.getAdminActionMfaResponse(true); return api .post( cfg.getAwsRdsDbsDeployServicesUrl(integrationName), @@ -317,7 +308,7 @@ export const integrationService = { integrationName: string, req: EnrollEksClustersRequest ): Promise { - const mfaResponse = await auth.getMfaChallengeResponseForAdminAction(true); + const mfaResponse = await auth.getAdminActionMfaResponse(true); return api.post( cfg.getEnrollEksClusterUrl(integrationName), diff --git a/web/packages/teleport/src/services/joinToken/joinToken.ts b/web/packages/teleport/src/services/joinToken/joinToken.ts index e6bc2e34b126d..c575b5803d689 100644 --- a/web/packages/teleport/src/services/joinToken/joinToken.ts +++ b/web/packages/teleport/src/services/joinToken/joinToken.ts @@ -17,10 +17,10 @@ */ import cfg from 'teleport/config'; -import { MfaContextValue } from 'teleport/MFAContext/MFAContext'; import api from 'teleport/services/api'; import { makeLabelMapOfStrArrs } from '../agents/make'; +import auth from '../auth/auth'; import makeJoinToken from './makeJoinToken'; import { JoinRule, JoinToken, JoinTokenRequest } from './types'; @@ -28,18 +28,12 @@ import { JoinRule, JoinToken, JoinTokenRequest } from './types'; const TeleportTokenNameHeader = 'X-Teleport-TokenName'; class JoinTokenService { - // MFA context is set late by the MFA Context provider. - mfa: MfaContextValue; - setMfaContext(mfa: MfaContextValue) { - this.mfa = mfa; - } - // TODO (avatus) refactor this code to eventually use `createJoinToken` async fetchJoinToken( req: JoinTokenRequest, signal: AbortSignal = null ): Promise { - const mfaResponse = await this.mfa.getAdminActionMfaResponse(); + const mfaResponse = await auth.getAdminActionMfaResponse(); return api .post( cfg.getJoinTokenUrl(), @@ -61,7 +55,7 @@ class JoinTokenService { req: JoinTokenRequest, tokenName: string ): Promise { - const mfaResponse = await this.mfa.getAdminActionMfaResponse(); + const mfaResponse = await auth.getAdminActionMfaResponse(); return api .putWithHeaders( cfg.getJoinTokenYamlUrl(), @@ -78,7 +72,7 @@ class JoinTokenService { } async createJoinToken(req: JoinTokenRequest): Promise { - const mfaResponse = await this.mfa.getAdminActionMfaResponse(); + const mfaResponse = await auth.getAdminActionMfaResponse(); return api .post(cfg.getJoinTokensUrl(), req, mfaResponse) .then(makeJoinToken); @@ -87,7 +81,7 @@ class JoinTokenService { async fetchJoinTokens( signal: AbortSignal = null ): Promise<{ items: JoinToken[] }> { - const mfaResponse = await this.mfa.getAdminActionMfaResponse(); + const mfaResponse = await auth.getAdminActionMfaResponse(); return api.get(cfg.getJoinTokensUrl(), signal, mfaResponse).then(resp => { return { items: resp.items?.map(makeJoinToken) || [], @@ -96,7 +90,7 @@ class JoinTokenService { } async deleteJoinToken(id: string, signal: AbortSignal = null) { - const mfaResponse = await this.mfa.getAdminActionMfaResponse(); + const mfaResponse = await auth.getAdminActionMfaResponse(); return api.deleteWithHeaders( cfg.getJoinTokensUrl(), { [TeleportTokenNameHeader]: id }, From d35da73b25f2375ca40d03f86454bfee894a3cde Mon Sep 17 00:00:00 2001 From: joerger Date: Wed, 18 Dec 2024 12:00:47 -0800 Subject: [PATCH 06/16] Small fixes. --- .../teleport/src/MFAContext/MFAContext.tsx | 2 +- web/packages/teleport/src/services/auth/auth.ts | 6 +++--- .../src/services/joinToken/joinToken.test.ts | 16 +++++++++++----- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/web/packages/teleport/src/MFAContext/MFAContext.tsx b/web/packages/teleport/src/MFAContext/MFAContext.tsx index daac6287bad00..bba93cfd177d8 100644 --- a/web/packages/teleport/src/MFAContext/MFAContext.tsx +++ b/web/packages/teleport/src/MFAContext/MFAContext.tsx @@ -38,7 +38,7 @@ export const MfaContextProvider = ({ children }: PropsWithChildren) => { return ( - + {children} ); diff --git a/web/packages/teleport/src/services/auth/auth.ts b/web/packages/teleport/src/services/auth/auth.ts index 2b3f4d7a0cde1..a8f7b5a5ab8d8 100644 --- a/web/packages/teleport/src/services/auth/auth.ts +++ b/web/packages/teleport/src/services/auth/auth.ts @@ -436,13 +436,13 @@ const auth = { .then(res => res?.webauthn_response); }, - getAdminActionMfaResponse(allowReuse?: boolean) { + async getAdminActionMfaResponse(allowReuse?: boolean) { return mfaContext.getAdminActionMfaResponse(allowReuse); }, // TODO(Joerger): Delete in favor of getMfaChallengeResponseForAdminAction once /e is updated. - getWebauthnResponseForAdminAction(allowReuse?: boolean) { - return mfaContext.getAdminActionMfaResponse(allowReuse); + async getWebauthnResponseForAdminAction(allowReuse?: boolean) { + return auth.getAdminActionMfaResponse(allowReuse); }, }; diff --git a/web/packages/teleport/src/services/joinToken/joinToken.test.ts b/web/packages/teleport/src/services/joinToken/joinToken.test.ts index 1f941345c1006..54b678b3f3b69 100644 --- a/web/packages/teleport/src/services/joinToken/joinToken.test.ts +++ b/web/packages/teleport/src/services/joinToken/joinToken.test.ts @@ -19,15 +19,17 @@ import cfg from 'teleport/config'; import api from 'teleport/services/api'; +import auth from '../auth/auth'; + import JoinTokenService from './joinToken'; import type { JoinTokenRequest } from './types'; - -test('fetchJoinToken with an empty request properly sets defaults', () => { +test('fetchJoinToken with an empty request properly sets defaults', async () => { const svc = new JoinTokenService(); jest.spyOn(api, 'post').mockResolvedValue(null); + jest.spyOn(auth, 'getAdminActionMfaResponse').mockResolvedValue(null); // Test with all empty fields. - svc.fetchJoinToken({} as any); + await svc.fetchJoinToken({} as any); expect(api.post).toHaveBeenCalledWith( cfg.getJoinTokenUrl(), { @@ -36,13 +38,15 @@ test('fetchJoinToken with an empty request properly sets defaults', () => { allow: [], suggested_agent_matcher_labels: {}, }, + null, null ); }); -test('fetchJoinToken request fields are set as requested', () => { +test('fetchJoinToken request fields are set as requested', async () => { const svc = new JoinTokenService(); jest.spyOn(api, 'post').mockResolvedValue(null); + jest.spyOn(auth, 'getAdminActionMfaResponse').mockResolvedValue(null); const mock: JoinTokenRequest = { roles: ['Node'], @@ -50,7 +54,8 @@ test('fetchJoinToken request fields are set as requested', () => { method: 'iam', suggestedAgentMatcherLabels: [{ name: 'env', value: 'dev' }], }; - svc.fetchJoinToken(mock); + await svc.fetchJoinToken(mock); + expect(api.post).toHaveBeenCalledWith( cfg.getJoinTokenUrl(), { @@ -59,6 +64,7 @@ test('fetchJoinToken request fields are set as requested', () => { allow: [{ aws_account: '1234', aws_arn: 'xxxx' }], suggested_agent_matcher_labels: { env: ['dev'] }, }, + null, null ); }); From 5c1e0866d1755ecc5dbbff61b7c76564431d7d28 Mon Sep 17 00:00:00 2001 From: joerger Date: Thu, 2 Jan 2025 13:32:12 -0800 Subject: [PATCH 07/16] Address comments. --- .../teleport/src/JoinTokens/JoinTokens.tsx | 4 +- .../teleport/src/MFAContext/MFAContext.tsx | 44 +++++++++++-------- 2 files changed, 26 insertions(+), 22 deletions(-) diff --git a/web/packages/teleport/src/JoinTokens/JoinTokens.tsx b/web/packages/teleport/src/JoinTokens/JoinTokens.tsx index 8af5200a07681..43cac62115b21 100644 --- a/web/packages/teleport/src/JoinTokens/JoinTokens.tsx +++ b/web/packages/teleport/src/JoinTokens/JoinTokens.tsx @@ -75,9 +75,7 @@ export const JoinTokens = () => { const [editingToken, setEditingToken] = useState(null); const [tokenToDelete, setTokenToDelete] = useState(null); const [joinTokensAttempt, runJoinTokensAttempt, setJoinTokensAttempt] = - useAsync(async () => { - return ctx.joinTokenService.fetchJoinTokens(null); - }); + useAsync(() => ctx.joinTokenService.fetchJoinTokens(null)); const resources = useResources( joinTokensAttempt.data?.items.map(makeTokenResource) || [], diff --git a/web/packages/teleport/src/MFAContext/MFAContext.tsx b/web/packages/teleport/src/MFAContext/MFAContext.tsx index bba93cfd177d8..03d0754566a9c 100644 --- a/web/packages/teleport/src/MFAContext/MFAContext.tsx +++ b/web/packages/teleport/src/MFAContext/MFAContext.tsx @@ -1,4 +1,5 @@ -import { PropsWithChildren, createContext, useCallback, useRef } from 'react'; +import { createContext, PropsWithChildren, useCallback, useState } from 'react'; + import AuthnDialog from 'teleport/components/AuthnDialog'; import { useMfa } from 'teleport/lib/useMfa'; import auth, { MfaChallengeScope } from 'teleport/services/auth/auth'; @@ -11,30 +12,35 @@ export interface MfaContextValue { export const MfaContext = createContext(null); export const MfaContextProvider = ({ children }: PropsWithChildren) => { - const allowReuse = useRef(false); - const adminMfa = useMfa({ - req: { - scope: MfaChallengeScope.ADMIN_ACTION, - allowReuse: allowReuse.current, - isMfaRequiredRequest: { - admin_action: {}, - }, - }, - }); + const adminMfa = useMfa({}); const getAdminActionMfaResponse = useCallback( async (reusable: boolean = false) => { - allowReuse.current = reusable; - return (await adminMfa.getChallengeResponse()) || {}; // return an empty challenge to prevent mfa retry. + const chal = await auth.getMfaChallenge({ + scope: MfaChallengeScope.ADMIN_ACTION, + allowReuse: reusable, + isMfaRequiredRequest: { + admin_action: {}, + }, + }); + + const res = await adminMfa.getChallengeResponse(chal); + if (!res) { + return {}; // return an empty challenge to prevent mfa retry. + } + + return res; }, - [adminMfa, allowReuse] + [adminMfa] ); - const mfaCtx = { - getAdminActionMfaResponse, - useMfa, - }; - auth.setMfaContext(mfaCtx); + const [mfaCtx, setMfaCtx] = useState(); + + if (!mfaCtx) { + const mfaCtx = { getAdminActionMfaResponse }; + setMfaCtx(mfaCtx); + auth.setMfaContext(mfaCtx); + } return ( From 73fd28a83e431b42f2f5262b888a83c5c82513ac Mon Sep 17 00:00:00 2001 From: joerger Date: Fri, 3 Jan 2025 12:09:12 -0800 Subject: [PATCH 08/16] Address comments. --- .../teleport/src/MFAContext/MFAContext.tsx | 27 ++++++++++--------- .../teleport/src/services/auth/auth.ts | 26 +++++++++++++++--- .../src/services/joinToken/joinToken.ts | 1 - 3 files changed, 38 insertions(+), 16 deletions(-) diff --git a/web/packages/teleport/src/MFAContext/MFAContext.tsx b/web/packages/teleport/src/MFAContext/MFAContext.tsx index 03d0754566a9c..8e77b81c58f63 100644 --- a/web/packages/teleport/src/MFAContext/MFAContext.tsx +++ b/web/packages/teleport/src/MFAContext/MFAContext.tsx @@ -2,27 +2,30 @@ import { createContext, PropsWithChildren, useCallback, useState } from 'react'; import AuthnDialog from 'teleport/components/AuthnDialog'; import { useMfa } from 'teleport/lib/useMfa'; -import auth, { MfaChallengeScope } from 'teleport/services/auth/auth'; +import { CreateAuthenticateChallengeRequest } from 'teleport/services/auth'; +import auth from 'teleport/services/auth/auth'; import { MfaChallengeResponse } from 'teleport/services/mfa'; export interface MfaContextValue { - getAdminActionMfaResponse(reusable?: boolean): Promise; + getMfaChallengeResponse( + req: CreateAuthenticateChallengeRequest + ): Promise; } export const MfaContext = createContext(null); +/** + * Provides a global MFA context to handle MFA prompts for methods outside + * of the React scope, such as admin action API calls in auth.ts or api.ts. + * This is intended as a workaround for such cases, and should not be used + * for methods with access to the React scope. Use useMfa directly instead. + */ export const MfaContextProvider = ({ children }: PropsWithChildren) => { const adminMfa = useMfa({}); - const getAdminActionMfaResponse = useCallback( - async (reusable: boolean = false) => { - const chal = await auth.getMfaChallenge({ - scope: MfaChallengeScope.ADMIN_ACTION, - allowReuse: reusable, - isMfaRequiredRequest: { - admin_action: {}, - }, - }); + const getMfaChallengeResponse = useCallback( + async (req: CreateAuthenticateChallengeRequest) => { + const chal = await auth.getMfaChallenge(req); const res = await adminMfa.getChallengeResponse(chal); if (!res) { @@ -37,7 +40,7 @@ export const MfaContextProvider = ({ children }: PropsWithChildren) => { const [mfaCtx, setMfaCtx] = useState(); if (!mfaCtx) { - const mfaCtx = { getAdminActionMfaResponse }; + const mfaCtx = { getMfaChallengeResponse }; setMfaCtx(mfaCtx); auth.setMfaContext(mfaCtx); } diff --git a/web/packages/teleport/src/services/auth/auth.ts b/web/packages/teleport/src/services/auth/auth.ts index a8f7b5a5ab8d8..5520c793d75bd 100644 --- a/web/packages/teleport/src/services/auth/auth.ts +++ b/web/packages/teleport/src/services/auth/auth.ts @@ -436,12 +436,32 @@ const auth = { .then(res => res?.webauthn_response); }, - async getAdminActionMfaResponse(allowReuse?: boolean) { - return mfaContext.getAdminActionMfaResponse(allowReuse); + getAdminActionMfaResponse(allowReuse?: boolean) { + // mfaContext is set once the react-scoped MFA Context Provider is initialized. + // Since this is a global object outside of the react scope, there is a marginal + // chance for a race condition here (the react scope should generally be initialized + // before this has a chance of being called). This conditional is not expected to + // be hit, but will catch any major issues that could arise from this solution. + if (!mfaContext) { + setTimeout(() => { + if (!mfaContext) + throw new Error( + 'Failed to set up MFA prompt for admin action. This is a bug.' + ); + }, 1000); + } + + return mfaContext.getMfaChallengeResponse({ + scope: MfaChallengeScope.ADMIN_ACTION, + allowReuse, + isMfaRequiredRequest: { + admin_action: {}, + }, + }); }, // TODO(Joerger): Delete in favor of getMfaChallengeResponseForAdminAction once /e is updated. - async getWebauthnResponseForAdminAction(allowReuse?: boolean) { + getWebauthnResponseForAdminAction(allowReuse?: boolean) { return auth.getAdminActionMfaResponse(allowReuse); }, }; diff --git a/web/packages/teleport/src/services/joinToken/joinToken.ts b/web/packages/teleport/src/services/joinToken/joinToken.ts index c575b5803d689..721dd586548bc 100644 --- a/web/packages/teleport/src/services/joinToken/joinToken.ts +++ b/web/packages/teleport/src/services/joinToken/joinToken.ts @@ -21,7 +21,6 @@ import api from 'teleport/services/api'; import { makeLabelMapOfStrArrs } from '../agents/make'; import auth from '../auth/auth'; - import makeJoinToken from './makeJoinToken'; import { JoinRule, JoinToken, JoinTokenRequest } from './types'; From 3dfc7bc2fa7543adbc43c2216b5baf42e57a1bc8 Mon Sep 17 00:00:00 2001 From: joerger Date: Fri, 3 Jan 2025 13:01:30 -0800 Subject: [PATCH 09/16] Remove circular dependency with auth. --- .../teleport/src/MFAContext/MFAContext.tsx | 2 + web/packages/teleport/src/services/api/api.ts | 39 ++++++++++++++++--- 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/web/packages/teleport/src/MFAContext/MFAContext.tsx b/web/packages/teleport/src/MFAContext/MFAContext.tsx index 8e77b81c58f63..3379eaa1891aa 100644 --- a/web/packages/teleport/src/MFAContext/MFAContext.tsx +++ b/web/packages/teleport/src/MFAContext/MFAContext.tsx @@ -2,6 +2,7 @@ import { createContext, PropsWithChildren, useCallback, useState } from 'react'; import AuthnDialog from 'teleport/components/AuthnDialog'; import { useMfa } from 'teleport/lib/useMfa'; +import api from 'teleport/services/api/api'; import { CreateAuthenticateChallengeRequest } from 'teleport/services/auth'; import auth from 'teleport/services/auth/auth'; import { MfaChallengeResponse } from 'teleport/services/mfa'; @@ -43,6 +44,7 @@ export const MfaContextProvider = ({ children }: PropsWithChildren) => { const mfaCtx = { getMfaChallengeResponse }; setMfaCtx(mfaCtx); auth.setMfaContext(mfaCtx); + api.setMfaContext(mfaCtx); } return ( diff --git a/web/packages/teleport/src/services/api/api.ts b/web/packages/teleport/src/services/api/api.ts index 07bb68f068905..ba504a0d242c7 100644 --- a/web/packages/teleport/src/services/api/api.ts +++ b/web/packages/teleport/src/services/api/api.ts @@ -18,18 +18,23 @@ import 'whatwg-fetch'; +import { MfaContextValue } from 'teleport/MFAContext/MFAContext'; import websession from 'teleport/services/websession'; -import 'whatwg-fetch'; +import { MfaChallengeScope } from '../auth/auth'; import { MfaChallengeResponse } from '../mfa'; import { storageService } from '../storageService'; -import auth from '../auth/auth'; - import parseError, { ApiError } from './parseError'; export const MFA_HEADER = 'Teleport-Mfa-Response'; +let mfaContext: MfaContextValue; + const api = { + setMfaContext(mfa: MfaContextValue) { + mfaContext = mfa; + }, + get( url: string, abortSignal?: AbortSignal, @@ -191,10 +196,10 @@ const api = { let mfaResponseForRetry; try { - mfaResponseForRetry = await auth.getAdminActionMfaResponse(); + mfaResponseForRetry = await api.getAdminActionMfaResponse(); } catch { throw new Error( - 'Failed to fetch MFA challenge. Please connect a registered hardware key and try again. If you do not have a hardware key registered, you can add one from your account settings page.' + 'This is an admin-level API request and requires MFA verification. Please try again with a registered MFA device. If you do not have an MFA device registered, you can add one in the account settings page.' ); } @@ -205,6 +210,30 @@ const api = { ); }, + getAdminActionMfaResponse(allowReuse?: boolean) { + // mfaContext is set once the react-scoped MFA Context Provider is initialized. + // Since this is a global object outside of the react scope, there is a marginal + // chance for a race condition here (the react scope should generally be initialized + // before this has a chance of being called). This conditional is not expected to + // be hit, but will catch any major issues that could arise from this solution. + if (!mfaContext) { + setTimeout(() => { + if (!mfaContext) + throw new Error( + 'Failed to set up MFA prompt for admin action. This is a bug.' + ); + }, 1000); + } + + return mfaContext.getMfaChallengeResponse({ + scope: MfaChallengeScope.ADMIN_ACTION, + allowReuse, + isMfaRequiredRequest: { + admin_action: {}, + }, + }); + }, + /** * fetch calls upon native fetch with options and headers set * to default (or provided) values. From 6b2d2947b58699d9125351ac841bbd11a8e9e400 Mon Sep 17 00:00:00 2001 From: joerger Date: Fri, 3 Jan 2025 13:07:34 -0800 Subject: [PATCH 10/16] Add useful information in admin mfa cancelled error message. --- web/packages/teleport/src/lib/useMfa.test.tsx | 5 +++-- web/packages/teleport/src/lib/useMfa.ts | 6 +++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/web/packages/teleport/src/lib/useMfa.test.tsx b/web/packages/teleport/src/lib/useMfa.test.tsx index dad6b2645b70b..cfcb3d1baf555 100644 --- a/web/packages/teleport/src/lib/useMfa.test.tsx +++ b/web/packages/teleport/src/lib/useMfa.test.tsx @@ -234,9 +234,10 @@ describe('useMfa', () => { mfa.current.cancelAttempt(); - await expect(respPromise).rejects.toThrow( - new Error('User canceled MFA attempt') + const expectErr = new Error( + 'User cancelled MFA attempt. This is an admin-level API request and requires MFA verification. Please try again with a registered MFA device. If you do not have an MFA device registered, you can add one in the account settings page.' ); + await expect(respPromise).rejects.toThrow(expectErr); // If the user cancels the MFA attempt and closes the dialog, the mfa status // should be 'success', or else the dialog would remain open to display the error. diff --git a/web/packages/teleport/src/lib/useMfa.ts b/web/packages/teleport/src/lib/useMfa.ts index af1e3d531330e..ca5e97b020b16 100644 --- a/web/packages/teleport/src/lib/useMfa.ts +++ b/web/packages/teleport/src/lib/useMfa.ts @@ -108,7 +108,11 @@ export function useMfa({ req, isMfaRequired }: MfaProps): MfaState { const cancelAttempt = () => { if (mfaResponseRef.current) { - mfaResponseRef.current.resolve(new Error('User canceled MFA attempt')); + mfaResponseRef.current.resolve( + new Error( + 'User cancelled MFA attempt. This is an admin-level API request and requires MFA verification. Please try again with a registered MFA device. If you do not have an MFA device registered, you can add one in the account settings page.' + ) + ); } }; From 4d489244d47cd39cc6943c360eb64f9865a0855e Mon Sep 17 00:00:00 2001 From: joerger Date: Wed, 8 Jan 2025 11:07:27 -0800 Subject: [PATCH 11/16] Remove mfaContext timeout logic, improve error message. --- web/packages/teleport/src/services/api/api.ts | 38 +++++++++---------- .../teleport/src/services/auth/auth.ts | 33 ++++++++-------- 2 files changed, 34 insertions(+), 37 deletions(-) diff --git a/web/packages/teleport/src/services/api/api.ts b/web/packages/teleport/src/services/api/api.ts index ba504a0d242c7..73e641accfb65 100644 --- a/web/packages/teleport/src/services/api/api.ts +++ b/web/packages/teleport/src/services/api/api.ts @@ -194,14 +194,7 @@ const api = { throw new ApiError(parseError(json), response, undefined, json.messages); } - let mfaResponseForRetry; - try { - mfaResponseForRetry = await api.getAdminActionMfaResponse(); - } catch { - throw new Error( - 'This is an admin-level API request and requires MFA verification. Please try again with a registered MFA device. If you do not have an MFA device registered, you can add one in the account settings page.' - ); - } + const mfaResponseForRetry = await api.getAdminActionMfaResponse(); return api.fetchJsonWithMfaAuthnRetry( url, @@ -217,21 +210,24 @@ const api = { // before this has a chance of being called). This conditional is not expected to // be hit, but will catch any major issues that could arise from this solution. if (!mfaContext) { - setTimeout(() => { - if (!mfaContext) - throw new Error( - 'Failed to set up MFA prompt for admin action. This is a bug.' - ); - }, 1000); + throw new Error( + 'Failed to set up MFA prompt for admin action. Please try refreshing the page to try again. If the issue persists, contact support as this is likely a bug.' + ); } - return mfaContext.getMfaChallengeResponse({ - scope: MfaChallengeScope.ADMIN_ACTION, - allowReuse, - isMfaRequiredRequest: { - admin_action: {}, - }, - }); + try { + return mfaContext.getMfaChallengeResponse({ + scope: MfaChallengeScope.ADMIN_ACTION, + allowReuse, + isMfaRequiredRequest: { + admin_action: {}, + }, + }); + } catch { + throw new Error( + 'This is an admin-level API request and requires MFA verification. Please try again with a registered MFA device. If you do not have an MFA device registered, you can add one in the account settings page.' + ); + } }, /** diff --git a/web/packages/teleport/src/services/auth/auth.ts b/web/packages/teleport/src/services/auth/auth.ts index 5520c793d75bd..abaa78bd02e8c 100644 --- a/web/packages/teleport/src/services/auth/auth.ts +++ b/web/packages/teleport/src/services/auth/auth.ts @@ -17,6 +17,7 @@ */ import cfg from 'teleport/config'; +import { MfaContextValue } from 'teleport/MFAContext/MFAContext'; import api from 'teleport/services/api'; import { DeviceType, @@ -25,8 +26,6 @@ import { MfaChallengeResponse, SsoChallenge, } from 'teleport/services/mfa'; - -import { MfaContextValue } from 'teleport/MFAContext/MFAContext'; import { CaptureEvent, userEventService } from 'teleport/services/userEvent'; import { @@ -35,7 +34,6 @@ import { parseMfaChallengeJson, parseMfaRegistrationChallengeJson, } from '../mfa/makeMfa'; - import { makeChangedUserAuthn } from './make'; import makePasswordToken from './makePasswordToken'; import { @@ -443,21 +441,24 @@ const auth = { // before this has a chance of being called). This conditional is not expected to // be hit, but will catch any major issues that could arise from this solution. if (!mfaContext) { - setTimeout(() => { - if (!mfaContext) - throw new Error( - 'Failed to set up MFA prompt for admin action. This is a bug.' - ); - }, 1000); + throw new Error( + 'Failed to set up MFA prompt for admin action. Please try refreshing the page to try again. If the issue persists, contact support as this is likely a bug.' + ); } - return mfaContext.getMfaChallengeResponse({ - scope: MfaChallengeScope.ADMIN_ACTION, - allowReuse, - isMfaRequiredRequest: { - admin_action: {}, - }, - }); + try { + return mfaContext.getMfaChallengeResponse({ + scope: MfaChallengeScope.ADMIN_ACTION, + allowReuse, + isMfaRequiredRequest: { + admin_action: {}, + }, + }); + } catch { + throw new Error( + 'This is an admin-level API request and requires MFA verification. Please try again with a registered MFA device. If you do not have an MFA device registered, you can add one in the account settings page.' + ); + } }, // TODO(Joerger): Delete in favor of getMfaChallengeResponseForAdminAction once /e is updated. From 656f425147e51da149be29b6d76332cb9835f771 Mon Sep 17 00:00:00 2001 From: joerger Date: Wed, 8 Jan 2025 11:12:31 -0800 Subject: [PATCH 12/16] Use async/await. --- web/packages/teleport/src/Users/useUsers.ts | 17 ++++-- .../src/services/integrations/integrations.ts | 33 +++++------ .../src/services/joinToken/joinToken.ts | 59 +++++++++---------- 3 files changed, 54 insertions(+), 55 deletions(-) diff --git a/web/packages/teleport/src/Users/useUsers.ts b/web/packages/teleport/src/Users/useUsers.ts index 404ea58fe252b..3716e859b9ca3 100644 --- a/web/packages/teleport/src/Users/useUsers.ts +++ b/web/packages/teleport/src/Users/useUsers.ts @@ -89,12 +89,17 @@ export default function useUsers({ async function onCreate(u: User) { const mfaResponse = await auth.getAdminActionMfaResponse(true); - return ctx.userService - .createUser(u, ExcludeUserField.Traits, mfaResponse) - .then(result => setUsers([result, ...users])) - .then(() => - ctx.userService.createResetPasswordToken(u.name, 'invite', mfaResponse) - ); + const result = await ctx.userService.createUser( + u, + ExcludeUserField.Traits, + mfaResponse + ); + setUsers([result, ...users]); + return ctx.userService.createResetPasswordToken( + u.name, + 'invite', + mfaResponse + ); } function onInviteCollaboratorsClose() { diff --git a/web/packages/teleport/src/services/integrations/integrations.ts b/web/packages/teleport/src/services/integrations/integrations.ts index ff78b8a90f2d4..8ae24bfb44192 100644 --- a/web/packages/teleport/src/services/integrations/integrations.ts +++ b/web/packages/teleport/src/services/integrations/integrations.ts @@ -23,7 +23,6 @@ import { App } from '../apps'; import makeApp from '../apps/makeApps'; import auth from '../auth/auth'; import makeNode from '../nodes/makeNode'; - import { AwsDatabaseVpcsResponse, AwsOidcDeployDatabaseServicesRequest, @@ -272,15 +271,14 @@ export const integrationService = { integrationName, req: AwsOidcDeployServiceRequest ): Promise { - const response = await auth.getAdminActionMfaResponse(true); - return api - .post( - cfg.getAwsDeployTeleportServiceUrl(integrationName), - req, - null, - response - ) - .then(resp => resp.serviceDashboardUrl); + const mfaResp = await auth.getAdminActionMfaResponse(true); + const resp = await api.post( + cfg.getAwsDeployTeleportServiceUrl(integrationName), + req, + null, + mfaResp + ); + return resp.serviceDashboardUrl; }, async createAwsAppAccess(integrationName): Promise { @@ -294,14 +292,13 @@ export const integrationService = { req: AwsOidcDeployDatabaseServicesRequest ): Promise { const mfaResponse = await auth.getAdminActionMfaResponse(true); - return api - .post( - cfg.getAwsRdsDbsDeployServicesUrl(integrationName), - req, - null, - mfaResponse - ) - .then(resp => resp.clusterDashboardUrl); + const resp = await api.post( + cfg.getAwsRdsDbsDeployServicesUrl(integrationName), + req, + null, + mfaResponse + ); + return resp.clusterDashboardUrl; }, async enrollEksClusters( diff --git a/web/packages/teleport/src/services/joinToken/joinToken.ts b/web/packages/teleport/src/services/joinToken/joinToken.ts index 721dd586548bc..298742acec108 100644 --- a/web/packages/teleport/src/services/joinToken/joinToken.ts +++ b/web/packages/teleport/src/services/joinToken/joinToken.ts @@ -33,21 +33,20 @@ class JoinTokenService { signal: AbortSignal = null ): Promise { const mfaResponse = await auth.getAdminActionMfaResponse(); - return api - .post( - cfg.getJoinTokenUrl(), - { - roles: req.roles, - join_method: req.method || 'token', - allow: makeAllowField(req.rules || []), - suggested_agent_matcher_labels: makeLabelMapOfStrArrs( - req.suggestedAgentMatcherLabels - ), - }, - signal, - mfaResponse - ) - .then(makeJoinToken); + const resp = await api.post( + cfg.getJoinTokenUrl(), + { + roles: req.roles, + join_method: req.method || 'token', + allow: makeAllowField(req.rules || []), + suggested_agent_matcher_labels: makeLabelMapOfStrArrs( + req.suggestedAgentMatcherLabels + ), + }, + signal, + mfaResponse + ); + return makeJoinToken(resp); } async upsertJoinTokenYAML( @@ -55,26 +54,24 @@ class JoinTokenService { tokenName: string ): Promise { const mfaResponse = await auth.getAdminActionMfaResponse(); - return api - .putWithHeaders( - cfg.getJoinTokenYamlUrl(), - { - content: req.content, - }, - { - [TeleportTokenNameHeader]: tokenName, - 'Content-Type': 'application/json', - }, - mfaResponse - ) - .then(makeJoinToken); + const resp = await api.putWithHeaders( + cfg.getJoinTokenYamlUrl(), + { + content: req.content, + }, + { + [TeleportTokenNameHeader]: tokenName, + 'Content-Type': 'application/json', + }, + mfaResponse + ); + return makeJoinToken(resp); } async createJoinToken(req: JoinTokenRequest): Promise { const mfaResponse = await auth.getAdminActionMfaResponse(); - return api - .post(cfg.getJoinTokensUrl(), req, mfaResponse) - .then(makeJoinToken); + const resp = api.post(cfg.getJoinTokensUrl(), req, mfaResponse); + return makeJoinToken(resp); } async fetchJoinTokens( From aded158ed3d6196e989bf613501aff78c19c63aa Mon Sep 17 00:00:00 2001 From: joerger Date: Wed, 8 Jan 2025 11:14:21 -0800 Subject: [PATCH 13/16] Use initializer function for mfaCtx. --- web/packages/teleport/src/MFAContext/MFAContext.tsx | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/web/packages/teleport/src/MFAContext/MFAContext.tsx b/web/packages/teleport/src/MFAContext/MFAContext.tsx index 3379eaa1891aa..fcb83604bb900 100644 --- a/web/packages/teleport/src/MFAContext/MFAContext.tsx +++ b/web/packages/teleport/src/MFAContext/MFAContext.tsx @@ -38,14 +38,12 @@ export const MfaContextProvider = ({ children }: PropsWithChildren) => { [adminMfa] ); - const [mfaCtx, setMfaCtx] = useState(); - - if (!mfaCtx) { + const [mfaCtx] = useState(() => { const mfaCtx = { getMfaChallengeResponse }; - setMfaCtx(mfaCtx); auth.setMfaContext(mfaCtx); api.setMfaContext(mfaCtx); - } + return mfaCtx; + }); return ( From b5581df3fdfaed2f84da2ca8b9206e485eb90102 Mon Sep 17 00:00:00 2001 From: joerger Date: Wed, 8 Jan 2025 11:27:02 -0800 Subject: [PATCH 14/16] Fix lint. --- web/packages/teleport/src/Teleport.tsx | 11 ++++------- .../teleport/src/services/joinToken/joinToken.test.ts | 2 +- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/web/packages/teleport/src/Teleport.tsx b/web/packages/teleport/src/Teleport.tsx index f192b6948bb39..8dce3f6460c0d 100644 --- a/web/packages/teleport/src/Teleport.tsx +++ b/web/packages/teleport/src/Teleport.tsx @@ -38,16 +38,13 @@ import { LoginClose } from './Login/LoginClose'; import { LoginFailedComponent as LoginFailed } from './Login/LoginFailed'; import { LoginSuccess } from './Login/LoginSuccess'; import { LoginTerminalRedirect } from './Login/LoginTerminalRedirect'; -import { SingleLogoutFailed } from './SingleLogoutFailed'; -import { Welcome } from './Welcome'; - -import { Player } from './Player'; - -import { MfaContextProvider } from './MFAContext/MFAContext'; - import { Main } from './Main'; +import { MfaContextProvider } from './MFAContext/MFAContext'; +import { Player } from './Player'; +import { SingleLogoutFailed } from './SingleLogoutFailed'; import TeleportContext from './teleportContext'; import TeleportContextProvider from './TeleportContextProvider'; +import { Welcome } from './Welcome'; const Teleport: React.FC = props => { const { ctx, history } = props; diff --git a/web/packages/teleport/src/services/joinToken/joinToken.test.ts b/web/packages/teleport/src/services/joinToken/joinToken.test.ts index 54b678b3f3b69..a748a55cbbcdd 100644 --- a/web/packages/teleport/src/services/joinToken/joinToken.test.ts +++ b/web/packages/teleport/src/services/joinToken/joinToken.test.ts @@ -20,9 +20,9 @@ import cfg from 'teleport/config'; import api from 'teleport/services/api'; import auth from '../auth/auth'; - import JoinTokenService from './joinToken'; import type { JoinTokenRequest } from './types'; + test('fetchJoinToken with an empty request properly sets defaults', async () => { const svc = new JoinTokenService(); jest.spyOn(api, 'post').mockResolvedValue(null); From 1024b38f64dc52ac41954d5d4f8c7a1ae6695611 Mon Sep 17 00:00:00 2001 From: joerger Date: Wed, 8 Jan 2025 12:35:41 -0800 Subject: [PATCH 15/16] Minor fixes. --- web/packages/teleport/src/JoinTokens/JoinTokens.tsx | 2 +- web/packages/teleport/src/services/auth/auth.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/web/packages/teleport/src/JoinTokens/JoinTokens.tsx b/web/packages/teleport/src/JoinTokens/JoinTokens.tsx index 43cac62115b21..4f53a90e25a9d 100644 --- a/web/packages/teleport/src/JoinTokens/JoinTokens.tsx +++ b/web/packages/teleport/src/JoinTokens/JoinTokens.tsx @@ -75,7 +75,7 @@ export const JoinTokens = () => { const [editingToken, setEditingToken] = useState(null); const [tokenToDelete, setTokenToDelete] = useState(null); const [joinTokensAttempt, runJoinTokensAttempt, setJoinTokensAttempt] = - useAsync(() => ctx.joinTokenService.fetchJoinTokens(null)); + useAsync(() => ctx.joinTokenService.fetchJoinTokens()); const resources = useResources( joinTokensAttempt.data?.items.map(makeTokenResource) || [], diff --git a/web/packages/teleport/src/services/auth/auth.ts b/web/packages/teleport/src/services/auth/auth.ts index abaa78bd02e8c..3f2af5aecf88e 100644 --- a/web/packages/teleport/src/services/auth/auth.ts +++ b/web/packages/teleport/src/services/auth/auth.ts @@ -461,7 +461,7 @@ const auth = { } }, - // TODO(Joerger): Delete in favor of getMfaChallengeResponseForAdminAction once /e is updated. + // TODO(Joerger): Delete in favor of getAdminActionMfaResponse once /e is updated. getWebauthnResponseForAdminAction(allowReuse?: boolean) { return auth.getAdminActionMfaResponse(allowReuse); }, From 9db6d2b5652e43afa9cf8260f45db25efa86d338 Mon Sep 17 00:00:00 2001 From: joerger Date: Wed, 8 Jan 2025 12:47:56 -0800 Subject: [PATCH 16/16] Fix test by using waitFor. --- .../Kubernetes/EnrollEKSCluster/EnrollEKSCluster.test.tsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/web/packages/teleport/src/Discover/Kubernetes/EnrollEKSCluster/EnrollEKSCluster.test.tsx b/web/packages/teleport/src/Discover/Kubernetes/EnrollEKSCluster/EnrollEKSCluster.test.tsx index 7cfb7dfd93b7d..75bbc151d6181 100644 --- a/web/packages/teleport/src/Discover/Kubernetes/EnrollEKSCluster/EnrollEKSCluster.test.tsx +++ b/web/packages/teleport/src/Discover/Kubernetes/EnrollEKSCluster/EnrollEKSCluster.test.tsx @@ -16,7 +16,7 @@ * along with this program. If not, see . */ -import { act, fireEvent, render, screen } from 'design/utils/testing'; +import { act, fireEvent, render, screen, waitFor } from 'design/utils/testing'; import cfg from 'teleport/config'; import { ComponentWrapper } from 'teleport/Discover/Fixtures/kubernetes'; @@ -179,6 +179,7 @@ describe('test EnrollEksCluster.tsx', () => { expect(integrationService.enrollEksClusters).not.toHaveBeenCalled(); }); + test('auto enroll disabled, enrolls cluster', async () => { jest.spyOn(integrationService, 'fetchEksClusters').mockResolvedValue({ clusters: mockEKSClusters, @@ -197,7 +198,9 @@ describe('test EnrollEksCluster.tsx', () => { act(() => screen.getByRole('radio').click()); - act(() => screen.getByText('Enroll EKS Cluster').click()); + await waitFor(() => { + screen.getByText('Enroll EKS Cluster').click(); + }); expect(discoveryService.createDiscoveryConfig).not.toHaveBeenCalled(); expect(KubeService.prototype.fetchKubernetes).toHaveBeenCalledTimes(1);