Using PathWatcher for jetty-security to make Windows happy

4 files changed, 234 insertions, 133 deletions

diff --git a/jetty-security/src/main/java/org/eclipse/jetty/security/HashLoginService.java b/jetty-security/src/main/java/org/eclipse/jetty/security/HashLoginService.java
index ff6ed1555d..5d606528ee 100644
--- a/jetty-security/src/main/java/org/eclipse/jetty/security/HashLoginService.java
+++ b/jetty-security/src/main/java/org/eclipse/jetty/security/HashLoginService.java
@@ -31,18 +31,18 @@ import org.eclipse.jetty.util.security.Credential;
 /* ------------------------------------------------------------ */
 /**
  * Properties User Realm.
- * 
+ * <p>
  * An implementation of UserRealm that stores users and roles in-memory in HashMaps.
- * <P>
+ * <p>
  * Typically these maps are populated by calling the load() method or passing a properties resource to the constructor. The format of the properties file is:
  * 
- * <PRE>
+ * <pre>
  *  username: password [,rolename ...]
- * </PRE>
+ * </pre>
  * 
  * Passwords may be clear text, obfuscated or checksummed. The class com.eclipse.Util.Password should be used to generate obfuscated passwords or password
  * checksums.
- * 
+ * <p>
  * If DIGEST Authentication is used, the password must be in a recoverable format, either plain text or OBF:.
  */
 public class HashLoginService extends MappedLoginService implements UserListener
@@ -53,7 +53,7 @@ public class HashLoginService extends MappedLoginService implements UserListener
     private String _config;
     private Resource _configResource;
     private Scanner _scanner;
-    private int _refreshInterval = 0;// default is not to reload
+    private boolean hotReload = false; // default is not to reload
 
     /* ------------------------------------------------------------ */
     public HashLoginService()
@@ -102,17 +102,51 @@ public class HashLoginService extends MappedLoginService implements UserListener
     {
         _config = config;
     }
+    
+    /**
+     * Is hot reload enabled on this user store
+     * 
+     * @return true if hot reload was enabled before startup
+     */
+    public boolean isHotReload()
+    {
+        return hotReload;
+    }
+
+    /**
+     * Enable Hot Reload of the Property File
+     * 
+     * @param enable true to enable, false to disable
+     */
+    public void setHotReload(boolean enable)
+    {
+        if (isRunning())
+        {
+            throw new IllegalStateException("Cannot set hot reload while user store is running");
+        }
+        this.hotReload = enable;
+    }
 
     /* ------------------------------------------------------------ */
-    public void setRefreshInterval(int msec)
+    /**
+     * sets the refresh interval (in seconds)
+     * @param sec the refresh interval
+     * @deprecated use {@link #setHotReload(boolean)} instead
+     */
+    @Deprecated
+    public void setRefreshInterval(int sec)
     {
-        _refreshInterval = msec;
     }
 
     /* ------------------------------------------------------------ */
+    /**
+     * @return refresh interval in seconds for how often the properties file should be checked for changes
+     * @deprecated use {@link #isHotReload()} instead
+     */
+    @Deprecated
     public int getRefreshInterval()
     {
-        return _refreshInterval;
+        return (hotReload)?1:0;
     }
 
     /* ------------------------------------------------------------ */
@@ -141,11 +175,11 @@ public class HashLoginService extends MappedLoginService implements UserListener
         if (_propertyUserStore == null)
         {
             if(LOG.isDebugEnabled())
-                LOG.debug("doStart: Starting new PropertyUserStore. PropertiesFile: " + _config + " refreshInterval: " + _refreshInterval);
+                LOG.debug("doStart: Starting new PropertyUserStore. PropertiesFile: " + _config + " hotReload: " + hotReload);
             
             _propertyUserStore = new PropertyUserStore();
-            _propertyUserStore.setRefreshInterval(_refreshInterval);
-            _propertyUserStore.setConfig(_config);
+            _propertyUserStore.setHotReload(hotReload);
+            _propertyUserStore.setConfigPath(_config);
             _propertyUserStore.registerUserListener(this);
             _propertyUserStore.start();
         }
