← 输入框|github.com|刷新
Skip to content

Re-encode PublicKey in OpenSSH authorized_keys format - #7

Merged
btoews merged 6 commits into
masterfrom
authorized_keys
Feb 21, 2019
Merged

btoews merged 6 commits into
masterfrom
authorized_keys

Conversation

@btoews

@btoews btoews commented Feb 11, 2019

Copy link
Copy Markdown
Contributor

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

@ptoomey3 ptoomey3 left a comment

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.

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