[FedCM] Plumb a last used timestamp to the FederatedAuthRequestImpl

Currently, we store the last used timestamp of an account in FedCM
storage. This CL allows the content code to query this value via a new
method. This method is used whenever an account ID is passed, whereas
the existing method is changed to only be used when no account ID is
passed. A followup will pass this timestamp to UI code.

Bug: 347963515
Change-Id: Iecdcf195201e5d7ec2ad0c4f30f3e75ff77c0bf6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5639797
Reviewed-by: Christian Dullweber <[email protected]>
Commit-Queue: Nicolás Peña <[email protected]>
Reviewed-by: Rakina Zata Amni <[email protected]>
Reviewed-by: Christian Biesinger <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1317358}

chrome/browser/browsing_data/chrome_browsing_data_model_delegate_unittest.cc

@@ -315,7 +315,7 @@ TEST_F(ChromeBrowsingDataModelDelegateTest, RemoveFederatedIdentityData) {
       federated_identity_permission_context();
   context->GrantSharingPermission(kRequester, kEmbedder, kIdentityProvider,
                                   kAccountId);
-  EXPECT_TRUE(context->HasSharingPermission(kRequester, kEmbedder,
+  EXPECT_TRUE(context->GetLastUsedTimestamp(kRequester, kEmbedder,
                                             kIdentityProvider, kAccountId));
   EXPECT_TRUE(context->HasSharingPermission(kRequester));
   EXPECT_FALSE(context->HasSharingPermission(kEmbedder));
@@ -329,7 +329,7 @@ TEST_F(ChromeBrowsingDataModelDelegateTest, RemoveFederatedIdentityData) {
       run_loop.QuitClosure());
   run_loop.Run();
 
-  EXPECT_FALSE(context->HasSharingPermission(kRequester, kEmbedder,
+  EXPECT_FALSE(context->GetLastUsedTimestamp(kRequester, kEmbedder,
                                              kIdentityProvider, kAccountId));
   EXPECT_FALSE(context->HasSharingPermission(kRequester));
 }

chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc

@@ -3251,7 +3251,7 @@ TEST_F(ChromeBrowsingDataRemoverDelegateTest, RemoveFederatedContentSettings) {
 
       federated_context.GrantSharingPermission(rp_origin, rp_embedder_origin,
                                                idp_origin, account_id);
-      ASSERT_TRUE(federated_context.HasSharingPermission(
+      ASSERT_TRUE(federated_context.GetLastUsedTimestamp(
           rp_origin, rp_embedder_origin, idp_origin, account_id));
 
       host_content_settings_map->SetContentSettingDefaultScope(
@@ -3273,7 +3273,7 @@ TEST_F(ChromeBrowsingDataRemoverDelegateTest, RemoveFederatedContentSettings) {
       // ObjectPermissionContextBase cache.
       FederatedIdentityPermissionContext federated_context(GetProfile());
 
-      EXPECT_FALSE(federated_context.HasSharingPermission(
+      EXPECT_FALSE(federated_context.GetLastUsedTimestamp(
           rp_origin, rp_embedder_origin, idp_origin, account_id));
 
       // Content setting is on by default.

chrome/browser/webid/federated_identity_account_keyed_permission_context.cc

@@ -138,8 +138,7 @@ bool FederatedIdentityAccountKeyedPermissionContext::HasPermission(
 bool FederatedIdentityAccountKeyedPermissionContext::HasPermission(
     const url::Origin& relying_party_requester,
     const url::Origin& relying_party_embedder,
-    const url::Origin& identity_provider,
-    const std::optional<std::string>& account_id) {
+    const url::Origin& identity_provider) {
   // TODO(crbug.com/40846157): This is currently origin-bound, but we would like
   // this grant to apply at the 'site' (aka eTLD+1) level. We should override
   // GetGrantedObject to find a grant that matches the RP's site rather
@@ -152,17 +151,50 @@ bool FederatedIdentityAccountKeyedPermissionContext::HasPermission(
     return false;
   }
 
+  base::Value::List* account_list =
+      granted_object->value.FindList(kAccountIdsKey);
+  return !!account_list;
+}
+
+std::optional<base::Time>
+FederatedIdentityAccountKeyedPermissionContext::GetLastUsedTimestamp(
+    const url::Origin& relying_party_requester,
+    const url::Origin& relying_party_embedder,
+    const url::Origin& identity_provider,
+    const std::string& account_id) {
+  std::string key = BuildKey(relying_party_requester, relying_party_embedder,
+                             identity_provider);
+  const auto granted_object = GetGrantedObject(relying_party_requester, key);
+
+  if (!granted_object) {
+    return std::nullopt;
+  }
+
   base::Value::List* account_list =
       granted_object->value.FindList(kAccountIdsKey);
   if (!account_list) {
-    return false;
+    return std::nullopt;
   }
 
-  if (!account_id) {
-    return true;
+  auto it = FindAccount(*account_list, account_id);
+  if (it == account_list->end()) {
+    return std::nullopt;
   }
 
-  return FindAccount(*account_list, *account_id) != account_list->end();
+  if (it->is_string()) {
+    // The account is returning but we do not have a timestamp.
+    return base::Time();
+  } else if (it->is_dict()) {
+    base::Value* timestamp = it->GetDict().Find(kTimestampKey);
+    CHECK(timestamp);
+    std::optional<base::Time> time = base::ValueToTime(timestamp);
+    // We stored a time in here, so it shouldn't fail when retrieving it.
+    DCHECK(time);
+    return time.value_or(base::Time());
+  }
+  // We do not expect this to happen, but account was found and no timestamp in
+  // this case.
+  return base::Time();
 }
 
 void FederatedIdentityAccountKeyedPermissionContext::GrantPermission(

chrome/browser/webid/federated_identity_account_keyed_permission_context.h

@@ -59,13 +59,20 @@ class FederatedIdentityAccountKeyedPermissionContext
                      const net::SchemefulSite& identity_provider);
 
   // Returns whether there is an existing permission for the
-  // (relying_party_requester, relying_party_embedder, identity_provider,
-  // account_id) tuple. `account_id` can be omitted to represent "sharing
-  // permission for any account".
+  // (relying_party_requester, relying_party_embedder, identity_provider) tuple.
   bool HasPermission(const url::Origin& relying_party_requester,
                      const url::Origin& relying_party_embedder,
-                     const url::Origin& identity_provider,
-                     const std::optional<std::string>& account_id);
+                     const url::Origin& identity_provider);
+
+  // Returns the last time when `account_id` was used via FedCM on the
+  // (relying_party_requester, relying_party_embedder, identity_provider). If
+  // there is no known last time, returns nullopt. If the `account_id` was known
+  // to be used but a timestamp is not known, returns 0.
+  std::optional<base::Time> GetLastUsedTimestamp(
+      const url::Origin& relying_party_requester,
+      const url::Origin& relying_party_embedder,
+      const url::Origin& identity_provider,
+      const std::string& account_id);
 
   // Grants permission for the (relying_party_requester, relying_party_embedder,
   // identity_provider, account_id) tuple.

chrome/browser/webid/federated_identity_account_keyed_permission_context_unittest.cc

@@ -151,12 +151,17 @@ TEST_F(FederatedIdentityAccountKeyedPermissionContextTest,
 
   // Permissions in the old format should only be returned when
   // relying-party-requester == relying-party-embedder.
-  EXPECT_TRUE(context()->HasPermission(rp, rp, idp1, account_a));
-  EXPECT_TRUE(context()->HasPermission(rp, rp, idp1, account_b));
-  EXPECT_TRUE(context()->HasPermission(rp, rp, idp2, account_c));
-  EXPECT_FALSE(context()->HasPermission(rp, other_origin, idp1, account_a));
-
-  EXPECT_FALSE(context()->HasPermission(rp, rp, idp1, account_c));
+  EXPECT_EQ(context()->GetLastUsedTimestamp(rp, rp, idp1, account_a),
+            base::Time());
+  EXPECT_EQ(context()->GetLastUsedTimestamp(rp, rp, idp1, account_b),
+            base::Time());
+  EXPECT_EQ(context()->GetLastUsedTimestamp(rp, rp, idp2, account_c),
+            base::Time());
+  EXPECT_EQ(context()->GetLastUsedTimestamp(rp, other_origin, idp1, account_a),
+            std::nullopt);
+
+  EXPECT_EQ(context()->GetLastUsedTimestamp(rp, rp, idp1, account_c),
+            std::nullopt);
 }
 
 namespace {
@@ -177,41 +182,41 @@ void TestGrantAndRevoke(FederatedIdentityAccountKeyedPermissionContext* context,
                            grant1.relying_party_embedder,
                            grant1.identity_provider, grant1.account_id);
 
-  EXPECT_TRUE(context->HasPermission(
+  EXPECT_TRUE(context->GetLastUsedTimestamp(
       grant1.relying_party_requester, grant1.relying_party_embedder,
       grant1.identity_provider, grant1.account_id));
-  EXPECT_FALSE(context->HasPermission(
+  EXPECT_FALSE(context->GetLastUsedTimestamp(
       grant2.relying_party_requester, grant2.relying_party_embedder,
       grant2.identity_provider, grant2.account_id));
 
   context->GrantPermission(grant2.relying_party_requester,
                            grant2.relying_party_embedder,
                            grant2.identity_provider, grant2.account_id);
 
-  EXPECT_TRUE(context->HasPermission(
+  EXPECT_TRUE(context->GetLastUsedTimestamp(
       grant1.relying_party_requester, grant1.relying_party_embedder,
       grant1.identity_provider, grant1.account_id));
-  EXPECT_TRUE(context->HasPermission(
+  EXPECT_TRUE(context->GetLastUsedTimestamp(
       grant2.relying_party_requester, grant2.relying_party_embedder,
       grant2.identity_provider, grant2.account_id));
 
   context->RevokePermission(
       grant1.relying_party_requester, grant1.relying_party_embedder,
       grant1.identity_provider, grant1.account_id, base::DoNothing());
-  EXPECT_FALSE(context->HasPermission(
+  EXPECT_FALSE(context->GetLastUsedTimestamp(
       grant1.relying_party_requester, grant1.relying_party_embedder,
       grant1.identity_provider, grant1.account_id));
-  EXPECT_TRUE(context->HasPermission(
+  EXPECT_TRUE(context->GetLastUsedTimestamp(
       grant2.relying_party_requester, grant2.relying_party_embedder,
       grant2.identity_provider, grant2.account_id));
 
   context->RevokePermission(
       grant2.relying_party_requester, grant2.relying_party_embedder,
       grant2.identity_provider, grant2.account_id, base::DoNothing());
-  EXPECT_FALSE(context->HasPermission(
+  EXPECT_FALSE(context->GetLastUsedTimestamp(
       grant1.relying_party_requester, grant1.relying_party_embedder,
       grant1.identity_provider, grant1.account_id));
-  EXPECT_FALSE(context->HasPermission(
+  EXPECT_FALSE(context->GetLastUsedTimestamp(
       grant2.relying_party_requester, grant2.relying_party_embedder,
       grant2.identity_provider, grant2.account_id));
 
@@ -312,7 +317,7 @@ TEST_F(FederatedIdentityAccountKeyedPermissionContextTest, RecoverFrom1381130) {
   context()->GrantObjectPermission(site, std::move(new_object));
 
   context()->GrantPermission(site, site, site, account);
-  EXPECT_TRUE(context()->HasPermission(site, site, site, account));
+  EXPECT_TRUE(context()->GetLastUsedTimestamp(site, site, site, account));
 }
 
 TEST_F(FederatedIdentityAccountKeyedPermissionContextTest,
@@ -353,25 +358,26 @@ TEST_F(FederatedIdentityAccountKeyedPermissionContextTest, RevokeNoMatch) {
                               base::DoNothing());
 
   context()->GrantPermission(rpRequester, rpEmbedder, idp, kAccountId);
-  EXPECT_TRUE(
-      context()->HasPermission(rpRequester, rpEmbedder, idp, kAccountId));
+  EXPECT_TRUE(context()->GetLastUsedTimestamp(rpRequester, rpEmbedder, idp,
+                                              kAccountId));
 
   // Revoke will remove the permission even if the account ID does not
   // match.
   context()->RevokePermission(rpRequester, rpEmbedder, idp, "noMatch",
                               base::DoNothing());
-  EXPECT_FALSE(
-      context()->HasPermission(rpRequester, rpEmbedder, idp, kAccountId));
+  EXPECT_FALSE(context()->GetLastUsedTimestamp(rpRequester, rpEmbedder, idp,
+                                               kAccountId));
 
   // Revoke will remove the permission when the account ID matches, but
   // only that permission.
   context()->GrantPermission(rpRequester, rpEmbedder, idp, kAccountId);
   context()->GrantPermission(rpRequester, rpEmbedder, idp, "other");
   context()->RevokePermission(rpRequester, rpEmbedder, idp, kAccountId,
                               base::DoNothing());
-  EXPECT_FALSE(
-      context()->HasPermission(rpRequester, rpEmbedder, idp, kAccountId));
-  EXPECT_TRUE(context()->HasPermission(rpRequester, rpEmbedder, idp, "other"));
+  EXPECT_FALSE(context()->GetLastUsedTimestamp(rpRequester, rpEmbedder, idp,
+                                               kAccountId));
+  EXPECT_TRUE(
+      context()->GetLastUsedTimestamp(rpRequester, rpEmbedder, idp, "other"));
 }
 
 TEST_F(FederatedIdentityAccountKeyedPermissionContextTest,
@@ -474,18 +480,17 @@ TEST_F(FederatedIdentityAccountKeyedPermissionContextTest,
   context()->GrantObjectPermission(relying_party_requester,
                                    std::move(new_object));
 
-  EXPECT_TRUE(context()->HasPermission(relying_party_requester,
-                                       relying_party_embedder,
-                                       identity_provider, account_a));
-  EXPECT_TRUE(context()->HasPermission(relying_party_requester,
-                                       relying_party_embedder,
-                                       identity_provider, account_b));
-  EXPECT_FALSE(context()->HasPermission(relying_party_requester,
-                                        relying_party_embedder,
-                                        identity_provider, account_c));
-  EXPECT_TRUE(
-      context()->HasPermission(relying_party_requester, relying_party_embedder,
-                               identity_provider, /*account_id=*/std::nullopt));
+  EXPECT_TRUE(context()->GetLastUsedTimestamp(relying_party_requester,
+                                              relying_party_embedder,
+                                              identity_provider, account_a));
+  EXPECT_TRUE(context()->GetLastUsedTimestamp(relying_party_requester,
+                                              relying_party_embedder,
+                                              identity_provider, account_b));
+  EXPECT_FALSE(context()->GetLastUsedTimestamp(relying_party_requester,
+                                               relying_party_embedder,
+                                               identity_provider, account_c));
+  EXPECT_TRUE(context()->HasPermission(
+      relying_party_requester, relying_party_embedder, identity_provider));
 
   // RefreshExistingPermission works with an old account but does not work if
   // account does not exist.
@@ -500,15 +505,15 @@ TEST_F(FederatedIdentityAccountKeyedPermissionContextTest,
   context()->GrantPermission(relying_party_requester, relying_party_embedder,
                              identity_provider, account_c);
 
-  EXPECT_TRUE(context()->HasPermission(relying_party_requester,
-                                       relying_party_embedder,
-                                       identity_provider, account_a));
-  EXPECT_TRUE(context()->HasPermission(relying_party_requester,
-                                       relying_party_embedder,
-                                       identity_provider, account_b));
-  EXPECT_TRUE(context()->HasPermission(relying_party_requester,
-                                       relying_party_embedder,
-                                       identity_provider, account_c));
+  EXPECT_TRUE(context()->GetLastUsedTimestamp(relying_party_requester,
+                                              relying_party_embedder,
+                                              identity_provider, account_a));
+  EXPECT_TRUE(context()->GetLastUsedTimestamp(relying_party_requester,
+                                              relying_party_embedder,
+                                              identity_provider, account_b));
+  EXPECT_TRUE(context()->GetLastUsedTimestamp(relying_party_requester,
+                                              relying_party_embedder,
+                                              identity_provider, account_c));
 
   // RefreshExistingPermission works with the new format.
   EXPECT_TRUE(context()->RefreshExistingPermission(
@@ -531,16 +536,15 @@ TEST_F(FederatedIdentityAccountKeyedPermissionContextTest,
                               future2.GetCallback());
   ASSERT_TRUE(future2.Wait());
 
-  EXPECT_FALSE(context()->HasPermission(relying_party_requester,
-                                        relying_party_embedder,
-                                        identity_provider, account_a));
-  EXPECT_TRUE(context()->HasPermission(relying_party_requester,
-                                       relying_party_embedder,
-                                       identity_provider, account_b));
-  EXPECT_FALSE(context()->HasPermission(relying_party_requester,
-                                        relying_party_embedder,
-                                        identity_provider, account_c));
-  EXPECT_TRUE(
-      context()->HasPermission(relying_party_requester, relying_party_embedder,
-                               identity_provider, /*account_id=*/std::nullopt));
+  EXPECT_FALSE(context()->GetLastUsedTimestamp(relying_party_requester,
+                                               relying_party_embedder,
+                                               identity_provider, account_a));
+  EXPECT_TRUE(context()->GetLastUsedTimestamp(relying_party_requester,
+                                              relying_party_embedder,
+                                              identity_provider, account_b));
+  EXPECT_FALSE(context()->GetLastUsedTimestamp(relying_party_requester,
+                                               relying_party_embedder,
+                                               identity_provider, account_c));
+  EXPECT_TRUE(context()->HasPermission(
+      relying_party_requester, relying_party_embedder, identity_provider));
 }

chrome/browser/webid/federated_identity_permission_context.cc

@@ -56,13 +56,22 @@ void FederatedIdentityPermissionContext::RemoveIdpSigninStatusObserver(
 }
 
 bool FederatedIdentityPermissionContext::HasSharingPermission(
+    const url::Origin& relying_party_requester,
+    const url::Origin& relying_party_embedder,
+    const url::Origin& identity_provider) {
+  return sharing_context_->HasPermission(
+      relying_party_requester, relying_party_embedder, identity_provider);
+}
+
+std::optional<base::Time>
+FederatedIdentityPermissionContext::GetLastUsedTimestamp(
     const url::Origin& relying_party_requester,
     const url::Origin& relying_party_embedder,
     const url::Origin& identity_provider,
-    const std::optional<std::string>& account_id) {
-  return sharing_context_->HasPermission(relying_party_requester,
-                                         relying_party_embedder,
-                                         identity_provider, account_id);
+    const std::string& account_id) {
+  return sharing_context_->GetLastUsedTimestamp(relying_party_requester,
+                                                relying_party_embedder,
+                                                identity_provider, account_id);
 }
 
 bool FederatedIdentityPermissionContext::HasSharingPermission(

chrome/browser/webid/federated_identity_permission_context.h

@@ -49,11 +49,14 @@ class FederatedIdentityPermissionContext
   void AddIdpSigninStatusObserver(IdpSigninStatusObserver* observer) override;
   void RemoveIdpSigninStatusObserver(
       IdpSigninStatusObserver* observer) override;
-  bool HasSharingPermission(
+  bool HasSharingPermission(const url::Origin& relying_party_requester,
+                            const url::Origin& relying_party_embedder,
+                            const url::Origin& identity_provider) override;
+  std::optional<base::Time> GetLastUsedTimestamp(
       const url::Origin& relying_party_requester,
       const url::Origin& relying_party_embedder,
       const url::Origin& identity_provider,
-      const std::optional<std::string>& account_id) override;
+      const std::string& account_id) override;
   bool HasSharingPermission(
       const url::Origin& relying_party_requester) override;
   void GrantSharingPermission(const url::Origin& relying_party_requester,

chrome/browser/webid/federated_identity_permission_context_unittest.cc

@@ -46,20 +46,20 @@ TEST_F(FederatedIdentityPermissionContextTest, RevokeSharingPermission) {
       url::Origin::Create(GURL("https://idp.com"));
   constexpr std::string kAccountId = "accountId";
 
-  EXPECT_FALSE(context_->HasSharingPermission(kRequester, kEmbedder,
+  EXPECT_FALSE(context_->GetLastUsedTimestamp(kRequester, kEmbedder,
                                               kIdentityProvider, kAccountId));
   EXPECT_FALSE(context_->HasSharingPermission(kRequester));
 
   context_->GrantSharingPermission(kRequester, kEmbedder, kIdentityProvider,
                                    kAccountId);
-  EXPECT_TRUE(context_->HasSharingPermission(kRequester, kEmbedder,
+  EXPECT_TRUE(context_->GetLastUsedTimestamp(kRequester, kEmbedder,
                                              kIdentityProvider, kAccountId));
   EXPECT_TRUE(context_->HasSharingPermission(kRequester));
   EXPECT_FALSE(context_->HasSharingPermission(kEmbedder));
 
   context_->RevokeSharingPermission(kRequester, kEmbedder, kIdentityProvider,
                                     kAccountId);
-  EXPECT_FALSE(context_->HasSharingPermission(kRequester, kEmbedder,
+  EXPECT_FALSE(context_->GetLastUsedTimestamp(kRequester, kEmbedder,
                                               kIdentityProvider, kAccountId));
   EXPECT_FALSE(context_->HasSharingPermission(kRequester));
 }