Gerrit 2.16.x only allows an email to exist in a single ExternalID

[zifnab06]

Hi!

We're running the 2.16 plugin from the releases page with Gerrit 2.16.2. Users are stored in NoteDB.

2.16.x only allows an email address to be attached to a single external ID: https://github.com/GerritCodeReview/gerrit/blob/stable-2.16/java/com/google/gerrit/server/account/AccountManager.java#L367-L374

When a user signs in that has a non-prefixed externalId entry (ie, "10000XXXX" instead of "google-oauth:10000XXXX"), the following shows up in logs and the user is redirected to a forbidden page:

[2019-03-04 05:10:34,243] [HTTP-258] INFO  com.google.gerrit.httpd.auth.oauth.OAuthSession : OAuth2: linking claimed identity to 16302
[2019-03-04 05:10:34,394] [HTTP-258] WARN  com.google.gerrit.server.account.AccountManager : Email zifnab06@gmail.com is already assigned to account 16302; cannot create external ID google-oauth:106504818271694337285 with the same email for account 16302.
[2019-03-04 05:10:34,394] [HTTP-258] ERROR com.google.gerrit.httpd.auth.oauth.OAuthSession : Unable to authenticate user "com.google.gerrit.extensions.auth.oauth.OAuthUserInfo@19e7e648"
com.google.gerrit.server.account.AccountException: Email 'zifnab06@gmail.com' in use by another account
        at com.google.gerrit.server.account.AccountManager.checkEmailNotUsed(AccountManager.java:388)
        at com.google.gerrit.server.account.AccountManager.update(AccountManager.java:243)
        at com.google.gerrit.server.account.AccountManager.authenticate(AccountManager.java:178)
        at com.google.gerrit.httpd.auth.oauth.OAuthSession.authenticateAndRedirect(OAuthSession.java:146)
        at com.google.gerrit.httpd.auth.oauth.OAuthSession.login(OAuthSession.java:110)
        at com.google.gerrit.httpd.auth.oauth.OAuthWebFilter.doFilter(OAuthWebFilter.java:105)
        at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:82)
        at com.google.gerrit.httpd.RequireSslFilter.doFilter(RequireSslFilter.java:72)
        at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:82)
        at com.google.gerrit.httpd.RunAsFilter.doFilter(RunAsFilter.java:121)
        at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:82)
        at com.google.gerrit.httpd.GwtCacheControlFilter.doFilter(GwtCacheControlFilter.java:72)
        at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:82)
        at com.google.gerrit.httpd.AllRequestFilter$FilterProxy$1.doFilter(AllRequestFilter.java:133)
        at com.google.gerrit.httpd.AllRequestFilter$FilterProxy.doFilter(AllRequestFilter.java:135)
        at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:82)
        at com.google.gerrit.httpd.RequestMetricsFilter.doFilter(RequestMetricsFilter.java:57)
        at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:82)
        at com.google.gerrit.httpd.RequestContextFilter.doFilter(RequestContextFilter.java:69)
        at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:82)
        at com.google.inject.servlet.ManagedFilterPipeline.dispatch(ManagedFilterPipeline.java:121)
        at com.google.inject.servlet.GuiceFilter.doFilter(GuiceFilter.java:133)
        at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1642)
        at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:533)
        at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:255)
        at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:1595)
        at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:255)
        at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1340)
        at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:203)
        at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:473)
        at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:1564)
        at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:201)
        at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1242)
        at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:144)
        at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:132)
        at org.eclipse.jetty.server.Server.handle(Server.java:503)
        at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:364)
        at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:260)
        at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:305)
        at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:103)
        at org.eclipse.jetty.io.ChannelEndPoint$2.run(ChannelEndPoint.java:118)
        at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.runTask(EatWhatYouKill.java:333)
        at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.doProduce(EatWhatYouKill.java:310)
        at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.tryProduce(EatWhatYouKill.java:168)
        at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.run(EatWhatYouKill.java:126)
        at org.eclipse.jetty.util.thread.ReservedThreadExecutor$ReservedThread.run(ReservedThreadExecutor.java:366)
        at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:765)
        at org.eclipse.jetty.util.thread.QueuedThreadPool$2.run(QueuedThreadPool.java:683)
        at java.lang.Thread.run(Thread.java:748)

An external id is still created for the user without an email, however they're redirected to a forbidden page when they log in.

So, given a user with the following external id block in All-Users.git's refs/meta/external-ids:

[externalId "106504818271694337285"]
        accountId = 16302
        email = zifnab06@gmail.com

gerrit creates an external ID, without an email:

commit ed53f615985fac0816b38b6aadabf7908fe10fef (HEAD)
Author: Gerrit Code Review <gerrit@review.lineageos.org>
Date:   Mon Mar 4 05:10:34 2019 +0100

    Link External ID

    Account: 16302

diff --git a/72/1d56f873e71629ff6f7cf2794640b0aba56d20 b/72/1d56f873e71629ff6f7cf2794640b0aba56d20
new file mode 100644
index 0000000000..ae350d31ba
--- /dev/null
+++ b/72/1d56f873e71629ff6f7cf2794640b0aba56d20
@@ -0,0 +1,2 @@
+[externalId "google-oauth:106504818271694337285"]
+       accountId = 16302

