[FedCM] Standardize on Login instead of Signin for browser automation

Based on recent discussions in the fedidcg, we prefer Login over Signin. This CL changes the exposed commands accordingly. Note that the IDP signin feature is still in origin trial and thus subject to change. Additionally, the confirmidpsignin command is vendor-prefixed and is not shipped yet, so it should be safe to change: https://chromiumdash.appspot.com/commit/c3773f834360675a1fffc8c147a8f9012b6b25af Spec PR: w3c-fedid/FedCM#436 Validate-Test-Flakiness: skip Low-Coverage-Reason: rename only, no functional changes Bug: 1451884 Change-Id: Ic3b3f3e144c6649b19f063258403c239e9ceac82 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4901302 Reviewed-by: Vladimir Nechaev <[email protected]> Commit-Queue: Christian Biesinger <[email protected]> Reviewed-by: Andrey Kosyakov <[email protected]> Cr-Commit-Position: refs/heads/main@{#1204075}
bayandin · Oct 2, 2023 · 28cfa4e · 28cfa4e
1 parent f95ef2c
commit 28cfa4e
Show file tree

Hide file tree

Showing 7 changed files with 25 additions and 25 deletions.
diff --git a/client/chromedriver.py b/client/chromedriver.py
@@ -746,9 +746,9 @@ def SelectAccount(self, index):
     params = {'accountIndex': index}
     return self.ExecuteCommand(Command.SELECT_ACCOUNT, params)
 
-  def ConfirmIdpSignin(self, vendorId):
+  def ConfirmIdpLogin(self, vendorId):
     params = {'vendorId': vendorId}
-    return self.ExecuteCommand(Command.CONFIRM_IDP_SIGNIN, params)
+    return self.ExecuteCommand(Command.CONFIRM_IDP_LOGIN, params)
 
   def GetAccounts(self):
     return self.ExecuteCommand(Command.GET_ACCOUNTS, {})

diff --git a/client/command_executor.py b/client/command_executor.py
@@ -227,9 +227,9 @@ class Command(object):
   SELECT_ACCOUNT = (
       _Method.POST,
       '/session/:sessionId/fedcm/selectaccount')
-  CONFIRM_IDP_SIGNIN = (
+  CONFIRM_IDP_LOGIN = (
       _Method.POST,
-      '/session/:sessionId/:vendorId/fedcm/confirmidpsignin')
+      '/session/:sessionId/:vendorId/fedcm/confirmidplogin')
   GET_ACCOUNTS = (
       _Method.GET,
       '/session/:sessionId/fedcm/accountlist')

diff --git a/fedcm_commands.cc b/fedcm_commands.cc
@@ -62,11 +62,11 @@ Status ExecuteSelectAccount(Session* session,
   return status;
 }
 
