Enforce Push Author may break with latest version of Stash (2.12) the Require matching display name option does not seem to work

[Dan]

The messages says:

remote: -----------------------------------------------------

remote: REJECTED: you can only push commits you have authored

remote: -----------------------------------------------------

remote: required name: {Display Name}

remote: required email: {user.email}

remote:

remote: The following commits do not match:

remote:

remote: a0384c71ca6a9d4c31abf40550262db4edeb1335 - {user.name} <{user.email}>

remote: 32532e8af35b0d0ddb74f00ce493fb916c39e856 - {user.name} <{user.email}>

remote: cdbb20360fdaad37d92ac66a9426839be0018ff1 - {user.name} <{user.email}>

Even though the commits it complains about where committed with the users correct display name.

Turning of this off allows the plugin to work but this is just a workaround as we do want it to make that check.

Stash has made some changes with the UI in regards to "user name linking to profile page", I wonder if the API or values returned from the API have been slightly modified.

https://confluence.atlassian.com/display/STASH/Releases

[Bryan Turner]

Dan,

As you've mentioned, Stash 2.12 includes user linking. This means Changeset.getAuthor() and Blame.getAuthor() now return StashUser instances where a Stash user can be found with an e-mail address that matches the committer's e-mail address.

Person.getName(), when un-matched, is whatever value the committer has set for their "user.name" value in git. In my case, that's "Bryan Turner"

StashUser.getName() is your username. In my case that's "bturner".

This change means that Changeset.getAuthor().getName() returns a different value depending on whether the committer was matched to a StashUser or not. That's likely the reason your hook is no longer accepting "valid" data.

One option would be to change the hook to detect when Changeset.getAuthor() returns a StashUser and compare the display names. A second, likely simpler, option would be to just require that Changeset.getAuthor() be a StashUser. And not just any StashUser; the same StashUser as is returned by StashAuthenticationContext.getCurrentUser(). You could use the StashUserEquality utility, or just compare getName(), or getId(). However you choose to do the comparison, if the changeset's author matches the current user, it's going to show up with the right data in the UI.

For a matched user, there is no way to access the raw SCM data. That data will never be shown on any screen in Stash; the user's details from their StashUser will be displayed instead. Our belief is that this is generally desirable. If a user's name changes in LDAP, all of their commits in Stash repositories will automatically show the new name. Based on that, it seems like, from 2.12 on, validating the actual "user.name" in the SCM makes little difference; any incorrect value in the SCM will be replaced when the commit is viewed online.

Hope this helps,
Bryan Turner
Atlassian Stash

[Landon Fuller]

Not having access to the raw data is a bit of a bummer; even if the incorrect name is replaced when viewed in Stash, it's still incorrect in the actual Git repository.

[edit] In putting together a patch for a plugin we're using, it occured to me that this is worse than an API breaking annoyance -- this change violates the Liskov Substitution Principal, in that StashUser.getName() is not actually compatible with the Person.getName() interface it claims to implement, and is not thus not safe to return from Changeset.getAuthor():

• Code that relies on Person.getName() must now use instanceof to check for a StashUser return type, and instead call getDisplayName() (or cast and use StashUserEquality)
• It's now impossible for git hooks to validate the canonical "user.name" data that will actually be written to the repository (perhaps we can work around this by using jgit to manually open the repository and look at the refs?)

I don't know if it's too late to walk back this API change, but it seems like Changeset could be modified to vend a "getStashUser" method, rather than overloading "getAuthor" with an incompatible return type.

[Landon Fuller]

To fix this locally, I made use of JGit to fetch the actual committer name directly from the repository:

https://github.com/sford/yet-another-commit-checker/pull/6/files

[Robin Krom]

There are multiple add-ons which have issues with this "behavioral" change, Bryans answer should help the developers to find a solution.

Still to make sure the next reader has all the needed information, here is a link to the JIRA for the change mentioned by Bryan: https://jira.atlassian.com/browse/STASH-3235

[Not CK]

Thanks Bryan, thats useful.

Rising Oak LLC, any intention of updating based on Bryan's recomendation?