Skip to content

Commit 40f2be4

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 caa9043 commit 40f2be4

File tree

8 files changed

+318
-20
lines changed

8 files changed

+318
-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=

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ indicatif = "=0.17.3"
6060
ipnetwork = "=0.20.0"
6161
tikv-jemallocator = { version = "=0.5.0", features = ['unprefixed_malloc_on_supported_platforms', 'profiling'] }
6262
lettre = { version = "=0.10.4", default-features = false, features = ["file-transport", "smtp-transport", "native-tls", "hostname", "builder"] }
63+
lazy_static = "=1.4.0"
6364
minijinja = "=0.32.1"
6465
moka = { version = "=0.11.0", features = ["future"] }
6566
oauth2 = { version = "=4.3.0", default-features = false, features = ["reqwest"] }

src/auth.rs

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ pub struct AuthCheck {
1616
allow_token: bool,
1717
endpoint_scope: Option<EndpointScope>,
1818
crate_name: Option<String>,
19+
require_admin: bool,
1920
}
2021

2122
impl AuthCheck {
@@ -27,6 +28,7 @@ impl AuthCheck {
2728
allow_token: true,
2829
endpoint_scope: None,
2930
crate_name: None,
31+
require_admin: false,
3032
}
3133
}
3234

@@ -36,6 +38,7 @@ impl AuthCheck {
3638
allow_token: false,
3739
endpoint_scope: None,
3840
crate_name: None,
41+
require_admin: false,
3942
}
4043
}
4144

@@ -44,6 +47,7 @@ impl AuthCheck {
4447
allow_token: self.allow_token,
4548
endpoint_scope: Some(endpoint_scope),
4649
crate_name: self.crate_name.clone(),
50+
require_admin: self.require_admin,
4751
}
4852
}
4953

@@ -52,6 +56,16 @@ impl AuthCheck {
5256
allow_token: self.allow_token,
5357
endpoint_scope: self.endpoint_scope,
5458
crate_name: Some(crate_name.to_string()),
59+
require_admin: self.require_admin,
60+
}
61+
}
62+
63+
pub fn require_admin(&self) -> Self {
64+
Self {
65+
allow_token: self.allow_token,
66+
endpoint_scope: self.endpoint_scope,
67+
crate_name: self.crate_name.clone(),
68+
require_admin: true,
5569
}
5670
}
5771

@@ -61,8 +75,10 @@ impl AuthCheck {
6175
request: &T,
6276
conn: &mut PgConnection,
6377
) -> AppResult<Authentication> {
64-
let auth = authenticate(request, conn)?;
78+
self.check_authentication(authenticate(request, conn)?)
79+
}
6580