-Status ExecuteConfirmIdpSignin(Session* session,
-                               WebView* web_view,
-                               const base::Value::Dict& params,
-                               std::unique_ptr<base::Value>* value,
-                               Timeout* timeout) {
+Status ExecuteConfirmIdpLogin(Session* session,
+                              WebView* web_view,
+                              const base::Value::Dict& params,
+                              std::unique_ptr<base::Value>* value,
+                              Timeout* timeout) {
   FedCmTracker* tracker = nullptr;
   Status status = web_view->GetFedCmTracker(&tracker);
   if (!status.IsOk()) {
@@ -80,7 +80,7 @@ Status ExecuteConfirmIdpSignin(Session* session,
   command_params.Set("dialogId", tracker->GetLastDialogId());
 
   std::unique_ptr<base::Value> result;
-  status = web_view->SendCommandAndGetResult("FedCm.confirmIdpSignin",
+  status = web_view->SendCommandAndGetResult("FedCm.confirmIdpLogin",
                                              command_params, &result);
   tracker->DialogClosed();
   return status;

diff --git a/fedcm_commands.h b/fedcm_commands.h
@@ -27,11 +27,11 @@ Status ExecuteSelectAccount(Session* session,
                             std::unique_ptr<base::Value>* value,
                             Timeout* timeout);
 
-Status ExecuteConfirmIdpSignin(Session* session,
-                               WebView* web_view,
-                               const base::Value::Dict& params,
-                               std::unique_ptr<base::Value>* value,
-                               Timeout* timeout);
+Status ExecuteConfirmIdpLogin(Session* session,
+                              WebView* web_view,
+                              const base::Value::Dict& params,
+                              std::unique_ptr<base::Value>* value,
+                              Timeout* timeout);
 
 Status ExecuteGetAccounts(Session* session,
                           WebView* web_view,

diff --git a/fedcm_commands_unittest.cc b/fedcm_commands_unittest.cc
@@ -44,7 +44,7 @@ class MockResponseWebView : public StubWebView {
     account.Set("givenName", "Foo");
     account.Set("pictureUrl", "https://pics/pic.jpg");
     account.Set("idpConfigUrl", "https://idp.example/fedcm.json");
-    account.Set("idpSigninUrl", "https://idp.example/signin");
+    account.Set("idpLoginUrl", "https://idp.example/login");
     account.Set("loginState", "SignIn");
 
     base::Value::List accounts;
@@ -145,9 +145,9 @@ TEST_F(FedCmCommandsTest, ExecuteGetAccounts) {
   std::string* idpConfigUrl = account->FindString("idpConfigUrl");
   ASSERT_TRUE(idpConfigUrl);
   EXPECT_EQ(*idpConfigUrl, "https://idp.example/fedcm.json");
-  std::string* idpSigninUrl = account->FindString("idpSigninUrl");
-  ASSERT_TRUE(idpSigninUrl);
-  EXPECT_EQ(*idpSigninUrl, "https://idp.example/signin");
+  std::string* idpLoginUrl = account->FindString("idpLoginUrl");
+  ASSERT_TRUE(idpLoginUrl);
+  EXPECT_EQ(*idpLoginUrl, "https://idp.example/login");
   std::string* loginState = account->FindString("loginState");
   ASSERT_TRUE(loginState);
   EXPECT_EQ(*loginState, "SignIn");

diff --git a/server/http_handler.cc b/server/http_handler.cc
@@ -950,9 +950,9 @@ HttpHandler::HttpHandler(
       // This command is prefixed because standardization is still pending:
       // https://github.com/fedidcg/FedCM/pull/436/files
       VendorPrefixedCommandMapping(
-          kPost, "session/:sessionId/%s/fedcm/confirmidpsignin",
-          WrapToCommand("ConfirmIdpSignin",
-                        base::BindRepeating(&ExecuteConfirmIdpSignin))),
+          kPost, "session/:sessionId/%s/fedcm/confirmidplogin",
+          WrapToCommand("ConfirmIdpLogin",
+                        base::BindRepeating(&ExecuteConfirmIdpLogin))),
 
       CommandMapping(kGet, "session/:sessionId/fedcm/accountlist",
                      WrapToCommand("GetAccounts",

diff --git a/test/run_py_tests.py b/test/run_py_tests.py
@@ -6985,7 +6985,7 @@ def testDismissDialog(self):
     token = self._driver.ExecuteScript('return getResult()')
     self.assertEqual('Error: NetworkError: Error retrieving a token.', token)
 
-  def testConfirmIdpSignin(self):
+  def testConfirmIdpLogin(self):
     self._accounts = ""
 
     self._driver.Load(self._https_server.GetUrl() + "/fedcm.html")
@@ -6998,12 +6998,12 @@ def testConfirmIdpSignin(self):
     self.assertTrue(self.WaitForCondition(self.FedCmDialogCondition))
 
     accounts = self._driver.GetAccounts()
-    self.assertEqual("ConfirmIdpSignin", self._driver.GetDialogType())
+    self.assertEqual("ConfirmIdpLogin", self._driver.GetDialogType())
     self.assertEqual(0, len(accounts))
 
     self._accounts = self._default_accounts
 
-    self._driver.ConfirmIdpSignin(self._vendor_id)
+    self._driver.ConfirmIdpLogin(self._vendor_id)
 
     self.assertTrue(self.WaitForCondition(self.FedCmDialogCondition))
     accounts = self._driver.GetAccounts()