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 Oct 10, 2024
1 parent 320bf4f commit cb0491c
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 1 deletion.
4 changes: 4 additions & 0 deletions lib/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -3843,6 +3843,8 @@ func (a *Server) deleteMFADeviceSafely(ctx context.Context, user, deviceName str
deviceToDelete = d
switch d.Device.(type) {
case *types.MFADevice_Totp, *types.MFADevice_U2F, *types.MFADevice_Webauthn:
case *types.MFADevice_Sso:
return nil, trace.BadParameter("cannot delete ephemeral SSO MFA device")
default:
return nil, trace.NotImplemented("cannot delete device of type %T", d.Device)
}
Expand All @@ -3854,6 +3856,8 @@ func (a *Server) deleteMFADeviceSafely(ctx context.Context, user, deviceName str
remainingDevices[types.SecondFactorType_SECOND_FACTOR_TYPE_OTP]++
case *types.MFADevice_U2F, *types.MFADevice_Webauthn:
remainingDevices[types.SecondFactorType_SECOND_FACTOR_TYPE_WEBAUTHN]++
case *types.MFADevice_Sso:
remainingDevices[types.SecondFactorType_SECOND_FACTOR_TYPE_SSO]++
default:
log.Warnf("Ignoring unknown device with type %T in deletion.", d.Device)
continue
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 cb0491c

Please sign in to comment.