ssh-ldap-pubkey

ssh-ldap-pubkey copied to clipboard

Hello,

my team has been using ssh-ldap-pubkey for a while, and made lots of changes to it, based on the 0.4.1 version. I'm currently porting these changes to the 1.3.2 version, and I wonder if some of them are worth being merged upstream. I'm aware this tool needs to keep its simplicity, but I'm sure some of these changes can benefit to many people, that's why I'd like to share them. Here is the usage text:

Usage:
  ssh-ldap-pubkey list [options]
  ssh-ldap-pubkey listall [options]
  ssh-ldap-pubkey add [options] FILE
  ssh-ldap-pubkey rm [options] FILE
  ssh-ldap-pubkey sync [options] FILE
  ssh-ldap-pubkey del [options] PATTERN
  ssh-ldap-pubkey purge [options]
  ssh-ldap-pubkey --help

  -                      Read public key from stdin.
  FILE                   Path to the public key file to add.
  PATTERN                Pattern that specifies public key(s) to delete, i.e.
                         a complete key or just a part of it.

Options:
  -a ATTRS --attrs=ATTRS
                         Comma separated list of returned attributes when
                         listing public key(s) for all users. [default: uid]
  -b DN --base=DN        Base DN where to search for the users' entry. If not
                         provided, then it's read from the config file.
  -c FILE --conf=FILE    Path of the ldap.conf (default is /etc/ldap.conf).
                         The ldap.conf is not required when at least --base is
                         provided.
  -D DN --binddn=DN      DN to bind with instead of the user's DN.
  -e DAYS --expire=DAYS  When adding a public key: add an expiration date, when
                         listing public key(s): check if the key will be
                         expired in DAYS days from today (instead of today).
  -H URI --uri=URI       URI of the LDAP server to connect; loaded from the
                         config file by default. If not defined even there,
                         then it defaults to ldap://localhost.
  -j --json              Output in json format (only for listall)
  -m MAX --max=MAX       Max count of keys included in FILE that is allowed to
                         be processed. If greater than MAX, no operation is
                         performed (checked in add/sync).
  -p --purge             In sync mode, purge stored entries instead of expiring
                         them when necessary.
  -q --quiet             Be quiet.
  -u LOGIN --user=LOGIN  Login of the user to bind as and change public key(s)
                         (default is the current user).
  -U --uid               Only display uid (only for listall)
  -V VALIDITY --validity=VALIDITY
                         VALIDITY can be: 'all' to list all public keys,
                         'valid' to list only valid (i.e. unexpired) public
                         key(s), 'invalid' to list public keys not having an
                         expiration date, 'expired' to list expired public
                         key(s) or 'expire' to list key(s) still valid but
                         that will be expired in DAYS days from today having
                         DAYS provided using -e DAYS, [default: all].
  -v --version           Show version information.
  -h --help              Show this message.

New commands

listall

Without options, lists public keys of all users. With the --uid option, lists all logins which have a public key. With the --json option, outputs logins and keys in JSON format. You can use --attrs if you want to list other attributes. Yes it adds a dependence to json, but it's really useful for admins / bots to have a global view of users and public keys.

rm

Same as del, but instead of using a pattern, it uses a file containing the keys to delete. Maybe should we add an option to del in order to use a file?

sync

Synchronize public keys of a user with a file. If a key is in the file, make sure it is in ldap. If a key is not in the file, delete it from ldap. The --purge option affects the behavior related to key expiration (see below).

purge

Delete all key of a user.

New options

--expire=DAYS and --validity=VALIDITY

These options add the support of key expiration. It's an important security feature, and allows to keep track of old expired keys (so they're not used again).

--max=MAX

Limits the number of authorized keys in a file during imports (with add / sync).

Your opinion?

What's your opinion about this? If you think some of this is worth being merged upstream, please tell me how you'd want me to proceed. I can make small patches and PRs in your preferred order if you don't want to include everything in one big patch (it's not that big in fact). If you think some functionality is interesting but should be made differently, please let me know, I'll be happy to rework it.

Hi, thanks for this proposal, it looks like very useful improvements! I suppose that you already have these features implemented in your local fork of this repository, so I think that the smoothest way is to rebase your commits on top of the current master and open a PR of what you have. Then I will ask to cherry-pick some chunks into separate commits if needed.

---

rm Maybe should we add an option to del in order to use a file?

Yes, I would prefer this way.

purge Delete all key of a user.

This seems to be redundant to the del command. The only thing missing is to add support for a wildcard pattern (*).

Thanks a lot for your answer. I haven't finished porting everything yet. I'll do it with your comments about rm and purge in mind, then I'll open the PR.