Commit f2ecbbf8 authored by Martin Kreichgauer's avatar Martin Kreichgauer Committed by Commit Bot

fido: fix a nonsensical cast in fido_request_handler_unittest

In crrev.com/c/1256016, we moved FidoAuthenticator instantiation and
ownership into the respective FidoDiscovery subclasses. This also
eliminated FidoRequestHandlerBase::CreateAuthenticatorFromDevice() which
was where we previously wrapped a discovered FidoDevice in a
FidoDeviceAuthenticator before adding it to the request handler.

In fido_request_handler_unittest, this was used as a testing seam to
inject a FakeFidoAuthenticator implementation. Injection of the fake was
dropped, but the "real" FidoDeviceAuthenticator was still casted into a
pointer to the fake elsewhere. While this seems to not bother compilers
too much, it's probably a Bad Idea. This CL therefore entirely removes
the FakeFidoAuthenticator class (without changing semantics of the
fixture or the tests).

Bug: 903294
Change-Id: I7a6bad330b1152dc455ded5b6db3d3309f4eb3be
Reviewed-on: https://chromium-review.googlesource.com/c/1340903Reviewed-by: default avatarJun Choi <hongjunchoi@chromium.org>
Commit-Queue: Jun Choi <hongjunchoi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609536}
parent b6efcf8e
......@@ -48,6 +48,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoDeviceAuthenticator
base::WeakPtr<FidoAuthenticator> GetWeakPtr() override;
FidoDevice* device() { return device_.get(); }
void SetTaskForTesting(std::unique_ptr<FidoTask> task);
protected:
void OnCtapMakeCredentialResponseReceived(
......@@ -57,8 +58,6 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoDeviceAuthenticator
GetAssertionCallback callback,
base::Optional<std::vector<uint8_t>> response_data);
void SetTaskForTesting(std::unique_ptr<FidoTask> task);
private:
void InitializeAuthenticatorDone(base::OnceClosure callback);
......
......@@ -157,17 +157,6 @@ class FakeFidoTask : public FidoTask {
base::WeakPtrFactory<FakeFidoTask> weak_factory_;
};
class FakeFidoAuthenticator : public FidoDeviceAuthenticator {
public:
explicit FakeFidoAuthenticator(std::unique_ptr<FidoDevice> device)
: FidoDeviceAuthenticator(std::move(device)) {}
void RunFakeTask(FakeTaskCallback callback) {
SetTaskForTesting(
std::make_unique<FakeFidoTask>(device(), std::move(callback)));
}
};
class FakeFidoRequestHandler : public FidoRequestHandler<std::vector<uint8_t>> {
public:
FakeFidoRequestHandler(service_manager::Connector* connector,
......@@ -185,10 +174,18 @@ class FakeFidoRequestHandler : public FidoRequestHandler<std::vector<uint8_t>> {
~FakeFidoRequestHandler() override = default;
void DispatchRequest(FidoAuthenticator* authenticator) override {
static_cast<FakeFidoAuthenticator*>(authenticator)
->RunFakeTask(
base::BindOnce(&FakeFidoRequestHandler::OnAuthenticatorResponse,
weak_factory_.GetWeakPtr(), authenticator));
// FidoRequestHandlerTest uses ScopedFakeDiscovery to inject mock devices
// that get wrapped in a FidoDeviceAuthenticator, so we can safely cast
// here.
auto* device_authenticator =
static_cast<FidoDeviceAuthenticator*>(authenticator);
// Instead of sending a real CTAP request, send an empty byte array. Note
// that during discovery, the device already has received a GetInfo command
// at this point.
device_authenticator->SetTaskForTesting(std::make_unique<FakeFidoTask>(
device_authenticator->device(),
base::BindOnce(&FakeFidoRequestHandler::OnAuthenticatorResponse,
weak_factory_.GetWeakPtr(), authenticator)));
}
private:
......@@ -507,7 +504,7 @@ TEST_F(FidoRequestHandlerTest, TestSetPlatformAuthenticator) {
CreateFakeSuccessDeviceResponse());
device->SetDeviceTransport(FidoTransportProtocol::kInternal);
auto authenticator =
std::make_unique<FakeFidoAuthenticator>(std::move(device));
std::make_unique<FidoDeviceAuthenticator>(std::move(device));
TestTransportAvailabilityObserver observer;
auto request_handler = std::make_unique<FakeFidoRequestHandler>(
......@@ -543,7 +540,7 @@ TEST_F(FidoRequestHandlerTest,
CreateFakeSuccessDeviceResponse());
device->SetDeviceTransport(FidoTransportProtocol::kInternal);
auto authenticator =
std::make_unique<FakeFidoAuthenticator>(std::move(device));
std::make_unique<FidoDeviceAuthenticator>(std::move(device));
TestTransportAvailabilityObserver observer;
auto request_handler = std::make_unique<FakeFidoRequestHandler>(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment