More tests (#729)

* permissions tests

* finished permissions; organization tests

* clippy, fmt

* post-merge fixes

* teams changes

* refactored to use new api

* fmt, clippy

* sqlx prepare

* revs

* revs

* re-tested

* re-added name

* reverted to matrix

.github/workflows/tests.yml

@@ -19,7 +19,7 @@ jobs:
     strategy:
       matrix:
         os: [ubuntu-latest]
-        rust: [beta, nightly, stable]
+        rust: [stable]
 
     steps:
       - uses: actions/checkout@v2

src/auth/flows.rs

@@ -2247,7 +2247,11 @@ pub async fn link_trolley(
     }
 
     if let Some(email) = user.email {
-        let id = payouts_queue.lock().await.register_recipient(&email, body.0).await?;
+        let id = payouts_queue
+            .lock()
+            .await
+            .register_recipient(&email, body.0)
+            .await?;
 
         let mut transaction = pool.begin().await?;
 

src/database/models/user_item.rs

@@ -380,7 +380,7 @@ impl User {
         redis
             .delete_many(
                 user_ids
-                    .into_iter()
+                    .iter()
                     .map(|id| (USERS_PROJECTS_NAMESPACE, Some(id.0.to_string()))),
             )
             .await?;

src/models/teams.rs

@@ -23,7 +23,7 @@ pub struct Team {
 }
 
 bitflags::bitflags! {
-    #[derive(Copy, Clone, Debug)]
+    #[derive(Copy, Clone, Debug, PartialEq, Eq)]
     pub struct ProjectPermissions: u64 {
         const UPLOAD_VERSION = 1 << 0;
         const DELETE_VERSION = 1 << 1;
@@ -35,8 +35,6 @@ bitflags::bitflags! {
         const DELETE_PROJECT = 1 << 7;
         const VIEW_ANALYTICS = 1 << 8;
         const VIEW_PAYOUTS = 1 << 9;
-
-        const ALL = 0b1111111111;
     }
 }
 
@@ -55,15 +53,19 @@ impl ProjectPermissions {
         organization_team_member: &Option<crate::database::models::TeamMember>, // team member of the user in the organization
     ) -> Option<Self> {
         if role.is_admin() {
-            return Some(ProjectPermissions::ALL);
+            return Some(ProjectPermissions::all());
         }
 
         if let Some(member) = project_team_member {
-            return Some(member.permissions);
+            if member.accepted {
+                return Some(member.permissions);
+            }
         }
 
         if let Some(member) = organization_team_member {
-            return Some(member.permissions); // Use default project permissions for the organization team member
+            if member.accepted {
+                return Some(member.permissions);
+            }
         }
 
         if role.is_mod() {
@@ -79,18 +81,16 @@ impl ProjectPermissions {
 }
 
 bitflags::bitflags! {
-    #[derive(Copy, Clone, Debug)]
+    #[derive(Copy, Clone, Debug, PartialEq, Eq)]
     pub struct OrganizationPermissions: u64 {
         const EDIT_DETAILS = 1 << 0;
-        const EDIT_BODY = 1 << 1;
-        const MANAGE_INVITES = 1 << 2;
-        const REMOVE_MEMBER = 1 << 3;
-        const EDIT_MEMBER = 1 << 4;
-        const ADD_PROJECT = 1 << 5;
-        const REMOVE_PROJECT = 1 << 6;
-        const DELETE_ORGANIZATION = 1 << 8;
-        const EDIT_MEMBER_DEFAULT_PERMISSIONS = 1 << 9; // Separate from EDIT_MEMBER
-        const ALL = 0b1111111111;
+        const MANAGE_INVITES = 1 << 1;
+        const REMOVE_MEMBER = 1 << 2;
+        const EDIT_MEMBER = 1 << 3;
+        const ADD_PROJECT = 1 << 4;
+        const REMOVE_PROJECT = 1 << 5;
+        const DELETE_ORGANIZATION = 1 << 6;
+        const EDIT_MEMBER_DEFAULT_PERMISSIONS = 1 << 7; // Separate from EDIT_MEMBER
         const NONE = 0b0;
     }
 }
@@ -109,17 +109,17 @@ impl OrganizationPermissions {
         team_member: &Option<crate::database::models::TeamMember>,
     ) -> Option<Self> {
         if role.is_admin() {
-            return Some(OrganizationPermissions::ALL);
+            return Some(OrganizationPermissions::all());
         }
 
         if let Some(member) = team_member {
-            return member.organization_permissions;
+            if member.accepted {
+                return member.organization_permissions;
+            }
         }
         if role.is_mod() {
             return Some(
-                OrganizationPermissions::EDIT_DETAILS
-                    | OrganizationPermissions::EDIT_BODY
-                    | OrganizationPermissions::ADD_PROJECT,
+                OrganizationPermissions::EDIT_DETAILS | OrganizationPermissions::ADD_PROJECT,
             );
         }
         None

src/routes/maven.rs

@@ -206,7 +206,11 @@ async fn find_version(
         })
         .collect::<Vec<_>>();
 
-    Ok(matched.get(0).or_else(|| exact_matches.get(0)).copied().cloned())
+    Ok(matched
+        .get(0)
+        .or_else(|| exact_matches.get(0))
+        .copied()
+        .cloned())
 }
 
 fn find_file<'a>(

src/routes/v2/admin.rs

@@ -120,7 +120,7 @@ pub async fn count_download(
 
     analytics_queue.add_download(Download {
         id: Uuid::new_v4(),
-            recorded: get_current_tenths_of_ms(),
+        recorded: get_current_tenths_of_ms(),
         domain: url.host_str().unwrap_or_default().to_string(),
         site_path: url.path().to_string(),
         user_id: user

src/routes/v2/organizations.rs

@@ -94,8 +94,8 @@ pub async fn organization_create(
         members: vec![team_item::TeamMemberBuilder {
             user_id: current_user.id.into(),
             role: crate::models::teams::OWNER_ROLE.to_owned(),
-            permissions: ProjectPermissions::ALL,
-            organization_permissions: Some(OrganizationPermissions::ALL),
+            permissions: ProjectPermissions::all(),
+            organization_permissions: Some(OrganizationPermissions::all()),
             accepted: true,
             payouts_split: Decimal::ONE_HUNDRED,
             ordering: 0,

src/routes/v2/teams.rs

@@ -407,12 +407,10 @@ pub async fn add_team_member(
     )
     .await?
     .1;
-
     let team_association = Team::get_association(team_id, &**pool)
         .await?
         .ok_or_else(|| ApiError::InvalidInput("The team specified does not exist".to_string()))?;
     let member = TeamMember::get_from_user_id(team_id, current_user.id.into(), &**pool).await?;
-
     match team_association {
         // If team is associated with a project, check if they have permissions to invite users to that project
         TeamAssociationId::Project(pid) => {
@@ -470,8 +468,8 @@ pub async fn add_team_member(
                 .contains(OrganizationPermissions::EDIT_MEMBER_DEFAULT_PERMISSIONS)
                 && !new_member.permissions.is_empty()
             {
-                return Err(ApiError::InvalidInput(
-                    "You do not have permission to give this user default project permissions."
+                return Err(ApiError::CustomAuthentication(
+                    "You do not have permission to give this user default project permissions. Ensure 'permissions' is set if it is not, and empty (0)."
                         .to_string(),
                 ));
             }
@@ -654,8 +652,8 @@ pub async fn edit_team_member(
                     .unwrap_or_default();
 
             if !organization_permissions.contains(OrganizationPermissions::EDIT_MEMBER) {
-                return Err(ApiError::InvalidInput(
-                    "You don't have permission to edit organization permissions".to_string(),
+                return Err(ApiError::CustomAuthentication(
+                    "You don't have permission to edit members of this team".to_string(),
                 ));
             }
 
@@ -672,7 +670,7 @@ pub async fn edit_team_member(
                 && !organization_permissions
                     .contains(OrganizationPermissions::EDIT_MEMBER_DEFAULT_PERMISSIONS)
             {
-                return Err(ApiError::InvalidInput(
+                return Err(ApiError::CustomAuthentication(
                     "You do not have permission to give this user default project permissions."
                         .to_string(),
                 ));
@@ -884,7 +882,6 @@ pub async fn remove_team_member(
                     // removed by a member with the REMOVE_MEMBER permission.
                     if Some(delete_member.user_id) == member.as_ref().map(|m| m.user_id)
                         || permissions.contains(ProjectPermissions::REMOVE_MEMBER)
-                            && member.as_ref().map(|m| m.accepted).unwrap_or(true)
                     // true as if the permission exists, but the member does not, they are part of an org
                     {
                         TeamMember::delete(id, user_id, &mut transaction).await?;
@@ -896,7 +893,6 @@ pub async fn remove_team_member(
                     }
                 } else if Some(delete_member.user_id) == member.as_ref().map(|m| m.user_id)
                     || permissions.contains(ProjectPermissions::MANAGE_INVITES)
-                        && member.as_ref().map(|m| m.accepted).unwrap_or(true)
                 // true as if the permission exists, but the member does not, they are part of an org
                 {
                     // This is a pending invite rather than a member, so the
@@ -913,49 +909,37 @@ pub async fn remove_team_member(
                 let organization_permissions =
                     OrganizationPermissions::get_permissions_by_role(&current_user.role, &member)
                         .unwrap_or_default();
-                if let Some(member) = member {
-                    // Organization teams requires a TeamMember, so we can 'unwrap'
-                    if delete_member.accepted {
-                        // Members other than the owner can either leave the team, or be
-                        // removed by a member with the REMOVE_MEMBER permission.
-                        if delete_member.user_id == member.user_id
-                            || organization_permissions
-                                .contains(OrganizationPermissions::REMOVE_MEMBER)
-                                && member.accepted
-                        {
-                            TeamMember::delete(id, user_id, &mut transaction).await?;
-                        } else {
-                            return Err(ApiError::CustomAuthentication(
-                            "You do not have permission to remove a member from this organization"
-                                .to_string(),
-                        ));
-                        }
-                    } else if delete_member.user_id == member.user_id
-                        || organization_permissions
-                            .contains(OrganizationPermissions::MANAGE_INVITES)
-                            && member.accepted
+                // Organization teams requires a TeamMember, so we can 'unwrap'
+                if delete_member.accepted {
+                    // Members other than the owner can either leave the team, or be
+                    // removed by a member with the REMOVE_MEMBER permission.
+                    if Some(delete_member.user_id) == member.map(|m| m.user_id)
+                        || organization_permissions.contains(OrganizationPermissions::REMOVE_MEMBER)
                     {
-                        // This is a pending invite rather than a member, so the
-                        // user being invited or team members with the MANAGE_INVITES
-                        // permission can remove it.
                         TeamMember::delete(id, user_id, &mut transaction).await?;
                     } else {
                         return Err(ApiError::CustomAuthentication(
-                            "You do not have permission to cancel an organization invite"
+                            "You do not have permission to remove a member from this organization"
                                 .to_string(),
                         ));
                     }
+                } else if Some(delete_member.user_id) == member.map(|m| m.user_id)
+                    || organization_permissions.contains(OrganizationPermissions::MANAGE_INVITES)
+                {
+                    // This is a pending invite rather than a member, so the
+                    // user being invited or team members with the MANAGE_INVITES
+                    // permission can remove it.
+                    TeamMember::delete(id, user_id, &mut transaction).await?;
                 } else {
                     return Err(ApiError::CustomAuthentication(
-                        "You do not have permission to remove a member from this organization"
-                            .to_string(),
+                        "You do not have permission to cancel an organization invite".to_string(),
                     ));
                 }
             }
         }
 
         TeamMember::clear_cache(id, &redis).await?;
-        User::clear_project_cache(&[delete_member.user_id.into()], &redis).await?;
+        User::clear_project_cache(&[delete_member.user_id], &redis).await?;
 
         transaction.commit().await?;
         Ok(HttpResponse::NoContent().body(""))

tests/common/actix.rs

@@ -18,11 +18,11 @@ pub enum MultipartSegmentData {
 }
 
 pub trait AppendsMultipart {
-    fn set_multipart(self, data: Vec<MultipartSegment>) -> Self;
+    fn set_multipart(self, data: impl IntoIterator<Item = MultipartSegment>) -> Self;
 }
 
 impl AppendsMultipart for TestRequest {
-    fn set_multipart(self, data: Vec<MultipartSegment>) -> Self {
+    fn set_multipart(self, data: impl IntoIterator<Item = MultipartSegment>) -> Self {
         let (boundary, payload) = generate_multipart(data);
         self.append_header((
             "Content-Type",
@@ -32,7 +32,7 @@ impl AppendsMultipart for TestRequest {
     }
 }
 
-fn generate_multipart(data: Vec<MultipartSegment>) -> (String, Bytes) {
+fn generate_multipart(data: impl IntoIterator<Item = MultipartSegment>) -> (String, Bytes) {
     let mut boundary = String::from("----WebKitFormBoundary");
     boundary.push_str(&rand::random::<u64>().to_string());
     boundary.push_str(&rand::random::<u64>().to_string());

tests/common/api_v2/mod.rs

@@ -0,0 +1,20 @@
+#![allow(dead_code)]
+
+use super::environment::LocalService;
+use actix_web::dev::ServiceResponse;
+use std::rc::Rc;
+
+pub mod organization;
+pub mod project;
+pub mod team;
+
+#[derive(Clone)]
+pub struct ApiV2 {
+    pub test_app: Rc<dyn LocalService>,
+}
+
+impl ApiV2 {
+    pub async fn call(&self, req: actix_http::Request) -> ServiceResponse {
+        self.test_app.call(req).await.unwrap()
+    }
+}