Migrate Webhook secret to String Credential and fix issue with persistent for JCasC

[fredg02]

Use Jenkins credentials for webhook secret.

fixes #162
fixes #283

• 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]

@jetersen is there anything I can do to get this merged in the foreseeable future? This affects a lot of our projects hosted at gitlab.eclipse.org.

[fredg02]

@mifitous can you take a look at this pull request? It would help us a lot if this can be fixed soon.

[fredg02]

I've kept the getter and setter for now.

I've manually tested the migrate method with the following scenarios:

• migration of webhook secret to Jenkins credential
  • Jenkins credential is created correctly ✅
  • content of new Jenkins credential == old webhook secret ✅
• credential (with a different name) already exists
  • worked (existing credential was selected in the GitLab Branch Source config) ✅
• dev version is installed without an existing config
  • no errors ✅

Not sure if it's necessary to create mock tests for the migrate method.

[jetersen]

@fredg02 Mind resolving the merge conflict? :)

[fredg02]

@fredg02 Mind resolving the merge conflict? :)

Done. I did a rebase and added a commit to fix indentation.

[fredg02]

@jetersen please let me know if anything is required from my side to push this forward.

[fredg02]

@jetersen friendly ping

[jetersen]

Fixed in https://github.com/jenkinsci/gitlab-branch-source-plugin/releases/tag/646.vb_9560d64b_69f

[fredg02]

Thanks for all your help! 🙏

[bguerin]

Well ... with these changes, if I configure a token, I got
#286
which is because this line
https://github.com/jenkinsci/gitlab-branch-source-plugin/blob/master/src/main/java/io/jenkins/plugins/gitlabserverconfig/servers/GitLabServer.java#L377

and if I do not configure token, I got a NullPointerException here ( str2 is null)
https://github.com/jenkinsci/gitlab-branch-source-plugin/blob/master/src/main/java/io/jenkins/plugins/gitlabbranchsource/GitLabHookCreator.java#L231

So, I am stuck ...

[jetersen]

@bguerin PRs are welcome

[fredg02]

@jetersen Would you consider it safe to remove

        Jenkins jenkins = Jenkins.get();
        jenkins.checkPermission(Jenkins.ADMINISTER);

https://github.com/jenkinsci/gitlab-branch-source-plugin/blob/master/src/main/java/io/jenkins/plugins/gitlabserverconfig/servers/GitLabServer.java#L376-L377 ?

[jetersen]

@fredg02 I believe the correct thing is this:

gitlab-branch-source-plugin/src/main/java/io/jenkins/plugins/gitlabserverconfig/servers/GitLabServer.java

Lines 278 to 291 in b9560d6

	public PersonalAccessToken getCredentials(AccessControlled context) {
	Jenkins jenkins = Jenkins.get();
	if (context == null) {
	jenkins.checkPermission(CredentialsProvider.USE_OWN);
	} else {
	context.checkPermission(CredentialsProvider.USE_OWN);
	}
	return StringUtils.isBlank(credentialsId) ? null
	: CredentialsMatchers.firstOrNull(lookupCredentials(
	PersonalAccessToken.class,
	jenkins,
	ACL.SYSTEM,
	fromUri(defaultIfBlank(serverUrl, GITLAB_SERVER_URL)).build()), withId(credentialsId));
	}

You need to supply the source.getOwner() as done here:

gitlab-branch-source-plugin/src/main/java/io/jenkins/plugins/gitlabbranchsource/GitLabHookCreator.java

Line 74 in b9560d6

credentials = server.getCredentials(source.getOwner());

[fredg02]

Yeah, I tried to copy that part, but I'm not sure how I can provide the context as parameter.

[jetersen]

Either source.GetOwner() or observer.getContext() it is used in other places.

Just check for usage on getCredentials

Otherwise check for AccessControlled usage

[fredg02]

Alright. I'll try to get this fixed in a timely manner. Sorry about the regression my PR introduced.

[fredg02]

@jetersen any idea how to aquire context to pass it to server.getSecretTokenAsPlainText() in:
https://github.com/jenkinsci/gitlab-branch-source-plugin/blob/master/src/main/java/io/jenkins/plugins/gitlabbranchsource/GitLabWebHookAction.java#L89
and
https://github.com/jenkinsci/gitlab-branch-source-plugin/blob/master/src/main/java/io/jenkins/plugins/gitlabbranchsource/GitLabSystemHookAction.java#L89
?

[fredg02]

I've tried to check how other plugins handle this (e.g. the GitHub plugin). It looks like there is no permissions check in https://github.com/jenkinsci/github-plugin/blob/master/src/main/java/org/jenkinsci/plugins/github/config/GitHubServerConfig.java#L288.

[jetersen]

Feel free to submit a PR :)

[MarkEWaite]

@fredg02 I believe that the regression has also been reported as JENKINS-70785. Are you willing to investigate a fix for that regression or is that an intentional case and the user needs to perform some specific upgrade step to change from secretToken to another way of defining the credential in configuration as code?

[fredg02]

The regression is not intentional. I will try to provide a fix asap. I had only limited connectivity last week.