server: Update permission checks for strict_readonly

This commit is contained in:
Valentin Tolmer 2022-07-08 18:30:15 +02:00 committed by nitnelave
parent 500a441df7
commit cf19fd41b0
4 changed files with 100 additions and 43 deletions

View File

@ -430,7 +430,7 @@ async fn opaque_register_start<Backend>(
data: web::Data<AppState<Backend>>, data: web::Data<AppState<Backend>>,
) -> TcpResult<registration::ServerRegistrationStartResponse> ) -> TcpResult<registration::ServerRegistrationStartResponse>
where where
Backend: OpaqueHandler + 'static, Backend: BackendHandler + OpaqueHandler + 'static,
{ {
use actix_web::FromRequest; use actix_web::FromRequest;
let validation_result = BearerAuth::from_request(&request, &mut payload.0) let validation_result = BearerAuth::from_request(&request, &mut payload.0)
@ -448,8 +448,14 @@ where
.await .await
.map_err(|e| TcpError::BadRequest(format!("{:#?}", e)))? .map_err(|e| TcpError::BadRequest(format!("{:#?}", e)))?
.into_inner(); .into_inner();
let user_id = &registration_start_request.username; let user_id = UserId::new(&registration_start_request.username);
if !validation_result.can_write(user_id) { let user_is_admin = data
.backend_handler
.get_user_groups(&user_id)
.await?
.iter()
.any(|g| g.display_name == "lldap_admin");
if !validation_result.can_change_password(&user_id, user_is_admin) {
return Err(TcpError::UnauthorizedError( return Err(TcpError::UnauthorizedError(
"Not authorized to change the user's password".to_string(), "Not authorized to change the user's password".to_string(),
)); ));
@ -466,7 +472,7 @@ async fn opaque_register_start_handler<Backend>(
data: web::Data<AppState<Backend>>, data: web::Data<AppState<Backend>>,
) -> ApiResult<registration::ServerRegistrationStartResponse> ) -> ApiResult<registration::ServerRegistrationStartResponse>
where where
Backend: OpaqueHandler + 'static, Backend: BackendHandler + OpaqueHandler + 'static,
{ {
opaque_register_start(request, payload, data) opaque_register_start(request, payload, data)
.await .await
@ -559,13 +565,14 @@ where
#[derive(Clone, Copy, PartialEq, Debug)] #[derive(Clone, Copy, PartialEq, Debug)]
pub enum Permission { pub enum Permission {
Admin, Admin,
PasswordManager,
Readonly, Readonly,
Regular, Regular,
} }
#[derive(Debug)] #[derive(Debug, Clone, PartialEq)]
pub struct ValidationResults { pub struct ValidationResults {
pub user: String, pub user: UserId,
pub permission: Permission, pub permission: Permission,
} }
@ -573,7 +580,7 @@ impl ValidationResults {
#[cfg(test)] #[cfg(test)]
pub fn admin() -> Self { pub fn admin() -> Self {
Self { Self {
user: "admin".to_string(), user: UserId::new("admin"),
permission: Permission::Admin, permission: Permission::Admin,
} }
} }
@ -585,19 +592,29 @@ impl ValidationResults {
#[must_use] #[must_use]
pub fn is_admin_or_readonly(&self) -> bool { 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::Admin
|| self.permission == Permission::Readonly || self.permission == Permission::Readonly
|| self.user == user || self.permission == Permission::PasswordManager
} }
#[must_use] #[must_use]
pub fn can_write(&self, user: &str) -> bool { pub fn can_read(&self, user: &UserId) -> bool {
self.permission == Permission::Admin || self.user == user self.permission == Permission::Admin
|| self.permission == Permission::PasswordManager
|| self.permission == Permission::Readonly
|| &self.user == user
}
#[must_use]
pub fn can_change_password(&self, user: &UserId, user_is_admin: bool) -> bool {
self.permission == Permission::Admin
|| (self.permission == Permission::PasswordManager && !user_is_admin)
|| &self.user == user
}
#[must_use]
pub fn can_write(&self, user: &UserId) -> bool {
self.permission == Permission::Admin || &self.user == user
} }
} }
@ -627,10 +644,12 @@ pub(crate) fn check_if_token_is_valid<Backend>(
} }
let is_in_group = |name| token.claims().groups.contains(name); let is_in_group = |name| token.claims().groups.contains(name);
Ok(ValidationResults { Ok(ValidationResults {
user: token.claims().user.clone(), user: UserId::new(&token.claims().user),
permission: if is_in_group("lldap_admin") { permission: if is_in_group("lldap_admin") {
Permission::Admin Permission::Admin
} else if is_in_group("lldap_readonly") { } else if is_in_group("lldap_password_manager") {
Permission::PasswordManager
} else if is_in_group("lldap_strict_readonly") {
Permission::Readonly Permission::Readonly
} else { } else {
Permission::Regular Permission::Regular

View File

@ -121,14 +121,15 @@ impl<Handler: BackendHandler + Sync> Mutation<Handler> {
span.in_scope(|| { span.in_scope(|| {
debug!(?user.id); debug!(?user.id);
}); });
if !context.validation_result.can_write(&user.id) { let user_id = UserId::new(&user.id);
if !context.validation_result.can_write(&user_id) {
span.in_scope(|| debug!("Unauthorized")); span.in_scope(|| debug!("Unauthorized"));
return Err("Unauthorized user update".into()); return Err("Unauthorized user update".into());
} }
context context
.handler .handler
.update_user(UpdateUserRequest { .update_user(UpdateUserRequest {
user_id: UserId::new(&user.id), user_id,
email: user.email, email: user.email,
display_name: user.display_name, display_name: user.display_name,
first_name: user.first_name, first_name: user.first_name,
@ -200,13 +201,14 @@ impl<Handler: BackendHandler + Sync> Mutation<Handler> {
span.in_scope(|| debug!("Unauthorized")); span.in_scope(|| debug!("Unauthorized"));
return Err("Unauthorized group membership modification".into()); return Err("Unauthorized group membership modification".into());
} }
let user_id = UserId::new(&user_id);
if context.validation_result.user == user_id && group_id == 1 { if context.validation_result.user == user_id && group_id == 1 {
span.in_scope(|| debug!("Cannot remove admin rights for current user")); span.in_scope(|| debug!("Cannot remove admin rights for current user"));
return Err("Cannot remove admin rights for current user".into()); return Err("Cannot remove admin rights for current user".into());
} }
context context
.handler .handler
.remove_user_from_group(&UserId::new(&user_id), GroupId(group_id)) .remove_user_from_group(&user_id, GroupId(group_id))
.instrument(span) .instrument(span)
.await?; .await?;
Ok(Success::new()) Ok(Success::new())
@ -217,6 +219,7 @@ impl<Handler: BackendHandler + Sync> Mutation<Handler> {
span.in_scope(|| { span.in_scope(|| {
debug!(?user_id); debug!(?user_id);
}); });
let user_id = UserId::new(&user_id);
if !context.validation_result.is_admin() { if !context.validation_result.is_admin() {
span.in_scope(|| debug!("Unauthorized")); span.in_scope(|| debug!("Unauthorized"));
return Err("Unauthorized user deletion".into()); return Err("Unauthorized user deletion".into());
@ -227,7 +230,7 @@ impl<Handler: BackendHandler + Sync> Mutation<Handler> {
} }
context context
.handler .handler
.delete_user(&UserId::new(&user_id)) .delete_user(&user_id)
.instrument(span) .instrument(span)
.await?; .await?;
Ok(Success::new()) Ok(Success::new())

View File

@ -113,13 +113,14 @@ impl<Handler: BackendHandler + Sync> Query<Handler> {
span.in_scope(|| { span.in_scope(|| {
debug!(?user_id); debug!(?user_id);
}); });
let user_id = UserId::new(&user_id);
if !context.validation_result.can_read(&user_id) { if !context.validation_result.can_read(&user_id) {
span.in_scope(|| debug!("Unauthorized")); span.in_scope(|| debug!("Unauthorized"));
return Err("Unauthorized access to user data".into()); return Err("Unauthorized access to user data".into());
} }
Ok(context Ok(context
.handler .handler
.get_user_details(&UserId::new(&user_id)) .get_user_details(&user_id)
.instrument(span) .instrument(span)
.await .await
.map(Into::into)?) .map(Into::into)?)

View File

@ -6,7 +6,7 @@ use crate::{
}, },
opaque_handler::OpaqueHandler, opaque_handler::OpaqueHandler,
}, },
infra::auth_service::Permission, infra::auth_service::{Permission, ValidationResults},
}; };
use anyhow::{bail, Context, Result}; use anyhow::{bail, Context, Result};
use itertools::Itertools; use itertools::Itertools;
@ -450,7 +450,7 @@ fn root_dse_response(base_dn: &str) -> LdapOp {
} }
pub struct LdapHandler<Backend: BackendHandler + LoginHandler + OpaqueHandler> { pub struct LdapHandler<Backend: BackendHandler + LoginHandler + OpaqueHandler> {
user_info: Option<(UserId, Permission)>, user_info: Option<ValidationResults>,
backend_handler: Backend, backend_handler: Backend,
pub base_dn: Vec<(String, String)>, pub base_dn: Vec<(String, String)>,
base_dn_str: String, base_dn_str: String,
@ -509,16 +509,18 @@ impl<Backend: BackendHandler + LoginHandler + OpaqueHandler> LdapHandler<Backend
.map(|groups| groups.iter().any(|g| g.display_name == name)) .map(|groups| groups.iter().any(|g| g.display_name == name))
.unwrap_or(false) .unwrap_or(false)
}; };
self.user_info = Some(( self.user_info = Some(ValidationResults {
user_id, user: user_id,
if is_in_group("lldap_admin") { permission: if is_in_group("lldap_admin") {
Permission::Admin Permission::Admin
} else if is_in_group("lldap_readonly") { } else if is_in_group("lldap_password_manager") {
Permission::PasswordManager
} else if is_in_group("lldap_strict_readonly") {
Permission::Readonly Permission::Readonly
} else { } else {
Permission::Regular Permission::Regular
}, },
)); });
debug!("Success!"); debug!("Success!");
(LdapResultCode::Success, "".to_string()) (LdapResultCode::Success, "".to_string())
} }
@ -553,7 +555,7 @@ impl<Backend: BackendHandler + LoginHandler + OpaqueHandler> LdapHandler<Backend
&mut self, &mut self,
request: &LdapPasswordModifyRequest, request: &LdapPasswordModifyRequest,
) -> Vec<LdapOp> { ) -> Vec<LdapOp> {
let (user_id, permission) = match &self.user_info { let credentials = match &self.user_info {
Some(info) => info, Some(info) => info,
_ => { _ => {
return vec![make_search_error( return vec![make_search_error(
@ -578,15 +580,12 @@ impl<Backend: BackendHandler + LoginHandler + OpaqueHandler> LdapHandler<Backend
)] )]
} }
}; };
if !(*permission == Permission::Admin if !credentials.can_change_password(&uid, user_is_admin) {
|| user_id == &uid
|| (*permission == Permission::Readonly && !user_is_admin))
{
return vec![make_extended_response( return vec![make_extended_response(
LdapResultCode::InsufficentAccessRights, LdapResultCode::InsufficentAccessRights,
format!( format!(
r#"User `{}` cannot modify the password of user `{}`"#, r#"User `{}` cannot modify the password of user `{}`"#,
&user_id, &uid &credentials.user, &uid
), ),
)]; )];
} }
@ -626,15 +625,14 @@ impl<Backend: BackendHandler + LoginHandler + OpaqueHandler> LdapHandler<Backend
} }
pub async fn do_search_or_dse(&mut self, request: &LdapSearchRequest) -> Vec<LdapOp> { pub async fn do_search_or_dse(&mut self, request: &LdapSearchRequest) -> Vec<LdapOp> {
let user_filter = match self.user_info.clone() { let user_info = match &self.user_info {
Some((_, Permission::Admin)) | Some((_, Permission::Readonly)) => None,
Some((user_id, Permission::Regular)) => Some(user_id),
None => { None => {
return vec![make_search_error( return vec![make_search_error(
LdapResultCode::InsufficentAccessRights, LdapResultCode::InsufficentAccessRights,
"No user currently bound".to_string(), "No user currently bound".to_string(),
)]; )]
} }
Some(u) => u,
}; };
if request.base.is_empty() if request.base.is_empty()
&& request.scope == LdapSearchScope::Base && request.scope == LdapSearchScope::Base
@ -643,6 +641,11 @@ impl<Backend: BackendHandler + LoginHandler + OpaqueHandler> LdapHandler<Backend
debug!("rootDSE request"); debug!("rootDSE request");
return vec![root_dse_response(&self.base_dn_str), make_search_success()]; return vec![root_dse_response(&self.base_dn_str), make_search_success()];
} }
let user_filter = if user_info.is_admin_or_readonly() {
None
} else {
Some(user_info.user.clone())
};
self.do_search(request, user_filter).await self.do_search(request, user_filter).await
} }
@ -1135,7 +1138,13 @@ mod tests {
async fn setup_bound_readonly_handler( async fn setup_bound_readonly_handler(
mock: MockTestBackendHandler, mock: MockTestBackendHandler,
) -> LdapHandler<MockTestBackendHandler> { ) -> LdapHandler<MockTestBackendHandler> {
setup_bound_handler_with_group(mock, "lldap_readonly").await setup_bound_handler_with_group(mock, "lldap_strict_readonly").await
}
async fn setup_bound_password_manager_handler(
mock: MockTestBackendHandler,
) -> LdapHandler<MockTestBackendHandler> {
setup_bound_handler_with_group(mock, "lldap_password_manager").await
} }
async fn setup_bound_admin_handler( async fn setup_bound_admin_handler(
@ -2312,7 +2321,7 @@ mod tests {
} }
#[tokio::test] #[tokio::test]
async fn test_password_change_readonly() { async fn test_password_change_password_manager() {
let mut mock = MockTestBackendHandler::new(); let mut mock = MockTestBackendHandler::new();
mock.expect_get_user_groups() mock.expect_get_user_groups()
.with(eq(UserId::new("bob"))) .with(eq(UserId::new("bob")))
@ -2340,7 +2349,7 @@ mod tests {
mock.expect_registration_finish() mock.expect_registration_finish()
.times(1) .times(1)
.return_once(|_| Ok(())); .return_once(|_| Ok(()));
let mut ldap_handler = setup_bound_readonly_handler(mock).await; let mut ldap_handler = setup_bound_password_manager_handler(mock).await;
let request = LdapOp::ExtendedRequest( let request = LdapOp::ExtendedRequest(
LdapPasswordModifyRequest { LdapPasswordModifyRequest {
user_identity: Some("uid=bob,ou=people,dc=example,dc=com".to_string()), user_identity: Some("uid=bob,ou=people,dc=example,dc=com".to_string()),
@ -2409,7 +2418,7 @@ mod tests {
} }
#[tokio::test] #[tokio::test]
async fn test_password_change_unauthorized_readonly() { async fn test_password_change_unauthorized_password_manager() {
let mut mock = MockTestBackendHandler::new(); let mut mock = MockTestBackendHandler::new();
let mut groups = HashSet::new(); let mut groups = HashSet::new();
groups.insert(GroupDetails { groups.insert(GroupDetails {
@ -2422,6 +2431,31 @@ mod tests {
.with(eq(UserId::new("bob"))) .with(eq(UserId::new("bob")))
.times(1) .times(1)
.return_once(|_| Ok(groups)); .return_once(|_| Ok(groups));
let mut ldap_handler = setup_bound_password_manager_handler(mock).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_password_change_unauthorized_readonly() {
let mut mock = MockTestBackendHandler::new();
mock.expect_get_user_groups()
.with(eq(UserId::new("bob")))
.times(1)
.return_once(|_| Ok(HashSet::new()));
let mut ldap_handler = setup_bound_readonly_handler(mock).await; let mut ldap_handler = setup_bound_readonly_handler(mock).await;
let request = LdapOp::ExtendedRequest( let request = LdapOp::ExtendedRequest(
LdapPasswordModifyRequest { LdapPasswordModifyRequest {