Skip to content

Define admin users and extend AuthCheck to handle them #6456

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .env.sample
Original file line number Diff line number Diff line change
Expand Up @@ -86,3 +86,6 @@ export SENTRY_ENV_API=local
# export TEST_S3_INDEX_REGION=http://127.0.0.1:19000
# export TEST_AWS_ACCESS_KEY=minio
# export TEST_AWS_SECRET_KEY=miniominio

# IDs of GitHub users that are admins on this instance, separated by commas.
export ADMIN_USER_GH_IDS=
66 changes: 66 additions & 0 deletions src/auth.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
use std::collections::HashSet;

use crate::controllers;
use crate::controllers::util::RequestPartsExt;
use crate::middleware::app::RequestApp;
use crate::middleware::log_request::RequestLogExt;
use crate::middleware::session::RequestSession;
use crate::models::token::{CrateScope, EndpointScope};
Expand All @@ -16,6 +19,7 @@ pub struct AuthCheck {
allow_token: bool,
endpoint_scope: Option<EndpointScope>,
crate_name: Option<String>,
require_admin: bool,
}

impl AuthCheck {
Expand All @@ -27,6 +31,7 @@ impl AuthCheck {
allow_token: true,
endpoint_scope: None,
crate_name: None,
require_admin: false,
}
}

Expand All @@ -36,6 +41,7 @@ impl AuthCheck {
allow_token: false,
endpoint_scope: None,
crate_name: None,
require_admin: false,
}
}

Expand All @@ -44,6 +50,7 @@ impl AuthCheck {
allow_token: self.allow_token,
endpoint_scope: Some(endpoint_scope),
crate_name: self.crate_name.clone(),
require_admin: self.require_admin,
}
}

Expand All @@ -52,6 +59,16 @@ impl AuthCheck {
allow_token: self.allow_token,
endpoint_scope: self.endpoint_scope,
crate_name: Some(crate_name.to_string()),
require_admin: self.require_admin,
}
}

pub fn require_admin(&self) -> Self {
Self {
allow_token: self.allow_token,
endpoint_scope: self.endpoint_scope,
crate_name: self.crate_name.clone(),
require_admin: true,
}
}

Expand All @@ -62,7 +79,14 @@ impl AuthCheck {
conn: &mut PgConnection,
) -> AppResult<Authentication> {
let auth = authenticate(request, conn)?;
self.check_authentication(auth, &request.app().config.admin_user_github_ids)
}

