fixed ratings authorisation

backend/server/src/handler/ratings.rs

@@ -1,5 +1,8 @@
 use crate::models::app::AppState;
-use crate::models::auth::{ApplicationReviewerAdmin, SuperUser};
+use crate::models::auth::{
+    ApplicationReviewerAdminGivenApplicationId, ApplicationReviewerAdminGivenRatingId,
+    RatingCreatorAdmin, SuperUser,
+};
 use crate::models::error::ChaosError;
 use crate::models::ratings::{NewRating, Rating};
 use crate::models::transaction::DBTransaction;
@@ -14,7 +17,8 @@ impl RatingsHandler {
     pub async fn create_rating(
         State(state): State<AppState>,
         Path(application_id): Path<i64>,
-        _admin: ApplicationReviewerAdmin,
+        // TODO: Potential bug: the check whether user is allowed to access rating doesn't make sense here, because no rating exists yet
+        _admin: ApplicationReviewerAdminGivenApplicationId,
         mut transaction: DBTransaction<'_>,
         Json(new_rating): Json<NewRating>,
     ) -> Result<impl IntoResponse, ChaosError> {
@@ -32,8 +36,7 @@ impl RatingsHandler {
     pub async fn update(
         State(_state): State<AppState>,
         Path(rating_id): Path<i64>,
-        // TODO: authorization: needs to be user that created the rating and is a current member of organisation.
-        _admin: ApplicationReviewerAdmin,
+        _admin: RatingCreatorAdmin,
         mut transaction: DBTransaction<'_>,
         Json(updated_rating): Json<NewRating>,
     ) -> Result<impl IntoResponse, ChaosError> {
@@ -45,7 +48,7 @@ impl RatingsHandler {
     pub async fn get_ratings_for_application(
         State(_state): State<AppState>,
         Path(application_id): Path<i64>,
-        _admin: ApplicationReviewerAdmin,
+        _admin: ApplicationReviewerAdminGivenApplicationId,
         mut transaction: DBTransaction<'_>,
     ) -> Result<impl IntoResponse, ChaosError> {
         let ratings =
@@ -58,7 +61,7 @@ impl RatingsHandler {
     pub async fn get(
         State(_state): State<AppState>,
         Path(rating_id): Path<i64>,
-        _admin: ApplicationReviewerAdmin,
+        _admin: ApplicationReviewerAdminGivenRatingId,
         mut transaction: DBTransaction<'_>,
     ) -> Result<impl IntoResponse, ChaosError> {
         let org = Rating::get_rating(rating_id, &mut transaction.tx).await?;
@@ -68,11 +71,11 @@ impl RatingsHandler {
 
     pub async fn delete(
         State(_state): State<AppState>,
-        Path(id): Path<i64>,
-        _admin: ApplicationReviewerAdmin,
+        Path(rating_id): Path<i64>,
+        _admin: ApplicationReviewerAdminGivenRatingId,
         mut transaction: DBTransaction<'_>,
     ) -> Result<impl IntoResponse, ChaosError> {
-        Rating::delete(id, &mut transaction.tx).await?;
+        Rating::delete(rating_id, &mut transaction.tx).await?;
         transaction.tx.commit().await?;
         Ok((StatusCode::OK, "Successfully deleted rating"))
     }

backend/server/src/models/auth.rs

@@ -4,7 +4,9 @@ use crate::service::auth::is_super_user;
 use crate::service::jwt::decode_auth_token;
 use crate::service::organisation::assert_user_is_admin;
 use crate::service::ratings::{
-    assert_user_is_application_reviewer_admin, assert_user_is_rating_creator,
+    assert_user_is_application_reviewer_admin_given_application_id,
+    assert_user_is_application_reviewer_admin_given_rating_id,
+    assert_user_is_rating_creator_and_organisation_member,
 };
 use axum::extract::{FromRef, FromRequestParts, Path};
 use axum::http::request::Parts;
@@ -148,12 +150,64 @@ where
     }
 }
 
-pub struct ApplicationReviewerAdmin {
+// TODO: Not very idiomatic way. The reason this impl was chosen was because we
+// couldn't figure out how to dynamically check whether the id passed in path
+// was a rating id or an application id.
+
+/// Get the application reviewer given a path that contains the application id.
+pub struct ApplicationReviewerAdminGivenApplicationId {
     pub user_id: i64,
 }
 
 #[async_trait]
-impl<S> FromRequestParts<S> for ApplicationReviewerAdmin
+impl<S> FromRequestParts<S> for ApplicationReviewerAdminGivenApplicationId
+where
+    AppState: FromRef<S>,
+    S: Send + Sync,
+{
+    type Rejection = ChaosError;
+
+    async fn from_request_parts(parts: &mut Parts, state: &S) -> Result<Self, Self::Rejection> {
+        // TODO: put into separate function, since this is just getting the id through jwt, and duplicated here.
+        let app_state = AppState::from_ref(state);
+        let decoding_key = &app_state.decoding_key;
+        let jwt_validator = &app_state.jwt_validator;
+        let TypedHeader(cookies) = parts
+            .extract::<TypedHeader<Cookie>>()
+            .await
+            .map_err(|_| ChaosError::NotLoggedIn)?;
+
+        let token = cookies.get("auth_token").ok_or(ChaosError::NotLoggedIn)?;
+
+        let claims =
+            decode_auth_token(token, decoding_key, jwt_validator).ok_or(ChaosError::NotLoggedIn)?;
+
+        let pool = &app_state.db;
+        let user_id = claims.sub;
+
+        let Path(application_id) = parts
+            .extract::<Path<i64>>()
+            .await
+            .map_err(|_| ChaosError::BadRequest)?;
+
+        assert_user_is_application_reviewer_admin_given_application_id(
+            user_id,
+            application_id,
+            pool,
+        )
+        .await?;
+
+        Ok(ApplicationReviewerAdminGivenApplicationId { user_id })
+    }
+}
+
+/// Get the application reviewer given a path that contains the rating id.
+pub struct ApplicationReviewerAdminGivenRatingId {
+    pub user_id: i64,
+}
+
+#[async_trait]
+impl<S> FromRequestParts<S> for ApplicationReviewerAdminGivenRatingId
 where
     AppState: FromRef<S>,
     S: Send + Sync,
@@ -178,16 +232,14 @@ where
         let pool = &app_state.db;
         let user_id = claims.sub;
 
-        // TODO: check if application_id or rating_id was passed.
-        // TODO: How am I guaranteeing this rating id is in the endpoint? Would it be a bad request if the rating id was left out?
         let Path(rating_id) = parts
             .extract::<Path<i64>>()
             .await
             .map_err(|_| ChaosError::BadRequest)?;
 
-        assert_user_is_application_reviewer_admin(user_id, rating_id, pool).await?;
+        assert_user_is_application_reviewer_admin_given_rating_id(user_id, rating_id, pool).await?;
 
-        Ok(ApplicationReviewerAdmin { user_id })
+        Ok(ApplicationReviewerAdminGivenRatingId { user_id })
     }
 }
 
@@ -221,13 +273,12 @@ where
         let pool = &app_state.db;
         let user_id = claims.sub;
 
-        // TODO
-        // let Path(rating_id) = parts
-        //     .extract::<Path<i64>>()
-        //     .await
-        //     .map_err(|_| ChaosError::BadRequest)?;
+        let Path(rating_id) = parts
+            .extract::<Path<i64>>()
+            .await
+            .map_err(|_| ChaosError::BadRequest)?;
 
-        // assert_user_is_rating_creator(user_id, rating_id, pool).await?;
+        assert_user_is_rating_creator_and_organisation_member(user_id, rating_id, pool).await?;
 
         Ok(RatingCreatorAdmin { user_id })
     }

backend/server/src/service/ratings.rs

@@ -10,7 +10,46 @@ use uuid::Uuid;
 /// Any member of the organisation that owns the campaign is an application
 /// viewer, because all members are either directors or execs (TODO: might be
 /// changed in the future).
-pub async fn assert_user_is_application_reviewer_admin(
+pub async fn assert_user_is_application_reviewer_admin_given_application_id(
+    user_id: i64,
+    application_id: i64,
+    pool: &Pool<Postgres>,
+) -> Result<(), ChaosError> {
+    let is_admin = sqlx::query!(
+        r#"
+        SELECT EXISTS (
+            SELECT 1
+            FROM organisation_members om
+            JOIN campaigns c ON om.organisation_id = c.organisation_id
+            JOIN applications a ON a.campaign_id = c.id
+            JOIN application_ratings ar ON a.campaign_id = c.id
+            -- Find the organisation that this application rating belongs to
+            -- (via the campaign the rating belongs to).
+            WHERE a.id = $1
+            -- Assert user is member of the organisation that owns the campaign
+            -- this application belongs to.
+            AND om.user_id = $2
+        )
+        "#,
+        application_id,
+        user_id
+    )
+    .fetch_one(pool)
+    .await?
+    .exists
+    .expect("`exists` should always exist in this query result");
+
+    if !is_admin {
+        return Err(ChaosError::Unauthorized);
+    }
+
+    Ok(())
+}
+
+/// Any member of the organisation that owns the campaign is an application
+/// viewer, because all members are either directors or execs (TODO: might be
+/// changed in the future).
+pub async fn assert_user_is_application_reviewer_admin_given_rating_id(
     user_id: i64,
     rating_id: i64,
     pool: &Pool<Postgres>,
@@ -46,10 +85,43 @@ pub async fn assert_user_is_application_reviewer_admin(
     Ok(())
 }
 
-pub async fn assert_user_is_rating_creator(
+/// Assert the given user is the creator of the rating with the given id and
+/// also a current member of the organisation the rating/application is
+/// associated with.
+pub async fn assert_user_is_rating_creator_and_organisation_member(
     user_id: i64,
     rating_id: i64,
     pool: &Pool<Postgres>,
 ) -> Result<(), ChaosError> {
-    todo!()
+    let is_admin = sqlx::query!(
+        r#"
+        SELECT EXISTS (
+            SELECT 1
+            FROM organisation_members om
+            JOIN campaigns c ON om.organisation_id = c.organisation_id
+            JOIN applications a ON a.campaign_id = c.id
+            JOIN application_ratings ar ON ar.application_id = a.id
+            -- Find the organisation that this application rating belongs to
+            -- (via the campaign the rating belongs to).
+            WHERE ar.id = $1
+            -- Assert user is the rater of the application rating.
+            AND ar.rater_id = $2
+            -- Assert user is current member of the organisation that owns the
+            -- campaign this application belongs to.
+            AND om.user_id = $2
+        )
+        "#,
+        rating_id,
+        user_id
+    )
+    .fetch_one(pool)
+    .await?
+    .exists
+    .expect("`exists` should always exist in this query result");
+
+    if !is_admin {
+        return Err(ChaosError::Unauthorized);
+    }
+
+    Ok(())
 }