Commit 33342491 authored by Sam McNally's avatar Sam McNally Committed by Commit Bot

Return a transient error if chrome hasn't loaded its refresh tokens.

The identity service fails fast if chrome's refresh tokens are not yet
loaded. However, it does not distinguish between a transient failure due
to the refresh tokens not having loaded yet and a persistent failure due
to not having a refresh token for the requested account. Avoid this by
returning transient errors until the refresh tokens are loaded.

Bug: 855002
Change-Id: I4cfa1d52987a3ed9fbec9a8600c87c535b9e0252
Reviewed-on: https://chromium-review.googlesource.com/1113086Reviewed-by: default avatarStuart Langley <slangley@chromium.org>
Commit-Queue: Sam McNally <sammc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570294}
parent d3f7a876
...@@ -259,6 +259,7 @@ class DriveIntegrationService::DriveFsHolder ...@@ -259,6 +259,7 @@ class DriveIntegrationService::DriveFsHolder
void OnMounted(const base::FilePath& path) override; void OnMounted(const base::FilePath& path) override;
std::unique_ptr<drivefs::DriveFsHost::MojoConnectionDelegate> std::unique_ptr<drivefs::DriveFsHost::MojoConnectionDelegate>
CreateMojoConnectionDelegate() override; CreateMojoConnectionDelegate() override;
bool AreRefreshTokensReady() override;
Profile* const profile_; Profile* const profile_;
...@@ -307,6 +308,14 @@ DriveIntegrationService::DriveFsHolder::CreateMojoConnectionDelegate() { ...@@ -307,6 +308,14 @@ DriveIntegrationService::DriveFsHolder::CreateMojoConnectionDelegate() {
return Delegate::CreateMojoConnectionDelegate(); return Delegate::CreateMojoConnectionDelegate();
} }
bool DriveIntegrationService::DriveFsHolder::AreRefreshTokensReady() {
const auto state = ProfileOAuth2TokenServiceFactory::GetForProfile(profile_)
->GetDelegate()
->GetLoadCredentialsState();
return state != OAuth2TokenServiceDelegate::LOAD_CREDENTIALS_IN_PROGRESS &&
state != OAuth2TokenServiceDelegate::LOAD_CREDENTIALS_NOT_STARTED;
}
void DriveIntegrationService::DriveFsHolder::OnMounted( void DriveIntegrationService::DriveFsHolder::OnMounted(
const base::FilePath& path) { const base::FilePath& path) {
on_drivefs_mounted_.Run(); on_drivefs_mounted_.Run();
......
...@@ -165,7 +165,8 @@ class DriveFsHost::MountState : public mojom::DriveFsDelegate, ...@@ -165,7 +165,8 @@ class DriveFsHost::MountState : public mojom::DriveFsDelegate,
const std::vector<std::string>& scopes, const std::vector<std::string>& scopes,
GetAccessTokenCallback callback) override { GetAccessTokenCallback callback) override {
DCHECK_CALLED_ON_VALID_SEQUENCE(host_->sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(host_->sequence_checker_);
if (get_access_token_callback_) { if (get_access_token_callback_ ||
!host_->delegate_->AreRefreshTokensReady()) {
std::move(callback).Run(mojom::AccessTokenStatus::kTransientError, ""); std::move(callback).Run(mojom::AccessTokenStatus::kTransientError, "");
return; return;
} }
......
...@@ -59,6 +59,7 @@ class COMPONENT_EXPORT(DRIVEFS) DriveFsHost ...@@ -59,6 +59,7 @@ class COMPONENT_EXPORT(DRIVEFS) DriveFsHost
virtual net::URLRequestContextGetter* GetRequestContext() = 0; virtual net::URLRequestContextGetter* GetRequestContext() = 0;
virtual service_manager::Connector* GetConnector() = 0; virtual service_manager::Connector* GetConnector() = 0;
virtual const AccountId& GetAccountId() = 0; virtual const AccountId& GetAccountId() = 0;
virtual bool AreRefreshTokensReady() = 0;
virtual std::unique_ptr<OAuth2MintTokenFlow> CreateMintTokenFlow( virtual std::unique_ptr<OAuth2MintTokenFlow> CreateMintTokenFlow(
OAuth2MintTokenFlow::Delegate* delegate, OAuth2MintTokenFlow::Delegate* delegate,
const std::string& client_id, const std::string& client_id,
......
...@@ -109,7 +109,10 @@ class TestingDriveFsHostDelegate : public DriveFsHost::Delegate { ...@@ -109,7 +109,10 @@ class TestingDriveFsHostDelegate : public DriveFsHost::Delegate {
TestingDriveFsHostDelegate( TestingDriveFsHostDelegate(
std::unique_ptr<service_manager::Connector> connector, std::unique_ptr<service_manager::Connector> connector,
const AccountId& account_id) const AccountId& account_id)
: connector_(std::move(connector)), account_id_(account_id) {} : connector_(std::move(connector)), account_id_(account_id) {
ON_CALL(*this, AreRefreshTokensReady())
.WillByDefault(testing::Return(true));
}
MockOAuth2MintTokenFlow& mock_flow() { return mock_flow_; } MockOAuth2MintTokenFlow& mock_flow() { return mock_flow_; }
...@@ -119,6 +122,7 @@ class TestingDriveFsHostDelegate : public DriveFsHost::Delegate { ...@@ -119,6 +122,7 @@ class TestingDriveFsHostDelegate : public DriveFsHost::Delegate {
// DriveFsHost::Delegate: // DriveFsHost::Delegate:
MOCK_METHOD1(OnMounted, void(const base::FilePath&)); MOCK_METHOD1(OnMounted, void(const base::FilePath&));
MOCK_METHOD0(AreRefreshTokensReady, bool());
private: private:
// DriveFsHost::Delegate: // DriveFsHost::Delegate:
...@@ -567,6 +571,29 @@ TEST_F(DriveFsHostTest, GetAccessToken_SequentialRequests) { ...@@ -567,6 +571,29 @@ TEST_F(DriveFsHostTest, GetAccessToken_SequentialRequests) {
} }
} }
TEST_F(DriveFsHostTest, GetAccessToken_RefreshTokensNotReadyYet) {
ASSERT_NO_FATAL_FAILURE(DoMount());
EXPECT_CALL(*host_delegate_, AreRefreshTokensReady())
.WillOnce(testing::Return(false));
EXPECT_CALL(mock_identity_manager_,
GetAccessToken("test@example.com", _, "drivefs"))
.Times(0);
host_delegate_->mock_flow().ExpectNoStartCalls();
base::RunLoop run_loop;
auto quit_closure = run_loop.QuitClosure();
delegate_ptr_->GetAccessToken(
"client ID", "app ID", {"scope1", "scope2"},
base::BindLambdaForTesting(
[&](mojom::AccessTokenStatus status, const std::string& token) {
EXPECT_EQ(mojom::AccessTokenStatus::kTransientError, status);
EXPECT_TRUE(token.empty());
std::move(quit_closure).Run();
}));
run_loop.Run();
}
TEST_F(DriveFsHostTest, GetAccessToken_GetAccessTokenFailure_Permanent) { TEST_F(DriveFsHostTest, GetAccessToken_GetAccessTokenFailure_Permanent) {
ASSERT_NO_FATAL_FAILURE(DoMount()); ASSERT_NO_FATAL_FAILURE(DoMount());
......
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