release: v0.4.0

CHANGELOG.md

@@ -7,6 +7,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.4.0] - 2022-07-08
+
+### Breaking
+
+The `lldap_readonly` group has been renamed `lldap_password_manager` (migration happens automatically) and a new `lldap_strict_readonly` group was introduced.
+
+### Added
+  - A new `lldap_strict_readonly` group allows granting readonly rights to users (not able to change other's passwords, in particular).
+
+### Changed
+  - The `lldap_readonly` group is renamed `lldap_password_manager` since it still allows users to change (non-admin) passwords.
+
+### Removed
+  - The `lldap_readonly` group was removed.
+
 ## [0.3.0] - 2022-07-08
 
 ### Breaking

Cargo.lock

@@ -1959,7 +1959,7 @@ checksum = "0717cef1bc8b636c6e1c1bbdefc09e6322da8a9321966e8928ef80d20f7f770f"
 
 [[package]]
 name = "lldap"
-version = "0.3.0-rc.1"
+version = "0.4.0"
 dependencies = [
  "actix",
  "actix-files",
@@ -2022,7 +2022,7 @@ dependencies = [
 
 [[package]]
 name = "lldap_app"
-version = "0.3.0-alpha.1"
+version = "0.4.0"
 dependencies = [
  "anyhow",
  "chrono",
@@ -2035,7 +2035,7 @@ dependencies = [
  "serde",
  "serde_json",
  "validator",
- "validator_derive 0.15.0",
+ "validator_derive",
  "wasm-bindgen",
  "web-sys",
  "yew",
@@ -3973,7 +3973,7 @@ dependencies = [
  "serde_derive",
  "serde_json",
  "url",
- "validator_types 0.14.0",
+ "validator_types",
 ]
 
 [[package]]
@@ -3989,23 +3989,7 @@ dependencies = [
  "quote",
  "regex",
  "syn",
- "validator_types 0.14.0",
+ "validator_types",
-]
-
-[[package]]
-name = "validator_derive"
-version = "0.15.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "ea7ed5e8cf2b6bdd64a6c4ce851da25388a89327b17b88424ceced6bd5017923"
-dependencies = [
- "if_chain",
- "lazy_static",
- "proc-macro-error",
- "proc-macro2",
- "quote",
- "regex",
- "syn",
- "validator_types 0.15.0",
 ]
 
 [[package]]
@@ -4018,16 +4002,6 @@ dependencies = [
  "syn",
 ]
 
-[[package]]
-name = "validator_types"
-version = "0.15.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "d2ddf34293296847abfc1493b15c6e2f5d3cd19f57ad7d22673bf4c6278da329"
-dependencies = [
- "proc-macro2",
- "syn",
-]
-
 [[package]]
 name = "valuable"
 version = "0.1.0"
@@ -4346,7 +4320,7 @@ version = "0.1.8"
 source = "git+https://github.com/sassman/yew_form/?rev=67050812695b7a8a90b81b0637e347fc6629daed#67050812695b7a8a90b81b0637e347fc6629daed"
 dependencies = [
  "validator",
- "validator_derive 0.14.0",
+ "validator_derive",
  "yew",
 ]
 

README.md

@@ -190,8 +190,8 @@ filter like: `(memberOf=cn=admins,ou=groups,dc=example,dc=com)`.
 
 The administrator group for LLDAP is `lldap_admin`: anyone in this group has
 admin rights in the Web UI. Most LDAP integrations should instead use a user in
-the `lldap_readonly` group, to avoid granting full administration access to
+the `lldap_strict_readonly` or `lldap_password_manager` group, to avoid granting full
-many services.
+administration access to many services.
 
 ### Sample client configurations
 

app/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lldap_app"
-version = "0.3.0"
+version = "0.4.0"
 authors = ["Valentin Tolmer <valentin@tolmer.fr>"]
 edition = "2021"
 

server/Cargo.toml

@@ -2,7 +2,7 @@
 authors = ["Valentin Tolmer <valentin@tolmer.fr>"]
 edition = "2021"
 name = "lldap"
-version = "0.3.0"
+version = "0.4.0"
 
 [dependencies]
 actix = "0.12"

server/src/domain/sql_backend_handler.rs

@@ -463,24 +463,7 @@ impl BackendHandler for SqlBackendHandler {
     #[instrument(skip_all, level = "debug", ret, err)]
     async fn create_group(&self, group_name: &str) -> Result<GroupId> {
         debug!(?group_name);
-        let now = chrono::Utc::now();
+        crate::domain::sql_tables::create_group(group_name, &self.sql_pool).await?;
-        let (query, values) = Query::insert()
-            .into_table(Groups::Table)
-            .columns(vec![
-                Groups::DisplayName,
-                Groups::CreationDate,
-                Groups::Uuid,
-            ])
-            .values_panic(vec![
-                group_name.into(),
-                now.naive_utc().into(),
-                Uuid::from_name_and_date(group_name, &now).into(),
-            ])
-            .build_sqlx(DbQueryBuilder {});
-        debug!(%query);
-        query_with(query.as_str(), values)
-            .execute(&self.sql_pool)
-            .await?;
         let (query, values) = Query::select()
             .column(Groups::GroupId)
             .from(Groups::Table)

server/src/domain/sql_tables.rs

@@ -1,7 +1,8 @@
 use super::handler::{GroupId, UserId, Uuid};
 use sea_query::*;
+use sea_query_binder::SqlxBinder;
 use sqlx::Row;
-use tracing::warn;
+use tracing::{debug, warn};
 
 pub type Pool = sqlx::sqlite::SqlitePool;
 pub type PoolOptions = sqlx::sqlite::SqlitePoolOptions;
@@ -83,6 +84,28 @@ async fn column_exists(pool: &Pool, table_name: &str, column_name: &str) -> sqlx
         > 0)
 }
 
+pub async fn create_group(group_name: &str, pool: &Pool) -> sqlx::Result<()> {
+    let now = chrono::Utc::now();
+    let (query, values) = Query::insert()
+        .into_table(Groups::Table)
+        .columns(vec![
+            Groups::DisplayName,
+            Groups::CreationDate,
+            Groups::Uuid,
+        ])
+        .values_panic(vec![
+            group_name.into(),
+            now.naive_utc().into(),
+            Uuid::from_name_and_date(group_name, &now).into(),
+        ])
+        .build_sqlx(DbQueryBuilder {});
+    debug!(%query);
+    sqlx::query_with(query.as_str(), values)
+        .execute(pool)
+        .await
+        .map(|_| ())
+}
+
 pub async fn init_table(pool: &Pool) -> sqlx::Result<()> {
     // SQLite needs this pragma to be turned on. Other DB might not understand this, so ignore the
     // error.
@@ -298,6 +321,29 @@ pub async fn init_table(pool: &Pool) -> sqlx::Result<()> {
     .execute(pool)
     .await?;
 
+    if sqlx::query(
+        &Query::select()
+            .from(Groups::Table)
+            .column(Groups::DisplayName)
+            .cond_where(Expr::col(Groups::DisplayName).eq("lldap_readonly"))
+            .to_string(DbQueryBuilder {}),
+    )
+    .fetch_one(pool)
+    .await
+    .is_ok()
+    {
+        sqlx::query(
+            &Query::update()
+                .table(Groups::Table)
+                .values(vec![(Groups::DisplayName, "lldap_password_manager".into())])
+                .cond_where(Expr::col(Groups::DisplayName).eq("lldap_readonly"))
+                .to_string(DbQueryBuilder {}),
+        )
+        .execute(pool)
+        .await?;
+        create_group("lldap_strict_readonly", pool).await?
+    }
+
     Ok(())
 }
 
@@ -349,14 +395,21 @@ mod tests {
         .execute(&sql_pool)
         .await
         .unwrap();
-        sqlx::query(r#"CREATE TABLE groups ( group_id int, display_name TEXT );"#)
+        sqlx::query(r#"CREATE TABLE groups ( group_id INTEGER PRIMARY KEY, display_name TEXT );"#)
             .execute(&sql_pool)
             .await
             .unwrap();
+        sqlx::query(
+            r#"INSERT INTO groups (display_name)
+                      VALUES ("lldap_admin"), ("lldap_readonly")"#,
+        )
+        .execute(&sql_pool)
+        .await
+        .unwrap();
         init_table(&sql_pool).await.unwrap();
         sqlx::query(
-            r#"INSERT INTO groups (group_id, display_name, creation_date, uuid)
+            r#"INSERT INTO groups (display_name, creation_date, uuid)
-                      VALUES (3, "test", "1970-01-01 00:00:00", "abc")"#,
+                      VALUES ("test", "1970-01-01 00:00:00", "abc")"#,
         )
         .execute(&sql_pool)
         .await
@@ -371,5 +424,23 @@ mod tests {
                 .collect::<Vec<_>>(),
             vec![crate::uuid!("a02eaf13-48a7-30f6-a3d4-040ff7c52b04")]
         );
+        assert_eq!(
+            sqlx::query(r#"SELECT group_id, display_name FROM groups"#)
+                .fetch_all(&sql_pool)
+                .await
+                .unwrap()
+                .into_iter()
+                .map(|row| (
+                    row.get::<GroupId, _>("group_id"),
+                    row.get::<String, _>("display_name")
+                ))
+                .collect::<Vec<_>>(),
+            vec![
+                (GroupId(1), "lldap_admin".to_string()),
+                (GroupId(2), "lldap_password_manager".to_string()),
+                (GroupId(3), "lldap_strict_readonly".to_string()),
+                (GroupId(4), "test".to_string())
+            ]
+        );
     }
 }

server/src/infra/auth_service.rs

@@ -430,7 +430,7 @@ async fn opaque_register_start<Backend>(
     data: web::Data<AppState<Backend>>,
 ) -> TcpResult<registration::ServerRegistrationStartResponse>
 where
-    Backend: OpaqueHandler + 'static,
+    Backend: BackendHandler + OpaqueHandler + 'static,
 {
     use actix_web::FromRequest;
     let validation_result = BearerAuth::from_request(&request, &mut payload.0)
@@ -448,8 +448,14 @@ where
         .await
         .map_err(|e| TcpError::BadRequest(format!("{:#?}", e)))?
         .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(
             "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>>,
 ) -> ApiResult<registration::ServerRegistrationStartResponse>
 where
-    Backend: OpaqueHandler + 'static,
+    Backend: BackendHandler + OpaqueHandler + 'static,
 {
     opaque_register_start(request, payload, data)
         .await
@@ -559,13 +565,14 @@ where
 #[derive(Clone, Copy, PartialEq, Debug)]
 pub enum Permission {
     Admin,
+    PasswordManager,
     Readonly,
     Regular,
 }
 
-#[derive(Debug)]
+#[derive(Debug, Clone, PartialEq)]
 pub struct ValidationResults {
-    pub user: String,
+    pub user: UserId,
     pub permission: Permission,
 }
 
@@ -573,7 +580,7 @@ impl ValidationResults {
     #[cfg(test)]
     pub fn admin() -> Self {
         Self {
-            user: "admin".to_string(),
+            user: UserId::new("admin"),
             permission: Permission::Admin,
         }
     }
@@ -585,19 +592,29 @@ impl ValidationResults {
 
     #[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
+            || self.permission == Permission::PasswordManager
     }
 
     #[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);
     Ok(ValidationResults {
-        user: token.claims().user.clone(),
+        user: UserId::new(&token.claims().user),
         permission: if is_in_group("lldap_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
         } else {
             Permission::Regular

server/src/infra/graphql/mutation.rs

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

server/src/infra/graphql/query.rs

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

server/src/infra/ldap_handler.rs

@@ -6,7 +6,7 @@ use crate::{
         },
         opaque_handler::OpaqueHandler,
     },
-    infra::auth_service::Permission,
+    infra::auth_service::{Permission, ValidationResults},
 };
 use anyhow::{bail, Context, Result};
 use itertools::Itertools;
@@ -450,7 +450,7 @@ fn root_dse_response(base_dn: &str) -> LdapOp {
 }
 
 pub struct LdapHandler<Backend: BackendHandler + LoginHandler + OpaqueHandler> {
-    user_info: Option<(UserId, Permission)>,
+    user_info: Option<ValidationResults>,
     backend_handler: Backend,
     pub base_dn: Vec<(String, 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))
                         .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
-                    } 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
                     } else {
                         Permission::Regular
                     },
-                ));
+                });
                 debug!("Success!");
                 (LdapResultCode::Success, "".to_string())
             }
@@ -553,7 +555,7 @@ impl<Backend: BackendHandler + LoginHandler + OpaqueHandler> LdapHandler<Backend
         &mut self,
         request: &LdapPasswordModifyRequest,
     ) -> Vec<LdapOp> {
-        let (user_id, permission) = match &self.user_info {
+        let credentials = match &self.user_info {
             Some(info) => info,
             _ => {
                 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(
                                 LdapResultCode::InsufficentAccessRights,
                                 format!(
                                     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> {
-        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 => {
                 return vec![make_search_error(
                     LdapResultCode::InsufficentAccessRights,
                     "No user currently bound".to_string(),
-                )];
+                )]
             }
+            Some(u) => u,
         };
         if request.base.is_empty()
             && request.scope == LdapSearchScope::Base
@@ -643,6 +641,11 @@ impl<Backend: BackendHandler + LoginHandler + OpaqueHandler> LdapHandler<Backend
             debug!("rootDSE request");
             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
     }
 
@@ -1135,7 +1138,13 @@ mod tests {
     async fn setup_bound_readonly_handler(
         mock: 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(
@@ -2312,7 +2321,7 @@ mod tests {
     }
 
     #[tokio::test]
-    async fn test_password_change_readonly() {
+    async fn test_password_change_password_manager() {
         let mut mock = MockTestBackendHandler::new();
         mock.expect_get_user_groups()
             .with(eq(UserId::new("bob")))
@@ -2340,7 +2349,7 @@ mod tests {
         mock.expect_registration_finish()
             .times(1)
             .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(
             LdapPasswordModifyRequest {
                 user_identity: Some("uid=bob,ou=people,dc=example,dc=com".to_string()),
@@ -2409,7 +2418,7 @@ mod tests {
     }
 
     #[tokio::test]
-    async fn test_password_change_unauthorized_readonly() {
+    async fn test_password_change_unauthorized_password_manager() {
         let mut mock = MockTestBackendHandler::new();
         let mut groups = HashSet::new();
         groups.insert(GroupDetails {
@@ -2422,6 +2431,31 @@ mod tests {
             .with(eq(UserId::new("bob")))
             .times(1)
             .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 request = LdapOp::ExtendedRequest(
             LdapPasswordModifyRequest {

server/src/main.rs

@@ -68,14 +68,18 @@ async fn set_up_server(config: Configuration) -> Result<ServerBuilder> {
     }
     if backend_handler
         .list_groups(Some(GroupRequestFilter::DisplayName(
-            "lldap_readonly".to_string(),
+            "lldap_password_manager".to_string(),
         )))
         .await?
         .is_empty()
     {
-        warn!("Could not find readonly group, trying to create it");
+        warn!("Could not find password_manager group, trying to create it");
         backend_handler
-            .create_group("lldap_readonly")
+            .create_group("lldap_password_manager")
+            .await
+            .context("while creating password_manager group")?;
+        backend_handler
+            .create_group("lldap_strict_readonly")
             .await
             .context("while creating readonly group")?;
     }