Skip to content

Commit

Permalink
backend: added auth to ratings CRUD
Browse files Browse the repository at this point in the history
  • Loading branch information
fritzrehde committed Sep 30, 2024
1 parent 26b3cac commit 09e41d1
Show file tree
Hide file tree
Showing 7 changed files with 105 additions and 13 deletions.
12 changes: 5 additions & 7 deletions backend/server/src/handler/ratings.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::models::app::AppState;
use crate::models::auth::SuperUser;
use crate::models::auth::{ApplicationReviewerAdmin, SuperUser};
use crate::models::error::ChaosError;
use crate::models::ratings::{NewRating, Rating};
use crate::models::transaction::DBTransaction;
Expand All @@ -13,7 +13,7 @@ impl RatingsHandler {
// TODO: are all the user permissions as required? Who should be able to do what with ratings?
pub async fn create_rating(
State(state): State<AppState>,
_user: SuperUser,
_admin: ApplicationReviewerAdmin,
mut transaction: DBTransaction<'_>,
Json(new_rating): Json<NewRating>,
) -> Result<impl IntoResponse, ChaosError> {
Expand All @@ -25,7 +25,7 @@ impl RatingsHandler {
pub async fn get_ratings_for_application(
State(_state): State<AppState>,
Path(application_id): Path<i64>,
_user: SuperUser,
_admin: ApplicationReviewerAdmin,
mut transaction: DBTransaction<'_>,
) -> Result<impl IntoResponse, ChaosError> {
let ratings =
Expand All @@ -35,23 +35,21 @@ impl RatingsHandler {
Ok((StatusCode::OK, Json(ratings)))
}

// TODO: should I use transaction here? Organisation still uses state.db for these simpler getters.
pub async fn get(
State(_state): State<AppState>,
Path(rating_id): Path<i64>,
_user: SuperUser,
_admin: ApplicationReviewerAdmin,
mut transaction: DBTransaction<'_>,
) -> Result<impl IntoResponse, ChaosError> {
let org = Rating::get_rating(rating_id, &mut transaction.tx).await?;
transaction.tx.commit().await?;
Ok((StatusCode::OK, Json(org)))
}

// TODO: should I use transaction here?
pub async fn delete(
State(_state): State<AppState>,
Path(id): Path<i64>,
_user: SuperUser,
_admin: ApplicationReviewerAdmin,
mut transaction: DBTransaction<'_>,
) -> Result<impl IntoResponse, ChaosError> {
Rating::delete(id, &mut transaction.tx).await?;
Expand Down
8 changes: 5 additions & 3 deletions backend/server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::handler::auth::google_callback;
use crate::handler::organisation::OrganisationHandler;
use crate::models::storage::Storage;
use anyhow::Result;
use axum::routing::{get, patch, post, put};
use axum::routing::{delete, get, patch, post, put};
use axum::Router;
use handler::ratings::RatingsHandler;
use jsonwebtoken::{Algorithm, DecodingKey, EncodingKey, Header, Validation};
Expand Down Expand Up @@ -90,7 +90,10 @@ async fn main() -> Result<()> {
.put(OrganisationHandler::update_admins)
.delete(OrganisationHandler::remove_admin),
)
.route("/api/v1/ratings/:rating_id", get(RatingsHandler::get))
.route(
"/api/v1/ratings/:rating_id",
get(RatingsHandler::get).delete(RatingsHandler::delete),
)
.route(
"/api/v1/:application_id/rating",
put(RatingsHandler::create_rating),
Expand All @@ -99,7 +102,6 @@ async fn main() -> Result<()> {
"/api/v1/:application_id/ratings",
get(RatingsHandler::get_ratings_for_application),
)
// TODO: should ratings also have a delete endpoint? I think old backend didn't
.with_state(state);

let listener = tokio::net::TcpListener::bind("0.0.0.0:3000").await.unwrap();
Expand Down
47 changes: 45 additions & 2 deletions backend/server/src/models/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ use crate::models::app::AppState;
use crate::models::error::ChaosError;
use crate::service::auth::is_super_user;
use crate::service::jwt::decode_auth_token;
use crate::service::organisation::user_is_admin;
use crate::service::organisation::assert_user_is_admin;
use crate::service::ratings::assert_user_is_application_reviewer_admin;
use axum::extract::{FromRef, FromRequestParts, Path};
use axum::http::request::Parts;
use axum::response::{IntoResponse, Redirect, Response};
Expand Down Expand Up @@ -139,8 +140,50 @@ where
.await
.map_err(|_| ChaosError::BadRequest)?;

user_is_admin(user_id, organisation_id, pool).await?;
assert_user_is_admin(user_id, organisation_id, pool).await?;

Ok(OrganisationAdmin { user_id })
}
}

pub struct ApplicationReviewerAdmin {
pub user_id: i64,
}

#[async_trait]
impl<S> FromRequestParts<S> for ApplicationReviewerAdmin
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;

// 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?;

Ok(ApplicationReviewerAdmin { user_id })
}
}
1 change: 1 addition & 0 deletions backend/server/src/models/ratings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ impl Rating {
rating_id: i64,
transaction: &mut Transaction<'_, Postgres>,
) -> Result<(), ChaosError> {
// TODO: fix sth kavika wanted fixed.
sqlx::query!(
"
DELETE FROM application_ratings WHERE id = $1
Expand Down
1 change: 1 addition & 0 deletions backend/server/src/service/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ pub mod auth;
pub mod jwt;
pub mod oauth2;
pub mod organisation;
pub mod ratings;
2 changes: 1 addition & 1 deletion backend/server/src/service/organisation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use sqlx::{Pool, Postgres, Transaction};
use std::ops::DerefMut;
use uuid::Uuid;

pub async fn user_is_admin(
pub async fn assert_user_is_admin(
user_id: i64,
organisation_id: i64,
pool: &Pool<Postgres>,
Expand Down
47 changes: 47 additions & 0 deletions backend/server/src/service/ratings.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
use crate::models::campaign::Campaign;
use crate::models::error::ChaosError;
use crate::models::organisation::{Member, MemberList, OrganisationDetails, OrganisationRole};
use chrono::{DateTime, Utc};
use snowflake::SnowflakeIdGenerator;
use sqlx::{Pool, Postgres, Transaction};
use std::ops::DerefMut;
use uuid::Uuid;

pub async fn assert_user_is_application_reviewer_admin(
user_id: i64,
rating_id: i64,
pool: &Pool<Postgres>,
) -> Result<(), ChaosError> {
// 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).
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 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(())
}

0 comments on commit 09e41d1

Please sign in to comment.