mirror of
https://github.com/zed-industries/zed.git
synced 2026-08-21 15:04:51 +00:00
git_ui: Dismiss askpass prompts when requests end (#61292)
# Objective Fixes #47623 - When authenticating git commands using a security key through `askpass`. The modal which asks for user presence does not get dismissed even after the git command finishes successfully. ## Solution Add a cancellation task to the `AskPassModal`, which gets dropped when the requested operation completes. This cancellation task then dismisses the modal. ## Testing - I've tried authentication through `askpass` using my own security key. Testing both successful and failed authentication. - I've added tests which confirm that the modal gets dismissed when a task is cancelled, and that the cancellation is triggered when `ask_password` Task gets dropped. Willing to pair on review, message me on Slack. Showcase video left out because it would leak private information. ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - Fixed authentication prompts not dismissing automatically when using security keys with ssh
This commit is contained in:
parent
fd5cd9398d
commit
fdad9186b8
13 changed files with 559 additions and 71 deletions
2
Cargo.lock
generated
2
Cargo.lock
generated
|
|
@ -15355,6 +15355,7 @@ dependencies = [
|
|||
"anyhow",
|
||||
"askpass",
|
||||
"auto_update",
|
||||
"editor",
|
||||
"futures 0.3.32",
|
||||
"gpui",
|
||||
"log",
|
||||
|
|
@ -15364,6 +15365,7 @@ dependencies = [
|
|||
"remote",
|
||||
"semver",
|
||||
"settings",
|
||||
"theme",
|
||||
"theme_settings",
|
||||
"ui",
|
||||
"ui_input",
|
||||
|
|
|
|||
|
|
@ -28,5 +28,8 @@ which.workspace = true
|
|||
[target.'cfg(target_os = "windows")'.dependencies]
|
||||
windows.workspace = true
|
||||
|
||||
[dev-dependencies]
|
||||
gpui = { workspace = true, features = ["test-support"] }
|
||||
|
||||
[package.metadata.cargo-machete]
|
||||
ignored = ["log"]
|
||||
|
|
|
|||
|
|
@ -42,8 +42,14 @@ pub enum AskPassResult {
|
|||
Timedout,
|
||||
}
|
||||
|
||||
type PasswordPrompt = (
|
||||
String,
|
||||
oneshot::Sender<EncryptedPassword>,
|
||||
oneshot::Receiver<()>,
|
||||
);
|
||||
|
||||
pub struct AskPassDelegate {
|
||||
tx: mpsc::UnboundedSender<(String, oneshot::Sender<EncryptedPassword>)>,
|
||||
tx: mpsc::UnboundedSender<PasswordPrompt>,
|
||||
executor: BackgroundExecutor,
|
||||
_task: Task<()>,
|
||||
}
|
||||
|
|
@ -56,10 +62,26 @@ impl AskPassDelegate {
|
|||
+ Sync
|
||||
+ 'static,
|
||||
) -> Self {
|
||||
let (tx, mut rx) = mpsc::unbounded::<(String, oneshot::Sender<_>)>();
|
||||
Self::new_with_cancellation(cx, move |prompt, response_sender, _cancellation, cx| {
|
||||
password_prompt(prompt, response_sender, cx)
|
||||
})
|
||||
}
|
||||
|
||||
pub fn new_with_cancellation(
|
||||
cx: &mut AsyncApp,
|
||||
password_prompt: impl Fn(
|
||||
String,
|
||||
oneshot::Sender<EncryptedPassword>,
|
||||
oneshot::Receiver<()>,
|
||||
&mut AsyncApp,
|
||||
) + Send
|
||||
+ Sync
|
||||
+ 'static,
|
||||
) -> Self {
|
||||
let (tx, mut rx) = mpsc::unbounded::<PasswordPrompt>();
|
||||
let task = cx.spawn(async move |cx: &mut AsyncApp| {
|
||||
while let Some((prompt, channel)) = rx.next().await {
|
||||
password_prompt(prompt, channel, cx);
|
||||
while let Some((prompt, response_sender, cancellation)) = rx.next().await {
|
||||
password_prompt(prompt, response_sender, cancellation, cx);
|
||||
}
|
||||
});
|
||||
Self {
|
||||
|
|
@ -69,12 +91,17 @@ impl AskPassDelegate {
|
|||
}
|
||||
}
|
||||
|
||||
pub fn ask_password(&mut self, prompt: String) -> Task<Option<EncryptedPassword>> {
|
||||
pub fn ask_password(&self, prompt: String) -> Task<Option<EncryptedPassword>> {
|
||||
let mut this_tx = self.tx.clone();
|
||||
self.executor.spawn(async move {
|
||||
let (tx, rx) = oneshot::channel();
|
||||
this_tx.send((prompt, tx)).await.ok()?;
|
||||
rx.await.ok()
|
||||
let (response_sender, response_receiver) = oneshot::channel();
|
||||
let (cancellation_sender, cancellation_receiver) = oneshot::channel();
|
||||
this_tx
|
||||
.send((prompt, response_sender, cancellation_receiver))
|
||||
.await
|
||||
.ok()?;
|
||||
let _cancellation_sender = cancellation_sender;
|
||||
response_receiver.await.ok()
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
@ -98,7 +125,7 @@ impl AskPassSession {
|
|||
/// This will create a new AskPassSession.
|
||||
/// You must retain this session until the master process exits.
|
||||
#[must_use]
|
||||
pub async fn new(executor: BackgroundExecutor, mut delegate: AskPassDelegate) -> Result<Self> {
|
||||
pub async fn new(executor: BackgroundExecutor, delegate: AskPassDelegate) -> Result<Self> {
|
||||
#[cfg(target_os = "windows")]
|
||||
let secret = std::sync::Arc::new(std::sync::Mutex::new(None));
|
||||
|
||||
|
|
@ -587,3 +614,33 @@ fn find_gpg_program() -> Option<std::path::PathBuf> {
|
|||
.into_iter()
|
||||
.find_map(|candidate| which::which(candidate).ok())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use gpui::TestAppContext;
|
||||
|
||||
#[gpui::test]
|
||||
async fn dropping_password_request_cancels_prompt(cx: &mut TestAppContext) {
|
||||
let (prompt_sender, mut prompt_receiver) = mpsc::unbounded();
|
||||
let delegate = AskPassDelegate::new_with_cancellation(
|
||||
&mut cx.to_async(),
|
||||
move |_, response_sender, cancellation, _| {
|
||||
prompt_sender
|
||||
.unbounded_send((response_sender, cancellation))
|
||||
.expect("prompt receiver should remain open");
|
||||
},
|
||||
);
|
||||
|
||||
let password_request = delegate.ask_password("Password:".to_string());
|
||||
let (response_sender, cancellation) = prompt_receiver
|
||||
.next()
|
||||
.await
|
||||
.expect("password prompt should be received");
|
||||
|
||||
drop(password_request);
|
||||
|
||||
assert!(cancellation.await.is_err());
|
||||
drop(response_sender);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -4210,17 +4210,27 @@ impl GitPanel {
|
|||
let workspace = self.workspace.clone();
|
||||
let operation = operation.into();
|
||||
let window = window.window_handle();
|
||||
AskPassDelegate::new(&mut cx.to_async(), move |prompt, tx, cx| {
|
||||
window
|
||||
.update(cx, |_, window, cx| {
|
||||
workspace.update(cx, |workspace, cx| {
|
||||
workspace.toggle_modal(window, cx, |window, cx| {
|
||||
AskPassModal::new(operation.clone(), prompt.into(), tx, window, cx)
|
||||
});
|
||||
AskPassDelegate::new_with_cancellation(
|
||||
&mut cx.to_async(),
|
||||
move |prompt, tx, cancellation, cx| {
|
||||
window
|
||||
.update(cx, |_, window, cx| {
|
||||
workspace.update(cx, |workspace, cx| {
|
||||
workspace.toggle_modal(window, cx, |window, cx| {
|
||||
AskPassModal::new(
|
||||
operation.clone(),
|
||||
prompt.into(),
|
||||
tx,
|
||||
cancellation,
|
||||
window,
|
||||
cx,
|
||||
)
|
||||
});
|
||||
})
|
||||
})
|
||||
})
|
||||
.ok();
|
||||
})
|
||||
.ok();
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
fn can_push_and_pull(&self, cx: &App) -> bool {
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
use askpass::EncryptedPassword;
|
||||
use editor::Editor;
|
||||
use futures::channel::oneshot;
|
||||
use gpui::{AppContext, DismissEvent, Entity, EventEmitter, Focusable, Styled};
|
||||
use gpui::{AppContext, DismissEvent, Entity, EventEmitter, Focusable, Styled, Task};
|
||||
use ui::{
|
||||
ActiveTheme, AnyElement, App, Button, Clickable, Color, Context, DynamicSpacing, Headline,
|
||||
HeadlineSize, Icon, IconName, IconSize, InteractiveElement, IntoElement, Label, LabelCommon,
|
||||
|
|
@ -17,6 +17,7 @@ pub struct AskPassModal {
|
|||
prompt: SharedString,
|
||||
editor: Entity<Editor>,
|
||||
tx: Option<oneshot::Sender<EncryptedPassword>>,
|
||||
_cancellation_task: Task<()>,
|
||||
}
|
||||
|
||||
impl EventEmitter<DismissEvent> for AskPassModal {}
|
||||
|
|
@ -32,6 +33,7 @@ impl AskPassModal {
|
|||
operation: SharedString,
|
||||
prompt: SharedString,
|
||||
tx: oneshot::Sender<EncryptedPassword>,
|
||||
cancellation: oneshot::Receiver<()>,
|
||||
window: &mut Window,
|
||||
cx: &mut Context<Self>,
|
||||
) -> Self {
|
||||
|
|
@ -44,11 +46,16 @@ impl AskPassModal {
|
|||
}
|
||||
editor
|
||||
});
|
||||
let cancellation_task = cx.spawn(async move |this, cx| {
|
||||
cancellation.await.ok();
|
||||
this.update(cx, |_, cx| cx.emit(DismissEvent)).ok();
|
||||
});
|
||||
Self {
|
||||
operation,
|
||||
prompt,
|
||||
editor,
|
||||
tx: Some(tx),
|
||||
_cancellation_task: cancellation_task,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -145,3 +152,53 @@ impl Render for AskPassModal {
|
|||
.children(self.render_hint(cx))
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::sync::{Arc, atomic::AtomicBool};
|
||||
|
||||
use gpui::TestAppContext;
|
||||
use settings::SettingsStore;
|
||||
|
||||
use super::*;
|
||||
|
||||
#[gpui::test]
|
||||
fn dismisses_when_password_request_is_cancelled(cx: &mut TestAppContext) {
|
||||
cx.update(|cx| {
|
||||
let settings_store = SettingsStore::test(cx);
|
||||
cx.set_global(settings_store);
|
||||
theme_settings::init(theme::LoadThemes::JustBase, cx);
|
||||
editor::init(cx);
|
||||
});
|
||||
|
||||
let (response_sender, _response_receiver) = oneshot::channel();
|
||||
let (cancellation_sender, cancellation) = oneshot::channel();
|
||||
let window = cx.add_window(|window, cx| Editor::single_line(window, cx));
|
||||
let modal = window
|
||||
.update(cx, |_, window, cx| {
|
||||
cx.new(|cx| {
|
||||
AskPassModal::new(
|
||||
"git fetch".into(),
|
||||
"Confirm user presence".into(),
|
||||
response_sender,
|
||||
cancellation,
|
||||
window,
|
||||
cx,
|
||||
)
|
||||
})
|
||||
})
|
||||
.expect("test window should remain open");
|
||||
let dismissed = Arc::new(AtomicBool::new(false));
|
||||
let _subscription = cx.update(|cx| {
|
||||
let dismissed = dismissed.clone();
|
||||
cx.subscribe(&modal, move |_, _: &DismissEvent, _| {
|
||||
dismissed.store(true, std::sync::atomic::Ordering::SeqCst);
|
||||
})
|
||||
});
|
||||
|
||||
drop(cancellation_sender);
|
||||
cx.run_until_parked();
|
||||
|
||||
assert!(dismissed.load(std::sync::atomic::Ordering::SeqCst));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -387,17 +387,27 @@ fn create_worktree_askpass_delegate(
|
|||
) -> AskPassDelegate {
|
||||
let operation = operation.into();
|
||||
let window = window.window_handle();
|
||||
AskPassDelegate::new(&mut cx.to_async(), move |prompt, tx, cx| {
|
||||
window
|
||||
.update(cx, |_, window, cx| {
|
||||
workspace.update(cx, |workspace, cx| {
|
||||
workspace.toggle_modal(window, cx, |window, cx| {
|
||||
AskPassModal::new(operation.clone(), prompt.into(), tx, window, cx)
|
||||
});
|
||||
AskPassDelegate::new_with_cancellation(
|
||||
&mut cx.to_async(),
|
||||
move |prompt, tx, cancellation, cx| {
|
||||
window
|
||||
.update(cx, |_, window, cx| {
|
||||
workspace.update(cx, |workspace, cx| {
|
||||
workspace.toggle_modal(window, cx, |window, cx| {
|
||||
AskPassModal::new(
|
||||
operation.clone(),
|
||||
prompt.into(),
|
||||
tx,
|
||||
cancellation,
|
||||
window,
|
||||
cx,
|
||||
)
|
||||
});
|
||||
})
|
||||
})
|
||||
})
|
||||
.ok();
|
||||
})
|
||||
.ok();
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
async fn fetch_remote_for_worktree_base(
|
||||
|
|
|
|||
|
|
@ -614,7 +614,7 @@ pub struct Repository {
|
|||
job_debug_queue: job_debug_queue::GitJobDebugQueue,
|
||||
pending_ops: SumTree<PendingOps>,
|
||||
job_id: JobId,
|
||||
askpass_delegates: Arc<Mutex<HashMap<u64, AskPassDelegate>>>,
|
||||
askpass_delegates: RemoteAskPassDelegates,
|
||||
latest_askpass_id: u64,
|
||||
repository_state: Shared<Task<Result<RepositoryState, String>>>,
|
||||
initial_graph_data: HashMap<(LogSource, LogOrder), InitialGitGraphData>,
|
||||
|
|
@ -622,6 +622,42 @@ pub struct Repository {
|
|||
commit_data: HashMap<Oid, CommitDataState>,
|
||||
}
|
||||
|
||||
type RemoteAskPassDelegates = Arc<Mutex<HashMap<u64, RemoteAskPassDelegate>>>;
|
||||
|
||||
struct RemoteAskPassDelegate {
|
||||
delegate: AskPassDelegate,
|
||||
active_request_cancellation: Option<oneshot::Sender<()>>,
|
||||
}
|
||||
|
||||
struct RemoteAskPassOperation {
|
||||
askpass_id: u64,
|
||||
delegates: RemoteAskPassDelegates,
|
||||
}
|
||||
|
||||
impl RemoteAskPassOperation {
|
||||
fn new(askpass_id: u64, delegate: AskPassDelegate, delegates: RemoteAskPassDelegates) -> Self {
|
||||
let previous = delegates.lock().insert(
|
||||
askpass_id,
|
||||
RemoteAskPassDelegate {
|
||||
delegate,
|
||||
active_request_cancellation: None,
|
||||
},
|
||||
);
|
||||
debug_assert!(previous.is_none());
|
||||
Self {
|
||||
askpass_id,
|
||||
delegates,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl Drop for RemoteAskPassOperation {
|
||||
fn drop(&mut self) {
|
||||
let delegate = self.delegates.lock().remove(&self.askpass_id);
|
||||
debug_assert!(delegate.is_some());
|
||||
}
|
||||
}
|
||||
|
||||
impl std::ops::Deref for Repository {
|
||||
type Target = RepositorySnapshot;
|
||||
|
||||
|
|
@ -4469,19 +4505,12 @@ impl GitStore {
|
|||
let repository = Self::repository_for_request(&this, repository_id, &mut cx)?;
|
||||
|
||||
let delegates = cx.update(|cx| repository.read(cx).askpass_delegates.clone());
|
||||
let Some(mut askpass) = delegates.lock().remove(&envelope.payload.askpass_id) else {
|
||||
debug_panic!("no askpass found");
|
||||
anyhow::bail!("no askpass found");
|
||||
};
|
||||
|
||||
let response = askpass
|
||||
.ask_password(envelope.payload.prompt)
|
||||
.await
|
||||
.ok_or_else(|| anyhow::anyhow!("askpass cancelled"))?;
|
||||
|
||||
delegates
|
||||
.lock()
|
||||
.insert(envelope.payload.askpass_id, askpass);
|
||||
let response = request_remote_password(
|
||||
&delegates,
|
||||
envelope.payload.askpass_id,
|
||||
envelope.payload.prompt,
|
||||
)
|
||||
.await?;
|
||||
|
||||
// In fact, we don't quite know what we're doing here, as we're sending askpass password unencrypted, but..
|
||||
Ok(proto::AskPassResponse {
|
||||
|
|
@ -5812,6 +5841,41 @@ fn make_remote_delegate(
|
|||
})
|
||||
}
|
||||
|
||||
async fn request_remote_password(
|
||||
delegates: &RemoteAskPassDelegates,
|
||||
askpass_id: u64,
|
||||
prompt: String,
|
||||
) -> Result<EncryptedPassword> {
|
||||
let (password_request, operation_cancellation) = {
|
||||
let mut delegates = delegates.lock();
|
||||
let delegate = delegates
|
||||
.get_mut(&askpass_id)
|
||||
.context("remote Git operation no longer exists")?;
|
||||
if delegate.active_request_cancellation.is_some() {
|
||||
bail!("another askpass request is already active");
|
||||
}
|
||||
|
||||
let (cancellation_sender, cancellation_receiver) = oneshot::channel();
|
||||
let password_request = delegate.delegate.ask_password(prompt);
|
||||
delegate.active_request_cancellation = Some(cancellation_sender);
|
||||
(password_request.fuse(), cancellation_receiver.fuse())
|
||||
};
|
||||
|
||||
futures::pin_mut!(password_request, operation_cancellation);
|
||||
let response = futures::select_biased! {
|
||||
response = password_request => response,
|
||||
_ = operation_cancellation => None,
|
||||
};
|
||||
|
||||
let mut delegates = delegates.lock();
|
||||
let delegate = delegates
|
||||
.get_mut(&askpass_id)
|
||||
.context("remote Git operation ended while awaiting askpass")?;
|
||||
delegate.active_request_cancellation.take();
|
||||
|
||||
response.context("askpass cancelled")
|
||||
}
|
||||
|
||||
impl RepositoryId {
|
||||
pub fn to_proto(self) -> u64 {
|
||||
self.0
|
||||
|
|
@ -8301,11 +8365,8 @@ impl Repository {
|
|||
.await
|
||||
}
|
||||
RepositoryState::Remote(RemoteRepositoryState { project_id, client }) => {
|
||||
askpass_delegates.lock().insert(askpass_id, askpass);
|
||||
let _defer = util::defer(|| {
|
||||
let askpass_delegate = askpass_delegates.lock().remove(&askpass_id);
|
||||
debug_assert!(askpass_delegate.is_some());
|
||||
});
|
||||
let _askpass_operation =
|
||||
RemoteAskPassOperation::new(askpass_id, askpass, askpass_delegates);
|
||||
let (name, email) = name_and_email.unzip();
|
||||
client
|
||||
.request(proto::Commit {
|
||||
|
|
@ -8404,11 +8465,8 @@ impl Repository {
|
|||
result
|
||||
}
|
||||
RepositoryState::Remote(RemoteRepositoryState { project_id, client }) => {
|
||||
askpass_delegates.lock().insert(askpass_id, askpass);
|
||||
let _defer = util::defer(|| {
|
||||
let askpass_delegate = askpass_delegates.lock().remove(&askpass_id);
|
||||
debug_assert!(askpass_delegate.is_some());
|
||||
});
|
||||
let _askpass_operation =
|
||||
RemoteAskPassOperation::new(askpass_id, askpass, askpass_delegates);
|
||||
|
||||
let response = client
|
||||
.request(proto::Fetch {
|
||||
|
|
@ -8487,11 +8545,8 @@ impl Repository {
|
|||
result
|
||||
}
|
||||
RepositoryState::Remote(RemoteRepositoryState { project_id, client }) => {
|
||||
askpass_delegates.lock().insert(askpass_id, askpass);
|
||||
let _defer = util::defer(|| {
|
||||
let askpass_delegate = askpass_delegates.lock().remove(&askpass_id);
|
||||
debug_assert!(askpass_delegate.is_some());
|
||||
});
|
||||
let _askpass_operation =
|
||||
RemoteAskPassOperation::new(askpass_id, askpass, askpass_delegates);
|
||||
let response = client
|
||||
.request(proto::Push {
|
||||
project_id: project_id.0,
|
||||
|
|
@ -8563,11 +8618,8 @@ impl Repository {
|
|||
.await
|
||||
}
|
||||
RepositoryState::Remote(RemoteRepositoryState { project_id, client }) => {
|
||||
askpass_delegates.lock().insert(askpass_id, askpass);
|
||||
let _defer = util::defer(|| {
|
||||
let askpass_delegate = askpass_delegates.lock().remove(&askpass_id);
|
||||
debug_assert!(askpass_delegate.is_some());
|
||||
});
|
||||
let _askpass_operation =
|
||||
RemoteAskPassOperation::new(askpass_id, askpass, askpass_delegates);
|
||||
let response = client
|
||||
.request(proto::Pull {
|
||||
project_id: project_id.0,
|
||||
|
|
@ -11078,6 +11130,165 @@ mod tests {
|
|||
});
|
||||
}
|
||||
|
||||
type TestPasswordPrompt = (
|
||||
String,
|
||||
oneshot::Sender<EncryptedPassword>,
|
||||
oneshot::Receiver<()>,
|
||||
);
|
||||
|
||||
fn test_askpass_delegate(
|
||||
cx: &mut TestAppContext,
|
||||
) -> (AskPassDelegate, mpsc::UnboundedReceiver<TestPasswordPrompt>) {
|
||||
let (prompt_sender, prompt_receiver) = mpsc::unbounded();
|
||||
let delegate = AskPassDelegate::new_with_cancellation(
|
||||
&mut cx.to_async(),
|
||||
move |prompt, response_sender, cancellation, _| {
|
||||
prompt_sender
|
||||
.unbounded_send((prompt, response_sender, cancellation))
|
||||
.expect("prompt receiver should remain open");
|
||||
},
|
||||
);
|
||||
(delegate, prompt_receiver)
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn ending_remote_operation_cancels_active_askpass(cx: &mut TestAppContext) {
|
||||
let delegates = RemoteAskPassDelegates::default();
|
||||
let (delegate, mut prompts) = test_askpass_delegate(cx);
|
||||
let operation = RemoteAskPassOperation::new(1, delegate, delegates.clone());
|
||||
let password_request = cx.executor().spawn({
|
||||
let delegates = delegates.clone();
|
||||
async move { request_remote_password(&delegates, 1, "Password:".to_string()).await }
|
||||
});
|
||||
let (_, response_sender, cancellation) = prompts
|
||||
.next()
|
||||
.await
|
||||
.expect("password prompt should be received");
|
||||
|
||||
drop(operation);
|
||||
|
||||
assert!(cancellation.await.is_err());
|
||||
let Err(error) = password_request.await else {
|
||||
panic!("password request should be cancelled")
|
||||
};
|
||||
assert!(error.to_string().contains("remote Git operation ended"));
|
||||
assert!(delegates.lock().is_empty());
|
||||
drop(response_sender);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn successful_remote_askpass_allows_another_prompt(cx: &mut TestAppContext) {
|
||||
let delegates = RemoteAskPassDelegates::default();
|
||||
let (delegate, mut prompts) = test_askpass_delegate(cx);
|
||||
let operation = RemoteAskPassOperation::new(1, delegate, delegates.clone());
|
||||
|
||||
for expected_prompt in ["Username:", "Password:"] {
|
||||
let password_request =
|
||||
cx.executor().spawn({
|
||||
let delegates = delegates.clone();
|
||||
async move {
|
||||
request_remote_password(&delegates, 1, expected_prompt.to_string()).await
|
||||
}
|
||||
});
|
||||
let (prompt, response_sender, cancellation) = prompts
|
||||
.next()
|
||||
.await
|
||||
.expect("password prompt should be received");
|
||||
assert_eq!(prompt, expected_prompt);
|
||||
assert!(
|
||||
response_sender
|
||||
.send(
|
||||
EncryptedPassword::try_from("secret")
|
||||
.expect("test password should be encryptable")
|
||||
)
|
||||
.is_ok()
|
||||
);
|
||||
assert!(password_request.await.is_ok());
|
||||
assert!(cancellation.await.is_err());
|
||||
assert!(
|
||||
delegates
|
||||
.lock()
|
||||
.get(&1)
|
||||
.expect("operation should remain registered")
|
||||
.active_request_cancellation
|
||||
.is_none()
|
||||
);
|
||||
}
|
||||
|
||||
drop(operation);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn concurrent_remote_askpass_request_is_rejected(cx: &mut TestAppContext) {
|
||||
let delegates = RemoteAskPassDelegates::default();
|
||||
let (delegate, mut prompts) = test_askpass_delegate(cx);
|
||||
let operation = RemoteAskPassOperation::new(1, delegate, delegates.clone());
|
||||
let first_request = cx.executor().spawn({
|
||||
let delegates = delegates.clone();
|
||||
async move { request_remote_password(&delegates, 1, "First:".to_string()).await }
|
||||
});
|
||||
let (_, response_sender, cancellation) = prompts
|
||||
.next()
|
||||
.await
|
||||
.expect("first password prompt should be received");
|
||||
|
||||
let Err(error) = request_remote_password(&delegates, 1, "Second:".to_string()).await else {
|
||||
panic!("concurrent prompt should be rejected");
|
||||
};
|
||||
|
||||
assert!(error.to_string().contains("already active"));
|
||||
assert!(prompts.next().now_or_never().is_none());
|
||||
|
||||
drop(operation);
|
||||
assert!(cancellation.await.is_err());
|
||||
assert!(first_request.await.is_err());
|
||||
drop(response_sender);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn remote_askpass_is_rejected_after_operation_ends(cx: &mut TestAppContext) {
|
||||
let delegates = RemoteAskPassDelegates::default();
|
||||
let (delegate, mut prompts) = test_askpass_delegate(cx);
|
||||
let operation = RemoteAskPassOperation::new(1, delegate, delegates.clone());
|
||||
drop(operation);
|
||||
|
||||
let Err(error) = request_remote_password(&delegates, 1, "Password:".to_string()).await
|
||||
else {
|
||||
panic!("prompt should be rejected after operation ends");
|
||||
};
|
||||
|
||||
assert!(error.to_string().contains("no longer exists"));
|
||||
assert!(prompts.next().now_or_never().is_none());
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn late_remote_askpass_response_does_not_restore_operation(cx: &mut TestAppContext) {
|
||||
let delegates = RemoteAskPassDelegates::default();
|
||||
let (delegate, mut prompts) = test_askpass_delegate(cx);
|
||||
let operation = RemoteAskPassOperation::new(1, delegate, delegates.clone());
|
||||
let password_request = cx.executor().spawn({
|
||||
let delegates = delegates.clone();
|
||||
async move { request_remote_password(&delegates, 1, "Password:".to_string()).await }
|
||||
});
|
||||
let (_, response_sender, cancellation) = prompts
|
||||
.next()
|
||||
.await
|
||||
.expect("password prompt should be received");
|
||||
|
||||
drop(operation);
|
||||
assert!(cancellation.await.is_err());
|
||||
assert!(
|
||||
response_sender
|
||||
.send(
|
||||
EncryptedPassword::try_from("secret")
|
||||
.expect("test password should be encryptable")
|
||||
)
|
||||
.is_err()
|
||||
);
|
||||
assert!(password_request.await.is_err());
|
||||
assert!(delegates.lock().is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_is_submodule_git_dir() {
|
||||
assert!(is_submodule_git_dir(Path::new("/Foo/.git/modules/Bar")));
|
||||
|
|
|
|||
|
|
@ -49,6 +49,7 @@ impl RemoteClientDelegate for BenchmarkRemoteClient {
|
|||
&self,
|
||||
prompt: String,
|
||||
tx: oneshot::Sender<EncryptedPassword>,
|
||||
_cancellation: oneshot::Receiver<()>,
|
||||
_cx: &mut gpui::AsyncApp,
|
||||
) {
|
||||
eprintln!("SSH asking for password: {}", prompt);
|
||||
|
|
|
|||
|
|
@ -137,6 +137,7 @@ pub trait RemoteClientDelegate: Send + Sync {
|
|||
&self,
|
||||
prompt: String,
|
||||
tx: oneshot::Sender<EncryptedPassword>,
|
||||
cancellation: oneshot::Receiver<()>,
|
||||
cx: &mut AsyncApp,
|
||||
);
|
||||
fn get_download_url(
|
||||
|
|
|
|||
|
|
@ -324,6 +324,7 @@ impl RemoteClientDelegate for MockDelegate {
|
|||
&self,
|
||||
_prompt: String,
|
||||
_sender: futures::channel::oneshot::Sender<askpass::EncryptedPassword>,
|
||||
_cancellation: futures::channel::oneshot::Receiver<()>,
|
||||
_cx: &mut AsyncApp,
|
||||
) {
|
||||
unreachable!("MockDelegate::ask_password should not be called in tests")
|
||||
|
|
|
|||
|
|
@ -655,9 +655,11 @@ impl SshRemoteConnection {
|
|||
let socket = SshSocket::new(connection_options, reused_path).await?;
|
||||
(socket, None)
|
||||
} else {
|
||||
let askpass_delegate = askpass::AskPassDelegate::new(cx, {
|
||||
let askpass_delegate = askpass::AskPassDelegate::new_with_cancellation(cx, {
|
||||
let delegate = delegate.clone();
|
||||
move |prompt, tx, cx| delegate.ask_password(prompt, tx, cx)
|
||||
move |prompt, tx, cancellation, cx| {
|
||||
delegate.ask_password(prompt, tx, cancellation, cx)
|
||||
}
|
||||
});
|
||||
|
||||
let mut askpass =
|
||||
|
|
@ -717,9 +719,11 @@ impl SshRemoteConnection {
|
|||
|
||||
#[cfg(windows)]
|
||||
let (socket, master_process_option) = {
|
||||
let askpass_delegate = askpass::AskPassDelegate::new(cx, {
|
||||
let askpass_delegate = askpass::AskPassDelegate::new_with_cancellation(cx, {
|
||||
let delegate = delegate.clone();
|
||||
move |prompt, tx, cx| delegate.ask_password(prompt, tx, cx)
|
||||
move |prompt, tx, cancellation, cx| {
|
||||
delegate.ask_password(prompt, tx, cancellation, cx)
|
||||
}
|
||||
});
|
||||
|
||||
let mut askpass =
|
||||
|
|
|
|||
|
|
@ -31,4 +31,10 @@ settings.workspace = true
|
|||
theme_settings.workspace = true
|
||||
ui.workspace = true
|
||||
ui_input.workspace = true
|
||||
workspace.workspace = true
|
||||
workspace.workspace = true
|
||||
|
||||
[dev-dependencies]
|
||||
editor.workspace = true
|
||||
gpui = { workspace = true, features = ["test-support"] }
|
||||
settings = { workspace = true, features = ["test-support"] }
|
||||
theme.workspace = true
|
||||
|
|
|
|||
|
|
@ -28,6 +28,7 @@ pub struct RemoteConnectionPrompt {
|
|||
is_devcontainer: bool,
|
||||
status_message: Option<SharedString>,
|
||||
prompt: Option<(Entity<Markdown>, oneshot::Sender<EncryptedPassword>)>,
|
||||
prompt_cancellation_task: Option<Task<()>>,
|
||||
cancellation: Option<oneshot::Sender<()>>,
|
||||
editor: Arc<dyn ErasedEditor>,
|
||||
is_password_prompt: bool,
|
||||
|
|
@ -72,6 +73,7 @@ impl RemoteConnectionPrompt {
|
|||
status_message: None,
|
||||
cancellation: None,
|
||||
prompt: None,
|
||||
prompt_cancellation_task: None,
|
||||
is_password_prompt: false,
|
||||
is_masked: true,
|
||||
}
|
||||
|
|
@ -85,6 +87,7 @@ impl RemoteConnectionPrompt {
|
|||
&mut self,
|
||||
prompt: String,
|
||||
tx: oneshot::Sender<EncryptedPassword>,
|
||||
cancellation: oneshot::Receiver<()>,
|
||||
window: &mut Window,
|
||||
cx: &mut Context<Self>,
|
||||
) {
|
||||
|
|
@ -95,6 +98,14 @@ impl RemoteConnectionPrompt {
|
|||
|
||||
let markdown = cx.new(|cx| Markdown::new_text(prompt.into(), cx));
|
||||
self.prompt = Some((markdown, tx));
|
||||
self.prompt_cancellation_task = Some(cx.spawn(async move |this, cx| {
|
||||
cancellation.await.ok();
|
||||
this.update(cx, |this, cx| {
|
||||
this.prompt.take();
|
||||
cx.notify();
|
||||
})
|
||||
.ok();
|
||||
}));
|
||||
self.status_message.take();
|
||||
window.focus(&self.editor.focus_handle(cx), cx);
|
||||
cx.notify();
|
||||
|
|
@ -107,6 +118,7 @@ impl RemoteConnectionPrompt {
|
|||
|
||||
pub fn confirm(&mut self, window: &mut Window, cx: &mut Context<Self>) {
|
||||
if let Some((_, tx)) = self.prompt.take() {
|
||||
self.prompt_cancellation_task.take();
|
||||
self.status_message = Some("Connecting".into());
|
||||
|
||||
let pw = self.editor.text(cx);
|
||||
|
|
@ -241,7 +253,7 @@ impl RemoteConnectionModal {
|
|||
(options.distro_name.clone(), None, true, false)
|
||||
}
|
||||
RemoteConnectionOptions::Docker(options) => (options.name.clone(), None, false, true),
|
||||
#[cfg(any(test, feature = "test-support"))]
|
||||
#[cfg(feature = "test-support")]
|
||||
RemoteConnectionOptions::Mock(options) => {
|
||||
(format!("mock-{}", options.id), None, false, false)
|
||||
}
|
||||
|
|
@ -452,6 +464,7 @@ impl remote::RemoteClientDelegate for RemoteClientDelegate {
|
|||
&self,
|
||||
prompt: String,
|
||||
tx: oneshot::Sender<EncryptedPassword>,
|
||||
cancellation: oneshot::Receiver<()>,
|
||||
cx: &mut AsyncApp,
|
||||
) {
|
||||
let mut known_password = self.known_password.clone();
|
||||
|
|
@ -461,7 +474,7 @@ impl remote::RemoteClientDelegate for RemoteClientDelegate {
|
|||
self.window
|
||||
.update(cx, |_, window, cx| {
|
||||
self.ui.update(cx, |modal, cx| {
|
||||
modal.set_prompt(prompt, tx, window, cx);
|
||||
modal.set_prompt(prompt, tx, cancellation, window, cx);
|
||||
})
|
||||
})
|
||||
.ok();
|
||||
|
|
@ -628,6 +641,7 @@ impl remote::RemoteClientDelegate for BackgroundRemoteClientDelegate {
|
|||
&self,
|
||||
prompt: String,
|
||||
_tx: oneshot::Sender<EncryptedPassword>,
|
||||
_cancellation: oneshot::Receiver<()>,
|
||||
_cx: &mut AsyncApp,
|
||||
) {
|
||||
log::warn!(
|
||||
|
|
@ -726,3 +740,114 @@ pub fn connect(
|
|||
}
|
||||
|
||||
use anyhow::Context as _;
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use editor::Editor;
|
||||
use gpui::TestAppContext;
|
||||
use settings::SettingsStore;
|
||||
|
||||
use super::*;
|
||||
|
||||
#[gpui::test]
|
||||
fn clears_prompt_when_password_request_is_cancelled(cx: &mut TestAppContext) {
|
||||
initialize_test(cx);
|
||||
|
||||
let window = cx.add_window(|window, cx| Editor::single_line(window, cx));
|
||||
let prompt = window
|
||||
.update(cx, |_, window, cx| {
|
||||
cx.new(|cx| {
|
||||
RemoteConnectionPrompt::new(
|
||||
"example.com".to_string(),
|
||||
None,
|
||||
false,
|
||||
false,
|
||||
window,
|
||||
cx,
|
||||
)
|
||||
})
|
||||
})
|
||||
.expect("test window should remain open");
|
||||
|
||||
let (response_sender, _response_receiver) = oneshot::channel();
|
||||
let (cancellation_sender, cancellation) = oneshot::channel();
|
||||
window
|
||||
.update(cx, |_, window, cx| {
|
||||
prompt.update(cx, |prompt, cx| {
|
||||
prompt.set_prompt(
|
||||
"Confirm user presence".to_string(),
|
||||
response_sender,
|
||||
cancellation,
|
||||
window,
|
||||
cx,
|
||||
);
|
||||
});
|
||||
})
|
||||
.expect("test window should remain open");
|
||||
|
||||
drop(cancellation_sender);
|
||||
cx.run_until_parked();
|
||||
|
||||
assert!(prompt.read_with(cx, |prompt, _| prompt.prompt.is_none()));
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
fn stale_cancellation_does_not_clear_replacement_prompt(cx: &mut TestAppContext) {
|
||||
initialize_test(cx);
|
||||
|
||||
let window = cx.add_window(|window, cx| Editor::single_line(window, cx));
|
||||
let prompt = window
|
||||
.update(cx, |_, window, cx| {
|
||||
cx.new(|cx| {
|
||||
RemoteConnectionPrompt::new(
|
||||
"example.com".to_string(),
|
||||
None,
|
||||
false,
|
||||
false,
|
||||
window,
|
||||
cx,
|
||||
)
|
||||
})
|
||||
})
|
||||
.expect("test window should remain open");
|
||||
|
||||
let (first_response_sender, _first_response_receiver) = oneshot::channel();
|
||||
let (first_cancellation_sender, first_cancellation) = oneshot::channel();
|
||||
let (second_response_sender, _second_response_receiver) = oneshot::channel();
|
||||
let (_second_cancellation_sender, second_cancellation) = oneshot::channel();
|
||||
window
|
||||
.update(cx, |_, window, cx| {
|
||||
prompt.update(cx, |prompt, cx| {
|
||||
prompt.set_prompt(
|
||||
"First prompt".to_string(),
|
||||
first_response_sender,
|
||||
first_cancellation,
|
||||
window,
|
||||
cx,
|
||||
);
|
||||
prompt.set_prompt(
|
||||
"Second prompt".to_string(),
|
||||
second_response_sender,
|
||||
second_cancellation,
|
||||
window,
|
||||
cx,
|
||||
);
|
||||
});
|
||||
})
|
||||
.expect("test window should remain open");
|
||||
|
||||
drop(first_cancellation_sender);
|
||||
cx.run_until_parked();
|
||||
|
||||
assert!(prompt.read_with(cx, |prompt, _| prompt.prompt.is_some()));
|
||||
}
|
||||
|
||||
fn initialize_test(cx: &mut TestAppContext) {
|
||||
cx.update(|cx| {
|
||||
let settings_store = SettingsStore::test(cx);
|
||||
cx.set_global(settings_store);
|
||||
theme_settings::init(theme::LoadThemes::JustBase, cx);
|
||||
editor::init(cx);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue