Skip to content

Commit 81c3446

Browse files
authored
Merge pull request #207 from refactor-group/fix-relationship-data-issue
Fix relationship data issue
2 parents 8514927 + bbe8913 commit 81c3446

File tree

9 files changed

+324
-48
lines changed

9 files changed

+324
-48
lines changed

domain/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,5 +23,8 @@ features = ["debug-print", "runtime-tokio-native-tls", "sqlx-postgres"]
2323
mockito = "1.6"
2424
serial_test = "3.1"
2525

26+
[features]
27+
mock = ["entity_api/mock"]
28+
2629

2730

domain/src/coaching_relationship.rs

Lines changed: 55 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
use crate::coaching_relationships::Model;
2-
use crate::error::Error;
2+
use crate::error::{DomainErrorKind, EntityErrorKind, Error, InternalErrorKind};
33
use entity_api::query::{IntoQueryFilterMap, QuerySort};
44
use entity_api::{coaching_relationships, query};
5-
use sea_orm::DatabaseConnection;
5+
use sea_orm::{DatabaseConnection, TransactionTrait};
66

77
pub use entity_api::coaching_relationship::{
88
create, find_by_id, find_by_organization_with_user_names, find_by_user,
9-
get_relationship_with_user_names, CoachingRelationshipWithUserNames,
9+
find_by_user_and_organization_with_user_names, get_relationship_with_user_names,
10+
CoachingRelationshipWithUserNames,
1011
};
1112

1213
pub async fn find_by<P>(db: &DatabaseConnection, params: P) -> Result<Vec<Model>, Error>
@@ -20,3 +21,54 @@ where
2021
.await?;
2122
Ok(coaching_relationships)
2223
}
24+
25+
/// Finds coaching relationships for a user within an organization, respecting role-based access.
26+
///
27+
/// - SuperAdmins (global role with organization_id = NULL) see all relationships in the organization
28+
/// - Admins (organization-specific role) see all relationships in their organization
29+
/// - Regular users see only relationships where they are the coach or coachee
30+
///
31+
/// This function uses a database transaction to prevent Time-of-Check to Time-of-Use (TOCTOU)
32+
/// vulnerabilities by ensuring the role check and data retrieval happen atomically within
33+
/// a consistent database snapshot.
34+
pub async fn find_by_organization_for_user_with_user_names(
35+
db: &DatabaseConnection,
36+
user_id: crate::Id,
37+
organization_id: crate::Id,
38+
) -> Result<Vec<CoachingRelationshipWithUserNames>, Error> {
39+
// Begin transaction to ensure atomicity and prevent TOCTOU vulnerabilities
40+
let txn = db.begin().await.map_err(|e| Error {
41+
source: Some(Box::new(e)),
42+
error_kind: DomainErrorKind::Internal(InternalErrorKind::Entity(
43+
EntityErrorKind::DbTransaction,
44+
)),
45+
})?;
46+
47+
// Check if user has admin access using entity_api layer
48+
let is_admin = entity_api::user::has_admin_access(&txn, user_id, organization_id)
49+
.await
50+
.map_err(|e| Error {
51+
source: Some(Box::new(e)),
52+
error_kind: DomainErrorKind::Internal(InternalErrorKind::Entity(
53+
EntityErrorKind::DbTransaction,
54+
)),
55+
})?;
56+
57+
let coaching_relationships = if is_admin {
58+
// Admin users see all relationships in the organization
59+
find_by_organization_with_user_names(&txn, organization_id).await?
60+
} else {
61+
// Regular users see only relationships they're associated with (as coach or coachee)
62+
find_by_user_and_organization_with_user_names(&txn, user_id, organization_id).await?
63+
};
64+
65+
// Commit transaction
66+
txn.commit().await.map_err(|e| Error {
67+
source: Some(Box::new(e)),
68+
error_kind: DomainErrorKind::Internal(InternalErrorKind::Entity(
69+
EntityErrorKind::DbTransaction,
70+
)),
71+
})?;
72+
73+
Ok(coaching_relationships)
74+
}

entity_api/src/coaching_relationship.rs

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ pub async fn find_by_organization(
125125
}
126126

