Merge pull request #1160 from fzs/sshLdapAuthenticator

LDAP SSH key manager

8 files changed, 833 insertions, 286 deletions

diff --git a/src/main/java/com/gitblit/auth/LdapAuthProvider.java b/src/main/java/com/gitblit/auth/LdapAuthProvider.java
index c31694ba..6a2cbde2 100644
--- a/src/main/java/com/gitblit/auth/LdapAuthProvider.java
+++ b/src/main/java/com/gitblit/auth/LdapAuthProvider.java
@@ -16,9 +16,6 @@
  */
 package com.gitblit.auth;
 
-import java.net.URI;
-import java.net.URISyntaxException;
-import java.security.GeneralSecurityException;
 import java.text.MessageFormat;
 import java.util.Arrays;
 import java.util.HashMap;
@@ -33,28 +30,20 @@ import com.gitblit.Constants.AccountType;
 import com.gitblit.Constants.Role;
 import com.gitblit.Keys;
 import com.gitblit.auth.AuthenticationProvider.UsernamePasswordAuthenticationProvider;
+import com.gitblit.ldap.LdapConnection;
 import com.gitblit.models.TeamModel;
 import com.gitblit.models.UserModel;
 import com.gitblit.service.LdapSyncService;
 import com.gitblit.utils.ArrayUtils;
 import com.gitblit.utils.StringUtils;
 import com.unboundid.ldap.sdk.Attribute;
-import com.unboundid.ldap.sdk.BindRequest;
 import com.unboundid.ldap.sdk.BindResult;
-import com.unboundid.ldap.sdk.DereferencePolicy;
-import com.unboundid.ldap.sdk.ExtendedResult;
-import com.unboundid.ldap.sdk.LDAPConnection;
 import com.unboundid.ldap.sdk.LDAPException;
-import com.unboundid.ldap.sdk.LDAPSearchException;
 import com.unboundid.ldap.sdk.ResultCode;
 import com.unboundid.ldap.sdk.SearchRequest;
 import com.unboundid.ldap.sdk.SearchResult;
 import com.unboundid.ldap.sdk.SearchResultEntry;
 import com.unboundid.ldap.sdk.SearchScope;
