Skip to content

Commit

Permalink
Handle SSO mfa devices in deletion flow.
Browse files Browse the repository at this point in the history
  • Loading branch information
Joerger committed Sep 21, 2024
1 parent b35e1ac commit 2e155d3
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 20 deletions.
40 changes: 21 additions & 19 deletions lib/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -3829,13 +3829,7 @@ func (a *Server) deleteMFADeviceSafely(ctx context.Context, user, deviceName str
return nil, trace.Wrap(err)
}

kindToSF := map[string]constants.SecondFactorType{
fmt.Sprintf("%T", &types.MFADevice_Totp{}): constants.SecondFactorOTP,
fmt.Sprintf("%T", &types.MFADevice_U2F{}): constants.SecondFactorWebauthn,
fmt.Sprintf("%T", &types.MFADevice_Webauthn{}): constants.SecondFactorWebauthn,
}
sfToCount := make(map[constants.SecondFactorType]int)
var knownDevices int
knownDevices := make(map[constants.SecondFactorType]int)
var deviceToDelete *types.MFADevice
var numResidentKeys int

Expand All @@ -3850,18 +3844,26 @@ func (a *Server) deleteMFADeviceSafely(ctx context.Context, user, deviceName str
deviceToDelete = d
}

sf, ok := kindToSF[fmt.Sprintf("%T", d.Device)]
switch {
case !ok && d == deviceToDelete:
return nil, trace.NotImplemented("cannot delete device of type %T", d.Device)
case !ok:
var sfType constants.SecondFactorType
switch d.Device.(type) {
case *types.MFADevice_Totp:
sfType = constants.SecondFactorOTP
case *types.MFADevice_U2F, *types.MFADevice_Webauthn:
sfType = constants.SecondFactorWebauthn
case *types.MFADevice_Sso:
// TODO(Joerger): sso will not be supported in the current `second_factor` option.
// It will be supported in the new `second_factors` option instead, which will not
// use the old constants.SecondFactorType type. This will be handled in a separate PR.
sfType = "sso"
if d == deviceToDelete {
return nil, trace.BadParameter("cannot delete ephemeral SSO MFA device")
}
default:
log.Warnf("Ignoring unknown device with type %T in deletion.", d.Device)
continue
}

sfToCount[sf]++
knownDevices++

knownDevices[sfType]++
if isResidentKey(d) {
numResidentKeys++
}
Expand All @@ -3875,12 +3877,12 @@ func (a *Server) deleteMFADeviceSafely(ctx context.Context, user, deviceName str
switch sf := readOnlyAuthPref.GetSecondFactor(); sf {
case constants.SecondFactorOff, constants.SecondFactorOptional: // MFA is not required, allow deletion
case constants.SecondFactorOn:
if knownDevices <= minDevices {
if len(knownDevices) <= minDevices {
return nil, trace.BadParameter(
"cannot delete the last MFA device for this user; add a replacement device first to avoid getting locked out")
}
case constants.SecondFactorOTP, constants.SecondFactorWebauthn:
if sfToCount[sf] <= minDevices {
if knownDevices[sf] <= minDevices {
return nil, trace.BadParameter(
"cannot delete the last %s device for this user; add a replacement device first to avoid getting locked out", sf)
}
Expand Down Expand Up @@ -3909,15 +3911,15 @@ func (a *Server) deleteMFADeviceSafely(ctx context.Context, user, deviceName str

// Minimum number of WebAuthn devices includes the passkey that we attempt
// to delete, hence 2.
if sfToCount[constants.SecondFactorWebauthn] >= 2 {
if knownDevices[constants.SecondFactorWebauthn] >= 2 {
return true, nil
}

// Whether we take TOTPs into consideration or not depends on whether it's
// enabled.
switch sf := readOnlyAuthPref.GetSecondFactor(); sf {
case constants.SecondFactorOTP, constants.SecondFactorOn, constants.SecondFactorOptional:
if sfToCount[constants.SecondFactorOTP] >= 1 {
if knownDevices[constants.SecondFactorOTP] >= 1 {
return true, nil
}
}
Expand Down
47 changes: 46 additions & 1 deletion lib/auth/grpcserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ func TestMFADeviceManagement(t *testing.T) {
// 2nd-to-last resident credential.
// This is already tested above so we just use RegisterTestDevice here.
const pwdless2DevName = "pwdless2"
_, err = RegisterTestDevice(ctx, userClient, pwdless2DevName, proto.DeviceType_DEVICE_TYPE_WEBAUTHN, devs.WebDev, WithPasswordless())
pwdlessDev, err := RegisterTestDevice(ctx, userClient, pwdless2DevName, proto.DeviceType_DEVICE_TYPE_WEBAUTHN, devs.WebDev, WithPasswordless())
require.NoError(t, err, "RegisterTestDevice failed")

// Check that all new devices are registered.
Expand Down Expand Up @@ -433,6 +433,51 @@ func TestMFADeviceManagement(t *testing.T) {
resp, err = userClient.GetMFADevices(ctx, &proto.GetMFADevicesRequest{})
require.NoError(t, err)
require.Equal(t, "pwdless2", resp.Devices[0].GetName())

// Change the user to an SSO user with an MFA enabled auth connector.
samlConnector, err := types.NewSAMLConnector("saml", types.SAMLConnectorSpecV2{
AssertionConsumerService: "http://localhost:65535/acs", // not called
Issuer: "test",
SSO: "https://localhost:65535/sso", // not called
AttributesToRoles: []types.AttributeMapping{
// not used. can be any name, value but role must exist
{Name: "groups", Value: "admin", Roles: user.GetRoles()},
},
MFASettings: &types.SAMLConnectorMFASettings{
Enabled: true,
},
})
require.NoError(t, err)
_, err = authServer.UpsertSAMLConnector(ctx, samlConnector)
require.NoError(t, err)
user.SetCreatedBy(types.CreatedBy{
Time: authServer.clock.Now(),
Connector: &types.ConnectorRef{
ID: "saml",
Type: "saml",
},
})
_, err = authServer.UpsertUser(ctx, user)
require.NoError(t, err)

// Ephemeral sso device should show up in the list now. It can't be deleted.
resp, err = userClient.GetMFADevices(ctx, &proto.GetMFADevicesRequest{})
require.NoError(t, err)
require.Len(t, resp.Devices, 2)

testDeleteMFADevice(ctx, t, userClient, mfaDeleteTestOpts{
deviceName: "saml",
authHandler: func(t *testing.T, challenge *proto.MFAAuthenticateChallenge) *proto.MFAAuthenticateResponse {
require.NotNil(t, challenge.WebauthnChallenge, "nil Webauthn challenge")
mfaResp, err := pwdlessDev.SolveAuthn(challenge)
require.NoError(t, err, "SolveAuthn")
return mfaResp
},
checkErr: func(t require.TestingT, err error, _ ...interface{}) {
require.ErrorAs(t, err, new(*trace.BadParameterError))
require.ErrorContains(t, err, "cannot delete ephemeral SSO MFA device")
}},
)
}

func TestDeletingLastPasswordlessDevice(t *testing.T) {
Expand Down

0 comments on commit 2e155d3

Please sign in to comment.