diff --git a/editoast/src/authentication.rs b/editoast/src/authentication.rs index fbad20da5be..e9ae6e7d739 100644 --- a/editoast/src/authentication.rs +++ b/editoast/src/authentication.rs @@ -18,9 +18,7 @@ pub enum Authentication { impersonated_identity: String, }, Skip { - #[expect(unused)] identity: Option, - #[expect(unused)] name: Option, }, } @@ -75,16 +73,4 @@ impl Authentication { }; Ok(authn) } - - pub fn origin(&self) -> Option<(&str, &str)> { - match self { - Authentication::Authenticated { identity, name } - | Authentication::Impersonating { - impersonator_identity: identity, - impersonator_name: name, - impersonated_identity: _, - } => Some((identity.as_str(), name.as_str())), - Authentication::Unauthenticated | Authentication::Skip { .. } => None, - } - } } diff --git a/editoast/src/views/authz.rs b/editoast/src/views/authz.rs index 17925ff2823..79a902a2356 100644 --- a/editoast/src/views/authz.rs +++ b/editoast/src/views/authz.rs @@ -1379,6 +1379,66 @@ mod tests { ); } + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + async fn whoami_impersonation() { + let app = test_app!().enable_authorization(true).build(); + let impersonator = app + .user("impersonator", "Impersonator") + .with_roles([Role::Admin]) + .create() + .await; + let impersonated = app + .user("impersonated", "Impersonated") + .with_roles([Role::Stdcm]) + .create() + .await; + + let request = app + .get("/authz/me") + .by_user(&impersonator) + .impersonate(&impersonated); + let user_data = app + .fetch(request) + .await + .assert_status(StatusCode::OK) + .json_into::(); + + assert_eq!( + user_data, + WhoamiResponse { + id: impersonated.id, + name: "Impersonated".to_string(), + roles: vec![Role::Stdcm], + } + ); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + async fn whoami_skip_with_user_info() { + let app = test_app!().enable_authorization(true).build(); + let user = app + .user("bob", "Bob") + .with_roles([Role::Admin]) + .create() + .await; + + let request = app.get("/authz/me").by_user(&user).skip_authz(); + let user_data = app + .fetch(request) + .await + .assert_status(StatusCode::OK) + .json_into::(); + + assert_eq!( + user_data, + WhoamiResponse { + id: user.id, + name: "Bob".to_string(), + roles: vec![Role::Admin], + } + ); + } + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn whoami_authorization_disabled() { let app = test_app!() diff --git a/editoast/src/views/server/middlewares.rs b/editoast/src/views/server/middlewares.rs index 2ff1c23a755..78063001fb9 100644 --- a/editoast/src/views/server/middlewares.rs +++ b/editoast/src/views/server/middlewares.rs @@ -1,3 +1,5 @@ +use std::convert::Infallible; + use authz::Authorizer; use authz::Role; use authz::StorageDriver as _; @@ -95,30 +97,29 @@ pub(in crate::views) async fn authentication_validation_middleware( mut req: Request, next: Next, ) -> Result { - let is_impersonation = matches!( - authn, - crate::authentication::Authentication::Impersonating { .. } - ); - - fn origin_user( - user: Option, + fn warn_mismatched_name( + user: &Option, identity: &str, header_name: &str, - ) -> Option<(editoast_models::User, ::authz::v2::Protected>)> { - user.inspect(|user| { - if user.name != header_name { - tracing::warn!( - identity, - header = header_name, - stored = user.name, - "provided name for identity differ from stored", - ); - } - }) - .map(|user| { - let authz_user = ::authz::Subject::user(user.id); - (user, ::authz::v2::subject_roles(authz_user)) - }) + ) { + if user.as_ref().is_some_and(|u| u.name != header_name) { + tracing::warn!( + identity, + header = header_name, + stored = user.as_ref().map(|u| u.name.clone()), + "provided name for identity differ from stored", + ); + } + } + + fn zip_roles( + user: editoast_models::User, + ) -> ( + Option, + ::authz::v2::Protected>, + ) { + let subject = ::authz::Subject::user(user.id); + (Some(user), ::authz::v2::subject_roles(subject)) } async fn register_origin_user( @@ -139,7 +140,7 @@ pub(in crate::views) async fn authentication_validation_middleware( Ok(user) => user, Err(AddIdentitiesError::DuplicateIdentity(_)) => { unreachable!( - "the current function is only called when the user don't exists, and the `.register()`\n\ + "the current function is only called when the user doesn't exists, and the `.register()`\n\ operation above only creates the user with one unique identity" ); } @@ -149,54 +150,83 @@ pub(in crate::views) async fn authentication_validation_middleware( Ok((Some(user), ::authz::v2::subject_roles(authz_user))) } - let (user, roles_prot) = if let Some(req_origin) = authn.origin() { - let conn = db_pool.get().await?; - conn.transaction(async |conn| { - let origin = match &authn { - crate::authentication::Authentication::Authenticated { identity, name } => { - let user = - editoast_models::User::retrieve_by_identity(identity, conn.clone()).await?; - origin_user(user, identity, name) + async fn check_impersonation_privilege( + openfga: &fga::Client, + user: &editoast_models::User, + ) -> Result<()> { + let Ok(roles) = ::authz::v2::subject_roles(::authz::Subject::user(user.id)) + .access_authorized::(openfga) + .access() + .await?; + if roles.contains(&Role::Admin) { + Ok(()) + } else { + Err(AuthorizationError::ForbiddenImpersonation.into()) + } + } + + let (user, roles_prot) = match &authn { + crate::authentication::Authentication::Authenticated { identity, name } + | crate::authentication::Authentication::Skip { + identity: Some(identity), + name: Some(name), + } => { + let conn = db_pool.get().await?; + conn.transaction(async |conn| { + let user = + editoast_models::User::retrieve_by_identity(identity, conn.clone()).await?; + warn_mismatched_name(&user, identity, name); + Ok::<_, crate::error::InternalError>(if let Some(user) = user { + zip_roles(user) + } else { + register_origin_user(conn.clone(), (identity, name)).await? + }) + }) + .await? + } + crate::authentication::Authentication::Impersonating { + impersonator_identity, + impersonator_name, + impersonated_identity, + } => { + let conn = db_pool.get().await?; + conn.transaction(async |conn| { + let (impersonator, impersonated) = tokio::try_join!( + // The batching API is annoying, that's the best I can do concisely for now. We should + // work on the DB user management that got worse since we added multiple identities support. + editoast_models::User::retrieve_by_identity( + impersonator_identity, + conn.clone() + ), + editoast_models::User::retrieve_by_identity( + impersonated_identity, + conn.clone() + ) + )?; + warn_mismatched_name(&impersonator, impersonator_identity, impersonator_name); + if let Some(impersonator) = impersonator { + check_impersonation_privilege(regulator.openfga(), &impersonator).await?; + } else { + register_origin_user(conn.clone(), (impersonator_identity, impersonator_name)) + .await?; } - crate::authentication::Authentication::Impersonating { - impersonator_identity, - impersonator_name, - impersonated_identity, - } => { - let (impersonator, impersonated) = tokio::try_join!( - // The batching API is annoying, that's the best I can do concisely for now. We should - // work on the DB user management that got worse since we added multiple identities support. - editoast_models::User::retrieve_by_identity( - impersonator_identity, - conn.clone() - ), - editoast_models::User::retrieve_by_identity( - impersonated_identity, - conn.clone() - ) - )?; - if impersonated.is_none() { - return Err::<_, crate::error::InternalError>( - AuthorizationError::ImpersonatedUserNotFound { - identity: impersonated_identity.to_owned(), - } - .into(), - ); - } - origin_user(impersonator, impersonator_identity, impersonator_name) + if let Some(impersonated) = impersonated { + Ok(zip_roles(impersonated)) + } else { + Err::<_, crate::error::InternalError>( + AuthorizationError::ImpersonatedUserNotFound { + identity: impersonated_identity.to_owned(), + } + .into(), + ) } - crate::authentication::Authentication::Unauthenticated - | crate::authentication::Authentication::Skip { .. } => None, - }; - - Ok(match origin { - Some((user, roles_prot)) => (Some(user), roles_prot), - None => register_origin_user(conn, req_origin).await?, }) - }) - .await? - } else { - (None, ::authz::v2::Protected::default()) + .await? + } + crate::authentication::Authentication::Unauthenticated + | crate::authentication::Authentication::Skip { .. } => { + (None, ::authz::v2::Protected::default()) + } }; // A failed OpenFGA request does not invalidate the creation of a new user @@ -206,14 +236,6 @@ pub(in crate::views) async fn authentication_validation_middleware( .await .map_err(AuthorizationError::from)?; - if is_impersonation { - if !roles.contains(&Role::Admin) { - return Err(AuthorizationError::ForbiddenImpersonation.into()); - } else { - tracing::info!("impersonation enabled"); - } - } - let span = tracing::Span::current(); if let Some(user) = &user { span.record("user.id", user.id); diff --git a/editoast/src/views/test_app.rs b/editoast/src/views/test_app.rs index 9885459652a..b6c3c9edcbf 100644 --- a/editoast/src/views/test_app.rs +++ b/editoast/src/views/test_app.rs @@ -628,12 +628,13 @@ impl<'a> GroupBuilder<'a> { } pub trait TestRequestExt { - fn by_user(self, user: &impl AsRef) -> Self; + fn by_user(self, user: impl AsRef) -> Self; + fn impersonate(self, impersonated: impl AsRef) -> Self; fn skip_authz(self) -> Self; } impl TestRequestExt for TestRequest { - fn by_user(self, user: &impl AsRef) -> Self { + fn by_user(self, user: impl AsRef) -> Self { let UserInfo { identities, name } = user.as_ref(); self.add_header( "x-remote-user-identity", @@ -642,6 +643,17 @@ impl TestRequestExt for TestRequest { .add_header("x-remote-user-name", name) } + fn impersonate(self, impersonated: impl AsRef) -> Self { + self.add_header( + "x-impersonate", + impersonated + .as_ref() + .identities + .first() + .expect("existing user must have an identity"), + ) + } + fn skip_authz(self) -> Self { self.add_header("x-osrd-skip-authz", "true") }