Switch GAIA e-mail address retrieval from /GetUserInfo to /ListAccounts

Previously, the Chrome OS SAML login flow was passing the LSID obtained
via /ServiceLogin to /GetUserInfo in order to retrieve the authenticated
user's e-mail address. It turns out that this is wrong because
/ServiceLogin yields a browser LSID and /GetUserInfo expects a
programmatic LSID. In many cases, the two LSID flavors are identical and
the existing code worked. But under some conditions, the browser LSID
could be different, causing /GetUserInfo to fail.

This CL switches to /ListAccounts instead, which handles browser LSIDs.
An additional advantage of /ListAccounts is that it will read the LSID
from cookies, removing the need to extract the LSID from the cookie jar
explicitly.

I could have further simplified the code by doing an XHR to /ListAccounts
from the JS code of the auth extension, avoiding the JS -> C++ -> JS round
trip. However, this would have been a CORS request, requiring the GAIA
URL to be hard-coded in the auth extension's manifest. The implementation
in this CL, which makes the /ListAccounts call from C++, is more flexible
as it preserves the ability to change the GAIA URL via a command-line
flag.

BUG=332132
TEST=Updated browser test and manual

Review URL: https://codereview.chromium.org/134483008

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

chrome/browser/ui/webui/chromeos/login/authenticated_user_email_retriever.cc

@@ -4,84 +4,39 @@
 
 #include "chrome/browser/ui/webui/chromeos/login/authenticated_user_email_retriever.h"
 
-#include "base/bind.h"
-#include "base/callback.h"
-#include "base/location.h"
-#include "base/logging.h"
-#include "content/public/browser/browser_thread.h"
-#include "google_apis/gaia/gaia_auth_fetcher.h"
+#include <vector>
+
+#include "google_apis/gaia/gaia_auth_util.h"
 #include "google_apis/gaia/gaia_constants.h"
-#include "google_apis/gaia/gaia_urls.h"
 #include "google_apis/gaia/google_service_auth_error.h"
