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

Generate PublicKey from PrivateKey #187

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bliepp
Copy link

@bliepp bliepp commented Jan 9, 2022

I know it's not that much of an important feature as a public key can be generated from a private key by simply doing

pubkey = rsa.PublicKey(privkey.n, privkey.e)

but I still find a separate method to be convenient as it eliminates a possible source of errors and increases readability.

With the rather simple implementation this would be just

pubkey = privkey.public_key()

@sybrenstuvel
Copy link
Owner

Thanks for the PR. I agree with you that it's good to have a dedicated method for this.

Before I can accept the PR it needs a bit of love, though. The new code should be covered by a unit test.

Maybe you could also write a little bit about this in the "Generating Keys" section of doc/usage.rst? Such new, useful functionality ought to be documented as well.

@bliepp
Copy link
Author

bliepp commented Jan 11, 2022

Thank you for your reply. I've written a test case in the KeyGenTest class and some documentation into usage.rst as you suggested.

Hope this is fine now.

@bliepp
Copy link
Author

bliepp commented Jan 20, 2022

It's been a week since I fixed this PR. Is this okay now or is there something I need to change?

@sybrenstuvel
Copy link
Owner

From a glance it looks good, thanks. For personal reasons I can't spend more brain power on this now, sorry. I'll get back to you, no worries. If there happen to be things to change, I'll be explicit in what & how.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants