Commit 5883f287 authored by Mihai Sardarescu's avatar Mihai Sardarescu Committed by Commit Bot

Clean-up the DeviceOAuth2TokenService public API.

This CL removes the robot account ID from the refresh token API of
the DeviceOAuth2TokenService as there is a single account in the
DeviceOAuth2TokenService. It also marks as private the
OAuth2TokenManagerDelegate functions that it implements.

Bug: 1028509

Change-Id: I0e6ab3255194d3181f3206304ddd61b3c4abda20
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2022705
Commit-Queue: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: default avatarColin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#737304}
parent 05689693
......@@ -674,8 +674,7 @@ class DeviceCloudPolicyManagerChromeOSEnrollmentTest
// account ID must not be empty.
token_service->set_robot_account_id_for_testing(CoreAccountId("dummy"));
EXPECT_TRUE(token_service->RefreshTokenIsAvailable(
token_service->GetRobotAccountId()));
EXPECT_TRUE(token_service->RefreshTokenIsAvailable());
EXPECT_EQ(device_policy_->GetBlob(),
session_manager_client_.device_policy());
}
......
......@@ -163,8 +163,7 @@ void CRDHostDelegate::FetchOAuthToken(
oauth_success_callback_ = std::move(success_callback);
error_callback_ = std::move(error_callback);
oauth_request_ = oauth_service->StartAccessTokenRequest(
oauth_service->GetRobotAccountId(), scopes, this);
oauth_request_ = oauth_service->StartAccessTokenRequest(scopes, this);
}
void CRDHostDelegate::OnGetTokenSuccess(
......
......@@ -18,7 +18,6 @@ class ActiveAccountAccessTokenFetcherImpl
OAuth2AccessTokenManager::Consumer {
public:
ActiveAccountAccessTokenFetcherImpl(
const CoreAccountId& active_account_id,
const std::string& oauth_consumer_name,
DeviceOAuth2TokenService* token_service,
const OAuth2AccessTokenManager::ScopeSet& scopes,
......@@ -48,15 +47,13 @@ class ActiveAccountAccessTokenFetcherImpl
} // namespace
ActiveAccountAccessTokenFetcherImpl::ActiveAccountAccessTokenFetcherImpl(
const CoreAccountId& active_account_id,
const std::string& oauth_consumer_name,
DeviceOAuth2TokenService* token_service,
const OAuth2AccessTokenManager::ScopeSet& scopes,
invalidation::ActiveAccountAccessTokenCallback callback)
: OAuth2AccessTokenManager::Consumer(oauth_consumer_name),
callback_(std::move(callback)) {
access_token_request_ =
token_service->StartAccessTokenRequest(active_account_id, scopes, this);
access_token_request_ = token_service->StartAccessTokenRequest(scopes, this);
}
ActiveAccountAccessTokenFetcherImpl::~ActiveAccountAccessTokenFetcherImpl() {}
......@@ -126,7 +123,7 @@ void DeviceIdentityProvider::SetActiveAccountId(
bool DeviceIdentityProvider::IsActiveAccountWithRefreshToken() {
if (GetActiveAccountId().empty() || !token_service_ ||
!token_service_->RefreshTokenIsAvailable(GetActiveAccountId()))
!token_service_->RefreshTokenIsAvailable())
return false;
return true;
......@@ -138,20 +135,17 @@ DeviceIdentityProvider::FetchAccessToken(
const OAuth2AccessTokenManager::ScopeSet& scopes,
invalidation::ActiveAccountAccessTokenCallback callback) {
return std::make_unique<ActiveAccountAccessTokenFetcherImpl>(
GetActiveAccountId(), oauth_consumer_name, token_service_, scopes,
std::move(callback));
oauth_consumer_name, token_service_, scopes, std::move(callback));
}
void DeviceIdentityProvider::InvalidateAccessToken(
const OAuth2AccessTokenManager::ScopeSet& scopes,
const std::string& access_token) {
token_service_->InvalidateAccessToken(GetActiveAccountId(), scopes,
access_token);
token_service_->InvalidateAccessToken(scopes, access_token);
}
void DeviceIdentityProvider::OnRefreshTokenAvailable(
const CoreAccountId& account_id) {
ProcessRefreshTokenUpdateForAccount(account_id);
void DeviceIdentityProvider::OnRefreshTokenAvailable() {
ProcessRefreshTokenUpdateForAccount(GetActiveAccountId());
}
} // namespace chromeos
......@@ -31,9 +31,9 @@ class DeviceIdentityProvider : public invalidation::IdentityProvider {
const std::string& access_token) override;
void SetActiveAccountId(const CoreAccountId& account_id) override;
void OnRefreshTokenAvailable(const CoreAccountId& account_id);
private:
void OnRefreshTokenAvailable();
chromeos::DeviceOAuth2TokenService* token_service_;
DISALLOW_COPY_AND_ASSIGN(DeviceIdentityProvider);
......
......@@ -94,7 +94,7 @@ void DeviceOAuth2TokenService::SetAndSaveRefreshToken(
// will be done from OnServiceAccountIdentityChanged() once the robot account
// ID becomes available as well.
if (!GetRobotAccountId().empty())
FireRefreshTokenAvailable(GetRobotAccountId());
FireRefreshTokenAvailable();
token_save_callbacks_.push_back(result_callback);
if (!waiting_for_salt) {
......@@ -127,24 +127,37 @@ void DeviceOAuth2TokenService::SetRefreshTokenAvailableCallback(
std::unique_ptr<OAuth2AccessTokenManager::Request>
DeviceOAuth2TokenService::StartAccessTokenRequest(
const CoreAccountId& account_id,
const OAuth2AccessTokenManager::ScopeSet& scopes,
OAuth2AccessTokenManager::Consumer* consumer) {
return token_manager_->StartRequest(account_id, scopes, consumer);
// Note: It is fine to pass an empty account id to |token_manager_| as this
// will just return a request that will always fail.
return token_manager_->StartRequest(GetRobotAccountId(), scopes, consumer);
}
void DeviceOAuth2TokenService::InvalidateAccessToken(
const CoreAccountId& account_id,
const OAuth2AccessTokenManager::ScopeSet& scopes,
const std::string& access_token) {
token_manager_->InvalidateAccessToken(account_id, scopes, access_token);
if (GetRobotAccountId().empty())
return;
token_manager_->InvalidateAccessToken(GetRobotAccountId(), scopes,
access_token);
}
bool DeviceOAuth2TokenService::RefreshTokenIsAvailable(
const CoreAccountId& account_id) const {
auto accounts = GetAccounts();
return std::find(accounts.begin(), accounts.end(), account_id) !=
accounts.end();
bool DeviceOAuth2TokenService::RefreshTokenIsAvailable() const {
switch (state_) {
case STATE_NO_TOKEN:
case STATE_TOKEN_INVALID:
return false;
case STATE_LOADING:
case STATE_VALIDATION_PENDING:
case STATE_VALIDATION_STARTED:
case STATE_TOKEN_VALID:
return !GetRobotAccountId().empty();
}
NOTREACHED() << "Unhandled state " << state_;
return false;
}
OAuth2AccessTokenManager* DeviceOAuth2TokenService::GetAccessTokenManager() {
......@@ -196,7 +209,13 @@ DeviceOAuth2TokenService::CreateAccessTokenFetcher(
bool DeviceOAuth2TokenService::HasRefreshToken(
const CoreAccountId& account_id) const {
return RefreshTokenIsAvailable(account_id);
if (account_id.empty())
return false;
if (GetRobotAccountId() != account_id)
return false;
return RefreshTokenIsAvailable();
}
scoped_refptr<network::SharedURLLoaderFactory>
......@@ -204,10 +223,12 @@ DeviceOAuth2TokenService::GetURLLoaderFactory() const {
return url_loader_factory_;
}
void DeviceOAuth2TokenService::FireRefreshTokenAvailable(
const CoreAccountId& account_id) {
if (on_refresh_token_available_callback_)
on_refresh_token_available_callback_.Run(account_id);
void DeviceOAuth2TokenService::FireRefreshTokenAvailable() {
if (!on_refresh_token_available_callback_)
return;
DCHECK(!GetRobotAccountId().empty());
on_refresh_token_available_callback_.Run();
}
bool DeviceOAuth2TokenService::HandleAccessTokenFetch(
......@@ -284,28 +305,9 @@ void DeviceOAuth2TokenService::FailRequest(
OAuth2AccessTokenConsumer::TokenResponse()));
}
std::vector<CoreAccountId> DeviceOAuth2TokenService::GetAccounts() const {
std::vector<CoreAccountId> accounts;
switch (state_) {
case STATE_NO_TOKEN:
case STATE_TOKEN_INVALID:
return accounts;
case STATE_LOADING:
case STATE_VALIDATION_PENDING:
case STATE_VALIDATION_STARTED:
case STATE_TOKEN_VALID:
if (!GetRobotAccountId().empty())
accounts.push_back(GetRobotAccountId());
return accounts;
}
NOTREACHED() << "Unhandled state " << state_;
return accounts;
}
void DeviceOAuth2TokenService::OnServiceAccountIdentityChanged() {
if (!GetRobotAccountId().empty() && !refresh_token_.empty())
FireRefreshTokenAvailable(GetRobotAccountId());
FireRefreshTokenAvailable();
}
void DeviceOAuth2TokenService::CheckRobotAccountId(
......@@ -407,7 +409,7 @@ void DeviceOAuth2TokenService::DidGetSystemSalt(
// Announce the token.
if (!GetRobotAccountId().empty()) {
FireRefreshTokenAvailable(GetRobotAccountId());
FireRefreshTokenAvailable();
}
}
......
......@@ -31,16 +31,12 @@ namespace chromeos {
// DeviceOAuth2TokenService retrieves OAuth2 access tokens for a given
// set of scopes using the device-level OAuth2 any-api refresh token
// obtained during enterprise device enrollment.
// When using DeviceOAuth2TokenService, a value of |GetRobotAccountId| should
// be used in places where API expects |account_id|.
//
// Note that requests must be made from the UI thread.
class DeviceOAuth2TokenService : public OAuth2AccessTokenManager::Delegate,
public gaia::GaiaOAuthClient::Delegate {
public:
typedef base::RepeatingCallback<void(const CoreAccountId& /* account_id */)>
RefreshTokenAvailableCallback;
typedef base::RepeatingCallback<void()> RefreshTokenAvailableCallback;
typedef base::Callback<void(bool)> StatusCallback;
// Persist the given refresh token on the device. Overwrites any previous
......@@ -54,16 +50,14 @@ class DeviceOAuth2TokenService : public OAuth2AccessTokenManager::Delegate,
// Pull the robot account ID from device policy.
CoreAccountId GetRobotAccountId() const;
// Can be used to override the robot account ID for testing purposes. Most
// common use case is to easily inject a non-empty account ID to make the
// refresh token for the robot account visible via GetAccounts() and
// RefreshTokenIsAvailable().
void set_robot_account_id_for_testing(const CoreAccountId& account_id);
// If set, this callback will be invoked when a new refresh token is
// available.
void SetRefreshTokenAvailableCallback(RefreshTokenAvailableCallback callback);
// Returns true if the refresh token is available and if the clients of this
// class may start fetching access tokens.
bool RefreshTokenIsAvailable() const;
// Checks in the cache for a valid access token for a specified |account_id|
// and |scopes|, and if not found starts a request for an OAuth2 access token
// using the OAuth2 refresh token maintained by this instance for that
......@@ -72,7 +66,6 @@ class DeviceOAuth2TokenService : public OAuth2AccessTokenManager::Delegate,
// the object that will be called back with results if the returned request
// is not deleted.
std::unique_ptr<OAuth2AccessTokenManager::Request> StartAccessTokenRequest(
const CoreAccountId& account_id,
const OAuth2AccessTokenManager::ScopeSet& scopes,
OAuth2AccessTokenManager::Consumer* consumer);
......@@ -81,21 +74,16 @@ class DeviceOAuth2TokenService : public OAuth2AccessTokenManager::Delegate,
// but was not accepted by the server (e.g., the server returned
// 401 Unauthorized). The token will be removed from the cache for the given
// scopes.
void InvalidateAccessToken(const CoreAccountId& account_id,
const OAuth2AccessTokenManager::ScopeSet& scopes,
void InvalidateAccessToken(const OAuth2AccessTokenManager::ScopeSet& scopes,
const std::string& access_token);
bool RefreshTokenIsAvailable(const CoreAccountId& account_id) const;
OAuth2AccessTokenManager* GetAccessTokenManager();
// gaia::GaiaOAuthClient::Delegate implementation.
void OnRefreshTokenResponse(const std::string& access_token,
int expires_in_seconds) override;
void OnGetTokenInfoResponse(
std::unique_ptr<base::DictionaryValue> token_info) override;
void OnOAuthError() override;
void OnNetworkError(int response_code) override;
// Can be used to override the robot account ID for testing purposes. Most
// common use case is to easily inject a non-empty account ID to make the
// refresh token for the robot account visible via GetAccounts() and
// RefreshTokenIsAvailable().
void set_robot_account_id_for_testing(const CoreAccountId& account_id);
private:
friend class DeviceOAuth2TokenServiceFactory;
......@@ -118,6 +106,14 @@ class DeviceOAuth2TokenService : public OAuth2AccessTokenManager::Delegate,
STATE_TOKEN_VALID,
};
// gaia::GaiaOAuthClient::Delegate implementation.
void OnRefreshTokenResponse(const std::string& access_token,
int expires_in_seconds) override;
void OnGetTokenInfoResponse(
std::unique_ptr<base::DictionaryValue> token_info) override;
void OnOAuthError() override;
void OnNetworkError(int response_code) override;
// OAuth2AccessTokenManager::Delegate:
std::unique_ptr<OAuth2AccessTokenFetcher> CreateAccessTokenFetcher(
const CoreAccountId& account_id,
......@@ -134,8 +130,7 @@ class DeviceOAuth2TokenService : public OAuth2AccessTokenManager::Delegate,
const std::string& client_secret,
const OAuth2AccessTokenManager::ScopeSet& scopes) override;
void FireRefreshTokenAvailable(const CoreAccountId& account_id);
void FireRefreshTokenRevoked(const CoreAccountId& account_id);
void FireRefreshTokenAvailable();
// Use DeviceOAuth2TokenServiceFactory to get an instance of this class.
explicit DeviceOAuth2TokenService(
......@@ -151,9 +146,6 @@ class DeviceOAuth2TokenService : public OAuth2AccessTokenManager::Delegate,
void FailRequest(OAuth2AccessTokenManager::RequestImpl* request,
GoogleServiceAuthError::State error);
// Returns a list of accounts based on |state_|.
std::vector<CoreAccountId> GetAccounts() const;
// Starts the token validation flow, i.e. token info fetch.
void StartValidation();
......
......@@ -73,9 +73,8 @@ class DeviceOAuth2TokenServiceTest : public testing::Test {
}
std::unique_ptr<OAuth2AccessTokenManager::Request> StartTokenRequest() {
return oauth2_service_->StartAccessTokenRequest(
oauth2_service_->GetRobotAccountId(), std::set<std::string>(),
&consumer_);
return oauth2_service_->StartAccessTokenRequest(std::set<std::string>(),
&consumer_);
}
void SetUp() override {
......@@ -126,8 +125,7 @@ class DeviceOAuth2TokenServiceTest : public testing::Test {
}
bool RefreshTokenIsAvailable() {
return oauth2_service_->RefreshTokenIsAvailable(
oauth2_service_->GetRobotAccountId());
return oauth2_service_->RefreshTokenIsAvailable();
}
std::string GetRefreshToken() {
......@@ -441,10 +439,10 @@ TEST_F(DeviceOAuth2TokenServiceTest, RefreshTokenValidation_Retry) {
TEST_F(DeviceOAuth2TokenServiceTest, DoNotAnnounceTokenWithoutAccountID) {
CreateService();
auto callback_without_id = base::BindRepeating(
[](const CoreAccountId& account_id) { EXPECT_TRUE(false); });
auto callback_that_should_not_be_called =
base::BindRepeating([]() { FAIL(); });
oauth2_service_->SetRefreshTokenAvailableCallback(
std::move(callback_without_id));
std::move(callback_that_should_not_be_called));
// Make a token available during enrollment. Verify that the token is not
// announced yet.
......@@ -452,12 +450,10 @@ TEST_F(DeviceOAuth2TokenServiceTest, DoNotAnnounceTokenWithoutAccountID) {
"test-token", DeviceOAuth2TokenService::StatusCallback());
base::RunLoop run_loop;
auto callback_with_id =
base::BindRepeating([](base::RunLoop* loop,
const CoreAccountId& account_id) { loop->Quit(); },
&run_loop);
auto callback_that_should_be_called_once =
base::BindRepeating([](base::RunLoop* loop) { loop->Quit(); }, &run_loop);
oauth2_service_->SetRefreshTokenAvailableCallback(
std::move(callback_with_id));
std::move(callback_that_should_be_called_once));
// Also make the robot account ID available. Verify that the token is
// announced now.
......
......@@ -820,8 +820,7 @@ void IdentityGetAuthTokenFunction::StartDeviceAccessTokenRequest() {
// request access token for [any-api] instead of login.
OAuth2AccessTokenManager::ScopeSet scopes;
scopes.insert(GaiaConstants::kAnyApiOAuth2Scope);
device_access_token_request_ = service->StartAccessTokenRequest(
service->GetRobotAccountId(), scopes, this);
device_access_token_request_ = service->StartAccessTokenRequest(scopes, this);
}
bool IdentityGetAuthTokenFunction::IsOriginWhitelistedInPublicSession() {
......
......@@ -492,10 +492,7 @@ class PrintPreviewHandler::AccessTokenService
chromeos::DeviceOAuth2TokenService* token_service =
chromeos::DeviceOAuth2TokenServiceFactory::Get();
CoreAccountId account_id = token_service->GetRobotAccountId();
device_request_ =
token_service->StartAccessTokenRequest(account_id, scopes, this);
device_request_ = token_service->StartAccessTokenRequest(scopes, this);
device_request_callback_ = std::move(callback);
}
......
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