and the user is redirected to https://review.lineageos.org/oauth?state=<snip>&code=<snip>scope=email+profile+https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fuserinfo.profile+https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fuserinfo.email wiht a "Forbidden" message.

I'm also able to reproduce this on a clean installation by removing the "google-oauth:" prefix on an account, moving the external-id file to the correct spot, pushing it, and trying to sign in again, even in an incognito session.

Relevant configuration block (note: the same thing happens without fix-legacy-user-id, the new ExternalID just isn't created)

[plugin "gerrit-oauth-provider-google-oauth"]
        client-id = <snip>
        client-secret = <snip>
        link-to-existing-openid-accounts = true
        fix-legacy-user-id = true

Is there a way to mass migrate users to the new format with NoteDB? Or possibly another workaround that doesn't involve changing ~5300 files in NoteDB?

Thanks!

[davido]

I wonder if this format change is a general problem in Gerrit core. There is also this thread on dev maling list, reporting similar problem after migrating to recent gerrit version: [1].

• [1] https://groups.google.com/forum/#!topic/repo-discuss/wCt_ePUoKg4

[zifnab06]

Back in 2.14, I opened a bug with gerrit about a similar issue (failure modifying All-Users refs/meta/external-ids if an email was duplicated: https://bugs.chromium.org/p/gerrit/issues/detail?id=9001&q=reporter%3Azifnab%40zifnab06.net). From that, and the commit that added the code I linked above, it appears intentional (GerritCodeReview/gerrit@15545de).

[davido]

As you pointed out in: [1], there seems to be only two workarounds:

1. stop prefixing external-id "foo" with "oauth-scheme"-"foo" in this plugin during the login operation
2. bulk change all accounts from "foo" to "oauth-scheme"-"foo" in users backend (before or after NoteDb migration)

[1] https://bugs.chromium.org/p/gerrit/issues/detail?id=9001

[Jmennius]

@zifnab06 You can try patching your user database, see #124 (comment).

[xnlcasad]

I'm facing the same problems when migrating the plugin version and gerrit to 3.0.0. Could you advice on which of the two workarounds would be more future proof?

[davido]

I would definitely suggest to patch the database prior to migration to 2.15. If you haver already migrated then you can use the script provided here: [1].

[1] #124 (comment)

[xnlcasad]

I'm doing a migration from 2.13 to 3.0.0 going thru all the intermediate steps. I'm not doing it on the live server, so I can test/rollback if I have problems.

What did you mean by:

patch the database prior to migration to 2.15

Patch the NoteDB or the ReviewDB ? How would you patch the DB ? using the script on issue 124

Thanks ...

[davido]

I would assume, that it is much easier to fix the database. If would need to prefix the id with specific prefix. Example: old id: "106504818271694337285", new id: "google-oauth:106504818271694337285".

Of course you need a new plugin version, that already supports new external id format.

Once you completed this conversion in the database, you could migrate to 2.15 and the account speific tables are migrated to NoteDb with already correc external id format.

[xnlcasad]

Once you completed this conversion in the database, you could migrate to 2.15 and the account speific tables are migrated to NoteDb with already correc external id format.

Which would be your suggestion to update the entries the NoteDB database?
Should I update the oauth plugin at every gerrit update?

Thanks for your support.

[davido]

You only need to upgrade oauth-plugin once, from older version, that doesn't require prefix, to a newer version that does require prefix for external if format.

[xnlcasad]

I would assume, that it is much easier to fix the database.

Sorry @davido, but I'm really stuck with this. Would you have a hint on how to update the database before migrating to 2.15 ?

[davido]

Connect per ssh gerrit gerrit gsql and issue update statement?

Say you have this table:

gerrit> \d FOO
                     Table FOO
 COLUMN_NAME | TYPE
 ------------+------------
 ID          | VARCHAR(80)

Indexes on FOO:

And say you have these rows:

gerrit> select id from FOO;
 ID
 ----
 4711
 4712
 4713
(3 rows; 2 ms)

Now, we would like to prefix the ids with some specific prefix, say 'prefix-', then we could do this update:

update FOO set id = 'prefix-' || id;
UPDATE 3; 1 ms

Now the content is:

gerrit> select id from FOO;
 ID
 -----------
 prefix-4711
 prefix-4712
 prefix-4713
(3 rows; 2 ms)

[xnlcasad]

@davido Thanks!
I managed to make it work following your suggestions. I used this query to modify only the ids that required adding the prefix:

gerrit> update ACCOUNT_EXTERNAL_IDS set EXTERNAL_ID = 'google-oauth:' || EXTERNAL_ID where (lower(EXTERNAL_ID) = upper(EXTERNAL_ID));

The lower() = upper() part is to select only those rows that have the oauth value which is only a number, without any letters.

[davido]

Good news: thanks @lucamilanesio this is now fixed: [1]. I will close this issue, when: [1] is merged.

• [1] https://gerrit-review.googlesource.com/c/gerrit/+/238833