Skip to content

Commit 6b553e4

Browse files
authored
collab: Remove leftover impersonation code (#49314)
This PR removes some code left over from how we used to do impersonation for local development. The local development impersonation path is separate now, so we don't need all of this plumbing in Collab. Release Notes: - N/A
1 parent e269569 commit 6b553e4

7 files changed

Lines changed: 16 additions & 264 deletions

File tree

crates/client/src/client.rs

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ use settings::{RegisterSetting, Settings, SettingsContent};
3434
use std::{
3535
any::TypeId,
3636
convert::TryFrom,
37-
fmt::Write as _,
3837
future::Future,
3938
marker::PhantomData,
4039
path::PathBuf,
@@ -1390,16 +1389,11 @@ impl Client {
13901389

13911390
// Open the Zed sign-in page in the user's browser, with query parameters that indicate
13921391
// that the user is signing in from a Zed app running on the same device.
1393-
let mut url = http.build_url(&format!(
1392+
let url = http.build_url(&format!(
13941393
"/native_app_signin?native_app_port={}&native_app_public_key={}",
13951394
port, public_key_string
13961395
));
13971396

1398-
if let Some(impersonate_login) = IMPERSONATE_LOGIN.as_ref() {
1399-
log::info!("impersonating user @{}", impersonate_login);
1400-
write!(&mut url, "&impersonate={}", impersonate_login).unwrap();
1401-
}
1402-
14031397
open_url_tx.send(url).log_err();
14041398

14051399
#[derive(Deserialize)]

crates/collab/src/auth.rs

Lines changed: 4 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -64,20 +64,7 @@ pub async fn validate_header<B>(mut req: Request<B>, next: Next<B>) -> impl Into
6464
)
6565
})?;
6666

67-
// In development, allow impersonation using the admin API token.
68-
// Don't allow this in production because we can't tell who is doing
69-
// the impersonating.
70-
let validate_result = if let (Some(admin_token), true) = (
71-
access_token.strip_prefix("ADMIN_TOKEN:"),
72-
state.config.is_development(),
73-
) {
74-
Ok(VerifyAccessTokenResult {
75-
is_valid: state.config.api_token == admin_token,
76-
impersonator_id: None,
77-
})
78-
} else {
79-
verify_access_token(access_token, user_id, &state.db).await
80-
};
67+
let validate_result = verify_access_token(access_token, user_id, &state.db).await;
8168

8269
if let Ok(validate_result) = validate_result
8370
&& validate_result.is_valid
@@ -88,17 +75,7 @@ pub async fn validate_header<B>(mut req: Request<B>, next: Next<B>) -> impl Into
8875
.await?
8976
.with_context(|| format!("user {user_id} not found"))?;
9077

91-
if let Some(impersonator_id) = validate_result.impersonator_id {
92-
let admin = state
93-
.db
94-
.get_user_by_id(impersonator_id)
95-
.await?
96-
.with_context(|| format!("user {impersonator_id} not found"))?;
97-
req.extensions_mut()
98-
.insert(Principal::Impersonated { user, admin });
99-
} else {
100-
req.extensions_mut().insert(Principal::User(user));
101-
};
78+
req.extensions_mut().insert(Principal::User(user));
10279
return Ok::<_, Error>(next.run(req).await);
10380
}
10481

@@ -125,7 +102,6 @@ pub fn hash_access_token(token: &str) -> String {
125102

126103
pub struct VerifyAccessTokenResult {
127104
pub is_valid: bool,
128-
pub impersonator_id: Option<UserId>,
129105
}
130106

131107
/// Checks that the given access token is valid for the given user.
@@ -147,8 +123,7 @@ pub async fn verify_access_token(
147123
let token: AccessTokenJson = serde_json::from_str(token)?;
148124

149125
let db_token = db.get_access_token(token.id).await?;
150-
let token_user_id = db_token.impersonated_user_id.unwrap_or(db_token.user_id);
151-
if token_user_id != user_id {
126+
if db_token.user_id != user_id {
152127
return Err(anyhow::anyhow!("no such access token"))?;
153128
}
154129
let t0 = Instant::now();
@@ -172,12 +147,5 @@ pub async fn verify_access_token(
172147
db.update_access_token_hash(db_token.id, &new_hash).await?;
173148
}
174149

175-
Ok(VerifyAccessTokenResult {
176-
is_valid,
177-
impersonator_id: if db_token.impersonated_user_id.is_some() {
178-
Some(db_token.user_id)
179-
} else {
180-
None
181-
},
182-
})
150+
Ok(VerifyAccessTokenResult { is_valid })
183151
}

crates/collab/src/db/queries/access_tokens.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ impl Database {
77
pub async fn create_access_token(
88
&self,
99
user_id: UserId,
10-
impersonated_user_id: Option<UserId>,
1110
access_token_hash: &str,
1211
max_access_token_count: usize,
1312
) -> Result<AccessTokenId> {
@@ -18,7 +17,6 @@ impl Database {
1817

1918
let token = access_token::ActiveModel {
2019
user_id: ActiveValue::set(user_id),
21-
impersonated_user_id: ActiveValue::set(impersonated_user_id),
2220
hash: ActiveValue::set(access_token_hash.into()),
2321
..Default::default()
2422
}

crates/collab/src/db/tables/access_token.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ pub struct Model {
77
#[sea_orm(primary_key)]
88
pub id: AccessTokenId,
99
pub user_id: UserId,
10-
pub impersonated_user_id: Option<UserId>,
1110
pub hash: String,
1211
}
1312

crates/collab/src/rpc.rs

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,6 @@ impl<R: RequestMessage> Response<R> {
130130
#[derive(Clone, Debug)]
131131
pub enum Principal {
132132
User(User),
133-
Impersonated { user: User, admin: User },
134133
}
135134

136135
impl Principal {
@@ -140,11 +139,6 @@ impl Principal {
140139
span.record("user_id", user.id.0);
141140
span.record("login", &user.github_login);
142141
}
143-
Principal::Impersonated { user, admin } => {
144-
span.record("user_id", user.id.0);
145-
span.record("login", &user.github_login);
146-
span.record("impersonator", &admin.github_login);
147-
}
148142
}
149143
}
150144
}
@@ -225,14 +219,12 @@ impl Session {
225219
fn is_staff(&self) -> bool {
226220
match &self.principal {
227221
Principal::User(user) => user.admin,
228-
Principal::Impersonated { .. } => true,
229222
}
230223
}
231224

232225
fn user_id(&self) -> UserId {
233226
match &self.principal {
234227
Principal::User(user) => user.id,
235-
Principal::Impersonated { user, .. } => user.id,
236228
}
237229
}
238230
}
@@ -244,10 +236,6 @@ impl Debug for Session {
244236
Principal::User(user) => {
245237
result.field("user", &user.github_login);
246238
}
247-
Principal::Impersonated { user, admin } => {
248-
result.field("user", &user.github_login);
249-
result.field("impersonator", &admin.github_login);
250-
}
251239
}
252240
result.field("connection_id", &self.connection_id).finish()
253241
}
@@ -763,7 +751,6 @@ impl Server {
763751
connection_id=field::Empty,
764752
user_id=field::Empty,
765753
login=field::Empty,
766-
impersonator=field::Empty,
767754
user_agent=field::Empty,
768755
geoip_country_code=field::Empty,
769756
release_channel=field::Empty,
@@ -872,7 +859,6 @@ impl Server {
872859
concurrent_handlers,
873860
user_id=field::Empty,
874861
login=field::Empty,
875-
impersonator=field::Empty,
876862
lsp_query_request=field::Empty,
877863
release_channel=field::Empty,
878864
{ TOTAL_DURATION_MS }=field::Empty,
@@ -936,7 +922,7 @@ impl Server {
936922
}
937923

938924
match &session.principal {
939-
Principal::User(user) | Principal::Impersonated { user, admin: _ } => {
925+
Principal::User(user) => {
940926
if !user.connected_once {
941927
self.peer.send(connection_id, proto::ShowContacts {})?;
942928
self.app_state

crates/collab/tests/integration/collab_tests.rs

Lines changed: 10 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -65,21 +65,12 @@ mod auth_token_tests {
6565

6666
const MAX_ACCESS_TOKENS_TO_STORE: usize = 8;
6767

68-
async fn create_access_token(
69-
db: &db::Database,
70-
user_id: UserId,
71-
impersonated_user_id: Option<UserId>,
72-
) -> Result<String> {
68+
async fn create_access_token(db: &db::Database, user_id: UserId) -> Result<String> {
7369
const VERSION: usize = 1;
7470
let access_token = ::rpc::auth::random_token();
7571
let access_token_hash = hash_access_token(&access_token);
7672
let id = db
77-
.create_access_token(
78-
user_id,
79-
impersonated_user_id,
80-
&access_token_hash,
81-
MAX_ACCESS_TOKENS_TO_STORE,
82-
)
73+
.create_access_token(user_id, &access_token_hash, MAX_ACCESS_TOKENS_TO_STORE)
8374
.await?;
8475
Ok(serde_json::to_string(&AccessTokenJson {
8576
version: VERSION,
@@ -106,16 +97,13 @@ mod auth_token_tests {
10697
.await
10798
.unwrap();
10899

109-
let token = create_access_token(db, user.user_id, None).await.unwrap();
100+
let token = create_access_token(db, user.user_id).await.unwrap();
110101
assert!(matches!(
111102
verify_access_token(&token, user.user_id, db).await.unwrap(),
112-
VerifyAccessTokenResult {
113-
is_valid: true,
114-
impersonator_id: None,
115-
}
103+
VerifyAccessTokenResult { is_valid: true }
116104
));
117105

118-
let old_token = create_previous_access_token(user.user_id, None, db)
106+
let old_token = create_previous_access_token(user.user_id, db)
119107
.await
120108
.unwrap();
121109

@@ -139,10 +127,7 @@ mod auth_token_tests {
139127
verify_access_token(&old_token, user.user_id, db)
140128
.await
141129
.unwrap(),
142-
VerifyAccessTokenResult {
143-
is_valid: true,
144-
impersonator_id: None,
145-
}
130+
VerifyAccessTokenResult { is_valid: true }
146131
));
147132

148133
let hash = db
@@ -161,35 +146,20 @@ mod auth_token_tests {
161146
verify_access_token(&old_token, user.user_id, db)
162147
.await
163148
.unwrap(),
164-
VerifyAccessTokenResult {
165-
is_valid: true,
166-
impersonator_id: None,
167-
}
149+
VerifyAccessTokenResult { is_valid: true }
168150
));
169151

170152
assert!(matches!(
171153
verify_access_token(&token, user.user_id, db).await.unwrap(),
172-
VerifyAccessTokenResult {
173-
is_valid: true,
174-
impersonator_id: None,
175-
}
154+
VerifyAccessTokenResult { is_valid: true }
176155
));
177156
}
178157

179-
async fn create_previous_access_token(
180-
user_id: UserId,
181-
impersonated_user_id: Option<UserId>,
182-
db: &Database,
183-
) -> Result<String> {
158+
async fn create_previous_access_token(user_id: UserId, db: &Database) -> Result<String> {
184159
let access_token = collab::auth::random_token();
185160
let access_token_hash = previous_hash_access_token(&access_token)?;
186161
let id = db
187-
.create_access_token(
188-
user_id,
189-
impersonated_user_id,
190-
&access_token_hash,
191-
MAX_ACCESS_TOKENS_TO_STORE,
192-
)
162+
.create_access_token(user_id, &access_token_hash, MAX_ACCESS_TOKENS_TO_STORE)
193163
.await?;
194164
Ok(serde_json::to_string(&AccessTokenJson {
195165
version: 1,

0 commit comments

Comments
 (0)