commit 105d9378a9061f0fdc4b4fb62b0227d4704e0f89
author Michael Lupo <mikelupo@gmail.com> Wed May 31 13:58:21 2017 -0400
committer Luca Milanesio <luca.milanesio@gmail.com> Mon Feb 11 21:32:54 2019 +0000
tree b3b4cab636e5ffa4f2c0515680e8d084eb7444d1
parent a9474ad7ce42954eee569ac0922f8dd9843f6380

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

src/main/java/com/thesamet/gerrit/plugins/saml/SamlConfig.java

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) {

src/main/java/com/thesamet/gerrit/plugins/saml/SamlWebFilter.java

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);