Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(174)

Issue 10332187: Properly handle accounts that don't have GMail. (Closed)

Created:
8 years, 4 months ago by Sergey Ulanov
Modified:
8 years, 4 months ago
Reviewers:
Wez
CC:
chromium-reviews, jamiewalch+watch_chromium.org, dcaiafa+watch_chromium.org, simonmorris+watch_chromium.org, hclam+watch_chromium.org, wez+watch_chromium.org, amit, sanjeevr, garykac+watch_chromium.org, lambroslambrou+watch_chromium.org, alexeypa+watch_chromium.org, sergeyu+watch_chromium.org
Visibility:
Public.

Description

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 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=137824

Patch Set 1 #

Patch Set 2 : #

Total comments: 16

Patch Set 3 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+129 lines, -63 lines) Patch
M remoting/host/gaia_oauth_client.h View 1 2 3 chunks +11 lines, -6 lines 0 comments Download
M remoting/host/gaia_oauth_client.cc View 1 2 5 chunks +84 lines, -26 lines 0 comments Download
M remoting/host/remoting_me2me_host.cc View 1 chunk +1 line, -1 line 0 comments Download
M remoting/host/signaling_connector.h View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M remoting/host/signaling_connector.cc View 1 2 2 chunks +12 lines, -2 lines 0 comments Download
M remoting/host/simple_host_process.cc View 1 2 1 chunk +2 lines, -2 lines 0 comments Download
M remoting/jingle_glue/xmpp_signal_strategy.cc View 1 1 chunk +0 lines, -18 lines 0 comments Download
M remoting/protocol/authenticator.h View 1 chunk +1 line, -0 lines 0 comments Download
M remoting/protocol/fake_authenticator.h View 1 chunk +1 line, -0 lines 0 comments Download
M remoting/protocol/fake_authenticator.cc View 1 chunk +1 line, -0 lines 0 comments Download
M remoting/protocol/it2me_host_authenticator_factory.h View 1 chunk +1 line, -0 lines 0 comments Download
M remoting/protocol/it2me_host_authenticator_factory.cc View 1 chunk +1 line, -0 lines 0 comments Download
M remoting/protocol/jingle_session_manager.cc View 1 chunk +2 lines, -1 line 0 comments Download
M remoting/protocol/me2me_host_authenticator_factory.h View 2 chunks +1 line, -1 line 0 comments Download
M remoting/protocol/me2me_host_authenticator_factory.cc View 1 chunk +9 lines, -5 lines 0 comments Download

Messages

Total messages: 13 (0 generated)
Sergey Ulanov
8 years, 4 months ago (2012-05-15 21:31:05 UTC) #1
Wez
Before I get into code-review, could you beef up the description to explain what it ...
8 years, 4 months ago (2012-05-15 23:14:07 UTC) #2
Sergey Ulanov
On 2012/05/15 23:14:07, Wez wrote: > Before I get into code-review, could you beef up ...
8 years, 4 months ago (2012-05-15 23:33:32 UTC) #3
Wez
To address the immediate issue this looks fine, but it takes us in the direction ...
8 years, 4 months ago (2012-05-16 01:21:20 UTC) #4
Sergey Ulanov
On 2012/05/16 01:21:20, Wez wrote: > To address the immediate issue this looks fine, but ...
8 years, 4 months ago (2012-05-16 17:53:15 UTC) #5
Wez
On 2012/05/16 17:53:15, sergeyu wrote: > On 2012/05/16 01:21:20, Wez wrote: > > To address ...
8 years, 4 months ago (2012-05-16 21:23:20 UTC) #6
Sergey Ulanov
On Wed, May 16, 2012 at 2:23 PM, <wez@chromium.org> wrote: > On 2012/05/16 17:53:15, sergeyu ...
8 years, 4 months ago (2012-05-16 21:33:21 UTC) #7
Wez
http://codereview.chromium.org/10332187/diff/3001/remoting/host/gaia_oauth_client.cc File remoting/host/gaia_oauth_client.cc (right): http://codereview.chromium.org/10332187/diff/3001/remoting/host/gaia_oauth_client.cc#newcode21 remoting/host/gaia_oauth_client.cc:21: const char kUserInfoUrl[] = "https://www.googleapis.com/oauth2/v1/userinfo"; It seems wrong for ...
8 years, 4 months ago (2012-05-16 23:00:06 UTC) #8
Sergey Ulanov
http://codereview.chromium.org/10332187/diff/3001/remoting/host/gaia_oauth_client.cc File remoting/host/gaia_oauth_client.cc (right): http://codereview.chromium.org/10332187/diff/3001/remoting/host/gaia_oauth_client.cc#newcode21 remoting/host/gaia_oauth_client.cc:21: const char kUserInfoUrl[] = "https://www.googleapis.com/oauth2/v1/userinfo"; On 2012/05/16 23:00:07, Wez ...
8 years, 4 months ago (2012-05-17 00:38:11 UTC) #9
Sergey Ulanov
ping
8 years, 4 months ago (2012-05-17 21:08:22 UTC) #10
Wez
lgtm
8 years, 4 months ago (2012-05-17 22:36:19 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sergeyu@chromium.org/10332187/11002
8 years, 4 months ago (2012-05-18 02:53:28 UTC) #12
commit-bot: I haz the power
8 years, 4 months ago (2012-05-18 04:31:14 UTC) #13
Change committed as 137824

Powered by Google App Engine
This is Rietveld 408576698