This broke M20 build.

Revert 137824 - Properly handle accounts that don't have GMail account.

1. Me2MeHostAuthenticatorFactory now verifies that the bare 
JID of the remote client matches bare JID of the host. 
Previously it was comparing it with the user's email, 
which may be different from JID.
2. GaiaOAuthClient now fetches user's email.
3. SignalingConnector verifies that user's email matches 
the expected value stored in xmpp_login. If it doesn't, 
then the auth token was generated for a different account 
and the host treats it as an authentication error.

BUG=128102

Review URL: https://chromiumcodereview.appspot.com/10332187

TBR=sergeyu@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10388226

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@138204 0039d316-1c4b-4281-b951-d872f2087c98

15 files changed, 63 insertions, 129 deletions

diff --git a/remoting/host/gaia_oauth_client.cc b/remoting/host/gaia_oauth_client.cc
index aef0ad2..d9d049f 100644
--- a/remoting/host/gaia_oauth_client.cc
+++ b/remoting/host/gaia_oauth_client.cc
@@ -17,38 +17,20 @@
 #include "remoting/host/url_fetcher.h"
 
 namespace {
-
-const char kDefaultOAuth2TokenUrl[] =
-    "https://accounts.google.com/o/oauth2/token";
-const char kDefaultOAuth2UserInfoUrl[] =
-    "https://www.googleapis.com/oauth2/v1/userinfo";
-
-// Values used to parse token response.
 const char kAccessTokenValue[] = "access_token";
 const char kRefreshTokenValue[] = "refresh_token";
 const char kExpiresInValue[] = "expires_in";
-
-// Values used when parsing userinfo response.
-const char kEmailValue[] = "email";
-
 }  // namespace
 
 namespace remoting {
 
-// static
-OAuthProviderInfo OAuthProviderInfo::GetDefault() {
-  OAuthProviderInfo result;
-  result.access_token_url = kDefaultOAuth2TokenUrl;
-  result.user_info_url = kDefaultOAuth2UserInfoUrl;
-  return result;
-}
-
 class GaiaOAuthClient::Core
     : public base::RefCountedThreadSafe<GaiaOAuthClient::Core> {
  public:
-  Core(const OAuthProviderInfo& info,
+  Core(const std::string& gaia_url,
        net::URLRequestContextGetter* request_context_getter)
-      : request_context_getter_(request_context_getter),
+      : gaia_url_(gaia_url),
+        request_context_getter_(request_context_getter),
         delegate_(NULL) {
   }
 
@@ -60,22 +42,14 @@ class GaiaOAuthClient::Core
   friend class base::RefCountedThreadSafe<Core>;
   virtual ~Core() {}
 
-  void OnAuthTokenFetchComplete(const net::URLRequestStatus& status,
-                                int response_code,
-                                const std::string& response);
-  void FetchUserInfoAndInvokeCallback();
-  void OnUserInfoFetchComplete(const net::URLRequestStatus& status,
-                               int response_code,
-                               const std::string& response);
-
-  OAuthProviderInfo provider_info_;
+  void OnUrlFetchComplete(const net::URLRequestStatus& status,
+                          int response_code,
+                          const std::string& response);
 
+  GURL gaia_url_;
   scoped_refptr<net::URLRequestContextGetter> request_context_getter_;
   GaiaOAuthClient::Delegate* delegate_;
   scoped_ptr<UrlFetcher> request_;
-
-  std::string access_token_;
-  int expires_in_seconds_;
 };
 
 void GaiaOAuthClient::Core::RefreshToken(
@@ -84,11 +58,6 @@ void GaiaOAuthClient::Core::RefreshToken(
     GaiaOAuthClient::Delegate* delegate) {
   DCHECK(!request_.get()) << "Tried to fetch two things at once!";
 
-  delegate_ = delegate;
-
-  access_token_.clear();
-  expires_in_seconds_ = 0;
-
   std::string post_body =
       "refresh_token=" + net::EscapeUrlEncodedData(refresh_token, true) +
       "&client_id=" + net::EscapeUrlEncodedData(oauth_client_info.client_id,
@@ -96,15 +65,14 @@ void GaiaOAuthClient::Core::RefreshToken(
       "&client_secret=" +
       net::EscapeUrlEncodedData(oauth_client_info.client_secret, true) +
       "&grant_type=refresh_token";
-  request_.reset(new UrlFetcher(GURL(provider_info_.access_token_url),
-                                UrlFetcher::POST));
+  delegate_ = delegate;
+  request_.reset(new UrlFetcher(gaia_url_, UrlFetcher::POST));
   request_->SetRequestContext(request_context_getter_);
   request_->SetUploadData("application/x-www-form-urlencoded", post_body);
-  request_->Start(
-      base::Bind(&GaiaOAuthClient::Core::OnAuthTokenFetchComplete, this));
+  request_->Start(base::Bind(&GaiaOAuthClient::Core::OnUrlFetchComplete, this));
 }
 
-void GaiaOAuthClient::Core::OnAuthTokenFetchComplete(
+void GaiaOAuthClient::Core::OnUrlFetchComplete(
     const net::URLRequestStatus& status,
     int response_code,
     const std::string& response) {
@@ -122,63 +90,37 @@ void GaiaOAuthClient::Core::OnAuthTokenFetchComplete(
     return;
   }
 
+  std::string access_token;
+  std::string refresh_token;
+  int expires_in_seconds = 0;
   if (response_code == net::HTTP_OK) {
     scoped_ptr<Value> message_value(base::JSONReader::Read(response));
     if (message_value.get() &&
         message_value->IsType(Value::TYPE_DICTIONARY)) {
       scoped_ptr<DictionaryValue> response_dict(
           static_cast<DictionaryValue*>(message_value.release()));
-      response_dict->GetString(kAccessTokenValue, &access_token_);
-      response_dict->GetInteger(kExpiresInValue, &expires_in_seconds_);
+      response_dict->GetString(kAccessTokenValue, &access_token);
+      response_dict->GetString(kRefreshTokenValue, &refresh_token);
+      response_dict->GetInteger(kExpiresInValue, &expires_in_seconds);
     }
-    VLOG(1) << "Gaia response: acess_token='" << access_token_
-            << "', expires in " << expires_in_seconds_ << " second(s)";
+    VLOG(1) << "Gaia response: acess_token='" << access_token
+            << "', refresh_token='" << refresh_token
+            << "', expires in " << expires_in_seconds << " second(s)";
   } else {
     LOG(ERROR) << "Gaia response: response code=" << response_code;
   }
 
-  if (access_token_.empty()) {
+  if (access_token.empty()) {
     delegate_->OnNetworkError(response_code);
-  } else {
-    FetchUserInfoAndInvokeCallback();
-  }
-}
-
-void GaiaOAuthClient::Core::FetchUserInfoAndInvokeCallback() {
-  request_.reset(new UrlFetcher(
-      GURL(provider_info_.user_info_url), UrlFetcher::GET));
-  request_->SetRequestContext(request_context_getter_);
-  request_->SetHeader("Authorization", "Bearer " + access_token_);
-  request_->Start(
-      base::Bind(&GaiaOAuthClient::Core::OnUserInfoFetchComplete, this));
-}
-
-void GaiaOAuthClient::Core::OnUserInfoFetchComplete(
-    const net::URLRequestStatus& status,
-    int response_code,
-    const std::string& response) {
-  std::string email;
-  if (response_code == net::HTTP_OK) {
-    scoped_ptr<Value> message_value(base::JSONReader::Read(response));
-    if (message_value.get() &&
-        message_value->IsType(Value::TYPE_DICTIONARY)) {
-      scoped_ptr<DictionaryValue> response_dict(
-          static_cast<DictionaryValue*>(message_value.release()));
-      response_dict->GetString(kEmailValue, &email);
-    }
-  }
-
-  if (email.empty()) {
-    delegate_->OnNetworkError(response_code);
-  } else {
-    delegate_->OnRefreshTokenResponse(
-        email, access_token_, expires_in_seconds_);
+  } else if (refresh_token.empty()) {
+    // If we only have an access token, then this was a refresh request.
+    delegate_->OnRefreshTokenResponse(access_token, expires_in_seconds);
   }
 }
 
-GaiaOAuthClient::GaiaOAuthClient(const OAuthProviderInfo& provider_info,
+GaiaOAuthClient::GaiaOAuthClient(const std::string& gaia_url,
                                  net::URLRequestContextGetter* context_getter) {
-  core_ = new Core(provider_info, context_getter);
+  core_ = new Core(gaia_url, context_getter);
 }
 
 GaiaOAuthClient::~GaiaOAuthClient() {
diff --git a/remoting/host/gaia_oauth_client.h b/remoting/host/gaia_oauth_client.h
index 9da6911..58178dc 100644
--- a/remoting/host/gaia_oauth_client.h
+++ b/remoting/host/gaia_oauth_client.h
@@ -21,18 +21,15 @@ class URLRequestContextGetter;
 // this duplication.
 namespace remoting {
 
+// TODO(jamiewalch): Make this configurable if we ever support other providers.
+static const char kGaiaOAuth2Url[] =
+    "https://accounts.google.com/o/oauth2/token";
+
 struct OAuthClientInfo {
   std::string client_id;
   std::string client_secret;
 };
 
-struct OAuthProviderInfo {
-  static OAuthProviderInfo GetDefault();
-
-  std::string access_token_url;
-  std::string user_info_url;
-};
-
 class GaiaOAuthClient {
  public:
   class Delegate {
@@ -40,8 +37,7 @@ class GaiaOAuthClient {
     virtual ~Delegate() { }
 
     // Invoked on a successful response to the RefreshToken request.
-    virtual void OnRefreshTokenResponse(const std::string& user_email,
-                                        const std::string& access_token,
+    virtual void OnRefreshTokenResponse(const std::string& access_token,
                                         int expires_in_seconds) = 0;
     // Invoked when there is an OAuth error with one of the requests.
     virtual void OnOAuthError() = 0;
@@ -49,8 +45,7 @@ class GaiaOAuthClient {
     // invalid response.
     virtual void OnNetworkError(int response_code) = 0;
   };
-
-  GaiaOAuthClient(const OAuthProviderInfo& provider_info,
+  GaiaOAuthClient(const std::string& gaia_url,
                   net::URLRequestContextGetter* context_getter);
   ~GaiaOAuthClient();
 
diff --git a/remoting/host/remoting_me2me_host.cc b/remoting/host/remoting_me2me_host.cc
index 806701d..0e06970 100644
--- a/remoting/host/remoting_me2me_host.cc
+++ b/remoting/host/remoting_me2me_host.cc
@@ -190,7 +190,7 @@ class HostProcess
   void CreateAuthenticatorFactory() {
     scoped_ptr<protocol::AuthenticatorFactory> factory(
         new protocol::Me2MeHostAuthenticatorFactory(
-            key_pair_.GenerateCertificate(),
+            xmpp_login_, key_pair_.GenerateCertificate(),
             *key_pair_.private_key(), host_secret_hash_));
     host_->SetAuthenticatorFactory(factory.Pass());
   }
diff --git a/remoting/host/signaling_connector.cc b/remoting/host/signaling_connector.cc
index a9bef15..072eced 100644
--- a/remoting/host/signaling_connector.cc
+++ b/remoting/host/signaling_connector.cc
@@ -55,8 +55,7 @@ void SignalingConnector::EnableOAuth(
     scoped_ptr<OAuthCredentials> oauth_credentials,
     net::URLRequestContextGetter* url_context) {
   oauth_credentials_ = oauth_credentials.Pass();
-  gaia_oauth_client_.reset(new GaiaOAuthClient(
-      OAuthProviderInfo::GetDefault(), url_context));
+  gaia_oauth_client_.reset(new GaiaOAuthClient(kGaiaOAuth2Url, url_context));
 }
 
 void SignalingConnector::OnSignalStrategyStateChange(
@@ -94,20 +93,11 @@ void SignalingConnector::OnOnlineStateChanged(bool online) {
   }
 }
 
-void SignalingConnector::OnRefreshTokenResponse(const std::string& user_email,
-                                                const std::string& access_token,
+void SignalingConnector::OnRefreshTokenResponse(const std::string& access_token,
                                                 int expires_seconds) {
   DCHECK(CalledOnValidThread());
   DCHECK(oauth_credentials_.get());
   LOG(INFO) << "Received OAuth token.";
-
-  if (user_email != oauth_credentials_->login) {
-    LOG(ERROR) << "OAuth token and email address do not refer to "
-        "the same account.";
-    auth_failed_callback_.Run();
-    return;
-  }
-
   refreshing_oauth_token_ = false;
   auth_token_expiry_time_ = base::Time::Now() +
       base::TimeDelta::FromSeconds(expires_seconds) -
diff --git a/remoting/host/signaling_connector.h b/remoting/host/signaling_connector.h
index 4b432f3..a03ae64 100644
--- a/remoting/host/signaling_connector.h
+++ b/remoting/host/signaling_connector.h
@@ -71,8 +71,7 @@ class SignalingConnector
   virtual void OnOnlineStateChanged(bool online) OVERRIDE;
 
   // GaiaOAuthClient::Delegate interface.
-  virtual void OnRefreshTokenResponse(const std::string& user_email,
-                                      const std::string& access_token,
+  virtual void OnRefreshTokenResponse(const std::string& access_token,
                                       int expires_seconds) OVERRIDE;
   virtual void OnOAuthError() OVERRIDE;
   virtual void OnNetworkError(int response_code) OVERRIDE;
diff --git a/remoting/host/simple_host_process.cc b/remoting/host/simple_host_process.cc
index 3e2bc2a..bb87ee4 100644
--- a/remoting/host/simple_host_process.cc
+++ b/remoting/host/simple_host_process.cc
@@ -266,8 +266,8 @@ class SimpleHost : public HeartbeatSender::Listener {
     if (!is_it2me_) {
       scoped_ptr<protocol::AuthenticatorFactory> factory(
           new protocol::Me2MeHostAuthenticatorFactory(
-              key_pair_.GenerateCertificate(), *key_pair_.private_key(),
-              host_secret_hash_));
+              xmpp_login_, key_pair_.GenerateCertificate(),
+              *key_pair_.private_key(), host_secret_hash_));
       host_->SetAuthenticatorFactory(factory.Pass());
     }
   }
diff --git a/remoting/jingle_glue/xmpp_signal_strategy.cc b/remoting/jingle_glue/xmpp_signal_strategy.cc
index 5cac0fc..6ae4f1a 100644
--- a/remoting/jingle_glue/xmpp_signal_strategy.cc
+++ b/remoting/jingle_glue/xmpp_signal_strategy.cc
@@ -167,6 +167,24 @@ void XmppSignalStrategy::OnConnectionStateChanged(
   DCHECK(CalledOnValidThread());
 
   if (state == buzz::XmppEngine::STATE_OPEN) {
+    // Verify that the JID that we've received matches the username
+    // that we have. If it doesn't, then the OAuth token was probably
+    // issued for a different account, so we treat is a an auth error.
+    //
+    // TODO(sergeyu): Some user accounts may not have associated
+    // e-mail address. The check below will fail for such
+    // accounts. Make sure we can handle this case proprely.
+    if (!StartsWithASCII(GetLocalJid(), username_, false)) {
+      LOG(ERROR) << "Received JID that is different from the expected value.";
+      error_ = AUTHENTICATION_FAILED;
+      xmpp_client_->SignalStateChange.disconnect(this);
+      MessageLoop::current()->PostTask(
+          FROM_HERE, base::Bind(&DisconnectXmppClient, xmpp_client_));
+      xmpp_client_ = NULL;
+      SetState(DISCONNECTED);
+      return;
+    }
+
     keep_alive_timer_.Start(
         FROM_HERE, base::TimeDelta::FromSeconds(kKeepAliveIntervalSeconds),
         this, &XmppSignalStrategy::SendKeepAlive);
diff --git a/remoting/protocol/authenticator.h b/remoting/protocol/authenticator.h
index b8aa37d..4ed7fa5 100644
--- a/remoting/protocol/authenticator.h
+++ b/remoting/protocol/authenticator.h
@@ -110,7 +110,6 @@ class AuthenticatorFactory {
   // rejected. ProcessMessage() should be called with |first_message|
   // for the result of this method.
   virtual scoped_ptr<Authenticator> CreateAuthenticator(
-      const std::string& local_jid,
       const std::string& remote_jid,
       const buzz::XmlElement* first_message) = 0;
 };
diff --git a/remoting/protocol/fake_authenticator.cc b/remoting/protocol/fake_authenticator.cc
index 681f2a9..52c78c0 100644
--- a/remoting/protocol/fake_authenticator.cc
+++ b/remoting/protocol/fake_authenticator.cc
@@ -156,7 +156,6 @@ FakeHostAuthenticatorFactory::~FakeHostAuthenticatorFactory() {
 }
 
 scoped_ptr<Authenticator> FakeHostAuthenticatorFactory::CreateAuthenticator(
-    const std::string& local_jid,
     const std::string& remote_jid,
     const buzz::XmlElement* first_message) {
   return scoped_ptr<Authenticator>(new FakeAuthenticator(
diff --git a/remoting/protocol/fake_authenticator.h b/remoting/protocol/fake_authenticator.h
index a6ab85e..2e60758 100644
--- a/remoting/protocol/fake_authenticator.h
+++ b/remoting/protocol/fake_authenticator.h
@@ -89,7 +89,6 @@ class FakeHostAuthenticatorFactory : public AuthenticatorFactory {
 
   // AuthenticatorFactory interface.
   virtual scoped_ptr<Authenticator> CreateAuthenticator(
-      const std::string& local_jid,
       const std::string& remote_jid,
       const buzz::XmlElement* first_message) OVERRIDE;
 
diff --git a/remoting/protocol/it2me_host_authenticator_factory.cc b/remoting/protocol/it2me_host_authenticator_factory.cc
index 19853ce..9599556 100644
--- a/remoting/protocol/it2me_host_authenticator_factory.cc
+++ b/remoting/protocol/it2me_host_authenticator_factory.cc
@@ -25,7 +25,6 @@ It2MeHostAuthenticatorFactory::~It2MeHostAuthenticatorFactory() {
 }
 
 scoped_ptr<Authenticator> It2MeHostAuthenticatorFactory::CreateAuthenticator(
-    const std::string& local_jid,
     const std::string& remote_jid,
     const buzz::XmlElement* first_message) {
   if (NegotiatingAuthenticator::IsNegotiableMessage(first_message)) {
diff --git a/remoting/protocol/it2me_host_authenticator_factory.h b/remoting/protocol/it2me_host_authenticator_factory.h
index c526cbf..aa04009 100644
--- a/remoting/protocol/it2me_host_authenticator_factory.h
+++ b/remoting/protocol/it2me_host_authenticator_factory.h
@@ -31,7 +31,6 @@ class It2MeHostAuthenticatorFactory : public AuthenticatorFactory {
 
   // AuthenticatorFactory interface.
   virtual scoped_ptr<Authenticator> CreateAuthenticator(
-      const std::string& local_jid,
       const std::string& remote_jid,
       const buzz::XmlElement* first_message) OVERRIDE;
 
diff --git a/remoting/protocol/jingle_session_manager.cc b/remoting/protocol/jingle_session_manager.cc
index 5f48518..f3231af 100644
--- a/remoting/protocol/jingle_session_manager.cc
+++ b/remoting/protocol/jingle_session_manager.cc
@@ -141,8 +141,7 @@ bool JingleSessionManager::OnSignalStrategyIncomingStanza(
 
     scoped_ptr<Authenticator> authenticator =
         authenticator_factory_->CreateAuthenticator(
-            signal_strategy_->GetLocalJid(), message.from,
-            message.description->authenticator_message());
+            message.from, message.description->authenticator_message());
 
     JingleSession* session = new JingleSession(this);
     session->InitializeIncomingConnection(message, authenticator.Pass());
diff --git a/remoting/protocol/me2me_host_authenticator_factory.cc b/remoting/protocol/me2me_host_authenticator_factory.cc
index ba46c4d..776c2d1 100644
--- a/remoting/protocol/me2me_host_authenticator_factory.cc
+++ b/remoting/protocol/me2me_host_authenticator_factory.cc
@@ -58,34 +58,30 @@ class RejectingAuthenticator : public Authenticator {
 }  // namespace
 
 Me2MeHostAuthenticatorFactory::Me2MeHostAuthenticatorFactory(
+    const std::string& local_jid,
     const std::string& local_cert,
     const crypto::RSAPrivateKey& local_private_key,
     const SharedSecretHash& shared_secret_hash)
     : local_cert_(local_cert),
       local_private_key_(local_private_key.Copy()),
       shared_secret_hash_(shared_secret_hash) {
+  // Verify that |local_jid| is bare.
+  DCHECK_EQ(local_jid.find('/'), std::string::npos);
+  local_jid_prefix_ = local_jid + '/';
 }
 
 Me2MeHostAuthenticatorFactory::~Me2MeHostAuthenticatorFactory() {
 }
 
 scoped_ptr<Authenticator> Me2MeHostAuthenticatorFactory::CreateAuthenticator(
-    const std::string& local_jid,
     const std::string& remote_jid,
     const buzz::XmlElement* first_message) {
-
-  size_t slash_pos = local_jid.find('/');
-  if (slash_pos == std::string::npos) {
-    LOG(DFATAL) << "Invalid local JID:" << local_jid;
-    return scoped_ptr<Authenticator>(new RejectingAuthenticator());
-  }
-
   // Verify that the client's jid is an ASCII string, and then check
   // that the client has the same bare jid as the host, i.e. client's
   // full JID starts with host's bare jid. Comparison is case
   // insensitive.
   if (!IsStringASCII(remote_jid) ||
-      !StartsWithASCII(remote_jid, local_jid.substr(0, slash_pos + 1), false)) {
+      !StartsWithASCII(remote_jid, local_jid_prefix_, false)) {
     LOG(ERROR) << "Rejecting incoming connection from " << remote_jid;
     return scoped_ptr<Authenticator>(new RejectingAuthenticator());
   }
diff --git a/remoting/protocol/me2me_host_authenticator_factory.h b/remoting/protocol/me2me_host_authenticator_factory.h
index 1a5acb1..b0f9b4e 100644
--- a/remoting/protocol/me2me_host_authenticator_factory.h
+++ b/remoting/protocol/me2me_host_authenticator_factory.h
@@ -24,6 +24,7 @@ class Me2MeHostAuthenticatorFactory : public AuthenticatorFactory {
  public:
   // Doesn't take ownership of |local_private_key|.
   Me2MeHostAuthenticatorFactory(
+      const std::string& local_jid,
       const std::string& local_cert,
       const crypto::RSAPrivateKey& local_private_key,
       const SharedSecretHash& shared_secret_hash);
@@ -31,7 +32,6 @@ class Me2MeHostAuthenticatorFactory : public AuthenticatorFactory {
 
   // AuthenticatorFactory interface.
   virtual scoped_ptr<Authenticator> CreateAuthenticator(
-      const std::string& local_jid,
       const std::string& remote_jid,
       const buzz::XmlElement* first_message) OVERRIDE;