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

Google OAUTH issue #124

Closed Miccia94 closed 4 years ago

Miccia94 commented 5 years ago

Hello Davido,

Im running Gerrit 2.16.8 fully migrated to noteDB. Im still using gerrit-oauth-provider version 2.13.2 and all is fine. I was planning to upgrade the plugin but im having problems. Same as issue 123 This is what i have in gerrit.config:

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

If i try to use the latest version of the plugin i get this error while authenticating then a redirect to a forbidden page

[2019-05-21 23:09:27,787] [HTTP-185] WARN  com.googlesource.gerrit.plugins.oauth.GoogleOAuthService : The support for: link-to-existing-openid-accounts is disconinued
[2019-05-21 23:09:30,529] [HTTP-186] INFO  com.google.gerrit.httpd.auth.oauth.OAuthSession : OAuth2: linking claimed identity to 1601
[2019-05-21 23:09:30,608] [HTTP-186] WARN  com.google.gerrit.server.account.AccountManager : Email *@gmail.com is already assigned to account 1601; cannot create external ID google-oauth:109031793006731947128 with the same email for account 1601.
[2019-05-21 23:09:30,608] [HTTP-186] ERROR com.google.gerrit.httpd.auth.oauth.OAuthSession : Unable to authenticate user "com.google.gerrit.extensions.auth.oauth.OAuthUserInfo@1c548b0a"
com.google.gerrit.server.account.AccountException: Email '*@gmail.com' in use by another account
[2019-05-21 23:10:55,719] [HTTP-187] WARN  org.eclipse.jetty.server.HttpChannel : /oauth
org.scribe.exceptions.OAuthException: Cannot extract an access token. Response was: {
  "error": "invalid_grant",
  "error_description": "Bad Request"
}
[2019-05-21 23:10:55,720] [HTTP-187] ERROR com.google.gerrit.pgm.http.jetty.HiddenErrorHandler : Error in GET /oauth?state=Miv5JCkWlMsF04EJ8017-fKK9rWDBGlAUvQs0YsXqN8&code=4/UgFvJ0GMZEOkUtzk0LYnZN3Eg_7YfmJZIzZz0UEBx2sDrEW0k5b07_daBFypG8G2I2YF0BWbhnqQbIkLCA4lyYo&scope=email+profile+https://www.googleapis.com/auth/userinfo.email+https://www.googleapis.com/auth/userinfo.profile
org.scribe.exceptions.OAuthException: Cannot extract an access token. Response was: {
  "error": "invalid_grant",
  "error_description": "Bad Request"
}

Here the details for the same account retrived from api GET /accounts/1601/external.ids

)]}'
[
  {
    "identity": "109031793006731947128",
    "email_address": "*@gmail.com",
    "trusted": true,
    "can_delete": true
  },
  {
    "identity": "google-oauth:109031793006731947128",
    "trusted": true,
    "can_delete": true
  },
  {
    "identity": "username:Mich",
    "trusted": true
  }
]

Thanks in advance for help

davido commented 5 years ago

The latest version of the plugin is trying to link the non-prefixed external-id to the prefixed one:

  OAuth2: linking claimed identity to 1601
  Email *@gmail.com is already assigned to account 1601;
  cannot create external ID google-oauth:109031793006731947128 with the same email for account 1601.

