Use global mfa context for all admin actions with explicit MFA checks.

gravitational · Dec 18, 2024 · aa5c0b7 · aa5c0b7
1 parent 79f4ce9
commit aa5c0b7
Show file tree

Hide file tree

Showing 6 changed files with 66 additions and 112 deletions.
diff --git 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<MfaChallengeResponse>;
 }
@@ -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 (
     <MfaContext.Provider value={mfaCtx}>

diff --git a/web/packages/teleport/src/Users/useUsers.ts b/web/packages/teleport/src/Users/useUsers.ts
@@ -87,7 +87,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
@@ -19,22 +19,15 @@
 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,
@@ -196,7 +189,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
@@ -16,8 +16,8 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-import api from 'teleport/services/api';
 import cfg from 'teleport/config';
+import api from 'teleport/services/api';
 import {
   DeviceType,
   DeviceUsage,
@@ -26,27 +26,34 @@ import {
   SsoChallenge,
 } from 'teleport/services/mfa';
 
+import { MfaContextValue } from 'teleport/MFAContext/MFAContext';
 import { CaptureEvent, userEventService } from 'teleport/services/userEvent';
 
 import {
-  parseMfaChallengeJson,
-  parseMfaRegistrationChallengeJson,
   makeWebauthnAssertionResponse,
   makeWebauthnCreationResponse,
+  parseMfaChallengeJson,
+  parseMfaRegistrationChallengeJson,
 } from '../mfa/makeMfa';
 
-import makePasswordToken from './makePasswordToken';
 import { makeChangedUserAuthn } from './make';
+import makePasswordToken from './makePasswordToken';
 import {
+  ChangePasswordReq,
+  CreateAuthenticateChallengeRequest,
+  CreateNewHardwareDeviceRequest,
   ResetPasswordReqWithEvent,
   ResetPasswordWithWebauthnReqWithEvent,
   UserCredentials,
-  ChangePasswordReq,
-  CreateNewHardwareDeviceRequest,
-  CreateAuthenticateChallengeRequest,
 } from './types';
 
+let mfaContext: MfaContextValue;
+
 const auth = {
+  setMfaContext(mfa: MfaContextValue) {
+    mfaContext = mfa;
+  },
+
   checkWebauthnSupport() {
     if (window.PublicKeyCredential) {
       return Promise.resolve();
@@ -279,22 +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<MfaChallengeResponse> {
-    // 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
@@ -16,50 +16,50 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-import api from 'teleport/services/api';
 import cfg from 'teleport/config';
+import api from 'teleport/services/api';
 
-import makeNode from '../nodes/makeNode';
-import auth, { MfaChallengeScope } from '../auth/auth';
 import { App } from '../apps';
 import makeApp from '../apps/makeApps';
+import auth from '../auth/auth';
+import makeNode from '../nodes/makeNode';
 
 import {
-  Integration,
-  IntegrationCreateRequest,
-  IntegrationUpdateRequest,
-  IntegrationStatusCode,
-  IntegrationListResponse,
+  AwsDatabaseVpcsResponse,
+  AwsOidcDeployDatabaseServicesRequest,
+  AwsOidcDeployServiceRequest,
   AwsOidcListDatabasesRequest,
+  AwsOidcPingRequest,
+  AwsOidcPingResponse,
   AwsRdsDatabase,
-  ListAwsRdsDatabaseResponse,
-  RdsEngineIdentifier,
-  AwsOidcDeployServiceRequest,
-  ListEc2InstancesRequest,
-  ListEc2InstancesResponse,
-  Ec2InstanceConnectEndpoint,
-  ListEc2InstanceConnectEndpointsRequest,
-  ListEc2InstanceConnectEndpointsResponse,
-  ListAwsSecurityGroupsRequest,
-  ListAwsSecurityGroupsResponse,
   DeployEc2InstanceConnectEndpointRequest,
   DeployEc2InstanceConnectEndpointResponse,
-  SecurityGroup,
-  SecurityGroupRule,
-  ListEksClustersResponse,
-  EnrollEksClustersResponse,
+  Ec2InstanceConnectEndpoint,
   EnrollEksClustersRequest,
-  ListEksClustersRequest,
-  AwsOidcDeployDatabaseServicesRequest,
-  Regions,
+  EnrollEksClustersResponse,
+  Integration,
+  IntegrationCreateRequest,
+  IntegrationListResponse,
+  IntegrationStatusCode,
+  IntegrationUpdateRequest,
+  IntegrationWithSummary,
+  ListAwsRdsDatabaseResponse,
   ListAwsRdsFromAllEnginesResponse,
+  ListAwsSecurityGroupsRequest,
+  ListAwsSecurityGroupsResponse,
   ListAwsSubnetsRequest,
   ListAwsSubnetsResponse,
+  ListEc2InstanceConnectEndpointsRequest,
+  ListEc2InstanceConnectEndpointsResponse,
+  ListEc2InstancesRequest,
+  ListEc2InstancesResponse,
+  ListEksClustersRequest,
+  ListEksClustersResponse,
+  RdsEngineIdentifier,
+  Regions,
+  SecurityGroup,
+  SecurityGroupRule,
   Subnet,
-  AwsDatabaseVpcsResponse,
-  AwsOidcPingResponse,
-  AwsOidcPingRequest,
-  IntegrationWithSummary,
 } from './types';
 
 export const integrationService = {
@@ -272,16 +272,7 @@ export const integrationService = {
     integrationName,
     req: AwsOidcDeployServiceRequest
   ): Promise<string> {
-    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),
@@ -302,8 +293,7 @@ export const integrationService = {
     integrationName,
     req: AwsOidcDeployDatabaseServicesRequest
   ): Promise<string> {
-    const mfaResponse = await auth.getMfaChallengeResponseForAdminAction(true);
-
+    const mfaResponse = await auth.getAdminActionMfaResponse(true);
     return api
       .post(
         cfg.getAwsRdsDbsDeployServicesUrl(integrationName),
@@ -318,7 +308,7 @@ export const integrationService = {
     integrationName: string,
     req: EnrollEksClustersRequest
   ): Promise<EnrollEksClustersResponse> {
-    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
@@ -17,29 +17,23 @@
  */
 
 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';
 
 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<JoinToken> {
-    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<JoinToken> {
-    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<JoinToken> {
-    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 },