Skip to content

Extract GitHubClient struct #3218

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

Merged
merged 7 commits into from
Feb 1, 2021
Merged
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
11 changes: 9 additions & 2 deletions src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -19,8 +20,11 @@ pub struct App {
/// The read-only replica database connection pool
pub read_only_replica_database: Option<db::DieselPool>,

/// 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,
Expand All @@ -47,7 +51,9 @@ impl App {
pub fn new(config: Config, http_client: Option<Client>) -> 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(),
Expand Down Expand Up @@ -122,6 +128,7 @@ impl App {
primary_database,
read_only_replica_database,
github,
github_oauth,
session_key: config.session_key.clone(),
config,
http_client,
Expand Down
2 changes: 2 additions & 0 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>,
pub env: Env,
Expand Down Expand Up @@ -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,
Expand Down
73 changes: 33 additions & 40 deletions src/controllers/user/session.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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();
Expand Down Expand Up @@ -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::<GithubUser>(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()
Expand All @@ -113,40 +113,33 @@ pub fn authorize(req: &mut dyn RequestExt) -> EndpointResult {
super::me::me(req)
}

#[derive(Deserialize)]
struct GithubUser {
email: Option<String>,
name: Option<String>,
login: String,
id: i32,
avatar_url: Option<String>,
}

impl GithubUser {
fn save_to_database(&self, access_token: &str, conn: &PgConnection) -> AppResult<User> {
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<dyn AppError>| {
// If we're in read only mode, we can't update their details
// just look for an existing user
if e.is::<ReadOnlyMode>() {
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<User> {
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<dyn AppError>| {
// If we're in read only mode, we can't update their details
// just look for an existing user
if e.is::<ReadOnlyMode>() {
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.
Expand Down Expand Up @@ -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(),
Expand Down
129 changes: 108 additions & 21 deletions src/github.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T>(app: &App, url: &str, auth: &AccessToken) -> AppResult<T>
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<Client>,
}

impl GitHubClient {
pub fn new(client: Option<Client>, base_url: String) -> Self {
Self { client, base_url }
}

pub fn current_user(&self, auth: &AccessToken) -> AppResult<GithubUser> {
self.request("/user", auth)
}

pub fn org_by_name(&self, org_name: &str, auth: &AccessToken) -> AppResult<GitHubOrganization> {
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<GitHubTeam> {
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<GitHubTeamMembership> {
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<T>(&self, url: &str, auth: &AccessToken) -> AppResult<T>
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<dyn AppError> {
Expand All @@ -52,6 +112,33 @@ fn handle_error_response(error: &reqwest::Error) -> Box<dyn AppError> {
}
}

#[derive(Debug, Deserialize)]
pub struct GithubUser {
pub avatar_url: Option<String>,
pub email: Option<String>,
pub id: i32,
pub login: String,
pub name: Option<String>,
}

#[derive(Debug, Deserialize)]
pub struct GitHubOrganization {
pub id: i32, // unique GH id (needed for membership queries)
pub avatar_url: Option<String>,
}

#[derive(Debug, Deserialize)]
pub struct GitHubTeam {
pub id: i32, // unique GH id (needed for membership queries)
pub name: Option<String>, // 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();
Expand Down
Loading