0

Ash: Treat RAPT errors as SCOPE_LIMITED_UNRECOVERABLE_ERROR

Treat RAPT errors as SCOPE_LIMITED_UNRECOVERABLE_ERROR on Ash. Please
check the attached bug for context.

Bug: b/295127872
Test: google_apis_unittests --gtest_filter="*OAuth2AccessTokenFetcherImplTest*"
Change-Id: I0c4e3379ccbe78642763d5c407e485e64a77198b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4765130
Reviewed-by: Anastasiia N <anastasiian@chromium.org>
Commit-Queue: Kush Sinha <sinhak@chromium.org>
Reviewed-by: Monica Basta <msalama@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1184643}
This commit is contained in:
Kush Sinha
2023-08-17 11:19:00 +00:00
committed by Chromium LUCI CQ
parent ba5ca05406
commit 2c0e568975
4 changed files with 82 additions and 9 deletions

@@ -189,7 +189,7 @@ std::string GoogleServiceAuthError::ToString() const {
return base::StringPrintf("Service responded with error: '%s'", return base::StringPrintf("Service responded with error: '%s'",
error_message_.c_str()); error_message_.c_str());
case SCOPE_LIMITED_UNRECOVERABLE_ERROR: case SCOPE_LIMITED_UNRECOVERABLE_ERROR:
return base::StringPrintf("Service responded with error: '%s'", return base::StringPrintf("OAuth scope error: '%s'",
error_message_.c_str()); error_message_.c_str());
case CHALLENGE_RESPONSE_REQUIRED: case CHALLENGE_RESPONSE_REQUIRED:
return "Service responded with a token binding challenge."; return "Service responded with a token binding challenge.";

@@ -4,7 +4,6 @@
#include "google_apis/gaia/oauth2_access_token_fetcher_impl.h" #include "google_apis/gaia/oauth2_access_token_fetcher_impl.h"
#include <algorithm>
#include <string> #include <string>
#include <vector> #include <vector>
@@ -16,6 +15,7 @@
#include "base/strings/stringprintf.h" #include "base/strings/stringprintf.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "base/values.h" #include "base/values.h"
#include "build/chromeos_buildflags.h"
#include "google_apis/credentials_mode.h" #include "google_apis/credentials_mode.h"
#include "google_apis/gaia/gaia_auth_util.h" #include "google_apis/gaia/gaia_auth_util.h"
#include "google_apis/gaia/google_service_auth_error.h" #include "google_apis/gaia/google_service_auth_error.h"
@@ -25,6 +25,12 @@
#include "services/network/public/cpp/simple_url_loader.h" #include "services/network/public/cpp/simple_url_loader.h"
#include "services/network/public/mojom/url_response_head.mojom.h" #include "services/network/public/mojom/url_response_head.mojom.h"
#if BUILDFLAG(IS_CHROMEOS_ASH)
BASE_FEATURE(kIgnoreRaptErrors,
"IgnoreRaptErrors",
base::FEATURE_ENABLED_BY_DEFAULT);
#endif // BUILDFLAG(IS_CHROMEOS_ASH)
namespace { namespace {
constexpr char kGetAccessTokenBodyFormat[] = constexpr char kGetAccessTokenBodyFormat[] =
@@ -54,6 +60,11 @@ constexpr char kErrorKey[] = "error";
constexpr char kErrorSubTypeKey[] = "error_subtype"; constexpr char kErrorSubTypeKey[] = "error_subtype";
constexpr char kErrorDescriptionKey[] = "error_description"; constexpr char kErrorDescriptionKey[] = "error_description";
#if BUILDFLAG(IS_CHROMEOS_ASH)
constexpr char kRaptRequiredError[] = "rapt_required";
constexpr char kInvalidRaptError[] = "invalid_rapt";
#endif // BUILDFLAG(IS_CHROMEOS_ASH)
OAuth2AccessTokenFetcherImpl::OAuth2Response OAuth2AccessTokenFetcherImpl::OAuth2Response
OAuth2ResponseErrorToOAuth2Response(const std::string& error) { OAuth2ResponseErrorToOAuth2Response(const std::string& error) {
if (error.empty()) if (error.empty())
@@ -120,6 +131,33 @@ static std::unique_ptr<network::SimpleURLLoader> CreateURLLoader(
return url_loader; return url_loader;
} }
GoogleServiceAuthError CreateErrorForInvalidGrant(
const std::string& error_subtype,
const std::string& error_description) {
#if BUILDFLAG(IS_CHROMEOS_ASH)
if (base::FeatureList::IsEnabled(kIgnoreRaptErrors)) {
// ChromeOS cannot handle RAPT-type re-authentication requests and is
// supposed to be excluded from RAPT re-authentication on the server side.
// Just to be safe we need to handle this anyways. If we do not handle this,
// any service requesting a RAPT re-auth protected OAuth scope can
// potentially invalidate the entire ChromeOS session and send the user into
// a never ending re-authentication loop.
std::string error_subtype_lowercase = base::ToLowerASCII(error_subtype);
if (error_subtype_lowercase == kRaptRequiredError ||
error_subtype_lowercase == kInvalidRaptError) {
return GoogleServiceAuthError::FromScopeLimitedUnrecoverableError(
error_description);
}
}
#endif // BUILDFLAG(IS_CHROMEOS_ASH)
// Persistent error requiring the user to sign in again.
return GoogleServiceAuthError::FromInvalidGaiaCredentialsReason(
GoogleServiceAuthError::InvalidGaiaCredentialsReason::
CREDENTIALS_REJECTED_BY_SERVER);
}
} // namespace } // namespace
OAuth2AccessTokenFetcherImpl::OAuth2AccessTokenFetcherImpl( OAuth2AccessTokenFetcherImpl::OAuth2AccessTokenFetcherImpl(
@@ -229,10 +267,7 @@ void OAuth2AccessTokenFetcherImpl::EndGetAccessToken(
break; break;
case kInvalidGrant: case kInvalidGrant:
// Persistent error requiring the user to sign in again. error = CreateErrorForInvalidGrant(error_subtype, error_description);
error = GoogleServiceAuthError::FromInvalidGaiaCredentialsReason(
GoogleServiceAuthError::InvalidGaiaCredentialsReason::
CREDENTIALS_REJECTED_BY_SERVER);
break; break;
case kInvalidScope: case kInvalidScope:
@@ -277,9 +312,7 @@ void OAuth2AccessTokenFetcherImpl::EndGetAccessToken(
// HTTP_BAD_REQUEST errors usually contains errors as per // HTTP_BAD_REQUEST errors usually contains errors as per
// http://tools.ietf.org/html/rfc6749#section-5.2. // http://tools.ietf.org/html/rfc6749#section-5.2.
if (response == kInvalidGrant) { if (response == kInvalidGrant) {
error = GoogleServiceAuthError::FromInvalidGaiaCredentialsReason( error = CreateErrorForInvalidGrant(error_subtype, error_description);
GoogleServiceAuthError::InvalidGaiaCredentialsReason::
CREDENTIALS_REJECTED_BY_SERVER);
} else { } else {
error = GoogleServiceAuthError::FromServiceError(response_str); error = GoogleServiceAuthError::FromServiceError(response_str);
} }

@@ -10,8 +10,10 @@
#include <vector> #include <vector>
#include "base/component_export.h" #include "base/component_export.h"
#include "base/feature_list.h"
#include "base/gtest_prod_util.h" #include "base/gtest_prod_util.h"
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
#include "build/chromeos_buildflags.h"
#include "google_apis/gaia/oauth2_access_token_consumer.h" #include "google_apis/gaia/oauth2_access_token_consumer.h"
#include "google_apis/gaia/oauth2_access_token_fetcher.h" #include "google_apis/gaia/oauth2_access_token_fetcher.h"
#include "net/traffic_annotation/network_traffic_annotation.h" #include "net/traffic_annotation/network_traffic_annotation.h"
@@ -24,6 +26,10 @@ class SimpleURLLoader;
class SharedURLLoaderFactory; class SharedURLLoaderFactory;
} }
#if BUILDFLAG(IS_CHROMEOS_ASH)
COMPONENT_EXPORT(GOOGLE_APIS) BASE_DECLARE_FEATURE(kIgnoreRaptErrors);
#endif // BUILDFLAG(IS_CHROMEOS_ASH)
// Abstracts the details to get OAuth2 access token from OAuth2 refresh token. // Abstracts the details to get OAuth2 access token from OAuth2 refresh token.
// See general document about Oauth2 in https://tools.ietf.org/html/rfc6749. // See general document about Oauth2 in https://tools.ietf.org/html/rfc6749.
// //

@@ -13,8 +13,10 @@
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/strings/stringprintf.h" #include "base/strings/stringprintf.h"
#include "base/test/metrics/histogram_tester.h" #include "base/test/metrics/histogram_tester.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/task_environment.h" #include "base/test/task_environment.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "build/chromeos_buildflags.h"
#include "google_apis/gaia/gaia_access_token_fetcher.h" #include "google_apis/gaia/gaia_access_token_fetcher.h"
#include "google_apis/gaia/gaia_urls.h" #include "google_apis/gaia/gaia_urls.h"
#include "google_apis/gaia/google_service_auth_error.h" #include "google_apis/gaia/google_service_auth_error.h"
@@ -222,6 +224,38 @@ TEST_F(OAuth2AccessTokenFetcherImplTest, CancelOngoingRequest) {
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
} }
#if BUILDFLAG(IS_CHROMEOS_ASH)
TEST_F(OAuth2AccessTokenFetcherImplTest, GetAccessTokenRaptRequiredFailure) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(kIgnoreRaptErrors);
SetupGetAccessToken(net::OK, net::HTTP_BAD_REQUEST,
kRaptRequiredErrorResponse);
EXPECT_CALL(consumer_,
OnGetTokenFailure(
GoogleServiceAuthError::FromScopeLimitedUnrecoverableError(
"reauth related error")))
.Times(1);
fetcher_->Start("client_id", "client_secret", ScopeList());
base::RunLoop().RunUntilIdle();
}
TEST_F(OAuth2AccessTokenFetcherImplTest,
GetAccessTokenRaptRequiredFailureIfIgnoreRaptErrorsIsDisabled) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndDisableFeature(kIgnoreRaptErrors);
SetupGetAccessToken(net::OK, net::HTTP_BAD_REQUEST,
kRaptRequiredErrorResponse);
EXPECT_CALL(consumer_,
OnGetTokenFailure(
GoogleServiceAuthError::FromInvalidGaiaCredentialsReason(
GoogleServiceAuthError::InvalidGaiaCredentialsReason::
CREDENTIALS_REJECTED_BY_SERVER)))
.Times(1);
fetcher_->Start("client_id", "client_secret", ScopeList());
base::RunLoop().RunUntilIdle();
}
#endif // BUILDFLAG(IS_CHROMEOS_ASH)
TEST_F(OAuth2AccessTokenFetcherImplTest, MakeGetAccessTokenBodyNoScope) { TEST_F(OAuth2AccessTokenFetcherImplTest, MakeGetAccessTokenBodyNoScope) {
std::string body = std::string body =
"client_id=cid1&" "client_id=cid1&"