Move cache to IKeyManager and implement isStale() in FileKeyManager

diff --git a/src/main/java/com/gitblit/transport/ssh/FileKeyManager.java b/src/main/java/com/gitblit/transport/ssh/FileKeyManager.java
index 1eb470bfd2bd5a52288152971666f76b30ac27ae..ae0bc9cfc728fb77bc1d11954ab5f894b3133032 100644 (file)
--- a/src/main/java/com/gitblit/transport/ssh/FileKeyManager.java
+++ b/src/main/java/com/gitblit/transport/ssh/FileKeyManager.java
@@ -21,6 +21,8 @@ import java.security.PublicKey;
 import java.text.MessageFormat;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 
 import org.apache.commons.codec.binary.Base64;
 import org.apache.sshd.common.util.Buffer;
@@ -38,12 +40,15 @@ import com.google.common.io.Files;
  * @author James Moger
  *
  */
-public class FileKeyManager implements IKeyManager {
+public class FileKeyManager extends IKeyManager {
 
        protected final IRuntimeManager runtimeManager;
 
+       protected final Map<File, Long> lastModifieds;
+
        public FileKeyManager(IRuntimeManager runtimeManager) {
                this.runtimeManager = runtimeManager;
+               this.lastModifieds = new ConcurrentHashMap<File, Long>();
        }
 
        @Override
@@ -68,15 +73,34 @@ public class FileKeyManager implements IKeyManager {
        }
 
        @Override
-       public List<PublicKey> getKeys(String username) {
+       protected boolean isStale(String username) {
+               File keystore = getKeystore(username);
+               if (!keystore.exists()) {
+                       // keystore may have been deleted
+                       return true;
+               }
+
+               if (lastModifieds.containsKey(keystore)) {
+                       // compare modification times
+                       long lastModified = lastModifieds.get(keystore);
+                       return lastModified != keystore.lastModified();
+               }
+
+               // assume stale
+               return true;
+       }
+
+       @Override
+       protected List<PublicKey> getKeysImpl(String username) {
                try {
-                       File keys = getKeystore(username);
-                       if (!keys.exists()) {
+                       log.info("loading keystore for {}", username);
+                       File keystore = getKeystore(username);
+                       if (!keystore.exists()) {
                                return null;
                        }
-                       if (keys.exists()) {
+                       if (keystore.exists()) {
                                List<PublicKey> list = new ArrayList<PublicKey>();
-                               for (String entry : Files.readLines(keys, Charsets.ISO_8859_1)) {
+                               for (String entry : Files.readLines(keystore, Charsets.ISO_8859_1)) {
                                        if (entry.trim().length() == 0) {
                                                // skip blanks
                                                continue;
@@ -93,6 +117,8 @@ public class FileKeyManager implements IKeyManager {
                                if (list.isEmpty()) {
                                        return null;
                                }
+
+                               lastModifieds.put(keystore, keystore.lastModified());
                                return list;
                        }
                } catch (IOException e) {
@@ -140,6 +166,9 @@ public class FileKeyManager implements IKeyManager {
                        // write keystore
                        String content = Joiner.on("\n").join(lines).trim().concat("\n");
                        Files.write(content, keystore, Charsets.ISO_8859_1);
+
+                       lastModifieds.remove(keystore);
+                       keyCache.invalidate(username);
                        return true;
                } catch (IOException e) {
                        throw new RuntimeException("Cannot add ssh key", e);
@@ -183,6 +212,9 @@ public class FileKeyManager implements IKeyManager {
                                        String content = Joiner.on("\n").join(lines).trim().concat("\n");
                                        Files.write(content, keystore, Charsets.ISO_8859_1);
                                }
+
+                               lastModifieds.remove(keystore);
+                               keyCache.invalidate(username);
                                return true;
                        }
                } catch (IOException e) {
@@ -193,7 +225,13 @@ public class FileKeyManager implements IKeyManager {
 
        @Override
        public boolean removeAllKeys(String username) {
-               return getKeystore(username).delete();
+               File keystore = getKeystore(username);
+               if (keystore.delete()) {
+                       lastModifieds.remove(keystore);
+                       keyCache.invalidate(username);
+                       return true;
+               }
+               return false;
        }
 
        protected File getKeystore(String username) {

diff --git a/src/main/java/com/gitblit/transport/ssh/IKeyManager.java b/src/main/java/com/gitblit/transport/ssh/IKeyManager.java
index cb32a020ef1274cfac7779dfaf1c18835ec62ea7..12fce3df4ae3acea45e61f169449a14931570ac5 100644 (file)
--- a/src/main/java/com/gitblit/transport/ssh/IKeyManager.java
+++ b/src/main/java/com/gitblit/transport/ssh/IKeyManager.java
@@ -16,26 +16,63 @@
 package com.gitblit.transport.ssh;
 
 import java.security.PublicKey;
+import java.text.MessageFormat;
 import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
 
 /**
- * 
+ *
  * @author James Moger
  *
  */
-public interface IKeyManager {
-
-       IKeyManager start();
-       
-       boolean isReady();
-       
-       IKeyManager stop();
-       
-       List<PublicKey> getKeys(String username);
-       
-       boolean addKey(String username, String data);
-       
-       boolean removeKey(String username, String data);
-
-       boolean removeAllKeys(String username);
+public abstract class IKeyManager {
+
+       protected final Logger log = LoggerFactory.getLogger(getClass());
+
+       protected final LoadingCache<String, List<PublicKey>> keyCache = CacheBuilder
+                       .newBuilder().
+                       expireAfterAccess(15, TimeUnit.MINUTES).
+                       maximumSize(100)
+                       .build(new CacheLoader<String, List<PublicKey>>() {
+                               @Override
+                               public List<PublicKey> load(String username) {
+                                       return getKeysImpl(username);
+                               }
+                       });
+
+       public abstract IKeyManager start();
+
+       public abstract boolean isReady();
+
+       public abstract IKeyManager stop();
+
+       public final List<PublicKey> getKeys(String username) {
+               try {
+                       if (isStale(username)) {
+                               keyCache.invalidate(username);
+                       }
+                       return keyCache.get(username);
+               } catch (ExecutionException e) {
+                       log.error(MessageFormat.format("failed to retrieve keys for {0}", username), e);
+               }
+               return null;
+       }
+
+       protected abstract boolean isStale(String username);
+
+       protected abstract List<PublicKey> getKeysImpl(String username);
+
+       public abstract boolean addKey(String username, String data);
+
+       public abstract boolean removeKey(String username, String data);
+
+       public abstract boolean removeAllKeys(String username);
 }

diff --git a/src/main/java/com/gitblit/transport/ssh/NullKeyManager.java b/src/main/java/com/gitblit/transport/ssh/NullKeyManager.java
index 454d3cfcc786b9eab49f6c541bee88546c74c361..c76728d890f95a7e662aab81f49a3504f62ecce8 100644 (file)
--- a/src/main/java/com/gitblit/transport/ssh/NullKeyManager.java
+++ b/src/main/java/com/gitblit/transport/ssh/NullKeyManager.java
@@ -20,15 +20,15 @@ import java.util.List;
 
 /**
  * Rejects all SSH key management requests.
- * 
+ *
  * @author James Moger
  *
  */
-public class NullKeyManager implements IKeyManager {
+public class NullKeyManager extends IKeyManager {
 
        public NullKeyManager() {
        }
-       
+
        @Override
        public String toString() {
                return getClass().getSimpleName();
@@ -38,19 +38,24 @@ public class NullKeyManager implements IKeyManager {
        public NullKeyManager start() {
                return this;
        }
-       
+
        @Override
        public boolean isReady() {
                return true;
        }
-       
+
        @Override
        public NullKeyManager stop() {
                return this;
        }
 
        @Override
-       public List<PublicKey> getKeys(String username) {
+       protected boolean isStale(String username) {
+               return false;
+       }
+
+       @Override
+       protected List<PublicKey> getKeysImpl(String username) {
                return null;
        }
 
@@ -58,7 +63,7 @@ public class NullKeyManager implements IKeyManager {
        public boolean addKey(String username, String data) {
                return false;
        }
-       
+
        @Override
        public boolean removeKey(String username, String data) {
                return false;

diff --git a/src/main/java/com/gitblit/transport/ssh/SshKeyAuthenticator.java b/src/main/java/com/gitblit/transport/ssh/SshKeyAuthenticator.java
index 36319226fb97ba43a420a582c85d591c2717ace2..922f25aedd4519aaa5b2de02a90642a6d63fa687 100644 (file)
--- a/src/main/java/com/gitblit/transport/ssh/SshKeyAuthenticator.java
+++ b/src/main/java/com/gitblit/transport/ssh/SshKeyAuthenticator.java
@@ -18,8 +18,6 @@ package com.gitblit.transport.ssh;
 import java.security.PublicKey;
 import java.util.List;
 import java.util.Locale;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.TimeUnit;
 
 import org.apache.sshd.server.PublickeyAuthenticator;
 import org.apache.sshd.server.session.ServerSession;
@@ -28,10 +26,6 @@ import org.slf4j.LoggerFactory;
 
 import com.gitblit.manager.IAuthenticationManager;
 import com.gitblit.models.UserModel;
-import com.google.common.cache.Cache;
-import com.google.common.cache.CacheBuilder;
-import com.google.common.cache.CacheLoader;
-import com.google.common.cache.LoadingCache;
 
 /**
  *
@@ -46,17 +40,6 @@ public class SshKeyAuthenticator implements PublickeyAuthenticator {
 
        protected final IAuthenticationManager authManager;
 
-       LoadingCache<String, List<PublicKey>> sshKeyCache = CacheBuilder
-                       .newBuilder().
-                       expireAfterAccess(15, TimeUnit.MINUTES).
-                       maximumSize(100)
-                       .build(new CacheLoader<String, List<PublicKey>>() {
-                               @Override
-                               public List<PublicKey> load(String username) {
-                                       return keyManager.getKeys(username);
-                               }
-                       });
-
        public SshKeyAuthenticator(IKeyManager keyManager, IAuthenticationManager authManager) {
                this.keyManager = keyManager;
                this.authManager = authManager;
@@ -74,23 +57,20 @@ public class SshKeyAuthenticator implements PublickeyAuthenticator {
                }
 
                username = username.toLowerCase(Locale.US);
-               try {
-                       List<PublicKey> keys = sshKeyCache.get(username);
-                       if (keys == null || keys.isEmpty()) {
-                               log.info("{} has not added any public keys for ssh authentication", username);
-                               return false;
-                       }
+               List<PublicKey> keys = keyManager.getKeys(username);
+               if (keys == null || keys.isEmpty()) {
+                       log.info("{} has not added any public keys for ssh authentication", username);
+                       return false;
+               }
 
-                       for (PublicKey key : keys) {
-                               if (key.equals(suppliedKey)) {
-                                       UserModel user = authManager.authenticate(username, key);
-                                       if (user != null) {
-                                               client.setUser(user);
-                                               return true;
-                                       }
+               for (PublicKey key : keys) {
+                       if (key.equals(suppliedKey)) {
+                               UserModel user = authManager.authenticate(username, key);
+                               if (user != null) {
+                                       client.setUser(user);
+                                       return true;
                                }
                        }
-               } catch (ExecutionException e) {
                }
 
                log.warn("could not authenticate {} for SSH using the supplied public key", username);
@@ -100,8 +80,4 @@ public class SshKeyAuthenticator implements PublickeyAuthenticator {
        public IKeyManager getKeyManager() {
                return keyManager;
        }
-
-       public Cache<String, List<PublicKey>> getKeyCache() {
-               return sshKeyCache;
-       }
 }

diff --git a/src/main/java/com/gitblit/transport/ssh/commands/AddKeyCommand.java b/src/main/java/com/gitblit/transport/ssh/commands/AddKeyCommand.java
index 69c4fecbc166217194cf0ddb438691ece7bc175a..35bb1bbfb62c6c8d49edf2b9cec0323451355195 100644 (file)
--- a/src/main/java/com/gitblit/transport/ssh/commands/AddKeyCommand.java
+++ b/src/main/java/com/gitblit/transport/ssh/commands/AddKeyCommand.java
@@ -49,6 +49,5 @@ public class AddKeyCommand extends BaseKeyCommand {
                        keyManager.addKey(username, key);
                        log.info("added SSH public key for {}", username);
                }
-               authenticator.getKeyCache().invalidate(username);
        }
 }

diff --git a/src/main/java/com/gitblit/transport/ssh/commands/RemoveKeyCommand.java b/src/main/java/com/gitblit/transport/ssh/commands/RemoveKeyCommand.java
index 0d491647cf28ee1c4ecc791547a9c2551447508a..90e704185bd6b6a962c55139457f999b1149341e 100644 (file)
--- a/src/main/java/com/gitblit/transport/ssh/commands/RemoveKeyCommand.java
+++ b/src/main/java/com/gitblit/transport/ssh/commands/RemoveKeyCommand.java
@@ -57,6 +57,5 @@ public class RemoveKeyCommand extends BaseKeyCommand {
                                log.info("removed SSH public key from {}", username);
                        }
                }
-               authenticator.getKeyCache().invalidate(username);
        }
 }

diff --git a/src/main/java/com/gitblit/transport/ssh/commands/SetAccountCommand.java b/src/main/java/com/gitblit/transport/ssh/commands/SetAccountCommand.java
index a22ca8563315d3d471e732f9ef541eff5c968bef..1f0d902b739ef7ee1f56c69c5b83ed9462ba2521 100644 (file)
--- a/src/main/java/com/gitblit/transport/ssh/commands/SetAccountCommand.java
+++ b/src/main/java/com/gitblit/transport/ssh/commands/SetAccountCommand.java
@@ -65,7 +65,6 @@ public class SetAccountCommand extends BaseKeyCommand {
                if (!deleteSshKeys.isEmpty()) {
                        deleteSshKeys(deleteSshKeys);
                }
-               authenticator.getKeyCache().invalidate(user);
        }
 
        private void addSshKeys(List<String> sshKeys) throws UnloggedFailure,