Skip to content

Commit

Permalink
[FedCM] Replace Account with IdentityCredential for extensibility
Browse files Browse the repository at this point in the history 
Today the FedCM API supports auto selecting an account during auto
re-authentication. Hence we introduced a boolean with "account" in it
to reflect the situation. There's some effort to support digital
credentials using the `IdentityCredential` object in which case the term
"account" would make less sense.

Bug: 1473976
Change-Id: I25dc47f420776a7b0c1997146ede21156c7bdce5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4930572
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: Yi Gu <yigu@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1209096}
  • Loading branch information
@yi-gu
yi-gu authored and Chromium LUCI CQ committed Oct 12, 2023
1 parent d1514f1 commit 41044a1
Show file tree
Hide file tree
Showing 31 changed files with 159 additions and 131 deletions.
11 changes: 6 additions & 5 deletions chrome/browser/about_flags.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8923,11 +8923,6 @@ const FeatureEntry kFeatureEntries[] = {
flag_descriptions::kFedCmAuthzDescription, kOsDesktop,
FEATURE_VALUE_TYPE(features::kFedCmAuthz)},

{"fedcm-account-auto-selected-flag",
flag_descriptions::kFedCmAccountAutoSelectedFlagName,
flag_descriptions::kFedCmAccountAutoSelectedFlagDescription, kOsAll,
FEATURE_VALUE_TYPE(features::kFedCmAccountAutoSelectedFlag)},

{"fedcm-error", flag_descriptions::kFedCmErrorName,
flag_descriptions::kFedCmErrorDescription, kOsAll,
FEATURE_VALUE_TYPE(features::kFedCmError)},
Expand All @@ -8936,6 +8931,12 @@ const FeatureEntry kFeatureEntries[] = {
flag_descriptions::kFedCmHostedDomainDescription, kOsAll,
FEATURE_VALUE_TYPE(features::kFedCmHostedDomain)},

{"fedcm-identity-credential-auto-selected-flag",
flag_descriptions::kFedCmIdentityCredentialAutoSelectedFlagName,
flag_descriptions::kFedCmIdentityCredentialAutoSelectedFlagDescription,
kOsAll,
FEATURE_VALUE_TYPE(features::kFedCmIdentityCredentialAutoSelectedFlag)},

