[FedCM] New syntax for account labels and use other account
Allows "supports_use_other_account" in the toplevel of the config file. Also renames "labels" in the account endpoint to "label_hints" and flattens "accounts: { include: "foo" }" to just a toplevel "account_label". All changes behind the new "FedCmOtherAccountAndLabelsNewSyntax" flag. Even with the flag enabled, the old syntax is still accepted for now. As per the discussions here: https://github.com/w3c-fedid/FedCM/pull/669 https://github.com/w3c-fedid/FedCM/pull/678 Bug: 404568028 Change-Id: Iba33c5f546c68f35ca2a1228096a7a31a68767a9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6370034 Reviewed-by: Nicolás Peña <npm@chromium.org> Reviewed-by: Alex Moshchuk <alexmos@chromium.org> Commit-Queue: Christian Biesinger <cbiesinger@chromium.org> Cr-Commit-Position: refs/heads/main@{#1435018}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
1f4ab6af32
commit
221a164a0c
@ -90,4 +90,9 @@ bool IsFedCmAlternativeIdentifiersEnabled() {
|
||||
bool IsFedCmCooldownOnIgnoreEnabled() {
|
||||
return base::FeatureList::IsEnabled(features::kFedCmCooldownOnIgnore);
|
||||
}
|
||||
|
||||
bool IsFedCmUseOtherAccountAndLabelsNewSyntaxEnabled() {
|
||||
return base::FeatureList::IsEnabled(
|
||||
features::kFedCmUseOtherAccountAndLabelsNewSyntax);
|
||||
}
|
||||
} // namespace content
|
||||
|
@ -65,6 +65,10 @@ bool IsFedCmAlternativeIdentifiersEnabled();
|
||||
|
||||
// Whether cooldown on ignore is enabled.
|
||||
bool IsFedCmCooldownOnIgnoreEnabled();
|
||||
|
||||
// Whether to support the newer syntax for the "Use Other Account"
|
||||
// and account labels features.
|
||||
bool IsFedCmUseOtherAccountAndLabelsNewSyntaxEnabled();
|
||||
} // namespace content
|
||||
|
||||
#endif // CONTENT_BROWSER_WEBID_FLAGS_H_
|
||||
|
@ -77,6 +77,7 @@ constexpr char kDisconnectEndpoint[] = "disconnect_endpoint";
|
||||
constexpr char kModesKey[] = "modes";
|
||||
constexpr char kTypesKey[] = "types";
|
||||
constexpr char kFormatsKey[] = "formats";
|
||||
constexpr char kAccountLabelKey[] = "account_label";
|
||||
|
||||
// Keys in the 'accounts' dictionary
|
||||
constexpr char kIncludeKey[] = "include";
|
||||
@ -116,6 +117,7 @@ constexpr char kAccountApprovedClientsKey[] = "approved_clients";
|
||||
constexpr char kHintsKey[] = "login_hints";
|
||||
constexpr char kDomainHintsKey[] = "domain_hints";
|
||||
constexpr char kLabelsKey[] = "labels";
|
||||
constexpr char kLabelHintsKey[] = "label_hints";
|
||||
|
||||
// Keys in 'branding' 'icons' dictionary in config for the IDP icon and client
|
||||
// metadata endpoint for the RP icon.
|
||||
@ -255,7 +257,13 @@ IdentityRequestAccountPtr ParseAccount(const base::Value::Dict& account,
|
||||
}
|
||||
|
||||
std::vector<std::string> labels;
|
||||
auto* labels_list = account.FindList(kLabelsKey);
|
||||
const base::ListValue* labels_list = nullptr;
|
||||
if (IsFedCmUseOtherAccountAndLabelsNewSyntaxEnabled()) {
|
||||
labels_list = account.FindList(kLabelHintsKey);
|
||||
}
|
||||
if (!labels_list) {
|
||||
labels_list = account.FindList(kLabelsKey);
|
||||
}
|
||||
if (labels_list) {
|
||||
for (const base::Value& entry : *labels_list) {
|
||||
if (entry.is_string()) {
|
||||
@ -652,31 +660,44 @@ void OnConfigParsed(const GURL& provider,
|
||||
}
|
||||
}
|
||||
|
||||
const base::Value::Dict* accounts_dict = response.FindDict(kAccountsKey);
|
||||
if (accounts_dict) {
|
||||
const std::string* requested_label = accounts_dict->FindString(kIncludeKey);
|
||||
if (requested_label) {
|
||||
idp_metadata.requested_label = *requested_label;
|
||||
const std::string* requested_label = nullptr;
|
||||
if (IsFedCmUseOtherAccountAndLabelsNewSyntaxEnabled()) {
|
||||
requested_label = response.FindString(kAccountLabelKey);
|
||||
}
|
||||
if (!requested_label) {
|
||||
const base::Value::Dict* accounts_dict = response.FindDict(kAccountsKey);
|
||||
if (accounts_dict) {
|
||||
requested_label = accounts_dict->FindString(kIncludeKey);
|
||||
}
|
||||
}
|
||||
if (requested_label) {
|
||||
idp_metadata.requested_label = *requested_label;
|
||||
}
|
||||
|
||||
if (IsFedCmUseOtherAccountEnabled()) {
|
||||
const base::Value::Dict* modes_dict = response.FindDict(kModesKey);
|
||||
const base::Value::Dict* selected_mode_dict = nullptr;
|
||||
if (modes_dict) {
|
||||
switch (rp_mode) {
|
||||
case blink::mojom::RpMode::kPassive:
|
||||
selected_mode_dict = modes_dict->FindDict(kPassiveModeKey);
|
||||
break;
|
||||
case blink::mojom::RpMode::kActive:
|
||||
selected_mode_dict = modes_dict->FindDict(kActiveModeKey);
|
||||
break;
|
||||
};
|
||||
std::optional<bool> supports_add_account;
|
||||
if (IsFedCmUseOtherAccountAndLabelsNewSyntaxEnabled()) {
|
||||
supports_add_account = response.FindBool(kSupportsUseOtherAccountKey);
|
||||
}
|
||||
|
||||
if (!supports_add_account) {
|
||||
const base::Value::Dict* modes_dict = response.FindDict(kModesKey);
|
||||
const base::Value::Dict* selected_mode_dict = nullptr;
|
||||
if (modes_dict) {
|
||||
switch (rp_mode) {
|
||||
case blink::mojom::RpMode::kPassive:
|
||||
selected_mode_dict = modes_dict->FindDict(kPassiveModeKey);
|
||||
break;
|
||||
case blink::mojom::RpMode::kActive:
|
||||
selected_mode_dict = modes_dict->FindDict(kActiveModeKey);
|
||||
break;
|
||||
};
|
||||
}
|
||||
if (selected_mode_dict) {
|
||||
supports_add_account =
|
||||
selected_mode_dict->FindBool(kSupportsUseOtherAccountKey);
|
||||
}
|
||||
}
|
||||
std::optional<bool> supports_add_account =
|
||||
selected_mode_dict
|
||||
? selected_mode_dict->FindBool(kSupportsUseOtherAccountKey)
|
||||
: std::nullopt;
|
||||
if (supports_add_account) {
|
||||
idp_metadata.supports_add_account = *supports_add_account;
|
||||
}
|
||||
|
@ -641,13 +641,15 @@ TEST_F(IdpNetworkRequestManagerTest, ParseAccountMalformed) {
|
||||
AccountsResponseInvalidReason::kResponseIsNotJsonOrDict, 1);
|
||||
}
|
||||
|
||||
TEST_F(IdpNetworkRequestManagerTest, ParseAccountLabels) {
|
||||
TEST_F(IdpNetworkRequestManagerTest, ParseAccountLabelsOldSyntax) {
|
||||
// New syntax should be ignored with the flag disabled.
|
||||
const auto* test_accounts_json = R"({
|
||||
"accounts" : [
|
||||
{
|
||||
"id": "1234",
|
||||
"email": "ken@idp.test",
|
||||
"name": "Ken R. Example",
|
||||
"label_hints": ["x1", 42, "x2"],
|
||||
"labels": ["l1", 42, "l2"]
|
||||
}
|
||||
]
|
||||
@ -667,6 +669,66 @@ TEST_F(IdpNetworkRequestManagerTest, ParseAccountLabels) {
|
||||
EXPECT_EQ("l2", accounts[0]->labels[1]);
|
||||
}
|
||||
|
||||
TEST_F(IdpNetworkRequestManagerTest, ParseAccountLabelsOldAndNewSyntax) {
|
||||
base::test::ScopedFeatureList list;
|
||||
list.InitAndEnableFeature(features::kFedCmUseOtherAccountAndLabelsNewSyntax);
|
||||
|
||||
// label_hints should take precedence.
|
||||
const auto* test_accounts_json = R"({
|
||||
"accounts" : [
|
||||
{
|
||||
"id": "1234",
|
||||
"email": "ken@idp.test",
|
||||
"name": "Ken R. Example",
|
||||
"label_hints": ["l1", 42, "l2"],
|
||||
"labels": ["x1", 42, "x2"]
|
||||
}
|
||||
]
|
||||
})";
|
||||
|
||||
FetchStatus accounts_response;
|
||||
std::vector<IdentityRequestAccountPtr> accounts;
|
||||
std::tie(accounts_response, accounts) =
|
||||
SendAccountsRequestAndWaitForResponse(test_accounts_json);
|
||||
|
||||
ASSERT_EQ(ParseStatus::kSuccess, accounts_response.parse_status);
|
||||
EXPECT_EQ(net::HTTP_OK, accounts_response.response_code);
|
||||
EXPECT_EQ("1234", accounts[0]->id);
|
||||
// The integer in the second position should be ignored.
|
||||
ASSERT_EQ(2u, accounts[0]->labels.size());
|
||||
EXPECT_EQ("l1", accounts[0]->labels[0]);
|
||||
EXPECT_EQ("l2", accounts[0]->labels[1]);
|
||||
}
|
||||
|
||||
TEST_F(IdpNetworkRequestManagerTest, ParseAccountLabelHints) {
|
||||
base::test::ScopedFeatureList list;
|
||||
list.InitAndEnableFeature(features::kFedCmUseOtherAccountAndLabelsNewSyntax);
|
||||
|
||||
const auto* test_accounts_json = R"({
|
||||
"accounts" : [
|
||||
{
|
||||
"id": "1234",
|
||||
"email": "ken@idp.test",
|
||||
"name": "Ken R. Example",
|
||||
"label_hints": ["l1", 42, "l2"]
|
||||
}
|
||||
]
|
||||
})";
|
||||
|
||||
FetchStatus accounts_response;
|
||||
std::vector<IdentityRequestAccountPtr> accounts;
|
||||
std::tie(accounts_response, accounts) =
|
||||
SendAccountsRequestAndWaitForResponse(test_accounts_json);
|
||||
|
||||
EXPECT_EQ(ParseStatus::kSuccess, accounts_response.parse_status);
|
||||
EXPECT_EQ(net::HTTP_OK, accounts_response.response_code);
|
||||
EXPECT_EQ("1234", accounts[0]->id);
|
||||
// The integer in the second position should be ignored.
|
||||
ASSERT_EQ(2u, accounts[0]->labels.size());
|
||||
EXPECT_EQ("l1", accounts[0]->labels[0]);
|
||||
EXPECT_EQ("l2", accounts[0]->labels[1]);
|
||||
}
|
||||
|
||||
TEST_F(IdpNetworkRequestManagerTest, ComputeWellKnownUrl) {
|
||||
EXPECT_EQ("https://localhost:8000/.well-known/web-identity",
|
||||
IdpNetworkRequestManager::ComputeWellKnownUrl(
|
||||
@ -1150,7 +1212,9 @@ TEST_F(IdpNetworkRequestManagerTest,
|
||||
base::test::ScopedFeatureList list;
|
||||
list.InitAndEnableFeature(features::kFedCmUseOtherAccount);
|
||||
|
||||
// The toplevel field should be ignored with the flag disabled.
|
||||
const char test_json[] = R"({
|
||||
"supports_use_other_account": false,
|
||||
"modes": {
|
||||
"passive": {
|
||||
"supports_use_other_account": true
|
||||
@ -1169,6 +1233,51 @@ TEST_F(IdpNetworkRequestManagerTest,
|
||||
EXPECT_EQ(true, idp_metadata.supports_add_account);
|
||||
}
|
||||
|
||||
TEST_F(IdpNetworkRequestManagerTest,
|
||||
ParseConfigSupportsOtherAccountOldAndNewSyntax) {
|
||||
base::test::ScopedFeatureList list;
|
||||
list.InitAndEnableFeature(features::kFedCmUseOtherAccountAndLabelsNewSyntax);
|
||||
|
||||
// The toplevel field should take precedence.
|
||||
const char test_json[] = R"({
|
||||
"supports_use_other_account": true,
|
||||
"modes": {
|
||||
"passive": {
|
||||
"supports_use_other_account": false
|
||||
}
|
||||
}
|
||||
})";
|
||||
|
||||
FetchStatus fetch_status;
|
||||
IdentityProviderMetadata idp_metadata;
|
||||
std::tie(fetch_status, idp_metadata) = SendConfigRequestAndWaitForResponse(
|
||||
test_json, net::HTTP_OK, "application/json",
|
||||
blink::mojom::RpMode::kPassive);
|
||||
|
||||
EXPECT_EQ(ParseStatus::kSuccess, fetch_status.parse_status);
|
||||
EXPECT_EQ(net::HTTP_OK, fetch_status.response_code);
|
||||
EXPECT_EQ(true, idp_metadata.supports_add_account);
|
||||
}
|
||||
|
||||
TEST_F(IdpNetworkRequestManagerTest, ParseConfigSupportsOtherAccountNewSyntax) {
|
||||
base::test::ScopedFeatureList list;
|
||||
list.InitAndEnableFeature(features::kFedCmUseOtherAccountAndLabelsNewSyntax);
|
||||
|
||||
const char test_json[] = R"({
|
||||
"supports_use_other_account": true
|
||||
})";
|
||||
|
||||
FetchStatus fetch_status;
|
||||
IdentityProviderMetadata idp_metadata;
|
||||
std::tie(fetch_status, idp_metadata) = SendConfigRequestAndWaitForResponse(
|
||||
test_json, net::HTTP_OK, "application/json",
|
||||
blink::mojom::RpMode::kPassive);
|
||||
|
||||
EXPECT_EQ(ParseStatus::kSuccess, fetch_status.parse_status);
|
||||
EXPECT_EQ(net::HTTP_OK, fetch_status.response_code);
|
||||
EXPECT_EQ(true, idp_metadata.supports_add_account);
|
||||
}
|
||||
|
||||
TEST_F(IdpNetworkRequestManagerTest,
|
||||
ParseConfigSupportsOtherAccountDifferentMode) {
|
||||
base::test::ScopedFeatureList list;
|
||||
@ -1258,8 +1367,10 @@ TEST_F(IdpNetworkRequestManagerTest,
|
||||
EXPECT_EQ(false, idp_metadata.supports_add_account);
|
||||
}
|
||||
|
||||
TEST_F(IdpNetworkRequestManagerTest, ParseConfigRequestedLabel) {
|
||||
TEST_F(IdpNetworkRequestManagerTest, ParseConfigRequestedLabelOldSyntax) {
|
||||
// New syntax should be ignored with flag disabled.
|
||||
const char test_json[] = R"({
|
||||
"account_label": "l1",
|
||||
"accounts": {
|
||||
"include": "l1"
|
||||
}
|
||||
@ -1275,6 +1386,46 @@ TEST_F(IdpNetworkRequestManagerTest, ParseConfigRequestedLabel) {
|
||||
EXPECT_EQ("l1", idp_metadata.requested_label);
|
||||
}
|
||||
|
||||
TEST_F(IdpNetworkRequestManagerTest, ParseConfigRequestedLabelOldAndNewSyntax) {
|
||||
base::test::ScopedFeatureList list;
|
||||
list.InitAndEnableFeature(features::kFedCmUseOtherAccountAndLabelsNewSyntax);
|
||||
|
||||
// New syntax should take precedence over old syntax.
|
||||
const char test_json[] = R"({
|
||||
"account_label": "l1",
|
||||
"accounts": {
|
||||
"include": "l5"
|
||||
}
|
||||
})";
|
||||
|
||||
FetchStatus fetch_status;
|
||||
IdentityProviderMetadata idp_metadata;
|
||||
std::tie(fetch_status, idp_metadata) =
|
||||
SendConfigRequestAndWaitForResponse(test_json);
|
||||
|
||||
EXPECT_EQ(ParseStatus::kSuccess, fetch_status.parse_status);
|
||||
EXPECT_EQ(net::HTTP_OK, fetch_status.response_code);
|
||||
EXPECT_EQ("l1", idp_metadata.requested_label);
|
||||
}
|
||||
|
||||
TEST_F(IdpNetworkRequestManagerTest, ParseConfigRequestedLabel) {
|
||||
base::test::ScopedFeatureList list;
|
||||
list.InitAndEnableFeature(features::kFedCmUseOtherAccountAndLabelsNewSyntax);
|
||||
|
||||
const char test_json[] = R"({
|
||||
"account_label": "l1"
|
||||
})";
|
||||
|
||||
FetchStatus fetch_status;
|
||||
IdentityProviderMetadata idp_metadata;
|
||||
std::tie(fetch_status, idp_metadata) =
|
||||
SendConfigRequestAndWaitForResponse(test_json);
|
||||
|
||||
EXPECT_EQ(ParseStatus::kSuccess, fetch_status.parse_status);
|
||||
EXPECT_EQ(net::HTTP_OK, fetch_status.response_code);
|
||||
EXPECT_EQ("l1", idp_metadata.requested_label);
|
||||
}
|
||||
|
||||
// Tests that we send the correct origin for account requests.
|
||||
TEST_F(IdpNetworkRequestManagerTest, AccountRequestOrigin) {
|
||||
bool called = false;
|
||||
|
@ -149,6 +149,12 @@ BASE_FEATURE(kFedCmFlexibleFields,
|
||||
"FedCmFlexibleFields",
|
||||
base::FEATURE_ENABLED_BY_DEFAULT);
|
||||
|
||||
// Whether to support the newer syntax for the "Use Other Account"
|
||||
// and account labels features.
|
||||
BASE_FEATURE(kFedCmUseOtherAccountAndLabelsNewSyntax,
|
||||
"FedCmUseOtherAccountAndLabelsNewSyntax",
|
||||
base::FEATURE_DISABLED_BY_DEFAULT);
|
||||
|
||||
// Enables sending SameSite=Lax cookies in credentialed FedCM requests
|
||||
// (accounts endpoint, ID assertion endpoint and disconnect endpoint).
|
||||
BASE_FEATURE(kFedCmSameSiteLax,
|
||||
|
@ -34,6 +34,7 @@ CONTENT_EXPORT BASE_DECLARE_FEATURE(kEmbeddingRequiresOptIn);
|
||||
CONTENT_EXPORT BASE_DECLARE_FEATURE(kExperimentalContentSecurityPolicyFeatures);
|
||||
CONTENT_EXPORT BASE_DECLARE_FEATURE(kFedCmAlternativeIdentifiers);
|
||||
CONTENT_EXPORT BASE_DECLARE_FEATURE(kFedCmFlexibleFields);
|
||||
CONTENT_EXPORT BASE_DECLARE_FEATURE(kFedCmUseOtherAccountAndLabelsNewSyntax);
|
||||
CONTENT_EXPORT BASE_DECLARE_FEATURE(kFedCmSameSiteLax);
|
||||
CONTENT_EXPORT BASE_DECLARE_FEATURE(kFilterInstalledAppsWebAppMatching);
|
||||
#if BUILDFLAG(IS_WIN)
|
||||
|
Reference in New Issue
Block a user