From ff698df2807aa4b846886ed5181ec963070d6626 Mon Sep 17 00:00:00 2001 From: Valentin Tolmer Date: Mon, 6 Jun 2022 16:48:22 +0200 Subject: [PATCH] server: Introduce a read-only user --- server/src/infra/auth_service.rs | 49 +++++++-- server/src/infra/graphql/mutation.rs | 16 +-- server/src/infra/graphql/query.rs | 10 +- server/src/infra/ldap_handler.rs | 153 ++++++++++++++++----------- server/src/main.rs | 15 ++- 5 files changed, 161 insertions(+), 82 deletions(-) diff --git a/server/src/infra/auth_service.rs b/server/src/infra/auth_service.rs index 67def73..149456c 100644 --- a/server/src/infra/auth_service.rs +++ b/server/src/infra/auth_service.rs @@ -438,7 +438,11 @@ where } .into_inner(); let user_id = ®istration_start_request.username; - validation_result.can_access(user_id); + if !validation_result.can_write(user_id) { + return ApiResult::Right( + HttpResponse::Unauthorized().body("Not authorized to change the user's password"), + ); + } data.backend_handler .registration_start(registration_start_request) .await @@ -519,9 +523,16 @@ where } } +#[derive(Clone, Copy, PartialEq, Debug)] +pub enum Permission { + Admin, + Readonly, + Regular, +} + pub struct ValidationResults { pub user: String, - pub is_admin: bool, + pub permission: Permission, } impl ValidationResults { @@ -529,12 +540,30 @@ impl ValidationResults { pub fn admin() -> Self { Self { user: "admin".to_string(), - is_admin: true, + permission: Permission::Admin, } } - pub fn can_access(&self, user: &str) -> bool { - self.is_admin || self.user == user + #[must_use] + pub fn is_admin(&self) -> bool { + self.permission == Permission::Admin + } + + #[must_use] + pub fn is_admin_or_readonly(&self) -> bool { + self.permission == Permission::Admin || self.permission == Permission::Readonly + } + + #[must_use] + pub fn can_read(&self, user: &str) -> bool { + self.permission == Permission::Admin + || self.permission == Permission::Readonly + || self.user == user + } + + #[must_use] + pub fn can_write(&self, user: &str) -> bool { + self.permission == Permission::Admin || self.user == user } } @@ -561,10 +590,16 @@ pub(crate) fn check_if_token_is_valid( if state.jwt_blacklist.read().unwrap().contains(&jwt_hash) { return Err(ErrorUnauthorized("JWT was logged out")); } - let is_admin = token.claims().groups.contains("lldap_admin"); + let is_in_group = |name| token.claims().groups.contains(name); Ok(ValidationResults { user: token.claims().user.clone(), - is_admin, + permission: if is_in_group("lldap_admin") { + Permission::Admin + } else if is_in_group("lldap_readonly") { + Permission::Readonly + } else { + Permission::Regular + }, }) } diff --git a/server/src/infra/graphql/mutation.rs b/server/src/infra/graphql/mutation.rs index e2c1557..94c6165 100644 --- a/server/src/infra/graphql/mutation.rs +++ b/server/src/infra/graphql/mutation.rs @@ -63,7 +63,7 @@ impl Mutation { context: &Context, user: CreateUserInput, ) -> FieldResult> { - if !context.validation_result.is_admin { + if !context.validation_result.is_admin() { return Err("Unauthorized user creation".into()); } let user_id = UserId::new(&user.id); @@ -88,7 +88,7 @@ impl Mutation { context: &Context, name: String, ) -> FieldResult> { - if !context.validation_result.is_admin { + if !context.validation_result.is_admin() { return Err("Unauthorized group creation".into()); } let group_id = context.handler.create_group(&name).await?; @@ -103,7 +103,7 @@ impl Mutation { context: &Context, user: UpdateUserInput, ) -> FieldResult { - if !context.validation_result.can_access(&user.id) { + if !context.validation_result.can_write(&user.id) { return Err("Unauthorized user update".into()); } context @@ -123,7 +123,7 @@ impl Mutation { context: &Context, group: UpdateGroupInput, ) -> FieldResult { - if !context.validation_result.is_admin { + if !context.validation_result.is_admin() { return Err("Unauthorized group update".into()); } if group.id == 1 { @@ -144,7 +144,7 @@ impl Mutation { user_id: String, group_id: i32, ) -> FieldResult { - if !context.validation_result.is_admin { + if !context.validation_result.is_admin() { return Err("Unauthorized group membership modification".into()); } context @@ -159,7 +159,7 @@ impl Mutation { user_id: String, group_id: i32, ) -> FieldResult { - if !context.validation_result.is_admin { + if !context.validation_result.is_admin() { return Err("Unauthorized group membership modification".into()); } if context.validation_result.user == user_id && group_id == 1 { @@ -173,7 +173,7 @@ impl Mutation { } async fn delete_user(context: &Context, user_id: String) -> FieldResult { - if !context.validation_result.is_admin { + if !context.validation_result.is_admin() { return Err("Unauthorized user deletion".into()); } if context.validation_result.user == user_id { @@ -184,7 +184,7 @@ impl Mutation { } async fn delete_group(context: &Context, group_id: i32) -> FieldResult { - if !context.validation_result.is_admin { + if !context.validation_result.is_admin() { return Err("Unauthorized group deletion".into()); } if group_id == 1 { diff --git a/server/src/infra/graphql/query.rs b/server/src/infra/graphql/query.rs index 8c592cb..497f8f3 100644 --- a/server/src/infra/graphql/query.rs +++ b/server/src/infra/graphql/query.rs @@ -107,7 +107,7 @@ impl Query { } pub async fn user(context: &Context, user_id: String) -> FieldResult> { - if !context.validation_result.can_access(&user_id) { + if !context.validation_result.can_read(&user_id) { return Err("Unauthorized access to user data".into()); } Ok(context @@ -121,7 +121,7 @@ impl Query { context: &Context, #[graphql(name = "where")] filters: Option, ) -> FieldResult>> { - if !context.validation_result.is_admin { + if !context.validation_result.is_admin_or_readonly() { return Err("Unauthorized access to user list".into()); } Ok(context @@ -132,7 +132,7 @@ impl Query { } async fn groups(context: &Context) -> FieldResult>> { - if !context.validation_result.is_admin { + if !context.validation_result.is_admin_or_readonly() { return Err("Unauthorized access to group list".into()); } Ok(context @@ -143,7 +143,7 @@ impl Query { } async fn group(context: &Context, group_id: i32) -> FieldResult> { - if !context.validation_result.is_admin { + if !context.validation_result.is_admin_or_readonly() { return Err("Unauthorized access to group data".into()); } Ok(context @@ -234,7 +234,7 @@ impl Group { } /// The groups to which this user belongs. async fn users(&self, context: &Context) -> FieldResult>> { - if !context.validation_result.is_admin { + if !context.validation_result.is_admin_or_readonly() { return Err("Unauthorized access to group data".into()); } Ok(context diff --git a/server/src/infra/ldap_handler.rs b/server/src/infra/ldap_handler.rs index 67766e8..1b039af 100644 --- a/server/src/infra/ldap_handler.rs +++ b/server/src/infra/ldap_handler.rs @@ -1,9 +1,12 @@ -use crate::domain::{ - handler::{ - BackendHandler, BindRequest, Group, GroupRequestFilter, LoginHandler, User, UserId, - UserRequestFilter, +use crate::{ + domain::{ + handler::{ + BackendHandler, BindRequest, Group, GroupRequestFilter, LoginHandler, User, UserId, + UserRequestFilter, + }, + opaque_handler::OpaqueHandler, }, - opaque_handler::OpaqueHandler, + infra::auth_service::Permission, }; use anyhow::{bail, Context, Result}; use itertools::Itertools; @@ -378,12 +381,6 @@ fn root_dse_response(base_dn: &str) -> LdapOp { }) } -#[derive(Clone, Copy, PartialEq, Debug)] -enum Permission { - Admin, - Regular, -} - pub struct LdapHandler { user_info: Option<(UserId, Permission)>, backend_handler: Backend, @@ -436,16 +433,19 @@ impl LdapHandler { - let is_admin = self - .backend_handler - .get_user_groups(&user_id) - .await - .map(|groups| groups.iter().any(|g| g.1 == "lldap_admin")) - .unwrap_or(false); + let user_groups = self.backend_handler.get_user_groups(&user_id).await; + let is_in_group = |name| { + user_groups + .as_ref() + .map(|groups| groups.iter().any(|g| g.1 == name)) + .unwrap_or(false) + }; self.user_info = Some(( user_id, - if is_admin { + if is_in_group("lldap_admin") { Permission::Admin + } else if is_in_group("lldap_readonly") { + Permission::Readonly } else { Permission::Regular }, @@ -497,10 +497,10 @@ impl LdapHandler { if *permission != Permission::Admin && user_id != &uid { - return vec![make_search_error( + return vec![make_extended_response( LdapResultCode::InsufficentAccessRights, format!( - r#"User {} cannot modify the password of user {}"#, + r#"User `{}` cannot modify the password of user `{}`"#, &user_id, &uid ), )]; @@ -542,7 +542,7 @@ impl LdapHandler Vec { let user_filter = match &self.user_info { - Some((_, Permission::Admin)) => None, + Some((_, Permission::Admin)) | Some((_, Permission::Readonly)) => None, Some((user_id, Permission::Regular)) => Some(user_id), None => { return vec![make_search_error( @@ -959,8 +959,9 @@ mod tests { make_search_request::("ou=people,Dc=example,dc=com", filter, attrs) } - async fn setup_bound_handler( + async fn setup_bound_handler_with_group( mut mock: MockTestBackendHandler, + group: &str, ) -> LdapHandler { mock.expect_bind() .with(eq(BindRequest { @@ -968,11 +969,12 @@ mod tests { password: "pass".to_string(), })) .return_once(|_| Ok(())); + let group = group.to_string(); mock.expect_get_user_groups() .with(eq(UserId::new("test"))) .return_once(|_| { let mut set = HashSet::new(); - set.insert(GroupIdAndName(GroupId(42), "lldap_admin".to_string())); + set.insert(GroupIdAndName(GroupId(42), group)); Ok(set) }); let mut ldap_handler = @@ -988,6 +990,18 @@ mod tests { ldap_handler } + async fn setup_bound_readonly_handler( + mock: MockTestBackendHandler, + ) -> LdapHandler { + setup_bound_handler_with_group(mock, "lldap_readonly").await + } + + async fn setup_bound_admin_handler( + mock: MockTestBackendHandler, + ) -> LdapHandler { + setup_bound_handler_with_group(mock, "lldap_admin").await + } + #[tokio::test] async fn test_bind() { let mut mock = MockTestBackendHandler::new(); @@ -1053,15 +1067,8 @@ mod tests { } #[tokio::test] - async fn test_search_non_admin_user() { + async fn test_search_regular_user() { let mut mock = MockTestBackendHandler::new(); - mock.expect_bind() - .with(eq(crate::domain::handler::BindRequest { - name: UserId::new("test"), - password: "pass".to_string(), - })) - .times(1) - .return_once(|_| Ok(())); mock.expect_list_users() .with(eq(Some(UserRequestFilter::And(vec![ UserRequestFilter::And(vec![]), @@ -1074,20 +1081,7 @@ mod tests { ..Default::default() }]) }); - mock.expect_get_user_groups() - .with(eq(UserId::new("test"))) - .return_once(|_| Ok(HashSet::new())); - let mut ldap_handler = - LdapHandler::new(mock, "dc=example,dc=com".to_string(), vec![], vec![]); - - let request = LdapBindRequest { - dn: "uid=test,ou=people,dc=example,dc=com".to_string(), - cred: LdapBindCred::Simple("pass".to_string()), - }; - assert_eq!( - ldap_handler.do_bind(&request).await.0, - LdapResultCode::Success - ); + let mut ldap_handler = setup_bound_handler_with_group(mock, "regular").await; let request = make_user_search_request::(LdapFilter::And(vec![]), vec!["1.1".to_string()]); @@ -1103,6 +1097,23 @@ mod tests { ); } + #[tokio::test] + async fn test_search_readonly_user() { + let mut mock = MockTestBackendHandler::new(); + mock.expect_list_users() + .with(eq(Some(UserRequestFilter::And(vec![])))) + .times(1) + .return_once(|_| Ok(vec![])); + let mut ldap_handler = setup_bound_readonly_handler(mock).await; + + let request = + make_user_search_request::(LdapFilter::And(vec![]), vec!["1.1".to_string()]); + assert_eq!( + ldap_handler.do_search(&request).await, + vec![make_search_success()], + ); + } + #[tokio::test] async fn test_bind_invalid_dn() { let mock = MockTestBackendHandler::new(); @@ -1208,7 +1219,7 @@ mod tests { }, ]) }); - let mut ldap_handler = setup_bound_handler(mock).await; + let mut ldap_handler = setup_bound_admin_handler(mock).await; let request = make_user_search_request( LdapFilter::And(vec![]), vec![ @@ -1334,7 +1345,7 @@ mod tests { }, ]) }); - let mut ldap_handler = setup_bound_handler(mock).await; + let mut ldap_handler = setup_bound_admin_handler(mock).await; let request = make_search_request( "ou=groups,dc=example,dc=cOm", LdapFilter::And(vec![]), @@ -1417,7 +1428,7 @@ mod tests { users: vec![], }]) }); - let mut ldap_handler = setup_bound_handler(mock).await; + let mut ldap_handler = setup_bound_admin_handler(mock).await; let request = make_search_request( "ou=groups,dc=example,dc=com", LdapFilter::And(vec![ @@ -1466,7 +1477,7 @@ mod tests { users: vec![], }]) }); - let mut ldap_handler = setup_bound_handler(mock).await; + let mut ldap_handler = setup_bound_admin_handler(mock).await; let request = make_search_request( "ou=groups,dc=example,dc=com", LdapFilter::Or(vec![LdapFilter::Not(Box::new(LdapFilter::Equality( @@ -1505,7 +1516,7 @@ mod tests { "Error getting groups".to_string(), )) }); - let mut ldap_handler = setup_bound_handler(mock).await; + let mut ldap_handler = setup_bound_admin_handler(mock).await; let request = make_search_request( "ou=groups,dc=example,dc=com", LdapFilter::Or(vec![LdapFilter::Not(Box::new(LdapFilter::Equality( @@ -1525,7 +1536,7 @@ mod tests { #[tokio::test] async fn test_search_groups_filter_error() { - let mut ldap_handler = setup_bound_handler(MockTestBackendHandler::new()).await; + let mut ldap_handler = setup_bound_admin_handler(MockTestBackendHandler::new()).await; let request = make_search_request( "ou=groups,dc=example,dc=com", LdapFilter::And(vec![LdapFilter::Substring( @@ -1561,7 +1572,7 @@ mod tests { ])))) .times(1) .return_once(|_| Ok(vec![])); - let mut ldap_handler = setup_bound_handler(mock).await; + let mut ldap_handler = setup_bound_admin_handler(mock).await; let request = make_user_search_request( LdapFilter::And(vec![LdapFilter::Or(vec![ LdapFilter::Not(Box::new(LdapFilter::Equality( @@ -1590,7 +1601,7 @@ mod tests { .with(eq(Some(UserRequestFilter::MemberOf("group_1".to_string())))) .times(1) .return_once(|_| Ok(vec![])); - let mut ldap_handler = setup_bound_handler(mock).await; + let mut ldap_handler = setup_bound_admin_handler(mock).await; let request = make_user_search_request( LdapFilter::Equality( "memberOf".to_string(), @@ -1645,7 +1656,7 @@ mod tests { ..Default::default() }]) }); - let mut ldap_handler = setup_bound_handler(mock).await; + let mut ldap_handler = setup_bound_admin_handler(mock).await; let request = make_user_search_request( LdapFilter::And(vec![LdapFilter::Or(vec![LdapFilter::Not(Box::new( LdapFilter::Equality("givenname".to_string(), "bob".to_string()), @@ -1695,7 +1706,7 @@ mod tests { users: vec![UserId::new("bob"), UserId::new("john")], }]) }); - let mut ldap_handler = setup_bound_handler(mock).await; + let mut ldap_handler = setup_bound_admin_handler(mock).await; let request = make_search_request( "dc=example,dc=com", LdapFilter::And(vec![]), @@ -1771,7 +1782,7 @@ mod tests { users: vec![UserId::new("bob"), UserId::new("john")], }]) }); - let mut ldap_handler = setup_bound_handler(mock).await; + let mut ldap_handler = setup_bound_admin_handler(mock).await; // Test simple wildcard let request = @@ -1895,7 +1906,7 @@ mod tests { #[tokio::test] async fn test_search_wrong_base() { - let mut ldap_handler = setup_bound_handler(MockTestBackendHandler::new()).await; + let mut ldap_handler = setup_bound_admin_handler(MockTestBackendHandler::new()).await; let request = make_search_request( "ou=users,dc=example,dc=com", LdapFilter::And(vec![]), @@ -1909,7 +1920,7 @@ mod tests { #[tokio::test] async fn test_search_unsupported_filters() { - let mut ldap_handler = setup_bound_handler(MockTestBackendHandler::new()).await; + let mut ldap_handler = setup_bound_admin_handler(MockTestBackendHandler::new()).await; let request = make_user_search_request( LdapFilter::Substring( "uid".to_string(), @@ -1952,7 +1963,7 @@ mod tests { mock.expect_registration_finish() .times(1) .return_once(|_| Ok(())); - let mut ldap_handler = setup_bound_handler(mock).await; + let mut ldap_handler = setup_bound_admin_handler(mock).await; let request = LdapOp::ExtendedRequest( LdapPasswordModifyRequest { user_identity: Some("uid=bob,ou=people,dc=example,dc=com".to_string()), @@ -1972,7 +1983,7 @@ mod tests { #[tokio::test] async fn test_password_change_errors() { - let mut ldap_handler = setup_bound_handler(MockTestBackendHandler::new()).await; + let mut ldap_handler = setup_bound_admin_handler(MockTestBackendHandler::new()).await; let request = LdapOp::ExtendedRequest( LdapPasswordModifyRequest { user_identity: None, @@ -2016,9 +2027,29 @@ mod tests { ); } + #[tokio::test] + async fn test_password_change_unauthorized() { + let mut ldap_handler = setup_bound_readonly_handler(MockTestBackendHandler::new()).await; + let request = LdapOp::ExtendedRequest( + LdapPasswordModifyRequest { + user_identity: Some("uid=bob,ou=people,dc=example,dc=com".to_string()), + old_password: Some("pass".to_string()), + new_password: Some("password".to_string()), + } + .into(), + ); + assert_eq!( + ldap_handler.handle_ldap_message(request).await, + Some(vec![make_extended_response( + LdapResultCode::InsufficentAccessRights, + "User `test` cannot modify the password of user `bob`".to_string(), + )]) + ); + } + #[tokio::test] async fn test_search_root_dse() { - let mut ldap_handler = setup_bound_handler(MockTestBackendHandler::new()).await; + let mut ldap_handler = setup_bound_admin_handler(MockTestBackendHandler::new()).await; let request = LdapSearchRequest { base: "".to_string(), scope: LdapSearchScope::Base, diff --git a/server/src/main.rs b/server/src/main.rs index c628061..7b637aa 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -4,7 +4,7 @@ use crate::{ domain::{ - handler::{BackendHandler, CreateUserRequest}, + handler::{BackendHandler, CreateUserRequest, GroupRequestFilter}, sql_backend_handler::SqlBackendHandler, sql_opaque_handler::register_password, sql_tables::PoolOptions, @@ -62,6 +62,19 @@ async fn run_server(config: Configuration) -> Result<()> { .map_err(|e| anyhow!("Error setting up admin login/account: {:#}", e)) .context("while creating the admin user")?; } + if backend_handler + .list_groups(Some(GroupRequestFilter::DisplayName( + "lldap_readonly".to_string(), + ))) + .await? + .is_empty() + { + warn!("Could not find readonly group, trying to create it"); + backend_handler + .create_group("lldap_readonly") + .await + .context("while creating readonly group")?; + } let server_builder = infra::ldap_server::build_ldap_server( &config, backend_handler.clone(),