From abbeed394e297e7bced3cc1771e28a2744b16549 Mon Sep 17 00:00:00 2001
From: Emma Alexia <emma@modrinth.com>
Date: Tue, 12 Dec 2023 22:45:32 -0500
Subject: [PATCH] Fix project visibility in hash routes (#792)

* Fix project visibility in hash routes

* improve

* clippy

* CLIPPYYYYYYYYYYYYYY

* clippy, I hope you know that I hate you

---------

Co-authored-by: Geometrically <18202329+Geometrically@users.noreply.github.com>
---
 src/auth/checks.rs            | 68 ++++++++++++-----------------------
 src/routes/updates.rs         |  1 +
 src/routes/v3/version_file.rs |  1 +
 src/routes/v3/versions.rs     |  4 +--
 4 files changed, 26 insertions(+), 48 deletions(-)

diff --git a/src/auth/checks.rs b/src/auth/checks.rs
index 4d47e72c..25e2f710 100644
--- a/src/auth/checks.rs
+++ b/src/auth/checks.rs
@@ -2,6 +2,7 @@ use crate::database;
 use crate::database::models::project_item::QueryProject;
 use crate::database::models::version_item::QueryVersion;
 use crate::database::models::Collection;
+use crate::database::redis::RedisPool;
 use crate::database::{models, Project, Version};
 use crate::models::users::User;
 use crate::routes::ApiError;
@@ -175,16 +176,16 @@ pub async fn is_authorized_version(
     Ok(authorized)
 }
 
-impl ValidateAuthorized for crate::database::models::OAuthClient {
+impl ValidateAuthorized for models::OAuthClient {
     fn validate_authorized(&self, user_option: Option<&User>) -> Result<(), ApiError> {
         if let Some(user) = user_option {
-            if user.role.is_mod() || user.id == self.created_by.into() {
-                return Ok(());
+            return if user.role.is_mod() || user.id == self.created_by.into() {
+                Ok(())
             } else {
-                return Err(crate::routes::ApiError::CustomAuthentication(
+                Err(ApiError::CustomAuthentication(
                     "You don't have sufficient permissions to interact with this OAuth application"
                         .to_string(),
-                ));
+                ))
             }
         }
 
@@ -196,9 +197,23 @@ pub async fn filter_authorized_versions(
     versions: Vec<QueryVersion>,
     user_option: &Option<User>,
     pool: &web::Data<PgPool>,
+    redis: web::Data<RedisPool>,
 ) -> Result<Vec<crate::models::projects::Version>, ApiError> {
     let mut return_versions = Vec::new();
-    let mut check_versions = Vec::new();
+
+    let project_ids = versions
+        .iter()
+        .map(|x| x.inner.project_id)
+        .collect::<Vec<_>>();
+
+    let authorized_projects = filter_authorized_projects(
+        Project::get_many_ids(&project_ids, &***pool, &redis).await?,
+        user_option,
+        pool,
+    )
+    .await?;
+
+    let authorized_project_ids: Vec<_> = authorized_projects.iter().map(|x| x.id.into()).collect();
 
     for version in versions {
         if !version.inner.status.is_hidden()
@@ -206,48 +221,9 @@ pub async fn filter_authorized_versions(
                 .as_ref()
                 .map(|x| x.role.is_mod())
                 .unwrap_or(false)
+            || (user_option.is_some() && authorized_project_ids.contains(&version.inner.project_id))
         {
             return_versions.push(version.into());
-        } else if user_option.is_some() {
-            check_versions.push(version);
-        }
-    }
-
-    if !check_versions.is_empty() {
-        if let Some(user) = user_option {
-            let user_id: models::ids::UserId = user.id.into();
-
-            use futures::TryStreamExt;
-
-            sqlx::query!(
-                "
-                SELECT m.id FROM mods m
-                INNER JOIN team_members tm ON tm.team_id = m.team_id AND user_id = $2
-                WHERE m.id = ANY($1)
-                ",
-                &check_versions
-                    .iter()
-                    .map(|x| x.inner.project_id.0)
-                    .collect::<Vec<_>>(),
-                user_id as database::models::ids::UserId,
-            )
-            .fetch_many(&***pool)
-            .try_for_each(|e| {
-                if let Some(row) = e.right() {
-                    check_versions.retain(|x| {
-                        let bool = x.inner.project_id.0 == row.id;
-
-                        if bool {
-                            return_versions.push(x.clone().into());
-                        }
-
-                        !bool
-                    });
-                }
-
-                futures::future::ready(Ok(()))
-            })
-            .await?;
         }
     }
 
diff --git a/src/routes/updates.rs b/src/routes/updates.rs
index f4d6d2f8..2d29af8b 100644
--- a/src/routes/updates.rs
+++ b/src/routes/updates.rs
@@ -75,6 +75,7 @@ pub async fn forge_updates(
             .collect(),
         &user_option,
         &pool,
+        redis,
     )
     .await?;
 
diff --git a/src/routes/v3/version_file.rs b/src/routes/v3/version_file.rs
index 2006d481..c6b9d6ee 100644
--- a/src/routes/v3/version_file.rs
+++ b/src/routes/v3/version_file.rs
@@ -234,6 +234,7 @@ pub async fn get_versions_from_hashes(
         database::models::Version::get_many(&version_ids, &**pool, &redis).await?,
         &user_option,
         &pool,
+        redis,
     )
     .await?;
 
diff --git a/src/routes/v3/versions.rs b/src/routes/v3/versions.rs
index 54da5fbb..7c27c986 100644
--- a/src/routes/v3/versions.rs
+++ b/src/routes/v3/versions.rs
@@ -134,7 +134,7 @@ pub async fn versions_get(
     .map(|x| x.1)
     .ok();
 
-    let versions = filter_authorized_versions(versions_data, &user_option, &pool).await?;
+    let versions = filter_authorized_versions(versions_data, &user_option, &pool, redis).await?;
 
     Ok(HttpResponse::Ok().json(versions))
 }
@@ -820,7 +820,7 @@ pub async fn version_list(
         response.sort();
         response.dedup_by(|a, b| a.inner.id == b.inner.id);
 
-        let response = filter_authorized_versions(response, &user_option, &pool).await?;
+        let response = filter_authorized_versions(response, &user_option, &pool, redis).await?;
 
         Ok(HttpResponse::Ok().json(response))
     } else {