diff --git a/jetty-security/src/main/java/org/eclipse/jetty/security/PropertyUserStore.java b/jetty-security/src/main/java/org/eclipse/jetty/security/PropertyUserStore.java
index d5c8244141..055bff09b6 100644
--- a/jetty-security/src/main/java/org/eclipse/jetty/security/PropertyUserStore.java
+++ b/jetty-security/src/main/java/org/eclipse/jetty/security/PropertyUserStore.java
@@ -19,8 +19,8 @@
 package org.eclipse.jetty.security;
 
 import java.io.File;
-import java.io.FilenameFilter;
 import java.io.IOException;
+import java.nio.file.Path;
 import java.security.Principal;
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -36,11 +36,12 @@ import javax.security.auth.Subject;
 import org.eclipse.jetty.security.MappedLoginService.KnownUser;
 import org.eclipse.jetty.security.MappedLoginService.RolePrincipal;
 import org.eclipse.jetty.server.UserIdentity;
-import org.eclipse.jetty.util.Scanner;
-import org.eclipse.jetty.util.Scanner.BulkListener;
+import org.eclipse.jetty.util.PathWatcher;
+import org.eclipse.jetty.util.PathWatcher.PathWatchEvent;
 import org.eclipse.jetty.util.component.AbstractLifeCycle;
 import org.eclipse.jetty.util.log.Log;
 import org.eclipse.jetty.util.log.Logger;
+import org.eclipse.jetty.util.resource.PathResource;
 import org.eclipse.jetty.util.resource.Resource;
 import org.eclipse.jetty.util.security.Credential;
 
@@ -58,14 +59,15 @@ import org.eclipse.jetty.util.security.Credential;
  *
  * If DIGEST Authentication is used, the password must be in a recoverable format, either plain text or OBF:.
  */
