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

Prompt for credentials when needed #715

Merged
merged 6 commits into from
Nov 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* It is now possible to specity configuration options on the command line
with he new `--config-toml` global option.

* (#469) `jj git` subcommands will prompt for credentials when
required for HTTPS remotes rather than failing.

### Fixed bugs

* `jj edit root` now fails gracefully.
Expand Down
11 changes: 11 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ pest = "2.4.0"
pest_derive = "2.4"
rand = "0.8.5"
regex = "1.6.0"
rpassword = "7.1.0"
serde = { version = "1.0", features = ["derive"] }
tempfile = "3.3.0"
textwrap = "0.16.0"
Expand Down
109 changes: 74 additions & 35 deletions lib/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@
use std::collections::{BTreeMap, BTreeSet, HashSet};
use std::fs::OpenOptions;
use std::io::{Read, Write};
use std::path::PathBuf;
use std::sync::Arc;

use git2::{Oid, RemoteCallbacks};
use git2::Oid;
use itertools::Itertools;
use thiserror::Error;

Expand Down Expand Up @@ -280,7 +281,7 @@ pub fn fetch(
mut_repo: &mut MutableRepo,
git_repo: &git2::Repository,
remote_name: &str,
progress: Option<&mut dyn FnMut(&Progress)>,
callbacks: RemoteCallbacks<'_>,
) -> Result<Option<String>, GitFetchError> {
let mut remote =
git_repo
Expand All @@ -298,7 +299,7 @@ pub fn fetch(
let mut proxy_options = git2::ProxyOptions::new();
proxy_options.auto();
fetch_options.proxy_options(proxy_options);
let callbacks = create_remote_callbacks(progress);
let callbacks = callbacks.into_git();
fetch_options.remote_callbacks(callbacks);
let refspec: &[&str] = &[];
remote.download(refspec, Some(&mut fetch_options))?;
Expand Down Expand Up @@ -346,6 +347,7 @@ pub fn push_commit(
// It's a blunt "force" option instead until git2-rs supports the "push negotiation" callback
// (https://github.com/rust-lang/git2-rs/issues/733).
force: bool,
callbacks: RemoteCallbacks<'_>,
) -> Result<(), GitPushError> {
push_updates(
git_repo,
Expand All @@ -355,6 +357,7 @@ pub fn push_commit(
force,
new_target: Some(target.id().clone()),
}],
callbacks,
)
}

Expand All @@ -371,6 +374,7 @@ pub fn push_updates(
git_repo: &git2::Repository,
remote_name: &str,
updates: &[GitRefUpdate],
callbacks: RemoteCallbacks<'_>,
) -> Result<(), GitPushError> {
let mut temp_refs = vec![];
let mut qualified_remote_refs = vec![];
Expand All @@ -396,7 +400,13 @@ pub fn push_updates(
refspecs.push(format!(":{}", update.qualified_name));
}
}
let result = push_refs(git_repo, remote_name, &qualified_remote_refs, &refspecs);
let result = push_refs(
git_repo,
remote_name,
&qualified_remote_refs,
&refspecs,
callbacks,
);
for mut temp_ref in temp_refs {
// TODO: Figure out how to do the equivalent of absl::Cleanup for
// temp_ref.delete().
Expand All @@ -417,6 +427,7 @@ fn push_refs(
remote_name: &str,
qualified_remote_refs: &[&str],
refspecs: &[String],
callbacks: RemoteCallbacks<'_>,
) -> Result<(), GitPushError> {
let mut remote =
git_repo
Expand All @@ -435,7 +446,7 @@ fn push_refs(
let mut proxy_options = git2::ProxyOptions::new();
proxy_options.auto();
push_options.proxy_options(proxy_options);
let mut callbacks = create_remote_callbacks(None);
let mut callbacks = callbacks.into_git();
callbacks.push_update_reference(|refname, status| {
// The status is Some if the ref update was rejected
if status.is_none() {
Expand Down Expand Up @@ -466,39 +477,67 @@ fn push_refs(
}
}

fn create_remote_callbacks(progress_cb: Option<&mut dyn FnMut(&Progress)>) -> RemoteCallbacks<'_> {
let mut callbacks = git2::RemoteCallbacks::new();
if let Some(progress_cb) = progress_cb {
callbacks.transfer_progress(move |progress| {
progress_cb(&Progress {
bytes_downloaded: if progress.received_objects() < progress.total_objects() {
Some(progress.received_bytes() as u64)
} else {
None
},
overall: (progress.indexed_objects() + progress.indexed_deltas()) as f32
/ (progress.total_objects() + progress.total_deltas()) as f32,
#[non_exhaustive]
#[derive(Default)]
#[allow(clippy::type_complexity)]
pub struct RemoteCallbacks<'a> {
pub progress: Option<&'a mut dyn FnMut(&Progress)>,
pub get_ssh_key: Option<&'a mut dyn FnMut(&str) -> Option<PathBuf>>,
pub get_password: Option<&'a mut dyn FnMut(&str, &str) -> Option<String>>,
pub get_username_password: Option<&'a mut dyn FnMut(&str) -> Option<(String, String)>>,
}

impl<'a> RemoteCallbacks<'a> {
fn into_git(mut self) -> git2::RemoteCallbacks<'a> {
let mut callbacks = git2::RemoteCallbacks::new();
if let Some(progress_cb) = self.progress {
callbacks.transfer_progress(move |progress| {
progress_cb(&Progress {
bytes_downloaded: if progress.received_objects() < progress.total_objects() {
Some(progress.received_bytes() as u64)
} else {
None
},
overall: (progress.indexed_objects() + progress.indexed_deltas()) as f32
/ (progress.total_objects() + progress.total_deltas()) as f32,
});
true
});
true
});
}
// TODO: We should expose the callbacks to the caller instead -- the library
// crate shouldn't look in $HOME etc.
callbacks.credentials(|_url, username_from_url, allowed_types| {
if allowed_types.contains(git2::CredentialType::SSH_KEY) {
if std::env::var("SSH_AUTH_SOCK").is_ok() || std::env::var("SSH_AGENT_PID").is_ok() {
return git2::Cred::ssh_key_from_agent(username_from_url.unwrap());
}
if let Ok(home_dir) = std::env::var("HOME") {
let key_path = std::path::Path::new(&home_dir).join(".ssh").join("id_rsa");
if key_path.is_file() {
return git2::Cred::ssh_key(username_from_url.unwrap(), None, &key_path, None);
}
// TODO: We should expose the callbacks to the caller instead -- the library
// crate shouldn't read environment variables.
callbacks.credentials(move |url, username_from_url, allowed_types| {
if let Some(username) = username_from_url {
if allowed_types.contains(git2::CredentialType::SSH_KEY) {
if std::env::var("SSH_AUTH_SOCK").is_ok()
|| std::env::var("SSH_AGENT_PID").is_ok()
{
return git2::Cred::ssh_key_from_agent(username);
}
if let Some(ref mut cb) = self.get_ssh_key {
if let Some(path) = cb(username) {
return git2::Cred::ssh_key(username, None, &path, None);
}
}
}
if allowed_types.contains(git2::CredentialType::USER_PASS_PLAINTEXT) {
if let Some(ref mut cb) = self.get_password {
if let Some(pw) = cb(url, username) {
return git2::Cred::userpass_plaintext(username, &pw);
}
}
}
} else if allowed_types.contains(git2::CredentialType::USER_PASS_PLAINTEXT) {
if let Some(ref mut cb) = self.get_username_password {
if let Some((username, pw)) = cb(url) {
return git2::Cred::userpass_plaintext(&username, &pw);
}
}
}
}
git2::Cred::default()
});
callbacks
git2::Cred::default()
});
callbacks
}
}

pub struct Progress {
Expand Down
Loading