Reland 137824 - was reverted by mistake

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=dharani@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10332285

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

15 files changed, 129 insertions, 63 deletions

diff --git a/remoting/host/gaia_oauth_client.cc b/remoting/host/gaia_oauth_client.cc
index d9d049f..aef0ad2 100644
--- a/remoting/host/gaia_oauth_client.cc
+++ b/remoting/host/gaia_oauth_client.cc
@@ -17,20 +17,38 @@
 #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 std::string& gaia_url,
+  Core(const OAuthProviderInfo& info,
        net::URLRequestContextGetter* request_context_getter)
-      : gaia_url_(gaia_url),
-        request_context_getter_(request_context_getter),
+      : request_context_getter_(request_context_getter),
         delegate_(NULL) {
   }
 
@@ -42,14 +60,22 @@ class GaiaOAuthClient::Core
   friend class base::RefCountedThreadSafe<Core>;
   virtual ~Core() {}
 
-  void OnUrlFetchComplete(const net::URLRequestStatus& status,
-                          int response_code,
-                          const std::string& response);
+  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_;
 
-  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(
@@ -58,6 +84,11 @@ 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,
@@ -65,14 +96,15 @@ void GaiaOAuthClient::Core::RefreshToken(
       "&client_secret=" +
       net::EscapeUrlEncodedData(oauth_client_info.client_secret, true) +
       "&grant_type=refresh_token";
-  delegate_ = delegate;
-  request_.reset(new UrlFetcher(gaia_url_, UrlFetcher::POST));
+  request_.reset(new UrlFetcher(GURL(provider_info_.access_token_url),
+                                UrlFetcher::POST));
   request_->SetRequestContext(request_context_getter_);
   request_->SetUploadData("application/x-www-form-urlencoded", post_body);
-  request_->Start(base::Bind(&GaiaOAuthClient::Core::OnUrlFetchComplete, this));
+  request_->Start(
+      base::Bind(&GaiaOAuthClient::Core::OnAuthTokenFetchComplete, this));
 }
 
-void GaiaOAuthClient::Core::OnUrlFetchComplete(
+void GaiaOAuthClient::Core::OnAuthTokenFetchComplete(
     const net::URLRequestStatus& status,
     int response_code,
     const std::string& response) {
@@ -90,37 +122,63 @@ void GaiaOAuthClient::Core::OnUrlFetchComplete(
     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->GetString(kRefreshTokenValue, &refresh_token);
-      response_dict->GetInteger(kExpiresInValue, &expires_in_seconds);
+      response_dict->GetString(kAccessTokenValue, &access_token_);
+      response_dict->GetInteger(kExpiresInValue, &expires_in_seconds_);
     }
-    VLOG(1) << "Gaia response: acess_token='" << access_token
-            << "', refresh_token='" << refresh_token
-            << "', expires in " << expires_in_seconds << " second(s)";
+    VLOG(1) << "Gaia response: acess_token='" << access_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 if (refresh_token.empty()) {
-    // If we only have an access token, then this was a refresh request.
-    delegate_->OnRefreshTokenResponse(access_token, expires_in_seconds);
+  } 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_);
   }
 }
 
