Skip to content

Commit e449586

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 cd180a0 commit e449586

File tree

5 files changed

+192
-20
lines changed

5 files changed

+192
-20
lines changed

.env.sample

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,3 +72,7 @@ export GH_CLIENT_SECRET=
7272
# Credentials for connecting to the Sentry error reporting service.
7373
# export SENTRY_DSN_API=
7474
export SENTRY_ENV_API=local
75+
76+
# IDs of GitHub users that are admins on this instance, separated by commas.
77+
# Whitespace will be ignored.
78+
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

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

src/config.rs

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,17 @@ pub use self::base::Base;
1313
pub use self::database_pools::{DatabasePools, DbPoolConfig};
1414
pub use crate::config::balance_capacity::BalanceCapacityConfig;
1515
use http::HeaderValue;
16-
use std::collections::HashSet;
17-
use std::time::Duration;
16+
use std::{collections::HashSet, num::ParseIntError, str::FromStr, time::Duration};
1817

1918
const DEFAULT_VERSION_ID_CACHE_SIZE: u64 = 10_000;
2019
const DEFAULT_VERSION_ID_CACHE_TTL: u64 = 5 * 60; // 5 minutes
2120

21+
// The defaults correspond to the current crates.io team, which as at the time of writing, is the
22+
// GitHub user names carols10cents, jtgeibel, Turbo87, JohnTitor, LawnGnome, and mdtro.
23+
//
24+
// FIXME: this needs to be removed once we can detect the admins from the Rust teams API.
25+
const DEFAULT_GH_ADMIN_USER_IDS: [i32; 6] = [193874, 22186, 141300, 25030997, 229984, 20070360];
26+
2227
pub struct Server {
2328
pub base: Base,
2429
pub db: DatabasePools,
@@ -47,6 +52,7 @@ pub struct Server {
4752
pub version_id_cache_ttl: Duration,
4853
pub cdn_user_agent: String,
4954
pub balance_capacity: BalanceCapacityConfig,
55+
pub gh_admin_user_ids: HashSet<i32>,
5056
}
5157

5258
impl Default for Server {
@@ -82,6 +88,9 @@ impl Default for Server {
8288
/// endpoint even with a healthy database pool.
8389
/// - `BLOCKED_ROUTES`: A comma separated list of HTTP route patterns that are manually blocked
8490
/// by an operator (e.g. `/crates/:crate_id/:version/download`).
91+
/// - `GH_ADMIN_USER_IDS`: A comma separated list of GitHub user IDs that will be considered
92+
/// admins. If not set, a default list comprised of the crates.io team as of May 2023 will be
93+
/// used.
8594
///
8695
/// # Panics
8796
///
@@ -151,6 +160,9 @@ impl Default for Server {
151160
cdn_user_agent: dotenvy::var("WEB_CDN_USER_AGENT")
152161
.unwrap_or_else(|_| "Amazon CloudFront".into()),
153162
balance_capacity: BalanceCapacityConfig::from_environment(),
163+
gh_admin_user_ids: env_optional("GH_ADMIN_USER_IDS")
164+
.map(parse_gh_admin_user_ids)
165+
.unwrap_or(DEFAULT_GH_ADMIN_USER_IDS.into_iter().collect()),
154166
}
155167
}
156168
}
@@ -289,3 +301,29 @@ fn parse_ipv6_based_cidr_blocks() {
289301
.unwrap()
290302
);
291303
}
304+
305+
fn parse_gh_admin_user_ids(users: String) -> HashSet<i32> {
306+
users
307+
.split(|c: char| !(c.is_ascii_digit()))
308+
.filter(|uid| !uid.is_empty())
309+
.map(i32::from_str)
310+
.collect::<Result<HashSet<i32>, ParseIntError>>()
311+
.expect("invalid GH_ADMIN_USER_IDS")
312+
}
313+
314+
#[test]
315+
fn test_parse_authorized_admin_users() {
316+
fn assert_authorized(input: &str, expected: &[i32]) {
317+
assert_eq!(
318+
parse_gh_admin_user_ids(input.into()),
319+
expected.iter().copied().collect()
320+
);
321+
}
322+
323+
assert_authorized("", &[]);
324+
assert_authorized("12345", &[12345]);
325+
assert_authorized("12345, 6789", &[12345, 6789]);
326+
assert_authorized(" 12345 6789 ", &[12345, 6789]);
327+
assert_authorized("12345;6789", &[12345, 6789]);
328+
assert_authorized("12345;6789;12345", &[12345, 6789]);
329+
}

src/models/token.rs

Lines changed: 76 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -102,31 +102,28 @@ impl std::fmt::Debug for CreatedApiToken {
102102
}
103103

104104
#[cfg(test)]
105-
mod tests {
105+
pub mod tests {
106106
use super::*;
107107
use chrono::NaiveDate;
108108

109109
#[test]
110110
fn api_token_serializes_to_rfc3339() {
111-
let tok = ApiToken {
112-
id: 12345,
113-
user_id: 23456,
114-
token: SecureToken::generate(SecureTokenKind::Api).into_inner(),
115-
revoked: false,
116-
name: "".to_string(),
117-
created_at: NaiveDate::from_ymd_opt(2017, 1, 6)
118-
.unwrap()
119-
.and_hms_opt(14, 23, 11)
120-
.unwrap(),
121-
last_used_at: Some(
111+
let tok = build_mock()
112+
.created_at(
122113
NaiveDate::from_ymd_opt(2017, 1, 6)
123114
.unwrap()
124-
.and_hms_opt(14, 23, 12),
115+
.and_hms_opt(14, 23, 11)
116+
.unwrap(),
125117
)
126-
.unwrap(),
127-
crate_scopes: None,
128-
endpoint_scopes: None,
129-
};
118+
.last_used_at(
119+
Some(
120+
NaiveDate::from_ymd_opt(2017, 1, 6)
121+
.unwrap()
122+
.and_hms_opt(14, 23, 12),
123+
)
124+
.unwrap(),
125+
)
126+
.token();
130127
let json = serde_json::to_string(&tok).unwrap();
131128
assert_some!(json
132129
.as_str()
@@ -135,4 +132,66 @@ mod tests {
135132
.as_str()
136133
.find(r#""last_used_at":"2017-01-06T14:23:12+00:00""#));
137134
}
135+
136+
pub struct MockBuilder(ApiToken);
137+
138+
impl MockBuilder {
139+
pub fn token(self) -> ApiToken {
140+
self.0
141+
}
142+
143+
pub fn id(mut self, id: i32) -> Self {
144+
self.0.id = id;
145+
self
146+
}
147+
148+
pub fn user_id(mut self, user_id: i32) -> Self {
149+
self.0.user_id = user_id;
150+
self
151+
}
152+
153+
pub fn name(mut self, name: String) -> Self {
154+
self.0.name = name;
155+
self
156+
}
157+
158+
pub fn created_at(mut self, created_at: NaiveDateTime) -> Self {
159+
self.0.created_at = created_at;
160+
self
161+
}
162+
163+
pub fn last_used_at(mut self, last_used_at: Option<NaiveDateTime>) -> Self {
164+
self.0.last_used_at = last_used_at;
165+
self
166+
}
167+
168+
pub fn revoked(mut self, revoked: bool) -> Self {
169+
self.0.revoked = revoked;
170+
self
171+
}
172+
173+
pub fn crate_scopes(mut self, crate_scopes: Option<Vec<CrateScope>>) -> Self {
174+
self.0.crate_scopes = crate_scopes;
175+
self
176+
}
177+
178+
pub fn endpoint_scopes(mut self, endpoint_scopes: Option<Vec<EndpointScope>>) -> Self {
179+
self.0.endpoint_scopes = endpoint_scopes;
180+
self
181+
}
182+
}
183+
184+
pub fn build_mock() -> MockBuilder {
185+
MockBuilder(ApiToken {
186+
id: 12345,
187+
user_id: 23456,
188+
token: SecureToken::generate(SecureTokenKind::Api).into_inner(),
189+
revoked: false,
190+
name: "".to_string(),
191+
created_at: NaiveDateTime::default(),
192+
last_used_at: None,
193+
crate_scopes: None,
194+
endpoint_scopes: None,
195+
})
196+
}
138197
}

src/tests/util/test_app.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,7 @@ fn simple_config() -> config::Server {
360360
version_id_cache_ttl: Duration::from_secs(5 * 60),
361361
cdn_user_agent: "Amazon CloudFront".to_string(),
362362
balance_capacity: BalanceCapacityConfig::for_testing(),
363+
gh_admin_user_ids: HashSet::new(),
363364
}
364365
}
365366

0 commit comments

Comments
 (0)