Adding a new verb group remove_members (#434)

Co-authored-by: William Brown <william@blackhats.net.au>

designs/idm_rest_layout.rst

@@ -193,7 +193,7 @@ group
         GET -> get this groups attr
         PUT -> overwrite this group attr value list
         POST -> append this list to group attr
-        DELETE -> purge this attr
+        DELETE -> purge this attr (if body empty) or the elements listed in the body
 
 schema
 ======

kanidm_client/src/asynchronous.rs

@@ -129,7 +129,6 @@ impl KanidmAsyncClient {
         // let dest = format!("{}{}", self.addr, dest);
 
         let req_string = serde_json::to_string(&request).map_err(ClientError::JSONEncode)?;
-
         let response = self
             .client
             .post(dest.as_str())
@@ -260,7 +259,55 @@ impl KanidmAsyncClient {
 
     async fn perform_delete_request(&self, dest: &str) -> Result<bool, ClientError> {
         let dest = format!("{}{}", self.addr, dest);
-        let response = self.client.delete(dest.as_str());
+
+        let response = self
+            .client
+            .delete(dest.as_str())
+            .header(CONTENT_TYPE, APPLICATION_JSON);
+        let response = if let Some(token) = &self.bearer_token {
+            response.bearer_auth(token)
+        } else {
+            response
+        };
+
+        let response = response.send().await.map_err(ClientError::Transport)?;
+
+        let opid = response
+            .headers()
+            .get(KOPID)
+            .and_then(|hv| hv.to_str().ok().map(|s| s.to_string()))
+            .unwrap_or_else(|| "missing_kopid".to_string());
+        debug!("opid -> {:?}", opid);
+
+        match response.status() {
+            reqwest::StatusCode::OK => {}
+            unexpect => {
+                return Err(ClientError::Http(
+                    unexpect,
+                    response.json().await.ok(),
+                    opid,
+                ))
+            }
+        }
+
+        response
+            .json()
+            .await
+            .map_err(|e| ClientError::JSONDecode(e, opid))
+    }
+    async fn perform_delete_request_with_body<R: Serialize>(
+        &self,
+        dest: &str,
+        request: R,
+    ) -> Result<bool, ClientError> {
+        let dest = format!("{}{}", self.addr, dest);
+
+        let req_string = serde_json::to_string(&request).map_err(ClientError::JSONEncode)?;
+        let response = self
+            .client
+            .delete(dest.as_str())
+            .body(req_string)
+            .header(CONTENT_TYPE, APPLICATION_JSON);
         let response = if let Some(token) = &self.bearer_token {
             response.bearer_auth(token)
         } else {
@@ -658,11 +705,28 @@ impl KanidmAsyncClient {
             .await
     }
 
-    /*
+    pub async fn idm_group_remove_members(
-    pub fn idm_group_remove_member(&self, id: &str, member: &str) -> Result<(), ClientError> {
+        &self,
-        unimplemented!();
+        group: &str,
+        members: &[&str],
+    ) -> Result<bool, ClientError> {
+        debug!(
+            "{}",
+            [
+                "Asked to remove members ",
+                &members.join(","),
+                " from ",
+                group
+            ]
+            .concat()
+            .to_string()
+        );
+        self.perform_delete_request_with_body(
+            ["/v1/group/", group, "/_attr/member"].concat().as_str(),
+            &members,
+        )
+        .await
     }
-    */
 
     pub async fn idm_group_purge_members(&self, id: &str) -> Result<bool, ClientError> {
         self.perform_delete_request(format!("/v1/group/{}/_attr/member", id).as_str())

kanidm_client/src/lib.rs

@@ -457,11 +457,13 @@ impl KanidmClient {
         tokio_block_on(self.asclient.idm_group_add_members(id, members))
     }
 
-    /*
+    pub fn idm_group_remove_members(
-    pub fn idm_group_remove_member(&self, id: &str, member: &str) -> Result<(), ClientError> {
+        &self,
-        unimplemented!();
+        group: &str,
+        members: &[&str],
+    ) -> Result<bool, ClientError> {
+        tokio_block_on(self.asclient.idm_group_remove_members(group, members))
     }
-    */
 
     pub fn idm_group_purge_members(&self, id: &str) -> Result<bool, ClientError> {
         tokio_block_on(self.asclient.idm_group_purge_members(id))

kanidm_client/tests/proto_v1_test.rs

@@ -286,13 +286,11 @@ fn test_server_rest_group_lifecycle() {
         );
 
         // Remove a member from the group
-        /*
         rsclient
-            .idm_group_remove_member("demo_group", "demo_group")
+            .idm_group_remove_members("demo_group", &["demo_group"])
             .unwrap();
         let members = rsclient.idm_group_get_members("demo_group").unwrap();
-        assert!(members == vec!["admin".to_string()]);
+        assert!(members == Some(vec!["admin@example.com".to_string()]));
-        */
 
         // purge members
         rsclient.idm_group_purge_members("demo_group").unwrap();

kanidm_tools/src/cli/group.rs

@@ -9,6 +9,7 @@ impl GroupOpt {
             GroupOpt::Delete(gcopt) => gcopt.copt.debug,
             GroupOpt::ListMembers(gcopt) => gcopt.copt.debug,
             GroupOpt::AddMembers(gcopt) => gcopt.copt.debug,
+            GroupOpt::RemoveMembers(gcopt) => gcopt.copt.debug,
             GroupOpt::SetMembers(gcopt) => gcopt.copt.debug,
             GroupOpt::PurgeMembers(gcopt) => gcopt.copt.debug,
             GroupOpt::Posix(gpopt) => match gpopt {
@@ -74,6 +75,18 @@ impl GroupOpt {
                     eprintln!("Error -> {:?}", e);
                 }
             }
+
+            GroupOpt::RemoveMembers(gcopt) => {
+                let client = gcopt.copt.to_client();
+                let remove_members: Vec<&str> = gcopt.members.iter().map(|s| s.as_str()).collect();
+
+                if let Err(e) =
+                    client.idm_group_remove_members(gcopt.name.as_str(), &remove_members)
+                {
+                    eprintln!("Failed to remove members -> {:?}", e);
+                }
+            }
+
             GroupOpt::SetMembers(gcopt) => {
                 let client = gcopt.copt.to_client();
                 let new_members: Vec<&str> = gcopt.members.iter().map(|s| s.as_str()).collect();

kanidm_tools/src/opt/kanidm.rs

@@ -64,6 +64,8 @@ pub enum GroupOpt {
     PurgeMembers(Named),
     #[structopt(name = "add_members")]
     AddMembers(GroupNamedMembers),
+    #[structopt(name = "remove_members")]
+    RemoveMembers(GroupNamedMembers),
     #[structopt(name = "posix")]
     Posix(GroupPosix),
 }

kanidmd/src/lib/actors/v1_write.rs

@@ -211,12 +211,12 @@ pub struct PurgeAttributeMessage {
     pub eventid: Uuid,
 }
 
-/// Delete a single attribute-value pair from the entry.
+/// Delete a set of attribute-value pair from the entry.
-pub struct RemoveAttributeValueMessage {
+pub struct RemoveAttributeValuesMessage {
     pub uat: Option<UserAuthToken>,
     pub uuid_or_name: String,
     pub attr: String,
-    pub value: String,
+    pub values: Vec<String>,
     pub filter: Filter<FilterInvalid>,
     pub eventid: Uuid,
 }
@@ -886,33 +886,46 @@ impl QueryServerWriteV1 {
         res
     }
 
-    pub async fn handle_removeattributevalue(
+    pub async fn handle_removeattributevalues(
         &self,
-        msg: RemoveAttributeValueMessage,
+        msg: RemoveAttributeValuesMessage,
     ) -> Result<(), OperationError> {
-        let mut audit = AuditScope::new("remove_attribute_value", msg.eventid, self.log_level);
+        let RemoveAttributeValuesMessage {
+            uat,
+            uuid_or_name,
+            attr,
+            values,
+            filter,
+            eventid,
+        } = msg;
+
+        let mut audit = AuditScope::new("remove_attribute_values", eventid, self.log_level);
         let idms_prox_write = self.idms.proxy_write_async(duration_from_epoch_now()).await;
         let res = lperf_op_segment!(
             &mut audit,
-            "actors::v1_write::handle<RemoveAttributeValueMessage>",
+            "actors::v1_write::handle<RemoveAttributeValuesMessage>",
             || {
                 let target_uuid = idms_prox_write
                     .qs_write
-                    .name_to_uuid(&mut audit, msg.uuid_or_name.as_str())
+                    .name_to_uuid(&mut audit, uuid_or_name.as_str())
                     .map_err(|e| {
                         ladmin_error!(audit, "Error resolving id to target");
                         e
                     })?;
 
-                let proto_ml =
+                let proto_ml = ProtoModifyList::new_list(
-                    ProtoModifyList::new_list(vec![ProtoModify::Removed(msg.attr, msg.value)]);
+                    values
+                        .into_iter()
+                        .map(|v| ProtoModify::Removed(attr.clone(), v))
+                        .collect(),
+                );
 
                 let mdf = match ModifyEvent::from_parts(
                     &mut audit,
-                    msg.uat.as_ref(),
+                    uat.as_ref(),
                     target_uuid,
                     &proto_ml,
-                    msg.filter,
+                    filter,
                     &idms_prox_write.qs_write,
                 ) {
                     Ok(m) => m,

kanidmd/src/lib/core/https.rs

@@ -11,7 +11,8 @@ use crate::actors::v1_write::{
     IdmAccountSetPasswordMessage, IdmAccountUnixExtendMessage, IdmAccountUnixSetCredMessage,
     IdmGroupUnixExtendMessage, InternalCredentialSetMessage, InternalDeleteMessage,
     InternalRegenerateRadiusMessage, InternalSshKeyCreateMessage, ModifyMessage,
-    PurgeAttributeMessage, RemoveAttributeValueMessage, ReviveRecycledMessage, SetAttributeMessage,
+    PurgeAttributeMessage, RemoveAttributeValuesMessage, ReviveRecycledMessage,
+    SetAttributeMessage,
 };
 use crate::config::TlsConfiguration;
 use crate::event::AuthResult;
@@ -360,7 +361,6 @@ async fn json_rest_event_post_id_attr(
     let id = req.get_url_param("id")?;
     let attr = req.get_url_param("attr")?;
     let values: Vec<String> = req.body_json().await?;
-
     let (eventid, hvalue) = new_eventid!();
     let m_obj = AppendAttributeMessage {
         uat,
@@ -408,17 +408,26 @@ async fn json_rest_event_put_id_attr(
 }
 
 async fn json_rest_event_delete_id_attr(
-    req: tide::Request<AppState>,
+    mut req: tide::Request<AppState>,
     filter: Filter<FilterInvalid>,
     // Seperate for account_delete_id_radius
     attr: String,
 ) -> tide::Result {
     let uat = req.get_current_uat();
     let id = req.get_url_param("id")?;
-
     let (eventid, hvalue) = new_eventid!();
+
     // TODO #211: Attempt to get an option Vec<String> here?
     // It's probably better to focus on SCIM instead, it seems richer than this.
+    let body = req.take_body();
+    let values: Vec<String> = if body.is_empty().unwrap_or(true) {
+        vec![]
+    } else {
+        // Must now be a valid list.
+        body.into_json().await?
+    };
+
+    if values.len() == 0 {
         let m_obj = PurgeAttributeMessage {
             uat,
             uuid_or_name: id,
@@ -433,6 +442,24 @@ async fn json_rest_event_delete_id_attr(
             .await
             .map(|()| true);
         to_tide_response(res, hvalue)
+    } else {
+        let obj = RemoveAttributeValuesMessage {
+            uat,
+            uuid_or_name: id,
+            attr,
+            values,
+            filter,
+            eventid,
+        };
+
+        let res = req
+            .state()
+            .qe_w_ref
+            .handle_removeattributevalues(obj)
+            .await
+            .map(|()| true);
+        to_tide_response(res, hvalue)
+    }
 }
 
 async fn json_rest_event_credential_put(
@@ -684,11 +711,11 @@ pub async fn account_delete_id_ssh_pubkey_tag(req: tide::Request<AppState>) -> t
     let tag = req.get_url_param("tag")?;
 
     let (eventid, hvalue) = new_eventid!();
-    let obj = RemoveAttributeValueMessage {
+    let obj = RemoveAttributeValuesMessage {
         uat,
         uuid_or_name: id,
         attr: "ssh_publickey".to_string(),
-        value: tag,
+        values: vec![tag],
         filter: filter_all!(f_eq("class", PartialValue::new_class("account"))),
         eventid,
     };
@@ -696,7 +723,7 @@ pub async fn account_delete_id_ssh_pubkey_tag(req: tide::Request<AppState>) -> t
     let res = req
         .state()
         .qe_w_ref
-        .handle_removeattributevalue(obj)
+        .handle_removeattributevalues(obj)
         .await
         .map(|()| true);
     to_tide_response(res, hvalue)
@@ -1449,10 +1476,10 @@ pub fn create_https_server(
         .delete(group_id_delete);
     group_route
         .at("/:id/_attr/:attr")
+        .delete(group_id_delete_attr)
         .get(group_id_get_attr)
         .put(group_id_put_attr)
-        .post(group_id_post_attr)
+        .post(group_id_post_attr);
-        .delete(group_id_delete_attr);
     group_route.at("/:id/_unix").post(group_post_id_unix);
     group_route
         .at("/:id/_unix/_token")