-import com.unboundid.ldap.sdk.SimpleBindRequest;
-import com.unboundid.ldap.sdk.extensions.StartTLSExtendedRequest;
-import com.unboundid.util.ssl.SSLUtil;
-import com.unboundid.util.ssl.TrustAllTrustManager;
 
 /**
  * Implementation of an LDAP user service.
@@ -109,7 +98,7 @@ public class LdapAuthProvider extends UsernamePasswordAuthenticationProvider {
 		if (enabled) {
 			logger.info("Synchronizing with LDAP @ " + settings.getRequiredString(Keys.realm.ldap.server));
 			final boolean deleteRemovedLdapUsers = settings.getBoolean(Keys.realm.ldap.removeDeletedUsers, true);
-			LdapConnection ldapConnection = new LdapConnection();
+			LdapConnection ldapConnection = new LdapConnection(settings);
 			if (ldapConnection.connect()) {
 				if (ldapConnection.bind() == null) {
 					ldapConnection.close();
@@ -118,9 +107,9 @@ public class LdapAuthProvider extends UsernamePasswordAuthenticationProvider {
 				}
 
 				try {
-					String accountBase = settings.getString(Keys.realm.ldap.accountBase, "");
 					String uidAttribute = settings.getString(Keys.realm.ldap.uid, "uid");
-					String accountPattern = settings.getString(Keys.realm.ldap.accountPattern, "(&(objectClass=person)(sAMAccountName=${username}))");
+					String accountBase = ldapConnection.getAccountBase();
+					String accountPattern = ldapConnection.getAccountPattern();
 					accountPattern = StringUtils.replace(accountPattern, "${username}", "*");
 
 					SearchResult result = doSearch(ldapConnection, accountBase, accountPattern);
@@ -265,7 +254,7 @@ public class LdapAuthProvider extends UsernamePasswordAuthenticationProvider {
 	public UserModel authenticate(String username, char[] password) {
 		String simpleUsername = getSimpleUsername(username);
 
-		LdapConnection ldapConnection = new LdapConnection();
+		LdapConnection ldapConnection = new LdapConnection(settings);
 		if (ldapConnection.connect()) {
 
 			// Try to bind either to the "manager" account,
@@ -286,11 +275,7 @@ public class LdapAuthProvider extends UsernamePasswordAuthenticationProvider {
 
 			try {
 				// Find the logging in user's DN
-				String accountBase = settings.getString(Keys.realm.ldap.accountBase, "");
-				String accountPattern = settings.getString(Keys.realm.ldap.accountPattern, "(&(objectClass=person)(sAMAccountName=${username}))");
-				accountPattern = StringUtils.replace(accountPattern, "${username}", escapeLDAPSearchFilter(simpleUsername));
-
-				SearchResult result = doSearch(ldapConnection, accountBase, accountPattern);
+				SearchResult result = ldapConnection.searchUser(simpleUsername);
 				if (result != null && result.getEntryCount() == 1) {
 					SearchResultEntry loggingInUser = result.getSearchEntries().get(0);
 					String loggingInUserDN = loggingInUser.getDN();
@@ -441,12 +426,12 @@ public class LdapAuthProvider extends UsernamePasswordAuthenticationProvider {
 		String groupBase = settings.getString(Keys.realm.ldap.groupBase, "");
 		String groupMemberPattern = settings.getString(Keys.realm.ldap.groupMemberPattern, "(&(objectClass=group)(member=${dn}))");
 
-		groupMemberPattern = StringUtils.replace(groupMemberPattern, "${dn}", escapeLDAPSearchFilter(loggingInUserDN));
-		groupMemberPattern = StringUtils.replace(groupMemberPattern, "${username}", escapeLDAPSearchFilter(simpleUsername));
+		groupMemberPattern = StringUtils.replace(groupMemberPattern, "${dn}", LdapConnection.escapeLDAPSearchFilter(loggingInUserDN));
+		groupMemberPattern = StringUtils.replace(groupMemberPattern, "${username}", LdapConnection.escapeLDAPSearchFilter(simpleUsername));
 
 		// Fill in attributes into groupMemberPattern
 		for (Attribute userAttribute : loggingInUser.getAttributes()) {
-			groupMemberPattern = StringUtils.replace(groupMemberPattern, "${" + userAttribute.getName() + "}", escapeLDAPSearchFilter(userAttribute.getValue()));
+			groupMemberPattern = StringUtils.replace(groupMemberPattern, "${" + userAttribute.getName() + "}", LdapConnection.escapeLDAPSearchFilter(userAttribute.getValue()));
 		}
 
 		SearchResult teamMembershipResult = searchTeamsInLdap(ldapConnection, groupBase, true, groupMemberPattern, Arrays.asList("cn"));
@@ -538,6 +523,7 @@ public class LdapAuthProvider extends UsernamePasswordAuthenticationProvider {
 
 
 
+
 	/**
 	 * Returns a simple username without any domain prefixes.
 	 *
@@ -553,34 +539,6 @@ public class LdapAuthProvider extends UsernamePasswordAuthenticationProvider {
 		return username;
 	}
 
-	// From: https://www.owasp.org/index.php/Preventing_LDAP_Injection_in_Java
-	private static final String escapeLDAPSearchFilter(String filter) {
-		StringBuilder sb = new StringBuilder();
-		for (int i = 0; i < filter.length(); i++) {
-			char curChar = filter.charAt(i);
-			switch (curChar) {
-			case '\\':
-				sb.append("\\5c");
-				break;
-			case '*':
-				sb.append("\\2a");
-				break;
-			case '(':
-				sb.append("\\28");
-				break;
-			case ')':
-				sb.append("\\29");
-				break;
-			case '\u0000':
-				sb.append("\\00");
-				break;
-			default:
-				sb.append(curChar);
-			}
-		}
-		return sb.toString();
-	}
-
 	private void configureSyncService() {
 		LdapSyncService ldapSyncService = new LdapSyncService(settings, this);
 		if (ldapSyncService.isReady()) {
@@ -593,226 +551,4 @@ public class LdapAuthProvider extends UsernamePasswordAuthenticationProvider {
 			logger.info("Ldap sync service is disabled.");
 		}
 	}
-
-
-
-	private class LdapConnection {
-		private LDAPConnection conn;
-		private SimpleBindRequest currentBindRequest;
-		private SimpleBindRequest managerBindRequest;
-		private SimpleBindRequest userBindRequest;
-
-
-		public LdapConnection() {
-			String bindUserName = settings.getString(Keys.realm.ldap.username, "");
-			String bindPassword = settings.getString(Keys.realm.ldap.password, "");
-			if (StringUtils.isEmpty(bindUserName) && StringUtils.isEmpty(bindPassword)) {
-				this.managerBindRequest = new SimpleBindRequest();
-			}
-			this.managerBindRequest = new SimpleBindRequest(bindUserName, bindPassword);
-		}
-
-
-		boolean connect() {
-			try {
-				URI ldapUrl = new URI(settings.getRequiredString(Keys.realm.ldap.server));
-				String ldapHost = ldapUrl.getHost();
-				int ldapPort = ldapUrl.getPort();
-
-				if (ldapUrl.getScheme().equalsIgnoreCase("ldaps")) {
-					// SSL
-					SSLUtil sslUtil = new SSLUtil(new TrustAllTrustManager());
-					conn = new LDAPConnection(sslUtil.createSSLSocketFactory());
-					if (ldapPort == -1) {
-						ldapPort = 636;
-					}
-				} else if (ldapUrl.getScheme().equalsIgnoreCase("ldap") || ldapUrl.getScheme().equalsIgnoreCase("ldap+tls")) {
-					// no encryption or StartTLS
-					conn = new LDAPConnection();
-					 if (ldapPort == -1) {
-						 ldapPort = 389;
-					 }
-				} else {
-					logger.error("Unsupported LDAP URL scheme: " + ldapUrl.getScheme());
-					return false;
-				}
-
-				conn.connect(ldapHost, ldapPort);
-
-				if (ldapUrl.getScheme().equalsIgnoreCase("ldap+tls")) {
-					SSLUtil sslUtil = new SSLUtil(new TrustAllTrustManager());
-					ExtendedResult extendedResult = conn.processExtendedOperation(
-							new StartTLSExtendedRequest(sslUtil.createSSLContext()));
-					if (extendedResult.getResultCode() != ResultCode.SUCCESS) {
-						throw new LDAPException(extendedResult.getResultCode());
-					}
-				}
-
-				return true;
-
-			} catch (URISyntaxException e) {
-				logger.error("Bad LDAP URL, should be in the form: ldap(s|+tls)://<server>:<port>", e);
-			} catch (GeneralSecurityException e) {
-				logger.error("Unable to create SSL Connection", e);
-			} catch (LDAPException e) {
-				logger.error("Error Connecting to LDAP", e);
-			}
-
-			return false;
-		}
-
-
-		void close() {
-			if (conn != null) {
-				conn.close();
-			}
-		}
-
-
-		SearchResult search(SearchRequest request) {
-			try {
-				return conn.search(request);
-			} catch (LDAPSearchException e) {
-				logger.error("Problem Searching LDAP [{}]",  e.getResultCode());
-				return e.getSearchResult();
-			}
-		}
-
-
-		SearchResult search(String base, boolean dereferenceAliases, String filter, List<String> attributes) {
-			try {
-				SearchRequest searchRequest = new SearchRequest(base, SearchScope.SUB, filter);
-				if (dereferenceAliases) {
-					searchRequest.setDerefPolicy(DereferencePolicy.SEARCHING);
-				}
-				if (attributes != null) {
-					searchRequest.setAttributes(attributes);
-				}
-				SearchResult result = search(searchRequest);
-				return result;
-
-			} catch (LDAPException e) {
-				logger.error("Problem creating LDAP search", e);
-				return null;
-			}
-		}
-
-
-
-		/**
-		 * Bind using the manager credentials set in realm.ldap.username and ..password
-		 * @return A bind result, or null if binding failed.
-		 */
-		BindResult bind() {
-			BindResult result = null;
-			try {
-				result = conn.bind(managerBindRequest);
-				currentBindRequest = managerBindRequest;
-			} catch (LDAPException e) {
-				logger.error("Error authenticating to LDAP with manager account to search the directory.");
-				logger.error("  Please check your settings for realm.ldap.username and realm.ldap.password.");
-				logger.debug("  Received exception when binding to LDAP", e);
-				return null;
-			}
-			return result;
-		}
-
-
-		/**
-		 * Bind using the given credentials, by filling in the username in the given {@code bindPattern} to
-		 * create the DN.
-		 * @return A bind result, or null if binding failed.
-		 */
-		BindResult bind(String bindPattern, String simpleUsername, String password) {
-			BindResult result = null;
-			try {
-				String bindUser = StringUtils.replace(bindPattern, "${username}", escapeLDAPSearchFilter(simpleUsername));
-				SimpleBindRequest request = new SimpleBindRequest(bindUser, password);
-				result = conn.bind(request);
-				userBindRequest = request;
-				currentBindRequest = userBindRequest;
-			} catch (LDAPException e) {
-				logger.error("Error authenticating to LDAP with user account to search the directory.");
-				logger.error("  Please check your settings for realm.ldap.bindpattern.");
-				logger.debug("  Received exception when binding to LDAP", e);
-				return null;
-			}
-			return result;
-		}
-
-
-		boolean rebindAsUser() {
-			if (userBindRequest == null || currentBindRequest == userBindRequest) {
-				return false;
-			}
-			try {
-				conn.bind(userBindRequest);
-				currentBindRequest = userBindRequest;
-			} catch (LDAPException e) {
-				conn.close();
-				logger.error("Error rebinding to LDAP with user account.", e);
-				return false;
-			}
-			return true;
-		}
-
-
-		boolean isAuthenticated(String userDn, String password) {
-			verifyCurrentBinding();
-
-			// If the currently bound DN is already the DN of the logging in user, authentication has already happened
-			// during the previous bind operation. We accept this and return with the current bind left in place.
-			// This could also be changed to always retry binding as the logging in user, to make sure that the
-			// connection binding has not been tampered with in between. So far I see no way how this could happen
-			// and thus skip the repeated binding.
-			// This check also makes sure that the DN in realm.ldap.bindpattern actually matches the DN that was found
-			// when searching the user entry.
-			String boundDN = currentBindRequest.getBindDN();
-			if (boundDN != null && boundDN.equals(userDn)) {
-				return true;
-			}
-
-			// Bind a the logging in user to check for authentication.
-			// Afterwards, bind as the original bound DN again, to restore the previous authorization.
-			boolean isAuthenticated = false;
-			try {
-				// Binding will stop any LDAP-Injection Attacks since the searched-for user needs to bind to that DN
-				SimpleBindRequest ubr = new SimpleBindRequest(userDn, password);
-				conn.bind(ubr);
-				isAuthenticated = true;
-				userBindRequest = ubr;
-			} catch (LDAPException e) {
-				logger.error("Error authenticating user ({})", userDn, e);
-			}
-
-			try {
-				conn.bind(currentBindRequest);
-			} catch (LDAPException e) {
-				logger.error("Error reinstating original LDAP authorization (code {}). Team information may be inaccurate for this log in.",
-							e.getResultCode(), e);
-			}
-			return isAuthenticated;
-		}
-
-
-
-		private boolean verifyCurrentBinding() {
-			BindRequest lastBind = conn.getLastBindRequest();
-			if (lastBind == currentBindRequest) {
-				return true;
-			}
-			logger.debug("Unexpected binding in LdapConnection. {} != {}", lastBind, currentBindRequest);
-
-			String lastBoundDN = ((SimpleBindRequest)lastBind).getBindDN();
-			String boundDN = currentBindRequest.getBindDN();
-			logger.debug("Currently bound as '{}', check authentication for '{}'", lastBoundDN, boundDN);
-			if (boundDN != null && ! boundDN.equals(lastBoundDN)) {
-				logger.warn("Unexpected binding DN in LdapConnection. '{}' != '{}'.", lastBoundDN, boundDN);
-				logger.warn("Updated binding information in LDAP connection.");
-				currentBindRequest = (SimpleBindRequest)lastBind;
-				return false;
-			}
-			return true;
-		}
-	}
 }
