gerrit> select * from account_external_ids where external_id LIKE '%ipcool%';
 account_id | email_address       | password | external_id
 -----------+---------------------+----------+-----------------
 609        | kipmaster@gmail.com | NULL     | gerrit:Kipcool
 609        | NULL                | NULL     | username:kipcool
(2 rows; 16 ms)

@mmodell and @thcipriani previously dealt with a similar issue in T197083. They would probably know what needs to be done.

His account is missing a external id. Also this is no longer stored in the db, the db will be out dated now. It’s all stored in All-Users under refs/external* or (similar)

Indeed sorry, the accounts external ids are stored as git notes in refs/meta/external-ids:

$ git fetch origin refs/meta/external-ids
$ git checkout -b meta/external-ids FETCH_HEAD
$ git grep -i kipcool
6c/84cc1cfca1fbbc7f405b7a9f5c6510a7d6ad82:[externalId "username:kipcool"]
c2/18e23df72c4871cc4089f0979df44c00cc4cd8:[externalId "gerrit:Kipcool"]

The file is the sha1 sum of the id (eg: username:kipcool or gerrit:Kipcool). I guess the issue is due to the mismatching case. It is easy to change, just edit the file, get the sha1sum of the new key and git move the file. Then one with Database access right can push the notes to refs/meta/external-ids.

Then, I am not sure whether we need to use the all lower case account (kipcool) or the camel case one (Kipcool).

From T197083 , we also have another case mismatch:

aa/5f73819965682d225b8cc56df028b2c626ddea:[externalId "username:Kaldari"]
b3/d4b7a688fcf1202457de9031d4bbe36eeb707d:[externalId "gerrit:kaldari"]
ba/4d5d08c2e71dfa36b36ae06f3a6f8ac90d81ff:[externalId "username:kaldari"]

@Albert221 has a similar issue:

All-Users$ git fetch origin refs/meta/externals-id
All-Users$ git grep -i albert221
80/e6adae5c49fadbb97697d11da542868c34d532:[externalId "username:albert221"]
af/f9df51330719d69bc40004cb18815156b8ab52:[externalId "gerrit:Albert221"]

The external ids mismatch :(

We can not fix accounts one by one. Gerrit enforces a validation when pushing to refs/meta/external-ids. I gave it a try using:

git clone All-Users.git
git fetch origin refs/meta/external-ids
git checkout -b meta/external-ids FETCH_HEAD
git touch foo && git commit -a -m 'dummy commit'
git push origin HEAD:refs/for/refs/meta/external-ids

Gerrit then complains with lot of accounts having duplicate emails. Since I don't know whether those emails are public, I pasted the output to a private/restricted task T216632.

Maybe a git push --force would bypass the validation, but I would rather not try that against the production Gerrit.

If we can find a reliable fix (eg normalize to all lower case?), we can write a quick script that fix all entries, push the result and we will be set.

Protecting this for now, since user emails are exposed, which we typically don't want in public Phab tasks.

In T216605#4969481, @sbassett wrote:

Protecting this for now, since user emails are exposed, which we typically don't want in public Phab tasks.

Though the emails are already publicly available. They are used by git/gerrit :)

Though the emails are already publicly available. They are used by git/gerrit :)

Fair enough, just being paranoid.

Our Gerrit instance has ldap.localUsernameToLowerCase = true. Reading the doc at https://gerrit-review.googlesource.com/Documentation/config-gerrit.html#ldap.localUsernameToLowerCase :

ldap.localUsernameToLowerCase

Converts the local username, that is used to login into the Gerrit Web UI, to lower case before doing the LDAP authentication. By setting this parameter to true, a case insensitive login to the Gerrit Web UI can be achieved.

If set, it must be ensured that the local usernames for all existing accounts are converted to lower case, otherwise a user that has a local username that contains upper case characters will not be able to login anymore.
The local usernames for the existing accounts can be converted to lower case by running the server program LocalUsernamesToLowerCase. Please be aware that the conversion of the local usernames to lower case can’t be undone. For newly created accounts the local username will be directly stored in lower case.

I then head to the doc for the program LocalUsernamesToLowerCase at https://gerrit-review.googlesource.com/Documentation/pgm-LocalUsernamesToLowerCase.html

