Add maxAuthLifetime configuration to SamlConfig
maxAuthLifetime = 86400
If no entry included in the gerrit.config the default of
86400 (24 * 60 * 60) is assumed.
This was recently added to the SAML plugin for jenkins to combat
org.pac4j.saml.exceptions.SAMLException:
Authentication issue instant is too old or in the future
Likewise, this should help with gerrit.
Logs in error_log:
[main] INFO com.thesamet.gerrit.plugins.saml.SamlWebFilter
: Max Authentication Lifetime: 86400
Builds against:
- gerrit-plugin-api version 2.13.12
- org.pac4j.pac4j-saml version 2.0.0-RC1
Forced change from pac4j-saml update removes RequiresHttpAction
in favor of HTTPAction. Also removes boolean parameter
from saml2Client.redirect(context, true) to saml2Client.redirect(context);
PR: https://github.com/thesamet/gerrit-saml-plugin/pull/12
Change-Id: If4b8dae65a5f14608551c2a158be529cb7ced967
diff --git a/README.md b/README.md
index fd5309a..e7ea1b4 100644
--- a/README.md
+++ b/README.md
@@ -110,6 +110,10 @@
keystore (needs to be the same as the password provided throguh the `keystore`
flag above.)
+**saml.maxAuthLifetime**: (Optional) Max Authentication Lifetime (secs) configuration.
+
+Default is `86400`
+
**saml.displayNameAttr**: Gerrit will look for an attribute with this name in
the assertion to find a display name for the user. If the attribute is not
found, the NameId from the SAML assertion is used instead.
diff --git a/build.sbt b/build.sbt
index f008805..5648170 100644
--- a/build.sbt
+++ b/build.sbt
@@ -1,8 +1,8 @@
name := "gerrit-saml-plugin"
-val GerritVersion = "2.12.9"
+val GerritVersion = "2.13.12"
-version := GerritVersion + "-1"
+version := GerritVersion + "-6"
scalaVersion := "2.12.8"
@@ -10,7 +10,7 @@
libraryDependencies += ("com.google.gerrit" % "gerrit-plugin-api" % GerritVersion % "provided")
-libraryDependencies += "org.pac4j" % "pac4j-saml" % "1.8.0-RC1"
+libraryDependencies += "org.pac4j" % "pac4j-saml" % "2.0.0-RC1"
libraryDependencies ~= { _ map {
case m => m
@@ -36,7 +36,9 @@
assemblyMergeStrategy in assembly := {
case PathList("META-INF", "INDEX.LIST") => MergeStrategy.discard
case PathList("META-INF", "MANIFEST.MF") => MergeStrategy.discard
+ case PathList("META-INF", "NOTICE.txt") => MergeStrategy.discard
case PathList("META-INF", "NOTICE") => MergeStrategy.discard
+ case PathList("META-INF", "LICENSE.txt") => MergeStrategy.concat
case PathList("META-INF", "LICENSE") => MergeStrategy.concat
// Trick is here: get all the initializers concatenated...
case PathList("META-INF", "services", "org.opensaml.core.config.Initializer") => MergeStrategy.concat
diff --git a/src/main/java/com/thesamet/gerrit/plugins/saml/SamlConfig.java b/src/main/java/com/thesamet/gerrit/plugins/saml/SamlConfig.java
index ffca75f..15af687 100644
--- a/src/main/java/com/thesamet/gerrit/plugins/saml/SamlConfig.java
+++ b/src/main/java/com/thesamet/gerrit/plugins/saml/SamlConfig.java
@@ -32,6 +32,8 @@
private final String displayNameAttr;
private final String userNameAttr;
private final String emailAddressAttr;
+ private final String maxAuthLifetimeAttr;
+ private final int maxAuthLifetimeDefault = 24 * 60 * 60; // 24h;
@Inject
SamlConfig(@GerritServerConfig final Config cfg) {
@@ -44,6 +46,8 @@
userNameAttr = getGetStringWithDefault(cfg, "userNameAttr", "UserName");
emailAddressAttr =
getGetStringWithDefault(cfg, "emailAddressAttr", "EmailAddress");
+ maxAuthLifetimeAttr =
+ getGetStringWithDefault(cfg, "maxAuthLifetime", Integer.toString(maxAuthLifetimeDefault));
}
public String getMetadataPath() {
@@ -74,6 +78,19 @@
return emailAddressAttr;
}
+ public String getMaxAuthLifetimeAttr() { return maxAuthLifetimeAttr; }
+
+ public int getMaxAuthLifetime() {
+ int maxLifetime;
+ try {
+ maxLifetime = Integer.parseInt(getMaxAuthLifetimeAttr());
+ } catch (NumberFormatException nfe) {
+ SamlWebFilter.logError("Error reading \"maxAuthLifetime\" attribute in gerrit.config. Please use digits only");
+ throw nfe; //rethrow so the server stops launching.
+ }
+ return maxLifetime;
+ }
+
private static String getString(Config cfg, String name) {
return cfg.getString("saml", null, name);
}
diff --git a/src/main/java/com/thesamet/gerrit/plugins/saml/SamlWebFilter.java b/src/main/java/com/thesamet/gerrit/plugins/saml/SamlWebFilter.java
index 9149a8b..b7e0379 100644
--- a/src/main/java/com/thesamet/gerrit/plugins/saml/SamlWebFilter.java
+++ b/src/main/java/com/thesamet/gerrit/plugins/saml/SamlWebFilter.java
@@ -22,7 +22,7 @@
import com.google.inject.Singleton;
import org.eclipse.jgit.lib.Config;
import org.pac4j.core.context.J2EContext;
-import org.pac4j.core.exception.RequiresHttpAction;
+import org.pac4j.core.exception.HttpAction;
import org.pac4j.core.exception.TechnicalException;
import org.pac4j.saml.client.SAML2Client;
import org.pac4j.saml.client.SAML2ClientConfiguration;
@@ -67,10 +67,13 @@
@Inject
SamlWebFilter(@GerritServerConfig Config gerritConfig, SamlConfig samlConfig) {
this.samlConfig = samlConfig;
+ log.info("Max Authentication Lifetime: "+ samlConfig.getMaxAuthLifetime());
+ SAML2ClientConfiguration samlClientConfig = new SAML2ClientConfiguration(
+ samlConfig.getKeystorePath(), samlConfig.getKeystorePassword(),
+ samlConfig.getPrivateKeyPassword(), samlConfig.getMetadataPath());
+ samlClientConfig.setMaximumAuthenticationLifetime(samlConfig.getMaxAuthLifetime());
saml2Client =
- new SAML2Client(new SAML2ClientConfiguration(
- samlConfig.getKeystorePath(), samlConfig.getKeystorePassword(),
- samlConfig.getPrivateKeyPassword(), samlConfig.getMetadataPath()));
+ new SAML2Client(samlClientConfig);
String callbackUrl = gerritConfig.getString("gerrit", null, "canonicalWebUrl") + "plugins/gerrit-saml-plugin/saml";
httpUserNameHeader = getHeaderFromConfig(gerritConfig, "httpHeader");
httpDisplaynameHeader = getHeaderFromConfig(gerritConfig, "httpDisplaynameHeader");
@@ -109,7 +112,7 @@
else return user;
}
- private void signin(J2EContext context) throws RequiresHttpAction, IOException {
+ private void signin(J2EContext context) throws HttpAction, IOException {
SAML2Credentials credentials = saml2Client.getCredentials(context);
SAML2Profile user = saml2Client.getUserProfile(credentials, context);
if (user != null) {
@@ -159,13 +162,13 @@
} else {
chain.doFilter(httpRequest, httpResponse);
}
- } catch (final RequiresHttpAction requiresHttpAction) {
+ } catch (final HttpAction requiresHttpAction) {
throw new TechnicalException("Unexpected HTTP action", requiresHttpAction);
}
}
private void redirectToIdentityProvider(J2EContext context)
- throws RequiresHttpAction {
+ throws HttpAction {
String redirectUri = Url.decode(context
.getRequest()
.getRequestURI()
@@ -173,7 +176,7 @@
context.getRequest().getContextPath().length()));
context.setSessionAttribute(SAML2Client.SAML_RELAY_STATE_ATTRIBUTE, redirectUri);
log.debug("Setting redirectUri: {}", redirectUri);
- saml2Client.redirect(context, true);
+ saml2Client.redirect(context);
}
private static boolean isGerritLogin(HttpServletRequest request) {
@@ -264,6 +267,14 @@
}
}
+ public static void logWarning(String message) {
+ log.warn(message);
+ }
+
+ public static void logError(String message) {
+ log.error(message);
+ }
+
private class AnonymousHttpRequest extends HttpServletRequestWrapper {
public AnonymousHttpRequest(HttpServletRequest request) {
super(request);
