Skip to content

Commit

Permalink
[SECURITY-1546] fix client secret is saved in plain text
Browse files Browse the repository at this point in the history 
https://issues.jenkins-ci.org/browse/SECURITY-1546
  • Loading branch information
@mallowlabs
mallowlabs committed Oct 14, 2019
1 parent 52e2c97 commit f55d222
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 13 deletions.
50 changes: 38 additions & 12 deletions src/main/java/org/jenkinsci/plugins/BitbucketSecurityRealm.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import java.util.logging.Level;
import java.util.logging.Logger;

import jenkins.security.SecurityListener;
import org.acegisecurity.Authentication;
import org.acegisecurity.AuthenticationException;
import org.acegisecurity.AuthenticationManager;
Expand Down Expand Up @@ -40,21 +39,26 @@
import hudson.security.GroupDetails;
import hudson.security.SecurityRealm;
import hudson.security.UserMayOrMayNotExistException;
import hudson.util.Secret;
import jenkins.model.Jenkins;
import jenkins.security.SecurityListener;

public class BitbucketSecurityRealm extends SecurityRealm {

private static final String REFERER_ATTRIBUTE = BitbucketSecurityRealm.class.getName() + ".referer";
private static final Logger LOGGER = Logger.getLogger(BitbucketSecurityRealm.class.getName());

private String clientID;
@Deprecated
private String clientSecret;
private Secret secretClientSecret;

@DataBoundConstructor
public BitbucketSecurityRealm(String clientID, String clientSecret) {
public BitbucketSecurityRealm(String clientID, String clientSecret, Secret secretClientSecret) {
super();
this.clientID = Util.fixEmptyAndTrim(clientID);
this.clientSecret = Util.fixEmptyAndTrim(clientSecret);
this.secretClientSecret = secretClientSecret;
}

public BitbucketSecurityRealm() {
Expand All @@ -70,8 +74,7 @@ public String getClientID() {
}

/**
* @param clientID
* the clientID to set
* @param clientID the clientID to set
*/
public void setClientID(String clientID) {
this.clientID = clientID;
Expand All @@ -80,18 +83,37 @@ public void setClientID(String clientID) {
/**
* @return the clientSecret
*/
@Deprecated
public String getClientSecret() {
return clientSecret;
}

/**
* @param clientSecret
* the clientSecret to set
* @param clientSecret the clientSecret to set
*/
@Deprecated
public void setClientSecret(String clientSecret) {
this.clientSecret = clientSecret;
}

/**
* @return the secretClientSecret
*/
public Secret getSecretClientSecret() {
// for backward compatibility
if (StringUtils.isNotEmpty(clientSecret)) {
return Secret.fromString(clientSecret);
}
return secretClientSecret;
}

/**
* @param secretClientSecret the secretClientSecret to set
*/
public void setSecretClientSecret(Secret secretClientSecret) {
this.secretClientSecret = secretClientSecret;
}

public HttpResponse doCommenceLogin(StaplerRequest request, @Header("Referer") final String referer)
throws IOException {

Expand All @@ -107,7 +129,7 @@ public HttpResponse doCommenceLogin(StaplerRequest request, @Header("Referer") f
}
String callback = rootUrl + "/securityRealm/finishLogin";

BitbucketApiService bitbucketApiService = new BitbucketApiService(clientID, clientSecret, callback);
BitbucketApiService bitbucketApiService = new BitbucketApiService(clientID, getSecretClientSecret().getPlainText(), callback);

return new HttpRedirect(bitbucketApiService.createAuthorizationCodeURL(null));
}
Expand All @@ -120,11 +142,13 @@ public HttpResponse doFinishLogin(StaplerRequest request) throws IOException {
return HttpResponses.redirectToContextRoot();
}

Token accessToken = new BitbucketApiService(clientID, clientSecret).getTokenByAuthorizationCode(code, null);
String rawClientSecret = getSecretClientSecret().getPlainText();

Token accessToken = new BitbucketApiService(clientID, rawClientSecret).getTokenByAuthorizationCode(code, null);

if (!accessToken.isEmpty()) {

BitbucketAuthenticationToken auth = new BitbucketAuthenticationToken(accessToken, clientID, clientSecret);
BitbucketAuthenticationToken auth = new BitbucketAuthenticationToken(accessToken, clientID, rawClientSecret);
SecurityContextHolder.getContext().setAuthentication(auth);

User u = User.current();
Expand Down Expand Up @@ -177,7 +201,7 @@ public UserDetails loadUserByUsername(String username) {
if (!(token instanceof BitbucketAuthenticationToken)) {
throw new UserMayOrMayNotExistException("Unexpected authentication type: " + token);
}
result = new BitbucketApiService(clientID, clientSecret).getUserByUsername(username);
result = new BitbucketApiService(clientID, getSecretClientSecret().getPlainText()).getUserByUsername(username);
if (result == null) {
throw new UsernameNotFoundException("User does not exist for login: " + username);
}
Expand Down Expand Up @@ -217,8 +241,8 @@ public void marshal(Object source, HierarchicalStreamWriter writer, MarshallingC
writer.setValue(realm.getClientID());
writer.endNode();

writer.startNode("clientSecret");
writer.setValue(realm.getClientSecret());
writer.startNode("secretClientSecret");
writer.setValue(realm.getSecretClientSecret().getEncryptedValue());
writer.endNode();
}

Expand Down Expand Up @@ -268,6 +292,8 @@ private void setValue(BitbucketSecurityRealm realm, String node, String value) {
realm.setClientID(value);
} else if (node.equalsIgnoreCase("clientsecret")) {
realm.setClientSecret(value);
} else if (node.equalsIgnoreCase("secretclientsecret")) {
realm.setSecretClientSecret(Secret.fromString(value));
} else {
throw new ConversionException("invalid node value = " + node);
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/resources/org/jenkinsci/plugins/BitbucketSecurityRealm/config.jelly
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<f:textbox />
</f:entry>

<f:entry title="Client Secret" field="clientSecret" help="/plugin/bitbucket-oauth/help/realm/client-secret-help.html">
<f:entry title="Client Secret" field="secretClientSecret" help="/plugin/bitbucket-oauth/help/realm/client-secret-help.html">
<f:password />
</f:entry>
</f:section>
Expand Down

0 comments on commit f55d222

Please sign in to comment.