0

[FedCM] Introduce boolean is_registered_config_url

The existing boolean in IdentityProviderConfig is pretty much unneeded,
so it is replaced with this. The boolean means whether the IDP it
represents corresponds to one related to IDP registration or not. It
will be used to relax the well-known checks for registered IdPs, as per
https://github.com/w3c-fedid/idp-registration/issues/4#issuecomment-2334800148

Bug: 346572117
Change-Id: I61655cebac98f26939f6e3621adb1c18fd607445
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6138067
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: Nicolás Peña <npm@chromium.org>
Owners-Override: Nicolás Peña <npm@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1401880}
This commit is contained in:
Nicolás Peña
2025-01-03 10:36:56 -08:00
committed by Chromium LUCI CQ
parent eb763cc410
commit 15b2696dea
4 changed files with 36 additions and 25 deletions

@@ -588,14 +588,15 @@ FederatedAuthRequestImpl::MaybeAddRegisteredProviders(
std::reverse(registered_config_urls.begin(), registered_config_urls.end()); std::reverse(registered_config_urls.begin(), registered_config_urls.end());
for (auto& provider : providers) { for (auto& provider : providers) {
if (!provider->config->use_registered_config_urls) { if (!provider->config->from_idp_registration_api) {
result.emplace_back(provider->Clone()); result.emplace_back(provider->Clone());
continue; continue;
} }
for (auto& configURL : registered_config_urls) { for (auto& configURL : registered_config_urls) {
blink::mojom::IdentityProviderRequestOptionsPtr idp = provider->Clone(); blink::mojom::IdentityProviderRequestOptionsPtr idp = provider->Clone();
idp->config->use_registered_config_urls = false; // Keep `from_idp_registration_api` so it is clear this is a registered
// provider.
idp->config->config_url = configURL; idp->config->config_url = configURL;
result.emplace_back(std::move(idp)); result.emplace_back(std::move(idp));
} }

@@ -1626,9 +1626,8 @@ class FederatedAuthRequestImplTest : public RenderViewHostImplTestHarness {
return federated_auth_request_impl_->MaybeAddRegisteredProviders(providers); return federated_auth_request_impl_->MaybeAddRegisteredProviders(providers);
} }
blink::mojom::IdentityProviderRequestOptionsPtr NewNamedIdP( blink::mojom::IdentityProviderRequestOptionsPtr
GURL config_url, NewNamedIdP(GURL config_url, std::string client_id, bool is_registered) {
std::string client_id) {
blink::mojom::IdentityProviderRequestOptionsPtr options = blink::mojom::IdentityProviderRequestOptionsPtr options =
blink::mojom::IdentityProviderRequestOptions::New(); blink::mojom::IdentityProviderRequestOptions::New();
blink::mojom::IdentityProviderConfigPtr config = blink::mojom::IdentityProviderConfigPtr config =
@@ -1636,6 +1635,7 @@ class FederatedAuthRequestImplTest : public RenderViewHostImplTestHarness {
config->config_url = config_url; config->config_url = config_url;
config->client_id = client_id; config->client_id = client_id;
options->config = std::move(config); options->config = std::move(config);
options->config->from_idp_registration_api = is_registered;
return options; return options;
} }
@@ -1643,7 +1643,7 @@ class FederatedAuthRequestImplTest : public RenderViewHostImplTestHarness {
std::string client_id) { std::string client_id) {
blink::mojom::IdentityProviderConfigPtr config = blink::mojom::IdentityProviderConfigPtr config =
blink::mojom::IdentityProviderConfig::New(); blink::mojom::IdentityProviderConfig::New();
config->use_registered_config_urls = true; config->from_idp_registration_api = true;
config->client_id = client_id; config->client_id = client_id;
blink::mojom::IdentityProviderRequestOptionsPtr options = blink::mojom::IdentityProviderRequestOptionsPtr options =
blink::mojom::IdentityProviderRequestOptions::New(); blink::mojom::IdentityProviderRequestOptions::New();
@@ -6915,14 +6915,16 @@ TEST_F(FederatedAuthRequestImplTest, MaybeAddRegisteredProvidersEmptyList) {
// Test that no registered IdP with only named providers requested. // Test that no registered IdP with only named providers requested.
TEST_F(FederatedAuthRequestImplTest, MaybeAddRegisteredProvidersNamed) { TEST_F(FederatedAuthRequestImplTest, MaybeAddRegisteredProvidersNamed) {
std::vector<blink::mojom::IdentityProviderRequestOptionsPtr> providers; std::vector<blink::mojom::IdentityProviderRequestOptionsPtr> providers;
providers.emplace_back(NewNamedIdP(GURL("https://idp.example"), kClientId)); providers.emplace_back(NewNamedIdP(GURL("https://idp.example"), kClientId,
/*is_registered=*/false));
std::vector<blink::mojom::IdentityProviderRequestOptionsPtr> result = std::vector<blink::mojom::IdentityProviderRequestOptionsPtr> result =
MaybeAddRegisteredProviders(providers); MaybeAddRegisteredProviders(providers);
// Expects the vector to be the same. // Expects the vector to be the same.
std::vector<blink::mojom::IdentityProviderRequestOptionsPtr> expected; std::vector<blink::mojom::IdentityProviderRequestOptionsPtr> expected;
expected.emplace_back(NewNamedIdP(GURL("https://idp.example"), kClientId)); expected.emplace_back(NewNamedIdP(GURL("https://idp.example"), kClientId,
/*is_registered=*/false));
EXPECT_EQ(expected, result); EXPECT_EQ(expected, result);
} }
@@ -6943,7 +6945,8 @@ TEST_F(FederatedAuthRequestImplTest, MaybeAddRegisteredProvidersAdded) {
// Expects that the registered IdP gets replaced by a named IdP. // Expects that the registered IdP gets replaced by a named IdP.
std::vector<blink::mojom::IdentityProviderRequestOptionsPtr> expected; std::vector<blink::mojom::IdentityProviderRequestOptionsPtr> expected;
expected.emplace_back(NewNamedIdP(GURL("https://idp.example"), kClientId)); expected.emplace_back(NewNamedIdP(GURL("https://idp.example"), kClientId,
/*is_registered=*/true));
EXPECT_EQ(expected, result); EXPECT_EQ(expected, result);
} }
@@ -6966,8 +6969,10 @@ TEST_F(FederatedAuthRequestImplTest,
// Expects that the registered IdP gets replaced by a named IdP. // Expects that the registered IdP gets replaced by a named IdP.
std::vector<blink::mojom::IdentityProviderRequestOptionsPtr> expected; std::vector<blink::mojom::IdentityProviderRequestOptionsPtr> expected;
expected.emplace_back(NewNamedIdP(GURL("https://idp.example"), kClientId)); expected.emplace_back(NewNamedIdP(GURL("https://idp.example"), kClientId,
expected.emplace_back(NewNamedIdP(GURL("https://idp.example"), kClientId)); /*is_registered=*/true));
expected.emplace_back(NewNamedIdP(GURL("https://idp.example"), kClientId,
/*is_registered=*/true));
EXPECT_EQ(expected, result); EXPECT_EQ(expected, result);
} }
@@ -6989,8 +6994,10 @@ TEST_F(FederatedAuthRequestImplTest,
MaybeAddRegisteredProviders(providers); MaybeAddRegisteredProviders(providers);
std::vector<blink::mojom::IdentityProviderRequestOptionsPtr> expected; std::vector<blink::mojom::IdentityProviderRequestOptionsPtr> expected;
expected.emplace_back(NewNamedIdP(GURL("https://idp2.example"), kClientId)); expected.emplace_back(NewNamedIdP(GURL("https://idp2.example"), kClientId,
expected.emplace_back(NewNamedIdP(GURL("https://idp1.example"), kClientId)); /*is_registered=*/true));
expected.emplace_back(NewNamedIdP(GURL("https://idp1.example"), kClientId,
/*is_registered=*/true));
EXPECT_EQ(expected, result); EXPECT_EQ(expected, result);
} }
@@ -6999,9 +7006,11 @@ TEST_F(FederatedAuthRequestImplTest,
TEST_F(FederatedAuthRequestImplTest, TEST_F(FederatedAuthRequestImplTest,
MaybeAddRegisteredProvidersInsertedInline) { MaybeAddRegisteredProvidersInsertedInline) {
std::vector<blink::mojom::IdentityProviderRequestOptionsPtr> providers; std::vector<blink::mojom::IdentityProviderRequestOptionsPtr> providers;
providers.emplace_back(NewNamedIdP(GURL("https://idp1.example"), kClientId)); providers.emplace_back(NewNamedIdP(GURL("https://idp1.example"), kClientId,
/*is_registered=*/false));
providers.emplace_back(NewRegisteredIdP(kClientId)); providers.emplace_back(NewRegisteredIdP(kClientId));
providers.emplace_back(NewNamedIdP(GURL("https://idp2.example"), kClientId)); providers.emplace_back(NewNamedIdP(GURL("https://idp2.example"), kClientId,
/*is_registered=*/false));
std::vector<GURL> registry; std::vector<GURL> registry;
registry.emplace_back("https://idp-registered1.example"); registry.emplace_back("https://idp-registered1.example");
@@ -7015,12 +7024,14 @@ TEST_F(FederatedAuthRequestImplTest,
// Expects that the registered IdP gets replaced by a named IdP. // Expects that the registered IdP gets replaced by a named IdP.
std::vector<blink::mojom::IdentityProviderRequestOptionsPtr> expected; std::vector<blink::mojom::IdentityProviderRequestOptionsPtr> expected;
expected.emplace_back(NewNamedIdP(GURL("https://idp1.example"), kClientId)); expected.emplace_back(NewNamedIdP(GURL("https://idp1.example"), kClientId,
expected.emplace_back( /*is_registered=*/false));
NewNamedIdP(GURL("https://idp-registered2.example"), kClientId)); expected.emplace_back(NewNamedIdP(GURL("https://idp-registered2.example"),
expected.emplace_back( kClientId, /*is_registered=*/true));
NewNamedIdP(GURL("https://idp-registered1.example"), kClientId)); expected.emplace_back(NewNamedIdP(GURL("https://idp-registered1.example"),
expected.emplace_back(NewNamedIdP(GURL("https://idp2.example"), kClientId)); kClientId, /*is_registered=*/true));
expected.emplace_back(NewNamedIdP(GURL("https://idp2.example"), kClientId,
/*is_registered=*/false));
EXPECT_EQ(expected, result); EXPECT_EQ(expected, result);
} }

@@ -74,9 +74,8 @@ struct IdentityProviderConfig {
// Explicitly references a specific provider by a Config URL. // Explicitly references a specific provider by a Config URL.
url.mojom.Url config_url; url.mojom.Url config_url;
// Indirectly references providers that have been registered in the past // Whether this config comes from the IDP registration API.
// rather than directly. bool from_idp_registration_api;
bool use_registered_config_urls;
// When using registered IDPs, this contains the type of IDP requested. // When using registered IDPs, this contains the type of IDP requested.
string? type; string? type;

@@ -944,7 +944,7 @@ TypeConverter<IdentityProviderRequestOptionsPtr,
CHECK(options.hasConfigURL()); CHECK(options.hasConfigURL());
if (blink::RuntimeEnabledFeatures::FedCmIdPRegistrationEnabled() && if (blink::RuntimeEnabledFeatures::FedCmIdPRegistrationEnabled() &&
options.configURL() == "any") { options.configURL() == "any") {
mojo_options->config->use_registered_config_urls = true; mojo_options->config->from_idp_registration_api = true;
// We only set the `type` if `configURL` is 'any'. // We only set the `type` if `configURL` is 'any'.
if (options.hasType()) { if (options.hasType()) {
mojo_options->config->type = options.type(); mojo_options->config->type = options.type();