Skip to content

Commit

Permalink
Remove extra createPrivilegeToken call from the account page; Add new…
Browse files Browse the repository at this point in the history
… tokenless mfa endpoints to register/delete mfa devices; add TODOs to use new endpoints in v19+.
  • Loading branch information
Joerger committed Dec 10, 2024
1 parent de5b57f commit e60ec3d
Show file tree
Hide file tree
Showing 9 changed files with 135 additions and 65 deletions.
3 changes: 3 additions & 0 deletions lib/web/apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions lib/web/apiserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
Expand Down
105 changes: 103 additions & 2 deletions lib/web/mfa.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -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).
Expand All @@ -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)
}
Expand Down Expand Up @@ -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) {
Expand Down
21 changes: 2 additions & 19 deletions web/packages/teleport/src/Account/Account.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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') {
Expand All @@ -140,7 +135,6 @@ export function Account({

const [notifications, setNotifications] = useState<NotificationItem[]>([]);
const [prevFetchStatus, setPrevFetchStatus] = useState<Attempt['status']>('');
const [prevTokenStatus, setPrevTokenStatus] = useState<Attempt['status']>('');

function addNotification(severity: NotificationSeverity, content: string) {
setNotifications(n => [
Expand All @@ -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();
Expand Down Expand Up @@ -220,9 +207,6 @@ export function Account({
</Box>
{!isSso && (
<PasswordBox
changeDisabled={
createRestrictedTokenAttempt.status === 'processing'
}
devices={devices}
passwordState={passwordState}
onPasswordChange={onPasswordChange}
Expand Down Expand Up @@ -278,7 +262,6 @@ export function Account({
<AddAuthDeviceWizard
usage={newDeviceUsage}
auth2faType={cfg.getAuth2faType()}
privilegeToken={token}
onClose={closeAddDeviceWizard}
onSuccess={onAddDeviceSuccess}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import { MfaChallengeScope } from 'teleport/services/auth/auth';
export default function useManageDevices(ctx: Ctx) {
const [devices, setDevices] = useState<MfaDevice[]>([]);
const [deviceToRemove, setDeviceToRemove] = useState<MfaDevice>();
const [token, setToken] = useState('');
const fetchDevicesAttempt = useAttempt('');
const [newDeviceUsage, setNewDeviceUsage] =
useState<DeviceUsage>('passwordless');
Expand All @@ -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)
Expand All @@ -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) {
Expand All @@ -95,15 +74,13 @@ export default function useManageDevices(ctx: Ctx) {

return {
devices,
token,
onAddDevice,
onRemoveDevice,
onDeviceAdded,
onDeviceRemoved,
deviceToRemove,
fetchDevicesAttempt: fetchDevicesAttempt.attempt,
createRestrictedTokenAttempt: createRestrictedTokenAttempt.attempt,
isReauthenticationRequired,
addDeviceWizardVisible,
hideRemoveDevice,
closeAddDeviceWizard,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Credential>(null);

const { attempt, clearAttempt, getMfaChallengeOptions, submitWithMfa } =
Expand All @@ -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(() => {
Expand All @@ -113,7 +120,6 @@ export function AddAuthDeviceWizard({
case 'processing':
return (
<Box textAlign="center" m={10}>
<div>hi there</div>
<Indicator />
</Box>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
},
});
Expand Down
Loading

0 comments on commit e60ec3d

Please sign in to comment.