Unless we drop this restriction in the gerrit core, see: [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

Miccia94 commented 5 years ago

Thanks for the feedback Davido,

I will try to compile the pluging without the external id prefix ( fastest way, instead of bulk edit whole notedb )

davido commented 5 years ago

I will try to look into it as well, and add aditional config option to unblock you and other plugin users who migrated to the latest Gerrit releases. This should be a trivial change.

Miccia94 commented 5 years ago

Hello David,

I can confirm that removing GOOGLE_PROVIDER_PREFIX from https://github.com/davido/gerrit-oauth-provider/blob/master/src/main/java/com/googlesource/gerrit/plugins/oauth/GoogleOAuthService.java#L144 allow me to login

Jmennius commented 5 years ago

I went with option 2 - changed all accounts in NoteDb.

First, I've added a prefix with simple sed (lost it in bash history). But that was not enough - also needed to change filenames. That is what I've used to rehash file names (rehash.sh):

#!/bin/bash

while read filename
do
    id="$(grep -UEo 'google-oauth:[[:digit:]]+' ${filename})"
    new_hash=$(echo -n ${id} | sha1sum | cut -f 1 -d ' ')
    echo "id ${id} renaming ${filename} to ${new_hash}"
    mv ${filename} ${new_hash}
done < "/dev/stdin"

And call it with file names to rehash on the stdin: grep -Er 'externalId "google-oauth:[[:digit:]]+"' . | cut -d ':' -f 1 | ~/gerrit-upgrade/rehash.sh

Althought not a full copy-and-run solution - hope it helps (should be easy to modify).

Jmennius commented 5 years ago

Decided to put together a complete solution since this issue should be relatively widespread (#123, #116 etc)..

Prerequisites:

How to use:

Use this script to patch all unprefixed external-ids to have some prefix. Optional arguments - path to All-Users (defaults to current dir) and oauth-prefix (defaults to google).


USERS_DIR="${1:-.}"
OAUTH_PREFIX="${2:-google-oauth}"

files="$(grep -Er 'externalId "[[:digit:]]+"' ${USERS_DIR} | cut -d ':' -f 1)"

while read -r filename
do
    sed -Ei "s/externalId \"([[:digit:]]+)\"/externalId \"${OAUTH_PREFIX}:\1\"/" ${filename}
    id=$(grep -UEo "${OAUTH_PREFIX}:[[:digit:]]+" ${filename})
    new_hash=$(echo -n ${id} | sha1sum | cut -f 1 -d ' ')
    mv ${filename} ${new_hash}
    echo "patched id: ${id}, renamed ${filename} to ${new_hash}"
done <<< "${files}"
pgeorgi commented 5 years ago

Thanks Jmennius for the script, I extended it somewhat:

USERS_DIR="${1:-.}"
OAUTH_PREFIX="${2:-google-oauth}"
MATCH="${3:-+}"

files="$(grep -Er 'externalId "[[:digit:]]'"${MATCH}"'"' ${USERS_DIR} | cut -d ':' -f 1)"

while read -r filename
do
    sed -Ei "s/externalId \"([[:digit:]]+)\"/externalId \"${OAUTH_PREFIX}:\1\"/" ${filename}
    id=$(grep -UEo "${OAUTH_PREFIX}:[[:digit:]]+" ${filename})
    new_hash=$(echo -n ${id} | sha1sum | cut -f 1 -d ' ')
    git mv ${filename} ${new_hash}
    echo "patched id: ${id}, renamed ${filename} to ${new_hash}"
done <<< "${files}"

This allows running it with a third argument to help separate google and github ID:

script . google-oauth "{10,}"
script . github-oauth "{,9}"

assigns the right prefix to each ID. It also registers the renames with git.

Jmennius commented 5 years ago

@pgeorgi Glad it helped!

davido commented 5 years ago

Thanks, @Jmennius, @pgeorgi great contribution.

How about to add this script to the plugin repository itself with documentation? Optionally also to the WIKI of this GH repository? Though, note, that I do not accept PRs here, but only CLs on the canonical git repository at: googlesource.com.

defer commented 4 years ago

@pgeorgi Thanks, this was useful. I just noticed that at least in 3.0.1 the migration script would leave files in the root instead of at ${SHA:0:2}/${SHA:2:38}, I updated your script a bit:

USERS_DIR="${1:-.}"
OAUTH_PREFIX="${2:-google-oauth}"
MATCH="${3:-+}"

files="$(grep -Er 'externalId "[[:digit:]]'"${MATCH}"'"' ${USERS_DIR} | cut -d ':' -f 1)"

while read -r filename
do
    sed -Ei "s/externalId \"([[:digit:]]+)\"/externalId \"${OAUTH_PREFIX}:\1\"/" ${filename}
    id=$(grep -UEo "${OAUTH_PREFIX}:[[:digit:]]+" ${filename})
    new_hash=$(echo -n ${id} | sha1sum | cut -f 1 -d ' ')
    prefix=${new_hash:0:2}
    remaining=${new_hash:2:38}
    mkdir -p ${prefix}
    git mv ${filename} ${prefix}/${remaining}
    echo "patched id: ${id}, renamed ${filename} to ${prefix}/${remaining}"
done <<< "${files}"
sebguilbaud commented 4 years ago

Prerequisites:

  • NoteDb

How to use:

  • clone All-Users project and checkout meta/external-ids

Could you please tell the exact commands you used ? i'm stuck on this step and I wonder if it's due to my weak git level or to permission issues on all-users repository

have a nice day

sebguilbaud commented 4 years ago

Prerequisites:

  • NoteDb

How to use:

  • clone All-Users project and checkout meta/external-ids

Could you please tell the exact commands you used ? i'm stuck on this step and I wonder if it's due to my weak git level or to permission issues on all-users repository

have a nice day

forget about it, I just discovered the global privilege "Access Database" as told here : https://www.gerritcodereview.com/config-accounts.html#external-ids

:)

davido commented 4 years ago

The problem was fixed in Gerrit: [1].

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