Skip to content

Commit

Permalink
Deprecate DeleteMFADevice and move tests to its "sync" variant (#32773)
Browse files Browse the repository at this point in the history
* Deprecate DeleteMFADevice and its top-level messages

* Update generated protos

* Deprecate api/client.Client.DeleteMFADevice

* Deprecate lib/auth.IdentityService.DeleteMFADevice

* Deprecate lib/auth.GRPCServer.DeleteMFADevice

* Combine "last device" tests

* TestMFADeviceManagement renames (no functional changes)

* Use DeleteDeviceMFASync on TestMFADeviceManagement

* Add a test for DeleteMFADevice

* Add deprecation nolints
  • Loading branch information
codingllama authored Oct 2, 2023
1 parent f391774 commit ba90737
Show file tree
Hide file tree
Showing 8 changed files with 1,101 additions and 1,106 deletions.
1 change: 1 addition & 0 deletions api/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -1598,6 +1598,7 @@ func (c *Client) AddMFADevice(ctx context.Context) (proto.AuthService_AddMFADevi
return stream, nil
}

// Deprecated: Use DeleteMFADeviceSync instead.
func (c *Client) DeleteMFADevice(ctx context.Context) (proto.AuthService_DeleteMFADeviceClient, error) {
stream, err := c.grpc.DeleteMFADevice(ctx)
if err != nil {
Expand Down
1,652 changes: 839 additions & 813 deletions api/client/proto/authservice.pb.go

Large diffs are not rendered by default.

19 changes: 18 additions & 1 deletion api/proto/teleport/legacy/client/proto/authservice.proto
Original file line number Diff line number Diff line change
Expand Up @@ -1174,6 +1174,8 @@ message AddMFADeviceResponseAck {

// DeleteMFADeviceRequest is a message sent by the client during
// DeleteMFADevice RPC.
//
// Deprecated: Use [AuthService.DeleteMFADeviceSync] instead.
message DeleteMFADeviceRequest {
oneof Request {
// Init describes the device to be deleted.
Expand All @@ -1183,6 +1185,7 @@ message DeleteMFADeviceRequest {
}
}

// Deprecated: Use [AuthService.DeleteMFADeviceSync] instead.
message DeleteMFADeviceResponse {
oneof Response {
// MFAChallenge is an auth challenge using any existing MFA device.
Expand All @@ -1193,6 +1196,8 @@ message DeleteMFADeviceResponse {
}

// DeleteMFADeviceRequestInit describes the device to be deleted.
//
// Deprecated: Use [AuthService.DeleteMFADeviceSync] instead.
message DeleteMFADeviceRequestInit {
// DeviceName is an MFA device name or ID to be deleted.
string DeviceName = 1;
Expand Down Expand Up @@ -2644,7 +2649,11 @@ service AuthService {
// <- MFAChallenge
// -> MFAResponse
// <- Ack
rpc DeleteMFADevice(stream DeleteMFADeviceRequest) returns (stream DeleteMFADeviceResponse);
//
// Deprecated: Use [DeleteMFADeviceSync] instead.
rpc DeleteMFADevice(stream DeleteMFADeviceRequest) returns (stream DeleteMFADeviceResponse) {
option deprecated = true;
}
// AddMFADeviceSync adds a new MFA device.
//
// A typical MFA registration sequence calls the following RPCs:
Expand All @@ -2655,6 +2664,14 @@ service AuthService {
// 4. AddMFADeviceSync
rpc AddMFADeviceSync(AddMFADeviceSyncRequest) returns (AddMFADeviceSyncResponse);
// DeleteMFADeviceSync deletes a users MFA device (nonstream).
//
// A typical MFA deletion sequence calls the following RPCs:
//
// 1. (optional) CreateAuthenticateChallenge
// (may be skipped depending on the token used, but is usually called
// regardless)
// 2. (optional) CreatePrivilegeToken
// 3. DeleteMFADeviceSync (using either authn challenge or token)
rpc DeleteMFADeviceSync(DeleteMFADeviceSyncRequest) returns (google.protobuf.Empty);
// GetMFADevices returns all MFA devices registered for the user calling
// this RPC.
Expand Down
13 changes: 12 additions & 1 deletion lib/auth/accountrecovery_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1393,11 +1393,22 @@ func createUserWithSecondFactors(testServer *TestTLSServer) (*userAuthCreds, err
return nil, trace.Wrap(err)
}

// Register a TOTP device.
userClient, err := testServer.NewClient(TestUser(username))
if err != nil {
return nil, trace.Wrap(err)
}

// Fetch the MFA device created above.
devicesResp, err := userClient.GetMFADevices(ctx, &proto.GetMFADevicesRequest{})
if err != nil {
return nil, trace.Wrap(err)
}
if len(devicesResp.Devices) != 1 {
return nil, fmt.Errorf("found an unexpected number of MFA devices: %v", devicesResp.Devices)
}
webDev.MFA = devicesResp.Devices[0]

// Register a TOTP device.
totpDev, err := RegisterTestDevice(
ctx,
userClient,
Expand Down
Loading

0 comments on commit ba90737

Please sign in to comment.