Converts the local username for every account to lower case. The local username is the username that is used to login into the Gerrit Web UI. The local username is stored a external ID with scheme gerrit.

IMPORTANT: This program does not modify usernames in the username scheme.

So if I understand it properly all local ids use the scheme gerrit and they should all have a lower case name (for example: gerrit:hashar). In the username scheme it seems acceptable to have mixed case (for example: username:Hashar).

In All-Users.git we have lot of local accounts having upper case letters:

$ git grep 'gerrit:.*[A-Z].*'|wc -l
1519

Which includes Kipcool and Albert211.

@thcipriani and @mmodell looks to me we want to shutdown Gerrit (to prevent user changes), make a backup of All-Users.git and run that conversion script. Knowing it can not be undone (hence the backup).

From the doc:

After all usernames have been migrated, the reindex program is automatically invoked to reindex all accounts.

This task cannot run in the background concurrently to the server; it must be run by itself.

I dont quite understand that last sentence. I seem to parse it as "must shutdown the actual server before running the program".

In T216605#4969869, @sbassett wrote:

Though the emails are already publicly available. They are used by git/gerrit :)

Fair enough, just being paranoid.

No worries :)

In T216605#4969887, @hashar wrote:

@thcipriani and @mmodell looks to me we want to shutdown Gerrit (to prevent user changes), make a backup of All-Users.git and run that conversion script. Knowing it can not be undone (hence the backup).

This was done once previously: T155766: Gerrit: Schedule downtime to migrate all users username to lowercase

Although in an upstream discussion I had recently it's notable that one of the maintainers said:

The expectation would have been that running the LocalUsernamesToLower would have converted the existing "gerrit:Kxxxxxx" external ID to "gerrit:kxxxxxx". However from the ReviewDb data that you have provided above we can see that it was still "gerrit:Kxxxxxx" in ReviewDb before the migration to NoteDb. So either this program wasn't run or it did not work correctly.

So it seems like you're right, we need to run it again.

I was able to recreate this in my test instance by adding a user "UppercaseThcipriani", logging in, then enabling ldap.localUsernameToLowerCase (as we have set in production), logging out, and then logging in yielded, Cannot assign user name "UpperThcipriani" to account 1000006; name already in use. Meanwhile the All-Accounts DB flailed like:

commit 355f2c932995ebd1a8bb995593174a0bc6d232e4
Author: Gerrit Code Review <gerrit@capella.tylercipriani.com>
Date:   Wed Feb 27 15:42:13 2019 +0000

    Update external IDs

diff --git a/21932e70862b3be5786420faa38b5013005e59a2 b/21932e70862b3be5786420faa38b5013005e59a2
deleted file mode 100644
index bf8e689..0000000
--- a/21932e70862b3be5786420faa38b5013005e59a2
+++ /dev/null
@@ -1,2 +0,0 @@
-[externalId "gerrit:upperthcipriani"]
-       accountId = 1000006

commit 57dee0e55bb46cf88bcf36e9bd8ede6c6e25947b
Author: Gerrit Code Review <gerrit@capella.tylercipriani.com>
Date:   Wed Feb 27 15:42:13 2019 +0000

    Update external IDs

diff --git a/21932e70862b3be5786420faa38b5013005e59a2 b/21932e70862b3be5786420faa38b5013005e59a2
new file mode 100644
index 0000000..bf8e689
--- /dev/null
+++ b/21932e70862b3be5786420faa38b5013005e59a2
@@ -0,0 +1,2 @@
+[externalId "gerrit:upperthcipriani"]
+       accountId = 1000006

Unfortunately, I wasn't able to test LocalUsernamesToLowerCase -- it failed with errors about injectors. Manually adding the gerrit:upperthcipriani externalId with the correct accountId using git on the local machine worked fine. i.e.:

git clone ~/gerrit_reviewsite/git/All-Users.git ~/All-Users-$(date -I)
cd ~/All-Users-$(date -I)
git fetch refs/meta/external-ids:refs/meta/external-ids
git checkout FETCH_HEAD
printf '[externalId "username:upperthcipriani"]\n\taccountId = 1000004\n' > \
    $(printf 'username:upperthcipriani' | shasum -a 1 | awk '{print $1}')
git add .
git commit -m 'thcipriani: fix accountId for UpperThcipriani'
git push origin HEAD:refs/meta/external-ids

