Update the plugin towards the 1.6.0 release

Jenkinsfile

@@ -0,0 +1,2 @@
+// Build the plugin using https://github.com/jenkins-infra/pipeline-library
+buildPlugin()

pom.xml

@@ -3,15 +3,20 @@
   <parent>
     <groupId>org.jenkins-ci.plugins</groupId>
     <artifactId>plugin</artifactId>
-    <version>2.37</version>
+    <version>3.2</version>
   </parent>
 
   <artifactId>reverse-proxy-auth-plugin</artifactId>
-  <version>1.5.1-SNAPSHOT</version>
+  <version>1.6.0-SNAPSHOT</version>
   <packaging>hpi</packaging>
 
+  <properties>
+    <jenkins.version>1.625.3</jenkins.version>
+    <java.level>7</java.level>
+  </properties>
+
   <name>Jenkins Reverse Proxy Auth Plugin</name>
-  <url>http://wiki.jenkins-ci.org/display/JENKINS/Reverse+Proxy+Auth+Plugin</url>
+  <url>https://wiki.jenkins.io/display/JENKINS/Reverse+Proxy+Auth+Plugin</url>
 
   <developers>
     <developer>
@@ -24,12 +29,23 @@
       <name>Wilder Rodrigues</name>
       <email>[email protected]</email>
     </developer>
+    <developer>
+      <id>oleg_nenashev</id>
+      <name>Oleg Nenashev</name>
+    </developer>
+    <developer>
+      <id>wfollonier</id>
+      <name>Wadeck Follonier</name>
+    </developer>
   </developers>
 
   <scm>
     <connection>scm:git:git://github.com/jenkinsci/${project.artifactId}.git</connection>
     <developerConnection>scm:git:[email protected]:jenkinsci/${project.artifactId}.git</developerConnection>
+    <url>https://github.com/jenkinsci/${project.artifactId}</url>
+    <tag>HEAD</tag>
   </scm>
+
     <dependencies>
         <dependency>
             <groupId>org.jenkins-ci.plugins</groupId>
@@ -46,21 +62,14 @@
   <repositories>
     <repository>
       <id>repo.jenkins-ci.org</id>
-      <url>http://repo.jenkins-ci.org/public/</url>
+      <url>https://repo.jenkins-ci.org/public/</url>
     </repository>
   </repositories>
 
-  <distributionManagement>
-    <repository>
-      <id>maven.jenkins-ci.org</id>
-      <url>http://maven.jenkins-ci.org:8081/content/repositories/releases/</url>
-    </repository>
-  </distributionManagement>
-
   <pluginRepositories>
     <pluginRepository>
        <id>repo.jenkins-ci.org</id>
-       <url>http://repo.jenkins-ci.org/public/</url>
+       <url>https://repo.jenkins-ci.org/public/</url>
     </pluginRepository>
   </pluginRepositories>
 </project>  

src/main/java/org/jenkinsci/plugins/reverse_proxy_auth/ReverseProxySecurityRealm.java

@@ -26,10 +26,11 @@
 import static hudson.Util.fixEmpty;
 import static hudson.Util.fixEmptyAndTrim;
 import static hudson.Util.fixNull;
+
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import groovy.lang.Binding;
 import hudson.Extension;
 import hudson.model.Descriptor;
-import hudson.model.Hudson;
 import hudson.model.User;
 import hudson.security.*;
 import hudson.tasks.Mailer;
@@ -42,6 +43,7 @@
 import java.io.FileInputStream;
 import java.io.FileNotFoundException;
 import java.io.IOException;
+import java.io.InputStream;
 import java.net.InetAddress;
 import java.net.Socket;
 import java.net.UnknownHostException;
@@ -57,6 +59,8 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import javax.annotation.CheckForNull;
+import javax.annotation.Nullable;
 import javax.naming.Context;
 import javax.naming.NamingException;
 import javax.naming.directory.Attribute;
@@ -101,6 +105,7 @@
 import org.kohsuke.stapler.QueryParameter;
 import org.kohsuke.stapler.StaplerRequest;
 import org.springframework.dao.DataAccessException;