-GaiaOAuthClient::GaiaOAuthClient(const std::string& gaia_url,
+GaiaOAuthClient::GaiaOAuthClient(const OAuthProviderInfo& provider_info,
                                  net::URLRequestContextGetter* context_getter) {
-  core_ = new Core(gaia_url, context_getter);
+  core_ = new Core(provider_info, context_getter);
 }
 
 GaiaOAuthClient::~GaiaOAuthClient() {
diff --git a/remoting/host/gaia_oauth_client.h b/remoting/host/gaia_oauth_client.h
index 58178dc..9da6911 100644
--- a/remoting/host/gaia_oauth_client.h
+++ b/remoting/host/gaia_oauth_client.h
@@ -21,15 +21,18 @@ 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 {
@@ -37,7 +40,8 @@ class GaiaOAuthClient {
     virtual ~Delegate() { }
 
     // Invoked on a successful response to the RefreshToken request.
-    virtual void OnRefreshTokenResponse(const std::string& access_token,
+    virtual void OnRefreshTokenResponse(const std::string& user_email,
+                                        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;
@@ -45,7 +49,8 @@ class GaiaOAuthClient {
     // invalid response.
     virtual void OnNetworkError(int response_code) = 0;
   };
-  GaiaOAuthClient(const std::string& gaia_url,
+
+  GaiaOAuthClient(const OAuthProviderInfo& provider_info,
                   net::URLRequestContextGetter* context_getter);
   ~GaiaOAuthClient();
 
diff --git a/remoting/host/remoting_me2me_host.cc b/remoting/host/remoting_me2me_host.cc
index 0e06970..806701d 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(
-            xmpp_login_, key_pair_.GenerateCertificate(),
+            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 072eced..a9bef15 100644
--- a/remoting/host/signaling_connector.cc
+++ b/remoting/host/signaling_connector.cc
@@ -55,7 +55,8 @@ void SignalingConnector::EnableOAuth(
     scoped_ptr<OAuthCredentials> oauth_credentials,
     net::URLRequestContextGetter* url_context) {
   oauth_credentials_ = oauth_credentials.Pass();
-  gaia_oauth_client_.reset(new GaiaOAuthClient(kGaiaOAuth2Url, url_context));
+  gaia_oauth_client_.reset(new GaiaOAuthClient(
+      OAuthProviderInfo::GetDefault(), url_context));
 }
 
 void SignalingConnector::OnSignalStrategyStateChange(
@@ -93,11 +94,20 @@ void SignalingConnector::OnOnlineStateChanged(bool online) {
   }
 }
 
-void SignalingConnector::OnRefreshTokenResponse(const std::string& access_token,
+void SignalingConnector::OnRefreshTokenResponse(const std::string& user_email,
+                                                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 a03ae64..4b432f3 100644
--- a/remoting/host/signaling_connector.h
+++ b/remoting/host/signaling_connector.h
@@ -71,7 +71,8 @@ class SignalingConnector
   virtual void OnOnlineStateChanged(bool online) OVERRIDE;
 
   // GaiaOAuthClient::Delegate interface.
-  virtual void OnRefreshTokenResponse(const std::string& access_token,
+  virtual void OnRefreshTokenResponse(const std::string& user_email,
+                                      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 bb87ee4..3e2bc2a 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(
-              xmpp_login_, key_pair_.GenerateCertificate(),
-              *key_pair_.private_key(), host_secret_hash_));
+              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 6ae4f1a..5cac0fc 100644
--- a/remoting/jingle_glue/xmpp_signal_strategy.cc
+++ b/remoting/jingle_glue/xmpp_signal_strategy.cc
@@ -167,24 +167,6 @@ 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 4ed7fa5..b8aa37d 100644
--- a/remoting/protocol/authenticator.h
+++ b/remoting/protocol/authenticator.h
@@ -110,6 +110,7 @@ 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 52c78c0..681f2a9 100644
--- a/remoting/protocol/fake_authenticator.cc
+++ b/remoting/protocol/fake_authenticator.cc
@@ -156,6 +156,7 @@ 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 2e60758..a6ab85e 100644
--- a/remoting/protocol/fake_authenticator.h
+++ b/remoting/protocol/fake_authenticator.h
@@ -89,6 +89,7 @@ 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 9599556..19853ce 100644
--- a/remoting/protocol/it2me_host_authenticator_factory.cc
+++ b/remoting/protocol/it2me_host_authenticator_factory.cc
@@ -25,6 +25,7 @@ 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 aa04009..c526cbf 100644
--- a/remoting/protocol/it2me_host_authenticator_factory.h
+++ b/remoting/protocol/it2me_host_authenticator_factory.h
@@ -31,6 +31,7 @@ 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 f3231af..5f48518 100644
--- a/remoting/protocol/jingle_session_manager.cc
+++ b/remoting/protocol/jingle_session_manager.cc
@@ -141,7 +141,8 @@ bool JingleSessionManager::OnSignalStrategyIncomingStanza(
 
     scoped_ptr<Authenticator> authenticator =
         authenticator_factory_->CreateAuthenticator(
-            message.from, message.description->authenticator_message());
+            signal_strategy_->GetLocalJid(), 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 776c2d1..ba46c4d 100644
--- a/remoting/protocol/me2me_host_authenticator_factory.cc
+++ b/remoting/protocol/me2me_host_authenticator_factory.cc
@@ -58,30 +58,34 @@ 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_prefix_, false)) {
+      !StartsWithASCII(remote_jid, local_jid.substr(0, slash_pos + 1), 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 b0f9b4e..1a5acb1 100644
--- a/remoting/protocol/me2me_host_authenticator_factory.h
+++ b/remoting/protocol/me2me_host_authenticator_factory.h
@@ -24,7 +24,6 @@ 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);
@@ -32,6 +31,7 @@ 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;