Add authorization and validation checks to coaching relationship crea… (#157)

* add authorization and validation checks to coaching relationship creation

* update failing test

* generate coaching relationship slugs from user first_names

* Update entity_api/src/coaching_relationship.rs

Co-authored-by: Jim Hodapp <[email protected]>

* set organization_id from path for creating Coaching Relationships

---------

Co-authored-by: Jim Hodapp <[email protected]>

Author: calebbourg

domain/src/user.rs

@@ -106,14 +106,19 @@ pub async fn create_user_and_coaching_relationship(
     let new_coaching_relationship_model = entity_api::coaching_relationships::Model {
         coachee_id: new_user.id,
         coach_id,
-        organization_id,
         // These will be overridden
+        organization_id: Default::default(),
         id: Default::default(),
         slug: "".to_string(),
         created_at: Utc::now().into(),
         updated_at: Utc::now().into(),
     };
-    entity_api::coaching_relationship::create(&txn, new_coaching_relationship_model).await?;
+    entity_api::coaching_relationship::create(
+        &txn,
+        organization_id,
+        new_coaching_relationship_model,
+    )
+    .await?;
     // This is not probably the type of error we'll ultimately be exposing. Again just temporary (hopfully
     txn.commit().await.map_err(|e| Error {
         source: Some(Box::new(e)),

entity/src/coaching_relationships.rs

@@ -15,6 +15,7 @@ pub struct Model {
     #[serde(skip_deserializing)]
     #[sea_orm(primary_key)]
     pub id: Id,
+    #[serde(skip_deserializing)]
     #[sea_orm(unique)]
     pub organization_id: Id,
     pub coach_id: Id,

entity_api/src/coaching_relationship.rs

@@ -1,4 +1,7 @@
-use super::error::{EntityApiErrorKind, Error};
+use super::{
+    error::{EntityApiErrorKind, Error},
+    organization,
+};
 use crate::user;
 use chrono::Utc;
 use entity::{
@@ -16,17 +19,42 @@ use slugify::slugify;
 
 pub async fn create(
     db: &impl ConnectionTrait,
+    organization_id: Id,
     coaching_relationship_model: Model,
-) -> Result<Model, Error> {
+) -> Result<CoachingRelationshipWithUserNames, Error> {
     debug!(
         "New Coaching Relationship Model to be inserted: {:?}",
         coaching_relationship_model
     );
 
+    let coach = user::find_by_id(db, coaching_relationship_model.coach_id).await?;
+    let coachee = user::find_by_id(db, coaching_relationship_model.coachee_id).await?;
+
+    let coach_organization_ids = organization::find_by_user(db, coach.id)
+        .await?
+        .iter()
+        .map(|org| org.id)
+        .collect::<Vec<Id>>();
+    let coachee_organization_ids = organization::find_by_user(db, coachee.id)
+        .await?
+        .iter()
+        .map(|org| org.id)
+        .collect::<Vec<Id>>();
+
+    // Check that the coach and coachee belong to the correct organization
+    if !coach_organization_ids.contains(&organization_id)
+        || !coachee_organization_ids.contains(&organization_id)
+    {
+        error!("Coach and coachee do not belong to the correct organization, not creating requested new coaching relationship between coach: {:?} and coachee: {:?} for organization: {:?}.", coaching_relationship_model.coach_id, coaching_relationship_model.coachee_id, organization_id);
+        return Err(Error {
+            source: None,
+            error_kind: EntityApiErrorKind::ValidationError,
+        });
+    }
+
     // Coaching Relationship must be unique within the context of an organization
     // Note: this is enforced at the database level as well
-    let existing_coaching_relationships =
-        find_by_organization(db, coaching_relationship_model.organization_id).await?;
+    let existing_coaching_relationships = find_by_organization(db, organization_id).await?;
     let existing_coaching_relationship = existing_coaching_relationships.iter().find(|cr| {
         cr.coach_id == coaching_relationship_model.coach_id
             && cr.coachee_id == coaching_relationship_model.coachee_id
@@ -46,15 +74,27 @@ pub async fn create(
     let slug = slugify!(format!("{} {}", coach.first_name, coachee.first_name).as_str());
 
     let coaching_relationship_active_model: ActiveModel = ActiveModel {
-        organization_id: Set(coaching_relationship_model.organization_id),
+        organization_id: Set(organization_id),
         coach_id: Set(coaching_relationship_model.coach_id),
         coachee_id: Set(coaching_relationship_model.coachee_id),
         slug: Set(slug),
         created_at: Set(now.into()),
         updated_at: Set(now.into()),
         ..Default::default()
     };
-    Ok(coaching_relationship_active_model.insert(db).await?)
+    let inserted: Model = coaching_relationship_active_model.insert(db).await?;
+
+    Ok(CoachingRelationshipWithUserNames {
+        id: inserted.id,
+        coach_id: inserted.coach_id,
+        coachee_id: inserted.coachee_id,
+        coach_first_name: coach.first_name,
+        coach_last_name: coach.last_name,
+        coachee_first_name: coachee.first_name,
+        coachee_last_name: coachee.last_name,
+        created_at: inserted.created_at,
+        updated_at: inserted.updated_at,
+    })
 }
 
 pub async fn find_by_id(db: &DatabaseConnection, id: Id) -> Result<Model, Error> {
@@ -206,7 +246,7 @@ pub async fn delete_by_user_id(db: &impl ConnectionTrait, user_id: Id) -> Result
 
 // A convenient combined struct that holds the results of looking up the Users associated
 // with the coach/coachee ids. This should be used as an implementation detail only.
-#[derive(FromQueryResult, Debug)]
+#[derive(FromQueryResult, Debug, PartialEq)]
 pub struct CoachingRelationshipWithUserNames {
     pub id: Id,
     pub coach_id: Id,
@@ -337,26 +377,76 @@ mod tests {
     #[tokio::test]
     async fn create_returns_validation_error_for_duplicate_relationship() -> Result<(), Error> {
         use entity::coaching_relationships::Model;
-        use sea_orm::{DatabaseBackend, MockDatabase, MockExecResult};
+        use sea_orm::{DatabaseBackend, MockDatabase};
 
         let organization_id = Id::new_v4();
         let coach_id = Id::new_v4();
         let coachee_id = Id::new_v4();
+        let coach_organization_id = Id::new_v4();
+        let coachee_organization_id = Id::new_v4();
+
+        let coach_user = entity::users::Model {
+            id: coach_id.clone(),
+            first_name: "Coach".to_string(),
+            last_name: "User".to_string(),
+            email: "[email protected]".to_string(),
+            password: "hash".to_string(),
+            display_name: Some("Coach User".to_string()),
+            github_username: Some("coach_user".to_string()),
+            role: entity::users::Role::User,
+            github_profile_url: Some("https://github.com/coach_user".to_string()),
+            created_at: chrono::Utc::now().into(),
+            updated_at: chrono::Utc::now().into(),
+        };
+
+        let coachee_user = entity::users::Model {
+            id: coachee_id.clone(),
+            first_name: "Coachee".to_string(),
+            last_name: "User".to_string(),
+            email: "[email protected]".to_string(),
+            password: "hash".to_string(),
+            display_name: Some("Coachee User".to_string()),
+            github_username: Some("coachee_user".to_string()),
+            role: entity::users::Role::User,
+            github_profile_url: Some("https://github.com/coachee_user".to_string()),
+            created_at: chrono::Utc::now().into(),
+            updated_at: chrono::Utc::now().into(),
+        };
+
+        let coaching_relationships = vec![Model {
+            id: Id::new_v4(),
+            organization_id: organization_id.clone(),
+            coach_id: coach_id.clone(),
+            coachee_id: coachee_id.clone(),
+            slug: "coach-coachee".to_string(),
+            created_at: chrono::Utc::now().into(),
+            updated_at: chrono::Utc::now().into(),
+        }];
+
+        let coach_organization = entity::organizations::Model {
+            id: coach_organization_id,
+            name: "Organization".to_string(),
+            slug: "organization".to_string(),
+            logo: None,
+            created_at: chrono::Utc::now().into(),
+            updated_at: chrono::Utc::now().into(),
+        };
+
+        let coachee_organization = entity::organizations::Model {
+            id: coachee_organization_id,
+            name: "Organization".to_string(),
+            slug: "organization".to_string(),
+            logo: None,
+            created_at: chrono::Utc::now().into(),
+            updated_at: chrono::Utc::now().into(),
+        };
 
         let db = MockDatabase::new(DatabaseBackend::Postgres)
-            .append_query_results(vec![vec![Model {
-                id: Id::new_v4(),
-                organization_id: organization_id.clone(),
-                coach_id: coach_id.clone(),
-                coachee_id: coachee_id.clone(),
-                slug: "coach-coachee".to_string(),
-                created_at: chrono::Utc::now().into(),
-                updated_at: chrono::Utc::now().into(),
-            }]])
-            .append_exec_results(vec![MockExecResult {
-                last_insert_id: 0,
-                rows_affected: 1,
-            }])
+            .append_query_results(vec![vec![coach_user]])
+            .append_query_results(vec![vec![coachee_user]])
+            .append_query_results(vec![vec![coach_organization]])
+            .append_query_results(vec![vec![coachee_organization]])
+            .append_query_results(vec![coaching_relationships])
             .into_connection();
 
         let model = Model {
@@ -369,7 +459,8 @@ mod tests {
             updated_at: chrono::Utc::now().into(),
         };
 
-        let result = create(&db, model).await;
+        let result = create(&db, organization_id, model).await;
+        println!("Result: {:?}", result);
         assert!(
             result
                 == Err(Error {

entity_api/src/organization.rs

@@ -3,15 +3,15 @@ use crate::{organization::Entity, uuid_parse_str};
 use chrono::Utc;
 use entity::{organizations::*, organizations_users, prelude::Organizations, Id};
 use sea_orm::{
-    entity::prelude::*, ActiveValue::Set, ActiveValue::Unchanged, DatabaseConnection, JoinType,
+    entity::prelude::*, ActiveValue::Set, ActiveValue::Unchanged, ConnectionTrait, JoinType,
     QuerySelect, TryIntoModel,
 };
 use slugify::slugify;
 use std::collections::HashMap;
 
 use log::*;
 
-pub async fn create(db: &DatabaseConnection, organization_model: Model) -> Result<Model, Error> {
+pub async fn create(db: &impl ConnectionTrait, organization_model: Model) -> Result<Model, Error> {
     debug!(
         "New Organization Model to be inserted: {:?}",
         organization_model
@@ -32,7 +32,7 @@ pub async fn create(db: &DatabaseConnection, organization_model: Model) -> Resul
     Ok(organization_active_model.insert(db).await?)
 }
 
-pub async fn update(db: &DatabaseConnection, id: Id, model: Model) -> Result<Model, Error> {
+pub async fn update(db: &impl ConnectionTrait, id: Id, model: Model) -> Result<Model, Error> {
     let organization = find_by_id(db, id).await?;
 
     let active_model: ActiveModel = ActiveModel {
@@ -46,13 +46,13 @@ pub async fn update(db: &DatabaseConnection, id: Id, model: Model) -> Result<Mod
     Ok(active_model.update(db).await?.try_into_model()?)
 }
 
-pub async fn delete_by_id(db: &DatabaseConnection, id: Id) -> Result<(), Error> {
+pub async fn delete_by_id(db: &impl ConnectionTrait, id: Id) -> Result<(), Error> {
     let organization_model = find_by_id(db, id).await?;
     organization_model.delete(db).await?;
     Ok(())
 }
 
-pub async fn find_all(db: &DatabaseConnection) -> Result<Vec<Model>, Error> {
+pub async fn find_all(db: &impl ConnectionTrait) -> Result<Vec<Model>, Error> {
     Ok(Entity::find().all(db).await?)
 }
 
@@ -64,7 +64,7 @@ pub async fn find_by_id(db: &impl ConnectionTrait, id: Id) -> Result<Model, Erro
 }
 
 pub async fn find_by(
-    db: &DatabaseConnection,
+    db: &impl ConnectionTrait,
     params: HashMap<String, String>,
 ) -> Result<Vec<Model>, Error> {
     let mut query = Entity::find();
@@ -87,7 +87,7 @@ pub async fn find_by(
     Ok(query.distinct().all(db).await?)
 }
 
-pub async fn find_by_user(db: &DatabaseConnection, user_id: Id) -> Result<Vec<Model>, Error> {
+pub async fn find_by_user(db: &impl ConnectionTrait, user_id: Id) -> Result<Vec<Model>, Error> {
     let organizations = by_user(Entity::find(), user_id).await.all(db).await?;
 
     Ok(organizations)

migration/src/refactor_platform_rs.sql

@@ -1,6 +1,6 @@
 -- SQL dump generated using DBML (dbml.dbdiagram.io)
 -- Database: PostgreSQL
--- Generated at: 2025-06-11T12:15:26.648Z
+-- Generated at: 2025-06-14T12:21:13.185Z
 
 
 CREATE TYPE "refactor_platform"."status" AS ENUM (

web/src/controller/organization/coaching_relationship_controller.rs

@@ -33,16 +33,21 @@ use log::*;
 pub async fn create(
     CompareApiVersion(_v): CompareApiVersion,
     State(app_state): State<AppState>,
+    Path(organization_id): Path<Id>,
+    AuthenticatedUser(_user): AuthenticatedUser,
     Json(coaching_relationship_model): Json<coaching_relationships::Model>,
 ) -> Result<impl IntoResponse, Error> {
     debug!(
         "CREATE new Coaching Relationship from: {:?}",
         coaching_relationship_model
     );
 
-    let coaching_relationship: coaching_relationships::Model =
-        CoachingRelationshipApi::create(app_state.db_conn_ref(), coaching_relationship_model)
-            .await?;
+    let coaching_relationship: CoachingRelationshipWithUserNames = CoachingRelationshipApi::create(
+        app_state.db_conn_ref(),
+        organization_id,
+        coaching_relationship_model,
+    )
+    .await?;
 
     debug!(
         "Newly created Coaching Relationship: {:?}",

web/src/controller/organization/user_controller.rs

@@ -63,17 +63,13 @@ pub async fn index(
 pub(crate) async fn create(
     CompareApiVersion(_v): CompareApiVersion,
     State(app_state): State<AppState>,
-    AuthenticatedUser(authenticated_user): AuthenticatedUser,
+    AuthenticatedUser(_authenticated_user): AuthenticatedUser,
     Path(organization_id): Path<Id>,
     Json(user_model): Json<users::Model>,
 ) -> Result<impl IntoResponse, Error> {
-    let user = UserApi::create_user_and_coaching_relationship(
-        app_state.db_conn_ref(),
-        organization_id,
-        authenticated_user.id,
-        user_model,
-    )
-    .await?;
+    let user =
+        UserApi::create_by_organization(app_state.db_conn_ref(), organization_id, user_model)
+            .await?;
     info!("User created: {:?}", user);
     Ok(Json(ApiResponse::new(StatusCode::CREATED.into(), user)))
 }

web/src/protect/organizations/coaching_relationships.rs

@@ -0,0 +1,27 @@
+use crate::protect::{Predicate, UserInOrganization, UserIsAdmin};
+use crate::{extractors::authenticated_user::AuthenticatedUser, AppState};
+use axum::{
+    extract::{Path, Request, State},
+    middleware::Next,
+    response::IntoResponse,
+};
+
+use domain::Id;
+
+/// Checks that the authenticated user is associated with the organization specified by `organization_id`
+/// and that the authenticated user is an admin
+/// Intended to be given to axum::middleware::from_fn_with_state in the router
+pub(crate) async fn create(
+    State(app_state): State<AppState>,
+    AuthenticatedUser(authenticated_user): AuthenticatedUser,
+    Path(organization_id): Path<Id>,
+    request: Request,
+    next: Next,
+) -> impl IntoResponse {
+    let checks: Vec<Predicate> = vec![
+        Predicate::new(UserInOrganization, vec![organization_id]),
+        Predicate::new(UserIsAdmin, vec![]),
+    ];
+
+    crate::protect::authorize(&app_state, authenticated_user, request, next, checks).await
+}

web/src/protect/organizations/mod.rs

@@ -1 +1,2 @@
+pub(crate) mod coaching_relationships;
 pub(crate) mod users;

web/src/router.rs

@@ -242,6 +242,10 @@ fn organization_coaching_relationship_routes(app_state: AppState) -> Router {
             "/organizations/:organization_id/coaching_relationships",
             post(coaching_relationship_controller::create),
         )
+        .route_layer(from_fn_with_state(
+            app_state.clone(),
+            protect::organizations::coaching_relationships::create,
+        ))
         .merge(
             // GET /organizations/:organization_id/coaching_relationships
             Router::new()