This is roughly equivalent to what we did in the case of T197083 and should work fine for one-offs, but if this is a bigger problem, we should schedule some downtime (since it needs an offline reindex to run) the LocalUsernamesToLowercase script...again.

In T216605#4969010, @hashar wrote:

Maybe a git push --force would bypass the validation, but I would rather not try that against the production Gerrit.

Nope. This is filed upstream as: https://bugs.chromium.org/p/gerrit/issues/detail?id=9318

If we can find a reliable fix (eg normalize to all lower case?), we can write a quick script that fix all entries, push the result and we will be set.

This is effectively what LocalUsernamesToLowercase does, i.e., all externalId "gerrit:Xxxx" -> externalId "gerrit:xxxx"

Good thanks Tyler for confirming my assumption!

Given LocalUsernamesToLowercase failed on your local Gerrit -which I assume is the same version as prod-, I guess we are a bit blocked there aren't we? :-/

That's fixed with the 2.16 (from my testing using the prod puppet class).

I think this commit https://github.com/GerritCodeReview/gerrit/commit/c9cb9a8bd5163cdd249e6abd3e399d8b40d4d9e6 fixed it but cannot be backported.

In T216605#4988976, @hashar wrote:

Good thanks Tyler for confirming my assumption!

Given LocalUsernamesToLowercase failed on your local Gerrit -which I assume is the same version as prod-, I guess we are a bit blocked there aren't we? :-/

I attempted some manual git-ref surgery this morning for the 2 users listed on this task. @Kipcool and @Albert221 -- could you try logging into gerrit again when you get a chance?

In T216605#4989050, @Paladox wrote:

That's fixed with the 2.16 (from my testing using the prod puppet class).

I think this commit https://github.com/GerritCodeReview/gerrit/commit/c9cb9a8bd5163cdd249e6abd3e399d8b40d4d9e6 fixed it but cannot be backported.

Bummer that it can't be backported. Does that mean we'll have to wait for the 2.16 upgrade to attempt this? I'd rather not try the upgrade and the rename all at once :\

This has been fixed in 2.15 with https://gerrit-review.googlesource.com/c/gerrit/+/215982

Backported it here https://gerrit.wikimedia.org/r/#/c/operations/software/gerrit/+/493412/

@thcipriani : It worked for me, I can now log into gerrit. Thanks!

I tried to use Gerrit but can't.

Cannot assign user name "stryn" to account 6932; name already in use.

@dschwen is also affected. I tried pushing an update to the external-ids and got this from gerrit:

remote: error: commit 38a1a1b: email address gerrit@wikimedia.org is not registered in your account, and you lack 'forge committer' permission.

I'm not sure why it's trying to set the committer to gerrit@wikimedia.org. How were we able to update these before? Any ideas @thcipriani?

In T216605#5059040, @mmodell wrote:

@dschwen is also affected. I tried pushing an update to the external-ids and got this from gerrit:

remote: error: commit 38a1a1b: email address gerrit@wikimedia.org is not registered in your account, and you lack 'forge committer' permission.

I'm not sure why it's trying to set the committer to gerrit@wikimedia.org. How were we able to update these before? Any ideas @thcipriani?

That is unrelated to this task. The git commit has Commit: XXXXX <gerrit@wikimedia.org> which is not a user registered in Gerrit and the end user does not have the permission to bypass that validation. See https://gerrit.wikimedia.org/r/Documentation/access-control.html#category_forge_committer When people send patches, they are usually the committer, an exception is when uploading third party patches eg when importing a repository (which also requires push rights).

There is also https://gerrit.wikimedia.org/r/Documentation/access-control.html#category_forge_author which lets one set the Author field, but there it also need to be a registered user in our instance.

TLDR: the commit author is wrong. We can fill another task and mention and the person should check the output of git config --get user.email.

In T216605#5059040, @mmodell wrote:

@dschwen is also affected. I tried pushing an update to the external-ids and got this from gerrit:

remote: error: commit 38a1a1b: email address gerrit@wikimedia.org is not registered in your account, and you lack 'forge committer' permission.

I'm not sure why it's trying to set the committer to gerrit@wikimedia.org. How were we able to update these before? Any ideas @thcipriani?