{"fedcm-idp-registration", flag_descriptions::kFedCmIdPRegistrationName,
flag_descriptions::kFedCmIdPRegistrationDescription, kOsDesktop,
FEATURE_VALUE_TYPE(features::kFedCmIdPRegistration)},
Expand Down
10 changes: 5 additions & 5 deletions chrome/browser/flag-metadata.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4282,11 +4282,6 @@
"owners": [ "shaktisahu"],
"expiry_milestone": 110
},
{
"name": "fedcm-account-auto-selected-flag",
"owners": [ "yigu", "web-identity-eng@google.com"],
"expiry_milestone": 125
},
{
"name": "fedcm-authz",
"owners": [ "goto", "web-identity-eng@google.com"],
Expand All @@ -4302,6 +4297,11 @@
"owners": [ "npm", "web-identity-eng@google.com"],
"expiry_milestone": 125
},
{
"name": "fedcm-identity-credential-auto-selected-flag",
"owners": [ "yigu", "web-identity-eng@google.com"],
"expiry_milestone": 125
},
{
"name": "fedcm-idp-registration",
"owners": [ "goto", "web-identity-eng@google.com"],
Expand Down
12 changes: 7 additions & 5 deletions chrome/browser/flag_descriptions.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1604,11 +1604,6 @@ const char kFractionalScrollOffsetsDescription[] =
"Enables fractional scroll offsets inside Blink, exposing non-integer "
"offsets to web APIs.";

const char kFedCmAccountAutoSelectedFlagName[] = "FedCmAccountAutoSelectedFlag";
const char kFedCmAccountAutoSelectedFlagDescription[] =
"Allows the browser to share whether an account was auto-selected with "
"developers post user permission to continue with the IdP.";

const char kFedCmAuthzName[] = "FedCmAuthz";
const char kFedCmAuthzDescription[] =
"Enables RPs to request authorization for custom IdP scopes.";
Expand All @@ -1622,6 +1617,13 @@ const char kFedCmHostedDomainDescription[] =
"Enables RPs to request only FedCM invocations to only show accounts "
"matching a given hosted domain.";

const char kFedCmIdentityCredentialAutoSelectedFlagName[] =
"FedCmIdentityCredentialAutoSelectedFlag";
const char kFedCmIdentityCredentialAutoSelectedFlagDescription[] =
"Allows the browser to share whether an identity credential was "
"auto-selected with developers post user permission to continue with the "
"IdP.";

const char kFedCmIdPRegistrationName[] = "FedCM with IdP Registration support";
const char kFedCmIdPRegistrationDescription[] =
"Enables RPs to get identity credentials from registered IdPs.";
Expand Down
6 changes: 3 additions & 3 deletions chrome/browser/flag_descriptions.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -913,9 +913,6 @@ extern const char kExtensionsOnChromeUrlsDescription[];
extern const char kFractionalScrollOffsetsName[];
extern const char kFractionalScrollOffsetsDescription[];

extern const char kFedCmAccountAutoSelectedFlagName[];
extern const char kFedCmAccountAutoSelectedFlagDescription[];

extern const char kFedCmAuthzName[];
extern const char kFedCmAuthzDescription[];

Expand All @@ -925,6 +922,9 @@ extern const char kFedCmErrorDescription[];
extern const char kFedCmHostedDomainName[];
extern const char kFedCmHostedDomainDescription[];

extern const char kFedCmIdentityCredentialAutoSelectedFlagName[];
extern const char kFedCmIdentityCredentialAutoSelectedFlagDescription[];

extern const char kFedCmIdPRegistrationName[];
extern const char kFedCmIdPRegistrationDescription[];

Expand Down
37 changes: 20 additions & 17 deletions content/browser/webid/federated_auth_request_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,15 +107,17 @@ std::string ComputeUrlEncodedTokenPostData(
}
query += "disclosure_text_shown=" + disclosure_text_shown;

if (IsFedCmAccountAutoSelectedFlagEnabled()) {
// Shares with IdP that whether the account was automatically selected. This
// could help developers to better comprehend the token request and segment
// metrics accordingly.
std::string is_account_auto_selected = is_auto_reauthn ? "true" : "false";
if (IsFedCmIdentityCredentialAutoSelectedFlagEnabled()) {
// Shares with IdP that whether the identity credential was automatically
// selected. This could help developers to better comprehend the token
// request and segment metrics accordingly.
std::string is_identity_credential_auto_selected =
is_auto_reauthn ? "true" : "false";
if (!query.empty()) {
query += "&";
}
query += "is_account_auto_selected=" + is_account_auto_selected;
query += "is_identity_credential_auto_selected=" +
is_identity_credential_auto_selected;
}

if (IsFedCmAuthzEnabled()) {
Expand Down Expand Up @@ -522,19 +524,19 @@ void FederatedAuthRequestImpl::CompleteDigitalCredentialRequest(
if (!digital_credential_provider_) {
std::move(digital_credential_request_callback_)
.Run(RequestTokenStatus::kError, absl::nullopt, "", /*error=*/nullptr,
/*is_account_auto_selected=*/false);
/*is_identity_credential_auto_selected=*/false);
return;
}

if (!response.empty()) {
std::move(digital_credential_request_callback_)
.Run(RequestTokenStatus::kSuccess, absl::nullopt, response,
/*error=*/nullptr,
/*is_account_auto_selected=*/false);
/*is_identity_credential_auto_selected=*/false);
} else {
std::move(digital_credential_request_callback_)
.Run(RequestTokenStatus::kError, absl::nullopt, "", /*error=*/nullptr,
/*is_account_auto_selected=*/false);
/*is_identity_credential_auto_selected=*/false);
}
}

Expand Down Expand Up @@ -610,7 +612,7 @@ void FederatedAuthRequestImpl::RequestToken(
if (is_multi_idp_input && !IsFedCmMultipleIdentityProvidersEnabled()) {
std::move(callback).Run(RequestTokenStatus::kError, absl::nullopt, "",
/*error=*/nullptr,
/*is_account_auto_selected=*/false);
/*is_identity_credential_auto_selected=*/false);
return;
}