+import org.springframework.dao.DataAccessResourceFailureException;
 import org.springframework.web.context.WebApplicationContext;
 
 /**
@@ -117,6 +122,7 @@ public class ReverseProxySecurityRealm extends SecurityRealm {
 	 * See http://msdn.microsoft.com/en-us/library/aa746475(VS.85).aspx for the syntax by example.
 	 * WANTED: The specification of the syntax.
 	 */
+	@SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "May be used in system groovy scripts")
 	public static String GROUP_SEARCH = System.getProperty(LDAPSecurityRealm.class.getName()+".groupSearch",
 			"(& (cn={0}) (| (objectclass=groupOfNames) (objectclass=groupOfUniqueNames) (objectclass=posixGroup)))");
 
@@ -240,6 +246,7 @@ public class ReverseProxySecurityRealm extends SecurityRealm {
 	/**
 	 * The name of the header which the username has to be extracted from.
 	 */
+	@CheckForNull
 	public final String forwardedUser;
 
 	/**
@@ -339,6 +346,7 @@ public String getHeaderGroupsDelimiter() {
 		return headerGroupsDelimiter;
 	}
 
+	@CheckForNull
 	public String getServerUrl() {
 		if (server == null) {
 			return null;
@@ -391,6 +399,7 @@ private String inferRootDN(String server) {
 				props.put(Context.SECURITY_CREDENTIALS, getManagerPassword());
 			}
 			props.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory");
+			//TODO: should it pass null instead and check the result?
 			props.put(Context.PROVIDER_URL, toProviderUrl(fixNull(getServerUrl()), ""));
 
 			DirContext ctx = new InitialDirContext(props);
@@ -412,7 +421,8 @@ private String inferRootDN(String server) {
 		}
 	}
 
-	public static String toProviderUrl(String serverUrl, String rootDN) {
+	@Nullable
+	public static String toProviderUrl(@CheckForNull String serverUrl, @CheckForNull String rootDN) {
 		if (serverUrl == null) {
 			return null;
 		}
@@ -474,15 +484,15 @@ public void doFilter(ServletRequest request,
 
 				String userFromHeader = null;
 
-				Authentication auth = Hudson.ANONYMOUS;
+				Authentication auth = Jenkins.ANONYMOUS;
 				if ((forwardedUser != null && (userFromHeader = r.getHeader(forwardedUser)) != null) || userFromApiToken != null) {
 					LOGGER.log(Level.FINE, "USER LOGGED IN: {0}", userFromHeader);
-			        if (userFromHeader == null && userFromApiToken != null) {
+			        if (userFromHeader == null) {
 				        userFromHeader = userFromApiToken;
 					}
 
                     if (authContext == null) {
-                        authContext = new Hashtable<String, GrantedAuthority[]>();
+                        authContext = new Hashtable<>();
                     }
 
 					if (getLDAPURL() != null) {
@@ -575,11 +585,11 @@ public String getPostLogOutUrl(StaplerRequest req, Authentication auth) {
 	}
 
 	@Override
-	public SecurityComponents createSecurityComponents() {
+	public SecurityComponents createSecurityComponents() throws DataAccessException {
 		Binding binding = new Binding();
 		binding.setVariable("instance", this);
 
-		BeanBuilder builder = new BeanBuilder(Jenkins.getInstance().pluginManager.uberClassLoader);
+		BeanBuilder builder = new BeanBuilder(Jenkins.getActiveInstance().pluginManager.uberClassLoader);
 
 		String fileName;
 		if (getLDAPURL() != null) {
@@ -588,12 +598,17 @@ public SecurityComponents createSecurityComponents() {
 			fileName = "ReverseProxySecurityRealm.groovy";
 		}
 
-		try {
-			File override = new File(Jenkins.getInstance().getRootDir(), fileName);
-			builder.parse(override.exists() ? new AutoCloseInputStream(new FileInputStream(override)) :
-				getClass().getResourceAsStream(fileName), binding);
-		} catch (FileNotFoundException e) {
-			throw new Error("Failed to load "+fileName,e);
+		File override = new File(Jenkins.getActiveInstance().getRootDir(), fileName);
+		try(InputStream istream = override.exists()
+				? new FileInputStream(override)
+				: getClass().getResourceAsStream(fileName)) {
+			if (istream == null) {
+				throw new FileNotFoundException("Cannot find resource " + fileName);
+			}
+			builder.parse(istream, binding);
+		} catch (IOException e) {
+			// loadUserByUsername() declares DataAccessException to be thrown, so it is better than the Error which was thrown before 1.6.0
+			throw new DataAccessResourceFailureException("Failed to load "+fileName, e);
 		}
 		WebApplicationContext appContext = builder.createApplicationContext();
 
@@ -722,7 +737,12 @@ public FormValidation doServerCheck(
 				@QueryParameter final String managerDN,
 				@QueryParameter final String managerPassword) {
 
-			if (!Jenkins.getInstance().hasPermission(Jenkins.ADMINISTER)) {
+			final String trimmedServer = fixEmptyAndTrim(server);
+			if (trimmedServer == null) {
+				return FormValidation.error("Server is null or empty");
+			}
+
+			if (!Jenkins.getActiveInstance().hasPermission(Jenkins.ADMINISTER)) {
 				return FormValidation.ok();
 			}
 
@@ -736,15 +756,15 @@ public FormValidation doServerCheck(
 				}
 
 				props.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory");
-				props.put(Context.PROVIDER_URL, toProviderUrl(server, ""));
+				props.put(Context.PROVIDER_URL, toProviderUrl(trimmedServer, ""));
 
 
 				DirContext ctx = new InitialDirContext(props);
 				ctx.getAttributes("");
 				return FormValidation.ok();   // connected
 			} catch (NamingException e) {
 				// trouble-shoot
-				Matcher m = Pattern.compile("(ldaps?://)?([^:]+)(?:\\:(\\d+))?(\\s+(ldaps?://)?([^:]+)(?:\\:(\\d+))?)*").matcher(server.trim());
+				Matcher m = Pattern.compile("(ldaps?://)?([^:]+)(?:\\:(\\d+))?(\\s+(ldaps?://)?([^:]+)(?:\\:(\\d+))?)*").matcher(trimmedServer.trim());
 				if(!m.matches())
 					return FormValidation.error(hudson.security.Messages.LDAPSecurityRealm_SyntaxOfServerField());
 
@@ -758,12 +778,12 @@ public FormValidation doServerCheck(
 				} catch (UnknownHostException x) {
 					return FormValidation.error(hudson.security.Messages.LDAPSecurityRealm_UnknownHost(x.getMessage()));
 				} catch (IOException x) {
-					return FormValidation.error(x,hudson.security.Messages.LDAPSecurityRealm_UnableToConnect(server, x.getMessage()));
+					return FormValidation.error(x,hudson.security.Messages.LDAPSecurityRealm_UnableToConnect(trimmedServer, x.getMessage()));
 				}
 
 				// otherwise we don't know what caused it, so fall back to the general error report
 				// getMessage() alone doesn't offer enough
-				return FormValidation.error(e,hudson.security.Messages.LDAPSecurityRealm_UnableToConnect(server, e));
+				return FormValidation.error(e,hudson.security.Messages.LDAPSecurityRealm_UnableToConnect(trimmedServer, e));
 			} catch (NumberFormatException x) {
 				// The getLdapCtxInstance method throws this if it fails to parse the port number
 				return FormValidation.error(hudson.security.Messages.LDAPSecurityRealm_InvalidPortNumber());

src/main/java/org/jenkinsci/plugins/reverse_proxy_auth/auth/DefaultReverseProxyAuthenticator.java

@@ -1,8 +1,10 @@
 package org.jenkinsci.plugins.reverse_proxy_auth.auth;
 
+import java.util.Arrays;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.acegisecurity.AcegiMessageSource;
 import org.acegisecurity.Authentication;
 import org.acegisecurity.GrantedAuthority;
@@ -16,6 +18,9 @@
 import org.springframework.context.support.MessageSourceAccessor;
 import org.springframework.util.Assert;
 
+import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
+
 
 /**
  * @author Wilder Rodrigues ([email protected])
@@ -25,17 +30,18 @@ public class DefaultReverseProxyAuthenticator implements ReverseProxyAuthenticat
 	private static final Logger LOGGER = Logger
 			.getLogger(ReverseProxySecurityRealm.class.getName());
 
+	@SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD", justification = "It is a part of public API :(")
 	protected MessageSourceAccessor messages = AcegiMessageSource.getAccessor();
 
 	private final String username;
 	private final GrantedAuthority[] authorities;
 
-	public DefaultReverseProxyAuthenticator(String username, GrantedAuthority[] authorities) {
+	public DefaultReverseProxyAuthenticator(String username, @CheckForNull GrantedAuthority[] authorities) {
 		this.username = username;
-		this.authorities = authorities;
+		this.authorities = authorities != null ? Arrays.copyOf(authorities, authorities.length) : null;
 	}
 
-	public void setMessageSource(MessageSource messageSource) {
+	public void setMessageSource(@Nonnull MessageSource messageSource) {
 		Assert.notNull("Message source must not be null");
 		messages = new MessageSourceAccessor(messageSource);
 	}

src/main/java/org/jenkinsci/plugins/reverse_proxy_auth/auth/DefaultReverseProxyAuthoritiesPopulator.java

@@ -55,14 +55,15 @@ public class DefaultReverseProxyAuthoritiesPopulator implements ReverseProxyAuth
 	private String rolePrefix = "ROLE_";
 	private boolean convertToUpperCase = true;
 
+	//TODO: replace by a modern collection?
 	protected Hashtable<String, GrantedAuthority[]> authContext;
 
 	/**
 	 * Constructor for group search scenarios. <tt>userRoleAttributes</tt> may still be
 	 * set as a property.
 	 */
 	public DefaultReverseProxyAuthoritiesPopulator(Hashtable<String, GrantedAuthority[]> authContext) {
-		this.authContext = authContext;
+		this.authContext = new Hashtable<>(authContext);
 		reverseProxyTemplate = new ReverseProxySearchTemplate();
 	}
 

src/main/java/org/jenkinsci/plugins/reverse_proxy_auth/model/ReverseProxyUserDetails.java

@@ -1,35 +1,44 @@
 package org.jenkinsci.plugins.reverse_proxy_auth.model;
 
+import javax.annotation.CheckForNull;
 import javax.naming.directory.Attributes;
 import javax.naming.directory.BasicAttributes;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.acegisecurity.GrantedAuthority;
 import org.acegisecurity.userdetails.UserDetails;
 
+import java.util.Arrays;
+
 /**
  * @author Wilder Rodrigues ([email protected])
  */
 public class ReverseProxyUserDetails implements UserDetails {
 
 	private static final long serialVersionUID = 8070729070782792157L;
 
-	private static Attributes attributes = new BasicAttributes();
+	private static final Attributes attributes = new BasicAttributes();
 
+	@CheckForNull
 	private GrantedAuthority[] authorities;
+	@CheckForNull
 	private String username;
 
+	@CheckForNull
+	@SuppressFBWarnings(value = "EI_EXPOSE_REP", justification = "We keep it as is due to performance reasons")
 	public GrantedAuthority[] getAuthorities() {
 		return authorities;
 	}
 
-	public void setAuthorities(GrantedAuthority[] authorities) {
-		this.authorities = authorities;
+	public void setAuthorities(@CheckForNull GrantedAuthority[] authorities) {
+		this.authorities = authorities != null ? Arrays.copyOf(authorities, authorities.length) : null;
 	}
 
 	public String getPassword() {
 		return "";
 	}
 
+	@CheckForNull
 	public String getUsername() {
 		return username;
 	}