diff --git a/lib/auth/password.go b/lib/auth/password.go index 319b92719be13..19d43f19cda0f 100644 --- a/lib/auth/password.go +++ b/lib/auth/password.go @@ -306,16 +306,27 @@ func (a *Server) changeUserAuthentication(ctx context.Context, req *proto.Change return nil, trace.BadParameter("expired token") } - err = a.changeUserSecondFactor(ctx, req, token) - if err != nil { + // Check if the user still exists before potentially recreating the user + // below. If the user was deleted, do NOT honor the request and delete any + // other tokens associated with the user. + if _, err := a.GetUser(token.GetUser(), false); err != nil { + if trace.IsNotFound(err) { + // Delete any remaining tokens for users that no longer exist. + if err := a.deleteUserTokens(ctx, token.GetUser()); err != nil { + return nil, trace.Wrap(err) + } + } + return nil, trace.Wrap(err) + } + + if err := a.changeUserSecondFactor(ctx, req, token); err != nil { return nil, trace.Wrap(err) } username := token.GetUser() // Delete this token first to minimize the chances // of partially updated user with still valid token. - err = a.deleteUserTokens(ctx, username) - if err != nil { + if err := a.deleteUserTokens(ctx, username); err != nil { return nil, trace.Wrap(err) } diff --git a/lib/auth/tls_test.go b/lib/auth/tls_test.go index ccaf5edc7c446..10614c1f7d99b 100644 --- a/lib/auth/tls_test.go +++ b/lib/auth/tls_test.go @@ -2846,7 +2846,10 @@ func TestChangeUserAuthenticationSettings(t *testing.T) { authPreference, err := types.NewAuthPreference(types.AuthPreferenceSpecV2{ Type: constants.Local, - SecondFactor: constants.SecondFactorOTP, + SecondFactor: constants.SecondFactorWebauthn, + Webauthn: &types.Webauthn{ + RPID: "localhost", + }, }) require.NoError(t, err) @@ -2858,32 +2861,69 @@ func TestChangeUserAuthenticationSettings(t *testing.T) { clt, err := testSrv.NewClient(TestAdmin()) require.NoError(t, err) - _, _, err = CreateUserAndRole(clt, username, []string{"role1"}, nil) - require.NoError(t, err) + t.Run("Reset works when user exists", func(t *testing.T) { + _, _, err = CreateUserAndRole(clt, username, []string{"role1"}, nil) + require.NoError(t, err) - token, err := testSrv.Auth().CreateResetPasswordToken(ctx, CreateUserTokenRequest{ - Name: username, - TTL: time.Hour, - }) - require.NoError(t, err) + token, err := testSrv.Auth().CreateResetPasswordToken(ctx, CreateUserTokenRequest{ + Name: username, + TTL: time.Hour, + }) + require.NoError(t, err) - res, err := testSrv.Auth().CreateRegisterChallenge(ctx, &proto.CreateRegisterChallengeRequest{ - TokenID: token.GetName(), - DeviceType: proto.DeviceType_DEVICE_TYPE_TOTP, + res, err := testSrv.Auth().CreateRegisterChallenge(ctx, &proto.CreateRegisterChallengeRequest{ + TokenID: token.GetName(), + DeviceType: proto.DeviceType_DEVICE_TYPE_WEBAUTHN, + }) + require.NoError(t, err) + + _, registerSolved, err := NewTestDeviceFromChallenge(res) + require.NoError(t, err) + + _, err = testSrv.Auth().ChangeUserAuthentication(ctx, &proto.ChangeUserAuthenticationRequest{ + TokenID: token.GetName(), + NewPassword: []byte("qweqweqwe"), + NewMFARegisterResponse: registerSolved, + }) + require.NoError(t, err) }) - require.NoError(t, err) - otpToken, err := totp.GenerateCode(res.GetTOTP().GetSecret(), testSrv.Clock().Now()) - require.NoError(t, err) + t.Run("Reset link not allowed when user does not exist", func(t *testing.T) { + var tokenID string + var resp *proto.MFARegisterResponse + for i := 0; i < 5; i++ { + token, err := testSrv.Auth().CreateResetPasswordToken(ctx, CreateUserTokenRequest{ + Name: username, + TTL: time.Hour, + }) + require.NoError(t, err) + + res, err := testSrv.Auth().CreateRegisterChallenge(ctx, &proto.CreateRegisterChallengeRequest{ + TokenID: token.GetName(), + DeviceType: proto.DeviceType_DEVICE_TYPE_WEBAUTHN, + }) + require.NoError(t, err) + + _, registerSolved, err := NewTestDeviceFromChallenge(res) + require.NoError(t, err) - _, err = testSrv.Auth().ChangeUserAuthentication(ctx, &proto.ChangeUserAuthenticationRequest{ - TokenID: token.GetName(), - NewPassword: []byte("qweqweqwe"), - NewMFARegisterResponse: &proto.MFARegisterResponse{Response: &proto.MFARegisterResponse_TOTP{ - TOTP: &proto.TOTPRegisterResponse{Code: otpToken}, - }}, + tokenID = token.GetName() + resp = registerSolved + } + + require.NoError(t, testSrv.Auth().DeleteUser(ctx, username)) + + _, err = testSrv.Auth().ChangeUserAuthentication(ctx, &proto.ChangeUserAuthenticationRequest{ + TokenID: tokenID, + NewPassword: []byte("qweqweqwe"), + NewMFARegisterResponse: resp, + }) + require.Error(t, err) + + tokens, err := testSrv.Auth().GetUserTokens(ctx) + require.NoError(t, err) + require.Empty(t, tokens) }) - require.NoError(t, err) } // TestLoginNoLocalAuth makes sure that logins for local accounts can not be