From 66a885b4e6db2db561440d7be7f27a9271daa40a Mon Sep 17 00:00:00 2001 From: daimond113 <72147841+daimond113@users.noreply.github.com> Date: Mon, 14 Oct 2024 17:55:11 +0200 Subject: [PATCH] fix(registry): prevent token usage from unauthorized apps --- registry/.env.example | 5 ++++ registry/src/auth/github.rs | 43 ++++++++++++++++++++++++++++------- registry/src/auth/mod.rs | 21 +++++++++-------- registry/src/auth/rw_token.rs | 4 ++-- registry/src/auth/token.rs | 2 +- registry/src/main.rs | 6 ++--- 6 files changed, 57 insertions(+), 24 deletions(-) diff --git a/registry/.env.example b/registry/.env.example index b17c136..7af3b7b 100644 --- a/registry/.env.example +++ b/registry/.env.example @@ -9,15 +9,20 @@ COMMITTER_GIT_EMAIL= # email of the committer used for index updates # AUTHENTICATION CONFIGURATION # Set the variables of the authentication you want to use in order to enable it +READ_NEEDS_AUTH= # set to any value to require authentication for read requests + # Single Token ACCESS_TOKEN= # a single token that is used to authenticate all publish requests # Read/Write Tokens +# READ_NEEDS_AUTH isn't required for this + READ_ACCESS_TOKEN= # a token that is used to authenticate read requests WRITE_ACCESS_TOKEN= # a token that is used to authenticate write requests # GitHub GITHUB_AUTH= # set to any value to enable GitHub authentication +GITHUB_CLIENT_SECRET= # client secret of the GitHub OAuth app configured in the index's `config.toml` # If none of the above is set, no authentication is required, even for write requests diff --git a/registry/src/auth/github.rs b/registry/src/auth/github.rs index aa39d22..782f7c1 100644 --- a/registry/src/auth/github.rs +++ b/registry/src/auth/github.rs @@ -1,29 +1,51 @@ use crate::auth::{get_token_from_req, AuthImpl, UserId}; use actix_web::{dev::ServiceRequest, Error as ActixError}; -use serde::Deserialize; +use reqwest::StatusCode; +use serde::{Deserialize, Serialize}; use std::fmt::Display; #[derive(Debug)] pub struct GitHubAuth { pub reqwest_client: reqwest::Client, + pub client_id: String, + pub client_secret: String, +} + +#[derive(Debug, Serialize)] +struct TokenRequestBody { + access_token: String, } impl AuthImpl for GitHubAuth { async fn for_write_request(&self, req: &ServiceRequest) -> Result, ActixError> { - let token = match get_token_from_req(req, true) { + let token = match get_token_from_req(req) { Some(token) => token, None => return Ok(None), }; let response = match self .reqwest_client - .get("https://api.github.com/user") - .header(reqwest::header::AUTHORIZATION, token) + .post(format!( + "https://api.github.com/applications/{}/token", + self.client_id + )) + .basic_auth(&self.client_id, Some(&self.client_secret)) + .json(&TokenRequestBody { + access_token: token, + }) .send() .await - .and_then(|res| res.error_for_status()) { - Ok(response) => response, + Ok(response) => match response.error_for_status_ref() { + Ok(_) => response, + Err(e) if e.status().is_some_and(|s| s == StatusCode::UNAUTHORIZED) => { + return Ok(None); + } + Err(e) => { + log::error!("failed to get user: {e}"); + return Ok(None); + } + }, Err(e) => { log::error!("failed to get user: {e}"); return Ok(None); @@ -31,7 +53,7 @@ impl AuthImpl for GitHubAuth { }; let user_id = match response.json::().await { - Ok(user) => user.id, + Ok(resp) => resp.user.id, Err(e) => { log::error!("failed to get user: {e}"); return Ok(None); @@ -49,6 +71,11 @@ impl Display for GitHubAuth { } #[derive(Debug, Deserialize)] -struct UserResponse { +struct User { id: u64, } + +#[derive(Debug, Deserialize)] +struct UserResponse { + user: User, +} diff --git a/registry/src/auth/mod.rs b/registry/src/auth/mod.rs index b047ca4..1a5042f 100644 --- a/registry/src/auth/mod.rs +++ b/registry/src/auth/mod.rs @@ -13,6 +13,7 @@ use actix_web::{ middleware::Next, web, HttpMessage, HttpResponse, }; +use pesde::{source::pesde::PesdePackageSource, Project}; use sha2::{Digest, Sha256}; use std::fmt::Display; @@ -55,7 +56,7 @@ pub trait AuthImpl: Display { } fn read_needs_auth(&self) -> bool { - false + benv!("READ_NEEDS_AUTH").is_ok() } } @@ -141,14 +142,20 @@ pub async fn read_mw( next.call(req).await.map(|res| res.map_into_left_body()) } -pub fn get_auth_from_env() -> Auth { +pub fn get_auth_from_env(index: &PesdePackageSource, project: &Project) -> Auth { if let Ok(token) = benv!("ACCESS_TOKEN") { Auth::Token(token::TokenAuth { token: *Sha256::digest(token.as_bytes()).as_ref(), }) } else if benv!("GITHUB_AUTH").is_ok() { + let config = index.config(project).expect("failed to get index config"); + Auth::GitHub(github::GitHubAuth { reqwest_client: make_reqwest(), + client_id: config + .github_oauth_client_id + .expect("index isn't configured for GitHub"), + client_secret: benv!(required "GITHUB_CLIENT_SECRET"), }) } else if let Ok((r, w)) = benv!("READ_ACCESS_TOKEN").and_then(|r| benv!("WRITE_ACCESS_TOKEN").map(|w| (r, w))) @@ -162,7 +169,7 @@ pub fn get_auth_from_env() -> Auth { } } -pub fn get_token_from_req(req: &ServiceRequest, bearer: bool) -> Option { +pub fn get_token_from_req(req: &ServiceRequest) -> Option { let token = match req .headers() .get(AUTHORIZATION) @@ -172,13 +179,7 @@ pub fn get_token_from_req(req: &ServiceRequest, bearer: bool) -> Option None => return None, }; - let token = if bearer { - if token.to_lowercase().starts_with("bearer ") { - token.to_string() - } else { - format!("Bearer {token}") - } - } else if token.to_lowercase().starts_with("bearer ") { + let token = if token.to_lowercase().starts_with("bearer ") { token[7..].to_string() } else { token.to_string() diff --git a/registry/src/auth/rw_token.rs b/registry/src/auth/rw_token.rs index 4798272..f15537d 100644 --- a/registry/src/auth/rw_token.rs +++ b/registry/src/auth/rw_token.rs @@ -12,7 +12,7 @@ pub struct RwTokenAuth { impl AuthImpl for RwTokenAuth { async fn for_write_request(&self, req: &ServiceRequest) -> Result, ActixError> { - let token = match get_token_from_req(req, false) { + let token = match get_token_from_req(req) { Some(token) => token, None => return Ok(None), }; @@ -27,7 +27,7 @@ impl AuthImpl for RwTokenAuth { } async fn for_read_request(&self, req: &ServiceRequest) -> Result, ActixError> { - let token = match get_token_from_req(req, false) { + let token = match get_token_from_req(req) { Some(token) => token, None => return Ok(None), }; diff --git a/registry/src/auth/token.rs b/registry/src/auth/token.rs index bec311b..c98b4ab 100644 --- a/registry/src/auth/token.rs +++ b/registry/src/auth/token.rs @@ -12,7 +12,7 @@ pub struct TokenAuth { impl AuthImpl for TokenAuth { async fn for_write_request(&self, req: &ServiceRequest) -> Result, ActixError> { - let token = match get_token_from_req(req, false) { + let token = match get_token_from_req(req) { Some(token) => token, None => return Ok(None), }; diff --git a/registry/src/main.rs b/registry/src/main.rs index 1419fbe..04884fd 100644 --- a/registry/src/main.rs +++ b/registry/src/main.rs @@ -104,18 +104,18 @@ async fn run(with_sentry: bool) -> std::io::Result<()> { let (search_reader, search_writer) = make_search(&project, &source); let app_data = web::Data::new(AppState { - source: Mutex::new(source), - project, storage: { let storage = get_storage_from_env(); info!("storage: {storage}"); storage }, auth: { - let auth = get_auth_from_env(); + let auth = get_auth_from_env(&source, &project); info!("auth: {auth}"); auth }, + source: Mutex::new(source), + project, search_reader, search_writer: Mutex::new(search_writer),