[fedcm] AuthZ API: insist that the continue_on url is same-origin with the config_url
In this CL, we check if the continue_on url is same-origin with the config_url. While it is not clear if this necessarily needs to be the case, it seems like a safer privacy default that will cover the current known needs. Bug: 1429083 Change-Id: I43887cc45895f985519ec93bec48cd1832e4aa78 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4581328 Commit-Queue: Sam Goto <goto@chromium.org> Reviewed-by: Christian Biesinger <cbiesinger@chromium.org> Cr-Commit-Position: refs/heads/main@{#1152137}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
6fd1597b18
commit
c67704ac38
@ -1598,7 +1598,16 @@ void FederatedAuthRequestImpl::OnContinueOnResponseReceived(
|
||||
IdentityProviderConfigPtr idp,
|
||||
IdpNetworkRequestManager::FetchStatus status,
|
||||
const GURL& continue_on) {
|
||||
if (!IsFedCmAuthzEnabled()) {
|
||||
// We only allow loading continue_on urls that are same-origin
|
||||
// with the IdP.
|
||||
// This isn't necessarily final, but seemed like a safer
|
||||
// and sufficient default for now.
|
||||
// This behavior may change in https://crbug.com/1429083
|
||||
bool is_same_origin =
|
||||
url::Origin::Create(continue_on)
|
||||
.IsSameOriginWith(url::Origin::Create(idp->config_url));
|
||||
|
||||
if (!IsFedCmAuthzEnabled() || !is_same_origin) {
|
||||
CompleteRequestWithError(
|
||||
FederatedAuthRequestResult::kErrorFetchingIdTokenInvalidResponse,
|
||||
TokenStatus::kIdTokenInvalidResponse,
|
||||
|
@ -4143,7 +4143,7 @@ TEST_F(FederatedAuthRequestImplTest, SuccessfulAuthZRequestWithPopUpWindow) {
|
||||
|
||||
// Set up the network expectations to return a "continue_on" response
|
||||
// rather than the typical idtoken response.
|
||||
GURL continue_on("/more-permissions.php");
|
||||
GURL continue_on = GURL(kProviderUrlFull).Resolve("/more-permissions.php");
|
||||
config.continue_on = std::move(continue_on);
|
||||
|
||||
// Set up the UI dialog controller to show a pop-up window, rather
|
||||
@ -4178,6 +4178,35 @@ TEST_F(FederatedAuthRequestImplTest, SuccessfulAuthZRequestWithPopUpWindow) {
|
||||
EXPECT_FALSE(DidFetch(FetchedEndpoint::CLIENT_METADATA));
|
||||
}
|
||||
|
||||
// Test successful AuthZ request that request the opening of pop-up
|
||||
// windows.
|
||||
TEST_F(FederatedAuthRequestImplTest,
|
||||
FailsLoadingAContinueOnForADifferentOrigin) {
|
||||
base::test::ScopedFeatureList list;
|
||||
list.InitAndEnableFeature(features::kFedCmAuthz);
|
||||
|
||||
RequestParameters parameters = kDefaultRequestParameters;
|
||||
parameters.identity_providers[0].scope = {"calendar.readonly"};
|
||||
|
||||
MockConfiguration config = kConfigurationValid;
|
||||
config.succeed_with_console_message = true;
|
||||
|
||||
// Set up the network expectations to return a "continue_on" response
|
||||
// rather than the typical idtoken response.
|
||||
GURL continue_on =
|
||||
GURL("https://another-origin.example").Resolve("/more-permissions.php");
|
||||
config.continue_on = std::move(continue_on);
|
||||
|
||||
RequestExpectations error = {
|
||||
RequestTokenStatus::kError,
|
||||
/*devtools_issue_statuses=*/{},
|
||||
// TODO(https://crbug.com/1429083): introduce a more granular error.
|
||||
/*standalone_console_messages=*/{"Provider's token is invalid."},
|
||||
/*selected_idp_config_url=*/absl::nullopt};
|
||||
|
||||
RunAuthTest(parameters, error, config);
|
||||
}
|
||||
|
||||
// Test that IdentityRegistry is notified when modal dialog view is closed.
|
||||
TEST_F(FederatedAuthRequestImplTest, IdentityRegistryIsNotified) {
|
||||
EXPECT_FALSE(test_identity_registry_->notified_);
|
||||
|
@ -538,8 +538,6 @@ void OnTokenRequestParsed(
|
||||
|
||||
if (continue_on) {
|
||||
GURL url = token_url.Resolve(*continue_on);
|
||||
// TODO(crbug.com/1429083): check that the continue_on url is
|
||||
// same-origin with the idp origin.
|
||||
if (url.is_valid()) {
|
||||
std::move(continue_on_callback)
|
||||
.Run({ParseStatus::kSuccess, fetch_status.response_code},
|
||||
|
Reference in New Issue
Block a user