Skip to content

Re-encode PublicKey in OpenSSH authorized_keys format#7

Merged
btoews merged 6 commits intomasterfrom
authorized_keys
Feb 21, 2019
Merged

Re-encode PublicKey in OpenSSH authorized_keys format#7
btoews merged 6 commits intomasterfrom
authorized_keys

Conversation

@btoews
Copy link
Copy Markdown
Contributor

@btoews btoews commented Feb 11, 2019

I need to be able to re-encode a public key in the format you'd fine in the authorized_keys file (that's what we store in the DB). This PR adds a PublicKey::Base#openssh method for doing this. It also renames some things and improves some comments.

@btoews btoews requested a review from ptoomey3 February 11, 2019 19:23
Copy link
Copy Markdown
Member

@ptoomey3 ptoomey3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems sensible. I had one question, but otherwise LGTM.

# Returns a PublicKey::Base subclass instance.
def self.parse(key)
def self.parse_openssh(key)
algo, raw, _ = SSHData.key_parts(key)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason we might not want to throw away the comment such that .openssh can round-trip an originally parsed key easily?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wouldn't hurt to keep it. The comment is OpenSSH specific though, so it seems "cleaner" to not keep it around...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair enough

@btoews btoews merged commit f7468f4 into master Feb 21, 2019
@btoews btoews deleted the authorized_keys branch February 21, 2019 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants