From 28cfa4e29942cdc9e01340660a2e3a0782c548f8 Mon Sep 17 00:00:00 2001 From: Christian Biesinger Date: Mon, 2 Oct 2023 16:49:45 +0000 Subject: [PATCH] [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: https://github.com/fedidcg/FedCM/pull/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 Commit-Queue: Christian Biesinger Reviewed-by: Andrey Kosyakov Cr-Commit-Position: refs/heads/main@{#1204075} --- client/chromedriver.py | 4 ++-- client/command_executor.py | 4 ++-- fedcm_commands.cc | 12 ++++++------ fedcm_commands.h | 10 +++++----- fedcm_commands_unittest.cc | 8 ++++---- server/http_handler.cc | 6 +++--- test/run_py_tests.py | 6 +++--- 7 files changed, 25 insertions(+), 25 deletions(-) diff --git a/client/chromedriver.py b/client/chromedriver.py index 3b19a39e..1826a953 100644 --- 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 index 82a1687d..c13f1575 100644 --- 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 index b2ba07d6..0cbc3b2b 100644 --- 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* value, - Timeout* timeout) { +Status ExecuteConfirmIdpLogin(Session* session, + WebView* web_view, + const base::Value::Dict& params, + std::unique_ptr* 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 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 index f47c0497..7f721bd8 100644 --- a/fedcm_commands.h +++ b/fedcm_commands.h @@ -27,11 +27,11 @@ Status ExecuteSelectAccount(Session* session, std::unique_ptr* value, Timeout* timeout); -Status ExecuteConfirmIdpSignin(Session* session, - WebView* web_view, - const base::Value::Dict& params, - std::unique_ptr* value, - Timeout* timeout); +Status ExecuteConfirmIdpLogin(Session* session, + WebView* web_view, + const base::Value::Dict& params, + std::unique_ptr* value, + Timeout* timeout); Status ExecuteGetAccounts(Session* session, WebView* web_view, diff --git a/fedcm_commands_unittest.cc b/fedcm_commands_unittest.cc index 578a8bb0..db288d05 100644 --- 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 index c304f59c..bdd1c5eb 100644 --- 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 index 983cf753..a48faed6 100755 --- 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()