-public class PropertyUserStore extends AbstractLifeCycle
+public class PropertyUserStore extends AbstractLifeCycle implements PathWatcher.Listener
 {
     private static final Logger LOG = Log.getLogger(PropertyUserStore.class);
 
-    private String _config;
+    private Path _configPath;
     private Resource _configResource;
-    private Scanner _scanner;
-    private int _refreshInterval = 0;// default is not to reload
+    
+    private PathWatcher pathWatcher;
+    private boolean hotReload = false; // default is not to reload
 
     private IdentityService _identityService = new DefaultIdentityService();
     private boolean _firstLoad = true; // true if first load, false from that point on
@@ -73,23 +75,76 @@ public class PropertyUserStore extends AbstractLifeCycle
     private final Map<String, UserIdentity> _knownUserIdentities = new HashMap<String, UserIdentity>();
     private List<UserListener> _listeners;
 
-    /* ------------------------------------------------------------ */
+    /**
+     * Get the config (as a string)
+     * @return the config path as a string
+     * @deprecated use {@link #getConfigPath()} instead
+     */
+    @Deprecated
     public String getConfig()
     {
-        return _config;
+        return _configPath.toString();
     }
 
-    /* ------------------------------------------------------------ */
-    public void setConfig(String config)
+    /**
+     * Set the Config Path from a String reference to a file
+     * @param configFile the config file
+     * @deprecated use {@link #setConfigPath(String)} instead
+     */
+    @Deprecated
+    public void setConfig(String configFile)
+    {
+        setConfigPath(configFile);
+    }
+    
+    /**
+     * Get the Config {@link Path} reference.
+     * @return the config path
+     */
+    public Path getConfigPath()
     {
-        _config = config;
+        return _configPath;
     }
 
-    /* ------------------------------------------------------------ */
-        public UserIdentity getUserIdentity(String userName)
+    /**
+     * Set the Config Path from a String reference to a file
+     * @param configFile the config file
+     */
+    public void setConfigPath(String configFile)
+    {
+        if (configFile == null)
+        {
+            _configPath = null;
+        }
+        else
         {
-            return _knownUserIdentities.get(userName);
+            _configPath = new File(configFile).toPath();
         }
+    }
+
+    /**
+     * Set the Config Path from a {@link File} reference
+     * @param configFile the config file
+     */
+    public void setConfigPath(File configFile)
+    {
+        _configPath = configFile.toPath();
+    }
+
+    /**
+     * Set the Config Path
+     * @param configPath the config path
+     */
+    public void setConfigPath(Path configPath)
+    {
+        _configPath = configPath;
+    }
+    
+    /* ------------------------------------------------------------ */
+    public UserIdentity getUserIdentity(String userName)
+    {
+        return _knownUserIdentities.get(userName);
+    }
 
     /* ------------------------------------------------------------ */
     /**
@@ -100,42 +155,85 @@ public class PropertyUserStore extends AbstractLifeCycle
     {
         if (_configResource == null)
         {
-            _configResource = Resource.newResource(_config);
+            _configResource = new PathResource(_configPath);
         }
 
         return _configResource;
     }
+    
+    /**
+     * Is hot reload enabled on this user store
+     * 
+     * @return true if hot reload was enabled before startup
+     */
+    public boolean isHotReload()
+    {
+        return hotReload;
+    }
+
+    /**
+     * Enable Hot Reload of the Property File
+     * 
+     * @param enable true to enable, false to disable
+     */
+    public void setHotReload(boolean enable)
+    {
+        if (isRunning())
+        {
+            throw new IllegalStateException("Cannot set hot reload while user store is running");
+        }
+        this.hotReload = enable;
+    }
 
     /* ------------------------------------------------------------ */
     /**
      * sets the refresh interval (in seconds)
      * @param sec the refresh interval
+     * @deprecated use {@link #setHotReload(boolean)} instead
      */
+    @Deprecated
     public void setRefreshInterval(int sec)
     {
-        _refreshInterval = sec;
     }
 
     /* ------------------------------------------------------------ */
     /**
      * @return refresh interval in seconds for how often the properties file should be checked for changes
+     * @deprecated use {@link #isHotReload()} instead
      */
+    @Deprecated
     public int getRefreshInterval()
     {
-        return _refreshInterval;
+        return (hotReload)?1:0;
+    }
+    
+    @Override
+    public String toString()
+    {
+        StringBuilder s = new StringBuilder();
+        s.append(this.getClass().getName());
+        s.append("[");
+        s.append("users.count=").append(this._knownUsers.size());
+        s.append("identityService=").append(this._identityService);
+        s.append("]");
+        return s.toString();
     }
 
     /* ------------------------------------------------------------ */
     private void loadUsers() throws IOException
     {
-        if (_config == null)
+        if (_configPath == null)
             return;
 
         if (LOG.isDebugEnabled())
-            LOG.debug("Load " + this + " from " + _config);
+        {
+            LOG.debug("Loading " + this + " from " + _configPath);
+        }
+        
         Properties properties = new Properties();
         if (getConfigResource().exists())
             properties.load(getConfigResource().getInputStream());
+        
         Set<String> known = new HashSet<String>();
 
         for (Map.Entry<Object, Object> entry : properties.entrySet())
@@ -212,8 +310,13 @@ public class PropertyUserStore extends AbstractLifeCycle
          * set initial load to false as there should be no more initial loads
          */
         _firstLoad = false;
+        
+        if (LOG.isDebugEnabled())
+        {
+            LOG.debug("Loaded " + this + " from " + _configPath);
+        }
     }
-
+    
     /* ------------------------------------------------------------ */
     /**
      * Depending on the value of the refresh interval, this method will either start up a scanner thread that will monitor the properties file for changes after
@@ -226,67 +329,31 @@ public class PropertyUserStore extends AbstractLifeCycle
     {
         super.doStart();
 
-        if (getRefreshInterval() > 0)
+        if ( isHotReload() && (_configPath != null) )
         {
-            _scanner = new Scanner();
-            _scanner.setScanInterval(getRefreshInterval());
-            List<File> dirList = new ArrayList<File>(1);
-            dirList.add(getConfigResource().getFile().getParentFile());
-            _scanner.setScanDirs(dirList);
-            _scanner.setFilenameFilter(new FilenameFilter()
-            {
-                public boolean accept(File dir, String name)
-                {
-                    File f = new File(dir,name);
-                    try
-                    {
-                        if (f.compareTo(getConfigResource().getFile()) == 0)
-                        {
-                            return true;
-                        }
-                    }
-                    catch (IOException e)
-                    {
-                        return false;
-                    }
-
-                    return false;
-                }
-
-            });
-
-            _scanner.addListener(new BulkListener()
-            {
-                public void filesChanged(List<String> filenames) throws Exception
-                {
-                    if (filenames == null)
-                        return;
-                    if (filenames.isEmpty())
-                        return;
-                    if (filenames.size() == 1)
-                    {
-                        Resource r = Resource.newResource(filenames.get(0));
-                        if (r.getFile().equals(_configResource.getFile()))
-                            loadUsers();
-                    }
-                }
-
-                public String toString()
-                {
-                    return "PropertyUserStore$Scanner";
-                }
-
-            });
-
-            _scanner.setReportExistingFilesOnStartup(true);
-            _scanner.setRecursive(false);
-            _scanner.start();
+            this.pathWatcher = new PathWatcher();
+            this.pathWatcher.addFileWatch(_configPath);
+            this.pathWatcher.addListener(this);
+            this.pathWatcher.start();
         }
         else
         {
             loadUsers();
         }
     }
+    
+    @Override
+    public void onPathWatchEvent(PathWatchEvent event)
+    {
+        try
+        {
+            loadUsers();
+        }
+        catch (IOException e)
+        {
+            LOG.warn(e);
+        }
+    }
 
     /* ------------------------------------------------------------ */
     /**
@@ -295,9 +362,8 @@ public class PropertyUserStore extends AbstractLifeCycle
     protected void doStop() throws Exception
     {
         super.doStop();
-        if (_scanner != null)
-            _scanner.stop();
-        _scanner = null;
+        if (this.pathWatcher != null)
+            this.pathWatcher.stop();
     }
 
     /**
diff --git a/jetty-security/src/test/java/org/eclipse/jetty/security/PropertyUserStoreTest.java b/jetty-security/src/test/java/org/eclipse/jetty/security/PropertyUserStoreTest.java
index e57c9ecb49..91ed6b7367 100644
--- a/jetty-security/src/test/java/org/eclipse/jetty/security/PropertyUserStoreTest.java
+++ b/jetty-security/src/test/java/org/eclipse/jetty/security/PropertyUserStoreTest.java
@@ -22,54 +22,45 @@ import java.io.BufferedWriter;
 import java.io.File;
 import java.io.FileWriter;
 import java.io.Writer;
+import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.atomic.AtomicInteger;
 
+import org.eclipse.jetty.toolchain.test.FS;
+import org.eclipse.jetty.toolchain.test.TestingDir;
 import org.eclipse.jetty.util.security.Credential;
-import org.junit.After;
 import org.junit.Assert;
-import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
 
 public class PropertyUserStoreTest
 {
-    String testFileDir = "target" + File.separator + "property-user-store-test";
-    String testFile = testFileDir + File.separator + "users.txt";
+    @Rule
+    public TestingDir testdir = new TestingDir();
 
-    @Before
-    public void before() throws Exception
+    private File initUsersText() throws Exception
     {
-        File file = new File(testFileDir);
-        file.mkdirs();
-
-        writeInitialUsers(testFile);
-    }
-
-    @After
-    public void after() throws Exception
-    {
-        File file = new File(testFile);
-
-        file.delete();
-    }
-
-    private void writeInitialUsers(String testFile) throws Exception
-    {
-        try (Writer writer = new BufferedWriter(new FileWriter(testFile)))
+        Path dir = testdir.getDir().toPath().toRealPath();
+        FS.ensureDirExists(dir.toFile());
+        File users = dir.resolve("users.txt").toFile();
+        
+        try (Writer writer = new BufferedWriter(new FileWriter(users)))
         {
             writer.append("tom: tom, roleA\n");
             writer.append("dick: dick, roleB\n");
             writer.append("harry: harry, roleA, roleB\n");
         }
+        
+        return users;
     }
 
-    private void writeAdditionalUser(String testFile) throws Exception
+    private void addAdditionalUser(File usersFile, String userRef) throws Exception
     {
         Thread.sleep(1001);
-        try (Writer writer = new BufferedWriter(new FileWriter(testFile,true)))
+        try (Writer writer = new BufferedWriter(new FileWriter(usersFile,true)))
         {
-            writer.append("skip: skip, roleA\n");
+            writer.append(userRef);
         }
     }
 
@@ -77,14 +68,13 @@ public class PropertyUserStoreTest
     public void testPropertyUserStoreLoad() throws Exception
     {
         final AtomicInteger userCount = new AtomicInteger();
+        final File usersFile = initUsersText();
 
         PropertyUserStore store = new PropertyUserStore();
-
-        store.setConfig(testFile);
+        store.setConfigPath(usersFile);
 
         store.registerUserListener(new PropertyUserStore.UserListener()
         {
-
             public void update(String username, Credential credential, String[] roleArray)
             {
                 userCount.getAndIncrement();
@@ -107,14 +97,13 @@ public class PropertyUserStoreTest
     @Test
     public void testPropertyUserStoreLoadUpdateUser() throws Exception
     {
-
         final AtomicInteger userCount = new AtomicInteger();
-
         final List<String> users = new ArrayList<String>();
+        final File usersFile = initUsersText();
 
         PropertyUserStore store = new PropertyUserStore();
-        store.setRefreshInterval(1);
-        store.setConfig(testFile);
+        store.setHotReload(true);
+        store.setConfigPath(usersFile);
 
         store.registerUserListener(new PropertyUserStore.UserListener()
         {
@@ -134,9 +123,12 @@ public class PropertyUserStoreTest
         });
 
         store.start();
+        
+        Thread.sleep(2000);
+        
         Assert.assertEquals(3,userCount.get());
 
-        writeAdditionalUser(testFile);
+        addAdditionalUser(usersFile,"skip: skip, roleA\n");
 
         long start = System.currentTimeMillis();
         while (userCount.get() < 4 && (System.currentTimeMillis() - start) < 10000)
@@ -153,19 +145,20 @@ public class PropertyUserStoreTest
     @Test
     public void testPropertyUserStoreLoadRemoveUser() throws Exception
     {
-        writeAdditionalUser(testFile);
-
+        // initial user file (3) users
+        final File usersFile = initUsersText();
         final AtomicInteger userCount = new AtomicInteger();
-
         final List<String> users = new ArrayList<String>();
+        
+        // adding 4th user
+        addAdditionalUser(usersFile,"skip: skip, roleA\n");
 
         PropertyUserStore store = new PropertyUserStore();
-        store.setRefreshInterval(2);
-        store.setConfig(testFile);
+        store.setHotReload(true);
+        store.setConfigPath(usersFile);
 
         store.registerUserListener(new PropertyUserStore.UserListener()
         {
-
             public void update(String username, Credential credential, String[] roleArray)
             {
                 if (!users.contains(username))
@@ -184,12 +177,13 @@ public class PropertyUserStoreTest
 
         store.start();
 
+        Thread.sleep(2000);
+
         Assert.assertEquals(4,userCount.get());
 
-        Thread.sleep(2000);
-        writeInitialUsers(testFile);
+        // rewrite file with original 3 users
+        initUsersText();
         Thread.sleep(3000);
         Assert.assertEquals(3,userCount.get());
     }
-
 }
diff --git a/jetty-security/src/test/resources/jetty-logging.properties b/jetty-security/src/test/resources/jetty-logging.properties
new file mode 100755
index 0000000000..24d5e3aadc
--- /dev/null
+++ b/jetty-security/src/test/resources/jetty-logging.properties
@@ -0,0 +1,7 @@
+# Setup default logging implementation for during testing
+org.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.StdErrLog
+
+#org.eclipse.jetty.LEVEL=DEBUG
+
+#org.eclipse.jetty.util.PathWatcher.LEVEL=DEBUG
+#org.eclipse.jetty.util.PathWatcher.Noisy.LEVEL=OFF