Page MenuHomePhabricator

Commits being made in random users names (bad associations)
Closed, ResolvedPublic

Description

Instance 1:

I got an email today that this commit was made on my behalf, however I did not write the code or commit it. Apparently it was made on June 29, but it says on my profile that the activity was on December 9.

I'm guessing that, because the usernames were matching, it thought I was that user.

Instance 2:

https://phabricator.wikimedia.org/rODCVbb2765dcfe11 for @CharlieTheCabbie

Event Timeline

Daviskr created this task.Dec 9 2014, 5:39 AM
Daviskr raised the priority of this task from to Needs Triage.
Daviskr updated the task description. (Show Details)
Daviskr changed Security from none to None.
Daviskr added a subscriber: Daviskr.
Qgil triaged this task as Normal priority.Dec 9 2014, 12:57 PM
Qgil edited projects, added Gitblit-Deprecate; removed Phabricator.
Qgil added a subscriber: Qgil.Dec 9 2014, 2:54 PM

Strange. I would assume that the check is done against email address and not username? This is the original changeset in Gerrit: https://gerrit.wikimedia.org/r/#/c/141266/ . There you can see the email address of DavisNT. For some strange reason, could that be the same address that you have now?

Thanks for noticing the 404. I don't know whether it is intended, but it happens to partially solve T75715 while we figure out what to do there.

chasemp added a subscriber: chasemp.Dec 9 2014, 3:25 PM

No, neither the username nor email matched mine (davis_at_daviskr.com) or any I have used in the past. Maybe DavisNT had changed his name from DavisKR to DavisNT? This is very strange.

This is expected. For committer a <b@c.com>, we follow this logic:

  • Match the user with email address b@c.com, if one exists.
  • Match the user with username a, if one exists.
  • Match the user with real name a, if exactly one exists.

Specifically, the name on the commit ("Davis") matches the real name on your account ("Davis"). In the common case these strings are more like "Evan Priestley" vs "Evan Priestley", which is a very high-confidence match for most installs.

You can work around this by choosing a different "real name" for your account, I suppose.

See also https://secure.phabricator.com/T1731

Qgil added a comment.EditedDec 9 2014, 11:39 PM

(epriestley was faster)

Oh -- the "Real Name" field on this install has been renamed to "Also Known As", which may generally reduce match strength. In the upstream, this field is called "Real Name".

If you want to disable this behavior, it lives here and could safely be commented out:

https://secure.phabricator.com/diffusion/P/browse/master/src/applications/diffusion/query/DiffusionResolveUserQuery.php;d5e7cd5590e973c1fd0b45f899b1d7330a444ee2$109-119

There's also an event listener you can use to override this behavior without forking, but we generally discourage using event listeners now (other approaches have superseded them for most customization/hooks):

https://secure.phabricator.com/book/phabricator/article/events/#diffusion-lookup-user

The scripts/repository/reparse.php --message <commit> script can fix bad user identifications after the initial import.

Note: the field label was changed due to T798

:-/

Qgil moved this task from To Triage to Need discussion on the Phabricator board.Dec 22 2014, 10:04 AM

Like the OP on this bug, I got an email about 15 minutes ago, stating that this patch was committed in my name, and authored by myself. The thing is, it wasn't. I couldn't code if my life depended on it. I hope something can be resolved to stop this happening - I don't want to be blamed for something if it goes toes up in my name :)

chasemp renamed this task from Commit made in my name to Commits being made in random users names (bad associations).Mar 3 2015, 9:44 PM
chasemp updated the task description. (Show Details)

@Daviskr I made this issue a bit more general to link two cases and provide more context. We still need to tackle this.

Not understanding. Are not these from the description what you mean?

I mean that in diffusion so far I only found diffusion's own interpretation of the metadata, but no way to see the metadata itself, for instance the email address (which would allow the user to double check usernames).

chasemp raised the priority of this task from Normal to High.Mar 23 2015, 4:36 PM
Restricted Application added a subscriber: scfc. · View Herald TranscriptJun 16 2015, 12:52 PM

I mean that in diffusion so far I only found diffusion's own interpretation of the metadata, but no way to see the metadata itself, for instance the email address (which would allow the user to double check usernames).

Given that ^ (unable to see the original metadata in Diffusion) we should probably do what Evan suggested:

greg added a comment.Dec 2 2015, 6:25 PM

Independently, we could prioritize this upstream task: https://secure.phabricator.com/T1731 ("Allow users to set their VCS names"), which would address part of the problem. Only part because if no one had their vcs-name set to this email, then David would still be getting associated with it if we don't comment out the function as described int he previous comment.

greg lowered the priority of this task from High to Normal.Dec 2 2015, 6:31 PM
greg raised the priority of this task from Normal to High.
greg moved this task from To Triage to In Progress on the Gitblit-Deprecate board.
greg moved this task from In Progress to Backlog on the Gitblit-Deprecate board.
Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptJul 2 2016, 7:33 PM
Aklapper moved this task from Need discussion to Misc on the Phabricator board.

Does this still need to be high priority? There has not been a discussion on here since december 2015.

Qgil removed a subscriber: Qgil.Jul 3 2017, 3:41 PM

This should finally be resolved. Most miss-attributed commits have been cleaned up by running bin/repository rebuild-identities. Any remaining can be re-assigned by a phabricator admin. If you are aware of any please reopen or ping me on irc and I'll fix it up.

mmodell closed this task as Resolved.Mar 25 2019, 6:36 PM
mmodell claimed this task.

@mmodell: Should slightly related non-public T195061 also be closed?

@mmodell: Should slightly related non-public T195061 also be closed?

done