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

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

4 answers

2 votes
Bryan Turner Atlassian Team Mar 31, 2014

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

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.

Thanks Bryan, thats useful.

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

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

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

Suggest an answer

Log in or Sign up to answer
Atlassian Community Anniversary

Happy Anniversary, Atlassian Community!

This community is celebrating its one-year anniversary and Atlassian co-founder Mike Cannon-Brookes has all the feels.

Read more
Community showcase
Bridget Sauer
Published yesterday in Marketplace Apps

Calling all developers––You're invited to Atlas Camp 2018

 Atlas Camp   is our developer event which will take place in Barcelona, Spain  from the 6th -7th of   September . This is a great opportunity to meet other developers and get n...

71 views 0 5
Read article

Atlassian User Groups

Connect with like-minded Atlassian users at free events near you!

Find a group

Connect with like-minded Atlassian users at free events near you!

Find my local user group

Unfortunately there are no AUG chapters near you at the moment.

Start an AUG

You're one step closer to meeting fellow Atlassian users at your local meet up. Learn more about AUGs

Groups near you