Use existing setting but with new values

Instead of adding another setting and having to explain how the new one
and the existing `requireClientCertificates` setting are interdependent,
let's use the existing setting and add new values.

It is changed from a boolean to a string, with the values `required`,
`optional` and `disabled`. To keep backward compatibility with the old
values, the `true` value is mapped to `required` and the `false` value
is mapped to `optional`.

3 files changed, 43 insertions, 11 deletions

diff --git a/src/main/distrib/data/defaults.properties b/src/main/distrib/data/defaults.properties
index 604caa8f..0d072b58 100644
--- a/src/main/distrib/data/defaults.properties
+++ b/src/main/distrib/data/defaults.properties
@@ -2132,18 +2132,25 @@ server.certificateAlias = localhost
 server.storePassword = gitblit
 
 # If serving over https (recommended) you might consider requiring clients to
-# authenticate with ssl certificates.  If enabled, only https clients with the
-# a valid client certificate will be able to access Gitblit.
+# authenticate with TLS certificates.
 #
-# If disabled, optional client certificate authentication is configurable by
-# server.wantClientCertificates
+# Possible values are: 'required' (or 'true'), 'optional' (or 'false') and 'none'
+#
+# If required, only https clients with a valid client certificate will be able
+# to access Gitblit.
+#
+# If optional, client certificate authentication is optional and will be tried
+# first before falling-back to form authentication or basic authentication.
+#
+# If completely disabled ('none'), then the server will not ask the client to
+# present a client certificate at all.
 #
 # Requiring client certificates to access any of Gitblit may be too extreme,
 # consider this carefully.
 #
 # SINCE 1.2.0
 # RESTART REQUIRED
-server.requireClientCertificates = false
+server.requireClientCertificates = optional
 
 # If enabled, client certificate authentication is optional and will be tried
 # first before falling-back to form authentication or basic authentication.
diff --git a/src/main/java/com/gitblit/Constants.java b/src/main/java/com/gitblit/Constants.java
index ab503bd3..c73bc24b 100644
--- a/src/main/java/com/gitblit/Constants.java
+++ b/src/main/java/com/gitblit/Constants.java
@@ -645,6 +645,28 @@ public class Constants {
 		}

 	}

 

+	public enum TlsClientCertPolicy {

+		REQUIRED, TRUE, OPTIONAL, FALSE, DISABLED, NONE;

+

+		public static TlsClientCertPolicy fromString(String value) {

+			for (TlsClientCertPolicy t : values()) {

+				if (t.name().equalsIgnoreCase(value)) {

+					switch(t) {

+						case TRUE:

+							return REQUIRED;

+						case FALSE:

+							return OPTIONAL;

+						case NONE:

+							return DISABLED;

+						default:

+							return t;

+					}

+				}

+			}

+			return TlsClientCertPolicy.OPTIONAL;

+		}

+	}

+

 	/**

 	 * The type of merge Gitblit will use when merging a ticket to the integration branch.

 	 * <p>

diff --git a/src/main/java/com/gitblit/GitBlitServer.java b/src/main/java/com/gitblit/GitBlitServer.java
index 190cc5d2..63914121 100644
--- a/src/main/java/com/gitblit/GitBlitServer.java
+++ b/src/main/java/com/gitblit/GitBlitServer.java
@@ -57,6 +57,7 @@ import org.kohsuke.args4j.Option;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.gitblit.Constants.TlsClientCertPolicy;
 import com.gitblit.authority.GitblitAuthority;
 import com.gitblit.authority.NewCertificateConfig;
 import com.gitblit.servlet.GitblitContext;
@@ -289,10 +290,15 @@ public class GitBlitServer {
 				logger.info("Setting up HTTPS transport on port " + params.securePort);
 				GitblitSslContextFactory factory = new GitblitSslContextFactory(params.alias,
 						serverKeyStore, serverTrustStore, params.storePassword, caRevocationList);
-				if (params.requireClientCertificates) {
+				TlsClientCertPolicy clientCertPolicy = TlsClientCertPolicy.fromString(params.requireClientCertificates);
+				if (clientCertPolicy == TlsClientCertPolicy.REQUIRED) {
 					factory.setNeedClientAuth(true);
+				} else if (clientCertPolicy == TlsClientCertPolicy.OPTIONAL) {
+					factory.setNeedClientAuth(false);
+					factory.setWantClientAuth(true);
 				} else {
-					factory.setWantClientAuth((params.wantClientCertificates));
+					factory.setNeedClientAuth(false);
+					factory.setWantClientAuth(false);
 				}
 
 				ServerConnector connector = new ServerConnector(server, factory);
@@ -600,10 +606,7 @@ public class GitBlitServer {
 		public Integer shutdownPort = FILESETTINGS.getInteger(Keys.server.shutdownPort, 8081);
 
 		@Option(name = "--requireClientCertificates", usage = "Require client X509 certificates for https connections.")
-		public Boolean requireClientCertificates = FILESETTINGS.getBoolean(Keys.server.requireClientCertificates, false);
-
-		@Option(name = "--wantClientCertificates", usage = "Ask for optional client X509 certificate for https connections. Ignored if client certificates are required.")
-		public Boolean wantClientCertificates = FILESETTINGS.getBoolean(Keys.server.wantClientCertificates, false);
+		public String requireClientCertificates = FILESETTINGS.getString(Keys.server.requireClientCertificates, "optional");
 
 		/*
 		 * Setting overrides