davido / gerrit-oauth-provider

OAuth2 authentication provider for Gerrit Code Review. Please upload changes for review to: https://gerrit-review.googlesource.com/#/admin/projects/plugins/oauth
Apache License 2.0
140 stars 84 forks source link

failure to associate oauth externalId to existing account based on email #132

Closed kardianos closed 4 years ago

kardianos commented 4 years ago

Currently running gerrit 3.0.3 and what I beleive to be the latest oauth plugin from https://github.com/davido/gerrit-oauth-provider/releases/tag/v3.0.0 .

We were previously authenticating using LDAP. I'm hoping to switch over to OAUTH (gitlab) authentication. My ideal would be to associate the accountId my matching emails.

For a given account, the they may have: [externalId "username:<username>"] and [externalId "gerrit:<username>"]. Some users have additional mailto externalIds as well.

Without creating the externalId to the OAUTH ID manually, I get the following (common) error: WARN com.google.gerrit.server.account.AccountManager : Email <username>@<domain> is already assigned to account 10YYYYY; cannot create external ID gitlab-oauth:NNN with the same email for account 10XXXXX.

However, when I git clone the All-Users.git repo, checkout refs/meta/external-ids add the file manually (sha1sum the externalId from gitlab as the filename), then push back to All-Users.git, then it logs in without complaint.

This feels like a bug to me, and it apparently is different then the recently closed issue https://github.com/davido/gerrit-oauth-provider/issues/123 which looks like the fix is in the 3.0.3 release. Perhaps I'm just missing a setting?I can also file an issue in monorail if that would help.

kardianos commented 4 years ago

For reference:

I've fixed-up my current accounts by checking out refs/meta/external-ids, then running a script for each user:

#!/bin/bash

F=$(echo -n "gitlab-oauth:$1"|sha1sum|cut -f 1 -d ' ')
GF=$(echo -n "gerrit:$2"|sha1sum|cut -f 1 -d ' ')
AID=$(git config -f $GF externalid.gerrit:${2}.accountid)

echo "Account=$AID"
echo "Wrote $F"

cat > ${F} <<EOT
[externalId "gitlab-oauth:$1"]
        accountId = ${AID}
EOT

Called with: add-user.sh <gitlab-id> <gerrit-username>

To use clone All-Users.git, then checkout HEAD:refs/meta/external-ids, run the script per user (loop through export or manually call), commit changes, then push to HEAD:refs/meta/external-ids.

Miccia94 commented 4 years ago

I've the same problem, actually i'm using 3.1.3 version. I fixed it int the oauth plugin: https://github.com/Miccia94/gerrit_oauth/commits/master

davido commented 4 years ago

Well this is unfortunate, that you have to patch the plugin.

davido commented 4 years ago

Is this issue fixed now? There were a number of fixes recently in this code area. Also how should Gerrit unserstand that you are linking those accounts?

Miccia94 commented 4 years ago

@davido Currently running 3.2.2 with oauth 41e558599a and seems to work correctly, but wont with some users:

com.google.gerrit.server.account.AccountManager : Email @gmail.com is already assigned to account 181; cannot create external ID google-oauth: with the same email for account 3.

davido commented 4 years ago

@Miccia94 Can you check All-Users repository for this user? Also, are you supporting more than one OAuth provider? We should be able to track down the problem to understand what is going on. Is the user trying to link two different identities?

In Gerrit 3.2.2 all problems in this code area should be fixed.

Miccia94 commented 4 years ago

@davido We are using github/google oauth. What should i check in All-Users repo?

This is what i get in All-Users.git

[externalId "3531345"] accountId = 181 email = usermail@gmail.com [externalId "108274519659425959690"] accountId = 3 email = usermail@gmail.com [externalId "mailto:usermail@gmail.com"] accountId = 3 email = usermail@gmail.com

davido commented 4 years ago

@Miccia94 As expected, the linking of identitied is broken.

Identitied linking works like this:

  1. User log in with Gmail
  2. User klick on link identitied
  3. Use log in with GitHub

In the end, different identities must be pointing to the same account:

[externalId "3531345"]
accountId = 3
email = usermail@github.com

Idealy, you would set up a staging instance, and test account linking from sctarch. Does it work as expected?

Miccia94 commented 4 years ago

Actually my new account looks fine @davido

[externalId "109031793006731947128"] accountId = 1601 email = bono.michele94@gmail.com [externalId "google-oauth:109031793006731947128"] accountId = 1601 email = bono.michele94@gmail.com

davido commented 4 years ago

Actually my new account looks fine @davido

If you link to GitHub as described above, you should get:

[externalId "github-oauth:<whatever-your GitHub-Id would be>"]
accountId = 1601
email = bono.michele94@github.com

The problem is: the broken user hasn't clicked "Link New Identity" button from the UI, but just registered with another provider.

In that case Gerrit doesn't know that this is the same user and create new account. It works as expected.

Now you have to merge the accounts for him to fix it. Previously that was done in ReviewDb. Now you have to messa round with NoteDb to fix it.

Miccia94 commented 4 years ago

@davido Also seems i miss the Link New Identity button under Settings -> identity

davido commented 4 years ago

@Miccia94 Yeah, you are not alone, unfortuantely. Many Gerrit users do it wrong and shoot themself in the foot: by creating yet another new Gerrit user instead of linking. Now the site admin would have to merge two different accounts to one single account.