Update Code based on comments by @thesamet - Change method getting SAML maxAuthLifetime from string method to int method. - Remove logging methods that are unneeded. PR: https://github.com/thesamet/gerrit-saml-plugin/pull/12 Change-Id: Ia63d4213ed1d91abc7e962e5cfe9c0d50483bea8
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 15af687..9667849 100644 --- a/src/main/java/com/thesamet/gerrit/plugins/saml/SamlConfig.java +++ b/src/main/java/com/thesamet/gerrit/plugins/saml/SamlConfig.java
@@ -25,6 +25,7 @@ */ @Singleton public class SamlConfig { + private final String metadataPath; private final String keystorePath; private final String privateKeyPassword; @@ -32,7 +33,7 @@ private final String displayNameAttr; private final String userNameAttr; private final String emailAddressAttr; - private final String maxAuthLifetimeAttr; + private final int maxAuthLifetimeAttr; private final int maxAuthLifetimeDefault = 24 * 60 * 60; // 24h; @Inject @@ -46,8 +47,7 @@ userNameAttr = getGetStringWithDefault(cfg, "userNameAttr", "UserName"); emailAddressAttr = getGetStringWithDefault(cfg, "emailAddressAttr", "EmailAddress"); - maxAuthLifetimeAttr = - getGetStringWithDefault(cfg, "maxAuthLifetime", Integer.toString(maxAuthLifetimeDefault)); + maxAuthLifetimeAttr = cfg.getInt("saml", null, maxAuthLifetimeDefault); } public String getMetadataPath() { @@ -78,17 +78,8 @@ 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; + public int getMaxAuthLifetimeAttr() { + return maxAuthLifetimeAttr; } private static String getString(Config cfg, String 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 b7e0379..2888581 100644 --- a/src/main/java/com/thesamet/gerrit/plugins/saml/SamlWebFilter.java +++ b/src/main/java/com/thesamet/gerrit/plugins/saml/SamlWebFilter.java
@@ -67,11 +67,11 @@ @Inject SamlWebFilter(@GerritServerConfig Config gerritConfig, SamlConfig samlConfig) { this.samlConfig = samlConfig; - log.info("Max Authentication Lifetime: "+ samlConfig.getMaxAuthLifetime()); + log.debug("Max Authentication Lifetime: " + samlConfig.getMaxAuthLifetimeAttr()); SAML2ClientConfiguration samlClientConfig = new SAML2ClientConfiguration( samlConfig.getKeystorePath(), samlConfig.getKeystorePassword(), samlConfig.getPrivateKeyPassword(), samlConfig.getMetadataPath()); - samlClientConfig.setMaximumAuthenticationLifetime(samlConfig.getMaxAuthLifetime()); + samlClientConfig.setMaximumAuthenticationLifetime(samlConfig.getMaxAuthLifetimeAttr()); saml2Client = new SAML2Client(samlClientConfig); String callbackUrl = gerritConfig.getString("gerrit", null, "canonicalWebUrl") + "plugins/gerrit-saml-plugin/saml"; @@ -267,14 +267,6 @@ } } - 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);