ssh-ldap-pubkey icon indicating copy to clipboard operation
ssh-ldap-pubkey copied to clipboard

New commands (including sync with file, validity management…)

Open cyrilst opened this issue 5 years ago • 0 comments

Hello,

as promised in https://github.com/jirutka/ssh-ldap-pubkey/issues/39 here is a PR containing many changes.

I have some comments about these changes (in addition to the ones already discussed in https://github.com/jirutka/ssh-ldap-pubkey/issues/39):

  • please have a look at _decode_all, as I'm not sure it's the right way to do it (first time I deal with decoding).
  • for the delete wildcard, I just added an or pattern == '*' condition, certainly the best approach if we want to keep it simple.
  • the add command now accepts more than one key in the file.
  • for the add command, I added the --update option which updates the expiration date of the keys if they are already existing.
  • please have a look at sync_pubkeys. I use print in this function, which is not a thing we want in __init__.py. But on the other hand, I have to print near private methods… Maybe should I append the output to a variable and return this variable, but it's not really good either, because some output could be missed in the case of an error raise. I'll take any good idea for this, I guess I'll have to refactor this method.
  • I'm not fond of a unique --help containing all the options. As a user, I really prefer when there are specific helps for each command, containing just the appropriate options. But on the other hand, doing this means the tool is now a bit more complex than before…
  • and finally, should I add some tests ? Maybe use docker to create an ldap server, and check if the different command / options give the expected results…

cyrilst avatar Jun 11 '20 12:06 cyrilst