Skip to content

Commit 28d359a

Browse files
committed
Define admin users and extend AuthCheck to handle them
This adds a concept of admin users, who are defined by their GitHub IDs, and allows them to be defined through an environment variable, falling back to a static list of the current `crates.io` team. `AuthCheck` now has a builder method to require that the current cookie or token belong to an admin user. In the future, this will be extended to use Rust's team API for the fallback.
1 parent aeb2ec2 commit 28d359a

File tree

5 files changed

+195
-17
lines changed

5 files changed

+195
-17
lines changed

.env.sample

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,3 +86,7 @@ export SENTRY_ENV_API=local
8686
# export TEST_S3_INDEX_REGION=http://127.0.0.1:19000
8787
# export TEST_AWS_ACCESS_KEY=minio
8888
# export TEST_AWS_SECRET_KEY=miniominio
89+
90+
# IDs of GitHub users that are admins on this instance, separated by commas.
91+
# Whitespace will be ignored.
92+
export GH_ADMIN_USER_IDS=

src/auth.rs

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1+
use std::collections::HashSet;
2+
13
use crate::controllers;
24
use crate::controllers::util::RequestPartsExt;
5+
use crate::middleware::app::RequestApp;
36
use crate::middleware::log_request::RequestLogExt;
47
use crate::middleware::session::RequestSession;
58
use crate::models::token::{CrateScope, EndpointScope};
@@ -16,6 +19,7 @@ pub struct AuthCheck {
1619
allow_token: bool,
1720
endpoint_scope: Option<EndpointScope>,
1821
crate_name: Option<String>,
22+
require_admin: bool,
1923
}
2024

2125
impl AuthCheck {
@@ -27,6 +31,7 @@ impl AuthCheck {
2731
allow_token: true,
2832
endpoint_scope: None,
2933
crate_name: None,
34+
require_admin: false,
3035
}
3136
}
3237

@@ -36,6 +41,7 @@ impl AuthCheck {
3641
allow_token: false,
3742
endpoint_scope: None,
3843
crate_name: None,
44+
require_admin: false,
3945
}
4046
}
4147

@@ -44,6 +50,7 @@ impl AuthCheck {
4450
allow_token: self.allow_token,
4551
endpoint_scope: Some(endpoint_scope),
4652
crate_name: self.crate_name.clone(),
53+
require_admin: self.require_admin,
4754
}
4855
}
4956

@@ -52,6 +59,16 @@ impl AuthCheck {
5259
allow_token: self.allow_token,
5360
endpoint_scope: self.endpoint_scope,
5461
crate_name: Some(crate_name.to_string()),
62+
require_admin: self.require_admin,
63+
}
64+
}
65+
66+
pub fn require_admin(&self) -> Self {
67+
Self {
68+
allow_token: self.allow_token,
69+
endpoint_scope: self.endpoint_scope,
70+
crate_name: self.crate_name.clone(),
71+
require_admin: true,
5572
}
5673
}
5774

@@ -61,8 +78,17 @@ impl AuthCheck {
6178
request: &T,
6279
conn: &mut PgConnection,
6380
) -> AppResult<Authentication> {
64-
let auth = authenticate(request, conn)?;
81+
self.check_authentication(
82+
authenticate(request, conn)?,
83+
&request.app().config.gh_admin_user_ids,
84+
)
85+
}
6586

87+
fn check_authentication(
88+
&self,
89+
auth: Authentication,
90+
gh_admin_user_ids: &HashSet<i32>,
91+
) -> AppResult<Authentication> {
6692
if let Some(token) = auth.api_token() {
6793
if !self.allow_token {
6894
let error_message =
@@ -81,6 +107,11 @@ impl AuthCheck {
81107
}
82108
}
83109

110+
if self.require_admin && !gh_admin_user_ids.contains(&auth.user().gh_id) {
111+
let error_message = "User is unauthorized";
112+
return Err(internal(error_message).chain(forbidden()));
113+
}
114+
84115
Ok(auth)
85116
}
86117

@@ -347,4 +378,43 @@ mod tests {
347378
assert!(!auth_check.crate_scope_matches(Some(&vec![cs("anyhow")])));
348379
assert!(!auth_check.crate_scope_matches(Some(&vec![cs("actix-*")])));
349380
}
381+
382+
#[test]
383+
fn require_admin() {
384+
let auth_check = AuthCheck::default().require_admin();
385+
let gh_admin_user_ids = [42, 43].into_iter().collect();
386+
387+
assert_ok!(auth_check.check_authentication(mock_cookie(42), &gh_admin_user_ids));
388+
assert_err!(auth_check.check_authentication(mock_cookie(44), &gh_admin_user_ids));
389+
assert_ok!(auth_check.check_authentication(mock_token(43), &gh_admin_user_ids));
390+
assert_err!(auth_check.check_authentication(mock_token(45), &gh_admin_user_ids));
391+
}
392+
393+
fn mock_user(gh_id: i32) -> User {
394+
User {
395+
id: 3,
396+
gh_access_token: "arbitrary".into(),
397+
gh_login: "literally_anything".into(),
398+
name: None,
399+
gh_avatar: None,
400+
gh_id,
401+
account_lock_reason: None,
402+
account_lock_until: None,
403+
}
404+
}
405+
406+
fn mock_cookie(gh_id: i32) -> Authentication {
407+
Authentication::Cookie(CookieAuthentication {
408+
user: mock_user(gh_id),
409+
})
410+
}
411+
412+
fn mock_token(gh_id: i32) -> Authentication {
413+
Authentication::Token(TokenAuthentication {
414+
token: crate::models::token::tests::build_mock()
415+
.user_id(gh_id)
416+
.token(),
417+
user: mock_user(gh_id),
418+
})
419+
}
350420
}

