opendev/gerrit
Code Issues Proposed changes

Simplify troubleshooting of OpenID <-> OAuth identity linking

Browse Source 
Currently there is no easy way to troubleshoot the problems that are
reported in the wild concerning failures to link different identities
using Hybrid OpenID+OAuth authentication scheme. Add some log output
to simplify this job.

For example with this change there should be hopefully clear what is
going on with this issue: [1].

Signing in phase with OpenID provider:
======================================

DEBUG com.google.gerrit.httpd.auth.openid.LoginForm : mode "SIGN_IN"
DEBUG com.google.gerrit.httpd.auth.openid.LoginForm : OpenId provider
"https://login.launchpad.net/+openid"
DEBUG com.google.gerrit.httpd.auth.openid.OpenIdServiceImpl : OpenID:
openid-realm=http://localhost:8080/

Following by Identity linking phase with OAuth provider:
========================================================

DEBUG com.google.gerrit.httpd.auth.openid.LoginForm : mode
"LINK_IDENTIY"
DEBUG com.google.gerrit.httpd.auth.openid.LoginForm : OAuth provider
"http://gerrit-oauth-provider_-google-oauth"
[...]
DEBUG com.google.gerrit.httpd.auth.openid.OAuthSessionOverOpenID :
Linking "34534523945376523984" to "1000000"

[1] https://github.com/davido/gerrit-oauth-provider/issues/46

Change-Id: Ida117aea49b86c3915c738ae1b9d8afd917b5eb9
This commit is contained in:
David Ostrovsky 2015-09-18 23:10:55 +02:00 committed by David Ostrovsky
parent a953e28de3
commit c55cb8902f
2 changed files with 11 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
gerrit-openid/src/main/java/com/google/gerrit/httpd/auth/openid/LoginForm.java
View File

@ -164,11 +164,14 @@ class LoginForm extends HttpServlet {
mode = SignInMode.SIGN_IN;
}
log.debug("mode \"{}\"", mode);
OAuthServiceProvider oauthProvider = lookupOAuthServiceProvider(id);
if (oauthProvider == null) {
log.debug("OpenId provider \"{}\"", id);
discover(req, res, link, id, remember, token, mode);
} else {
log.debug("OAuth provider \"{}\"", id);
OAuthSessionOverOpenID oauthSession = oauthSessionProvider.get();
if (!currentUserProvider.get().isIdentifiedUser()
&& oauthSession.isLoggedIn()) {

11
gerrit-openid/src/main/java/com/google/gerrit/httpd/auth/openid/OAuthSessionOverOpenID.java
View File

@ -127,10 +127,12 @@ class OAuthSessionOverOpenID {
Account.Id actualId = accountManager.lookup(user.getExternalId());
// Use case 1: claimed identity was provided during handshake phase
if (!Strings.isNullOrEmpty(claimedIdentifier)) {
log.debug("Claimed identity is set");
Account.Id claimedId = accountManager.lookup(claimedIdentifier);
if (claimedId != null && actualId != null) {
if (claimedId.equals(actualId)) {
// Both link to the same account, that's what we expected.
log.debug("Both link to the same account. All is fine.");
} else {
// This is (for now) a fatal error. There are two records
// for what might be the same user.
@ -144,7 +146,7 @@ class OAuthSessionOverOpenID {
}
} else if (claimedId != null && actualId == null) {
// Claimed account already exists: link to it.
//
log.debug("Claimed account already exists: link to it.");
try {
accountManager.link(claimedId, areq);
} catch (OrmException e) {
@ -157,11 +159,14 @@ class OAuthSessionOverOpenID {
}
} else if (linkMode) {
// Use case 2: link mode activated from the UI
Account.Id accountId = identifiedUser.get().getAccountId();
try {
accountManager.link(identifiedUser.get().getAccountId(), areq);
log.debug("Linking \"{}\" to \"{}\"", user.getExternalId(),
accountId);
accountManager.link(accountId, areq);
} catch (OrmException e) {
log.error("Cannot link: " + user.getExternalId()
+ " to user identity: " + identifiedUser.get().getAccountId());
+ " to user identity: " + accountId);
rsp.sendError(HttpServletResponse.SC_FORBIDDEN);
return;
} finally {