diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index 2731462a5a4dd5c9da173c637fee773a99925e89..55d7eb5dce2be133e8c48dd23e8f34ebb68c50bf 100644 --- a/crates/client/src/client.rs +++ b/crates/client/src/client.rs @@ -34,7 +34,6 @@ use settings::{RegisterSetting, Settings, SettingsContent}; use std::{ any::TypeId, convert::TryFrom, - fmt::Write as _, future::Future, marker::PhantomData, path::PathBuf, @@ -1390,16 +1389,11 @@ impl Client { // Open the Zed sign-in page in the user's browser, with query parameters that indicate // that the user is signing in from a Zed app running on the same device. - let mut url = http.build_url(&format!( + let url = http.build_url(&format!( "/native_app_signin?native_app_port={}&native_app_public_key={}", port, public_key_string )); - if let Some(impersonate_login) = IMPERSONATE_LOGIN.as_ref() { - log::info!("impersonating user @{}", impersonate_login); - write!(&mut url, "&impersonate={}", impersonate_login).unwrap(); - } - open_url_tx.send(url).log_err(); #[derive(Deserialize)] diff --git a/crates/collab/src/auth.rs b/crates/collab/src/auth.rs index b844bdcd58b3aad706e30611d8f37aad61d29b13..f87f97c453f209de09c77475e73171d2a6863ce7 100644 --- a/crates/collab/src/auth.rs +++ b/crates/collab/src/auth.rs @@ -64,20 +64,7 @@ pub async fn validate_header(mut req: Request, next: Next) -> impl Into ) })?; - // In development, allow impersonation using the admin API token. - // Don't allow this in production because we can't tell who is doing - // the impersonating. - let validate_result = if let (Some(admin_token), true) = ( - access_token.strip_prefix("ADMIN_TOKEN:"), - state.config.is_development(), - ) { - Ok(VerifyAccessTokenResult { - is_valid: state.config.api_token == admin_token, - impersonator_id: None, - }) - } else { - verify_access_token(access_token, user_id, &state.db).await - }; + let validate_result = verify_access_token(access_token, user_id, &state.db).await; if let Ok(validate_result) = validate_result && validate_result.is_valid @@ -88,17 +75,7 @@ pub async fn validate_header(mut req: Request, next: Next) -> impl Into .await? .with_context(|| format!("user {user_id} not found"))?; - if let Some(impersonator_id) = validate_result.impersonator_id { - let admin = state - .db - .get_user_by_id(impersonator_id) - .await? - .with_context(|| format!("user {impersonator_id} not found"))?; - req.extensions_mut() - .insert(Principal::Impersonated { user, admin }); - } else { - req.extensions_mut().insert(Principal::User(user)); - }; + req.extensions_mut().insert(Principal::User(user)); return Ok::<_, Error>(next.run(req).await); } @@ -125,7 +102,6 @@ pub fn hash_access_token(token: &str) -> String { pub struct VerifyAccessTokenResult { pub is_valid: bool, - pub impersonator_id: Option, } /// Checks that the given access token is valid for the given user. @@ -147,8 +123,7 @@ pub async fn verify_access_token( let token: AccessTokenJson = serde_json::from_str(token)?; let db_token = db.get_access_token(token.id).await?; - let token_user_id = db_token.impersonated_user_id.unwrap_or(db_token.user_id); - if token_user_id != user_id { + if db_token.user_id != user_id { return Err(anyhow::anyhow!("no such access token"))?; } let t0 = Instant::now(); @@ -172,12 +147,5 @@ pub async fn verify_access_token( db.update_access_token_hash(db_token.id, &new_hash).await?; } - Ok(VerifyAccessTokenResult { - is_valid, - impersonator_id: if db_token.impersonated_user_id.is_some() { - Some(db_token.user_id) - } else { - None - }, - }) + Ok(VerifyAccessTokenResult { is_valid }) } diff --git a/crates/collab/src/db/queries/access_tokens.rs b/crates/collab/src/db/queries/access_tokens.rs index c7921639d206fe0ff086df790234fa1be7f45658..0e06b67693f505c5883c2b2c52f625ccb17601a9 100644 --- a/crates/collab/src/db/queries/access_tokens.rs +++ b/crates/collab/src/db/queries/access_tokens.rs @@ -7,7 +7,6 @@ impl Database { pub async fn create_access_token( &self, user_id: UserId, - impersonated_user_id: Option, access_token_hash: &str, max_access_token_count: usize, ) -> Result { @@ -18,7 +17,6 @@ impl Database { let token = access_token::ActiveModel { user_id: ActiveValue::set(user_id), - impersonated_user_id: ActiveValue::set(impersonated_user_id), hash: ActiveValue::set(access_token_hash.into()), ..Default::default() } diff --git a/crates/collab/src/db/tables/access_token.rs b/crates/collab/src/db/tables/access_token.rs index 22635fb64d94538687eac590efaf75049c64c864..da7392b98c444f3d83fd549525f9af4a2eb125d3 100644 --- a/crates/collab/src/db/tables/access_token.rs +++ b/crates/collab/src/db/tables/access_token.rs @@ -7,7 +7,6 @@ pub struct Model { #[sea_orm(primary_key)] pub id: AccessTokenId, pub user_id: UserId, - pub impersonated_user_id: Option, pub hash: String, } diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index 8dea69d973a74472e5a7e8c49a4b8dd53b959218..087dbe2a0ba23851689e75401c62b64775cf2282 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -130,7 +130,6 @@ impl Response { #[derive(Clone, Debug)] pub enum Principal { User(User), - Impersonated { user: User, admin: User }, } impl Principal { @@ -140,11 +139,6 @@ impl Principal { span.record("user_id", user.id.0); span.record("login", &user.github_login); } - Principal::Impersonated { user, admin } => { - span.record("user_id", user.id.0); - span.record("login", &user.github_login); - span.record("impersonator", &admin.github_login); - } } } } @@ -225,14 +219,12 @@ impl Session { fn is_staff(&self) -> bool { match &self.principal { Principal::User(user) => user.admin, - Principal::Impersonated { .. } => true, } } fn user_id(&self) -> UserId { match &self.principal { Principal::User(user) => user.id, - Principal::Impersonated { user, .. } => user.id, } } } @@ -244,10 +236,6 @@ impl Debug for Session { Principal::User(user) => { result.field("user", &user.github_login); } - Principal::Impersonated { user, admin } => { - result.field("user", &user.github_login); - result.field("impersonator", &admin.github_login); - } } result.field("connection_id", &self.connection_id).finish() } @@ -763,7 +751,6 @@ impl Server { connection_id=field::Empty, user_id=field::Empty, login=field::Empty, - impersonator=field::Empty, user_agent=field::Empty, geoip_country_code=field::Empty, release_channel=field::Empty, @@ -872,7 +859,6 @@ impl Server { concurrent_handlers, user_id=field::Empty, login=field::Empty, - impersonator=field::Empty, lsp_query_request=field::Empty, release_channel=field::Empty, { TOTAL_DURATION_MS }=field::Empty, @@ -936,7 +922,7 @@ impl Server { } match &session.principal { - Principal::User(user) | Principal::Impersonated { user, admin: _ } => { + Principal::User(user) => { if !user.connected_once { self.peer.send(connection_id, proto::ShowContacts {})?; self.app_state diff --git a/crates/collab/tests/integration/collab_tests.rs b/crates/collab/tests/integration/collab_tests.rs index c5d3e05c48dff81ea6589c2aa0c65f809c4da4f9..3376f12b203c532cfb77f561f6300a832eafc6e1 100644 --- a/crates/collab/tests/integration/collab_tests.rs +++ b/crates/collab/tests/integration/collab_tests.rs @@ -65,21 +65,12 @@ mod auth_token_tests { const MAX_ACCESS_TOKENS_TO_STORE: usize = 8; - async fn create_access_token( - db: &db::Database, - user_id: UserId, - impersonated_user_id: Option, - ) -> Result { + async fn create_access_token(db: &db::Database, user_id: UserId) -> Result { const VERSION: usize = 1; let access_token = ::rpc::auth::random_token(); let access_token_hash = hash_access_token(&access_token); let id = db - .create_access_token( - user_id, - impersonated_user_id, - &access_token_hash, - MAX_ACCESS_TOKENS_TO_STORE, - ) + .create_access_token(user_id, &access_token_hash, MAX_ACCESS_TOKENS_TO_STORE) .await?; Ok(serde_json::to_string(&AccessTokenJson { version: VERSION, @@ -106,16 +97,13 @@ mod auth_token_tests { .await .unwrap(); - let token = create_access_token(db, user.user_id, None).await.unwrap(); + let token = create_access_token(db, user.user_id).await.unwrap(); assert!(matches!( verify_access_token(&token, user.user_id, db).await.unwrap(), - VerifyAccessTokenResult { - is_valid: true, - impersonator_id: None, - } + VerifyAccessTokenResult { is_valid: true } )); - let old_token = create_previous_access_token(user.user_id, None, db) + let old_token = create_previous_access_token(user.user_id, db) .await .unwrap(); @@ -139,10 +127,7 @@ mod auth_token_tests { verify_access_token(&old_token, user.user_id, db) .await .unwrap(), - VerifyAccessTokenResult { - is_valid: true, - impersonator_id: None, - } + VerifyAccessTokenResult { is_valid: true } )); let hash = db @@ -161,35 +146,20 @@ mod auth_token_tests { verify_access_token(&old_token, user.user_id, db) .await .unwrap(), - VerifyAccessTokenResult { - is_valid: true, - impersonator_id: None, - } + VerifyAccessTokenResult { is_valid: true } )); assert!(matches!( verify_access_token(&token, user.user_id, db).await.unwrap(), - VerifyAccessTokenResult { - is_valid: true, - impersonator_id: None, - } + VerifyAccessTokenResult { is_valid: true } )); } - async fn create_previous_access_token( - user_id: UserId, - impersonated_user_id: Option, - db: &Database, - ) -> Result { + async fn create_previous_access_token(user_id: UserId, db: &Database) -> Result { let access_token = collab::auth::random_token(); let access_token_hash = previous_hash_access_token(&access_token)?; let id = db - .create_access_token( - user_id, - impersonated_user_id, - &access_token_hash, - MAX_ACCESS_TOKENS_TO_STORE, - ) + .create_access_token(user_id, &access_token_hash, MAX_ACCESS_TOKENS_TO_STORE) .await?; Ok(serde_json::to_string(&AccessTokenJson { version: 1, diff --git a/crates/collab/tests/integration/db_tests/db_tests.rs b/crates/collab/tests/integration/db_tests/db_tests.rs index 0b3de7eea6febf14dcb3ed43eea1973b0361d439..e2006b7fb9984c4bd0cf16a62e9321b2f7007e9e 100644 --- a/crates/collab/tests/integration/db_tests/db_tests.rs +++ b/crates/collab/tests/integration/db_tests/db_tests.rs @@ -136,169 +136,6 @@ async fn test_update_or_create_user_by_github_account(db: &Arc) { assert_eq!(user.email_address, Some("user3@example.com".into())); } -test_both_dbs!( - test_create_access_tokens, - test_create_access_tokens_postgres, - test_create_access_tokens_sqlite -); - -async fn test_create_access_tokens(db: &Arc) { - let user_1 = db - .create_user( - "u1@example.com", - None, - false, - NewUserParams { - github_login: "u1".into(), - github_user_id: 1, - }, - ) - .await - .unwrap() - .user_id; - let user_2 = db - .create_user( - "u2@example.com", - None, - false, - NewUserParams { - github_login: "u2".into(), - github_user_id: 2, - }, - ) - .await - .unwrap() - .user_id; - - let token_1 = db.create_access_token(user_1, None, "h1", 2).await.unwrap(); - let token_2 = db.create_access_token(user_1, None, "h2", 2).await.unwrap(); - assert_eq!( - db.get_access_token(token_1).await.unwrap(), - access_token::Model { - id: token_1, - user_id: user_1, - impersonated_user_id: None, - hash: "h1".into(), - } - ); - assert_eq!( - db.get_access_token(token_2).await.unwrap(), - access_token::Model { - id: token_2, - user_id: user_1, - impersonated_user_id: None, - hash: "h2".into() - } - ); - - let token_3 = db.create_access_token(user_1, None, "h3", 2).await.unwrap(); - assert_eq!( - db.get_access_token(token_3).await.unwrap(), - access_token::Model { - id: token_3, - user_id: user_1, - impersonated_user_id: None, - hash: "h3".into() - } - ); - assert_eq!( - db.get_access_token(token_2).await.unwrap(), - access_token::Model { - id: token_2, - user_id: user_1, - impersonated_user_id: None, - hash: "h2".into() - } - ); - assert!(db.get_access_token(token_1).await.is_err()); - - let token_4 = db.create_access_token(user_1, None, "h4", 2).await.unwrap(); - assert_eq!( - db.get_access_token(token_4).await.unwrap(), - access_token::Model { - id: token_4, - user_id: user_1, - impersonated_user_id: None, - hash: "h4".into() - } - ); - assert_eq!( - db.get_access_token(token_3).await.unwrap(), - access_token::Model { - id: token_3, - user_id: user_1, - impersonated_user_id: None, - hash: "h3".into() - } - ); - assert!(db.get_access_token(token_2).await.is_err()); - assert!(db.get_access_token(token_1).await.is_err()); - - // An access token for user 2 impersonating user 1 does not - // count against user 1's access token limit (of 2). - let token_5 = db - .create_access_token(user_2, Some(user_1), "h5", 2) - .await - .unwrap(); - assert_eq!( - db.get_access_token(token_5).await.unwrap(), - access_token::Model { - id: token_5, - user_id: user_2, - impersonated_user_id: Some(user_1), - hash: "h5".into() - } - ); - assert_eq!( - db.get_access_token(token_3).await.unwrap(), - access_token::Model { - id: token_3, - user_id: user_1, - impersonated_user_id: None, - hash: "h3".into() - } - ); - - // Only a limited number (2) of access tokens are stored for user 2 - // impersonating other users. - let token_6 = db - .create_access_token(user_2, Some(user_1), "h6", 2) - .await - .unwrap(); - let token_7 = db - .create_access_token(user_2, Some(user_1), "h7", 2) - .await - .unwrap(); - assert_eq!( - db.get_access_token(token_6).await.unwrap(), - access_token::Model { - id: token_6, - user_id: user_2, - impersonated_user_id: Some(user_1), - hash: "h6".into() - } - ); - assert_eq!( - db.get_access_token(token_7).await.unwrap(), - access_token::Model { - id: token_7, - user_id: user_2, - impersonated_user_id: Some(user_1), - hash: "h7".into() - } - ); - assert!(db.get_access_token(token_5).await.is_err()); - assert_eq!( - db.get_access_token(token_3).await.unwrap(), - access_token::Model { - id: token_3, - user_id: user_1, - impersonated_user_id: None, - hash: "h3".into() - } - ); -} - test_both_dbs!( test_add_contacts, test_add_contacts_postgres,