From aaecd8f2a36d2c0d780b42425aa57725fe708551 Mon Sep 17 00:00:00 2001 From: James Moger Date: Fri, 14 Mar 2014 14:53:26 -0400 Subject: [PATCH] Move cache to IKeyManager and implement isStale() in FileKeyManager --- .../gitblit/transport/ssh/FileKeyManager.java | 52 ++++++++++++-- .../gitblit/transport/ssh/IKeyManager.java | 69 ++++++++++++++----- .../gitblit/transport/ssh/NullKeyManager.java | 19 +++-- .../transport/ssh/SshKeyAuthenticator.java | 46 +++---------- .../transport/ssh/commands/AddKeyCommand.java | 1 - .../ssh/commands/RemoveKeyCommand.java | 1 - .../ssh/commands/SetAccountCommand.java | 1 - 7 files changed, 121 insertions(+), 68 deletions(-) diff --git a/src/main/java/com/gitblit/transport/ssh/FileKeyManager.java b/src/main/java/com/gitblit/transport/ssh/FileKeyManager.java index 1eb470bf..ae0bc9cf 100644 --- 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 lastModifieds; + public FileKeyManager(IRuntimeManager runtimeManager) { this.runtimeManager = runtimeManager; + this.lastModifieds = new ConcurrentHashMap(); } @Override @@ -68,15 +73,34 @@ public class FileKeyManager implements IKeyManager { } @Override - public List 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 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 list = new ArrayList(); - 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 cb32a020..12fce3df 100644 --- 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 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> keyCache = CacheBuilder + .newBuilder(). + expireAfterAccess(15, TimeUnit.MINUTES). + maximumSize(100) + .build(new CacheLoader>() { + @Override + public List load(String username) { + return getKeysImpl(username); + } + }); + + public abstract IKeyManager start(); + + public abstract boolean isReady(); + + public abstract IKeyManager stop(); + + public final List 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 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 454d3cfc..c76728d8 100644 --- 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 getKeys(String username) { + protected boolean isStale(String username) { + return false; + } + + @Override + protected List 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 36319226..922f25ae 100644 --- 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> sshKeyCache = CacheBuilder - .newBuilder(). - expireAfterAccess(15, TimeUnit.MINUTES). - maximumSize(100) - .build(new CacheLoader>() { - @Override - public List 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 keys = sshKeyCache.get(username); - if (keys == null || keys.isEmpty()) { - log.info("{} has not added any public keys for ssh authentication", username); - return false; - } + List 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> 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 69c4fecb..35bb1bbf 100644 --- 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 0d491647..90e70418 100644 --- 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 a22ca856..1f0d902b 100644 --- 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 sshKeys) throws UnloggedFailure, -- 2.39.5