Expand All @@ -621,7 +623,7 @@ void FederatedAuthRequestImpl::RequestToken(
// Multi IdP API support.
std::move(callback).Run(RequestTokenStatus::kError, absl::nullopt, "",
/*error=*/nullptr,
/*is_account_auto_selected=*/false);
/*is_identity_credential_auto_selected=*/false);
return;
}

Expand All @@ -631,7 +633,7 @@ void FederatedAuthRequestImpl::RequestToken(
// requests.
std::move(callback).Run(RequestTokenStatus::kErrorTooManyRequests,
absl::nullopt, "", /*error=*/nullptr,
/*is_account_auto_selected=*/false);
/*is_identity_credential_auto_selected=*/false);
return;
}

Expand All @@ -644,7 +646,7 @@ void FederatedAuthRequestImpl::RequestToken(
if (!digital_credential_provider_) {
std::move(digital_credential_request_callback_)
.Run(RequestTokenStatus::kError, absl::nullopt, "", /*error=*/nullptr,
/*is_account_auto_selected=*/false);
/*is_identity_credential_auto_selected=*/false);
return;
}

Expand Down Expand Up @@ -693,7 +695,7 @@ void FederatedAuthRequestImpl::RequestToken(

std::move(callback).Run(RequestTokenStatus::kErrorTooManyRequests,
absl::nullopt, "", /*error=*/nullptr,
/*is_account_auto_selected=*/false);
/*is_identity_credential_auto_selected=*/false);
return;
}

Expand Down Expand Up @@ -2144,8 +2146,9 @@ void FederatedAuthRequestImpl::CompleteRequest(
}
}

bool is_account_auto_selected =
IsFedCmAccountAutoSelectedFlagEnabled() && dialog_type_ == kAutoReauth;
bool is_identity_credential_auto_selected =
IsFedCmIdentityCredentialAutoSelectedFlagEnabled() &&
dialog_type_ == kAutoReauth;

CleanUp();

Expand All @@ -2163,7 +2166,7 @@ void FederatedAuthRequestImpl::CompleteRequest(
FederatedAuthRequestResultToRequestTokenStatus(result);
std::move(auth_request_token_callback_)
.Run(status, selected_idp_config_url, id_token, std::move(error),
is_account_auto_selected);
is_identity_credential_auto_selected);
auth_request_token_callback_.Reset();
} else {
base::SequencedTaskRunner::GetCurrentDefault()->PostDelayedTask(
Expand Down
75 changes: 43 additions & 32 deletions content/browser/webid/federated_auth_request_impl_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2973,7 +2973,7 @@ TEST_F(FederatedAuthRequestImplTest, DisclosureTextShownForFirstTimeUser) {
checker->SetExpectedTokenPostData(
"client_id=" + std::string(kClientId) + "&nonce=" + std::string(kNonce) +
"&account_id=" + std::string(kAccountId) + "&disclosure_text_shown=true" +
"&is_account_auto_selected=false");
"&is_identity_credential_auto_selected=false");
SetNetworkRequestManager(std::move(checker));

RunAuthTest(kDefaultRequestParameters, kExpectationSuccess,
Expand All @@ -2996,7 +2996,8 @@ TEST_F(FederatedAuthRequestImplTest, DisclosureTextNotShownForReturningUser) {
checker->SetExpectedTokenPostData(
"client_id=" + std::string(kClientId) + "&nonce=" + std::string(kNonce) +
"&account_id=" + std::string(kAccountId) +
"&disclosure_text_shown=false" + "&is_account_auto_selected=false");
"&disclosure_text_shown=false" +
"&is_identity_credential_auto_selected=false");
SetNetworkRequestManager(std::move(checker));

MockConfiguration config = kConfigurationValid;
Expand All @@ -3013,20 +3014,23 @@ TEST_F(FederatedAuthRequestImplTest, TokenEndpointPostDataEscaping) {

std::unique_ptr<IdpNetworkRequestManagerParamChecker> checker =
std::make_unique<IdpNetworkRequestManagerParamChecker>();
checker->SetExpectedTokenPostData(
"client_id=" + std::string(kClientId) + "&nonce=" + std::string(kNonce) +
"&account_id=account+id&disclosure_text_shown=true&is_account_auto_"
"selected=false");
checker->SetExpectedTokenPostData("client_id=" + std::string(kClientId) +
"&nonce=" + std::string(kNonce) +
"&account_id=account+id&disclosure_text_"
"shown=true&is_identity_credential_auto_"
"selected=false");
SetNetworkRequestManager(std::move(checker));

RunAuthTest(kDefaultRequestParameters, kExpectationSuccess, configuration);
}

// Test that the is_account_auto_selected field is not included in the request
// if the feature is disabled.
TEST_F(FederatedAuthRequestImplTest, AccountAutoSelectedFlagDisabled) {
// Test that the is_identity_credential_auto_selected field is not included in
// the request if the feature is disabled.
TEST_F(FederatedAuthRequestImplTest,
IdentityCredentialAutoSelectedFlagDisabled) {
base::test::ScopedFeatureList list;
list.InitAndDisableFeature(features::kFedCmAccountAutoSelectedFlag);
list.InitAndDisableFeature(
features::kFedCmIdentityCredentialAutoSelectedFlag);

std::unique_ptr<IdpNetworkRequestManagerParamChecker> checker =
std::make_unique<IdpNetworkRequestManagerParamChecker>();
Expand All @@ -3039,30 +3043,32 @@ TEST_F(FederatedAuthRequestImplTest, AccountAutoSelectedFlagDisabled) {
kConfigurationValid);
}

// Test that the is_account_auto_selected value in the token post data for
// sign-up case.
TEST_F(FederatedAuthRequestImplTest, AccountAutoSelectedFlagForNewUser) {
// Test that the is_identity_credential_auto_selected value in the token post
// data for sign-up case.
TEST_F(FederatedAuthRequestImplTest,
IdentityCredentialAutoSelectedFlagForNewUser) {
base::test::ScopedFeatureList list;
list.InitAndEnableFeature(features::kFedCmAccountAutoSelectedFlag);
list.InitAndEnableFeature(features::kFedCmIdentityCredentialAutoSelectedFlag);

std::unique_ptr<IdpNetworkRequestManagerParamChecker> checker =
std::make_unique<IdpNetworkRequestManagerParamChecker>();
checker->SetExpectedTokenPostData(
"client_id=" + std::string(kClientId) + "&nonce=" + std::string(kNonce) +
"&account_id=" + std::string(kAccountId) + "&disclosure_text_shown=true" +
"&is_account_auto_selected=false");
"&is_identity_credential_auto_selected=false");
SetNetworkRequestManager(std::move(checker));

RunAuthTest(kDefaultRequestParameters, kExpectationSuccess,
kConfigurationValid);
}

// Test that the is_account_auto_selected value in the token post data for
// returning user with `mediation:required`.
TEST_F(FederatedAuthRequestImplTest,
AccountAutoSelectedFlagForReturningUserWithMediationRequired) {
// Test that the is_identity_credential_auto_selected value in the token post
// data for returning user with `mediation:required`.
TEST_F(
FederatedAuthRequestImplTest,
IdentityCredentialAutoSelectedFlagForReturningUserWithMediationRequired) {
base::test::ScopedFeatureList list;
list.InitAndEnableFeature(features::kFedCmAccountAutoSelectedFlag);
list.InitAndEnableFeature(features::kFedCmIdentityCredentialAutoSelectedFlag);
// Pretend the sharing permission has been granted for this account.
EXPECT_CALL(
*test_permission_delegate_,
Expand All @@ -3076,20 +3082,22 @@ TEST_F(FederatedAuthRequestImplTest,
checker->SetExpectedTokenPostData(
"client_id=" + std::string(kClientId) + "&nonce=" + std::string(kNonce) +
"&account_id=" + std::string(kAccountId) +
"&disclosure_text_shown=false" + "&is_account_auto_selected=false");
"&disclosure_text_shown=false" +
"&is_identity_credential_auto_selected=false");
SetNetworkRequestManager(std::move(checker));

MockConfiguration config = kConfigurationValid;
config.mediation_requirement = MediationRequirement::kRequired;
RunAuthTest(kDefaultRequestParameters, kExpectationSuccess, config);
}

// Test that the is_account_auto_selected value in the token post data for
// returning user with `mediation:optional`.
TEST_F(FederatedAuthRequestImplTest,
AccountAutoSelectedFlagForReturningUserWithMediationOptional) {
// Test that the is_identity_credential_auto_selected value in the token post
// data for returning user with `mediation:optional`.
TEST_F(
FederatedAuthRequestImplTest,
IdentityCredentialAutoSelectedFlagForReturningUserWithMediationOptional) {
base::test::ScopedFeatureList list;
list.InitAndEnableFeature(features::kFedCmAccountAutoSelectedFlag);
list.InitAndEnableFeature(features::kFedCmIdentityCredentialAutoSelectedFlag);
// Pretend the sharing permission has been granted for this account.
EXPECT_CALL(
*test_permission_delegate_,
Expand All @@ -3109,19 +3117,21 @@ TEST_F(FederatedAuthRequestImplTest,
checker->SetExpectedTokenPostData(
"client_id=" + std::string(kClientId) + "&nonce=" + std::string(kNonce) +
"&account_id=" + std::string(kAccountId) +
"&disclosure_text_shown=false" + "&is_account_auto_selected=true");
"&disclosure_text_shown=false" +
"&is_identity_credential_auto_selected=true");
SetNetworkRequestManager(std::move(checker));

MockConfiguration config = kConfigurationValid;
config.mediation_requirement = MediationRequirement::kOptional;
RunAuthTest(kDefaultRequestParameters, kExpectationSuccess, config);
}

// Test that the is_account_auto_selected value in the token post data for the
// quiet period use case.
TEST_F(FederatedAuthRequestImplTest, AccountAutoSelectedFlagIfInQuietPeriod) {
// Test that the is_identity_credential_auto_selected value in the token post
// data for the quiet period use case.
TEST_F(FederatedAuthRequestImplTest,
IdentityCredentialAutoSelectedFlagIfInQuietPeriod) {
base::test::ScopedFeatureList list;
list.InitAndEnableFeature(features::kFedCmAccountAutoSelectedFlag);
list.InitAndEnableFeature(features::kFedCmIdentityCredentialAutoSelectedFlag);
// Pretend the sharing permission has been granted for this account.
EXPECT_CALL(
*test_permission_delegate_,
Expand All @@ -3146,7 +3156,8 @@ TEST_F(FederatedAuthRequestImplTest, AccountAutoSelectedFlagIfInQuietPeriod) {
checker->SetExpectedTokenPostData(
"client_id=" + std::string(kClientId) + "&nonce=" + std::string(kNonce) +
"&account_id=" + std::string(kAccountId) +
"&disclosure_text_shown=false" + "&is_account_auto_selected=false");
"&disclosure_text_shown=false" +
"&is_identity_credential_auto_selected=false");
SetNetworkRequestManager(std::move(checker));

RequestExpectations expectations = kExpectationSuccess;
Expand Down
5 changes: 3 additions & 2 deletions content/browser/webid/flags.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,9 @@ bool IsWebIdentityMDocsEnabled() {
return base::FeatureList::IsEnabled(features::kWebIdentityMDocs);
}

bool IsFedCmAccountAutoSelectedFlagEnabled() {
return base::FeatureList::IsEnabled(features::kFedCmAccountAutoSelectedFlag);
bool IsFedCmIdentityCredentialAutoSelectedFlagEnabled() {
return base::FeatureList::IsEnabled(
features::kFedCmIdentityCredentialAutoSelectedFlag);
}

bool IsFedCmHostedDomainEnabled() {
Expand Down
4 changes: 2 additions & 2 deletions content/browser/webid/flags.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ bool IsFedCmWithoutWellKnownEnforcementEnabled();
// Whether the Web Identity MDocs API is enabled.
bool IsWebIdentityMDocsEnabled();

// Whether the AccountAutoSelected feature is enabled.
bool IsFedCmAccountAutoSelectedFlagEnabled();
// Whether the IdentityCredentialAutoSelected feature is enabled.
bool IsFedCmIdentityCredentialAutoSelectedFlagEnabled();

// Whether the HostedDomain feature is enabled.
bool IsFedCmHostedDomainEnabled();
Expand Down

0 comments on commit 41044a1

Please sign in to comment.