src/config/server.rs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,19 @@ use crate::storage::StorageConfig;
1212
use http::HeaderValue;
1313
use std::collections::HashSet;
1414
use std::net::IpAddr;
15+
use std::num::ParseIntError;
16+
use std::str::FromStr;
1517
use std::time::Duration;
1618

1719
const DEFAULT_VERSION_ID_CACHE_SIZE: u64 = 10_000;
1820
const DEFAULT_VERSION_ID_CACHE_TTL: u64 = 5 * 60; // 5 minutes
1921

22+
// The defaults correspond to the current crates.io team, which as at the time of writing, is the
23+
// GitHub user names carols10cents, jtgeibel, Turbo87, JohnTitor, LawnGnome, and mdtro.
24+
//
25+
// FIXME: this needs to be removed once we can detect the admins from the Rust teams API.
26+
const DEFAULT_GH_ADMIN_USER_IDS: [i32; 6] = [193874, 22186, 141300, 25030997, 229984, 20070360];
27+
2028
pub struct Server {
2129
pub base: Base,
2230
pub ip: IpAddr,
@@ -50,6 +58,7 @@ pub struct Server {
5058
pub version_id_cache_ttl: Duration,
5159
pub cdn_user_agent: String,
5260
pub balance_capacity: BalanceCapacityConfig,
61+
pub gh_admin_user_ids: HashSet<i32>,
5362

5463
/// Should the server serve the frontend assets in the `dist` directory?
5564
pub serve_dist: bool,
@@ -94,6 +103,9 @@ impl Default for Server {
94103
/// endpoint even with a healthy database pool.
95104
/// - `BLOCKED_ROUTES`: A comma separated list of HTTP route patterns that are manually blocked
96105
/// by an operator (e.g. `/crates/:crate_id/:version/download`).
106+
/// - `GH_ADMIN_USER_IDS`: A comma separated list of GitHub user IDs that will be considered
107+
/// admins. If not set, a default list comprised of the crates.io team as of May 2023 will be
108+
/// used.
97109
///
98110
/// # Panics
99111
///
@@ -185,6 +197,10 @@ impl Default for Server {
185197
cdn_user_agent: dotenvy::var("WEB_CDN_USER_AGENT")
186198
.unwrap_or_else(|_| "Amazon CloudFront".into()),
187199
balance_capacity: BalanceCapacityConfig::from_environment(),
200+
gh_admin_user_ids: env_optional("GH_ADMIN_USER_IDS")
201+
.map(parse_gh_admin_user_ids)
202+
.unwrap_or_else(|| Ok(DEFAULT_GH_ADMIN_USER_IDS.into_iter().collect()))
203+
.expect("invalid GH_ADMIN_USER_IDS"),
188204
serve_dist: true,
189205
serve_html: true,
190206
use_fastboot: dotenvy::var("USE_FASTBOOT").ok(),
@@ -326,3 +342,28 @@ fn parse_ipv6_based_cidr_blocks() {
326342
.unwrap()
327343
);
328344
}
345+
346+
fn parse_gh_admin_user_ids(users: String) -> Result<HashSet<i32>, ParseIntError> {
347+
users
348+
.split(|c: char| !(c.is_ascii_digit()))
349+
.filter(|uid| !uid.is_empty())
350+
.map(i32::from_str)
351+
.collect()
352+
}
353+
354+
#[test]
355+
fn test_gh_admin_user_ids() {
356+
fn assert_authorized(input: &str, expected: &[i32]) {
357+
assert_eq!(
358+
parse_gh_admin_user_ids(input.into()).unwrap(),
359+
expected.iter().copied().collect()
360+
);
361+
}
362+
363+
assert_authorized("", &[]);
364+
assert_authorized("12345", &[12345]);
365+
assert_authorized("12345, 6789", &[12345, 6789]);
366+
assert_authorized(" 12345 6789 ", &[12345, 6789]);
367+
assert_authorized("12345;6789", &[12345, 6789]);
368+
assert_authorized("12345;6789;12345", &[12345, 6789]);
369+
}

src/models/token.rs

Lines changed: 78 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -98,28 +98,28 @@ pub struct CreatedApiToken {
9898
}
9999

100100
#[cfg(test)]
101-
mod tests {
101+
pub mod tests {
102102
use super::*;
103103
use chrono::NaiveDate;
104104

105105
#[test]
106106
fn api_token_serializes_to_rfc3339() {
107-
let tok = ApiToken {
108-
id: 12345,
109-
user_id: 23456,
110-
revoked: false,
111-
name: "".to_string(),
112-
created_at: NaiveDate::from_ymd_opt(2017, 1, 6)
113-
.unwrap()
114-
.and_hms_opt(14, 23, 11)
107+
let tok = build_mock()
108+
.created_at(
109+
NaiveDate::from_ymd_opt(2017, 1, 6)
110+
.unwrap()
111+
.and_hms_opt(14, 23, 11)
112+
.unwrap(),
113+
)
114+
.last_used_at(
115+
Some(
116+
NaiveDate::from_ymd_opt(2017, 1, 6)
117+
.unwrap()
118+
.and_hms_opt(14, 23, 12),
119+
)
115120
.unwrap(),
116-
last_used_at: NaiveDate::from_ymd_opt(2017, 1, 6)
117-
.unwrap()
118-
.and_hms_opt(14, 23, 12),
119-
crate_scopes: None,
120-
endpoint_scopes: None,
121-
expired_at: None,
122-
};
121+
)
122+
.token();
123123
let json = serde_json::to_string(&tok).unwrap();
124124
assert_some!(json
125125
.as_str()
@@ -128,4 +128,66 @@ mod tests {
128128
.as_str()
129129
.find(r#""last_used_at":"2017-01-06T14:23:12+00:00""#));
130130
}
131+
132+
pub struct MockBuilder(ApiToken);
133+
134+
impl MockBuilder {
135+
pub fn token(self) -> ApiToken {
136+
self.0
137+
}
138+
139+
pub fn id(mut self, id: i32) -> Self {
140+
self.0.id = id;
141+
self
142+
}
143+
144+
pub fn user_id(mut self, user_id: i32) -> Self {
145+
self.0.user_id = user_id;
146+
self
147+
}
148+
149+
pub fn name(mut self, name: String) -> Self {
150+
self.0.name = name;
151+
self
152+
}
153+
154+
pub fn created_at(mut self, created_at: NaiveDateTime) -> Self {
155+
self.0.created_at = created_at;
156+
self
157+
}
158+
159+
pub fn last_used_at(mut self, last_used_at: Option<NaiveDateTime>) -> Self {
160+
self.0.last_used_at = last_used_at;
161+
self
162+
}
163+
164+
pub fn revoked(mut self, revoked: bool) -> Self {
165+
self.0.revoked = revoked;
166+
self
167+
}
168+
169+
pub fn crate_scopes(mut self, crate_scopes: Option<Vec<CrateScope>>) -> Self {
170+
self.0.crate_scopes = crate_scopes;
171+
self
172+
}
173+
174+
pub fn endpoint_scopes(mut self, endpoint_scopes: Option<Vec<EndpointScope>>) -> Self {
175+
self.0.endpoint_scopes = endpoint_scopes;
176+
self
177+
}
178+
}
179+
180+
pub fn build_mock() -> MockBuilder {
181+
MockBuilder(ApiToken {
182+
id: 12345,
183+
user_id: 23456,
184+
revoked: false,
185+
name: "".to_string(),
186+
created_at: NaiveDateTime::default(),
187+
last_used_at: None,
188+
crate_scopes: None,
189+
endpoint_scopes: None,
190+
expired_at: None,
191+
})
192+
}
131193
}

src/tests/util/test_app.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,7 @@ fn simple_config() -> config::Server {
439439
version_id_cache_ttl: Duration::from_secs(5 * 60),
440440
cdn_user_agent: "Amazon CloudFront".to_string(),
441441
balance_capacity,
442+
gh_admin_user_ids: HashSet::new(),
442443

443444
// The frontend code is not needed for the backend tests.
444445
serve_dist: false,

0 commit comments

Comments
 (0)