[Passwords] Modify affiliation response parsing to support groupings.

This change modifies the response parsing to support FacetGroups.

Bug: 1108279
Change-Id: Iadc6078223110c5f1b890921ba3c84cc5afbd6ec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2362748
Commit-Queue: Nela Kaczmarek <nelakaczmarek@google.com>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800949}

components/password_manager/core/browser/android_affiliation/affiliation_backend.cc

@@ -256,7 +256,7 @@ bool AffiliationBackend::OnCanSendNetworkRequest() {
   if (requested_facet_uris.empty())
     return false;
 
-  fetcher_.reset(AffiliationFetcher::Create(url_loader_factory_, this));
+  fetcher_ = AffiliationFetcher::Create(url_loader_factory_, this);
   fetcher_->StartRequest(requested_facet_uris, {.branding_info = true});
   ReportStatistics(requested_facet_uris.size());
   return true;

components/password_manager/core/browser/android_affiliation/affiliation_fetcher.cc

@@ -55,14 +55,17 @@ AffiliationFetcher::AffiliationFetcher(
 AffiliationFetcher::~AffiliationFetcher() = default;
 
 // static
-AffiliationFetcherInterface* AffiliationFetcher::Create(
+std::unique_ptr<AffiliationFetcherInterface> AffiliationFetcher::Create(
     scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
     AffiliationFetcherDelegate* delegate) {
   if (g_testing_factory) {
-    return g_testing_factory->CreateInstance(std::move(url_loader_factory),
-                                             delegate);
+    return base::WrapUnique(g_testing_factory->CreateInstance(
+        std::move(url_loader_factory), delegate));
   }
-  return new AffiliationFetcher(std::move(url_loader_factory), delegate);
+  // Using `new` to access a non-public constructor.
+  // (https://abseil.io/tips/134#recommendations)
+  return base::WrapUnique(
+      new AffiliationFetcher(std::move(url_loader_factory), delegate));
 }
 
 // static

components/password_manager/core/browser/android_affiliation/affiliation_fetcher.h

@@ -39,7 +39,7 @@ class AffiliationFetcher : public AffiliationFetcherInterface {
   // Constructs a fetcher using the specified |url_loader_factory|, and will
   // provide the results to the |delegate| on the same thread that creates the
   // instance.
-  static AffiliationFetcherInterface* Create(
+  static std::unique_ptr<AffiliationFetcherInterface> Create(
       scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
       AffiliationFetcherDelegate* delegate);
 

components/password_manager/core/browser/android_affiliation/affiliation_fetcher_unittest.cc

@@ -32,6 +32,10 @@ const char kExampleAndroidPlayName[] = "Example Android App";
 const char kExampleAndroidIconURL[] = "https://example.com/icon.png";
 const char kExampleWebFacet1URI[] = "https://www.example.com";
 const char kExampleWebFacet2URI[] = "https://www.example.org";
+const char kExampleWebFacet1ChangePasswordURI[] =
+    "https://www.example.com/.well-known/change-password";
+const char kExampleWebFacet2ChangePasswordURI[] =
+    "https://www.example.org/settings/passwords";
 
 class MockAffiliationFetcherDelegate
     : public testing::StrictMock<AffiliationFetcherDelegate> {
@@ -158,8 +162,8 @@ TEST_F(AffiliationFetcherTest, BasicReqestAndResponse) {
   SetupSuccessfulResponse(test_response.SerializeAsString());
   MockAffiliationFetcherDelegate mock_delegate;
   EXPECT_CALL(mock_delegate, OnFetchSucceededProxy());
-  std::unique_ptr<AffiliationFetcherInterface> fetcher(
-      AffiliationFetcher::Create(test_shared_loader_factory(), &mock_delegate));
+  auto fetcher =
+      AffiliationFetcher::Create(test_shared_loader_factory(), &mock_delegate);
   fetcher->StartRequest(requested_uris, request_info);
   WaitForResponse();
 
@@ -198,8 +202,8 @@ TEST_F(AffiliationFetcherTest, AndroidBrandingInfoIsReturnedIfPresent) {
   SetupSuccessfulResponse(test_response.SerializeAsString());
   MockAffiliationFetcherDelegate mock_delegate;
   EXPECT_CALL(mock_delegate, OnFetchSucceededProxy());
-  std::unique_ptr<AffiliationFetcherInterface> fetcher(
-      AffiliationFetcher::Create(test_shared_loader_factory(), &mock_delegate));
+  auto fetcher =
+      AffiliationFetcher::Create(test_shared_loader_factory(), &mock_delegate);
   fetcher->StartRequest(requested_uris, request_info);
   WaitForResponse();
 
@@ -216,6 +220,50 @@ TEST_F(AffiliationFetcherTest, AndroidBrandingInfoIsReturnedIfPresent) {
                                           GURL(kExampleAndroidIconURL)}}));
 }
 
+TEST_F(AffiliationFetcherTest, ChangePasswordInfoIsReturnedIfPresent) {
+  affiliation_pb::LookupAffiliationResponse test_response;
+  affiliation_pb::FacetGroup* eq_class = test_response.add_group();
+
+  // kExampleWebFacet1URI, kExampleWebFacet1ChangePasswordURI
+  affiliation_pb::Facet* web_facet_1 = eq_class->add_facet();
+  web_facet_1->set_id(kExampleWebFacet1URI);
+  web_facet_1->mutable_change_password_info()->set_change_password_url(
+      kExampleWebFacet1ChangePasswordURI);
+
+  // kExampleWebFacet2URI, kExampleWebFacet2ChangePasswordURI
+  affiliation_pb::Facet* web_facet_2 = eq_class->add_facet();
+  web_facet_2->set_id(kExampleWebFacet2URI);
+  web_facet_2->mutable_change_password_info()->set_change_password_url(
+      kExampleWebFacet2ChangePasswordURI);
+
+  AffiliationFetcher::RequestInfo request_info{.change_password_info = true};
+
+  std::vector<FacetURI> requested_uris = {
+      FacetURI::FromCanonicalSpec(kExampleWebFacet1URI)};
+
+  SetupSuccessfulResponse(test_response.SerializeAsString());
+  MockAffiliationFetcherDelegate mock_delegate;
+  EXPECT_CALL(mock_delegate, OnFetchSucceededProxy());
+  auto fetcher =
+      AffiliationFetcher::Create(test_shared_loader_factory(), &mock_delegate);
+  fetcher->StartRequest(requested_uris, request_info);
+  WaitForResponse();
+
+  ASSERT_NO_FATAL_FAILURE(VerifyRequestPayload(requested_uris, request_info));
+  ASSERT_TRUE(testing::Mock::VerifyAndClearExpectations(&mock_delegate));
+
+  ASSERT_EQ(1u, mock_delegate.groupings().size());
+  EXPECT_THAT(
+      mock_delegate.groupings()[0],
+      testing::UnorderedElementsAre(
+          Facet{
+              .uri = FacetURI::FromCanonicalSpec(kExampleWebFacet1URI),
+              .change_password_url = GURL(kExampleWebFacet1ChangePasswordURI)},
+          Facet{.uri = FacetURI::FromCanonicalSpec(kExampleWebFacet2URI),
+                .change_password_url =
+                    GURL(kExampleWebFacet2ChangePasswordURI)}));
+}
+
 // The API contract of this class is to return an equivalence class for all
 // requested facets; however, the server will not return anything for facets
 // that are not affiliated with any other facet. Make sure an equivalence class
@@ -230,8 +278,8 @@ TEST_F(AffiliationFetcherTest, MissingEquivalenceClassesAreCreated) {
   SetupSuccessfulResponse(empty_test_response.SerializeAsString());
   MockAffiliationFetcherDelegate mock_delegate;
   EXPECT_CALL(mock_delegate, OnFetchSucceededProxy());
-  std::unique_ptr<AffiliationFetcherInterface> fetcher(
-      AffiliationFetcher::Create(test_shared_loader_factory(), &mock_delegate));
+  auto fetcher =
+      AffiliationFetcher::Create(test_shared_loader_factory(), &mock_delegate);
   fetcher->StartRequest(requested_uris, request_info);
   WaitForResponse();
 
@@ -261,8 +309,8 @@ TEST_F(AffiliationFetcherTest, DuplicateEquivalenceClassesAreIgnored) {
   SetupSuccessfulResponse(test_response.SerializeAsString());
   MockAffiliationFetcherDelegate mock_delegate;
   EXPECT_CALL(mock_delegate, OnFetchSucceededProxy());
-  std::unique_ptr<AffiliationFetcherInterface> fetcher(
-      AffiliationFetcher::Create(test_shared_loader_factory(), &mock_delegate));
+  auto fetcher =
+      AffiliationFetcher::Create(test_shared_loader_factory(), &mock_delegate);
   fetcher->StartRequest(requested_uris, {});
   WaitForResponse();
 
@@ -289,8 +337,8 @@ TEST_F(AffiliationFetcherTest, EmptyEquivalenceClassesAreIgnored) {
   SetupSuccessfulResponse(test_response.SerializeAsString());
   MockAffiliationFetcherDelegate mock_delegate;
   EXPECT_CALL(mock_delegate, OnFetchSucceededProxy());
-  std::unique_ptr<AffiliationFetcherInterface> fetcher(
-      AffiliationFetcher::Create(test_shared_loader_factory(), &mock_delegate));
+  auto fetcher =
+      AffiliationFetcher::Create(test_shared_loader_factory(), &mock_delegate);
   fetcher->StartRequest(requested_uris, {});
   WaitForResponse();
 
@@ -321,8 +369,8 @@ TEST_F(AffiliationFetcherTest, UnrecognizedFacetURIsAreIgnored) {
   SetupSuccessfulResponse(test_response.SerializeAsString());
   MockAffiliationFetcherDelegate mock_delegate;
   EXPECT_CALL(mock_delegate, OnFetchSucceededProxy());
-  std::unique_ptr<AffiliationFetcherInterface> fetcher(
-      AffiliationFetcher::Create(test_shared_loader_factory(), &mock_delegate));
+  auto fetcher =
+      AffiliationFetcher::Create(test_shared_loader_factory(), &mock_delegate);
   fetcher->StartRequest(requested_uris, {});
   WaitForResponse();
 
@@ -345,8 +393,8 @@ TEST_F(AffiliationFetcherTest, FailureBecauseResponseIsNotAProtobuf) {
   SetupSuccessfulResponse(kMalformedResponse);
   MockAffiliationFetcherDelegate mock_delegate;
   EXPECT_CALL(mock_delegate, OnMalformedResponse());
-  std::unique_ptr<AffiliationFetcherInterface> fetcher(
-      AffiliationFetcher::Create(test_shared_loader_factory(), &mock_delegate));
+  auto fetcher =
+      AffiliationFetcher::Create(test_shared_loader_factory(), &mock_delegate);
   fetcher->StartRequest(uris, {});
   WaitForResponse();
 }
@@ -369,8 +417,8 @@ TEST_F(AffiliationFetcherTest,
   SetupSuccessfulResponse(test_response.SerializeAsString());
   MockAffiliationFetcherDelegate mock_delegate;
   EXPECT_CALL(mock_delegate, OnMalformedResponse());
-  std::unique_ptr<AffiliationFetcherInterface> fetcher(
-      AffiliationFetcher::Create(test_shared_loader_factory(), &mock_delegate));
+  auto fetcher =
+      AffiliationFetcher::Create(test_shared_loader_factory(), &mock_delegate);
   fetcher->StartRequest(uris, {});
   WaitForResponse();
 }
@@ -382,8 +430,8 @@ TEST_F(AffiliationFetcherTest, FailOnServerError) {
   SetupServerErrorResponse();
   MockAffiliationFetcherDelegate mock_delegate;
   EXPECT_CALL(mock_delegate, OnFetchFailed());
-  std::unique_ptr<AffiliationFetcherInterface> fetcher(
-      AffiliationFetcher::Create(test_shared_loader_factory(), &mock_delegate));
+  auto fetcher =
+      AffiliationFetcher::Create(test_shared_loader_factory(), &mock_delegate);
   fetcher->StartRequest(uris, {});
   WaitForResponse();
 }
@@ -395,8 +443,8 @@ TEST_F(AffiliationFetcherTest, FailOnNetworkError) {
   SetupNetworkErrorResponse();
   MockAffiliationFetcherDelegate mock_delegate;
   EXPECT_CALL(mock_delegate, OnFetchFailed());
-  std::unique_ptr<AffiliationFetcherInterface> fetcher(
-      AffiliationFetcher::Create(test_shared_loader_factory(), &mock_delegate));
+  auto fetcher =
+      AffiliationFetcher::Create(test_shared_loader_factory(), &mock_delegate);
   fetcher->StartRequest(uris, {});
   WaitForResponse();
 }

components/password_manager/core/browser/android_affiliation/lookup_affiliation_response_parser.cc

@@ -6,20 +6,22 @@
 
 namespace password_manager {
 
-bool ParseLookupAffiliationResponse(
-    const std::vector<FacetURI>& requested_facet_uris,
-    const affiliation_pb::LookupAffiliationResponse& response,
-    AffiliationFetcherDelegate::Result* result) {
-  result->affiliations.reserve(requested_facet_uris.size());
+namespace {
 
-  std::map<FacetURI, size_t> facet_uri_to_class_index;
-  for (int i = 0; i < response.affiliation_size(); ++i) {
-    const affiliation_pb::Affiliation& equivalence_class(
-        response.affiliation(i));
+// Template for the affiliation_pb message:
+// > affiliation_pb::Affiliation
+// > affiliation_pb::FacetGroup
+template <typename MessageT>
+bool ParseFacets(const std::vector<FacetURI>& requested_facet_uris,
+                 const MessageT& response,
+                 std::vector<std::vector<Facet>>& result) {
+  result.reserve(requested_facet_uris.size());
 
-    AffiliatedFacets affiliated_facets;
-    for (int j = 0; j < equivalence_class.facet_size(); ++j) {
-      const affiliation_pb::Facet& facet(equivalence_class.facet(j));
+  std::map<FacetURI, size_t> facet_uri_to_class_index;
+  for (const auto& equivalence_class : response) {
+    std::vector<Facet> facets;
+    facets.reserve(equivalence_class.facet().size());
+    for (const auto& facet : equivalence_class.facet()) {
       const std::string& uri_spec(facet.id());
       FacetURI uri = FacetURI::FromPotentiallyInvalidSpec(uri_spec);
       // Ignore potential future kinds of facet URIs (e.g. for new platforms).
@@ -35,29 +37,28 @@ bool ParseLookupAffiliationResponse(
         new_facet.change_password_url =
             GURL(facet.change_password_info().change_password_url());
       }
-      affiliated_facets.push_back(new_facet);
+      facets.push_back(std::move(new_facet));
     }
 
     // Be lenient and ignore empty (after filtering) equivalence classes.
-    if (affiliated_facets.empty())
+    if (facets.empty())
       continue;
 
     // Ignore equivalence classes that are duplicates of earlier ones. However,
     // fail in the case of a partial overlap, which violates the invariant that
     // affiliations must form an equivalence relation.
-    for (const Facet& facet : affiliated_facets) {
+    for (const Facet& facet : facets) {
       if (!facet_uri_to_class_index.count(facet.uri))
-        facet_uri_to_class_index[facet.uri] = result->affiliations.size();
+        facet_uri_to_class_index[facet.uri] = result.size();
       if (facet_uri_to_class_index[facet.uri] !=
-          facet_uri_to_class_index[affiliated_facets[0].uri]) {
+          facet_uri_to_class_index[facets[0].uri]) {
         return false;
       }
     }
 
     // Filter out duplicate equivalence classes in the response.
-    if (facet_uri_to_class_index[affiliated_facets[0].uri] ==
-        result->affiliations.size()) {
-      result->affiliations.push_back(affiliated_facets);
+    if (facet_uri_to_class_index[facets[0].uri] == result.size()) {
+      result.push_back(std::move(facets));
     }
   }
 
@@ -65,10 +66,21 @@ bool ParseLookupAffiliationResponse(
   // appear in the server response due to not being affiliated with any others.
   for (const FacetURI& uri : requested_facet_uris) {
     if (!facet_uri_to_class_index.count(uri))
-      result->affiliations.push_back({{uri}});
+      result.push_back({{uri}});
   }
 
   return true;
 }
 
+}  // namespace
+
+bool ParseLookupAffiliationResponse(
+    const std::vector<FacetURI>& requested_facet_uris,
+    const affiliation_pb::LookupAffiliationResponse& response,
+    AffiliationFetcherDelegate::Result* result) {
+  return ParseFacets(requested_facet_uris, response.affiliation(),
+                     result->affiliations) &&
+         ParseFacets(requested_facet_uris, response.group(), result->groupings);
+}
+
 }  // namespace password_manager

components/password_manager/core/browser/site_affiliation/affiliation_service_impl.cc

@@ -66,7 +66,7 @@ AffiliationServiceImpl::ConvertMissingSchemeHostPortsToFacets(
 void AffiliationServiceImpl::RequestFacetsAffiliations(
     const std::vector<FacetURI>& facets,
     const AffiliationFetcherInterface::RequestInfo request_info) {
-  fetcher_.reset(AffiliationFetcher::Create(url_loader_factory_, this));
+  fetcher_ = AffiliationFetcher::Create(url_loader_factory_, this);
   fetcher_->StartRequest(facets, request_info);
 }