diff --git a/src/app.rs b/src/app.rs index 242de0c2fe0..db099ff2b61 100644 --- a/src/app.rs +++ b/src/app.rs @@ -3,6 +3,7 @@ use crate::{db, Config, Env}; use std::{sync::Arc, time::Duration}; +use crate::github::GitHubClient; use diesel::r2d2; use oauth2::basic::BasicClient; use reqwest::blocking::Client; @@ -19,8 +20,11 @@ pub struct App { /// The read-only replica database connection pool pub read_only_replica_database: Option, + /// GitHub API client + pub github: GitHubClient, + /// The GitHub OAuth2 configuration - pub github: BasicClient, + pub github_oauth: BasicClient, /// A unique key used with conduit_cookie to generate cookies pub session_key: String, @@ -47,7 +51,9 @@ impl App { pub fn new(config: Config, http_client: Option) -> App { use oauth2::{AuthUrl, ClientId, ClientSecret, TokenUrl}; - let github = BasicClient::new( + let github = GitHubClient::new(http_client.clone(), config.gh_base_url.clone()); + + let github_oauth = BasicClient::new( ClientId::new(config.gh_client_id.clone()), Some(ClientSecret::new(config.gh_client_secret.clone())), AuthUrl::new(String::from("https://github.com/login/oauth/authorize")).unwrap(), @@ -122,6 +128,7 @@ impl App { primary_database, read_only_replica_database, github, + github_oauth, session_key: config.session_key.clone(), config, http_client, diff --git a/src/config.rs b/src/config.rs index 300badf8df4..cdc5753b04f 100644 --- a/src/config.rs +++ b/src/config.rs @@ -7,6 +7,7 @@ pub struct Config { pub session_key: String, pub gh_client_id: String, pub gh_client_secret: String, + pub gh_base_url: String, pub db_url: String, pub replica_db_url: Option, pub env: Env, @@ -131,6 +132,7 @@ impl Default for Config { session_key: env("SESSION_KEY"), gh_client_id: env("GH_CLIENT_ID"), gh_client_secret: env("GH_CLIENT_SECRET"), + gh_base_url: "https://api.github.com".to_string(), db_url: env("DATABASE_URL"), replica_db_url: dotenv::var("READ_ONLY_REPLICA_URL").ok(), env: cargo_env, diff --git a/src/controllers/user/session.rs b/src/controllers/user/session.rs index 9595374cd6e..6095509073c 100644 --- a/src/controllers/user/session.rs +++ b/src/controllers/user/session.rs @@ -1,10 +1,10 @@ use crate::controllers::frontend_prelude::*; -use crate::github; use conduit_cookie::RequestSession; use oauth2::reqwest::http_client; use oauth2::{AuthorizationCode, Scope, TokenResponse}; +use crate::github::GithubUser; use crate::middleware::current_user::TrustedUserId; use crate::models::{NewUser, User}; use crate::schema::users; @@ -28,7 +28,7 @@ use crate::util::errors::ReadOnlyMode; pub fn begin(req: &mut dyn RequestExt) -> EndpointResult { let (url, state) = req .app() - .github + .github_oauth .authorize_url(oauth2::CsrfToken::new_random) .add_scope(Scope::new("read:org".to_string())) .url(); @@ -95,15 +95,15 @@ pub fn authorize(req: &mut dyn RequestExt) -> EndpointResult { let code = AuthorizationCode::new(code); let token = req .app() - .github + .github_oauth .exchange_code(code) .request(http_client) .chain_error(|| server_error("Error obtaining token"))?; let token = token.access_token(); // Fetch the user info from GitHub using the access token we just got and create a user record - let ghuser = github::github_api::(req.app(), "/user", token)?; - let user = ghuser.save_to_database(&token.secret(), &*req.db_conn()?)?; + let ghuser = req.app().github.current_user(token)?; + let user = save_user_to_database(&ghuser, &token.secret(), &*req.db_conn()?)?; // Log in by setting a cookie and the middleware authentication req.session_mut() @@ -113,40 +113,33 @@ pub fn authorize(req: &mut dyn RequestExt) -> EndpointResult { super::me::me(req) } -#[derive(Deserialize)] -struct GithubUser { - email: Option, - name: Option, - login: String, - id: i32, - avatar_url: Option, -} - -impl GithubUser { - fn save_to_database(&self, access_token: &str, conn: &PgConnection) -> AppResult { - NewUser::new( - self.id, - &self.login, - self.name.as_deref(), - self.avatar_url.as_deref(), - access_token, - ) - .create_or_update(self.email.as_deref(), conn) - .map_err(Into::into) - .or_else(|e: Box| { - // If we're in read only mode, we can't update their details - // just look for an existing user - if e.is::() { - users::table - .filter(users::gh_id.eq(self.id)) - .first(conn) - .optional()? - .ok_or(e) - } else { - Err(e) - } - }) - } +fn save_user_to_database( + user: &GithubUser, + access_token: &str, + conn: &PgConnection, +) -> AppResult { + NewUser::new( + user.id, + &user.login, + user.name.as_deref(), + user.avatar_url.as_deref(), + access_token, + ) + .create_or_update(user.email.as_deref(), conn) + .map_err(Into::into) + .or_else(|e: Box| { + // If we're in read only mode, we can't update their details + // just look for an existing user + if e.is::() { + users::table + .filter(users::gh_id.eq(user.id)) + .first(conn) + .optional()? + .ok_or(e) + } else { + Err(e) + } + }) } /// Handles the `DELETE /api/private/session` route. @@ -175,7 +168,7 @@ mod tests { id: -1, avatar_url: None, }; - let result = gh_user.save_to_database("arbitrary_token", &conn); + let result = save_user_to_database(&gh_user, "arbitrary_token", &conn); assert!( result.is_ok(), diff --git a/src/github.rs b/src/github.rs index 924a5c0fe08..3e1034f05bf 100644 --- a/src/github.rs +++ b/src/github.rs @@ -7,29 +7,89 @@ use serde::de::DeserializeOwned; use std::str; -use crate::app::App; use crate::util::errors::{cargo_err, internal, not_found, AppError, AppResult}; +use reqwest::blocking::Client; -/// Does all the nonsense for sending a GET to Github. Doesn't handle parsing -/// because custom error-code handling may be desirable. Use -/// `parse_github_response` to handle the "common" processing of responses. -pub fn github_api(app: &App, url: &str, auth: &AccessToken) -> AppResult -where - T: DeserializeOwned, -{ - let url = format!("{}://api.github.com{}", app.config.api_protocol, url); - info!("GITHUB HTTP: {}", url); - - app.http_client() - .get(&url) - .header(header::ACCEPT, "application/vnd.github.v3+json") - .header(header::AUTHORIZATION, format!("token {}", auth.secret())) - .header(header::USER_AGENT, "crates.io (https://crates.io)") - .send()? - .error_for_status() - .map_err(|e| handle_error_response(&e))? - .json() - .map_err(Into::into) +#[derive(Debug)] +pub struct GitHubClient { + base_url: String, + client: Option, +} + +impl GitHubClient { + pub fn new(client: Option, base_url: String) -> Self { + Self { client, base_url } + } + + pub fn current_user(&self, auth: &AccessToken) -> AppResult { + self.request("/user", auth) + } + + pub fn org_by_name(&self, org_name: &str, auth: &AccessToken) -> AppResult { + let url = format!("/orgs/{}", org_name); + self.request(&url, auth) + } + + pub fn team_by_name( + &self, + org_name: &str, + team_name: &str, + auth: &AccessToken, + ) -> AppResult { + let url = format!("/orgs/{}/teams/{}", org_name, team_name); + self.request(&url, auth) + } + + pub fn team_membership( + &self, + org_id: i32, + team_id: i32, + username: &str, + auth: &AccessToken, + ) -> AppResult { + let url = format!( + "/organizations/{}/team/{}/memberships/{}", + org_id, team_id, username + ); + self.request(&url, auth) + } + + /// Does all the nonsense for sending a GET to Github. Doesn't handle parsing + /// because custom error-code handling may be desirable. Use + /// `parse_github_response` to handle the "common" processing of responses. + pub fn request(&self, url: &str, auth: &AccessToken) -> AppResult + where + T: DeserializeOwned, + { + let url = format!("{}{}", self.base_url, url); + info!("GITHUB HTTP: {}", url); + + self.client() + .get(&url) + .header(header::ACCEPT, "application/vnd.github.v3+json") + .header(header::AUTHORIZATION, format!("token {}", auth.secret())) + .header(header::USER_AGENT, "crates.io (https://crates.io)") + .send()? + .error_for_status() + .map_err(|e| handle_error_response(&e))? + .json() + .map_err(Into::into) + } + + /// Returns a client for making HTTP requests to upload crate files. + /// + /// The client will go through a proxy if the application was configured via + /// `TestApp::with_proxy()`. + /// + /// # Panics + /// + /// Panics if the application was not initialized with a client. This should only occur in + /// tests that were not properly initialized. + fn client(&self) -> &Client { + self.client + .as_ref() + .expect("No HTTP client is configured. In tests, use `TestApp::with_proxy()`.") + } } fn handle_error_response(error: &reqwest::Error) -> Box { @@ -52,6 +112,33 @@ fn handle_error_response(error: &reqwest::Error) -> Box { } } +#[derive(Debug, Deserialize)] +pub struct GithubUser { + pub avatar_url: Option, + pub email: Option, + pub id: i32, + pub login: String, + pub name: Option, +} + +#[derive(Debug, Deserialize)] +pub struct GitHubOrganization { + pub id: i32, // unique GH id (needed for membership queries) + pub avatar_url: Option, +} + +#[derive(Debug, Deserialize)] +pub struct GitHubTeam { + pub id: i32, // unique GH id (needed for membership queries) + pub name: Option, // Pretty name + pub organization: GitHubOrganization, +} + +#[derive(Debug, Deserialize)] +pub struct GitHubTeamMembership { + pub state: String, +} + pub fn team_url(login: &str) -> String { let mut login_pieces = login.split(':'); login_pieces.next(); diff --git a/src/models/team.rs b/src/models/team.rs index 347ad6bb477..b4d4ccc836d 100644 --- a/src/models/team.rs +++ b/src/models/team.rs @@ -1,7 +1,6 @@ use diesel::prelude::*; use crate::app::App; -use crate::github::github_api; use crate::util::errors::{cargo_err, AppResult, NotFound}; use oauth2::AccessToken; @@ -139,26 +138,16 @@ impl Team { ))); } - #[derive(Deserialize)] - struct GithubOrganization { - id: i32, // unique GH id (needed for membership queries) - } - - #[derive(Deserialize)] - struct GithubTeam { - id: i32, // unique GH id (needed for membership queries) - name: Option, // Pretty name - organization: GithubOrganization, - } - - let url = format!("/orgs/{}/teams/{}", org_name, team_name); let token = AccessToken::new(req_user.gh_access_token.clone()); - let team = github_api::(app, &url, &token).map_err(|_| { - cargo_err(&format_args!( - "could not find the github team {}/{}", - org_name, team_name - )) - })?; + let team = app + .github + .team_by_name(org_name, team_name, &token) + .map_err(|_| { + cargo_err(&format_args!( + "could not find the github team {}/{}", + org_name, team_name + )) + })?; let org_id = team.organization.id; @@ -166,13 +155,7 @@ impl Team { return Err(cargo_err("only members of a team can add it as an owner")); } - #[derive(Deserialize)] - struct Org { - avatar_url: Option, - } - - let url = format!("/orgs/{}", org_name); - let org = github_api::(app, &url, &token)?; + let org = app.github.org_by_name(org_name, &token)?; NewTeam::new( &login.to_lowercase(), @@ -223,21 +206,16 @@ fn team_with_gh_id_contains_user( // GET /organizations/:org_id/team/:team_id/memberships/:username // check that "state": "active" - #[derive(Deserialize)] - struct Membership { - state: String, - } - - let url = format!( - "/organizations/{}/team/{}/memberships/{}", - &github_org_id, &github_team_id, &user.gh_login - ); let token = AccessToken::new(user.gh_access_token.clone()); - let membership = match github_api::(app, &url, &token) { - // Officially how `false` is returned - Err(ref e) if e.is::() => return Ok(false), - x => x?, - }; + let membership = + match app + .github + .team_membership(github_org_id, github_team_id, &user.gh_login, &token) + { + // Officially how `false` is returned + Err(ref e) if e.is::() => return Ok(false), + x => x?, + }; // There is also `state: pending` for which we could possibly give // some feedback, but it's not obvious how that should work. diff --git a/src/tests/all.rs b/src/tests/all.rs index 8c13f3aecfb..feb24754332 100644 --- a/src/tests/all.rs +++ b/src/tests/all.rs @@ -129,6 +129,7 @@ fn simple_config() -> Config { session_key: "test this has to be over 32 bytes long".to_string(), gh_client_id: dotenv::var("GH_CLIENT_ID").unwrap_or_default(), gh_client_secret: dotenv::var("GH_CLIENT_SECRET").unwrap_or_default(), + gh_base_url: "http://api.github.com".to_string(), db_url: env("TEST_DATABASE_URL"), replica_db_url: None, env: Env::Test,