[FedCM] Revoke at least one account after sending IDP request

If an invalid account hint is sent, the user agent should still revoke an existing sharing permission. This guarantees that revoke() cannot be repeatedly called while sending a credentialed request to the IDP each time. We also remove a comment about the FedCM settings, as it has been decided that the FedCM setting should affect revoke() as well. Bug: 1473134, 1495108 Change-Id: I65154c049fa7811b6507d3054678ae718916e7ab Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5019345 Commit-Queue: Nicolás Peña <npm@chromium.org> Reviewed-by: danakj <danakj@chromium.org> Cr-Commit-Position: refs/heads/main@{#1225125}
web-platform-tests · Nov 15, 2023 · 6759f84 · 6759f84
1 parent 1bc99b6
commit 6759f84
Show file tree

Hide file tree

Showing 5 changed files with 100 additions and 42 deletions.
diff --git a/credential-management/fedcm-revoke.https.html b/credential-management/fedcm-revoke.https.html
diff --git a/credential-management/fedcm-revoke.sub.https.html b/credential-management/fedcm-revoke.sub.https.html
@@ -0,0 +1,81 @@
+<!DOCTYPE html>
+<title>Federated Credential Management API revoke() tests.</title>
+<link rel="help" href="https://fedidcg.github.io/FedCM">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/testdriver.js"></script>
+<script src="/resources/testdriver-vendor.js"></script>
+
+<body>
+
+<script type="module">
+import {fedcm_test,
+        mark_signed_in,
+        set_fedcm_cookie,
+        revoke_options,
+        fedcm_get_and_select_first_account,
+        request_options_with_mediation_required,
+        alt_manifest_origin,
+        alt_request_options_with_mediation_required,
+        alt_revoke_options,
+        set_alt_fedcm_cookie} from './support/fedcm-helper.sub.js';
+
+fedcm_test(async t => {
+  await mark_signed_in();
+  await set_fedcm_cookie();
+  // Get at least one connected account that can be revoked.
+  const cred = await fedcm_get_and_select_first_account(t, request_options_with_mediation_required());
+  // The IDP implementation will accept any account hint, so this is really testing that the user
+  // agent eventually stops sending the requests to the IDP.
+  // This test clears the connection just created above, but it also clears any previously existing
+  // connected accounts, which helps the logic of the other tests.
+  return new Promise(async resolve => {
+    while (true) {
+      try {
+        await IdentityCredential.revoke(revoke_options("1234"));
+      } catch(e) {
+        resolve();
+        break;
+      }
+    }
+  });
+}, "Repeatedly calling revoke should eventually fail");
+
+fedcm_test(async t => {
+  const revoke = IdentityCredential.revoke(revoke_options("nonExistent"));
+  return promise_rejects_dom(t, 'NetworkError', revoke);
+}, 'Test that revoke fails when there is no account to revoke');
+
+fedcm_test(async t => {
+  const cred = await fedcm_get_and_select_first_account(t, request_options_with_mediation_required());
+
+  return IdentityCredential.revoke(revoke_options("1234"));
+}, 'Test that revoke succeeds when there is an account to revoke');
+
+fedcm_test(async t => {
+  const cred = await fedcm_get_and_select_first_account(t, request_options_with_mediation_required());
+
+  await IdentityCredential.revoke(revoke_options("1234"));
+
+  const revoke = IdentityCredential.revoke(revoke_options("1234"));
+  return promise_rejects_dom(t, 'NetworkError', revoke);
+}, 'Test that revoking the same account twice results in failure.');
+
+fedcm_test(async t => {
+  const cred = await fedcm_get_and_select_first_account(t, request_options_with_mediation_required());
+  // A connected account is guaranteed by the above, and IDP accepts any account hint, so this tests
+  // that the user agent allows the request to go through to the IDP.
+  return IdentityCredential.revoke(revoke_options("noMatch"));
+}, 'Revoke passing an incorrect ID can still succeed');
+
+fedcm_test(async t => {
+  await set_alt_fedcm_cookie();
+  await mark_signed_in(alt_manifest_origin);
+  await fedcm_get_and_select_first_account(t, alt_request_options_with_mediation_required());
+  await fedcm_get_and_select_first_account(t, request_options_with_mediation_required());
+
+  // Await the first revocation since they cannot happen in parallel. Both should succeed.
+  await IdentityCredential.revoke(revoke_options("1"));
+  return IdentityCredential.revoke(alt_revoke_options("2"));
+}, 'Revocation is bound to each IDP');
+</script>
diff --git a/credential-management/support/fedcm-helper.sub.js b/credential-management/support/fedcm-helper.sub.js
@@ -225,3 +225,16 @@ credential-management/support/fedcm/${manifest_filename}`;
       accountHint: accountHint
       };
 }
+
+export function alt_revoke_options(accountHint, manifest_filename) {
+  if (manifest_filename === undefined) {
+    manifest_filename = "manifest.py";
+  }
+  const manifest_path = `${alt_manifest_origin}/\
+credential-management/support/fedcm/${manifest_filename}`;
+  return {
+      configURL: manifest_path,
+      clientId: '1',
+      accountHint: accountHint
+  };
+}
diff --git a/credential-management/support/fedcm/revoke.py b/credential-management/support/fedcm/revoke.py
@@ -2,12 +2,13 @@
 error_checker = importlib.import_module("credential-management.support.fedcm.request-params-check")
 
 def main(request, response):
+  response.headers.set(b"Content-Type", b"application/json")
+  response.headers.set(b"Access-Control-Allow-Origin", request.headers.get(b"origin"))
+  response.headers.set(b"Access-Control-Allow-Credentials", b"true")
   request_error = error_checker.revokeCheck(request)
-  if (request_error):
+  if request_error:
     return request_error
 
-  response.headers.set(b"Content-Type", b"application/json")
-
   # Pass the account_hint as the accountId.
-  account_hint = request.POST.get(b"account_hint");
+  account_hint = request.POST.get(b"account_hint")
   return f"{{\"account_id\": \"{account_hint}\"}}"
diff --git a/credential-management/support/set_cookie.headers b/credential-management/support/set_cookie.headers
@@ -1,2 +1,2 @@
 Content-Type: text/html
-Set-Cookie: cookie=1; SameSite=Strict; Secure
+Set-Cookie: cookie=1; SameSite=None; Secure