-#include "net/cookies/cookie_monster.h"
-#include "net/url_request/url_request_context.h"
-#include "url/gurl.h"
+#include "net/url_request/url_request_context_getter.h"
 
 namespace chromeos {
 
-namespace {
-
-scoped_refptr<net::CookieStore> GetCookieStoreOnIO(
-    scoped_refptr<net::URLRequestContextGetter> url_request_context_getter) {
-  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
-  return url_request_context_getter->GetURLRequestContext()->cookie_store();
-}
-
-}  // namespace
-
 AuthenticatedUserEmailRetriever::AuthenticatedUserEmailRetriever(
     const AuthenticatedUserEmailCallback& callback,
     scoped_refptr<net::URLRequestContextGetter> url_request_context_getter)
     : callback_(callback),
-      url_request_context_getter_(url_request_context_getter),
-      weak_factory_(this) {
-  content::BrowserThread::PostTaskAndReplyWithResult(
-      content::BrowserThread::IO,
-      FROM_HERE,
-      base::Bind(&GetCookieStoreOnIO, url_request_context_getter),
-      base::Bind(&AuthenticatedUserEmailRetriever::ExtractCookies,
-                 weak_factory_.GetWeakPtr()));
+      gaia_auth_fetcher_(this,
+                         GaiaConstants::kChromeSource,
+                         url_request_context_getter) {
+  gaia_auth_fetcher_.StartListAccounts();
 }
 
 AuthenticatedUserEmailRetriever::~AuthenticatedUserEmailRetriever() {
 }
 
-void AuthenticatedUserEmailRetriever::OnGetUserInfoSuccess(
-    const UserInfoMap& data) {
-  UserInfoMap::const_iterator it = data.find("email");
-  callback_.Run(it != data.end() ? it->second : "");
-}
-
-void AuthenticatedUserEmailRetriever::OnGetUserInfoFailure(
-    const GoogleServiceAuthError& error) {
-  callback_.Run(std::string());
-}
-
-void AuthenticatedUserEmailRetriever::ExtractCookies(
-    scoped_refptr<net::CookieStore> cookie_store) {
-  if (!cookie_store) {
+void AuthenticatedUserEmailRetriever::OnListAccountsSuccess(
+    const std::string& data) {
+  std::vector<std::string> accounts = gaia::ParseListAccountsData(data);
+  if (accounts.size() != 1)
     callback_.Run(std::string());
-    return;
-  }
-  cookie_store->GetCookieMonster()->GetAllCookiesForURLAsync(
-      GaiaUrls::GetInstance()->gaia_url(),
-      base::Bind(&AuthenticatedUserEmailRetriever::ExtractLSIDFromCookies,
-                 weak_factory_.GetWeakPtr()));
+  else
+    callback_.Run(accounts.front());
 }
 
-void AuthenticatedUserEmailRetriever::ExtractLSIDFromCookies(
-    const net::CookieList& cookies) {
-  for (net::CookieList::const_iterator it = cookies.begin();
-       it != cookies.end(); ++it) {
-    if (it->Name() == "LSID") {
-      gaia_auth_fetcher_.reset(new GaiaAuthFetcher(
-          this,
-          GaiaConstants::kChromeSource,
-          url_request_context_getter_));
-      gaia_auth_fetcher_->StartGetUserInfo(it->Value());
-      return;
-    }
-  }
+void AuthenticatedUserEmailRetriever::OnListAccountsFailure(
+    const GoogleServiceAuthError& error) {
   callback_.Run(std::string());
 }
 

chrome/browser/ui/webui/chromeos/login/authenticated_user_email_retriever.h

@@ -8,17 +8,18 @@
 #include <string>
 
 #include "base/basictypes.h"
-#include "base/callback_forward.h"
+#include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/memory/ref_counted.h"
-#include "base/memory/weak_ptr.h"
 #include "google_apis/gaia/gaia_auth_consumer.h"
-#include "net/cookies/canonical_cookie.h"
-#include "net/cookies/cookie_store.h"
-#include "net/url_request/url_request_context_getter.h"
+#include "google_apis/gaia/gaia_auth_fetcher.h"
 
 class GaiaAuthFetcher;
 
+namespace net {
+class URLRequestContextGetter;
+}
+
 namespace chromeos {
 
 // Helper class that retrieves the authenticated user's e-mail address.
@@ -27,30 +28,23 @@ class AuthenticatedUserEmailRetriever : public GaiaAuthConsumer {
   typedef base::Callback<void(const std::string&)>
       AuthenticatedUserEmailCallback;
 
-  // Extracts the GAIA cookies from |url_request_context_getter|, retrieves the
-  // authenticated user's e-mail address from GAIA and passes it to |callback|.
-  // If the e-mail address cannot be retrieved, an empty string is passed to
-  // the |callback|.
+  // Retrieves the authenticated user's e-mail address from GAIA and passes it
+  // to |callback|. Requires that |url_request_context_getter| contain the GAIA
+  // cookies for exactly one user. If the e-mail address cannot be retrieved, an
+  // empty string is passed to the |callback|.
   AuthenticatedUserEmailRetriever(
       const AuthenticatedUserEmailCallback& callback,
       scoped_refptr<net::URLRequestContextGetter> url_request_context_getter);
   virtual ~AuthenticatedUserEmailRetriever();
 
   // GaiaAuthConsumer:
-  virtual void OnGetUserInfoSuccess(const UserInfoMap& data) OVERRIDE;
-  virtual void OnGetUserInfoFailure(
-      const GoogleServiceAuthError& error) OVERRIDE;
+  virtual void OnListAccountsSuccess(const std::string& data) OVERRIDE;
+  virtual void OnListAccountsFailure(const GoogleServiceAuthError& error)
+      OVERRIDE;
 
  private:
-  void ExtractCookies(scoped_refptr<net::CookieStore> cookie_store);
-  void ExtractLSIDFromCookies(const net::CookieList& cookies);
-
-  AuthenticatedUserEmailCallback callback_;
-
-  scoped_refptr<net::URLRequestContextGetter> url_request_context_getter_;
-  scoped_ptr<GaiaAuthFetcher> gaia_auth_fetcher_;
-
-  base::WeakPtrFactory<AuthenticatedUserEmailRetriever> weak_factory_;
+  const AuthenticatedUserEmailCallback callback_;
+  GaiaAuthFetcher gaia_auth_fetcher_;
 
   DISALLOW_COPY_AND_ASSIGN(AuthenticatedUserEmailRetriever);
 };

google_apis/gaia/fake_gaia.cc

@@ -47,6 +47,9 @@ const base::FilePath::CharType kServiceLogin[] =
 const char kAuthHeaderBearer[] = "Bearer ";
 const char kAuthHeaderOAuth[] = "OAuth ";
 
+const char kListAccountsResponseFormat[] =
+    "[\"gaia.l.a.r\",[[\"gaia.l.a\",1,\"\",\"%s\",\"\",1,1,0]]]";
+
 typedef std::map<std::string, std::string> CookieMap;
 
 // Parses cookie name-value map our of |request|.
@@ -169,9 +172,9 @@ void FakeGaia::Initialize() {
   REGISTER_RESPONSE_HANDLER(
       gaia_urls->oauth2_issue_token_url(), HandleIssueToken);
 
-  // Handles /GetUserInfo GAIA call.
+  // Handles /ListAccounts GAIA call.
   REGISTER_RESPONSE_HANDLER(
-      gaia_urls->get_user_info_url(), HandleGetUserInfo);
+      gaia_urls->list_accounts_url(), HandleListAccounts);
 }
 
 scoped_ptr<HttpResponse> FakeGaia::HandleRequest(const HttpRequest& request) {
@@ -519,20 +522,9 @@ void FakeGaia::HandleIssueToken(const HttpRequest& request,
   }
 }
 
-void FakeGaia::HandleGetUserInfo(const HttpRequest& request,
+void FakeGaia::HandleListAccounts(const HttpRequest& request,
                                  BasicHttpResponse* http_response) {
-  std::string lsid;
-  if (!GetQueryParameter(request.content, "LSID", &lsid)) {
-    http_response->set_code(net::HTTP_BAD_REQUEST);
-    LOG(ERROR) << "/GetUserInfo missing LSID";
-    return;
-  }
-  if (lsid != merge_session_params_.auth_lsid_cookie) {
-    http_response->set_code(net::HTTP_BAD_REQUEST);
-    LOG(ERROR) << "/GetUserInfo contains unknown LSID";
-    return;
-  }
   http_response->set_content(base::StringPrintf(
-      "email=%s", merge_session_params_.email.c_str()));
+      kListAccountsResponseFormat, merge_session_params_.email.c_str()));
   http_response->set_code(net::HTTP_OK);
 }

google_apis/gaia/fake_gaia.h

@@ -72,7 +72,7 @@ class FakeGaia {
     std::string session_sid_cookie;
     std::string session_lsid_cookie;
 
-    // The e-mail address returned by /GetUserInfo.
+    // The e-mail address returned by /ListAccounts.
     std::string email;
   };
 
@@ -148,8 +148,8 @@ class FakeGaia {
                        net::test_server::BasicHttpResponse* http_response);
   void HandleIssueToken(const net::test_server::HttpRequest& request,
                         net::test_server::BasicHttpResponse* http_response);
-  void HandleGetUserInfo(const net::test_server::HttpRequest& request,
-                         net::test_server::BasicHttpResponse* http_response);
+  void HandleListAccounts(const net::test_server::HttpRequest& request,
+                          net::test_server::BasicHttpResponse* http_response);
 
   // Returns the access token associated with |auth_token| that matches the
   // given |client_id| and |scope_string|. If |scope_string| is empty, the first