Skip to content

Commit

Permalink
Swap from keyring to using security directly for debug builds.
Browse files Browse the repository at this point in the history
This isn't *great*, but it does significantly improve local developer
ergonomics.

Prior to this change, things worked just fine (WRT the keychain parts),
but the ergonomics kinda sucked. This is because, by default, only the
application that writes the keychain entry is allowed to read from it.
So what would happen during development is that you'd compile, go
through the oauth device flow then realize you have to fix some bug, fix
the bug, then try to run the application again: 💥 macOS password
prompt.

The changes here in this commit work around the issue by using
`security` (the macOS built-in CLI for handling this stuff). That
results in the keychain entry always being read/written by a stable
binary so debug builds/re-builds don't have issue.

The downside here, is that "anyone" that can execute `security` can read
the password. For what it's worth, this is exactly what `gh` does. See
some references below:

- cli/cli#7043
- https://github.com/zalando/go-keyring
- cli/cli#7123
- cli/cli#7023 (comment)
  • Loading branch information
rwjblue committed Feb 8, 2024
1 parent c7a44e2 commit 9b11158
Showing 1 changed file with 75 additions and 10 deletions.
85 changes: 75 additions & 10 deletions src/keychain.rs
Original file line number Diff line number Diff line change
@@ -1,24 +1,89 @@
use keyring::Entry;

use super::errors::DeviceFlowError;

#[cfg(debug_assertions)]
use std::process::Command;

#[cfg(not(debug_assertions))]
use keyring::Entry;

pub fn get_password() -> Result<String, DeviceFlowError> {
let service = "test-github-device-flow";
let username = "github_token";
let entry = Entry::new(service, username)?;

match entry.get_password() {
Ok(token) => Ok(token),
Err(error) => Err(DeviceFlowError::Keyring(error)),
#[cfg(debug_assertions)]
{
let service = format!("{}:debug", service);
let output = Command::new("security")
.args([
"find-generic-password",
"-a",
username,
"-s",
&service,
"-w",
])
.output()
.map_err(|e| DeviceFlowError::Other(e.to_string()))?;

if output.status.success() {
Ok(String::from_utf8_lossy(&output.stdout).trim().to_string())
} else {
Err(DeviceFlowError::Other(
String::from_utf8_lossy(&output.stderr).trim().to_string(),
))
}
}

#[cfg(not(debug_assertions))]
{
let service = format!("{}:release", service);
let entry = Entry::new(&service, username)?;

match entry.get_password() {
Ok(token) => Ok(token),
Err(error) => Err(DeviceFlowError::Keyring(error)),
}
}
}

pub fn save_password(token: String) -> Result<String, DeviceFlowError> {
let service = "test-github-device-flow";
let username = "github_token";
let entry = Entry::new(service, username)?;
let service = "test-github-device-flow";

#[cfg(debug_assertions)]
{
let service = format!("{}:debug", service);

let output = Command::new("security")
.args([
"add-generic-password",
"-a",
username,
"-s",
&service,
"-w",
&token,
"-U",
])
.output()
.map_err(|e| DeviceFlowError::Other(e.to_string()))?;

entry.set_password(&token)?;
if output.status.success() {
Ok(token)
} else {
Err(DeviceFlowError::Other(
String::from_utf8_lossy(&output.stderr).trim().to_string(),
))
}
}

#[cfg(not(debug_assertions))]
{
let service = format!("{}:release", service);
let entry = Entry::new(&service, username)?;

Ok(token)
entry.set_password(&token)?;

Ok(token)
}
}

0 comments on commit 9b11158

Please sign in to comment.