[refactor] PersistentLogin for readability and maintainability

line-o · line-o · commit f27a5dce3c3d · 2024-08-29T15:53:13.000+02:00
diff --git a/extensions/modules/persistentlogin/src/main/java/org/exist/xquery/modules/persistentlogin/PersistentLogin.java b/extensions/modules/persistentlogin/src/main/java/org/exist/xquery/modules/persistentlogin/PersistentLogin.java
@@ -31,6 +31,7 @@
 
 import java.security.SecureRandom;
 import java.util.*;
+import java.util.concurrent.ConcurrentHashMap;
 
 /**
  * A persistent login feature ("remember me") similar to the implementation in <a href="https://github.com/SpringSource/spring-security">Spring Security</a>,
@@ -60,9 +61,9 @@ public static PersistentLogin getInstance() {
 
     public final static int DEFAULT_TOKEN_LENGTH = 16;
 
-    public final static int INVALIDATION_TIMEOUT = 20000;
+    public final static int INVALIDATION_TIMEOUT = 1000;
 
-    private Map<String, LoginDetails> seriesMap = Collections.synchronizedMap(new HashMap<>());
+    private Map<String, LoginDetails> seriesMap = new ConcurrentHashMap<>();
 
     private SecureRandom random;
 
@@ -76,7 +77,7 @@ public PersistentLogin() {
      *
      * The generated token will have the format base64(series-hash):base64(token-hash).
      *
-     * @param user the user name
+     * @param user the username
      * @param password the password
      * @param timeToLive timeout of the token
      * @return a first login token
@@ -154,17 +155,18 @@ private String generateToken() {
 
     public class LoginDetails {
 
-        private String userName;
-        private String password;
         private String token;
-        private String series;
-        private long expires;
-        private DurationValue timeToLive;
+
+        private final String userName;
+        private final String password;
+        private final String series;
+        private final long expires;
+        private final DurationValue timeToLive;
 
         // disable sequential token checking by default
-        private boolean seqBehavior = false;
+        private final boolean seqBehavior = false;
 
-        private Map<String, Long> invalidatedTokens = new HashMap<>();
+        private final Map<String, Long> invalidatedTokens = new HashMap<>();
 
         public LoginDetails(String user, String password, DurationValue timeToLive, long expires) {
             this.userName = user;
@@ -176,19 +178,19 @@ public LoginDetails(String user, String password, DurationValue timeToLive, long
         }
 
         public String getToken() {
-            return this.token;
+            return token;
         }
 
         public String getSeries() {
-            return this.series;
+            return series;
         }
 
         public String getUser() {
-            return this.userName;
+            return userName;
         }
 
         public String getPassword() {
-            return this.password;
+            return password;
         }
 
         public DurationValue getTimeToLive() {
@@ -197,13 +199,15 @@ public DurationValue getTimeToLive() {
 
         public boolean checkAndUpdateToken(String token) {
             if (this.token.equals(token)) {
+                timeoutCheck();
                 update();
                 return true;
             }
             // check map of invalidating tokens
-            Long timeout = invalidatedTokens.get(token);
-            if (timeout == null)
+            final Long timeout = invalidatedTokens.get(token);
+            if (timeout == null) {
                 return false;
+            }
             // timed out: remove
             if (System.currentTimeMillis() > timeout) {
                 invalidatedTokens.remove(token);
@@ -214,22 +218,21 @@ public boolean checkAndUpdateToken(String token) {
         }
 
         public String update() {
-            timeoutCheck();
-            // leave a small time window until previous token is deleted
+            // leave a small time-window until previous token is deleted
             // to allow for concurrent requests
-            invalidatedTokens.put(this.token, System.currentTimeMillis() + INVALIDATION_TIMEOUT);
-            this.token = generateToken();
-            return this.token;
+            invalidatedTokens.put(token, System.currentTimeMillis() + INVALIDATION_TIMEOUT);
+            token = generateToken();
+            return token;
         }
 
         private void timeoutCheck() {
-            long now = System.currentTimeMillis();
+            final long now = System.currentTimeMillis();
             invalidatedTokens.entrySet().removeIf(entry -> entry.getValue() < now);
         }
 
         @Override
         public String toString() {
-            return this.series + ":" + this.token;
+            return series + ":" + token;
         }
     }
 }
diff --git a/extensions/modules/persistentlogin/src/main/java/org/exist/xquery/modules/persistentlogin/PersistentLoginFunctions.java b/extensions/modules/persistentlogin/src/main/java/org/exist/xquery/modules/persistentlogin/PersistentLoginFunctions.java
@@ -21,6 +21,7 @@
  */
 package org.exist.xquery.modules.persistentlogin;
 
