Commit bcb816c0 authored by Marc Treib's avatar Marc Treib Committed by Commit Bot

Rename "auth_token" to "access_token" in syncer::ServerConnectionManager

It's an (OAuth2) access token, so let's call it what it is.

Bug: 933799
Change-Id: Ie7439bdf707991df913cf56a502f0d92143c4679
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1556798
Commit-Queue: Marc Treib <treib@chromium.org>
Auto-Submit: Marc Treib <treib@chromium.org>
Reviewed-by: default avatarJan Krcal <jkrcal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#648645}
parent ed5f7469
...@@ -19,7 +19,7 @@ LoopbackConnectionManager::~LoopbackConnectionManager() {} ...@@ -19,7 +19,7 @@ LoopbackConnectionManager::~LoopbackConnectionManager() {}
bool LoopbackConnectionManager::PostBufferToPath( bool LoopbackConnectionManager::PostBufferToPath(
PostBufferParams* params, PostBufferParams* params,
const std::string& path, const std::string& path,
const std::string& auth_token) { const std::string& access_token) {
params->response.http_status_code = params->response.http_status_code =
loopback_server_.HandleCommand(params->buffer_in, &params->buffer_out); loopback_server_.HandleCommand(params->buffer_in, &params->buffer_out);
DCHECK_GE(params->response.http_status_code, 0); DCHECK_GE(params->response.http_status_code, 0);
......
...@@ -26,7 +26,7 @@ class LoopbackConnectionManager : public ServerConnectionManager { ...@@ -26,7 +26,7 @@ class LoopbackConnectionManager : public ServerConnectionManager {
// Overridden ServerConnectionManager functions. // Overridden ServerConnectionManager functions.
bool PostBufferToPath(PostBufferParams* params, bool PostBufferToPath(PostBufferParams* params,
const std::string& path, const std::string& path,
const std::string& auth_token) override; const std::string& access_token) override;
// The loopback server that will handle the requests locally. // The loopback server that will handle the requests locally.
LoopbackServer loopback_server_; LoopbackServer loopback_server_;
......
...@@ -152,17 +152,17 @@ ServerConnectionManager::MakeActiveConnection() { ...@@ -152,17 +152,17 @@ ServerConnectionManager::MakeActiveConnection() {
return MakeConnection(); return MakeConnection();
} }
bool ServerConnectionManager::SetAuthToken(const std::string& auth_token) { bool ServerConnectionManager::SetAccessToken(const std::string& access_token) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (!auth_token.empty()) { if (!access_token.empty()) {
auth_token_.assign(auth_token); access_token_.assign(access_token);
return true; return true;
} }
auth_token_.clear(); access_token_.clear();
// The auth token could be non-empty in cases like server outage/bug. E.g. // The access token could be non-empty in cases like server outage/bug. E.g.
// token returned by first request is considered invalid by sync server and // token returned by first request is considered invalid by sync server and
// because of token server's caching policy, etc, same token is returned on // because of token server's caching policy, etc, same token is returned on
// second request. Need to notify sync frontend again to request new token, // second request. Need to notify sync frontend again to request new token,
...@@ -172,8 +172,8 @@ bool ServerConnectionManager::SetAuthToken(const std::string& auth_token) { ...@@ -172,8 +172,8 @@ bool ServerConnectionManager::SetAuthToken(const std::string& auth_token) {
return false; return false;
} }
void ServerConnectionManager::ClearAuthToken() { void ServerConnectionManager::ClearAccessToken() {
auth_token_.clear(); access_token_.clear();
} }
void ServerConnectionManager::SetServerStatus( void ServerConnectionManager::SetServerStatus(
...@@ -199,7 +199,7 @@ bool ServerConnectionManager::PostBufferWithCachedAuth( ...@@ -199,7 +199,7 @@ bool ServerConnectionManager::PostBufferWithCachedAuth(
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
string path = string path =
MakeSyncServerPath(proto_sync_path(), MakeSyncQueryString(client_id_)); MakeSyncServerPath(proto_sync_path(), MakeSyncQueryString(client_id_));
bool result = PostBufferToPath(params, path, auth_token()); bool result = PostBufferToPath(params, path, access_token_);
SetServerStatus(params->response.server_status); SetServerStatus(params->response.server_status);
net_error_code_ = params->response.net_error_code; net_error_code_ = params->response.net_error_code;
return result; return result;
...@@ -207,14 +207,14 @@ bool ServerConnectionManager::PostBufferWithCachedAuth( ...@@ -207,14 +207,14 @@ bool ServerConnectionManager::PostBufferWithCachedAuth(
bool ServerConnectionManager::PostBufferToPath(PostBufferParams* params, bool ServerConnectionManager::PostBufferToPath(PostBufferParams* params,
const string& path, const string& path,
const string& auth_token) { const string& access_token) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (auth_token.empty()) { if (access_token.empty()) {
params->response.server_status = HttpResponse::SYNC_AUTH_ERROR; params->response.server_status = HttpResponse::SYNC_AUTH_ERROR;
// Print a log to distinguish this "known failure" from others. // Print a log to distinguish this "known failure" from others.
DVLOG(1) << "ServerConnectionManager forcing SYNC_AUTH_ERROR due to missing" DVLOG(1) << "ServerConnectionManager forcing SYNC_AUTH_ERROR due to missing"
" auth token"; " access token";
return false; return false;
} }
...@@ -226,11 +226,11 @@ bool ServerConnectionManager::PostBufferToPath(PostBufferParams* params, ...@@ -226,11 +226,11 @@ bool ServerConnectionManager::PostBufferToPath(PostBufferParams* params,
// Note that |post| may be aborted by now, which will just cause Init to fail // Note that |post| may be aborted by now, which will just cause Init to fail
// with CONNECTION_UNAVAILABLE. // with CONNECTION_UNAVAILABLE.
bool ok = connection->Init(path.c_str(), auth_token, params->buffer_in, bool ok = connection->Init(path.c_str(), access_token, params->buffer_in,
&params->response); &params->response);
if (params->response.server_status == HttpResponse::SYNC_AUTH_ERROR) { if (params->response.server_status == HttpResponse::SYNC_AUTH_ERROR) {
auth_token_.clear(); access_token_.clear();
} }
if (!ok || net::HTTP_OK != params->response.http_status_code) if (!ok || net::HTTP_OK != params->response.http_status_code)
......
...@@ -111,7 +111,7 @@ class ServerConnectionManager { ...@@ -111,7 +111,7 @@ class ServerConnectionManager {
// Called to initialize and perform an HTTP POST. // Called to initialize and perform an HTTP POST.
virtual bool Init(const char* path, virtual bool Init(const char* path,
const std::string& auth_token, const std::string& access_token,
const std::string& payload, const std::string& payload,
HttpResponse* response) = 0; HttpResponse* response) = 0;
...@@ -149,7 +149,7 @@ class ServerConnectionManager { ...@@ -149,7 +149,7 @@ class ServerConnectionManager {
virtual ~ServerConnectionManager(); virtual ~ServerConnectionManager();
// POSTS buffer_in and reads a response into buffer_out. Uses our currently // POSTS buffer_in and reads a response into buffer_out. Uses our currently
// set auth token in our headers. // set access token in our headers.
// //
// Returns true if executed successfully. // Returns true if executed successfully.
virtual bool PostBufferWithCachedAuth(PostBufferParams* params); virtual bool PostBufferWithCachedAuth(PostBufferParams* params);
...@@ -179,17 +179,12 @@ class ServerConnectionManager { ...@@ -179,17 +179,12 @@ class ServerConnectionManager {
client_id_.assign(client_id); client_id_.assign(client_id);
} }
// Sets a new auth token. If |auth_token| is empty, the current token is // Sets a new access token. If |access_token| is empty, the current token is
// invalidated and cleared. Returns false if the server is in authentication // invalidated and cleared. Returns false if the server is in authentication
// error state. // error state.
bool SetAuthToken(const std::string& auth_token); bool SetAccessToken(const std::string& access_token);
bool HasInvalidAuthToken() { return auth_token_.empty(); } bool HasInvalidAccessToken() { return access_token_.empty(); }
const std::string auth_token() const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
return auth_token_;
}
protected: protected:
inline std::string proto_sync_path() const { return proto_sync_path_; } inline std::string proto_sync_path() const { return proto_sync_path_; }
...@@ -202,9 +197,9 @@ class ServerConnectionManager { ...@@ -202,9 +197,9 @@ class ServerConnectionManager {
// Internal PostBuffer base function. // Internal PostBuffer base function.
virtual bool PostBufferToPath(PostBufferParams*, virtual bool PostBufferToPath(PostBufferParams*,
const std::string& path, const std::string& path,
const std::string& auth_token); const std::string& access_token);
void ClearAuthToken(); void ClearAccessToken();
// Helper to check terminated flags and build a Connection object. If this // Helper to check terminated flags and build a Connection object. If this
// ServerConnectionManager has been terminated, this will return null. // ServerConnectionManager has been terminated, this will return null.
...@@ -228,8 +223,8 @@ class ServerConnectionManager { ...@@ -228,8 +223,8 @@ class ServerConnectionManager {
// The paths we post to. // The paths we post to.
std::string proto_sync_path_; std::string proto_sync_path_;
// The auth token to use in authenticated requests. // The access token to use in authenticated requests.
std::string auth_token_; std::string access_token_;
base::ObserverList<ServerConnectionEventListener>::Unchecked listeners_; base::ObserverList<ServerConnectionEventListener>::Unchecked listeners_;
......
...@@ -33,7 +33,7 @@ SyncBridgedConnection::~SyncBridgedConnection() { ...@@ -33,7 +33,7 @@ SyncBridgedConnection::~SyncBridgedConnection() {
} }
bool SyncBridgedConnection::Init(const char* path, bool SyncBridgedConnection::Init(const char* path,
const std::string& auth_token, const std::string& access_token,
const std::string& payload, const std::string& payload,
HttpResponse* response) { HttpResponse* response) {
std::string sync_server; std::string sync_server;
...@@ -45,9 +45,9 @@ bool SyncBridgedConnection::Init(const char* path, ...@@ -45,9 +45,9 @@ bool SyncBridgedConnection::Init(const char* path,
HttpPostProviderInterface* http = post_provider_; HttpPostProviderInterface* http = post_provider_;
http->SetURL(connection_url.c_str(), sync_server_port); http->SetURL(connection_url.c_str(), sync_server_port);
if (!auth_token.empty()) { if (!access_token.empty()) {
std::string headers; std::string headers;
headers = "Authorization: Bearer " + auth_token; headers = "Authorization: Bearer " + access_token;
http->SetExtraRequestHeaders(headers.c_str()); http->SetExtraRequestHeaders(headers.c_str());
} }
......
...@@ -30,7 +30,7 @@ class SyncBridgedConnection : public ServerConnectionManager::Connection, ...@@ -30,7 +30,7 @@ class SyncBridgedConnection : public ServerConnectionManager::Connection,
~SyncBridgedConnection() override; ~SyncBridgedConnection() override;
bool Init(const char* path, bool Init(const char* path,
const std::string& auth_token, const std::string& access_token,
const std::string& payload, const std::string& payload,
HttpResponse* response) override; HttpResponse* response) override;
......
...@@ -464,10 +464,6 @@ const SyncScheduler* SyncManagerImpl::scheduler() const { ...@@ -464,10 +464,6 @@ const SyncScheduler* SyncManagerImpl::scheduler() const {
return scheduler_.get(); return scheduler_.get();
} }
bool SyncManagerImpl::GetHasInvalidAuthTokenForTest() const {
return connection_manager_->HasInvalidAuthToken();
}
bool SyncManagerImpl::OpenDirectory(const InitArgs* args) { bool SyncManagerImpl::OpenDirectory(const InitArgs* args) {
DCHECK(!initialized_) << "Should only happen once"; DCHECK(!initialized_) << "Should only happen once";
...@@ -538,7 +534,7 @@ void SyncManagerImpl::UpdateCredentials(const SyncCredentials& credentials) { ...@@ -538,7 +534,7 @@ void SyncManagerImpl::UpdateCredentials(const SyncCredentials& credentials) {
cycle_context_->set_account_name(credentials.email); cycle_context_->set_account_name(credentials.email);
observing_network_connectivity_changes_ = true; observing_network_connectivity_changes_ = true;
if (!connection_manager_->SetAuthToken(credentials.access_token)) if (!connection_manager_->SetAccessToken(credentials.access_token))
return; // Auth token is known to be invalid, so exit early. return; // Auth token is known to be invalid, so exit early.
scheduler_->OnCredentialsUpdated(); scheduler_->OnCredentialsUpdated();
...@@ -548,7 +544,7 @@ void SyncManagerImpl::UpdateCredentials(const SyncCredentials& credentials) { ...@@ -548,7 +544,7 @@ void SyncManagerImpl::UpdateCredentials(const SyncCredentials& credentials) {
void SyncManagerImpl::InvalidateCredentials() { void SyncManagerImpl::InvalidateCredentials() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
connection_manager_->SetAuthToken(std::string()); connection_manager_->SetAccessToken(std::string());
} }
void SyncManagerImpl::AddObserver(SyncManager::Observer* observer) { void SyncManagerImpl::AddObserver(SyncManager::Observer* observer) {
......
...@@ -173,8 +173,6 @@ class SyncManagerImpl ...@@ -173,8 +173,6 @@ class SyncManagerImpl
const SyncScheduler* scheduler() const; const SyncScheduler* scheduler() const;
bool GetHasInvalidAuthTokenForTest() const;
protected: protected:
// Helper functions. Virtual for testing. // Helper functions. Virtual for testing.
virtual void NotifyInitializationSuccess(); virtual void NotifyInitializationSuccess();
......
...@@ -155,7 +155,7 @@ void SyncSchedulerImpl::OnServerConnectionErrorFixed() { ...@@ -155,7 +155,7 @@ void SyncSchedulerImpl::OnServerConnectionErrorFixed() {
// //
// 1. We're in exponential backoff. // 1. We're in exponential backoff.
// 2. We're silenced / throttled. // 2. We're silenced / throttled.
// 3. A nudge was saved previously due to not having a valid auth token. // 3. A nudge was saved previously due to not having a valid access token.
// 4. A nudge was scheduled + saved while in configuration mode. // 4. A nudge was scheduled + saved while in configuration mode.
// //
// In all cases except (2), we want to retry contacting the server. We // In all cases except (2), we want to retry contacting the server. We
...@@ -288,8 +288,8 @@ bool SyncSchedulerImpl::CanRunJobNow(JobPriority priority) { ...@@ -288,8 +288,8 @@ bool SyncSchedulerImpl::CanRunJobNow(JobPriority priority) {
} }
if (!ignore_auth_credentials_ && if (!ignore_auth_credentials_ &&
cycle_context_->connection_manager()->HasInvalidAuthToken()) { cycle_context_->connection_manager()->HasInvalidAccessToken()) {
SDVLOG(1) << "Unable to run a job because we have no valid auth token."; SDVLOG(1) << "Unable to run a job because we have no valid access token.";
return false; return false;
} }
...@@ -689,7 +689,7 @@ void SyncSchedulerImpl::TrySyncCycleJobImpl() { ...@@ -689,7 +689,7 @@ void SyncSchedulerImpl::TrySyncCycleJobImpl() {
// We must be in an error state. Transitioning out of each of these // We must be in an error state. Transitioning out of each of these
// error states should trigger a canary job. // error states should trigger a canary job.
DCHECK(IsGlobalThrottle() || IsGlobalBackoff() || DCHECK(IsGlobalThrottle() || IsGlobalBackoff() ||
cycle_context_->connection_manager()->HasInvalidAuthToken()); cycle_context_->connection_manager()->HasInvalidAccessToken());
} }
RestartWaiting(); RestartWaiting();
......
...@@ -514,12 +514,12 @@ TEST_F(SyncSchedulerImplTest, ConfigWithStop) { ...@@ -514,12 +514,12 @@ TEST_F(SyncSchedulerImplTest, ConfigWithStop) {
ASSERT_EQ(0, ready_counter.times_called()); ASSERT_EQ(0, ready_counter.times_called());
} }
// Verify that in the absence of valid auth token the command will fail. // Verify that in the absence of valid access token the command will fail.
TEST_F(SyncSchedulerImplTest, ConfigNoAuthToken) { TEST_F(SyncSchedulerImplTest, ConfigNoAccessToken) {
SyncShareTimes times; SyncShareTimes times;
const ModelTypeSet model_types(THEMES); const ModelTypeSet model_types(THEMES);
connection()->ResetAuthToken(); connection()->ResetAccessToken();
StartSyncConfiguration(); StartSyncConfiguration();
...@@ -532,14 +532,14 @@ TEST_F(SyncSchedulerImplTest, ConfigNoAuthToken) { ...@@ -532,14 +532,14 @@ TEST_F(SyncSchedulerImplTest, ConfigNoAuthToken) {
ASSERT_EQ(0, ready_counter.times_called()); ASSERT_EQ(0, ready_counter.times_called());
} }
// Verify that in the absence of valid auth token the command will pass if local // Verify that in the absence of valid access token the command will pass if
// sync backend is used. // local sync backend is used.
TEST_F(SyncSchedulerImplTest, ConfigNoAuthTokenLocalSync) { TEST_F(SyncSchedulerImplTest, ConfigNoAccessTokenLocalSync) {
SyncShareTimes times; SyncShareTimes times;
const ModelTypeSet model_types(THEMES); const ModelTypeSet model_types(THEMES);
NewSchedulerForLocalBackend(); NewSchedulerForLocalBackend();
connection()->ResetAuthToken(); connection()->ResetAccessToken();
EXPECT_CALL(*syncer(), ConfigureSyncShare(_, _, _)) EXPECT_CALL(*syncer(), ConfigureSyncShare(_, _, _))
.WillOnce(DoAll(Invoke(test_util::SimulateConfigureSuccess), .WillOnce(DoAll(Invoke(test_util::SimulateConfigureSuccess),
......
...@@ -27,7 +27,7 @@ using sync_pb::SyncEnums; ...@@ -27,7 +27,7 @@ using sync_pb::SyncEnums;
namespace syncer { namespace syncer {
static char kValidAuthToken[] = "AuthToken"; static char kValidAccessToken[] = "AccessToken";
static char kCacheGuid[] = "kqyg7097kro6GSUod+GSg=="; static char kCacheGuid[] = "kqyg7097kro6GSUod+GSg==";
MockConnectionManager::MockConnectionManager(syncable::Directory* directory, MockConnectionManager::MockConnectionManager(syncable::Directory* directory,
...@@ -49,7 +49,7 @@ MockConnectionManager::MockConnectionManager(syncable::Directory* directory, ...@@ -49,7 +49,7 @@ MockConnectionManager::MockConnectionManager(syncable::Directory* directory,
next_position_in_parent_(2), next_position_in_parent_(2),
num_get_updates_requests_(0) { num_get_updates_requests_(0) {
SetNewTimestamp(0); SetNewTimestamp(0);
SetAuthToken(kValidAuthToken); SetAccessToken(kValidAccessToken);
} }
MockConnectionManager::~MockConnectionManager() { MockConnectionManager::~MockConnectionManager() {
...@@ -72,7 +72,7 @@ void MockConnectionManager::SetMidCommitObserver( ...@@ -72,7 +72,7 @@ void MockConnectionManager::SetMidCommitObserver(
bool MockConnectionManager::PostBufferToPath(PostBufferParams* params, bool MockConnectionManager::PostBufferToPath(PostBufferParams* params,
const string& path, const string& path,
const string& auth_token) { const string& access_token) {
ClientToServerMessage post; ClientToServerMessage post;
if (!post.ParseFromString(params->buffer_in)) { if (!post.ParseFromString(params->buffer_in)) {
ADD_FAILURE(); ADD_FAILURE();
...@@ -108,15 +108,15 @@ bool MockConnectionManager::PostBufferToPath(PostBufferParams* params, ...@@ -108,15 +108,15 @@ bool MockConnectionManager::PostBufferToPath(PostBufferParams* params,
syncable::WriteTransaction wt(FROM_HERE, syncable::UNITTEST, directory_); syncable::WriteTransaction wt(FROM_HERE, syncable::UNITTEST, directory_);
} }
if (auth_token.empty()) { if (access_token.empty()) {
params->response.server_status = HttpResponse::SYNC_AUTH_ERROR; params->response.server_status = HttpResponse::SYNC_AUTH_ERROR;
return false; return false;
} }
if (auth_token != kValidAuthToken) { if (access_token != kValidAccessToken) {
// Simulate server-side auth failure. // Simulate server-side auth failure.
params->response.server_status = HttpResponse::SYNC_AUTH_ERROR; params->response.server_status = HttpResponse::SYNC_AUTH_ERROR;
ClearAuthToken(); ClearAccessToken();
} }
if (--countdown_to_postbuffer_fail_ == 0) { if (--countdown_to_postbuffer_fail_ == 0) {
......
...@@ -45,7 +45,7 @@ class MockConnectionManager : public ServerConnectionManager { ...@@ -45,7 +45,7 @@ class MockConnectionManager : public ServerConnectionManager {
// Overridden ServerConnectionManager functions. // Overridden ServerConnectionManager functions.
bool PostBufferToPath(PostBufferParams*, bool PostBufferToPath(PostBufferParams*,
const std::string& path, const std::string& path,
const std::string& auth_token) override; const std::string& access_token) override;
// Control of commit response. // Control of commit response.
// NOTE: Commit callback is invoked only once then reset. // NOTE: Commit callback is invoked only once then reset.
...@@ -268,7 +268,7 @@ class MockConnectionManager : public ServerConnectionManager { ...@@ -268,7 +268,7 @@ class MockConnectionManager : public ServerConnectionManager {
// Adds a new progress marker to the last update. // Adds a new progress marker to the last update.
sync_pb::DataTypeProgressMarker* AddUpdateProgressMarker(); sync_pb::DataTypeProgressMarker* AddUpdateProgressMarker();
void ResetAuthToken() { ClearAuthToken(); } void ResetAccessToken() { ClearAccessToken(); }
private: private:
sync_pb::SyncEntity* AddUpdateFull(syncable::Id id, sync_pb::SyncEntity* AddUpdateFull(syncable::Id id,
...@@ -375,7 +375,7 @@ class MockConnectionManager : public ServerConnectionManager { ...@@ -375,7 +375,7 @@ class MockConnectionManager : public ServerConnectionManager {
// The AUTHENTICATE response we'll return for auth requests. // The AUTHENTICATE response we'll return for auth requests.
sync_pb::AuthenticateResponse auth_response_; sync_pb::AuthenticateResponse auth_response_;
// What we use to determine if we should return SUCCESS or BAD_AUTH_TOKEN. // What we use to determine if we should return SUCCESS or BAD_AUTH_TOKEN.
std::string valid_auth_token_; std::string valid_access_token_;
// Whether we are faking a server mandating clients to throttle requests. // Whether we are faking a server mandating clients to throttle requests.
// Protected by |response_code_override_lock_|. // Protected by |response_code_override_lock_|.
......
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