-
-
Notifications
You must be signed in to change notification settings - Fork 503
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
Implement Optional Private Keys #161
Conversation
Step 1: Implement Public / Preshared keys manual input and validation |
This reverts commit e2b4997.
Ready for review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Private keys seem to be a part of the persisted state, but I cannot see how their presence reflects in GUI, or how are they ever accessed for anything after the user has been set up.
- Is there still any need for storing the private key at all?
- When the private key is persisted, consider making this explicit for the user: maybe a red "Forget private key" button
|
With this PR, the private key becomes
Oh |
@ngoduykhanh can you please have a look? |
@ferrine thank you for your contribution. The code changes look good to me but I have a few concerns/comments:
|
The user does not want to reveal his private key
User manages his private key
No QR can be generated
Did not think about this. Seems to be important but treated as a corner case.
Yes, I can try to make that more pretty TODOs:
|
@ngoduykhanh
It still generates the QR code but the config is incomplete (it was supposed to be so). Incomplete config is an expected behavior so I treat this as a feature. UPD: I checked to import the no private key config. It raises an error in the wireguard app. So I'll remove QR code in this case |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ferrine for addressing the issues. I have one more comment below. Other changes look good.
Done |
Address #160