81+
fn check_authentication(&self, auth: Authentication) -> AppResult<Authentication> {
6682
if let Some(token) = auth.api_token() {
6783
if !self.allow_token {
6884
let error_message =
@@ -81,6 +97,11 @@ impl AuthCheck {
8197
}
8298
}
8399

100+
if self.require_admin && auth.user().admin().is_err() {
101+
let error_message = "User is unauthorized";
102+
return Err(internal(error_message).chain(forbidden()));
103+
}
104+
84105
Ok(auth)
85106
}
86107

@@ -247,6 +268,13 @@ fn ensure_not_locked(user: &User) -> AppResult<()> {
247268

248269
#[cfg(test)]
249270
mod tests {
271+
use chrono::NaiveDate;
272+
273+
use crate::{
274+
models::user::tests::MockUserGenerator,
275+
util::token::{SecureToken, SecureTokenKind},
276+
};
277+
250278
use super::*;
251279

252280
fn cs(scope: &str) -> CrateScope {
@@ -343,4 +371,31 @@ mod tests {
343371
assert!(!auth_check.crate_scope_matches(Some(&vec![cs("anyhow")])));
344372
assert!(!auth_check.crate_scope_matches(Some(&vec![cs("actix-*")])));
345373
}
374+
375+
#[test]
376+
fn require_admin() {
377+
let auth_check = AuthCheck::default().require_admin();
378+
379+
let mut gen = MockUserGenerator::default();
380+
let admin = gen.admin();
381+
let non_admin = gen.regular();
382+
383+
assert_ok!(auth_check.check_authentication(mock_cookie(admin.clone())));
384+
assert_err!(auth_check.check_authentication(mock_cookie(non_admin.clone())));
385+
assert_ok!(auth_check.check_authentication(mock_token(admin)));
386+
assert_err!(auth_check.check_authentication(mock_token(non_admin)));
387+
}
388+
389+
fn mock_cookie(user: User) -> Authentication {
390+
Authentication::Cookie(CookieAuthentication { user })
391+
}
392+
393+
fn mock_token(user: User) -> Authentication {
394+
Authentication::Token(TokenAuthentication {
395+
token: crate::models::token::tests::build_mock()
396+
.user_id(user.id)
397+
.token(),
398+
user,
399+
})
400+
}
346401
}

src/models/helpers.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
1+
pub mod admin;
12
pub mod with_count;

src/models/helpers/admin.rs

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
use std::{collections::HashSet, num::ParseIntError, str::FromStr};
2+
3+
use lazy_static::lazy_static;
4+
5+
use crate::util::errors::{forbidden, AppResult};
6+
7+
lazy_static! {
8+
static ref AUTHORIZED_ADMIN_USER_IDS: HashSet<i32> =
9+
parse_authorized_admin_users(dotenv::var("GH_ADMIN_USER_IDS"));
10+
}
11+
12+
// The defaults correspond to the current crates.io team.
13+
//
14+
// FIXME: this needs to be removed once we can detect the admins from the Rust teams API.
15+
const DEFAULT_ADMIN_USER_IDS: [i32; 5] = [193874, 22186, 141300, 229984, 20070360];
16+
17+
fn parse_authorized_admin_users(maybe_users: dotenv::Result<String>) -> HashSet<i32> {
18+
match maybe_users {
19+
Ok(users) => users
20+
.split(|c: char| !(c.is_ascii_digit()))
21+
.filter(|uid| !uid.is_empty())
22+
.map(i32::from_str)
23+
.collect::<Result<HashSet<i32>, ParseIntError>>()
24+
.expect("parsing admin users from $GH_ADMIN_USER_IDS"),
25+
Err(_err) => DEFAULT_ADMIN_USER_IDS.into_iter().collect(),
26+
}
27+
}
28+
29+
/// Return `Ok(())` if the given GitHub user ID matches a known admin (set
30+
/// either via the `$GH_ADMIN_USER_IDS` environment variable, or the builtin
31+
/// fallback list if that variable is unset), or a forbidden error otherwise.
32+
pub fn is_authorized_admin(github_uid: i32) -> AppResult<()> {
33+
// This hack is here to allow tests to have a consistent set of admin users
34+
// (in this case, just the contents of the `DEFAULT_ADMIN_USER_IDS` constant
35+
// above).
36+
37+
#[cfg(not(test))]
38+
fn check_user_id(uid: i32) -> bool {
39+
AUTHORIZED_ADMIN_USER_IDS.contains(&uid)
40+
}
41+
42+
#[cfg(test)]
43+
fn check_user_id(uid: i32) -> bool {
44+
DEFAULT_ADMIN_USER_IDS.contains(&uid)
45+
}
46+
47+
if check_user_id(github_uid) {
48+
Ok(())
49+
} else {
50+
Err(forbidden())
51+
}
52+
}
53+
54+
#[cfg(test)]
55+
pub(crate) fn authorized_user_ids() -> &'static HashSet<i32> {
56+
&AUTHORIZED_ADMIN_USER_IDS
57+
}
58+
59+
#[cfg(test)]
60+
mod tests {
61+
use std::io::{self, ErrorKind};
62+
63+
use super::{is_authorized_admin, parse_authorized_admin_users, DEFAULT_ADMIN_USER_IDS};
64+
65+
#[test]
66+
fn test_is_authorized_admin() {
67+
assert_ok!(is_authorized_admin(193874));
68+
assert_err!(is_authorized_admin(0));
69+
assert_err!(is_authorized_admin(141301));
70+
}
71+
72+
#[test]
73+
fn test_parse_authorized_admin_users() {
74+
fn assert_authorized(input: dotenv::Result<&str>, expected: &[i32]) {
75+
assert_eq!(
76+
parse_authorized_admin_users(input.map(String::from)),
77+
expected.iter().copied().collect()
78+
);
79+
}
80+
81+
assert_authorized(Ok(""), &[]);
82+
assert_authorized(Ok("12345"), &[12345]);
83+
assert_authorized(Ok("12345, 6789"), &[12345, 6789]);
84+
assert_authorized(Ok(" 12345 6789 "), &[12345, 6789]);
85+
assert_authorized(Ok("12345;6789"), &[12345, 6789]);
86+
assert_authorized(Ok("12345;6789;12345"), &[12345, 6789]);
87+
88+
let not_found_error = dotenv::Error::Io(io::Error::new(ErrorKind::NotFound, "not found"));
89+
assert_authorized(Err(not_found_error), DEFAULT_ADMIN_USER_IDS.as_slice());
90+
}
91+
}

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
}

0 commit comments

Comments
 (0)