127127
pub async fn find_by_organization_with_user_names(
128-
db: &DatabaseConnection,
128+
db: &impl ConnectionTrait,
129129
organization_id: Id,
130130
) -> Result<Vec<CoachingRelationshipWithUserNames>, Error> {
131131
let coaches = Alias::new("coaches");
@@ -159,6 +159,47 @@ pub async fn find_by_organization_with_user_names(
159159
Ok(query.all(db).await?)
160160
}
161161

162+
pub async fn find_by_user_and_organization_with_user_names(
163+
db: &impl ConnectionTrait,
164+
user_id: Id,
165+
organization_id: Id,
166+
) -> Result<Vec<CoachingRelationshipWithUserNames>, Error> {
167+
let coaches = Alias::new("coaches");
168+
let coachees = Alias::new("coachees");
169+
170+
let query = by_organization(coaching_relationships::Entity::find(), organization_id)
171+
.await
172+
.filter(
173+
Condition::any()
174+
.add(coaching_relationships::Column::CoachId.eq(user_id))
175+
.add(coaching_relationships::Column::CoacheeId.eq(user_id)),
176+
)
177+
.join_as(
178+
JoinType::Join,
179+
coaches::Relation::CoachingRelationships.def().rev(),
180+
coaches.clone(),
181+
)
182+
.join_as(
183+
JoinType::Join,
184+
coachees::Relation::CoachingRelationships.def().rev(),
185+
coachees.clone(),
186+
)
187+
.select_only()
188+
.column(coaching_relationships::Column::Id)
189+
.column(coaching_relationships::Column::OrganizationId)
190+
.column(coaching_relationships::Column::CoachId)
191+
.column(coaching_relationships::Column::CoacheeId)
192+
.column(coaching_relationships::Column::CreatedAt)
193+
.column(coaching_relationships::Column::UpdatedAt)
194+
.column_as(Expr::cust("coaches.first_name"), "coach_first_name")
195+
.column_as(Expr::cust("coaches.last_name"), "coach_last_name")
196+
.column_as(Expr::cust("coachees.first_name"), "coachee_first_name")
197+
.column_as(Expr::cust("coachees.last_name"), "coachee_last_name")
198+
.into_model::<CoachingRelationshipWithUserNames>();
199+
200+
Ok(query.all(db).await?)
201+
}
202+
162203
pub async fn get_relationship_with_user_names(
163204
db: &DatabaseConnection,
164205
relationship_id: Id,
@@ -237,7 +278,7 @@ pub async fn delete_by_user_id(db: &impl ConnectionTrait, user_id: Id) -> Result
237278

238279
// A convenient combined struct that holds the results of looking up the Users associated
239280
// with the coach/coachee ids. This should be used as an implementation detail only.
240-
#[derive(FromQueryResult, Debug, PartialEq)]
281+
#[derive(FromQueryResult, Debug, PartialEq, Clone)]
241282
pub struct CoachingRelationshipWithUserNames {
242283
pub id: Id,
243284
pub coach_id: Id,

entity_api/src/user.rs

Lines changed: 195 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@ use entity::users::{ActiveModel, Column, Entity, Model};
77
use entity::{roles, user_roles, Id};
88
use log::*;
99
use password_auth;
10-
use sea_orm::{entity::prelude::*, ConnectionTrait, DatabaseConnection, Set, TransactionTrait};
10+
use sea_orm::{
11+
entity::prelude::*, Condition, ConnectionTrait, DatabaseConnection, Set, TransactionTrait,
12+
};
1113
use serde::Deserialize;
1214
use std::sync::Arc;
1315
use utoipa::{IntoParams, ToSchema};
@@ -127,6 +129,42 @@ pub async fn find_by_organization(
127129
.collect())
128130
}
129131

132+
/// Checks if a user has admin privileges for an organization.
133+
///
134+
/// Returns `true` if the user is:
135+
/// - A SuperAdmin (has `SuperAdmin` role with `organization_id = NULL`), OR
136+
/// - An Admin for the specific organization (has `Admin` role for the given organization)
137+
///
138+
/// This function encapsulates the SeaORM query logic for role checking,
139+
/// keeping database-specific implementation details out of the domain layer.
140+
pub async fn has_admin_access(
141+
db: &impl ConnectionTrait,
142+
user_id: Id,
143+
organization_id: Id,
144+
) -> Result<bool, Error> {
145+
let admin_role = user_roles::Entity::find()
146+
.filter(user_roles::Column::UserId.eq(user_id))
147+
.filter(
148+
Condition::any()
149+
// SuperAdmin with organization_id = NULL
150+
.add(
151+
Condition::all()
152+
.add(user_roles::Column::Role.eq(Role::SuperAdmin))
153+
.add(user_roles::Column::OrganizationId.is_null()),
154+
)
155+
// Admin for this specific organization
156+
.add(
157+
Condition::all()
158+
.add(user_roles::Column::Role.eq(Role::Admin))
159+
.add(user_roles::Column::OrganizationId.eq(organization_id)),
160+
),
161+
)
162+
.one(db)
163+
.await?;
164+
165+
Ok(admin_role.is_some())
166+
}
167+
130168
pub async fn delete(db: &impl ConnectionTrait, user_id: Id) -> Result<(), Error> {
131169
Entity::delete_by_id(user_id).exec(db).await?;
132170
Ok(())
@@ -382,4 +420,160 @@ mod test {
382420

383421
Ok(())
384422
}
423+
424+
#[tokio::test]
425+
async fn has_admin_access_returns_true_for_super_admin() -> Result<(), Error> {
426+
let user_id = Id::new_v4();
427+
let organization_id = Id::new_v4();
428+
let role_id = Id::new_v4();
429+
let now = chrono::Utc::now();
430+
431+
// Create a SuperAdmin role with NULL organization_id
432+
let super_admin_role = user_roles::Model {
433+
id: role_id,
434+
user_id,
435+
role: Role::SuperAdmin,
436+
organization_id: None,
437+
created_at: now.into(),
438+
updated_at: now.into(),
439+
};
440+
441+
let db = MockDatabase::new(DatabaseBackend::Postgres)
442+
.append_query_results(vec![vec![super_admin_role]])
443+
.into_connection();
444+
445+
let result = has_admin_access(&db, user_id, organization_id).await?;
446+
447+
assert!(
448+
result,
449+
"SuperAdmin should have admin access to any organization"
450+
);
451+
452+
Ok(())
453+
}
454+
455+
#[tokio::test]
456+
async fn has_admin_access_returns_true_for_organization_admin() -> Result<(), Error> {
457+
let user_id = Id::new_v4();
458+
let organization_id = Id::new_v4();
459+
let role_id = Id::new_v4();
460+
let now = chrono::Utc::now();
461+
462+
// Create an Admin role for the specific organization
463+
let org_admin_role = user_roles::Model {
464+
id: role_id,
465+
user_id,
466+
role: Role::Admin,
467+
organization_id: Some(organization_id),
468+
created_at: now.into(),
469+
updated_at: now.into(),
470+
};
471+
472+
let db = MockDatabase::new(DatabaseBackend::Postgres)
473+
.append_query_results(vec![vec![org_admin_role]])
474+
.into_connection();
475+
476+
let result = has_admin_access(&db, user_id, organization_id).await?;
477+
478+
assert!(
479+
result,
480+
"Organization Admin should have admin access to their organization"
481+
);
482+
483+
Ok(())
484+
}
485+
486+
#[tokio::test]
487+
async fn has_admin_access_returns_false_for_regular_user() -> Result<(), Error> {
488+
let user_id = Id::new_v4();
489+
let organization_id = Id::new_v4();
490+
491+
// Mock returns empty result (no admin roles found)
492+
let db = MockDatabase::new(DatabaseBackend::Postgres)
493+
.append_query_results(vec![Vec::<user_roles::Model>::new()])
494+
.into_connection();
495+
496+
let result = has_admin_access(&db, user_id, organization_id).await?;
497+
498+
assert!(!result, "Regular users should not have admin access");
499+
500+
Ok(())
501+
}
502+
503+
#[tokio::test]
504+
async fn has_admin_access_returns_false_for_admin_of_different_organization(
505+
) -> Result<(), Error> {
506+
let user_id = Id::new_v4();
507+
let organization_id_a = Id::new_v4(); // Organization being queried
508+
let _organization_id_b = Id::new_v4(); // Organization where user is admin
509+
510+
// Mock returns empty result (no matching admin role for org A)
511+
let db = MockDatabase::new(DatabaseBackend::Postgres)
512+
.append_query_results(vec![Vec::<user_roles::Model>::new()])
513+
.into_connection();
514+
515+
let result = has_admin_access(&db, user_id, organization_id_a).await?;
516+
517+
assert!(
518+
!result,
519+
"Admin of different organization should not have access"
520+
);
521+
522+
Ok(())
523+
}
524+
525+
#[tokio::test]
526+
async fn has_admin_access_returns_false_for_nonexistent_user() -> Result<(), Error> {
527+
let nonexistent_user_id = Id::new_v4();
528+
let organization_id = Id::new_v4();
529+
530+
// Mock returns empty result (user doesn't exist)
531+
let db = MockDatabase::new(DatabaseBackend::Postgres)
532+
.append_query_results(vec![Vec::<user_roles::Model>::new()])
533+
.into_connection();
534+
535+
let result = has_admin_access(&db, nonexistent_user_id, organization_id).await?;
536+
537+
assert!(!result, "Nonexistent user should not have admin access");
538+
539+
Ok(())
540+
}
541+
542+
#[tokio::test]
543+
async fn has_admin_access_with_admin_role_for_multiple_organizations() -> Result<(), Error> {
544+
let user_id = Id::new_v4();
545+
let organization_id_a = Id::new_v4();
546+
let organization_id_b = Id::new_v4();
547+
let role_id = Id::new_v4();
548+
let now = chrono::Utc::now();
549+
550+
// Create an Admin role for organization A
551+
let org_admin_role = user_roles::Model {
552+
id: role_id,
553+
user_id,
554+
role: Role::Admin,
555+
organization_id: Some(organization_id_a),
556+
created_at: now.into(),
557+
updated_at: now.into(),
558+
};
559+
560+
let db = MockDatabase::new(DatabaseBackend::Postgres)
561+
.append_query_results(vec![vec![org_admin_role]])
562+
.into_connection();
563+
564+
// Should have access to organization A
565+
let result_a = has_admin_access(&db, user_id, organization_id_a).await?;
566+
assert!(result_a, "Should have admin access to organization A");
567+
568+
// Create new mock for organization B query (no matching role)
569+
let db_b = MockDatabase::new(DatabaseBackend::Postgres)
570+
.append_query_results(vec![Vec::<user_roles::Model>::new()])
571+
.into_connection();
572+
573+
// Should NOT have access to organization B
574+
let result_b = has_admin_access(&db_b, user_id, organization_id_b).await?;
575+
assert!(!result_b, "Should not have admin access to organization B");
576+
577+
Ok(())
578+
}
385579
}

web/src/controller/organization/coaching_relationship_controller.rs

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -115,16 +115,22 @@ pub async fn read(
115115
)]
116116
pub async fn index(
117117
CompareApiVersion(_v): CompareApiVersion,
118-
AuthenticatedUser(_user): AuthenticatedUser,
118+
AuthenticatedUser(user): AuthenticatedUser,
119119
State(app_state): State<AppState>,
120120
Path(organization_id): Path<Id>,
121121
) -> Result<impl IntoResponse, Error> {
122-
debug!("GET all CoachingRelationships");
123-
let coaching_relationships = CoachingRelationshipApi::find_by_organization_with_user_names(
124-
app_state.db_conn_ref(),
125-
organization_id,
126-
)
127-
.await?;
122+
debug!(
123+
"GET all CoachingRelationships for user {} in organization {}",
124+
user.id, organization_id
125+
);
126+
127+
let coaching_relationships =
128+
CoachingRelationshipApi::find_by_organization_for_user_with_user_names(
129+
app_state.db_conn_ref(),
130+
user.id,
131+
organization_id,
132+
)
133+
.await?;
128134

129135
debug!("Found CoachingRelationships: {coaching_relationships:?}");
130136

0 commit comments

Comments
 (0)