Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SSH LDAP key manager #1160

merged 7 commits into from Dec 18, 2016

SSH LDAP key manager #1160

merged 7 commits into from Dec 18, 2016


Copy link

@flaix flaix commented Dec 7, 2016

This PR introduces a SSH public key manager that retrieves SSH public keys via LDAP from a directory.
The key manager replaces the file key manager and only accesses LDAP, i.e. it can not be used for a mix of local and external users. All key relevant data is stored in the directory, no data is kept locally.
Key permissions can be set by using the "environment" option from authenticates keys file to set a environment variable gbPerm to the permission value.

flaix added 7 commits Nov 23, 2016
Extract the creation of the in-memory servers and the interceptor
code to a base class that LDAP related unit tests can extend to
have the servers available.
Extract the inner class `LdapConnection` from the `LdapAuthProvider`
into a separate class, so that it can be used from multiple classes
that have to connect to an LDAP directory.
The new class is placed into the new package `com.gitblit.ldap`, since
it isn't specific to authentication.
Add new class `LdapPublicKeyManager` which retrieves public SSH keys
from LDAP.

The attribute can be configured with the new configuration option
`realm.ldap.sshPublicKey`. The setting can be a simple attribute name,
like `sshPublicKey`, or an attribute name and a prefix for the value,
like `altSecurityIdentities:SshKey`, in which case attributes are selected
that have the name `altSecurityIdentities` and whose values start with
Instead of using fixed ports for the listeners of the in-memory
LDAP server, let the listeners select ports and then save them in
the authentication mode instance. This way we prevent port collisions,
which especially showed up under Windows.
The `SshKeysDispatcher` tests that use the keys list command are failing
on Windows because they assume a Unix line ending after each key. But
the command will use a system line ending. So this fix uses system line
endings in the reference string for the assert, too.

In addition, two `assertTrue(false)´ are replaced with a proper `fail`.
Some public key mangers may be read-only, i.e. not allow to add or
delete keys, or to change the key comment or assigned permissions.
In such a case the respective commands should not be available on the
SSH shell and the SSH Keys panel should also not offer the possibility.

The `IPublicKeyManager` gets three new methods, modelled after the
`supportsWritingKeys`, `supportsCommentChanges` and
`supportsPermissionChanges`. They return true if a key manager allows for
keys to be written or updated.
For example the existing `FileKeyManager` will return true for all three
since it allows to store and update keys in a file.
The new `LdapKeyManager` returns false since it only accesses LDAP and
can not add or update any keys in the directory.
A future key manager might get keys from an LDAP directory but still
keep comments and permissions for it in a local copy.

If writing of keys is not supported:
* the welcome shell does not suggest adding a key,
* the `SshKeysDispatcher` does not offer the "add", "remove", "comment" and
  "permission" commands, and
* the SSH keys panel hides the "delete" button in the key list, and the
  "Add Key" form.

The hiding of the "Add key" form is not perfect since the surrounding
div is still shown, but I don't know how to hide it and it didn't look
too bad, either.
@flaix flaix added this to the 1.9.0 milestone Dec 13, 2016
@flaix flaix added this to the 1.9.0 milestone Dec 13, 2016
@flaix flaix merged commit d6ddafd into gitblit:master Dec 18, 2016
@flaix flaix deleted the sshLdapAuthenticator branch Jun 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
None yet

Successfully merging this pull request may close these issues.

None yet

1 participant