Skip to content
This repository has been archived by the owner on May 11, 2023. It is now read-only.

auth: Read passphrase from env var or stdin #237

Merged
merged 4 commits into from
Aug 31, 2022

Conversation

erak
Copy link
Contributor

@erak erak commented Aug 30, 2022

Closes #213.

@erak erak requested a review from cloudhead as a code owner August 30, 2022 11:40
auth/src/lib.rs Outdated
let passphrase = term::read_passphrase(options.stdin, false)?;
let secret = keys::pwhash(passphrase);

// let pass = keys::pwhash(passphrase);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// let pass = keys::pwhash(passphrase);

@@ -29,6 +29,7 @@ pub mod setup {

pub fn lnk_home() -> Result<(), BoxedError> {
env::set_var(LNK_HOME, env::current_dir()?.join("lnk_home"));
env::set_var("RAD_PASSPHRASE", USER_PASS);
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps introducing a constant for the env var name to avoid typos.

@@ -310,6 +302,47 @@ pub fn secret_input_with_confirmation() -> SecUtf8 {
)
}

pub fn secret_stdin() -> Result<SecUtf8, anyhow::Error> {
let mut input: Zeroizing<String> = Zeroizing::new(Default::default());
std::io::stdin().read_line(&mut input)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't stdin().read_line() read into SecUtf8 type directly, bypassing Zeroizing? SecUtf8 does everything Zeroizing does and more, so maybe we could ditch the zeroize dependency altogether?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mmh, it seems that it should work with the latest version of secstr: https://docs.rs/secstr/latest/secstr/struct.SecUtf8.html#method.unsecure_mut.

Unfortunately, we're on 0.3.2 though and it's not supported it that version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Opened a PR in radicle-keystore that updates secstr to 0.5.x: radicle-dev/radicle-keystore#35

Copy link
Contributor

Choose a reason for hiding this comment

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

Great, thanks! I Didn't realise it's going to require a version update 😬

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool, we can fix this in a later PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tracked here: #240

@cloudhead cloudhead merged commit 8484ea3 into radicle-dev:master Aug 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Add environment variable support for rad auth
3 participants