Skip to content

Commit

Permalink
[FedCM] Prototype AutoReauthnFlag behind a flag
Browse files Browse the repository at this point in the history
ChromeStatus: https://chromestatus.com/guide/edit/5384360374566912

Bug: 1473976
Change-Id: Idb4f702f23ef96c3ebd08c642609f2a7fa2ead44
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4792440
Commit-Queue: Yi Gu <yigu@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1185429}
  • Loading branch information
yi-gu authored and Chromium LUCI CQ committed Aug 18, 2023
1 parent b757a9d commit 55757e1
Show file tree
Hide file tree
Showing 25 changed files with 238 additions and 29 deletions.
4 changes: 4 additions & 0 deletions chrome/browser/about_flags.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8902,6 +8902,10 @@ const FeatureEntry kFeatureEntries[] = {
flag_descriptions::kFedCmAuthzDescription, kOsDesktop,
FEATURE_VALUE_TYPE(features::kFedCmAuthz)},

{"fedcm-auto-reauthn-flag", flag_descriptions::kFedCmAutoReauthnFlagName,
flag_descriptions::kFedCmAutoReauthnFlagDescription, kOsAll,
FEATURE_VALUE_TYPE(features::kFedCmAutoReauthnFlag)},

{"fedcm-error", flag_descriptions::kFedCmErrorName,
flag_descriptions::kFedCmErrorDescription, kOsAll,
FEATURE_VALUE_TYPE(features::kFedCmError)},
Expand Down
5 changes: 5 additions & 0 deletions chrome/browser/flag-metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -4207,6 +4207,11 @@
"owners": [ "goto", "web-identity-eng@google.com"],
"expiry_milestone": 120
},
{
"name": "fedcm-auto-reauthn-flag",
"owners": [ "yigu", "web-identity-eng@google.com"],
"expiry_milestone": 122
},
{
"name": "fedcm-error",
"owners": [ "tanzachary", "web-identity-eng@google.com"],
Expand Down
6 changes: 6 additions & 0 deletions chrome/browser/flag_descriptions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1642,6 +1642,12 @@ const char kFedCmAuthzName[] = "FedCmAuthz";
const char kFedCmAuthzDescription[] =
"Enables RPs to request authorization for custom IdP scopes.";

const char kFedCmAutoReauthnFlagName[] = "FedCmAutoReauthnFlag";
const char kFedCmAutoReauthnFlagDescription[] =
"Allows the browser to share whether a user has gone through the FedCM "
"auto re-authn flow with developers post user permission to continue with "
"the IdP.";

const char kFedCmErrorName[] = "FedCmError";
const char kFedCmErrorDescription[] =
"Enables IDPs to show information about an error.";
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/flag_descriptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -924,6 +924,9 @@ extern const char kFedCmDescription[];
extern const char kFedCmAuthzName[];
extern const char kFedCmAuthzDescription[];

extern const char kFedCmAutoReauthnFlagName[];
extern const char kFedCmAutoReauthnFlagDescription[];

extern const char kFedCmErrorName[];
extern const char kFedCmErrorDescription[];

Expand Down
48 changes: 35 additions & 13 deletions content/browser/webid/federated_auth_request_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ std::string ComputeUrlEncodedTokenPostData(
const std::string& nonce,
const std::string& account_id,
bool is_sign_in,
bool is_auto_reauthn,
const std::vector<std::string>& scope,
const std::vector<std::string>& responseType,
const base::flat_map<std::string, std::string>& params) {
Expand Down Expand Up @@ -102,7 +103,19 @@ std::string ComputeUrlEncodedTokenPostData(
// whether the user has been shown such disclosure text.
std::string disclosure_text_shown = is_sign_in ? "false" : "true";
if (!query.empty()) {
query += "&disclosure_text_shown=" + disclosure_text_shown;
query += "&";
}
query += "disclosure_text_shown=" + disclosure_text_shown;

if (IsFedCmAutoReauthnFlagEnabled()) {
// Shares with IdP that whether a user ha gone through the auto
// re-authentication flow. This could help developers to better
// comprehend the token request and segment metrics accordingly.
std::string is_auto_reauthn_string = is_auto_reauthn ? "true" : "false";
if (!query.empty()) {
query += "&";
}
query += "is_auto_reauthn=" + is_auto_reauthn_string;
}

if (IsFedCmAuthzEnabled()) {
Expand Down Expand Up @@ -526,16 +539,19 @@ FederatedAuthRequestImpl& FederatedAuthRequestImpl::CreateForTesting(
void FederatedAuthRequestImpl::CompleteMDocRequest(std::string mdoc) {
if (!mdoc_provider_) {
std::move(mdoc_request_callback_)
.Run(RequestTokenStatus::kError, absl::nullopt, "");
.Run(RequestTokenStatus::kError, absl::nullopt, "",
/*is_auto_reauthn=*/false);
return;
}

if (!mdoc.empty()) {
std::move(mdoc_request_callback_)
.Run(RequestTokenStatus::kSuccess, absl::nullopt, mdoc);
.Run(RequestTokenStatus::kSuccess, absl::nullopt, mdoc,
/*is_auto_reauthn=*/false);
} else {
std::move(mdoc_request_callback_)
.Run(RequestTokenStatus::kError, absl::nullopt, "");
.Run(RequestTokenStatus::kError, absl::nullopt, "",
/*is_auto_reauthn=*/false);
}
}

Expand All @@ -560,7 +576,8 @@ void FederatedAuthRequestImpl::RequestToken(
const bool is_multi_idp_input = idp_get_params_ptrs.size() > 1u ||
idp_get_params_ptrs[0]->providers.size() > 1u;
if (is_multi_idp_input && !IsFedCmMultipleIdentityProvidersEnabled()) {
std::move(callback).Run(RequestTokenStatus::kError, absl::nullopt, "");
std::move(callback).Run(RequestTokenStatus::kError, absl::nullopt, "",
/*is_auto_reauthn=*/false);
return;
}

Expand All @@ -569,7 +586,8 @@ void FederatedAuthRequestImpl::RequestToken(
IsFedCmMultipleIdentityProvidersEnabled()) {
// TODO(https://crbug.com/1416939): Support calling the MDocs API with the
// Multi IdP API support.
std::move(callback).Run(RequestTokenStatus::kError, absl::nullopt, "");
std::move(callback).Run(RequestTokenStatus::kError, absl::nullopt, "",
/*is_auto_reauthn=*/false);
return;
}

Expand All @@ -578,7 +596,7 @@ void FederatedAuthRequestImpl::RequestToken(
// TODO(https://crbug.com/1416939): Reconcile with federated identity
// requests.
std::move(callback).Run(RequestTokenStatus::kErrorTooManyRequests,
absl::nullopt, "");
absl::nullopt, "", /*is_auto_reauthn=*/false);
return;
}

Expand All @@ -590,7 +608,8 @@ void FederatedAuthRequestImpl::RequestToken(
}
if (!mdoc_provider_) {
std::move(mdoc_request_callback_)
.Run(RequestTokenStatus::kError, absl::nullopt, "");
.Run(RequestTokenStatus::kError, absl::nullopt, "",
/*is_auto_reauthn=*/false);
return;
}

Expand All @@ -614,7 +633,8 @@ void FederatedAuthRequestImpl::RequestToken(
// Check that providers are non-empty.
for (auto& idp_get_params_ptr : idp_get_params_ptrs) {
if (idp_get_params_ptr->providers.size() == 0) {
std::move(callback).Run(RequestTokenStatus::kError, absl::nullopt, "");
std::move(callback).Run(RequestTokenStatus::kError, absl::nullopt, "",
/*is_auto_reauthn=*/false);
return;
}
}
Expand All @@ -638,7 +658,7 @@ void FederatedAuthRequestImpl::RequestToken(
fedcm_metrics_->RecordRequestTokenStatus(TokenStatus::kTooManyRequests,
requirement);
std::move(callback).Run(RequestTokenStatus::kErrorTooManyRequests,
absl::nullopt, "");
absl::nullopt, "", /*is_auto_reauthn=*/false);
return;
}

Expand Down Expand Up @@ -1626,8 +1646,8 @@ void FederatedAuthRequestImpl::OnAccountSelected(const GURL& idp_config_url,
idp_info.endpoints.token, account_id_,
ComputeUrlEncodedTokenPostData(
idp_info.provider->client_id, idp_info.provider->nonce, account_id,
is_sign_in, idp_info.provider->scope, idp_info.provider->responseType,
idp_info.provider->params),
is_sign_in, dialog_type_ == kAutoReauth, idp_info.provider->scope,
idp_info.provider->responseType, idp_info.provider->params),
base::BindOnce(&FederatedAuthRequestImpl::OnTokenResponseReceived,
weak_ptr_factory_.GetWeakPtr(),
idp_info.provider->Clone()),
Expand Down Expand Up @@ -1958,6 +1978,8 @@ void FederatedAuthRequestImpl::CompleteRequest(
}
}

bool is_auto_reauthn = dialog_type_ == kAutoReauth;

CleanUp();

if (!should_delay_callback || should_complete_request_immediately_) {
Expand All @@ -1967,7 +1989,7 @@ void FederatedAuthRequestImpl::CompleteRequest(
RequestTokenStatus status =
FederatedAuthRequestResultToRequestTokenStatus(result);
std::move(auth_request_token_callback_)
.Run(status, selected_idp_config_url, id_token);
.Run(status, selected_idp_config_url, id_token, is_auto_reauthn);
auth_request_token_callback_.Reset();
} else {
base::SequencedTaskRunner::GetCurrentDefault()->PostDelayedTask(
Expand Down
116 changes: 116 additions & 0 deletions content/browser/webid/federated_auth_request_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2961,6 +2961,122 @@ TEST_F(FederatedAuthRequestImplTest, TokenEndpointPostDataEscaping) {
RunAuthTest(kDefaultRequestParameters, kExpectationSuccess, configuration);
}

// Test that the is_auto_reauthn value in the token post data for sign-up case.
TEST_F(FederatedAuthRequestImplTest, AutoReauthnFlagForNewUser) {
base::test::ScopedFeatureList list;
list.InitAndEnableFeature(features::kFedCmAutoReauthnFlag);

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_auto_reauthn=false");
SetNetworkRequestManager(std::move(checker));

RunAuthTest(kDefaultRequestParameters, kExpectationSuccess,
kConfigurationValid);
}

// Test that the is_auto_reauthn value in the token post data for returning
// user with `mediation:required`.
TEST_F(FederatedAuthRequestImplTest,
AutoReauthnFlagForReturningUserWithMediationRequired) {
base::test::ScopedFeatureList list;
list.InitAndEnableFeature(features::kFedCmAutoReauthnFlag);
// Pretend the sharing permission has been granted for this account.
EXPECT_CALL(
*test_permission_delegate_,
HasSharingPermission(OriginFromString(kRpUrl), OriginFromString(kRpUrl),
OriginFromString(kProviderUrlFull),
Optional(std::string(kAccountId))))
.WillOnce(Return(true));

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=false" + "&is_auto_reauthn=false");
SetNetworkRequestManager(std::move(checker));

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

// Test that the is_auto_reauthn value in the token post data for returning
// user with `mediation:optional`.
TEST_F(FederatedAuthRequestImplTest,
AutoReauthnFlagForReturningUserWithMediationOptional) {
base::test::ScopedFeatureList list;
list.InitAndEnableFeature(features::kFedCmAutoReauthnFlag);
// Pretend the sharing permission has been granted for this account.
EXPECT_CALL(
*test_permission_delegate_,
HasSharingPermission(OriginFromString(kRpUrl), OriginFromString(kRpUrl),
OriginFromString(kProviderUrlFull),
Optional(std::string(kAccountId))))
.Times(2)
.WillRepeatedly(Return(true));

// Pretend the auto re-authn permission has been granted.
EXPECT_CALL(*test_auto_reauthn_permission_delegate_,
IsAutoReauthnSettingEnabled())
.WillOnce(Return(true));

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=false" + "&is_auto_reauthn=true");
SetNetworkRequestManager(std::move(checker));

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

// Test that the is_auto_reauthn value in the token post data for the quiet
// period use case.
TEST_F(FederatedAuthRequestImplTest, AutoReauthnFlagIfInQuietPeriod) {
base::test::ScopedFeatureList list;
list.InitAndEnableFeature(features::kFedCmAutoReauthnFlag);
// Pretend the sharing permission has been granted for this account.
EXPECT_CALL(
*test_permission_delegate_,
HasSharingPermission(OriginFromString(kRpUrl), OriginFromString(kRpUrl),
OriginFromString(kProviderUrlFull),
Optional(std::string(kAccountId))))
.Times(2)
.WillRepeatedly(Return(true));

// Pretend the auto re-authn permission has been granted.
EXPECT_CALL(*test_auto_reauthn_permission_delegate_,
IsAutoReauthnSettingEnabled())
.WillOnce(Return(true));

// Pretend the auto re-authn is in quiet period.
EXPECT_CALL(*test_auto_reauthn_permission_delegate_,
IsAutoReauthnEmbargoed(OriginFromString(kRpUrl)))
.WillOnce(Return(true));

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=false" + "&is_auto_reauthn=false");
SetNetworkRequestManager(std::move(checker));

RequestExpectations expectations = kExpectationSuccess;
expectations.standalone_console_message =
"Auto re-authn was previously triggered less than 10 minutes ago. Only "
"one auto re-authn request can be made every 10 minutes.";
RunAuthTest(kDefaultRequestParameters, expectations, kConfigurationValid);
}

namespace {

// TestIdpNetworkRequestManager subclass which runs the `account_list_task`
Expand Down
4 changes: 4 additions & 0 deletions content/browser/webid/flags.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ bool IsWebIdentityMDocsEnabled() {
return base::FeatureList::IsEnabled(features::kWebIdentityMDocs);
}

bool IsFedCmAutoReauthnFlagEnabled() {
return base::FeatureList::IsEnabled(features::kFedCmAutoReauthnFlag);
}

bool IsFedCmHostedDomainEnabled() {
return base::FeatureList::IsEnabled(features::kFedCmHostedDomain);
}
Expand Down
4 changes: 3 additions & 1 deletion content/browser/webid/flags.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,11 @@ bool IsFedCmWithoutWellKnownEnforcementEnabled();
// Whether the Web Identity MDocs API is enabled.
bool IsWebIdentityMDocsEnabled();

// Whether the AutoReauthnFlag feature is enabled.
bool IsFedCmAutoReauthnFlagEnabled();

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

} // namespace content

#endif // CONTENT_BROWSER_WEBID_FLAGS_H_
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,13 @@ void FederatedAuthRequestRequestTokenCallbackHelper::Reset() {
void FederatedAuthRequestRequestTokenCallbackHelper::ReceiverMethod(
blink::mojom::RequestTokenStatus status,
const absl::optional<GURL>& selected_idp_config_url,
const absl::optional<std::string>& token) {
const absl::optional<std::string>& token,
bool is_auto_reauthn) {
CHECK(!was_called_);
status_ = status;
selected_idp_config_url_ = selected_idp_config_url;
token_ = token;
is_auto_reauthn_ = is_auto_reauthn;
was_called_ = true;
wait_for_callback_loop_.Quit();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ class FederatedAuthRequestRequestTokenCallbackHelper {
private:
void ReceiverMethod(blink::mojom::RequestTokenStatus status,
const absl::optional<GURL>& selected_idp_config_url,
const absl::optional<std::string>& token);
const absl::optional<std::string>& token,
bool is_auto_reauthn);

void Quit();

Expand All @@ -71,6 +72,7 @@ class FederatedAuthRequestRequestTokenCallbackHelper {
absl::optional<blink::mojom::RequestTokenStatus> status_;
absl::optional<GURL> selected_idp_config_url_;
absl::optional<std::string> token_;
bool is_auto_reauthn_{false};
};

} // namespace content
Expand Down
2 changes: 2 additions & 0 deletions content/child/runtime_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,8 @@ void SetRuntimeFeaturesFromChromiumFeatures() {
raw_ref(features::kDocumentPolicyNegotiation)},
{wf::EnableFedCm, raw_ref(features::kFedCm), kSetOnlyIfOverridden},
{wf::EnableFedCmAuthz, raw_ref(features::kFedCmAuthz), kDefault},
{wf::EnableFedCmAutoReauthnFlag, raw_ref(features::kFedCmAutoReauthnFlag),
kSetOnlyIfOverridden},
{wf::EnableFedCmError, raw_ref(features::kFedCmError), kDefault},
{wf::EnableFedCmHostedDomain, raw_ref(features::kFedCmHostedDomain),
kSetOnlyIfOverridden},
Expand Down
6 changes: 6 additions & 0 deletions content/public/common/content_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,12 @@ const char kFedCmIdpSignoutFieldTrialParamName[] = "IdpSignout";
// Enables usage of the FedCM Authz API.
BASE_FEATURE(kFedCmAuthz, "FedCmAuthz", base::FEATURE_DISABLED_BY_DEFAULT);

// Enables usage of the FedCM AutoReauthnFlag feature. ChromeStatus entry:
// https://chromestatus.com/feature/5384360374566912
BASE_FEATURE(kFedCmAutoReauthnFlag,
"FedCmAutoReauthnFlag",
base::FEATURE_DISABLED_BY_DEFAULT);

// Enables usage of the FedCM Error API.
BASE_FEATURE(kFedCmError, "FedCmError", base::FEATURE_DISABLED_BY_DEFAULT);

Expand Down
1 change: 1 addition & 0 deletions content/public/common/content_features.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ CONTENT_EXPORT BASE_DECLARE_FEATURE(
CONTENT_EXPORT BASE_DECLARE_FEATURE(kFedCm);
CONTENT_EXPORT extern const char kFedCmIdpSignoutFieldTrialParamName[];
CONTENT_EXPORT BASE_DECLARE_FEATURE(kFedCmAuthz);
CONTENT_EXPORT BASE_DECLARE_FEATURE(kFedCmAutoReauthnFlag);
CONTENT_EXPORT BASE_DECLARE_FEATURE(kFedCmError);
CONTENT_EXPORT BASE_DECLARE_FEATURE(kFedCmHostedDomain);
CONTENT_EXPORT BASE_DECLARE_FEATURE(kFedCmIdPRegistration);
Expand Down

0 comments on commit 55757e1

Please sign in to comment.