+import org.checkerframework.checker.nullness.qual.Nullable;
 import org.exist.EXistException;
 import org.exist.dom.QName;
 import org.exist.security.AuthenticationException;
@@ -30,14 +31,42 @@
 import org.exist.xquery.*;
 import org.exist.xquery.value.*;
 
+import java.util.EnumSet;
+import java.util.HashMap;
+import java.util.Map;
+
 /**
  * Functions to access the persistent login module.
  */
 public class PersistentLoginFunctions extends UserSwitchingBasicFunction {
+    private enum Fn {
+        REGISTER("register"),
+        LOGIN("login"),
+        INVALIDATE("invalidate");
+
+        final static Map<QName, Fn> lookup = new HashMap<>();
+        static {
+            for (Fn fn: EnumSet.allOf(Fn.class)) {
+                lookup.put(fn.getQName(), fn);
+            }
+        }
+
+        static Fn get(Function f) {
+            return lookup.get(f.getName());
+        }
+
+        private final QName qname;
+
+        Fn(String name) {
+            qname = new QName(name, PersistentLoginModule.NAMESPACE, PersistentLoginModule.PREFIX);
+        }
 
-    public final static FunctionSignature signatures[] = {
+        public QName getQName() { return qname; }
+    }
+
+    public final static FunctionSignature[] signatures = {
             new FunctionSignature(
-                    new QName("register", PersistentLoginModule.NAMESPACE, PersistentLoginModule.PREFIX),
+                    Fn.REGISTER.getQName(),
                     "Try to log in the user and create a one-time login token. The token can be stored to a cookie and used to log in " +
                             "(via the login function) as the same user without " +
                             "providing credentials. However, for security reasons the token will be valid only for " +
@@ -55,7 +84,7 @@ public class PersistentLoginFunctions extends UserSwitchingBasicFunction {
                     new FunctionReturnSequenceType(Type.ITEM, Cardinality.ZERO_OR_MORE, "result of the callback function or the empty sequence")
             ),
             new FunctionSignature(
-                    new QName("login", PersistentLoginModule.NAMESPACE, PersistentLoginModule.PREFIX),
+                    Fn.LOGIN.getQName(),
                     "Try to log in the user based on the supplied token. If the login succeeds, the provided callback function " +
                             "is called with 4 arguments: $token as xs:string, $user as xs:string, $password as xs:string, $timeToLive as duration. " +
                             "$token will be a new token which can be used for the next request. The old token is deleted.",
@@ -67,7 +96,7 @@ public class PersistentLoginFunctions extends UserSwitchingBasicFunction {
                     new FunctionReturnSequenceType(Type.ITEM, Cardinality.ZERO_OR_MORE, "result of the callback function or the empty sequence")
             ),
             new FunctionSignature(
-                    new QName("invalidate", PersistentLoginModule.NAMESPACE, PersistentLoginModule.PREFIX),
+                    Fn.INVALIDATE.getQName(),
                     "Invalidate the supplied one-time token, so it can no longer be used to log in.",
                     new SequenceType[]{
                             new FunctionParameterSequenceType("token", Type.STRING, Cardinality.EXACTLY_ONE, "a valid one-time token")
@@ -85,77 +114,59 @@ public PersistentLoginFunctions(final XQueryContext context, final FunctionSigna
     @Override
     public void analyze(final AnalyzeContextInfo contextInfo) throws XPathException {
         super.analyze(contextInfo);
-        this.cachedContextInfo = new AnalyzeContextInfo(contextInfo);
+        cachedContextInfo = new AnalyzeContextInfo(contextInfo);
     }
 
     @Override
     public Sequence eval(final Sequence[] args, final Sequence contextSequence) throws XPathException {
-        if (isCalledAs("register")) {
-            final String user = args[0].getStringValue();
-            final String pass;
-            if (!args[1].isEmpty()) {
-                pass = args[1].getStringValue();
-            } else {
-                pass = null;
-            }
-            final DurationValue timeToLive = (DurationValue) args[2].itemAt(0);
-            final FunctionReference callback;
-            if (!args[3].isEmpty()) {
-                callback = (FunctionReference) args[3].itemAt(0);
-            } else {
-                callback = null;
-            }
-            try {
-                return register(user, pass, timeToLive, callback);
-            } finally {
-                if (callback != null) {
-                    callback.close();
-                }
-            }
-        } else if (isCalledAs("login")) {
-            final String token = args[0].getStringValue();
-            final FunctionReference callback;
-            if (!args[1].isEmpty()) {
-                callback = (FunctionReference) args[1].itemAt(0);
-            } else {
-                callback = null;
-            }
-            try {
-                return authenticate(token, callback);
-            } finally {
-                if (callback != null) {
-                    callback.close();
-                }
-            }
-        } else {
-            PersistentLogin.getInstance().invalidate(args[0].getStringValue());
-            return Sequence.EMPTY_SEQUENCE;
+        switch (Fn.get(this)) {
+            case REGISTER: return register(args);
+            case LOGIN: return login(args);
+            case INVALIDATE: return invalidate(args);
+            default: throw new XPathException(this, ErrorCodes.ERROR, "Unknown function: " + getName());
         }
     }
 
-    private Sequence register(final String user, final String pass, final DurationValue timeToLive, final FunctionReference callback) throws XPathException {
-        if (login(user, pass)) {
+    private Sequence register(Sequence[] args) throws XPathException {
+        final String user = args[0].getStringValue();
+
+        final String pass;
+        if (args[1].isEmpty()) {
+            pass = null;
+        } else {
+            pass = args[1].getStringValue();
+        }
+
+        final DurationValue timeToLive = (DurationValue) args[2].itemAt(0);
+
+        try (FunctionReference callback = getCallBack(args[3])) {
+            if (!authenticated(user, pass)) {
+                return Sequence.EMPTY_SEQUENCE;
+            }
             final PersistentLogin.LoginDetails details = PersistentLogin.getInstance().register(user, pass, timeToLive);
-            return callback(callback, null, details);
+            return call(callback, null, details);
         }
-        return Sequence.EMPTY_SEQUENCE;
     }
 
-    private Sequence authenticate(final String token, final FunctionReference callback) throws XPathException {
-        final PersistentLogin.LoginDetails data = PersistentLogin.getInstance().lookup(token);
+    private Sequence login(Sequence[] args) throws XPathException {
+        final String token = args[0].getStringValue();
+        try (FunctionReference callback = getCallBack(args[1])) {
+            final PersistentLogin.LoginDetails data = PersistentLogin.getInstance().lookup(token);
 
-        if (data == null) {
-            return Sequence.EMPTY_SEQUENCE;
-        }
+            if (data == null || !authenticated(data.getUser(), data.getPassword())) {
+                return Sequence.EMPTY_SEQUENCE;
+            }
+            return call(callback, token, data);
 
-        if (login(data.getUser(), data.getPassword())) {
-            return callback(callback, token, data);
         }
+    }
 
+    private static Sequence invalidate(Sequence[] args) throws XPathException {
+        PersistentLogin.getInstance().invalidate(args[0].getStringValue());
         return Sequence.EMPTY_SEQUENCE;
     }
 
-    private boolean login(final String user, final String pass) throws XPathException {
+    private boolean authenticated(final String user, final String pass) {
         try {
             final SecurityManager sm = BrokerPool.getInstance().getSecurityManager();
             final Subject subject = sm.authenticate(user, pass);
@@ -169,7 +180,7 @@ private boolean login(final String user, final String pass) throws XPathExceptio
         }
     }
 
-    private Sequence callback(final FunctionReference func, final String oldToken, final PersistentLogin.LoginDetails details) throws XPathException {
+    private Sequence call(@Nullable final FunctionReference func, final String oldToken, final PersistentLogin.LoginDetails details) throws XPathException {
         final Sequence[] args = new Sequence[4];
         final String newToken = details.toString();
 
@@ -185,4 +196,11 @@ private Sequence callback(final FunctionReference func, final String oldToken, f
         func.analyze(cachedContextInfo);
         return func.evalFunction(null, null, args);
     }
+
+    private @Nullable FunctionReference getCallBack(final Sequence arg) {
+        if (arg.isEmpty()) {
+            return null;
+        }
+        return (FunctionReference) arg.itemAt(0);
+    }
 }
