Skip to content

Commit

Permalink
Migrate Webhook secret to String Credential and fix issue with persis…
Browse files Browse the repository at this point in the history 
…tent for JCasC (#267)

* Webhook secret is deleted by JCasC on every restart #162

Use Jenkins credentials for webhook secret.

* Fix SpotBugs issues

* Add migration methods and rename variable

* Add readResolve and migrateWebhookSecretCredentials methods to be able
to migrate old secret tokens to Jenkins credentials.
* Rename secretTokenCredentialsId to webhookSecretCredentialsId

* Fix checkstyle error

* Rename help file

* Fix indentation
  • Loading branch information
@fredg02
fredg02 authored Mar 3, 2023
1 parent 62ab6fc commit b9560d6
Show file tree
Hide file tree
Showing 4 changed files with 148 additions and 18 deletions.
156 changes: 143 additions & 13 deletions src/main/java/io/jenkins/plugins/gitlabserverconfig/servers/GitLabServer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import com.cloudbees.plugins.credentials.CredentialsMatcher;
import com.cloudbees.plugins.credentials.CredentialsMatchers;
import com.cloudbees.plugins.credentials.CredentialsProvider;
import com.cloudbees.plugins.credentials.CredentialsScope;
import com.cloudbees.plugins.credentials.SystemCredentialsProvider;
import com.cloudbees.plugins.credentials.common.StandardCredentials;
import com.cloudbees.plugins.credentials.common.StandardListBoxModel;
import edu.umd.cs.findbugs.annotations.CheckForNull;
Expand All @@ -11,6 +13,8 @@
import hudson.Util;
import hudson.model.AbstractDescribableImpl;
import hudson.model.Descriptor;
import hudson.model.Item;
import hudson.model.ItemGroup;
import hudson.security.ACL;
import hudson.security.AccessControlled;
import hudson.util.FormValidation;
Expand All @@ -20,6 +24,8 @@
import java.net.MalformedURLException;
import java.net.URL;
import java.security.SecureRandom;
import java.util.Collections;
import java.util.List;
import java.util.logging.Level;
import java.util.logging.Logger;
import jenkins.model.Jenkins;
Expand All @@ -29,6 +35,8 @@
import org.gitlab4j.api.GitLabApi;
import org.gitlab4j.api.GitLabApiException;
import org.gitlab4j.api.models.User;
import org.jenkinsci.plugins.plaincredentials.StringCredentials;
import org.jenkinsci.plugins.plaincredentials.impl.StringCredentialsImpl;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.DoNotUse;
import org.kohsuke.accmod.restrictions.NoExternalUse;
Expand Down Expand Up @@ -126,8 +134,22 @@ public class GitLabServer extends AbstractDescribableImpl<GitLabServer> {

/**
* The secret token used while setting up hook url in the GitLab server
* @Deprecated Use webhookSecretCredentialsId instead
*/
private Secret secretToken;
private transient Secret secretToken;

/**
* The credentials id of the webhook secret token used while setting up hook url in the GitLab server
*/
@NonNull
private String webhookSecretCredentialsId;


/**
* The credentials matcher for StringCredentials
*/
public static final CredentialsMatcher WEBHOOK_SECRET_CREDENTIALS_MATCHER = CredentialsMatchers
.instanceOf(StringCredentials.class);

/**
* {@code true} if and only if Jenkins should trigger a build immediately on a
Expand Down Expand Up @@ -159,6 +181,7 @@ public GitLabServer(@NonNull String serverUrl, @NonNull String name,
? getRandomName()
: StringUtils.trim(name);
this.credentialsId = credentialsId;
this.webhookSecretCredentialsId = "";
}

/**
Expand Down Expand Up @@ -287,16 +310,53 @@ public String getHooksRootUrl() {
return Util.ensureEndsWith(Util.fixEmptyAndTrim(hooksRootUrl), "/");
}

@DataBoundSetter
@DataBoundSetter @Deprecated
public void setSecretToken(Secret token) {
this.secretToken = token;
}

// TODO: Use some UI element to trigger (what is the best way?)
private void generateSecretToken() {
byte[] random = new byte[16]; // 16x8=128bit worth of randomness, since we use md5 digest as the API token
RANDOM.nextBytes(random);
this.secretToken = Secret.decrypt(Util.toHexString(random));
@DataBoundSetter
public void setWebhookSecretCredentialsId(String token) {
this.webhookSecretCredentialsId = token;
}

public String getWebhookSecretCredentialsId() {
return webhookSecretCredentialsId;
}

/**
* Looks up for StringCredentials
*
* @return {@link StringCredentials}
*/
public StringCredentials getWebhookSecretCredentials(AccessControlled context) {
Jenkins jenkins = Jenkins.get();
if (context == null) {
jenkins.checkPermission(CredentialsProvider.USE_OWN);
return StringUtils.isBlank(webhookSecretCredentialsId) ? null : CredentialsMatchers.firstOrNull( lookupCredentials(
StringCredentials.class,
jenkins,
ACL.SYSTEM,
fromUri(defaultIfBlank(serverUrl, GITLAB_SERVER_URL)).build()
), withId(webhookSecretCredentialsId));
} else {
context.checkPermission(CredentialsProvider.USE_OWN);
if (context instanceof ItemGroup) {
return StringUtils.isBlank(webhookSecretCredentialsId) ? null : CredentialsMatchers.firstOrNull( lookupCredentials(
StringCredentials.class,
(ItemGroup) context,
ACL.SYSTEM,
fromUri(defaultIfBlank(serverUrl, GITLAB_SERVER_URL)).build()
), withId(webhookSecretCredentialsId));
} else {
return StringUtils.isBlank(webhookSecretCredentialsId) ? null : CredentialsMatchers.firstOrNull( lookupCredentials(
StringCredentials.class,
(Item) context,
ACL.SYSTEM,
fromUri(defaultIfBlank(serverUrl, GITLAB_SERVER_URL)).build()
), withId(webhookSecretCredentialsId));
}
}
}

/**
Expand All @@ -307,15 +367,63 @@ public DescriptorImpl getDescriptor() {
return (DescriptorImpl) super.getDescriptor();
}

@Deprecated
public Secret getSecretToken() {
return secretToken;
}

private StringCredentials getWebhookSecretCredentials(String webhookSecretCredentialsId) {
Jenkins jenkins = Jenkins.get();
jenkins.checkPermission(Jenkins.ADMINISTER);
return StringUtils.isBlank(webhookSecretCredentialsId) ? null : CredentialsMatchers.firstOrNull(
lookupCredentials(StringCredentials.class, jenkins),
withId(webhookSecretCredentialsId)
);
}

public String getSecretTokenAsPlainText() {
if (this.secretToken == null) {
StringCredentials credentials = getWebhookSecretCredentials(webhookSecretCredentialsId);
String secretToken = "";
if (credentials != null) {
secretToken = credentials.getSecret().getPlainText();
} else {

This comment has been minimized.

Sign in to view

Copy link
@natsen

natsen Mar 20, 2023 •
edited
Loading

@fredg02
Hi,
for existing users that are configuring webhooks using secretToken="" the new change breaks the setup. This happens because

  1. secretToken is no more a valid parameter
  2. and removing secretToken="" to avoid using deprecated secretToken is not working as well. This is likely due to this else block which doesn't default secretToken to empty but instead returns null.

The likely workaround is to setup a credential with empty token and refer that id in the webhookSecretCredentialsId but it is preferable that the default behavior uses empty value for secretToken in which case user doesn't need to configure a credentials for empty token. What do you think. Let me know if I should create a new bug request.

This comment has been minimized.

Sign in to view

Copy link
@qerub

qerub Aug 16, 2023

Related issue: #106
return null;
}
return this.secretToken.getPlainText();
return secretToken;
}

private Object readResolve() {
if(StringUtils.isBlank(webhookSecretCredentialsId) && secretToken != null) {
migrateWebhookSecretCredentials();
}
return this;
}

/**
* Migrate webhook secret token to Jenkins credentials
*/
private void migrateWebhookSecretCredentials() {
final List<StringCredentials> credentials =
CredentialsProvider.lookupCredentials(StringCredentials.class, Jenkins.get(), ACL.SYSTEM, Collections.emptyList());
for (final StringCredentials cred : credentials) {
if (StringUtils.equals(secretToken.getPlainText(), Secret.toString(cred.getSecret()))) {
// If a credential has the same secret, use it.
webhookSecretCredentialsId = cred.getId();
break;
}
}
if (StringUtils.isBlank(webhookSecretCredentialsId)) {
// If we couldn't find any existing credentials, create new credential
final StringCredentials newCredentials =
new StringCredentialsImpl(
CredentialsScope.GLOBAL,
null,
"Migrated from gitlab-branch-source-plugin webhook secret",
secretToken);
SystemCredentialsProvider.getInstance().getCredentials().add(newCredentials);
webhookSecretCredentialsId = newCredentials.getId();
}
secretToken = null;
}

/**
Expand Down Expand Up @@ -485,13 +593,11 @@ public FormValidation doTestConnection(@QueryParameter String serverUrl,
try {
User user = gitLabApi.getUserApi().getCurrentUser();
LOGGER.log(Level.FINEST, String
.format("Connection established with the GitLab Server for %s",
user.getUsername()));
.format("Connection established with the GitLab Server for %s", user.getUsername()));
return FormValidation
.ok(String.format("Credentials verified for user %s", user.getUsername()));
} catch (GitLabApiException e) {
LOGGER.log(Level.SEVERE,
"Failed to connect with GitLab Server - %s", e.getMessage());
LOGGER.log(Level.SEVERE, "Failed to connect with GitLab Server - %s", e.getMessage());
return FormValidation.error(e,
Messages.GitLabServer_failedValidation(Util.escape(e.getMessage())));
}
Expand Down Expand Up @@ -522,6 +628,30 @@ public ListBoxModel doFillCredentialsIdItems(@QueryParameter String serverUrl,
CREDENTIALS_MATCHER);
}

/**
* Stapler form completion.
*
* @param webhookSecretCredentialsId the webhook secret credentials Id
* @return the available credentials.
*/
@Restricted(NoExternalUse.class) // stapler
@SuppressWarnings("unused")
public ListBoxModel doFillWebhookSecretCredentialsIdItems(@QueryParameter String serverUrl,
@QueryParameter String webhookSecretCredentialsId) {
Jenkins jenkins = Jenkins.get();
if (!jenkins.hasPermission(Jenkins.ADMINISTER)) {
return new StandardListBoxModel().includeCurrentValue(webhookSecretCredentialsId);
}
return new StandardListBoxModel()
.includeEmptyValue()
.includeMatchingAs(ACL.SYSTEM,
jenkins,
StringCredentials.class,
fromUri(serverUrl).build(),
WEBHOOK_SECRET_CREDENTIALS_MATCHER
);
}

private PersonalAccessToken getCredentials(String serverUrl, String credentialsId) {
Jenkins jenkins = Jenkins.get();
jenkins.checkPermission(Jenkins.ADMINISTER);
Expand Down
4 changes: 2 additions & 2 deletions src/main/resources/io/jenkins/plugins/gitlabserverconfig/servers/GitLabServer/config.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ f.entry(title: _("System Hook"), field: "manageSystemHooks", "description": "Do
f.checkbox(title: _("Manage System Hooks"))
}

f.entry(title: _("Secret Token"), field: "secretToken", "description": "The secret token used while setting up hook url in the GitLab server") {
f.password()
f.entry(title: _("Secret Token"), field: "webhookSecretCredentialsId", "description": "The secret token used while setting up hook url in the GitLab server") {
c.select(context: app)
}

f.entry(title: _("Root URL for hooks"), field: "hooksRootUrl", "description": "Jenkins root URL to use in hooks URL (if different from the public Jenkins root URL)") {
Expand Down
3 changes: 0 additions & 3 deletions ...esources/io/jenkins/plugins/gitlabserverconfig/servers/GitLabServer/help-secretToken.html
View file

This file was deleted.

3 changes: 3 additions & 0 deletions ...kins/plugins/gitlabserverconfig/servers/GitLabServer/help-webhookSecretCredentialsId.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<div>
The secret token is required to authenticate the webhook payloads received from a GitLab Server. Use generate secret token from Advanced options or use your own. If you are an old plugin user and did not set a secret token previously and want the secret token to be applied to the hooks of your existing jobs, you can add the secret token and rescan your jobs. Existing hooks with new secret token will be applied.
</div>

0 comments on commit b9560d6

Please sign in to comment.