fn check_authentication(
&self,
auth: Authentication,
gh_admin_user_ids: &HashSet<i32>,
) -> AppResult<Authentication> {
if let Some(token) = auth.api_token() {
if !self.allow_token {
let error_message =
Expand All @@ -81,6 +105,11 @@ impl AuthCheck {
}
}

if self.require_admin && !gh_admin_user_ids.contains(&auth.user().gh_id) {
let error_message = "User is unauthorized";
return Err(internal(error_message).chain(forbidden()));
}

Ok(auth)
}

Expand Down Expand Up @@ -347,4 +376,41 @@ mod tests {
assert!(!auth_check.crate_scope_matches(Some(&vec![cs("anyhow")])));
assert!(!auth_check.crate_scope_matches(Some(&vec![cs("actix-*")])));
}

#[test]
fn require_admin() {
let auth_check = AuthCheck::default().require_admin();
let gh_admin_user_ids = [42, 43].into_iter().collect();

assert_ok!(auth_check.check_authentication(mock_cookie(42), &gh_admin_user_ids));
assert_err!(auth_check.check_authentication(mock_cookie(44), &gh_admin_user_ids));
assert_ok!(auth_check.check_authentication(mock_token(43), &gh_admin_user_ids));
assert_err!(auth_check.check_authentication(mock_token(45), &gh_admin_user_ids));
}

fn mock_user(gh_id: i32) -> User {
User {
id: 3,
gh_access_token: "arbitrary".into(),
gh_login: "literally_anything".into(),
name: None,
gh_avatar: None,
gh_id,
account_lock_reason: None,
account_lock_until: None,
}
}

fn mock_cookie(gh_id: i32) -> Authentication {
Authentication::Cookie(CookieAuthentication {
user: mock_user(gh_id),
})
}

fn mock_token(gh_id: i32) -> Authentication {
Authentication::Token(TokenAuthentication {
token: ApiToken::mock_builder().user_id(gh_id).build().unwrap(),
user: mock_user(gh_id),
})
}
}
7 changes: 7 additions & 0 deletions src/config/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ pub struct Server {
pub version_id_cache_ttl: Duration,
pub cdn_user_agent: String,
pub balance_capacity: BalanceCapacityConfig,
pub admin_user_github_ids: HashSet<i32>,

/// Instructs the `cargo_compat` middleware whether to adjust response
/// status codes to `200 OK` for all endpoints that are relevant for cargo.
Expand Down Expand Up @@ -104,6 +105,8 @@ impl Server {
/// endpoint even with a healthy database pool.
/// - `BLOCKED_ROUTES`: A comma separated list of HTTP route patterns that are manually blocked
/// by an operator (e.g. `/crates/:crate_id/:version/download`).
/// - `ADMIN_USER_GH_IDS`: A comma separated list of GitHub user IDs that will be considered
/// admins.
///
/// # Panics
///
Expand Down Expand Up @@ -206,6 +209,10 @@ impl Server {
balance_capacity: BalanceCapacityConfig::from_environment()?,
cargo_compat_status_code_config: var_parsed("CARGO_COMPAT_STATUS_CODES")?
.unwrap_or(StatusCodeConfig::AdjustAll),
admin_user_github_ids: HashSet::from_iter(list_parsed(
"ADMIN_USER_GH_IDS",
i32::from_str,
)?),
serve_dist: true,
serve_html: true,
content_security_policy: Some(content_security_policy.parse()?),
Expand Down
49 changes: 32 additions & 17 deletions src/models/token.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
mod scopes;

use chrono::NaiveDateTime;
use derive_builder::Builder;
use diesel::prelude::*;

pub use self::scopes::{CrateScope, EndpointScope};
Expand All @@ -11,24 +12,34 @@ use crate::util::rfc3339;
use crate::util::token::{HashedToken, PlainToken};

/// The model representing a row in the `api_tokens` database table.
#[derive(Debug, Identifiable, Queryable, Selectable, Associations, Serialize)]
#[derive(Debug, Identifiable, Queryable, Selectable, Associations, Serialize, Builder)]
#[diesel(belongs_to(User))]
#[builder(name = "MockApiTokenBuilder")]
pub struct ApiToken {
#[builder(default)]
pub id: i32,
#[serde(skip)]
#[builder(default)]
pub user_id: i32,
#[builder(default, setter(into))]
pub name: String,
#[serde(with = "rfc3339")]
#[builder(default, setter(strip_option))]
pub created_at: NaiveDateTime,
#[serde(with = "rfc3339::option")]
#[builder(default, setter(strip_option))]
pub last_used_at: Option<NaiveDateTime>,
#[serde(skip)]
#[builder(default = "false")]
pub revoked: bool,
/// `None` or a list of crate scope patterns (see RFC #2947)
#[builder(default, setter(strip_option))]
pub crate_scopes: Option<Vec<CrateScope>>,
/// A list of endpoint scopes or `None` for the `legacy` endpoint scope (see RFC #2947)
#[builder(default, setter(strip_option))]
pub endpoint_scopes: Option<Vec<EndpointScope>>,
#[serde(with = "rfc3339::option")]
#[builder(default, setter(strip_option))]
pub expired_at: Option<NaiveDateTime>,
}

Expand Down Expand Up @@ -95,6 +106,10 @@ impl ApiToken {
.or_else(|_| tokens.select(ApiToken::as_select()).first(conn))
.map_err(Into::into)
}

pub fn mock_builder() -> MockApiTokenBuilder {
MockApiTokenBuilder::default()
}
}

#[derive(Debug)]
Expand All @@ -110,22 +125,22 @@ mod tests {

#[test]
fn api_token_serializes_to_rfc3339() {
let tok = ApiToken {
id: 12345,
user_id: 23456,
revoked: false,
name: "".to_string(),
created_at: NaiveDate::from_ymd_opt(2017, 1, 6)
.unwrap()
.and_hms_opt(14, 23, 11)
.unwrap(),
last_used_at: NaiveDate::from_ymd_opt(2017, 1, 6)
.unwrap()
.and_hms_opt(14, 23, 12),
crate_scopes: None,
endpoint_scopes: None,
expired_at: None,
};
let created_at = NaiveDate::from_ymd_opt(2017, 1, 6)
.unwrap()
.and_hms_opt(14, 23, 11)
.unwrap();

let last_used_at = NaiveDate::from_ymd_opt(2017, 1, 6)
.unwrap()
.and_hms_opt(14, 23, 12)
.unwrap();

let tok = ApiToken::mock_builder()
.created_at(created_at)
.last_used_at(last_used_at)
.build()
.unwrap();

let json = serde_json::to_string(&tok).unwrap();
assert_some!(json
.as_str()
Expand Down
1 change: 1 addition & 0 deletions src/tests/util/test_app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,7 @@ fn simple_config() -> config::Server {
version_id_cache_ttl: Duration::from_secs(5 * 60),
cdn_user_agent: "Amazon CloudFront".to_string(),
balance_capacity,
admin_user_github_ids: HashSet::new(),

// The middleware has its own unit tests to verify its functionality.
// Here, we can test what would happen if we toggled the status code
Expand Down