Skip to content

fix(org): match account names and logins case-insensitively - #9064

Open
vbhanuchander-lang wants to merge 1 commit into
apache:mainfrom
vbhanuchander-lang:issue-8698-fold-case-account-names
Open

fix(org): match account names and logins case-insensitively#9064
vbhanuchander-lang wants to merge 1 commit into
apache:mainfrom
vbhanuchander-lang:issue-8698-fold-case-account-names

Conversation

@vbhanuchander-lang

Copy link
Copy Markdown
Contributor

Summary

ConnectUserAccountsExact links a user to an account through three exact comparisons: email,
FullName, and UserName. #9051 made the email comparison case-insensitive. The two name
comparisons were left matching exactly, and this makes them consistent with it.

The case that matters most is UserName, which holds the provider login. Provider logins are
themselves case-insensitive — GitHub resolves TyroneKCummings and tyronekcummings to the same
account — so comparing them case-sensitively against the users.csv name is comparing on a
distinction the provider does not make. Display names are folded for the same reason emails were: a
corporate git config and a provider profile routinely record the same name with different
capitalisation.

The consequence of a missed link is silent and easy to misread as missing data: the account is never
associated with the user, so every activity attributing through it disappears from work logs while
other activities for the same person keep appearing.

Does this close any open issues?

Towards #8698 — it does not close it. In that issue the GitHub account has no public email
(GitHub returns an empty string for users who have not made it public), so linkage can only happen
through the two name paths this PR fixes. That removes the case-mismatch failure, but a user whose
CSV name simply differs from their GitHub login is still unlinked. Closing #8698 properly needs a
way to declare the provider login rather than guess it, which is a larger change to the
crossdomain.User model and worth discussing on the issue first — user_account_mapping.csv
already covers the declared case and may be the preferred answer.

Other Information

Tested by extending the existing org e2e fixtures with account a12, whose login N7 differs from
user U007's name n7 only in case, and with no email so the match is forced down the login path.
With the change it links; reverting the change drops the expected user_accounts row count from 12
to 11 and the test fails.

gofmt and go vet clean. No documentation change: this corrects existing matching behaviour
rather than adding a knob.

Emails were made case-insensitive in apache#9051, but the display name and
provider login comparisons in ConnectUserAccountsExact still matched
exactly. Provider logins are themselves case-insensitive, and a corporate
git config and a provider profile routinely record the same display name
with different capitalisation, so an account whose login differs from the
users.csv name only in case is silently left unlinked - and every activity
that attributes through that account goes missing.

Towards apache#8698, where a GitHub account with no public email can only link
through these name paths.

Extends the org e2e fixtures with an account whose login differs from the
user's name only in case; without the change it is not linked and the
expected row count drops from 12 to 11.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant