Skip to content

Commit

Permalink
removed redundant authorisation service
Browse files Browse the repository at this point in the history
  • Loading branch information
fritzrehde committed Nov 11, 2024
1 parent 5ed0c70 commit d7b5851
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 50 deletions.
7 changes: 3 additions & 4 deletions backend/server/src/handler/ratings.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::models::app::AppState;
use crate::models::auth::{
ApplicationReviewerAdminGivenApplicationId, ApplicationReviewerAdminGivenRatingId,
RatingCreatorAdmin, SuperUser,
ApplicationCreatorAdminGivenApplicationId, ApplicationReviewerAdminGivenApplicationId,
ApplicationReviewerAdminGivenRatingId, RatingCreatorAdmin, SuperUser,
};
use crate::models::error::ChaosError;
use crate::models::ratings::{NewRating, Rating};
Expand All @@ -17,8 +17,7 @@ impl RatingsHandler {
pub async fn create_rating(
State(state): State<AppState>,
Path(application_id): Path<i64>,
// TODO: Potential bug: the check whether user is allowed to access rating doesn't make sense here, because no rating exists yet
_admin: ApplicationReviewerAdminGivenApplicationId,
_admin: ApplicationCreatorAdminGivenApplicationId,
mut transaction: DBTransaction<'_>,
Json(new_rating): Json<NewRating>,
) -> Result<impl IntoResponse, ChaosError> {
Expand Down
11 changes: 4 additions & 7 deletions backend/server/src/models/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ 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_given_application_id,
assert_user_is_application_reviewer_admin_given_rating_id, assert_user_is_organisation_member,
assert_user_is_rating_creator_and_organisation_member,
};
Expand Down Expand Up @@ -154,6 +153,9 @@ where
// couldn't figure out how to dynamically check whether the id passed in path
// was a rating id or an application id.

// TODO: there is currently no diff between ApplicationReviewerAdminGivenApplicationId
// and ApplicationCreatorAdminGivenApplicationId, but that might change, so separating just in case.

/// Get the application reviewer given a path that contains the application id.
pub struct ApplicationReviewerAdminGivenApplicationId {
pub user_id: i64,
Expand Down Expand Up @@ -190,12 +192,7 @@ where
.await
.map_err(|_| ChaosError::BadRequest)?;

assert_user_is_application_reviewer_admin_given_application_id(
user_id,
application_id,
pool,
)
.await?;
assert_user_is_organisation_member(user_id, application_id, pool).await?;

Ok(ApplicationReviewerAdminGivenApplicationId { user_id })
}
Expand Down
39 changes: 0 additions & 39 deletions backend/server/src/service/ratings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,45 +7,6 @@ use sqlx::{Pool, Postgres, Transaction};
use std::ops::DerefMut;
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_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).
Expand Down

0 comments on commit d7b5851

Please sign in to comment.