Yehowshua/ZIm
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Add CredentialsProvider to silence keychain prompts in development (#25266)

Browse source 
This PR adds a new `CredentialsProvider` trait that abstracts over
interacting with the system keychain.

We had previously introduced a version of this scoped just to Zed auth
in https://github.com/zed-industries/zed/pull/11505.

However, after landing https://github.com/zed-industries/zed/pull/25123,
we now have a similar issue with the credentials for language model
providers that are also stored in the keychain (and thus also produce a
spam of popups when running a development build of Zed).

This PR takes the existing approach and makes it more generic, such that
we can use it everywhere that we need to read/store credentials in the
keychain.

There are still two credential provider implementations:

- `KeychainCredentialsProvider` will interact with the system keychain
(using the existing GPUI APIs)
- `DevelopmentCredentialsProvider` will use a local file on the file
system

We only use the `DevelopmentCredentialsProvider` when:

1. We are running a development build of Zed
2. The `ZED_DEVELOPMENT_AUTH` environment variable is set
- I am considering removing the need for this and making it the default,
but that will be explored in a follow-up PR.

Release Notes:

- N/A
This commit is contained in:
Marshall Bowers 2025-02-20 12:58:50 -05:00 • committed by GitHub
parent 31aad858f8
commit 21bb7242ea
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
15 changed files with 401 additions and 226 deletions
Download patch file Download diff file Expand all files Collapse all files

220
crates/client/src/client.rs
View file

@ -15,6 +15,7 @@ use async_tungstenite::tungstenite::{
};
use chrono::{DateTime, Utc};
use clock::SystemClock;
use credentials_provider::CredentialsProvider;
use futures::{
channel::oneshot, future::BoxFuture, AsyncReadExt, FutureExt, SinkExt, Stream, StreamExt,
TryFutureExt as _, TryStreamExt,
@ -57,14 +58,6 @@ static ZED_SERVER_URL: LazyLock<Option<String>> =
LazyLock::new(|| std::env::var("ZED_SERVER_URL").ok());
static ZED_RPC_URL: LazyLock<Option<String>> = LazyLock::new(|| std::env::var("ZED_RPC_URL").ok());
/// An environment variable whose presence indicates that the development auth
/// provider should be used.
///
/// Only works in development. Setting this environment variable in other release
/// channels is a no-op.
pub static ZED_DEVELOPMENT_AUTH: LazyLock<bool> = LazyLock::new(|| {
std::env::var("ZED_DEVELOPMENT_AUTH").map_or(false, |value| !value.is_empty())
});
pub static IMPERSONATE_LOGIN: LazyLock<Option<String>> = LazyLock::new(|| {
std::env::var("ZED_IMPERSONATE")
.ok()
@ -193,7 +186,7 @@ pub struct Client {
peer: Arc<Peer>,
http: Arc<HttpClientWithUrl>,
telemetry: Arc<Telemetry>,
credentials_provider: Arc<dyn CredentialsProvider + Send + Sync + 'static>,
credentials_provider: ClientCredentialsProvider,
state: RwLock<ClientState>,
handler_set: parking_lot::Mutex<ProtoMessageHandlerSet>,
@ -304,16 +297,46 @@ impl Credentials {
}
}
/// A provider for [`Credentials`].
///
/// Used to abstract over reading and writing credentials to some form of
/// persistence (like the system keychain).
trait CredentialsProvider {
pub struct ClientCredentialsProvider {
provider: Arc<dyn CredentialsProvider>,
}
impl ClientCredentialsProvider {
pub fn new(cx: &App) -> Self {
Self {
provider: <dyn CredentialsProvider>::global(cx),
}
}
fn server_url(&self, cx: &AsyncApp) -> Result<String> {
cx.update(|cx| ClientSettings::get_global(cx).server_url.clone())
}
/// Reads the credentials from the provider.
fn read_credentials<'a>(
&'a self,
cx: &'a AsyncApp,
) -> Pin<Box<dyn Future<Output = Option<Credentials>> + 'a>>;
) -> Pin<Box<dyn Future<Output = Option<Credentials>> + 'a>> {
async move {
if IMPERSONATE_LOGIN.is_some() {
return None;
}
let server_url = self.server_url(cx).ok()?;
let (user_id, access_token) = self
.provider
.read_credentials(&server_url, cx)
.await
.log_err()
.flatten()?;
Some(Credentials {
user_id: user_id.parse().ok()?,
access_token: String::from_utf8(access_token).ok()?,
})
}
.boxed_local()
}
/// Writes the credentials to the provider.
fn write_credentials<'a>(
@ -321,13 +344,32 @@ trait CredentialsProvider {
user_id: u64,
access_token: String,
cx: &'a AsyncApp,
) -> Pin<Box<dyn Future<Output = Result<()>> + 'a>>;
) -> Pin<Box<dyn Future<Output = Result<()>> + 'a>> {
async move {
let server_url = self.server_url(cx)?;
self.provider
.write_credentials(
&server_url,
&user_id.to_string(),
access_token.as_bytes(),
cx,
)
.await
}
.boxed_local()
}
/// Deletes the credentials from the provider.
fn delete_credentials<'a>(
&'a self,
cx: &'a AsyncApp,
) -> Pin<Box<dyn Future<Output = Result<()>> + 'a>>;
) -> Pin<Box<dyn Future<Output = Result<()>> + 'a>> {
async move {
let server_url = self.server_url(cx)?;
self.provider.delete_credentials(&server_url, cx).await
}
.boxed_local()
}
}
impl Default for ClientState {
@ -484,27 +526,12 @@ impl Client {
http: Arc<HttpClientWithUrl>,
cx: &mut App,
) -> Arc<Self> {
let use_zed_development_auth = match ReleaseChannel::try_global(cx) {
Some(ReleaseChannel::Dev) => *ZED_DEVELOPMENT_AUTH,
Some(ReleaseChannel::Nightly | ReleaseChannel::Preview | ReleaseChannel::Stable)
| None => false,
};
let credentials_provider: Arc<dyn CredentialsProvider + Send + Sync + 'static> =
if use_zed_development_auth {
Arc::new(DevelopmentCredentialsProvider {
path: paths::config_dir().join("development_auth"),
})
} else {
Arc::new(KeychainCredentialsProvider)
};
Arc::new(Self {
id: AtomicU64::new(0),
peer: Peer::new(0),
telemetry: Telemetry::new(clock, http.clone(), cx),
http,
credentials_provider,
credentials_provider: ClientCredentialsProvider::new(cx),
state: Default::default(),
handler_set: Default::default(),
@ -842,8 +869,7 @@ impl Client {
Ok(conn) => {
self.state.write().credentials = Some(credentials.clone());
if !read_from_provider && IMPERSONATE_LOGIN.is_none() {
self.credentials_provider.write_credentials(credentials.user_id, credentials.access_token, cx).await.log_err();
self.credentials_provider.write_credentials(credentials.user_id, credentials.access_token, cx).await.log_err();
}
futures::select_biased! {
@ -1588,130 +1614,6 @@ impl ProtoClient for Client {
}
}
#[derive(Serialize, Deserialize)]
struct DevelopmentCredentials {
user_id: u64,
access_token: String,
}
/// A credentials provider that stores credentials in a local file.
///
/// This MUST only be used in development, as this is not a secure way of storing
/// credentials on user machines.
///
/// Its existence is purely to work around the annoyance of having to constantly
/// re-allow access to the system keychain when developing Zed.
struct DevelopmentCredentialsProvider {
path: PathBuf,
}
impl CredentialsProvider for DevelopmentCredentialsProvider {
fn read_credentials<'a>(
&'a self,
_cx: &'a AsyncApp,
) -> Pin<Box<dyn Future<Output = Option<Credentials>> + 'a>> {
async move {
if IMPERSONATE_LOGIN.is_some() {
return None;
}
let json = std::fs::read(&self.path).log_err()?;
let credentials: DevelopmentCredentials = serde_json::from_slice(&json).log_err()?;
Some(Credentials {
user_id: credentials.user_id,
access_token: credentials.access_token,
})
}
.boxed_local()
}
fn write_credentials<'a>(
&'a self,
user_id: u64,
access_token: String,
_cx: &'a AsyncApp,
) -> Pin<Box<dyn Future<Output = Result<()>> + 'a>> {
async move {
let json = serde_json::to_string(&DevelopmentCredentials {
user_id,
access_token,
})?;
std::fs::write(&self.path, json)?;
Ok(())
}
.boxed_local()
}
fn delete_credentials<'a>(
&'a self,
_cx: &'a AsyncApp,
) -> Pin<Box<dyn Future<Output = Result<()>> + 'a>> {
async move { Ok(std::fs::remove_file(&self.path)?) }.boxed_local()
}
}
/// A credentials provider that stores credentials in the system keychain.
struct KeychainCredentialsProvider;
impl CredentialsProvider for KeychainCredentialsProvider {
fn read_credentials<'a>(
&'a self,
cx: &'a AsyncApp,
) -> Pin<Box<dyn Future<Output = Option<Credentials>> + 'a>> {
async move {
if IMPERSONATE_LOGIN.is_some() {
return None;
}
let (user_id, access_token) = cx
.update(|cx| cx.read_credentials(&ClientSettings::get_global(cx).server_url))
.log_err()?
.await
.log_err()??;
Some(Credentials {
user_id: user_id.parse().ok()?,
access_token: String::from_utf8(access_token).ok()?,
})
}
.boxed_local()
}
fn write_credentials<'a>(
&'a self,
user_id: u64,
access_token: String,
cx: &'a AsyncApp,
) -> Pin<Box<dyn Future<Output = Result<()>> + 'a>> {
async move {
cx.update(move |cx| {
cx.write_credentials(
&ClientSettings::get_global(cx).server_url,
&user_id.to_string(),
access_token.as_bytes(),
)
})?
.await
}
.boxed_local()
}
fn delete_credentials<'a>(
&'a self,
cx: &'a AsyncApp,
) -> Pin<Box<dyn Future<Output = Result<()>> + 'a>> {
async move {
cx.update(move |cx| cx.delete_credentials(&ClientSettings::get_global(cx).server_url))?
.await
}
.boxed_local()
}
}
/// prefix for the zed:// url scheme
pub const ZED_URL_SCHEME: &str = "zed";