diff --git a/src/main/java/com/gitblit/ldap/LdapConnection.java b/src/main/java/com/gitblit/ldap/LdapConnection.java
new file mode 100644
index 00000000..14fedf10
--- /dev/null
+++ b/src/main/java/com/gitblit/ldap/LdapConnection.java
@@ -0,0 +1,338 @@
+/*
+ * Copyright 2016 gitblit.com.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.gitblit.ldap;
+
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.security.GeneralSecurityException;
+import java.util.List;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.gitblit.IStoredSettings;
+import com.gitblit.Keys;
+import com.gitblit.utils.StringUtils;
+import com.unboundid.ldap.sdk.BindRequest;
+import com.unboundid.ldap.sdk.BindResult;
+import com.unboundid.ldap.sdk.DereferencePolicy;
+import com.unboundid.ldap.sdk.ExtendedResult;
+import com.unboundid.ldap.sdk.LDAPConnection;
+import com.unboundid.ldap.sdk.LDAPException;
+import com.unboundid.ldap.sdk.LDAPSearchException;
+import com.unboundid.ldap.sdk.ResultCode;
+import com.unboundid.ldap.sdk.SearchRequest;
+import com.unboundid.ldap.sdk.SearchResult;
+import com.unboundid.ldap.sdk.SearchScope;
+import com.unboundid.ldap.sdk.SimpleBindRequest;
+import com.unboundid.ldap.sdk.extensions.StartTLSExtendedRequest;
+import com.unboundid.util.ssl.SSLUtil;
+import com.unboundid.util.ssl.TrustAllTrustManager;
+
+public class LdapConnection implements AutoCloseable {
+
+	private final Logger logger = LoggerFactory.getLogger(getClass());
+
+	private IStoredSettings settings;
+
+	private LDAPConnection conn;
+	private SimpleBindRequest currentBindRequest;
+	private SimpleBindRequest managerBindRequest;
+	private SimpleBindRequest userBindRequest;
+
+
+	// From: https://www.owasp.org/index.php/Preventing_LDAP_Injection_in_Java
+	public static final String escapeLDAPSearchFilter(String filter) {
+		StringBuilder sb = new StringBuilder();
+		for (int i = 0; i < filter.length(); i++) {
+			char curChar = filter.charAt(i);
+			switch (curChar) {
+			case '\\':
+				sb.append("\\5c");
+				break;
+			case '*':
+				sb.append("\\2a");
+				break;
+			case '(':
+				sb.append("\\28");
+				break;
+			case ')':
+				sb.append("\\29");
+				break;
+			case '\u0000':
+				sb.append("\\00");
+				break;
+			default:
+				sb.append(curChar);
+			}
+		}
+		return sb.toString();
+	}
+
+
+
+	public static String getAccountBase(IStoredSettings settings) {
+		return settings.getString(Keys.realm.ldap.accountBase, "");
+	}
+
+	public static String getAccountPattern(IStoredSettings settings) {
+		return settings.getString(Keys.realm.ldap.accountPattern, "(&(objectClass=person)(sAMAccountName=${username}))");
+	}
+
+
+
+	public LdapConnection(IStoredSettings settings) {
+		this.settings = settings;
+
+		String bindUserName = settings.getString(Keys.realm.ldap.username, "");
+		String bindPassword = settings.getString(Keys.realm.ldap.password, "");
+		if (StringUtils.isEmpty(bindUserName) && StringUtils.isEmpty(bindPassword)) {
+			this.managerBindRequest = new SimpleBindRequest();
+		}
+		this.managerBindRequest = new SimpleBindRequest(bindUserName, bindPassword);
+	}
+
+
+
+	public String getAccountBase() {
+		return getAccountBase(settings);
+	}
+
+	public String getAccountPattern() {
+		return getAccountPattern(settings);
+	}
+
+
+
+	public boolean connect() {
+		try {
+			URI ldapUrl = new URI(settings.getRequiredString(Keys.realm.ldap.server));
+			String ldapHost = ldapUrl.getHost();
+			int ldapPort = ldapUrl.getPort();
+
+			if (ldapUrl.getScheme().equalsIgnoreCase("ldaps")) {
+				// SSL
+				SSLUtil sslUtil = new SSLUtil(new TrustAllTrustManager());
+				conn = new LDAPConnection(sslUtil.createSSLSocketFactory());
+				if (ldapPort == -1) {
+					ldapPort = 636;
+				}
+			} else if (ldapUrl.getScheme().equalsIgnoreCase("ldap") || ldapUrl.getScheme().equalsIgnoreCase("ldap+tls")) {
+				// no encryption or StartTLS
+				conn = new LDAPConnection();
+				 if (ldapPort == -1) {
+					 ldapPort = 389;
+				 }
+			} else {
+				logger.error("Unsupported LDAP URL scheme: " + ldapUrl.getScheme());
+				return false;
+			}
+
+			conn.connect(ldapHost, ldapPort);
+
+			if (ldapUrl.getScheme().equalsIgnoreCase("ldap+tls")) {
+				SSLUtil sslUtil = new SSLUtil(new TrustAllTrustManager());
+				ExtendedResult extendedResult = conn.processExtendedOperation(
+						new StartTLSExtendedRequest(sslUtil.createSSLContext()));
+				if (extendedResult.getResultCode() != ResultCode.SUCCESS) {
+					throw new LDAPException(extendedResult.getResultCode());
+				}
+			}
+
+			return true;
+
+		} catch (URISyntaxException e) {
+			logger.error("Bad LDAP URL, should be in the form: ldap(s|+tls)://<server>:<port>", e);
+		} catch (GeneralSecurityException e) {
+			logger.error("Unable to create SSL Connection", e);
+		} catch (LDAPException e) {
+			logger.error("Error Connecting to LDAP", e);
+		}
+
+		return false;
+	}
+
+
+	public void close() {
+		if (conn != null) {
+			conn.close();
+		}
+	}
+
+
+
+	/**
+	 * Bind using the manager credentials set in realm.ldap.username and ..password
+	 * @return A bind result, or null if binding failed.
+	 */
+	public BindResult bind() {
+		BindResult result = null;
+		try {
+			result = conn.bind(managerBindRequest);
+			currentBindRequest = managerBindRequest;
+		} catch (LDAPException e) {
+			logger.error("Error authenticating to LDAP with manager account to search the directory.");
+			logger.error("  Please check your settings for realm.ldap.username and realm.ldap.password.");
+			logger.debug("  Received exception when binding to LDAP", e);
+			return null;
+		}
+		return result;
+	}
+
+
+	/**
+	 * Bind using the given credentials, by filling in the username in the given {@code bindPattern} to
+	 * create the DN.
+	 * @return A bind result, or null if binding failed.
+	 */
+	public BindResult bind(String bindPattern, String simpleUsername, String password) {
+		BindResult result = null;
+		try {
+			String bindUser = StringUtils.replace(bindPattern, "${username}", escapeLDAPSearchFilter(simpleUsername));
+			SimpleBindRequest request = new SimpleBindRequest(bindUser, password);
+			result = conn.bind(request);
+			userBindRequest = request;
+			currentBindRequest = userBindRequest;
+		} catch (LDAPException e) {
+			logger.error("Error authenticating to LDAP with user account to search the directory.");
+			logger.error("  Please check your settings for realm.ldap.bindpattern.");
+			logger.debug("  Received exception when binding to LDAP", e);
+			return null;
+		}
+		return result;
+	}
+
+
+	public boolean rebindAsUser() {
+		if (userBindRequest == null || currentBindRequest == userBindRequest) {
+			return false;
+		}
+		try {
+			conn.bind(userBindRequest);
+			currentBindRequest = userBindRequest;
+		} catch (LDAPException e) {
+			conn.close();
+			logger.error("Error rebinding to LDAP with user account.", e);
+			return false;
+		}
+		return true;
+	}
+
+
+
+	public boolean isAuthenticated(String userDn, String password) {
+		verifyCurrentBinding();
+
+		// If the currently bound DN is already the DN of the logging in user, authentication has already happened
+		// during the previous bind operation. We accept this and return with the current bind left in place.
+		// This could also be changed to always retry binding as the logging in user, to make sure that the
+		// connection binding has not been tampered with in between. So far I see no way how this could happen
+		// and thus skip the repeated binding.
+		// This check also makes sure that the DN in realm.ldap.bindpattern actually matches the DN that was found
+		// when searching the user entry.
+		String boundDN = currentBindRequest.getBindDN();
+		if (boundDN != null && boundDN.equals(userDn)) {
+			return true;
+		}
+
+		// Bind a the logging in user to check for authentication.
+		// Afterwards, bind as the original bound DN again, to restore the previous authorization.
+		boolean isAuthenticated = false;
+		try {
+			// Binding will stop any LDAP-Injection Attacks since the searched-for user needs to bind to that DN
+			SimpleBindRequest ubr = new SimpleBindRequest(userDn, password);
+			conn.bind(ubr);
+			isAuthenticated = true;
+			userBindRequest = ubr;
+		} catch (LDAPException e) {
+			logger.error("Error authenticating user ({})", userDn, e);
+		}
+
+		try {
+			conn.bind(currentBindRequest);
+		} catch (LDAPException e) {
+			logger.error("Error reinstating original LDAP authorization (code {}). Team information may be inaccurate for this log in.",
+						e.getResultCode(), e);
+		}
+		return isAuthenticated;
+	}
+
+
+
+
+	public SearchResult search(SearchRequest request) {
+		try {
+			return conn.search(request);
+		} catch (LDAPSearchException e) {
+			logger.error("Problem Searching LDAP [{}]",  e.getResultCode());
+			return e.getSearchResult();
+		}
+	}
+
+
+	public SearchResult search(String base, boolean dereferenceAliases, String filter, List<String> attributes) {
+		try {
+			SearchRequest searchRequest = new SearchRequest(base, SearchScope.SUB, filter);
+			if (dereferenceAliases) {
+				searchRequest.setDerefPolicy(DereferencePolicy.SEARCHING);
+			}
+			if (attributes != null) {
+				searchRequest.setAttributes(attributes);
+			}
+			SearchResult result = search(searchRequest);
+			return result;
+
+		} catch (LDAPException e) {
+			logger.error("Problem creating LDAP search", e);
+			return null;
+		}
+	}
+
+
+	public SearchResult searchUser(String username, List<String> attributes) {
+
+		String accountPattern = getAccountPattern();
+		accountPattern = StringUtils.replace(accountPattern, "${username}", escapeLDAPSearchFilter(username));
+
+		return search(getAccountBase(), false, accountPattern, attributes);
+	}
+
+
+	public SearchResult searchUser(String username) {
+		return searchUser(username, null);
+	}
+
+
+
+	private boolean verifyCurrentBinding() {
+		BindRequest lastBind = conn.getLastBindRequest();
+		if (lastBind == currentBindRequest) {
+			return true;
+		}
+		logger.debug("Unexpected binding in LdapConnection. {} != {}", lastBind, currentBindRequest);
+
+		String lastBoundDN = ((SimpleBindRequest)lastBind).getBindDN();
+		String boundDN = currentBindRequest.getBindDN();
+		logger.debug("Currently bound as '{}', check authentication for '{}'", lastBoundDN, boundDN);
+		if (boundDN != null && ! boundDN.equals(lastBoundDN)) {
+			logger.warn("Unexpected binding DN in LdapConnection. '{}' != '{}'.", lastBoundDN, boundDN);
+			logger.warn("Updated binding information in LDAP connection.");
+			currentBindRequest = (SimpleBindRequest)lastBind;
+			return false;
+		}
+		return true;
+	}
+}
diff --git a/src/main/java/com/gitblit/transport/ssh/IPublicKeyManager.java b/src/main/java/com/gitblit/transport/ssh/IPublicKeyManager.java
index 1e74b2f0..ffe64f59 100644
--- a/src/main/java/com/gitblit/transport/ssh/IPublicKeyManager.java
+++ b/src/main/java/com/gitblit/transport/ssh/IPublicKeyManager.java
@@ -25,6 +25,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.gitblit.manager.IManager;
+import com.gitblit.models.UserModel;
 import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.CacheLoader.InvalidCacheLoadException;
@@ -99,4 +100,16 @@ public abstract class IPublicKeyManager implements IManager {
 	public abstract boolean removeKey(String username, SshKey key);
 
 	public abstract boolean removeAllKeys(String username);
+
+	public boolean supportsWritingKeys(UserModel user) {
+		return (user != null);
+	}
+
+	public boolean supportsCommentChanges(UserModel user) {
+		return (user != null);
+	}
+
+	public boolean supportsPermissionChanges(UserModel user) {
+		return (user != null);
+	}
 }
diff --git a/src/main/java/com/gitblit/transport/ssh/LdapKeyManager.java b/src/main/java/com/gitblit/transport/ssh/LdapKeyManager.java
new file mode 100644
index 00000000..c62c4dee
--- /dev/null
+++ b/src/main/java/com/gitblit/transport/ssh/LdapKeyManager.java
@@ -0,0 +1,435 @@
+/*
+ * Copyright 2016 gitblit.com.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.gitblit.transport.ssh;
+
+import java.io.IOException;
+import java.security.GeneralSecurityException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.TreeMap;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.sshd.common.config.keys.KeyUtils;
+import org.apache.sshd.common.util.GenericUtils;
+import org.apache.sshd.server.config.keys.AuthorizedKeyEntry;
+
+import com.gitblit.IStoredSettings;
+import com.gitblit.Keys;
+import com.gitblit.Constants.AccessPermission;
+import com.gitblit.ldap.LdapConnection;
+import com.gitblit.models.UserModel;
+import com.gitblit.utils.StringUtils;
+import com.google.common.base.Joiner;
+import com.google.inject.Inject;
+import com.unboundid.ldap.sdk.BindResult;
+import com.unboundid.ldap.sdk.ResultCode;
+import com.unboundid.ldap.sdk.SearchResult;
+import com.unboundid.ldap.sdk.SearchResultEntry;
+
+/**
+ * LDAP-only public key manager
+ *
+ * Retrieves public keys from user's LDAP entries. Using this key manager,
+ * no SSH keys can be edited, i.e. added, removed, permissions changed, etc.
+ *
+ * This key manager supports SSH key entries in LDAP of the following form:
+ * [<prefix>:] [<options>] <type> <key> [<comment>]
+ * This follows the required form of entries in the authenticated_keys file,
+ * with an additional optional prefix. Key entries must have a key type
+ * (like "ssh-rsa") and a key, and may have a comment at the end.
+ *
+ * An entry may specify login options as specified for the authorized_keys file.
+ * The 'environment' option may be used to set the permissions for the key
+ * by setting a 'gbPerm' environment variable. The key manager will interpret
+ * such a environment variable option and use the set permission string to set
+ * the permission on the key in Gitblit. Example:
+ *   environment="gbPerm=V",pty ssh-rsa AAAxjka.....dv= Clone only key
+ * Above entry would create a RSA key with the comment "Clone only key" and
+ * set the key permission to CLONE. All other options are ignored.
+ *
+ * In Active Directory SSH public keys are sometimes stored in the attribute
+ * 'altSecurityIdentity'. The attribute value is usually prefixed by a type
+ * identifier. LDAP entries could have the following attribute values:
+ *   altSecurityIdentity: X.509: ADKEJBAKDBZUPABBD...
+ *   altSecurityIdentity: SshKey: ssh-dsa AAAAknenazuzucbhda...
+ * This key manager supports this by allowing an optional prefix to identify
+ * SSH keys. The prefix to be used should be set in the 'realm.ldap.sshPublicKey'
+ * setting by separating it from the attribute name with a colon, e.g.:
+ *    realm.ldap.sshPublicKey = altSecurityIdentity:SshKey
+ *
+ * @author Florian Zschocke
+ *
+ */
+public class LdapKeyManager extends IPublicKeyManager {
+
+	/**
+	 * Pattern to find prefixes like 'SSHKey:' in key entries.
+	 * These prefixes describe the type of an altSecurityIdentity.
+	 * The pattern accepts anything but quote and colon up to the
+	 * first colon at the start of a string.
+	 */
+	private static final Pattern PREFIX_PATTERN = Pattern.compile("^([^\":]+):");
+	/**
+	 * Pattern to find the string describing Gitblit permissions for a SSH key.
+	 * The pattern matches on a string starting with 'gbPerm', matched case-insensitive,
+	 * followed by '=' with optional whitespace around it, followed by a string of
+	 * upper and lower case letters and '+' and '-' for the permission, which can optionally
+	 * be enclosed in '"' or '\"' (only the leading quote is matched in the pattern).
+	 * Only the group describing the permission is a capturing group.
+	 */
+	private static final Pattern GB_PERM_PATTERN = Pattern.compile("(?i:gbPerm)\\s*=\\s*(?:\\\\\"|\")?\\s*([A-Za-z+-]+)");
+
+
+	private final IStoredSettings settings;
+
+
+
+	@Inject
+	public LdapKeyManager(IStoredSettings settings) {
+		this.settings = settings;
+	}
+
+
+	@Override
+	public String toString() {
+		return getClass().getSimpleName();
+	}
+
+	@Override
+	public LdapKeyManager start() {
+		log.info(toString());
+		return this;
+	}
+
+	@Override
+	public boolean isReady() {
+		return true;
+	}
+
+	@Override
+	public LdapKeyManager stop() {
+		return this;
+	}
+
+	@Override
+	protected boolean isStale(String username) {
+		// always return true so we gets keys from LDAP every time
+		return true;
+	}
+
+	@Override
+	protected List<SshKey> getKeysImpl(String username) {
+		try (LdapConnection conn = new LdapConnection(settings)) {
+			if (conn.connect()) {
+				log.info("loading ssh key for {} from LDAP directory", username);
+
+				BindResult bindResult = conn.bind();
+				if (bindResult == null) {
+					conn.close();
+					return null;
+				}
+
+				// Search the user entity
+
+				// Support prefixing the key data, e.g. when using altSecurityIdentities in AD.
+				String pubKeyAttribute = settings.getString(Keys.realm.ldap.sshPublicKey, "sshPublicKey");
+				String pkaPrefix = null;
+				int idx = pubKeyAttribute.indexOf(':');
+				if (idx > 0) {
+					pkaPrefix = pubKeyAttribute.substring(idx +1);
+					pubKeyAttribute = pubKeyAttribute.substring(0, idx);
+				}
+
+				SearchResult result = conn.searchUser(getSimpleUsername(username), Arrays.asList(pubKeyAttribute));
+				conn.close();
+
+				if (result != null && result.getResultCode() == ResultCode.SUCCESS) {
+					if ( result.getEntryCount() > 1) {
+						log.info("Found more than one entry for user {} in LDAP. Cannot retrieve SSH key.", username);
+						return null;
+					} else if ( result.getEntryCount() < 1) {
+						log.info("Found no entry for user {} in LDAP. Cannot retrieve SSH key.", username);
+						return null;
+					}
+
+					// Retrieve the SSH key attributes
+					SearchResultEntry foundUser = result.getSearchEntries().get(0);
+					String[] attrs = foundUser.getAttributeValues(pubKeyAttribute);
+					if (attrs == null ||attrs.length == 0) {
+						log.info("found no keys for user {} under attribute {} in directory", username, pubKeyAttribute);
+						return null;
+					}
+
+
+					// Filter resulting list to match with required special prefix in entry
+					List<GbAuthorizedKeyEntry> authorizedKeys = new ArrayList<>(attrs.length);
+					Matcher m = PREFIX_PATTERN.matcher("");
+					for (int i = 0; i < attrs.length; ++i) {
+						// strip out line breaks
+						String keyEntry = Joiner.on("").join(attrs[i].replace("\r\n", "\n").split("\n"));
+						m.reset(keyEntry);
+						try {
+							if (m.lookingAt()) { // Key is prefixed in LDAP
+								if (pkaPrefix == null) {
+									continue;
+								}
+								String prefix = m.group(1).trim();
+								if (! pkaPrefix.equalsIgnoreCase(prefix)) {
+									continue;
+								}
+								String s = keyEntry.substring(m.end()); // Strip prefix off
+								authorizedKeys.add(GbAuthorizedKeyEntry.parseAuthorizedKeyEntry(s));
+
+							} else { // Key is not prefixed in LDAP
+								if (pkaPrefix != null) {
+									continue;
+								}
+								String s = keyEntry; // Strip prefix off
+								authorizedKeys.add(GbAuthorizedKeyEntry.parseAuthorizedKeyEntry(s));
+							}
+						} catch (IllegalArgumentException e) {
+							log.info("Failed to parse key entry={}:", keyEntry, e.getMessage());
+						}
+					}
+
+					List<SshKey> keyList = new ArrayList<>(authorizedKeys.size());
+					for (GbAuthorizedKeyEntry keyEntry : authorizedKeys) {
+						try {
+							SshKey key = new SshKey(keyEntry.resolvePublicKey());
+							key.setComment(keyEntry.getComment());
+							setKeyPermissions(key, keyEntry);
+							keyList.add(key);
+						} catch (GeneralSecurityException | IOException e) {
+							log.warn("Error resolving key entry for user {}. Entry={}", username, keyEntry, e);
+						}
+					}
+					return keyList;
+				}
+			}
+		}
+
+		return null;
+	}
+
+
+	@Override
+	public boolean addKey(String username, SshKey key) {
+		return false;
+	}
+
+	@Override
+	public boolean removeKey(String username, SshKey key) {
+		return false;
+	}
+
+	@Override
+	public boolean removeAllKeys(String username) {
+		return false;
+	}
+
+
+	public boolean supportsWritingKeys(UserModel user) {
+		return false;
+	}
+
+	public boolean supportsCommentChanges(UserModel user) {
+		return false;
+	}
+
+	public boolean supportsPermissionChanges(UserModel user) {
+		return false;
+	}
+
+
+	private void setKeyPermissions(SshKey key, GbAuthorizedKeyEntry keyEntry) {
+		List<String> env = keyEntry.getLoginOptionValues("environment");
+		if (env != null && !env.isEmpty()) {
+			// Walk over all entries and find one that sets 'gbPerm'. The last one wins.
+			for (String envi : env) {
+				Matcher m = GB_PERM_PATTERN.matcher(envi);
+				if (m.find()) {
+					String perm = m.group(1).trim();
+					AccessPermission ap = AccessPermission.fromCode(perm);
+					if (ap == AccessPermission.NONE) {
+						ap = AccessPermission.valueOf(perm.toUpperCase());
+					}
+
+					if (ap != null && ap != AccessPermission.NONE) {
+						try {
+							key.setPermission(ap);
+						} catch (IllegalArgumentException e) {
+							log.warn("Incorrect permissions ({}) set for SSH key entry {}.", ap, envi, e);
+						}
+					}
+				}
+			}
+		}
+	}
+
+
+	/**
+	 * Returns a simple username without any domain prefixes.
+	 *
+	 * @param username
+	 * @return a simple username
+	 */
+	private String getSimpleUsername(String username) {
+		int lastSlash = username.lastIndexOf('\\');
+		if (lastSlash > -1) {
+			username = username.substring(lastSlash + 1);
+		}
+
+		return username;
+	}
+
+
+	/**
+	 * Extension of the AuthorizedKeyEntry from Mina SSHD with better option parsing.
+	 *
+	 * The class makes use of code from the two methods copied from the original
+	 * Mina SSHD AuthorizedKeyEntry class. The code is rewritten to improve user login
+	 * option support. Options are correctly parsed even if they have whitespace within
+	 * double quotes. Options can occur multiple times, which is needed for example for
+	 * the "environment" option. Thus for an option a list of strings is kept, holding
+	 * multiple option values.
+	 */
+	private static class GbAuthorizedKeyEntry extends AuthorizedKeyEntry {
+
+		private static final long serialVersionUID = 1L;
+		/**
+		 * Pattern to extract the first part of the key entry without whitespace or only with quoted whitespace.
+		 * The pattern essentially splits the line in two parts with two capturing groups. All other groups
+		 * in the pattern are non-capturing. The first part is a continuous string that only includes double quoted
+		 * whitespace and ends in whitespace. The second part is the rest of the line.
+		 * The first part is at the beginning of the line, the lead-in. For a SSH key entry this can either be
+		 * login options (see authorized keys file description) or the key type. Since options, other than the
+		 * key type, can include whitespace and escaped double quotes within double quotes, the pattern takes
+		 * care of that by searching for either "characters that are not whitespace and not double quotes"
+		 * or "a double quote, followed by 'characters that are not a double quote or backslash, or a backslash
+		 * and then a double quote, or a backslash', followed by a double quote".
+		 */
+		private static final Pattern LEADIN_PATTERN = Pattern.compile("^((?:[^\\s\"]*|(?:\"(?:[^\"\\\\]|\\\\\"|\\\\)*\"))*\\s+)(.+)");
+		/**
+		 * Pattern to split a comma separated list of options.
+		 * Since an option could contain commas (as well as escaped double quotes) within double quotes
+		 * in the option value, a simple split on comma is not enough. So the pattern searches for multiple
+		 * occurrences of:
+		 * characters that are not double quotes or a comma, or
+		 * a double quote followed by: characters that are not a double quote or backslash, or
+		 *                             a backslash and then a double quote, or
+		 *                             a backslash,
+		 *   followed by a double quote.
+		 */
+		private static final Pattern OPTION_PATTERN = Pattern.compile("([^\",]+|(?:\"(?:[^\"\\\\]|\\\\\"|\\\\)*\"))+");
+
+		// for options that have no value, "true" is used
+		private Map<String, List<String>> loginOptionsMulti = Collections.emptyMap();
+
+
+		List<String> getLoginOptionValues(String option) {
+			return loginOptionsMulti.get(option);
+		}
+
+
+
+		/**
+		 * @param line Original line from an <code>authorized_keys</code> file
+		 * @return {@link GbAuthorizedKeyEntry} or {@code null} if the line is
+		 * {@code null}/empty or a comment line
+		 * @throws IllegalArgumentException If failed to parse/decode the line
+		 * @see #COMMENT_CHAR
+		 */
+		public static GbAuthorizedKeyEntry parseAuthorizedKeyEntry(String line) throws IllegalArgumentException {
+			line = GenericUtils.trimToEmpty(line);
+			if (StringUtils.isEmpty(line) || (line.charAt(0) == COMMENT_CHAR) /* comment ? */) {
+				return null;
+			}
+
+			Matcher m = LEADIN_PATTERN.matcher(line);
+			if (! m.lookingAt()) {
+				throw new IllegalArgumentException("Bad format (no key data delimiter): " + line);
+			}
+
+			String keyType = m.group(1).trim();
+			final GbAuthorizedKeyEntry entry;
+			if (KeyUtils.getPublicKeyEntryDecoder(keyType) == null) {  // assume this is due to the fact that it starts with login options
+				entry = parseAuthorizedKeyEntry(m.group(2));
+				if (entry == null) {
+					throw new IllegalArgumentException("Bad format (no key data after login options): " + line);
+				}
+
+				entry.parseAndSetLoginOptions(keyType);
+			} else {
+				int startPos = line.indexOf(' ');
+				if (startPos <= 0) {
+					throw new IllegalArgumentException("Bad format (no key data delimiter): " + line);
+				}
+
+				int endPos = line.indexOf(' ', startPos + 1);
+				if (endPos <= startPos) {
+					endPos = line.length();
+				}
+
+				String encData = (endPos < (line.length() - 1)) ? line.substring(0, endPos).trim() : line;
+				String comment = (endPos < (line.length() - 1)) ? line.substring(endPos + 1).trim() : null;
+				entry = parsePublicKeyEntry(new GbAuthorizedKeyEntry(), encData);
+				entry.setComment(comment);
+			}
+
+			return entry;
+		}
+
+		private void parseAndSetLoginOptions(String options) {
+			Matcher m = OPTION_PATTERN.matcher(options);
+			if (! m.find()) {
+				loginOptionsMulti = Collections.emptyMap();
+			}
+			Map<String, List<String>> optsMap = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
+
+			do {
+				String p = m.group();
+				p = GenericUtils.trimToEmpty(p);
+				if (StringUtils.isEmpty(p)) {
+					continue;
+				}
+
+				int pos = p.indexOf('=');
+				String name = (pos < 0) ? p : GenericUtils.trimToEmpty(p.substring(0, pos));
+				CharSequence value = (pos < 0) ? null : GenericUtils.trimToEmpty(p.substring(pos + 1));
+				value = GenericUtils.stripQuotes(value);
+
+				// For options without value the value is set to TRUE.
+				if (value == null) {
+					value = Boolean.TRUE.toString();
+				}
+
+				List<String> opts = optsMap.get(name);
+				if (opts == null) {
+					opts = new ArrayList<String>();
+					optsMap.put(name, opts);
+				}
+				opts.add(value.toString());
+			} while(m.find());
+
+			loginOptionsMulti = optsMap;
+		}
+	}
+
+}
diff --git a/src/main/java/com/gitblit/transport/ssh/SshDaemon.java b/src/main/java/com/gitblit/transport/ssh/SshDaemon.java
index 5a94c9a3..4fb05f79 100644
--- a/src/main/java/com/gitblit/transport/ssh/SshDaemon.java
+++ b/src/main/java/com/gitblit/transport/ssh/SshDaemon.java
@@ -134,7 +134,7 @@ public class SshDaemon {
 		sshd.setFileSystemFactory(new DisabledFilesystemFactory());
 		sshd.setTcpipForwardingFilter(new NonForwardingFilter());
 		sshd.setCommandFactory(new SshCommandFactory(gitblit, workQueue));
-		sshd.setShellFactory(new WelcomeShell(settings));
+		sshd.setShellFactory(new WelcomeShell(gitblit));
 
 		// Set the server id.  This can be queried with:
 		//   ssh-keyscan -t rsa,dsa -p 29418 localhost
diff --git a/src/main/java/com/gitblit/transport/ssh/WelcomeShell.java b/src/main/java/com/gitblit/transport/ssh/WelcomeShell.java
index ec6f7291..7c407d36 100644
--- a/src/main/java/com/gitblit/transport/ssh/WelcomeShell.java
+++ b/src/main/java/com/gitblit/transport/ssh/WelcomeShell.java
@@ -34,6 +34,7 @@ import org.eclipse.jgit.util.SystemReader;
 
 import com.gitblit.IStoredSettings;
 import com.gitblit.Keys;
+import com.gitblit.manager.IGitblit;
 import com.gitblit.models.UserModel;
 import com.gitblit.transport.ssh.commands.DispatchCommand;
 import com.gitblit.transport.ssh.commands.SshCommandFactory;
@@ -45,19 +46,20 @@ import com.gitblit.utils.StringUtils;
  */
 public class WelcomeShell implements Factory<Command> {
 
-	private final IStoredSettings settings;
+	private final IGitblit gitblit;
 
-	public WelcomeShell(IStoredSettings settings) {
-		this.settings = settings;
+	public WelcomeShell(IGitblit gitblit) {
+		this.gitblit = gitblit;
 	}
 
 	@Override
 	public Command create() {
-		return new SendMessage(settings);
+		return new SendMessage(gitblit);
 	}
 
 	private static class SendMessage implements Command, SessionAware {
 
+		private final IPublicKeyManager km;
 		private final IStoredSettings settings;
 		private ServerSession session;
 
@@ -66,8 +68,9 @@ public class WelcomeShell implements Factory<Command> {
 		private OutputStream err;
 		private ExitCallback exit;
 
-		SendMessage(IStoredSettings settings) {
-			this.settings = settings;
+		SendMessage(IGitblit gitblit) {
+			this.km = gitblit.getPublicKeyManager();
+			this.settings = gitblit.getSettings();
 		}
 
 		@Override
@@ -116,6 +119,10 @@ public class WelcomeShell implements Factory<Command> {
 			UserModel user = client.getUser();
 			String hostname = getHostname();
 			int port = settings.getInteger(Keys.git.sshPort, 0);
+			boolean writeKeysIsSupported = true;
+			if (km != null) {
+				writeKeysIsSupported = km.supportsWritingKeys(user);
+			}
 
 			final String b1 = StringUtils.rightPad("", 72, '═');
 			final String b2 = StringUtils.rightPad("", 72, '─');
@@ -159,7 +166,7 @@ public class WelcomeShell implements Factory<Command> {
 			msg.append(nl);
 			msg.append(nl);
 
-			if (client.getKey() == null) {
+			if (writeKeysIsSupported && client.getKey() == null) {
 				// user has authenticated with a password
 				// display add public key instructions
 				msg.append(" You may upload an SSH public key with the following syntax:");
diff --git a/src/main/java/com/gitblit/transport/ssh/keys/KeysDispatcher.java b/src/main/java/com/gitblit/transport/ssh/keys/KeysDispatcher.java
index da58584c..817a98ff 100644
--- a/src/main/java/com/gitblit/transport/ssh/keys/KeysDispatcher.java
+++ b/src/main/java/com/gitblit/transport/ssh/keys/KeysDispatcher.java
@@ -25,6 +25,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.gitblit.Constants.AccessPermission;
+import com.gitblit.models.UserModel;
 import com.gitblit.transport.ssh.IPublicKeyManager;
 import com.gitblit.transport.ssh.SshKey;
 import com.gitblit.transport.ssh.commands.CommandMetaData;
@@ -47,12 +48,20 @@ public class KeysDispatcher extends DispatchCommand {
 
 	@Override
 	protected void setup() {
-		register(AddKey.class);
-		register(RemoveKey.class);
+		IPublicKeyManager km = getContext().getGitblit().getPublicKeyManager();
+		UserModel user = getContext().getClient().getUser();
+		if (km != null && km.supportsWritingKeys(user)) {
+			register(AddKey.class);
+			register(RemoveKey.class);
+		}
 		register(ListKeys.class);
 		register(WhichKey.class);
-		register(CommentKey.class);
-		register(PermissionKey.class);
+		if (km != null && km.supportsCommentChanges(user)) {
+			register(CommentKey.class);
+		}
+		if (km != null && km.supportsPermissionChanges(user)) {
+			register(PermissionKey.class);
+		}
 	}
 
 	@CommandMetaData(name = "add", description = "Add an SSH public key to your account")
diff --git a/src/main/java/com/gitblit/wicket/panels/SshKeysPanel.java b/src/main/java/com/gitblit/wicket/panels/SshKeysPanel.java
index 15ebd67b..4b878763 100644
--- a/src/main/java/com/gitblit/wicket/panels/SshKeysPanel.java
+++ b/src/main/java/com/gitblit/wicket/panels/SshKeysPanel.java
@@ -48,11 +48,13 @@ public class SshKeysPanel extends BasePanel {
 	private static final long serialVersionUID = 1L;

 

 	private final UserModel user;

+	private final boolean canWriteKeys;

 

 	public SshKeysPanel(String wicketId, UserModel user) {

 		super(wicketId);

 

 		this.user = user;

+		this.canWriteKeys = app().keys().supportsWritingKeys(user);

 	}

 

 	@Override

@@ -90,6 +92,9 @@ public class SshKeysPanel extends BasePanel {
 						}

 					}

 				};

+				if (!canWriteKeys) {

+					delete.setVisibilityAllowed(false);

+				}

 				item.add(delete);

 			}

 		};

@@ -164,6 +169,10 @@ public class SshKeysPanel extends BasePanel {
 			}

 		});

 

+		if (! canWriteKeys) {

+			addKeyForm.setVisibilityAllowed(false);

+		}

+

 		add(addKeyForm);

 	}

 }