[JENKINS-70746] Fix missing permission error when processing changes

[fredg02]

This commit removes the permission check when accessing the webhook secret. Since this can be called from a webhook or systemhook trigger which have no permission context, there is no way (that I know of) to set the right context.

The regression was introduced by #267.

This fixes #286 and related https://issues.jenkins.io/browse/JENKINS-70746.

I have tested this successfully in our production environment.

• Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
• Ensure that the pull request title represents the desired changelog entry
• Please describe what you did
• Link to relevant issues in GitHub or Jira
• Link to relevant pull requests, esp. upstream and downstream changes
• Ensure you have provided tests - that demonstrates feature works or fixes the issue

[fredg02]

@MarkEWaite a review would be appreciated.

[harbulot]

Just a bit of feedback to say I've just tested it on a local installation, and it seems to work. Thank you!

[MarkEWaite]

I confirmed that I can duplicate the problem in the 646 release and confirmed that this resolves the problem. Thanks very much.

Could you explain the last argument to the lookupCredentials? I'm not clear why the credentials being read need to be a subclass of the domain credentials. I approve based on my interactive testing but would like to better understand that condition.

Thanks very much for the fast fix!

[MarkEWaite]

As an optional comment, it is easier for readers of the changelog if you change the title of the pull request to provide more details about the issue being fixed. I would propose:

[JENKINS-70746] Fix missing permission error when processing changes

[fredg02]

@MarkEWaite thanks for the timely review!

Could you explain the last argument to the lookupCredentials? I'm not clear why the credentials being read need to be a subclass of the domain credentials. I approve based on my interactive testing but would like to better understand that condition.

The intention was simply to remove the warning about the deprecated lookupCredentials method. So there might be better ways to do this.

As an optional comment, it is easier for readers of the changelog if you change the title of the pull request to provide more details about the issue being fixed.

You are absolutely right. I used to know this. 😬

[fredg02]

@jetersen friendly ping

[fredg02]

@jetersen it would be nice to release this regression fix soon, since it affects all user wanting to upgrade to the latest version. Do you have an ETA for the next release?

[jetersen]

LGTM

[fredg02]

@jetersen Thanks!