diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index d674d094248c8..d4f97c550ff12 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -912,7 +912,10 @@ func (h *Handler) bindDefaultEndpoints() { // MFA private endpoints. h.GET("/webapi/mfa/devices", h.WithAuth(h.getMFADevicesHandle)) + h.DELETE("/webapi/mfa/devices", h.WithAuth(h.deleteMFADeviceHandle)) h.POST("/webapi/mfa/authenticatechallenge", h.WithAuth(h.createAuthenticateChallengeHandle)) + h.POST("/webapi/mfa/registerchallenge", h.WithAuth(h.createRegisterChallengeHandle)) + h.POST("/webapi/mfa/devices", h.WithAuth(h.addMFADeviceHandle)) // DEPRECATED in favor of mfa/authenticatechallenge. // TODO(bl-nero): DELETE IN 17.0.0 diff --git a/lib/web/apiserver_test.go b/lib/web/apiserver_test.go index 5b78a5cd60c54..50eb30f4a4f37 100644 --- a/lib/web/apiserver_test.go +++ b/lib/web/apiserver_test.go @@ -5245,24 +5245,24 @@ func TestCreateRegisterChallenge(t *testing.T) { tests := []struct { name string - req *createRegisterChallengeRequest + req *createRegisterChallengeWithTokenRequest assertChallenge func(t *testing.T, c *client.MFARegisterChallenge) }{ { name: "totp", - req: &createRegisterChallengeRequest{ + req: &createRegisterChallengeWithTokenRequest{ DeviceType: "totp", }, }, { name: "webauthn", - req: &createRegisterChallengeRequest{ + req: &createRegisterChallengeWithTokenRequest{ DeviceType: "webauthn", }, }, { name: "passwordless", - req: &createRegisterChallengeRequest{ + req: &createRegisterChallengeWithTokenRequest{ DeviceType: "webauthn", DeviceUsage: "passwordless", }, diff --git a/lib/web/mfa.go b/lib/web/mfa.go index 9116966a9ed40..2ab9bfa281636 100644 --- a/lib/web/mfa.go +++ b/lib/web/mfa.go @@ -75,6 +75,41 @@ func (h *Handler) deleteMFADeviceWithTokenHandle(w http.ResponseWriter, r *http. return OK(), nil } +type deleteMfaDeviceRequest struct { + // DeviceName is the name of the device to delete. + DeviceName string `json:"deviceName"` + // ExistingMFAResponse is an MFA challenge response from an existing device. + // Not required if the user has no existing devices. + ExistingMFAResponse *client.MFAChallengeResponse `json:"existingMfaResponse"` +} + +// deleteMFADeviceHandle deletes an mfa device for the user defined in the `token`, given as a query parameter. +func (h *Handler) deleteMFADeviceHandle(w http.ResponseWriter, r *http.Request, p httprouter.Params, c *SessionContext) (interface{}, error) { + var req deleteMfaDeviceRequest + if err := httplib.ReadJSON(r, &req); err != nil { + return nil, trace.Wrap(err) + } + + mfaResponse, err := req.ExistingMFAResponse.GetOptionalMFAResponseProtoReq() + if err != nil { + return nil, trace.Wrap(err) + } + + clt, err := c.GetClient() + if err != nil { + return nil, trace.Wrap(err) + } + + if err := clt.DeleteMFADeviceSync(r.Context(), &proto.DeleteMFADeviceSyncRequest{ + DeviceName: req.DeviceName, + ExistingMFAResponse: mfaResponse, + }); err != nil { + return nil, trace.Wrap(err) + } + + return OK(), nil +} + type addMFADeviceRequest struct { // PrivilegeTokenID is privilege token id. PrivilegeTokenID string `json:"tokenId"` @@ -203,7 +238,7 @@ func (h *Handler) createAuthenticateChallengeWithTokenHandle(w http.ResponseWrit return makeAuthenticateChallenge(chal), nil } -type createRegisterChallengeRequest struct { +type createRegisterChallengeWithTokenRequest struct { // DeviceType is the type of MFA device to get a register challenge for. DeviceType string `json:"deviceType"` // DeviceUsage is the intended usage of the device (MFA, Passwordless, etc). @@ -214,7 +249,7 @@ type createRegisterChallengeRequest struct { // createRegisterChallengeWithTokenHandle creates and returns MFA register challenges for a new device for the specified device type. func (h *Handler) createRegisterChallengeWithTokenHandle(w http.ResponseWriter, r *http.Request, p httprouter.Params) (interface{}, error) { - var req createRegisterChallengeRequest + var req createRegisterChallengeWithTokenRequest if err := httplib.ReadJSON(r, &req); err != nil { return nil, trace.Wrap(err) } @@ -256,6 +291,72 @@ func (h *Handler) createRegisterChallengeWithTokenHandle(w http.ResponseWriter, return resp, nil } +type createRegisterChallengeRequest struct { + // DeviceType is the type of MFA device to get a register challenge for. + DeviceType string `json:"deviceType"` + // DeviceUsage is the intended usage of the device (MFA, Passwordless, etc). + // It mimics the proto.DeviceUsage enum. + // Defaults to MFA. + DeviceUsage string `json:"deviceUsage"` + // ExistingMFAResponse is an MFA challenge response from an existing device. + // Not required if the user has no existing devices. + ExistingMFAResponse *client.MFAChallengeResponse `json:"existingMfaResponse"` +} + +// createRegisterChallengeHandle creates and returns MFA register challenges for a new device for the specified device type. +func (h *Handler) createRegisterChallengeHandle(w http.ResponseWriter, r *http.Request, p httprouter.Params, c *SessionContext) (interface{}, error) { + var req createRegisterChallengeRequest + if err := httplib.ReadJSON(r, &req); err != nil { + return nil, trace.Wrap(err) + } + + var deviceType proto.DeviceType + switch req.DeviceType { + case "totp": + deviceType = proto.DeviceType_DEVICE_TYPE_TOTP + case "webauthn": + deviceType = proto.DeviceType_DEVICE_TYPE_WEBAUTHN + default: + return nil, trace.BadParameter("MFA device type %q unsupported", req.DeviceType) + } + + deviceUsage, err := getDeviceUsage(req.DeviceUsage) + if err != nil { + return nil, trace.Wrap(err) + } + + mfaResponse, err := req.ExistingMFAResponse.GetOptionalMFAResponseProtoReq() + if err != nil { + return nil, trace.Wrap(err) + } + + clt, err := c.GetClient() + if err != nil { + return nil, trace.Wrap(err) + } + + chal, err := clt.CreateRegisterChallenge(r.Context(), &proto.CreateRegisterChallengeRequest{ + DeviceType: deviceType, + DeviceUsage: deviceUsage, + ExistingMFAResponse: mfaResponse, + }) + if err != nil { + return nil, trace.Wrap(err) + } + + resp := &client.MFARegisterChallenge{} + switch chal.GetRequest().(type) { + case *proto.MFARegisterChallenge_TOTP: + resp.TOTP = &client.TOTPRegisterChallenge{ + QRCode: chal.GetTOTP().GetQRCode(), + } + case *proto.MFARegisterChallenge_Webauthn: + resp.Webauthn = wantypes.CredentialCreationFromProto(chal.GetWebauthn()) + } + + return resp, nil +} + func getDeviceUsage(reqUsage string) (proto.DeviceUsage, error) { var deviceUsage proto.DeviceUsage switch strings.ToLower(reqUsage) { diff --git a/web/packages/teleport/src/Account/Account.tsx b/web/packages/teleport/src/Account/Account.tsx index c2109fb51dd9d..db0fbff3d46a6 100644 --- a/web/packages/teleport/src/Account/Account.tsx +++ b/web/packages/teleport/src/Account/Account.tsx @@ -105,14 +105,12 @@ export interface AccountProps extends ManageDevicesState, AccountPageProps { export function Account({ devices, - token, onAddDevice, onRemoveDevice, onDeviceAdded, onDeviceRemoved, deviceToRemove, fetchDevicesAttempt, - createRestrictedTokenAttempt, addDeviceWizardVisible, hideRemoveDevice, closeAddDeviceWizard, @@ -127,11 +125,8 @@ export function Account({ }: AccountProps) { const passkeys = devices.filter(d => d.usage === 'passwordless'); const mfaDevices = devices.filter(d => d.usage === 'mfa'); - const disableAddDevice = - createRestrictedTokenAttempt.status === 'processing' || - fetchDevicesAttempt.status !== 'success'; - const disableAddPasskey = disableAddDevice || !canAddPasskeys; - const disableAddMfa = disableAddDevice || !canAddMfa; + const disableAddPasskey = !canAddPasskeys; + const disableAddMfa = !canAddMfa; let mfaPillState = undefined; if (fetchDevicesAttempt.status !== 'processing') { @@ -140,7 +135,6 @@ export function Account({ const [notifications, setNotifications] = useState([]); const [prevFetchStatus, setPrevFetchStatus] = useState(''); - const [prevTokenStatus, setPrevTokenStatus] = useState(''); function addNotification(severity: NotificationSeverity, content: string) { setNotifications(n => [ @@ -165,13 +159,6 @@ export function Account({ } } - if (prevTokenStatus !== createRestrictedTokenAttempt.status) { - setPrevTokenStatus(createRestrictedTokenAttempt.status); - if (createRestrictedTokenAttempt.status === 'failed') { - addNotification('error', createRestrictedTokenAttempt.statusText); - } - } - function onPasswordChange() { addNotification('info', 'Your password has been changed.'); onPasswordChangeCb(); @@ -220,9 +207,6 @@ export function Account({ {!isSso && ( diff --git a/web/packages/teleport/src/Account/ManageDevices/useManageDevices.ts b/web/packages/teleport/src/Account/ManageDevices/useManageDevices.ts index 7c6ec1917673f..d1eb02c6b7f59 100644 --- a/web/packages/teleport/src/Account/ManageDevices/useManageDevices.ts +++ b/web/packages/teleport/src/Account/ManageDevices/useManageDevices.ts @@ -28,7 +28,6 @@ import { MfaChallengeScope } from 'teleport/services/auth/auth'; export default function useManageDevices(ctx: Ctx) { const [devices, setDevices] = useState([]); const [deviceToRemove, setDeviceToRemove] = useState(); - const [token, setToken] = useState(''); const fetchDevicesAttempt = useAttempt(''); const [newDeviceUsage, setNewDeviceUsage] = useState('passwordless'); @@ -38,8 +37,6 @@ export default function useManageDevices(ctx: Ctx) { // the user has no devices yet and thus can't authenticate using the ReAuthenticate dialog const createRestrictedTokenAttempt = useAttempt(''); - const isReauthenticationRequired = !token; - function fetchDevices() { fetchDevicesAttempt.run(() => ctx.mfaService.fetchDevices().then(setDevices) @@ -48,30 +45,12 @@ export default function useManageDevices(ctx: Ctx) { async function onAddDevice(usage: DeviceUsage) { setNewDeviceUsage(usage); - const challenge = await auth.getMfaChallenge({ - scope: MfaChallengeScope.MANAGE_DEVICES, - }); - // If the user doesn't receieve any challenges from the backend, that means - // they have no valid devices to be challenged and should instead use a privilege token - // to add a new device. - // TODO (avatus): add SSO challenge here as well when we add SSO for MFA - // TODO(Joerger): privilege token is no longer required to add first device. - if (!challenge) { - createRestrictedTokenAttempt.run(() => - auth.createRestrictedPrivilegeToken().then(token => { - setToken(token); - setAddDeviceWizardVisible(true); - }) - ); - } else { - setAddDeviceWizardVisible(true); - } + setAddDeviceWizardVisible(true); } function onDeviceAdded() { fetchDevices(); setAddDeviceWizardVisible(false); - setToken(null); } function onRemoveDevice(device: MfaDevice) { @@ -95,7 +74,6 @@ export default function useManageDevices(ctx: Ctx) { return { devices, - token, onAddDevice, onRemoveDevice, onDeviceAdded, @@ -103,7 +81,6 @@ export default function useManageDevices(ctx: Ctx) { deviceToRemove, fetchDevicesAttempt: fetchDevicesAttempt.attempt, createRestrictedTokenAttempt: createRestrictedTokenAttempt.attempt, - isReauthenticationRequired, addDeviceWizardVisible, hideRemoveDevice, closeAddDeviceWizard, diff --git a/web/packages/teleport/src/Account/ManageDevices/wizards/AddAuthDeviceWizard.tsx b/web/packages/teleport/src/Account/ManageDevices/wizards/AddAuthDeviceWizard.tsx index bd0291736e037..6a029fd78c7d5 100644 --- a/web/packages/teleport/src/Account/ManageDevices/wizards/AddAuthDeviceWizard.tsx +++ b/web/packages/teleport/src/Account/ManageDevices/wizards/AddAuthDeviceWizard.tsx @@ -62,24 +62,18 @@ interface AddAuthDeviceWizardProps { usage: DeviceUsage; /** MFA type setting, as configured in the cluster's configuration. */ auth2faType: Auth2faType; - /** - * A privilege token that may have been created previously; if present, the - * reauthentication step will be skipped. - */ - privilegeToken?: string; onClose(): void; onSuccess(): void; } /** A wizard for adding MFA and passkey devices. */ export function AddAuthDeviceWizard({ - privilegeToken: privilegeTokenProp = '', usage, auth2faType, onClose, onSuccess, }: AddAuthDeviceWizardProps) { - const [privilegeToken, setPrivilegeToken] = useState(privilegeTokenProp); + const [privilegeToken, setPrivilegeToken] = useState(); const [credential, setCredential] = useState(null); const { attempt, clearAttempt, getMfaChallengeOptions, submitWithMfa } = @@ -101,7 +95,20 @@ export function AddAuthDeviceWizard({ // empty, the user has no existing device (e.g. SSO user) and can register their // first device without re-authentication. const [reauthMfaOptions, getMfaOptions] = useAsync(async () => { - return getMfaChallengeOptions(); + const reauthMfaOptions = await getMfaChallengeOptions(); + + // registering first device does not require reauth, just get a privilege token. + // + // TODO(Joerger): v19.0.0 + // Registering first device does not require a privilege token anymore, + // but the existing web register endpoint requires privilege token. + // We have a new endpoint "/v1/webapi/users/privilege" which does not + // require token, but can't be used until v19 for backwards compatibility. + if (reauthMfaOptions.length === 0) { + await auth.createPrivilegeToken().then(setPrivilegeToken); + } + + return reauthMfaOptions; }); useEffect(() => { @@ -113,7 +120,6 @@ export function AddAuthDeviceWizard({ case 'processing': return ( -
hi there
 ); diff --git a/web/packages/teleport/src/Account/ManageDevices/wizards/DeleteAuthDeviceWizard.tsx b/web/packages/teleport/src/Account/ManageDevices/wizards/DeleteAuthDeviceWizard.tsx index dcd6f4d910e15..0ee99393df36b 100644 --- a/web/packages/teleport/src/Account/ManageDevices/wizards/DeleteAuthDeviceWizard.tsx +++ b/web/packages/teleport/src/Account/ManageDevices/wizards/DeleteAuthDeviceWizard.tsx @@ -64,6 +64,11 @@ export function DeleteAuthDeviceWizard({ useReAuthenticate({ challengeScope: MfaChallengeScope.MANAGE_DEVICES, onMfaResponse: mfaResponse => { + // TODO(Joerger): v19.0.0 + // Devices can be deleted with an MFA response, so exchanging it for a + // privilege token adds an unnecessary API call. The device deletion + // endpoint requires a token, but the new endpoint "DELETE: /webapi/mfa/devices" + // can be used after v19 backwards compatibly. auth.createPrivilegeToken(mfaResponse).then(setPrivilegeToken); }, }); diff --git a/web/packages/teleport/src/Account/PasswordBox.tsx b/web/packages/teleport/src/Account/PasswordBox.tsx index bef6f8220745a..dfb0c7dae981e 100644 --- a/web/packages/teleport/src/Account/PasswordBox.tsx +++ b/web/packages/teleport/src/Account/PasswordBox.tsx @@ -33,14 +33,12 @@ import { ChangePasswordWizard } from './ChangePasswordWizard'; import { StatePill, AuthMethodState } from './StatePill'; export interface PasswordBoxProps { - changeDisabled: boolean; devices: MfaDevice[]; passwordState: PasswordState; onPasswordChange: () => void; } export function PasswordBox({ - changeDisabled, devices, passwordState, onPasswordChange, @@ -67,10 +65,7 @@ export function PasswordBox({ } icon={} actions={ - setDialogOpen(true)} - > + setDialogOpen(true)}> Change Password } diff --git a/web/packages/teleport/src/services/auth/auth.ts b/web/packages/teleport/src/services/auth/auth.ts index c6c9ea277846d..45cadd4a8fdf0 100644 --- a/web/packages/teleport/src/services/auth/auth.ts +++ b/web/packages/teleport/src/services/auth/auth.ts @@ -323,7 +323,7 @@ const auth = { ); }, - createPrivilegeToken(existingMfaResponse: MfaChallengeResponse) { + createPrivilegeToken(existingMfaResponse?: MfaChallengeResponse) { return api.post(cfg.api.createPrivilegeTokenPath, { existingMfaResponse, // TODO(Joerger): DELETE IN v19.0.0