Commit 240ba84a authored by Antonio Gomes's avatar Antonio Gomes Committed by Commit Bot

[s13n] Convert permission_request_creator_apiary.cc to IdentityManager

Similarly to [1], this CL replaces the Use of OAuth2TokenService,
OAuth2TokenService::Consumer and SigninManager APIs with the
corresponding IdentityManager counterparts.

The unittests could not be only minimally adapted, and converted on a
follow up pass, given that CL needed to replace the uses of
FakeProfileOAuth2TokenService with IdentityTestEnvironment upfront.

[1] https://crrev.com/c/1348670

BUG=907526

Change-Id: I59c2a49d20341ddadff15a28c7ad81e3056161e6
Reviewed-on: https://chromium-review.googlesource.com/c/1348671
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Reviewed-by: default avatarMarc Treib <treib@chromium.org>
Reviewed-by: default avatarColin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610581}
parent 912050e1
...@@ -13,15 +13,11 @@ ...@@ -13,15 +13,11 @@
#include "base/strings/stringprintf.h" #include "base/strings/stringprintf.h"
#include "base/values.h" #include "base/values.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/signin/profile_oauth2_token_service_factory.h" #include "chrome/browser/signin/identity_manager_factory.h"
#include "chrome/browser/signin/signin_manager_factory.h"
#include "chrome/browser/supervised_user/child_accounts/kids_management_api.h" #include "chrome/browser/supervised_user/child_accounts/kids_management_api.h"
#include "chrome/browser/supervised_user/supervised_user_constants.h" #include "chrome/browser/supervised_user/supervised_user_constants.h"
#include "chrome/common/chrome_switches.h" #include "chrome/common/chrome_switches.h"
#include "components/data_use_measurement/core/data_use_user_data.h" #include "components/data_use_measurement/core/data_use_user_data.h"
#include "components/signin/core/browser/profile_oauth2_token_service.h"
#include "components/signin/core/browser/signin_manager.h"
#include "components/signin/core/browser/signin_manager_base.h"
#include "content/public/browser/browser_context.h" #include "content/public/browser/browser_context.h"
#include "content/public/browser/storage_partition.h" #include "content/public/browser/storage_partition.h"
#include "google_apis/gaia/google_service_auth_error.h" #include "google_apis/gaia/google_service_auth_error.h"
...@@ -29,6 +25,9 @@ ...@@ -29,6 +25,9 @@
#include "net/base/net_errors.h" #include "net/base/net_errors.h"
#include "net/http/http_status_code.h" #include "net/http/http_status_code.h"
#include "net/traffic_annotation/network_traffic_annotation.h" #include "net/traffic_annotation/network_traffic_annotation.h"
#include "services/identity/public/cpp/access_token_fetcher.h"
#include "services/identity/public/cpp/access_token_info.h"
#include "services/identity/public/cpp/identity_manager.h"
#include "services/network/public/cpp/resource_request.h" #include "services/network/public/cpp/resource_request.h"
#include "services/network/public/cpp/shared_url_loader_factory.h" #include "services/network/public/cpp/shared_url_loader_factory.h"
#include "services/network/public/cpp/simple_url_loader.h" #include "services/network/public/cpp/simple_url_loader.h"
...@@ -64,7 +63,7 @@ struct PermissionRequestCreatorApiary::Request { ...@@ -64,7 +63,7 @@ struct PermissionRequestCreatorApiary::Request {
std::string request_type; std::string request_type;
std::string object_ref; std::string object_ref;
SuccessCallback callback; SuccessCallback callback;
std::unique_ptr<OAuth2TokenService::Request> access_token_request; std::unique_ptr<identity::AccessTokenFetcher> access_token_fetcher;
std::string access_token; std::string access_token;
bool access_token_expired; bool access_token_expired;
std::unique_ptr<network::SimpleURLLoader> simple_url_loader; std::unique_ptr<network::SimpleURLLoader> simple_url_loader;
...@@ -82,11 +81,10 @@ PermissionRequestCreatorApiary::Request::Request( ...@@ -82,11 +81,10 @@ PermissionRequestCreatorApiary::Request::Request(
PermissionRequestCreatorApiary::Request::~Request() {} PermissionRequestCreatorApiary::Request::~Request() {}
PermissionRequestCreatorApiary::PermissionRequestCreatorApiary( PermissionRequestCreatorApiary::PermissionRequestCreatorApiary(
OAuth2TokenService* oauth2_token_service, identity::IdentityManager* identity_manager,
const std::string& account_id, const std::string& account_id,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory)
: OAuth2TokenService::Consumer("permissions_creator"), : identity_manager_(identity_manager),
oauth2_token_service_(oauth2_token_service),
account_id_(account_id), account_id_(account_id),
url_loader_factory_(std::move(url_loader_factory)), url_loader_factory_(std::move(url_loader_factory)),
retry_on_network_change_(true) {} retry_on_network_change_(true) {}
...@@ -96,11 +94,9 @@ PermissionRequestCreatorApiary::~PermissionRequestCreatorApiary() {} ...@@ -96,11 +94,9 @@ PermissionRequestCreatorApiary::~PermissionRequestCreatorApiary() {}
// static // static
std::unique_ptr<PermissionRequestCreator> std::unique_ptr<PermissionRequestCreator>
PermissionRequestCreatorApiary::CreateWithProfile(Profile* profile) { PermissionRequestCreatorApiary::CreateWithProfile(Profile* profile) {
ProfileOAuth2TokenService* token_service = auto* identity_manager = IdentityManagerFactory::GetForProfile(profile);
ProfileOAuth2TokenServiceFactory::GetForProfile(profile);
SigninManagerBase* signin = SigninManagerFactory::GetForProfile(profile);
return base::WrapUnique(new PermissionRequestCreatorApiary( return base::WrapUnique(new PermissionRequestCreatorApiary(
token_service, signin->GetAuthenticatedAccountId(), identity_manager, identity_manager->GetPrimaryAccountId(),
content::BrowserContext::GetDefaultStoragePartition(profile) content::BrowserContext::GetDefaultStoragePartition(profile)
->GetURLLoaderFactoryForBrowserProcess())); ->GetURLLoaderFactoryForBrowserProcess()));
} }
...@@ -161,23 +157,41 @@ void PermissionRequestCreatorApiary::CreateRequest( ...@@ -161,23 +157,41 @@ void PermissionRequestCreatorApiary::CreateRequest(
} }
void PermissionRequestCreatorApiary::StartFetching(Request* request) { void PermissionRequestCreatorApiary::StartFetching(Request* request) {
OAuth2TokenService::ScopeSet scopes; identity::ScopeSet scopes;
scopes.insert(GetApiScope()); scopes.insert(GetApiScope());
request->access_token_request = oauth2_token_service_->StartRequest( // It is safe to use Unretained(this) here given that the callback
account_id_, scopes, this); // will not be invoked if this object is deleted. Likewise, |request|
// only comes from |requests_|, which are owned by this object too.
request->access_token_fetcher =
identity_manager_->CreateAccessTokenFetcherForAccount(
account_id_, "permissions_creator", scopes,
base::BindOnce(
&PermissionRequestCreatorApiary::OnAccessTokenFetchComplete,
base::Unretained(this), request),
identity::AccessTokenFetcher::Mode::kImmediate);
} }
void PermissionRequestCreatorApiary::OnGetTokenSuccess( void PermissionRequestCreatorApiary::OnAccessTokenFetchComplete(
const OAuth2TokenService::Request* request, Request* request,
const OAuth2AccessTokenConsumer::TokenResponse& token_response) { GoogleServiceAuthError error,
identity::AccessTokenInfo token_info) {
auto it = requests_.begin(); auto it = requests_.begin();
while (it != requests_.end()) { while (it != requests_.end()) {
if (request == (*it)->access_token_request.get()) if (request->access_token_fetcher.get() ==
(*it)->access_token_fetcher.get()) {
break; break;
}
++it; ++it;
} }
DCHECK(it != requests_.end()); DCHECK(it != requests_.end());
(*it)->access_token = token_response.access_token;
if (error.state() != GoogleServiceAuthError::NONE) {
LOG(WARNING) << "Token error: " << error.ToString();
DispatchResult(it, false);
return;
}
(*it)->access_token = token_info.token;
net::NetworkTrafficAnnotationTag traffic_annotation = net::NetworkTrafficAnnotationTag traffic_annotation =
net::DefineNetworkTrafficAnnotation("permission_request_creator", R"( net::DefineNetworkTrafficAnnotation("permission_request_creator", R"(
...@@ -221,7 +235,7 @@ void PermissionRequestCreatorApiary::OnGetTokenSuccess( ...@@ -221,7 +235,7 @@ void PermissionRequestCreatorApiary::OnGetTokenSuccess(
resource_request->headers.SetHeader( resource_request->headers.SetHeader(
net::HttpRequestHeaders::kAuthorization, net::HttpRequestHeaders::kAuthorization,
base::StringPrintf(supervised_users::kAuthorizationHeaderFormat, base::StringPrintf(supervised_users::kAuthorizationHeaderFormat,
token_response.access_token.c_str())); token_info.token.c_str()));
(*it)->simple_url_loader = network::SimpleURLLoader::Create( (*it)->simple_url_loader = network::SimpleURLLoader::Create(
std::move(resource_request), traffic_annotation); std::move(resource_request), traffic_annotation);
(*it)->simple_url_loader->AttachStringForUpload(body, "application/json"); (*it)->simple_url_loader->AttachStringForUpload(body, "application/json");
...@@ -239,20 +253,6 @@ void PermissionRequestCreatorApiary::OnGetTokenSuccess( ...@@ -239,20 +253,6 @@ void PermissionRequestCreatorApiary::OnGetTokenSuccess(
base::Unretained(this), std::move(it))); base::Unretained(this), std::move(it)));
} }
void PermissionRequestCreatorApiary::OnGetTokenFailure(
const OAuth2TokenService::Request* request,
const GoogleServiceAuthError& error) {
auto it = requests_.begin();
while (it != requests_.end()) {
if (request == (*it)->access_token_request.get())
break;
++it;
}
DCHECK(it != requests_.end());
LOG(WARNING) << "Token error: " << error.ToString();
DispatchResult(it, false);
}
void PermissionRequestCreatorApiary::OnSimpleLoaderComplete( void PermissionRequestCreatorApiary::OnSimpleLoaderComplete(
RequestList::iterator it, RequestList::iterator it,
std::unique_ptr<std::string> response_body) { std::unique_ptr<std::string> response_body) {
...@@ -269,10 +269,10 @@ void PermissionRequestCreatorApiary::OnSimpleLoaderComplete( ...@@ -269,10 +269,10 @@ void PermissionRequestCreatorApiary::OnSimpleLoaderComplete(
if (response_code == net::HTTP_UNAUTHORIZED && if (response_code == net::HTTP_UNAUTHORIZED &&
!request->access_token_expired) { !request->access_token_expired) {
request->access_token_expired = true; request->access_token_expired = true;
OAuth2TokenService::ScopeSet scopes; identity::ScopeSet scopes;
scopes.insert(GetApiScope()); scopes.insert(GetApiScope());
oauth2_token_service_->InvalidateAccessToken(account_id_, scopes, identity_manager_->RemoveAccessTokenFromCache(account_id_, scopes,
request->access_token); request->access_token);
StartFetching(request); StartFetching(request);
return; return;
} }
......
...@@ -12,20 +12,24 @@ ...@@ -12,20 +12,24 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/scoped_refptr.h" #include "base/memory/scoped_refptr.h"
#include "chrome/browser/supervised_user/permission_request_creator.h" #include "chrome/browser/supervised_user/permission_request_creator.h"
#include "google_apis/gaia/oauth2_token_service.h" #include "google_apis/gaia/google_service_auth_error.h"
class GURL; class GURL;
class Profile; class Profile;
namespace identity {
class IdentityManager;
struct AccessTokenInfo;
} // namespace identity
namespace network { namespace network {
class SharedURLLoaderFactory; class SharedURLLoaderFactory;
} }
class PermissionRequestCreatorApiary : public PermissionRequestCreator, class PermissionRequestCreatorApiary : public PermissionRequestCreator {
public OAuth2TokenService::Consumer {
public: public:
PermissionRequestCreatorApiary( PermissionRequestCreatorApiary(
OAuth2TokenService* oauth2_token_service, identity::IdentityManager* identity_manager,
const std::string& account_id, const std::string& account_id,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory); scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory);
~PermissionRequestCreatorApiary() override; ~PermissionRequestCreatorApiary() override;
...@@ -48,12 +52,9 @@ class PermissionRequestCreatorApiary : public PermissionRequestCreator, ...@@ -48,12 +52,9 @@ class PermissionRequestCreatorApiary : public PermissionRequestCreator,
struct Request; struct Request;
using RequestList = std::list<std::unique_ptr<Request>>; using RequestList = std::list<std::unique_ptr<Request>>;
// OAuth2TokenService::Consumer implementation: void OnAccessTokenFetchComplete(Request* request,
void OnGetTokenSuccess( GoogleServiceAuthError error,
const OAuth2TokenService::Request* request, identity::AccessTokenInfo token_info);
const OAuth2AccessTokenConsumer::TokenResponse& token_response) override;
void OnGetTokenFailure(const OAuth2TokenService::Request* request,
const GoogleServiceAuthError& error) override;
void OnSimpleLoaderComplete(RequestList::iterator it, void OnSimpleLoaderComplete(RequestList::iterator it,
std::unique_ptr<std::string> response_body); std::unique_ptr<std::string> response_body);
...@@ -71,7 +72,7 @@ class PermissionRequestCreatorApiary : public PermissionRequestCreator, ...@@ -71,7 +72,7 @@ class PermissionRequestCreatorApiary : public PermissionRequestCreator,
void DispatchResult(RequestList::iterator it, bool success); void DispatchResult(RequestList::iterator it, bool success);
OAuth2TokenService* oauth2_token_service_; identity::IdentityManager* identity_manager_;
std::string account_id_; std::string account_id_;
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_; scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
bool retry_on_network_change_; bool retry_on_network_change_;
......
...@@ -11,10 +11,9 @@ ...@@ -11,10 +11,9 @@
#include "base/message_loop/message_loop.h" #include "base/message_loop/message_loop.h"
#include "base/threading/thread_task_runner_handle.h" #include "base/threading/thread_task_runner_handle.h"
#include "base/values.h" #include "base/values.h"
#include "components/prefs/testing_pref_service.h"
#include "components/signin/core/browser/fake_profile_oauth2_token_service.h"
#include "net/base/net_errors.h" #include "net/base/net_errors.h"
#include "net/http/http_util.h" #include "net/http/http_util.h"
#include "services/identity/public/cpp/identity_test_environment.h"
#include "services/network/public/cpp/shared_url_loader_factory.h" #include "services/network/public/cpp/shared_url_loader_factory.h"
#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h" #include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
#include "services/network/test/test_url_loader_factory.h" #include "services/network/test/test_url_loader_factory.h"
...@@ -23,7 +22,7 @@ ...@@ -23,7 +22,7 @@
namespace { namespace {
const char kAccountId[] = "account@gmail.com"; const char kEmail[] = "account@gmail.com";
std::string BuildResponse() { std::string BuildResponse() {
base::DictionaryValue dict; base::DictionaryValue dict;
...@@ -40,29 +39,27 @@ std::string BuildResponse() { ...@@ -40,29 +39,27 @@ std::string BuildResponse() {
class PermissionRequestCreatorApiaryTest : public testing::Test { class PermissionRequestCreatorApiaryTest : public testing::Test {
public: public:
PermissionRequestCreatorApiaryTest() PermissionRequestCreatorApiaryTest()
: token_service_(&pref_service_), : test_shared_loader_factory_(
test_shared_loader_factory_(
base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>( base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
&test_url_loader_factory_)), &test_url_loader_factory_)) {
permission_creator_(&token_service_, AccountInfo account_info = identity_test_env_.MakeAccountAvailable(kEmail);
kAccountId, account_id_ = account_info.account_id;
test_shared_loader_factory_) { permission_creator_ = std::make_unique<PermissionRequestCreatorApiary>(
permission_creator_.retry_on_network_change_ = false; identity_test_env_.identity_manager(), account_id_,
token_service_.UpdateCredentials(kAccountId, "refresh_token"); test_shared_loader_factory_);
permission_creator_->retry_on_network_change_ = false;
} }
protected: protected:
void IssueAccessTokens() { void IssueAccessTokens() {
token_service_.IssueAllTokensForAccount( identity_test_env_.WaitForAccessTokenRequestIfNecessaryAndRespondWithToken(
kAccountId, account_id_, "access_token",
"access_token",
base::Time::Now() + base::TimeDelta::FromHours(1)); base::Time::Now() + base::TimeDelta::FromHours(1));
} }
void IssueAccessTokenErrors() { void IssueAccessTokenErrors() {
token_service_.IssueErrorForAllPendingRequestsForAccount( identity_test_env_.WaitForAccessTokenRequestIfNecessaryAndRespondWithError(
kAccountId, account_id_, GoogleServiceAuthError::FromServiceError("Error!"));
GoogleServiceAuthError::FromServiceError("Error!"));
} }
void SetupResponse(net::Error error, const std::string& response) { void SetupResponse(net::Error error, const std::string& response) {
...@@ -72,12 +69,12 @@ class PermissionRequestCreatorApiaryTest : public testing::Test { ...@@ -72,12 +69,12 @@ class PermissionRequestCreatorApiaryTest : public testing::Test {
net::HttpUtil::AssembleRawHeaders(headers.c_str(), headers.size())); net::HttpUtil::AssembleRawHeaders(headers.c_str(), headers.size()));
network::URLLoaderCompletionStatus status(error); network::URLLoaderCompletionStatus status(error);
status.decoded_body_length = response.size(); status.decoded_body_length = response.size();
test_url_loader_factory_.AddResponse(permission_creator_.GetApiUrl(), head, test_url_loader_factory_.AddResponse(permission_creator_->GetApiUrl(), head,
response, status); response, status);
} }
void CreateRequest(const GURL& url) { void CreateRequest(const GURL& url) {
permission_creator_.CreateURLAccessRequest( permission_creator_->CreateURLAccessRequest(
url, url,
base::BindOnce(&PermissionRequestCreatorApiaryTest::OnRequestCreated, base::BindOnce(&PermissionRequestCreatorApiaryTest::OnRequestCreated,
base::Unretained(this))); base::Unretained(this)));
...@@ -88,20 +85,17 @@ class PermissionRequestCreatorApiaryTest : public testing::Test { ...@@ -88,20 +85,17 @@ class PermissionRequestCreatorApiaryTest : public testing::Test {
MOCK_METHOD1(OnRequestCreated, void(bool success)); MOCK_METHOD1(OnRequestCreated, void(bool success));
base::MessageLoop message_loop_; base::MessageLoop message_loop_;
TestingPrefServiceSimple pref_service_; std::string account_id_;
FakeProfileOAuth2TokenService token_service_; identity::IdentityTestEnvironment identity_test_env_;
network::TestURLLoaderFactory test_url_loader_factory_; network::TestURLLoaderFactory test_url_loader_factory_;
scoped_refptr<network::SharedURLLoaderFactory> test_shared_loader_factory_; scoped_refptr<network::SharedURLLoaderFactory> test_shared_loader_factory_;
PermissionRequestCreatorApiary permission_creator_; std::unique_ptr<PermissionRequestCreatorApiary> permission_creator_;
}; };
TEST_F(PermissionRequestCreatorApiaryTest, Success) { TEST_F(PermissionRequestCreatorApiaryTest, Success) {
CreateRequest(GURL("http://randomurl.com")); CreateRequest(GURL("http://randomurl.com"));
CreateRequest(GURL("http://anotherurl.com")); CreateRequest(GURL("http://anotherurl.com"));
// We should have gotten a request for an access token.
EXPECT_GT(token_service_.GetPendingRequests().size(), 0U);
IssueAccessTokens(); IssueAccessTokens();
EXPECT_CALL(*this, OnRequestCreated(true)).Times(2); EXPECT_CALL(*this, OnRequestCreated(true)).Times(2);
...@@ -113,9 +107,6 @@ TEST_F(PermissionRequestCreatorApiaryTest, Success) { ...@@ -113,9 +107,6 @@ TEST_F(PermissionRequestCreatorApiaryTest, Success) {
TEST_F(PermissionRequestCreatorApiaryTest, AccessTokenError) { TEST_F(PermissionRequestCreatorApiaryTest, AccessTokenError) {
CreateRequest(GURL("http://randomurl.com")); CreateRequest(GURL("http://randomurl.com"));
// We should have gotten a request for an access token.
EXPECT_EQ(1U, token_service_.GetPendingRequests().size());
// Our callback should get called immediately on an error. // Our callback should get called immediately on an error.
EXPECT_CALL(*this, OnRequestCreated(false)); EXPECT_CALL(*this, OnRequestCreated(false));
IssueAccessTokenErrors(); IssueAccessTokenErrors();
...@@ -125,9 +116,6 @@ TEST_F(PermissionRequestCreatorApiaryTest, NetworkError) { ...@@ -125,9 +116,6 @@ TEST_F(PermissionRequestCreatorApiaryTest, NetworkError) {
const GURL& url = GURL("http://randomurl.com"); const GURL& url = GURL("http://randomurl.com");
CreateRequest(url); CreateRequest(url);
// We should have gotten a request for an access token.
EXPECT_EQ(1U, token_service_.GetPendingRequests().size());
IssueAccessTokens(); IssueAccessTokens();
// Our callback should get called on an error. // Our callback should get called on an error.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment