Commit 5236daa8 authored by Nela Kaczmarek's avatar Nela Kaczmarek Committed by Commit Bot

[Passwords] Add RequestInfo struct.

This change covers adding change_password_url field to Facet and adding a RequestInfo struct that enables caller to set Affiliation Fetcher request mask.

Change-Id: I544e5ab0d9f07d0b711e4b8775a40f1130ba1563
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2362590
Commit-Queue: Nela Kaczmarek <nelakaczmarek@google.com>
Reviewed-by: default avatarJan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800524}
parent b10cbdcc
......@@ -257,7 +257,7 @@ bool AffiliationBackend::OnCanSendNetworkRequest() {
return false;
fetcher_.reset(AffiliationFetcher::Create(url_loader_factory_, this));
fetcher_->StartRequest(requested_facet_uris);
fetcher_->StartRequest(requested_facet_uris, {.branding_info = true});
ReportStatistics(requested_facet_uris.size());
return true;
}
......
......@@ -71,7 +71,8 @@ void AffiliationFetcher::SetFactoryForTesting(
g_testing_factory = factory;
}
void AffiliationFetcher::StartRequest(const std::vector<FacetURI>& facet_uris) {
void AffiliationFetcher::StartRequest(const std::vector<FacetURI>& facet_uris,
RequestInfo request_info) {
DCHECK(!simple_url_loader_);
requested_facet_uris_ = facet_uris;
......@@ -114,7 +115,7 @@ void AffiliationFetcher::StartRequest(const std::vector<FacetURI>& facet_uris) {
resource_request->method = "POST";
simple_url_loader_ = network::SimpleURLLoader::Create(
std::move(resource_request), traffic_annotation);
simple_url_loader_->AttachStringForUpload(PreparePayload(),
simple_url_loader_->AttachStringForUpload(PreparePayload(request_info),
"application/x-protobuf");
simple_url_loader_->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
url_loader_factory_.get(),
......@@ -137,14 +138,18 @@ GURL AffiliationFetcher::BuildQueryURL() {
"key", google_apis::GetAPIKey());
}
std::string AffiliationFetcher::PreparePayload() const {
std::string AffiliationFetcher::PreparePayload(RequestInfo request_info) const {
affiliation_pb::LookupAffiliationRequest lookup_request;
for (const FacetURI& uri : requested_facet_uris_)
lookup_request.add_facet(uri.canonical_spec());
// Enable request for branding information.
auto mask = std::make_unique<affiliation_pb::LookupAffiliationMask>();
mask->set_branding_info(true);
mask->set_branding_info(request_info.branding_info);
// Change password info requires grouping info enabled.
mask->set_grouping_info(request_info.change_password_info);
mask->set_change_password_info(request_info.change_password_info);
lookup_request.set_allocated_mask(mask.release());
std::string serialized_request;
......
......@@ -53,14 +53,16 @@ class AffiliationFetcher : public AffiliationFetcherInterface {
// calls. The caller may pass in NULL to resume using the default factory.
static void SetFactoryForTesting(TestAffiliationFetcherFactory* factory);
// Actually starts the request to retrieve affiliations for each facet in
// |facet_uris|, and will call the delegate with the results on the same
// thread when done. If |this| is destroyed before completion, the in-flight
// request is cancelled, and the delegate will not be called. Further details:
// Actually starts the request to retrieve affiliations and optionally
// groupings for each facet in |facet_uris| along with the details based on
// |request_info|. Calls the delegate with the results on the same thread when
// done. If |this| is destroyed before completion, the in-flight request is
// cancelled, and the delegate will not be called. Further details:
// * No cookies are sent/saved with the request.
// * In case of network/server errors, the request will not be retried.
// * Results are guaranteed to be always fresh and will never be cached.
void StartRequest(const std::vector<FacetURI>& facet_uris) override;
void StartRequest(const std::vector<FacetURI>& facet_uris,
RequestInfo request_info) override;
const std::vector<FacetURI>& GetRequestedFacetURIs() const override;
......@@ -73,8 +75,8 @@ class AffiliationFetcher : public AffiliationFetcherInterface {
private:
// Prepares and returns the serialized protocol buffer message that will be
// the payload of the POST request.
std::string PreparePayload() const;
// the payload of the POST request. Sets mask request based on |request_info|.
std::string PreparePayload(RequestInfo request_info) const;
// Parses and validates the response protocol buffer message for a list of
// equivalence classes, stores them into |result| and returns true on success.
......
......@@ -18,7 +18,6 @@ class AffiliationFetcherDelegate {
public:
// Encapsulates the response to an affiliations request.
struct Result {
// copyable and movable
Result();
Result(const Result& other);
Result(Result&& other);
......
......@@ -14,10 +14,20 @@ namespace password_manager {
class AffiliationFetcherInterface {
public:
// A struct that enables to set Affiliation Fetcher request mask.
struct RequestInfo {
bool branding_info = false;
bool change_password_info = false;
bool operator==(const RequestInfo& other) const {
return branding_info == other.branding_info &&
change_password_info == other.change_password_info;
}
};
AffiliationFetcherInterface() = default;
virtual ~AffiliationFetcherInterface() = default;
// Not copyable or movable
AffiliationFetcherInterface(const AffiliationFetcherInterface&) = delete;
AffiliationFetcherInterface& operator=(const AffiliationFetcherInterface&) =
delete;
......@@ -27,7 +37,8 @@ class AffiliationFetcherInterface {
// Starts the request to retrieve affiliations for each facet in
// |facet_uris|.
virtual void StartRequest(const std::vector<FacetURI>& facet_uris) = 0;
virtual void StartRequest(const std::vector<FacetURI>& facet_uris,
RequestInfo request_info) = 0;
// Returns requested facet uris.
virtual const std::vector<FacetURI>& GetRequestedFacetURIs() const = 0;
......
......@@ -74,7 +74,8 @@ class AffiliationFetcherTest : public testing::Test {
}
protected:
void VerifyRequestPayload(const std::vector<FacetURI>& expected_facet_uris) {
void VerifyRequestPayload(const std::vector<FacetURI>& expected_facet_uris,
AffiliationFetcher::RequestInfo request_info) {
affiliation_pb::LookupAffiliationRequest request;
ASSERT_TRUE(request.ParseFromString(intercepted_body_));
......@@ -89,9 +90,12 @@ class AffiliationFetcherTest : public testing::Test {
EXPECT_THAT(actual_uris,
testing::UnorderedElementsAreArray(expected_facet_uris));
// Verify that an affiliation mask is present that has branding_info set to
// true.
EXPECT_TRUE(request.mask().branding_info());
EXPECT_EQ(request.mask().branding_info(), request_info.branding_info);
// Change password info requires grouping info enabled.
EXPECT_EQ(request.mask().grouping_info(),
request_info.change_password_info);
EXPECT_EQ(request.mask().change_password_info(),
request_info.change_password_info);
}
GURL interception_url() { return AffiliationFetcher::BuildQueryURL(); }
......@@ -135,6 +139,7 @@ TEST_F(AffiliationFetcherTest, BasicReqestAndResponse) {
const char kNotExampleAndroidFacetURI[] =
"android://hash1234@com.example.not";
const char kNotExampleWebFacetURI[] = "https://not.example.com";
AffiliationFetcher::RequestInfo request_info{.branding_info = true};
affiliation_pb::LookupAffiliationResponse test_response;
affiliation_pb::Affiliation* eq_class1 = test_response.add_affiliation();
......@@ -155,10 +160,10 @@ TEST_F(AffiliationFetcherTest, BasicReqestAndResponse) {
EXPECT_CALL(mock_delegate, OnFetchSucceededProxy());
std::unique_ptr<AffiliationFetcherInterface> fetcher(
AffiliationFetcher::Create(test_shared_loader_factory(), &mock_delegate));
fetcher->StartRequest(requested_uris);
fetcher->StartRequest(requested_uris, request_info);
WaitForResponse();
ASSERT_NO_FATAL_FAILURE(VerifyRequestPayload(requested_uris));
ASSERT_NO_FATAL_FAILURE(VerifyRequestPayload(requested_uris, request_info));
ASSERT_TRUE(testing::Mock::VerifyAndClearExpectations(&mock_delegate));
ASSERT_EQ(2u, mock_delegate.affiliations().size());
......@@ -185,6 +190,7 @@ TEST_F(AffiliationFetcherTest, AndroidBrandingInfoIsReturnedIfPresent) {
affiliation_pb::Facet* android_facet = eq_class->add_facet();
android_facet->set_id(kExampleAndroidFacetURI);
android_facet->set_allocated_branding_info(android_branding_info.release());
AffiliationFetcher::RequestInfo request_info{.branding_info = true};
std::vector<FacetURI> requested_uris;
requested_uris.push_back(FacetURI::FromCanonicalSpec(kExampleWebFacet1URI));
......@@ -194,10 +200,10 @@ TEST_F(AffiliationFetcherTest, AndroidBrandingInfoIsReturnedIfPresent) {
EXPECT_CALL(mock_delegate, OnFetchSucceededProxy());
std::unique_ptr<AffiliationFetcherInterface> fetcher(
AffiliationFetcher::Create(test_shared_loader_factory(), &mock_delegate));
fetcher->StartRequest(requested_uris);
fetcher->StartRequest(requested_uris, request_info);
WaitForResponse();
ASSERT_NO_FATAL_FAILURE(VerifyRequestPayload(requested_uris));
ASSERT_NO_FATAL_FAILURE(VerifyRequestPayload(requested_uris, request_info));
ASSERT_TRUE(testing::Mock::VerifyAndClearExpectations(&mock_delegate));
ASSERT_EQ(1u, mock_delegate.affiliations().size());
......@@ -216,6 +222,7 @@ TEST_F(AffiliationFetcherTest, AndroidBrandingInfoIsReturnedIfPresent) {
// of size one is created for each of the missing facets.
TEST_F(AffiliationFetcherTest, MissingEquivalenceClassesAreCreated) {
affiliation_pb::LookupAffiliationResponse empty_test_response;
AffiliationFetcher::RequestInfo request_info{.branding_info = true};
std::vector<FacetURI> requested_uris;
requested_uris.push_back(FacetURI::FromCanonicalSpec(kExampleWebFacet1URI));
......@@ -225,10 +232,10 @@ TEST_F(AffiliationFetcherTest, MissingEquivalenceClassesAreCreated) {
EXPECT_CALL(mock_delegate, OnFetchSucceededProxy());
std::unique_ptr<AffiliationFetcherInterface> fetcher(
AffiliationFetcher::Create(test_shared_loader_factory(), &mock_delegate));
fetcher->StartRequest(requested_uris);
fetcher->StartRequest(requested_uris, request_info);
WaitForResponse();
ASSERT_NO_FATAL_FAILURE(VerifyRequestPayload(requested_uris));
ASSERT_NO_FATAL_FAILURE(VerifyRequestPayload(requested_uris, request_info));
ASSERT_TRUE(testing::Mock::VerifyAndClearExpectations(&mock_delegate));
ASSERT_EQ(1u, mock_delegate.affiliations().size());
......@@ -256,7 +263,7 @@ TEST_F(AffiliationFetcherTest, DuplicateEquivalenceClassesAreIgnored) {
EXPECT_CALL(mock_delegate, OnFetchSucceededProxy());
std::unique_ptr<AffiliationFetcherInterface> fetcher(
AffiliationFetcher::Create(test_shared_loader_factory(), &mock_delegate));
fetcher->StartRequest(requested_uris);
fetcher->StartRequest(requested_uris, {});
WaitForResponse();
ASSERT_TRUE(testing::Mock::VerifyAndClearExpectations(&mock_delegate));
......@@ -284,7 +291,7 @@ TEST_F(AffiliationFetcherTest, EmptyEquivalenceClassesAreIgnored) {
EXPECT_CALL(mock_delegate, OnFetchSucceededProxy());
std::unique_ptr<AffiliationFetcherInterface> fetcher(
AffiliationFetcher::Create(test_shared_loader_factory(), &mock_delegate));
fetcher->StartRequest(requested_uris);
fetcher->StartRequest(requested_uris, {});
WaitForResponse();
ASSERT_TRUE(testing::Mock::VerifyAndClearExpectations(&mock_delegate));
......@@ -316,7 +323,7 @@ TEST_F(AffiliationFetcherTest, UnrecognizedFacetURIsAreIgnored) {
EXPECT_CALL(mock_delegate, OnFetchSucceededProxy());
std::unique_ptr<AffiliationFetcherInterface> fetcher(
AffiliationFetcher::Create(test_shared_loader_factory(), &mock_delegate));
fetcher->StartRequest(requested_uris);
fetcher->StartRequest(requested_uris, {});
WaitForResponse();
ASSERT_TRUE(testing::Mock::VerifyAndClearExpectations(&mock_delegate));
......@@ -340,7 +347,7 @@ TEST_F(AffiliationFetcherTest, FailureBecauseResponseIsNotAProtobuf) {
EXPECT_CALL(mock_delegate, OnMalformedResponse());
std::unique_ptr<AffiliationFetcherInterface> fetcher(
AffiliationFetcher::Create(test_shared_loader_factory(), &mock_delegate));
fetcher->StartRequest(uris);
fetcher->StartRequest(uris, {});
WaitForResponse();
}
......@@ -364,7 +371,7 @@ TEST_F(AffiliationFetcherTest,
EXPECT_CALL(mock_delegate, OnMalformedResponse());
std::unique_ptr<AffiliationFetcherInterface> fetcher(
AffiliationFetcher::Create(test_shared_loader_factory(), &mock_delegate));
fetcher->StartRequest(uris);
fetcher->StartRequest(uris, {});
WaitForResponse();
}
......@@ -377,7 +384,7 @@ TEST_F(AffiliationFetcherTest, FailOnServerError) {
EXPECT_CALL(mock_delegate, OnFetchFailed());
std::unique_ptr<AffiliationFetcherInterface> fetcher(
AffiliationFetcher::Create(test_shared_loader_factory(), &mock_delegate));
fetcher->StartRequest(uris);
fetcher->StartRequest(uris, {});
WaitForResponse();
}
......@@ -390,7 +397,7 @@ TEST_F(AffiliationFetcherTest, FailOnNetworkError) {
EXPECT_CALL(mock_delegate, OnFetchFailed());
std::unique_ptr<AffiliationFetcherInterface> fetcher(
AffiliationFetcher::Create(test_shared_loader_factory(), &mock_delegate));
fetcher->StartRequest(uris);
fetcher->StartRequest(uris, {});
WaitForResponse();
}
......
......@@ -171,6 +171,7 @@ struct FacetBrandingInfo {
struct Facet {
FacetURI uri;
FacetBrandingInfo branding_info;
GURL change_password_url;
};
// A collection of facets affiliated with each other, i.e. an equivalence class.
......
......@@ -25,9 +25,17 @@ bool ParseLookupAffiliationResponse(
// Ignore potential future kinds of facet URIs (e.g. for new platforms).
if (!uri.is_valid())
continue;
affiliated_facets.push_back(
{uri, FacetBrandingInfo{facet.branding_info().name(),
GURL(facet.branding_info().icon_url())}});
Facet new_facet = {uri};
if (facet.has_branding_info()) {
new_facet.branding_info =
FacetBrandingInfo{facet.branding_info().name(),
GURL(facet.branding_info().icon_url())};
}
if (facet.has_change_password_info()) {
new_facet.change_password_url =
GURL(facet.change_password_info().change_password_url());
}
affiliated_facets.push_back(new_facet);
}
// Be lenient and ignore empty (after filtering) equivalence classes.
......
......@@ -17,7 +17,10 @@ class MockAffiliationFetcher : public AffiliationFetcherInterface {
MockAffiliationFetcher();
~MockAffiliationFetcher() override;
MOCK_METHOD(void, StartRequest, (const std::vector<FacetURI>&), (override));
MOCK_METHOD(void,
StartRequest,
(const std::vector<FacetURI>&, RequestInfo),
(override));
MOCK_METHOD(std::vector<FacetURI>&,
GetRequestedFacetURIs,
(),
......
......@@ -25,7 +25,8 @@ void AffiliationServiceImpl::PrefetchChangePasswordURLs(
const std::vector<url::SchemeHostPort>& tuple_origins) {
if (ShouldAffiliationBasedMatchingBeActive(sync_service_)) {
RequestFacetsAffiliations(
ConvertMissingSchemeHostPortsToFacets(tuple_origins));
ConvertMissingSchemeHostPortsToFacets(tuple_origins),
{.change_password_info = true});
}
}
......@@ -63,9 +64,10 @@ AffiliationServiceImpl::ConvertMissingSchemeHostPortsToFacets(
// TODO(crbug.com/1117045): New request resets the pointer to
// AffiliationFetcher, therefore the previous request gets canceled.
void AffiliationServiceImpl::RequestFacetsAffiliations(
const std::vector<FacetURI>& facets) {
const std::vector<FacetURI>& facets,
const AffiliationFetcherInterface::RequestInfo request_info) {
fetcher_.reset(AffiliationFetcher::Create(url_loader_factory_, this));
fetcher_->StartRequest(facets);
fetcher_->StartRequest(facets, request_info);
}
} // namespace password_manager
......@@ -13,6 +13,7 @@
#include "base/memory/scoped_refptr.h"
#include "components/password_manager/core/browser/android_affiliation/affiliation_fetcher_delegate.h"
#include "components/password_manager/core/browser/android_affiliation/affiliation_fetcher_interface.h"
namespace network {
class SharedURLLoaderFactory;
......@@ -24,8 +25,6 @@ class SyncService;
namespace password_manager {
class AffiliationFetcherInterface;
class AffiliationServiceImpl : public AffiliationService,
public AffiliationFetcherDelegate {
public:
......@@ -63,7 +62,9 @@ class AffiliationServiceImpl : public AffiliationService,
const std::vector<url::SchemeHostPort>& tuple_origins);
// Calls Affiliation Fetcher and starts a request for |facets| affiliations.
void RequestFacetsAffiliations(const std::vector<FacetURI>& facets);
void RequestFacetsAffiliations(
const std::vector<FacetURI>& facets,
const AffiliationFetcherInterface::RequestInfo request_info);
syncer::SyncService* sync_service_;
const scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
......
......@@ -104,7 +104,9 @@ TEST_F(AffiliationServiceImplTest, ClearStopsOngoingAffiliationFetcherRequest) {
EXPECT_CALL(*mock_fetcher_factory(), CreateInstance)
.WillOnce(Return(mock_fetcher));
EXPECT_CALL(*mock_fetcher,
StartRequest(SchemeHostPortsToFacetsURIs(tuple_origins)));
StartRequest(SchemeHostPortsToFacetsURIs(tuple_origins),
AffiliationFetcherInterface::RequestInfo{
.change_password_info = true}));
service()->PrefetchChangePasswordURLs(tuple_origins);
EXPECT_NE(nullptr, service()->GetFetcherForTesting());
......@@ -127,11 +129,14 @@ TEST_F(AffiliationServiceImplTest,
scheme_host_port5};
MockAffiliationFetcher* mock_fetcher = new MockAffiliationFetcher();
MockAffiliationFetcher* new_mock_fetcher = new MockAffiliationFetcher();
EXPECT_CALL(*mock_fetcher,
StartRequest(SchemeHostPortsToFacetsURIs(tuple_origins_1)));
EXPECT_CALL(*new_mock_fetcher,
StartRequest(SchemeHostPortsToFacetsURIs(tuple_origins_2)));
AffiliationFetcher::RequestInfo request_info{.change_password_info = true};
EXPECT_CALL(
*mock_fetcher,
StartRequest(SchemeHostPortsToFacetsURIs(tuple_origins_1), request_info));
EXPECT_CALL(
*new_mock_fetcher,
StartRequest(SchemeHostPortsToFacetsURIs(tuple_origins_2), request_info));
EXPECT_CALL(*mock_fetcher_factory(), CreateInstance)
.WillOnce(Return(mock_fetcher))
.WillOnce(Return(new_mock_fetcher));
......@@ -154,7 +159,9 @@ TEST_F(AffiliationServiceImplTest,
EXPECT_CALL(*mock_fetcher_factory(), CreateInstance)
.WillOnce(Return(mock_fetcher));
EXPECT_CALL(*mock_fetcher,
StartRequest(SchemeHostPortsToFacetsURIs(tuple_origins)));
StartRequest(SchemeHostPortsToFacetsURIs(tuple_origins),
AffiliationFetcherInterface::RequestInfo{
.change_password_info = true}));
service()->PrefetchChangePasswordURLs(tuple_origins);
}
......
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