Secure auth route, fix quilt deps bug, optimize queries more (#374)

* Secure auth route, fix quilt deps bug, optimize queries more

* Add to_lowercase for multiple hashes functions

src/routes/auth.rs

@@ -1,3 +1,16 @@
+ /*!
+This auth module is primarily for use within the main website. Applications interacting with the
+authenticated API (a very small portion - notifications, private projects, editing/creating projects
+and versions) should either retrieve the Modrinth GitHub token through the site, or create a personal
+app token for use with Modrinth.
+
+JUst as a summary: Don't implement this flow in your application! Instead, use a personal access token
+or create your own GitHub OAuth2 application.
+
+This system will be revisited and allow easier interaction with the authenticated API once we roll
+out our own authentication system.
+*/
+
 use crate::database::models::{generate_state_id, User};
 use crate::models::error::ApiError;
 use crate::models::ids::base62_impl::{parse_base62, to_base62};
@@ -11,6 +24,7 @@ use serde::{Deserialize, Serialize};
 use sqlx::postgres::PgPool;
 use thiserror::Error;
 use time::OffsetDateTime;
+use crate::parse_strings_from_var;
 
 pub fn config(cfg: &mut ServiceConfig) {
     cfg.service(scope("auth").service(auth_callback).service(init));
@@ -34,6 +48,8 @@ pub enum AuthorizationError {
     Authentication(#[from] crate::util::auth::AuthenticationError),
     #[error("Error while decoding Base62")]
     Decoding(#[from] DecodingError),
+    #[error("Invalid callback URL specified")]
+    Url,
 }
 impl actix_web::ResponseError for AuthorizationError {
     fn status_code(&self) -> StatusCode {
@@ -50,6 +66,7 @@ impl actix_web::ResponseError for AuthorizationError {
             AuthorizationError::InvalidCredentials => StatusCode::UNAUTHORIZED,
             AuthorizationError::Decoding(..) => StatusCode::BAD_REQUEST,
             AuthorizationError::Authentication(..) => StatusCode::UNAUTHORIZED,
+            AuthorizationError::Url => StatusCode::BAD_REQUEST,
         }
     }
 
@@ -65,7 +82,8 @@ impl actix_web::ResponseError for AuthorizationError {
                 AuthorizationError::Decoding(..) => "decoding_error",
                 AuthorizationError::Authentication(..) => {
                     "authentication_error"
-                }
+                },
+                AuthorizationError::Url => "url_error",
             },
             description: &self.to_string(),
         })
@@ -96,6 +114,16 @@ pub async fn init(
     Query(info): Query<AuthorizationInit>,
     client: Data<PgPool>,
 ) -> Result<HttpResponse, AuthorizationError> {
+    let url = url::Url::parse(&info.url).map_err(|_| AuthorizationError::Url)?;
+
+    let allowed_callback_urls = parse_strings_from_var("ALLOWED_CALLBACK_URLS")
+        .unwrap_or_default();
+
+    let domain = url.domain().ok_or(AuthorizationError::Url)?;
+    if !allowed_callback_urls.iter().any(|x| domain.ends_with(x)) {
+        return Err(AuthorizationError::Url);
+    }
+
     let mut transaction = client.begin().await?;
 
     let state = generate_state_id(&mut transaction).await?;
@@ -136,7 +164,7 @@ pub async fn auth_callback(
 
     let result_option = sqlx::query!(
         "
-            SELECT url,expires FROM states
+            SELECT url, expires FROM states
             WHERE id = $1
             ",
         state_id as i64
@@ -145,13 +173,11 @@ pub async fn auth_callback(
     .await?;
 
     if let Some(result) = result_option {
-        // let now = OffsetDateTime::now_utc();
-        // TODO: redo this condition later..
-        // let duration = now - result.expires;
-        //
-        // if duration.whole_seconds() < 0 {
-        //     return Err(AuthorizationError::InvalidCredentials);
-        // }
+        let duration = result.expires - OffsetDateTime::now_utc();
+
+        if duration.whole_seconds() < 0 {
+            return Err(AuthorizationError::InvalidCredentials);
+        }
 
         sqlx::query!(
             "

src/routes/teams.rs

@@ -385,18 +385,29 @@ pub async fn transfer_ownership(
     let id = info.into_inner().0;
 
     let current_user = get_user_from_headers(req.headers(), &**pool).await?;
-    let member = TeamMember::get_from_user_id(
-        id.into(),
-        current_user.id.into(),
-        &**pool,
-    )
-    .await?
-    .ok_or_else(|| {
-        ApiError::CustomAuthentication(
-            "You don't have permission to edit members of this team"
-                .to_string(),
+
+    if !current_user.role.is_mod() {
+        let member = TeamMember::get_from_user_id(
+            id.into(),
+            current_user.id.into(),
+            &**pool,
         )
-    })?;
+        .await?
+        .ok_or_else(|| {
+            ApiError::CustomAuthentication(
+                "You don't have permission to edit members of this team"
+                    .to_string(),
+            )
+        })?;
+
+        if member.role != crate::models::teams::OWNER_ROLE {
+            return Err(ApiError::CustomAuthentication(
+                "You don't have permission to edit the ownership of this team"
+                    .to_string(),
+            ));
+        }
+    }
+
     let new_member = TeamMember::get_from_user_id(
         id.into(),
         new_owner.user_id.into(),
@@ -409,13 +420,6 @@ pub async fn transfer_ownership(
         )
     })?;
 
-    if member.role != crate::models::teams::OWNER_ROLE {
-        return Err(ApiError::CustomAuthentication(
-            "You don't have permission to edit the ownership of this team"
-                .to_string(),
-        ));
-    }
-
     if !new_member.accepted {
         return Err(ApiError::InvalidInput(
             "You can only transfer ownership to members who are currently in your team".to_string(),

src/routes/version_creation.rs

@@ -35,7 +35,7 @@ pub struct InitialVersionData {
         regex = "crate::util::validate::RE_URL_SAFE"
     )]
     pub version_number: String,
-    #[validate(length(min = 3, max = 256))]
+    #[validate(length(min = 1, max = 256))]
     #[serde(alias = "name")]
     pub version_title: String,
     #[validate(length(max = 65536))]
@@ -639,11 +639,11 @@ pub async fn upload_file(
     field: &mut Field,
     file_host: &dyn FileHost,
     uploaded_files: &mut Vec<UploadedFile>,
-    version_files: &mut Vec<models::version_item::VersionFileBuilder>,
-    dependencies: &mut Vec<models::version_item::DependencyBuilder>,
+    version_files: &mut Vec<VersionFileBuilder>,
+    dependencies: &mut Vec<DependencyBuilder>,
     cdn_url: &str,
     content_disposition: &actix_web::http::header::ContentDisposition,
-    project_id: crate::models::ids::ProjectId,
+    project_id: ProjectId,
     version_number: &str,
     project_type: &str,
     loaders: Vec<Loader>,

src/routes/version_file.rs

@@ -303,7 +303,7 @@ pub async fn get_versions_from_hashes(
     let hashes_parsed: Vec<Vec<u8>> = file_data
         .hashes
         .iter()
-        .map(|x| x.as_bytes().to_vec())
+        .map(|x| x.to_lowercase().as_bytes().to_vec())
         .collect();
 
     let result = sqlx::query!(
@@ -360,7 +360,7 @@ pub async fn download_files(
     let hashes_parsed: Vec<Vec<u8>> = file_data
         .hashes
         .iter()
-        .map(|x| x.as_bytes().to_vec())
+        .map(|x| x.to_lowercase().as_bytes().to_vec())
         .collect();
 
     let mut transaction = pool.begin().await?;
@@ -411,7 +411,7 @@ pub async fn update_files(
     let hashes_parsed: Vec<Vec<u8>> = update_data
         .hashes
         .iter()
-        .map(|x| x.as_bytes().to_vec())
+        .map(|x| x.to_lowercase().as_bytes().to_vec())
         .collect();
 
     let mut transaction = pool.begin().await?;