models/team: Replace App arguments with GitHubClient (#10571)

We don't need the full-blown `App` struct. All we need is a way to talk to the GitHub API.

Author: Turbo87

src/controllers/crate_owner_invitation.rs

@@ -157,7 +157,7 @@ async fn prepare_list(
                 // Only allow crate owners to query pending invitations for their crate.
                 let krate: Crate = Crate::by_name(&crate_name).first(conn).await?;
                 let owners = krate.owners(conn).await?;
-                if user.rights(state, &owners).await? != Rights::Full {
+                if user.rights(&*state.github, &owners).await? != Rights::Full {
                     let detail = "only crate owners can query pending invitations for their crate";
                     return Err(forbidden(detail));
                 }

src/controllers/krate/delete.rs

@@ -68,7 +68,7 @@ pub async fn delete_crate(
     // Check that the user is an owner of the crate (team owners are not allowed to delete crates)
     let user = auth.user();
     let owners = krate.owners(&mut conn).await?;
-    match user.rights(&app, &owners).await? {
+    match user.rights(&*app.github, &owners).await? {
         Rights::Full => {}
         Rights::Publish => {
             let msg = "team members don't have permission to delete crates";

src/controllers/krate/owners.rs

@@ -16,6 +16,7 @@ use axum_extra::json;
 use axum_extra::response::ErasedJson;
 use chrono::Utc;
 use crates_io_database::schema::crate_owners;
+use crates_io_github::GitHubClient;
 use diesel::prelude::*;
 use diesel_async::scoped_futures::ScopedFutureExt;
 use diesel_async::{AsyncConnection, AsyncPgConnection, RunQueryDsl};
@@ -176,7 +177,7 @@ async fn modify_owners(
 
                 let owners = krate.owners(conn).await?;
 
-                match user.rights(&app, &owners).await? {
+                match user.rights(&*app.github, &owners).await? {
                     Rights::Full => {}
                     // Yes!
                     Rights::Publish => {
@@ -292,7 +293,7 @@ async fn add_owner(
     login: &str,
 ) -> Result<NewOwnerInvite, OwnerAddError> {
     if login.contains(':') {
-        add_team_owner(app, conn, req_user, krate, login).await
+        add_team_owner(&*app.github, conn, req_user, krate, login).await
     } else {
         invite_user_owner(app, conn, req_user, krate, login).await
     }
@@ -330,14 +331,14 @@ async fn invite_user_owner(
 }
 
 async fn add_team_owner(
-    app: &App,
+    gh_client: &dyn GitHubClient,
     conn: &mut AsyncPgConnection,
     req_user: &User,
     krate: &Crate,
     login: &str,
 ) -> Result<NewOwnerInvite, OwnerAddError> {
     // Always recreate teams to get the most up-to-date GitHub ID
-    let team = Team::create_or_update(app, conn, login, req_user).await?;
+    let team = Team::create_or_update(gh_client, conn, login, req_user).await?;
 
     // Teams are added as owners immediately, since the above call ensures
     // the user is a team member.

src/controllers/krate/publish.rs

@@ -356,7 +356,7 @@ pub async fn publish(app: AppState, req: Parts, body: Body) -> AppResult<Json<Go
         };
 
         let owners = krate.owners(conn).await?;
-        if user.rights(&app, &owners).await? < Rights::Publish {
+        if user.rights(&*app.github, &owners).await? < Rights::Publish {
             return Err(custom(StatusCode::FORBIDDEN, MISSING_RIGHTS_ERROR_MESSAGE));
         }
 

src/controllers/version/update.rs

@@ -122,7 +122,7 @@ pub async fn perform_version_yank_update(
 
     let yanked = yanked.unwrap_or(version.yanked);
 
-    if user.rights(state, &owners).await? < Rights::Publish {
+    if user.rights(&*state.github, &owners).await? < Rights::Publish {
         if user.is_admin {
             let action = if yanked { "yanking" } else { "unyanking" };
             warn!(

src/models/team.rs

@@ -3,10 +3,9 @@ use diesel::prelude::*;
 use diesel_async::{AsyncPgConnection, RunQueryDsl};
 use http::StatusCode;
 
-use crate::app::App;
 use crate::util::errors::{bad_request, custom, AppResult};
 
-use crates_io_github::GitHubError;
+use crates_io_github::{GitHubClient, GitHubError};
 use oauth2::AccessToken;
 
 use crate::models::{Crate, CrateOwner, Owner, OwnerKind, User};
@@ -64,7 +63,7 @@ impl Team {
     ///
     /// This function will panic if login contains less than 2 `:` characters.
     pub async fn create_or_update(
-        app: &App,
+        gh_client: &dyn GitHubClient,
         conn: &mut AsyncPgConnection,
         login: &str,
         req_user: &User,
@@ -84,7 +83,7 @@ impl Team {
                     )
                 })?;
                 Team::create_or_update_github_team(
-                    app,
+                    gh_client,
                     conn,
                     &login.to_lowercase(),
                     org,
@@ -104,7 +103,7 @@ impl Team {
     /// correctly parsed out of the full `name`. `name` is passed as a
     /// convenience to avoid rebuilding it.
     async fn create_or_update_github_team(
-        app: &App,
+        gh_client: &dyn GitHubClient,
         conn: &mut AsyncPgConnection,
         login: &str,
         org_name: &str,
@@ -127,7 +126,7 @@ impl Team {
         }
 
         let token = AccessToken::new(req_user.gh_access_token.clone());
-        let team = app.github.team_by_name(org_name, team_name, &token).await
+        let team = gh_client.team_by_name(org_name, team_name, &token).await
             .map_err(|_| {
                 bad_request(format_args!(
                     "could not find the github team {org_name}/{team_name}. \
@@ -138,14 +137,14 @@ impl Team {
 
         let org_id = team.organization.id;
 
-        if !can_add_team(app, org_id, team.id, req_user).await? {
+        if !can_add_team(gh_client, org_id, team.id, req_user).await? {
             return Err(custom(
                 StatusCode::FORBIDDEN,
                 "only members of a team or organization owners can add it as an owner",
             ));
         }
 
-        let org = app.github.org_by_name(org_name, &token).await?;
+        let org = gh_client.org_by_name(org_name, &token).await?;
 
         NewTeam::builder()
             .login(&login.to_lowercase())
@@ -163,9 +162,15 @@ impl Team {
     /// Note that we're assuming that the given user is the one interested in
     /// the answer. If this is not the case, then we could accidentally leak
     /// private membership information here.
-    pub async fn contains_user(&self, app: &App, user: &User) -> AppResult<bool> {
+    pub async fn contains_user(
+        &self,
+        gh_client: &dyn GitHubClient,
+        user: &User,
+    ) -> AppResult<bool> {
         match self.org_id {
-            Some(org_id) => team_with_gh_id_contains_user(app, org_id, self.github_id, user).await,
+            Some(org_id) => {
+                team_with_gh_id_contains_user(gh_client, org_id, self.github_id, user).await
+            }
             // This means we don't have an org_id on file for the `self` team. It much
             // probably was deleted from github by the time we backfilled the database.
             // Short-circuiting to false since a non-existent team cannot contain any
@@ -189,17 +194,25 @@ impl Team {
     }
 }
 
-async fn can_add_team(app: &App, org_id: i32, team_id: i32, user: &User) -> AppResult<bool> {
+async fn can_add_team(
+    gh_client: &dyn GitHubClient,
+    org_id: i32,
+    team_id: i32,
+    user: &User,
+) -> AppResult<bool> {
     Ok(
-        team_with_gh_id_contains_user(app, org_id, team_id, user).await?
-            || is_gh_org_owner(app, org_id, user).await?,
+        team_with_gh_id_contains_user(gh_client, org_id, team_id, user).await?
+            || is_gh_org_owner(gh_client, org_id, user).await?,
     )
 }
 
-async fn is_gh_org_owner(app: &App, org_id: i32, user: &User) -> AppResult<bool> {
+async fn is_gh_org_owner(
+    gh_client: &dyn GitHubClient,
+    org_id: i32,
+    user: &User,
+) -> AppResult<bool> {
     let token = AccessToken::new(user.gh_access_token.clone());
-    match app
-        .github
+    match gh_client
         .org_membership(org_id, &user.gh_login, &token)
         .await
     {
@@ -210,7 +223,7 @@ async fn is_gh_org_owner(app: &App, org_id: i32, user: &User) -> AppResult<bool>
 }
 
 async fn team_with_gh_id_contains_user(
-    app: &App,
+    gh_client: &dyn GitHubClient,
     github_org_id: i32,
     github_team_id: i32,
     user: &User,
@@ -219,8 +232,7 @@ async fn team_with_gh_id_contains_user(
     // check that "state": "active"
 
     let token = AccessToken::new(user.gh_access_token.clone());
-    let membership = match app
-        .github
+    let membership = match gh_client
         .team_membership(github_org_id, github_team_id, &user.gh_login, &token)
         .await
     {

src/models/user.rs

@@ -6,12 +6,12 @@ use diesel::sql_types::Integer;
 use diesel::upsert::excluded;
 use diesel_async::{AsyncPgConnection, RunQueryDsl};
 
-use crate::app::App;
 use crate::util::errors::AppResult;
 
 use crate::models::{Crate, CrateOwner, Email, Owner, OwnerKind, Rights};
 use crate::schema::{crate_owners, emails, users};
 use crates_io_diesel_helpers::lower;
+use crates_io_github::GitHubClient;
 
 /// The model representing a row in the `users` database table.
 #[derive(Clone, Debug, PartialEq, Eq, Queryable, Identifiable, AsChangeset, Selectable)]
@@ -63,7 +63,11 @@ impl User {
     /// `Publish` as well, but this is a non-obvious invariant so we don't bother.
     /// Sweet free optimization if teams are proving burdensome to check.
     /// More than one team isn't really expected, though.
-    pub async fn rights(&self, app: &App, owners: &[Owner]) -> AppResult<Rights> {
+    pub async fn rights(
+        &self,
+        gh_client: &dyn GitHubClient,
+        owners: &[Owner],
+    ) -> AppResult<Rights> {
         let mut best = Rights::None;
         for owner in owners {
             match *owner {
@@ -73,7 +77,7 @@ impl User {
                     }
                 }
                 Owner::Team(ref team) => {
-                    if team.contains_user(app, self).await? {
+                    if team.contains_user(gh_client, self).await? {
                         best = Rights::Publish;
                     }
                 }