recall last time we did this we ran into not only this problem, but also: https://bugs.chromium.org/p/gerrit/issues/detail?id=9318

What I've been doing, is creating my patch as the user on the server and pushing directly with cGit rather than passing through gerrit and jGit.

After pushing you need to reindex accounts which is a pretty painless process (https://www.gerritcodereview.com/cmd-index-start.html)

ssh -p 29418 gerrit.wikimedia.org -- gerrit index start accounts --force

I completely missed that it is @mmodell who was pushing to external-ids :-( Sorry!

Would it be because Gerrit generates the commit itself (hence committer being gerrit@wikimedia.org) and Mukunda lacks Administrative right to external-ids / Forge committer. Thus Gerrit ends up rejecting a commit it crafted itself since it uses Mukunda permissions.

I bet a member of the Administrator group would not face the same error. Or it is something entirely different.

@hashar: I had forgotten that there are multiple other things blocking me from pushing to that repo, so it would be futile even if I got past the permissions problem.

So we fixed @dschwen and @thcipriani made a nice script along the way. @thcipriani: did you post your gerrit-usernametoupper shell script somewhere?

A couple more:

• T220867: Gerrit: Cannot assign user name "vladi2016" to account XXXX; name already in use.
• T221440: Gerrit: cannot assign username "aldnonymous" to account XXX; name already in use

I guess we can keep renaming affected users manually?

And probably want to add Gerrit 2.16 upgrade as a blocker for the magic script LocalUsernamesToLowercase.

In T216605#5069480, @mmodell wrote:

So we fixed @dschwen and @thcipriani made a nice script along the way. @thcipriani: did you post your gerrit-usernametoupper shell script somewhere?

https://gist.github.com/thcipriani/a46c63c5baa76a876b3b499df6690f6e

Hi, after several weeks, I have already been able to log in to Gerrit.

Hey, can this be done for these three users? Specially since the bash script is already there.

• T221440: Gerrit: cannot assign username "aldnonymous" to account XXX; name already in use
• T220867: Gerrit: Cannot assign user name "vladi2016" to account XXXX; name already in use.
• T222186: Gerrit login failure for user tk-999

The last one needs it for the hackathon and I doubt 2.16 will be deployed by then.

In T216605#5164039, @Ladsgroup wrote:

Hey, can this be done for these three users? Specially since the bash script is already there.

• T221440: Gerrit: cannot assign username "aldnonymous" to account XXX; name already in use
• T220867: Gerrit: Cannot assign user name "vladi2016" to account XXXX; name already in use.
• T222186: Gerrit login failure for user tk-999

The last one needs it for the hackathon and I doubt 2.16 will be deployed by then.

Thanks for amalgamating these. I did fixed all of these accounts just now (I hope).

Is there something in this ticket that can't be public? We have/had several individual user tickets that are public and for the same problem and normally i would just send them here.

It has been converted to a security/private task via T216605#4969481 due to user emails being disclosed. Then those emails are already publicly available via the Gerrit web interface. One might want to have a close look at all the content of this task, but my guess is that it can be made public again.

@hashar @Dzahn - this task should be fine to be made public in regards to PII exposure, I was just being overly-paranoid before re: gerrit account emails. And as long as there aren't any outstanding security issues being actively discussed or worked on here, that should be fine as well.

I've done some digging and have found that over 1512 users account are broken! (they contain caps).

See {P8523}

Happening to me too:

In T216605#5224338, @Mhurd wrote:

Happening to me too:

Fixed this one up last week.

In T216605#5181643, @Paladox wrote:

I've done some digging and have found that over 1512 users account are broken! (they contain caps).

Some of those users, weirdly, have lowercase versions of their uppercase username; i.e., gerrit:UpperCase and gerrit:uppercase both tied to the same account.

I wrote a small script to update All-Users, skipping those accounts that already have lowercase versions. It also does the sha1 file renaming needed for the strange structure of this repo:

This found 1305 accounts that had mixed/uppercase only usernames. This does essentially the same thing as the upstream usernameToLowercase script afaict.

Since logins are case-insensitive (and, seemingly, ldap login through the GUI is case insensitive, i.e., I can login with tHcIPrIani) -- this should be safe to run and apply, correct?

If so, we can apply the generated patch, reindex user accounts and resolve this ticket -- trying to think of any other implications of this that I missed here.

In T216605#5251322, @thcipriani wrote:

If so, we can apply the generated patch, reindex user accounts and resolve this ticket -- trying to think of any other implications of this that I missed here.

I say do it!

I concur with ^^.

Thanks for the enthusiasm :)

Could I get someone who is up-to-date on our LDAP setup (either @akosiaris or @bd808, I'm guessing) to review my assertions above? In particular:

In T216605#5251322, @thcipriani wrote:

Since logins are case-insensitive (and, seemingly, ldap login through the GUI is case insensitive, i.e., I can login with tHcIPrIani) -- this should be safe to run and apply, correct?

And

If so, we can apply the generated patch, reindex user accounts and resolve this ticket -- trying to think of any other implications of this that I missed here.

A conceptual +1 that I'm not about to shoot myself in the foot somehow would be nice assurance :)

In T216605#5256926, @thcipriani wrote:

Thanks for the enthusiasm :)

Could I get someone who is up-to-date on our LDAP setup (either @akosiaris or @bd808, I'm guessing) to review my assertions above? In particular:

In T216605#5251322, @thcipriani wrote:

Since logins are case-insensitive (and, seemingly, ldap login through the GUI is case insensitive, i.e., I can login with tHcIPrIani) --

This assertion is correct. All logins in LDAP are case-insensitive (have always been FWIW). This has led to other issues in the past and major effort to solve. You can have a look at T170150 (for my own past pain story) and T165795 (for bd808's pain story). The latter is a bit more involved and also proposes a solution to make the login case sensitive, albeit at a performance cost (see the task for numbers as it may or may not be a solution in this case).

this should be safe to run and apply, correct?

Well...

• what's stopping people from making the same case mistakes and causing the issue again? Won't new accounts face that same problem? Or do we expect LocalUsernamesToLowerCase to mitigate that?
• Are there perhaps people that have actually relied on the uppercase form (for reason not guessable by me currently)? Maybe they should be contacted and asked beforehand?

And

If so, we can apply the generated patch, reindex user accounts and resolve this ticket -- trying to think of any other implications of this that I missed here.

A conceptual +1 that I'm not about to shoot myself in the foot somehow would be nice assurance :)

Aside from my questions above, I don't see an issue with that.

I've tried to log in into Gerrit (my last access was 2 years ago) with my Wikitech account (which works fine), but I'm affected by the same issue (as portraited in the image below:

@thcipriani, could you please fix also my account (or at least explain how to solve it on my own, if possible)? Thank you.

In T216605#5265588, @akosiaris wrote:

• what's stopping people from making the same case mistakes and causing the issue again? Won't new accounts face that same problem? Or do we expect LocalUsernamesToLowerCase to mitigate that?

The setting in our gerrit.config ldap.localUsernamesToLowercase = true means that new folks who login will have their username stored in the gerrit "database" (read:All-Users) as lowercase.

• Are there perhaps people that have actually relied on the uppercase form (for reason not guessable by me currently)? Maybe they should be contacted and asked beforehand?

The local user scheme (gerrit: usernames in All-Users) are only used for UI login. Your account.fullName and your shell access should be stored in different places. My understanding is that folks with mixed-case usernames in the gerrit schema can no longer login (after 2.15.8). Prior to the ldap change mentioned above, a script was run that was intended to do what my shell script does; however, there must have been some bug. In short, I do not think that anyone relies on this and I think the remaining mixed case gerrit: usernames are a mistake.

Aside from my questions above, I don't see an issue with that.

I have a local patch, I plan to apply it tomorrow so that I can be around to ensure there is no other fallout.

In T216605#5294028, @Mess wrote:

I've tried to log in into Gerrit (my last access was 2 years ago) with my Wikitech account (which works fine), but I'm affected by the same issue (as portraited in the image above:

@thcipriani, could you please fix also my account (or at least explain how to solve it on my own, if possible)? Thank you.

@Mess you are part of my local patch. I will update this task tomorrow after a reindex.

ΟΚ, LGTM then.

In T216605#5301271, @thcipriani wrote:

Applied patch and reindexed accounts 2019-07-02 17:53 UTC.

@Mess your problem should be resolved.

@thcipriani I confirm you my problem was fixed with your patch. Thank you so much for your kindness!

Congratulations @thcipriani !