From 20751254392d19616bfb62de73873071f4862ab5 Mon Sep 17 00:00:00 2001 From: Firstyear Date: Tue, 15 Oct 2024 14:29:45 +1000 Subject: [PATCH] Working scim entry get for person (#3088) --- Cargo.lock | 2 + Cargo.toml | 1 + book/src/support.md | 4 +- libs/client/Cargo.toml | 1 + libs/client/src/lib.rs | 26 +++++- libs/client/src/scim.rs | 15 +++- proto/Cargo.toml | 1 + proto/src/attribute.rs | 22 +++-- proto/src/scim_v1/client.rs | 10 +++ proto/src/scim_v1/mod.rs | 47 +++++++++-- server/core/src/actors/v1_scim.rs | 16 ++-- server/core/src/https/apidocs/mod.rs | 1 + server/core/src/https/v1_scim.rs | 51 +++++++++++- server/lib/src/entry.rs | 38 +-------- server/lib/src/server/mod.rs | 115 ++++++++++++++++++++++----- server/testkit/tests/scim_test.rs | 58 +++++++++++++- tools/cli/src/cli/person.rs | 32 ++++++-- 17 files changed, 347 insertions(+), 93 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9d44f1974..c69dc064b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3265,6 +3265,7 @@ dependencies = [ "reqwest", "serde", "serde_json", + "serde_urlencoded", "time", "tokio", "toml", @@ -3314,6 +3315,7 @@ dependencies = [ "scim_proto", "serde", "serde_json", + "serde_urlencoded", "serde_with", "smartstring", "sshkey-attest", diff --git a/Cargo.toml b/Cargo.toml index 0360e6af9..5f76049b2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -255,6 +255,7 @@ selinux = "^0.4.6" serde = "^1.0.210" serde_cbor = { version = "0.12.0-dev", package = "serde_cbor_2" } serde_json = "^1.0.128" +serde_urlencoded = "^0.7.1" serde-wasm-bindgen = "0.5" serde_with = "3.11.0" sha-crypt = "0.5.0" diff --git a/book/src/support.md b/book/src/support.md index edbbb0b8d..fc10a9592 100644 --- a/book/src/support.md +++ b/book/src/support.md @@ -94,8 +94,6 @@ Stable APIs are: - LDAP protocol operations - JSON HTTP end points which use elements from [`proto/src/v1`](https://github.com/kanidm/kanidm/blob/master/proto/src/v1) -- SCIM operations from - [`proto/src/scim_v1`](https://github.com/kanidm/kanidm/blob/master/proto/src/scim_v1) All other APIs and interactions are not considered stable. Changes will be minimised if possible. This includes but is not limited to: @@ -107,6 +105,8 @@ This includes but is not limited to: - CLI interface of any command provided by kanidm unless otherwise noted above - JSON HTTP end points which use elements from [`proto/src/internal.rs`](https://github.com/kanidm/kanidm/blob/master/proto/src/internal.rs) +- SCIM operations from + [`proto/src/scim_v1`](https://github.com/kanidm/kanidm/blob/master/proto/src/scim_v1) ### Deprecation Policy diff --git a/libs/client/Cargo.toml b/libs/client/Cargo.toml index 71fb600af..f3230292e 100644 --- a/libs/client/Cargo.toml +++ b/libs/client/Cargo.toml @@ -28,6 +28,7 @@ http = { workspace = true } hyper = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } +serde_urlencoded = { workspace = true } time = { workspace = true, features = ["serde", "std"] } tokio = { workspace = true, features = [ "rt", diff --git a/libs/client/src/lib.rs b/libs/client/src/lib.rs index 7db9c1a90..3175202a2 100644 --- a/libs/client/src/lib.rs +++ b/libs/client/src/lib.rs @@ -14,7 +14,7 @@ extern crate tracing; use std::collections::{BTreeMap, BTreeSet as Set}; -use std::fmt::{Display, Formatter}; +use std::fmt::{Debug, Display, Formatter}; use std::fs::File; #[cfg(target_family = "unix")] // not needed for windows builds use std::fs::{metadata, Metadata}; @@ -41,6 +41,7 @@ pub use reqwest::StatusCode; use serde::de::DeserializeOwned; use serde::{Deserialize, Serialize}; use serde_json::error::Error as SerdeJsonError; +use serde_urlencoded::ser::Error as UrlEncodeError; use tokio::sync::{Mutex, RwLock}; use url::Url; use uuid::Uuid; @@ -72,6 +73,7 @@ pub enum ClientError { JsonDecode(reqwest::Error, String), InvalidResponseFormat(String), JsonEncode(SerdeJsonError), + UrlEncode(UrlEncodeError), SystemError, ConfigParseIssue(String), CertParseIssue(String), @@ -1003,7 +1005,27 @@ impl KanidmClient { &self, dest: &str, ) -> Result { - let response = self.client.get(self.make_url(dest)); + let query: Option<()> = None; + self.perform_get_request_query(dest, query).await + } + + #[instrument(level = "debug", skip(self))] + pub async fn perform_get_request_query( + &self, + dest: &str, + query: Option, + ) -> Result { + let mut dest_url = self.make_url(dest); + + if let Some(query) = query { + let txt = serde_urlencoded::to_string(&query).map_err(ClientError::UrlEncode)?; + + if !txt.is_empty() { + dest_url.set_query(Some(txt.as_str())); + } + } + + let response = self.client.get(dest_url); let response = { let tguard = self.bearer_token.read().await; if let Some(token) = &(*tguard) { diff --git a/libs/client/src/scim.rs b/libs/client/src/scim.rs index d3607382a..922ad05d8 100644 --- a/libs/client/src/scim.rs +++ b/libs/client/src/scim.rs @@ -1,5 +1,5 @@ use crate::{ClientError, KanidmClient}; -use kanidm_proto::scim_v1::{ScimEntryGeneric, ScimSyncRequest, ScimSyncState}; +use kanidm_proto::scim_v1::{ScimEntryGeneric, ScimEntryGetQuery, ScimSyncRequest, ScimSyncState}; impl KanidmClient { // TODO: testing for this @@ -21,8 +21,19 @@ impl KanidmClient { pub async fn scim_v1_entry_get( &self, name_or_uuid: &str, + query: Option, ) -> Result { - self.perform_get_request(format!("/scim/v1/Entry/{}", name_or_uuid).as_str()) + self.perform_get_request_query(format!("/scim/v1/Entry/{}", name_or_uuid).as_str(), query) + .await + } + + /// Retrieve a Person as a SCIM JSON Value. + pub async fn scim_v1_person_get( + &self, + name_or_uuid: &str, + query: Option, + ) -> Result { + self.perform_get_request_query(format!("/scim/v1/Person/{}", name_or_uuid).as_str(), query) .await } } diff --git a/proto/Cargo.toml b/proto/Cargo.toml index cff492cf3..bc12d0a92 100644 --- a/proto/Cargo.toml +++ b/proto/Cargo.toml @@ -26,6 +26,7 @@ num_enum = { workspace = true } scim_proto = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } +serde_urlencoded = { workspace = true } serde_with = { workspace = true, features = ["time_0_3", "base64", "hex"] } smartstring = { workspace = true, features = ["serde"] } time = { workspace = true, features = ["serde", "std"] } diff --git a/proto/src/attribute.rs b/proto/src/attribute.rs index 282b97ced..5e43b249b 100644 --- a/proto/src/attribute.rs +++ b/proto/src/attribute.rs @@ -3,7 +3,9 @@ use utoipa::ToSchema; use crate::constants::*; use crate::internal::OperationError; +use std::convert::Infallible; use std::fmt; +use std::str::FromStr; pub use smartstring::alias::String as AttrString; @@ -205,13 +207,13 @@ impl TryFrom<&AttrString> for Attribute { type Error = OperationError; fn try_from(value: &AttrString) -> Result { - Ok(Attribute::from_str(value.as_str())) + Ok(Attribute::inner_from_str(value.as_str())) } } impl From<&str> for Attribute { fn from(value: &str) -> Self { - Self::from_str(value) + Self::inner_from_str(value) } } @@ -227,6 +229,14 @@ impl From for AttrString { } } +impl FromStr for Attribute { + type Err = Infallible; + + fn from_str(value: &str) -> Result { + Ok(Self::inner_from_str(value)) + } +} + impl Attribute { pub fn as_str(&self) -> &str { match self { @@ -406,7 +416,7 @@ impl Attribute { // We allow this because the standard lib from_str is fallible, and we want an infallible version. #[allow(clippy::should_implement_trait)] - pub fn from_str(value: &str) -> Self { + fn inner_from_str(value: &str) -> Self { // Could this be something like heapless to save allocations? Also gives a way // to limit length of str? match value.to_lowercase().as_str() { @@ -603,9 +613,9 @@ mod test { #[test] fn test_valueattribute_from_str() { - assert_eq!(Attribute::Uuid, Attribute::from_str("UUID")); - assert_eq!(Attribute::Uuid, Attribute::from_str("UuiD")); - assert_eq!(Attribute::Uuid, Attribute::from_str("uuid")); + assert_eq!(Attribute::Uuid, Attribute::from("UUID")); + assert_eq!(Attribute::Uuid, Attribute::from("UuiD")); + assert_eq!(Attribute::Uuid, Attribute::from("uuid")); } #[test] diff --git a/proto/src/scim_v1/client.rs b/proto/src/scim_v1/client.rs index 8b1378917..397f25b75 100644 --- a/proto/src/scim_v1/client.rs +++ b/proto/src/scim_v1/client.rs @@ -1 +1,11 @@ +use serde::{Deserialize, Serialize}; +use sshkey_attest::proto::PublicKey as SshPublicKey; +pub type ScimSshPublicKeys = Vec; + +#[derive(Deserialize, Serialize, Debug, Clone)] +#[serde(rename_all = "camelCase")] +pub struct ScimSshPublicKey { + pub label: String, + pub value: SshPublicKey, +} diff --git a/proto/src/scim_v1/mod.rs b/proto/src/scim_v1/mod.rs index f868cb2c7..0b912daec 100644 --- a/proto/src/scim_v1/mod.rs +++ b/proto/src/scim_v1/mod.rs @@ -22,10 +22,13 @@ use serde_json::Value as JsonValue; use std::collections::BTreeMap; use utoipa::ToSchema; +use serde_with::formats::CommaSeparator; +use serde_with::{serde_as, skip_serializing_none, StringWithSeparator}; + pub use self::synch::*; pub use scim_proto::prelude::*; -mod client; +pub mod client; pub mod server; mod synch; @@ -40,28 +43,37 @@ pub struct ScimEntryGeneric { pub attrs: BTreeMap, } +/// SCIM Query Parameters used during the get of a single entry +#[serde_as] +#[skip_serializing_none] +#[derive(Serialize, Deserialize, Debug, Default)] +pub struct ScimEntryGetQuery { + #[serde_as(as = "Option>")] + pub attributes: Option>, +} + #[cfg(test)] mod tests { // use super::*; #[test] - fn test_scim_rfc_to_generic() { + fn scim_rfc_to_generic() { // Assert that we can transition from the rfc generic entries to the // kanidm types. } #[test] - fn test_scim_kani_to_generic() { + fn scim_kani_to_generic() { // Assert that a kanidm strong entry can convert to generic. } #[test] - fn test_scim_kani_to_rfc() { + fn scim_kani_to_rfc() { // Assert that a kanidm strong entry can convert to rfc. } #[test] - fn test_scim_sync_kani_to_rfc() { + fn scim_sync_kani_to_rfc() { use super::*; // Group @@ -114,4 +126,29 @@ mod tests { assert!(entry.is_ok()); } + + #[test] + fn scim_entry_get_query() { + use super::*; + + let q = ScimEntryGetQuery { attributes: None }; + + let txt = serde_urlencoded::to_string(&q).unwrap(); + + assert_eq!(txt, ""); + + let q = ScimEntryGetQuery { + attributes: Some(vec![Attribute::Name]), + }; + + let txt = serde_urlencoded::to_string(&q).unwrap(); + assert_eq!(txt, "attributes=name"); + + let q = ScimEntryGetQuery { + attributes: Some(vec![Attribute::Name, Attribute::Spn]), + }; + + let txt = serde_urlencoded::to_string(&q).unwrap(); + assert_eq!(txt, "attributes=name%2Cspn"); + } } diff --git a/server/core/src/actors/v1_scim.rs b/server/core/src/actors/v1_scim.rs index d03e19ed3..723869c45 100644 --- a/server/core/src/actors/v1_scim.rs +++ b/server/core/src/actors/v1_scim.rs @@ -1,13 +1,12 @@ -use kanidmd_lib::prelude::*; - +use super::{QueryServerReadV1, QueryServerWriteV1}; +use kanidm_proto::scim_v1::{ + server::ScimEntryKanidm, ScimEntryGetQuery, ScimSyncRequest, ScimSyncState, +}; use kanidmd_lib::idm::scim::{ GenerateScimSyncTokenEvent, ScimSyncFinaliseEvent, ScimSyncTerminateEvent, ScimSyncUpdateEvent, }; use kanidmd_lib::idm::server::IdmServerTransaction; - -use kanidm_proto::scim_v1::{server::ScimEntryKanidm, ScimSyncRequest, ScimSyncState}; - -use super::{QueryServerReadV1, QueryServerWriteV1}; +use kanidmd_lib::prelude::*; impl QueryServerWriteV1 { #[instrument( @@ -208,6 +207,8 @@ impl QueryServerReadV1 { client_auth_info: ClientAuthInfo, eventid: Uuid, uuid_or_name: String, + class: EntryClass, + query: ScimEntryGetQuery, ) -> Result { let ct = duration_from_epoch_now(); let mut idms_prox_read = self.idms.proxy_read().await?; @@ -226,7 +227,6 @@ impl QueryServerReadV1 { idms_prox_read .qs_read - .impersonate_search_ext_uuid(target_uuid, &ident) - .and_then(|entry| entry.to_scim_kanidm(idms_prox_read.qs_read)) + .scim_entry_id_get_ext(target_uuid, class, query, ident) } } diff --git a/server/core/src/https/apidocs/mod.rs b/server/core/src/https/apidocs/mod.rs index ea77d8d09..177a6b39d 100644 --- a/server/core/src/https/apidocs/mod.rs +++ b/server/core/src/https/apidocs/mod.rs @@ -77,6 +77,7 @@ impl Modify for SecurityAddon { super::v1_scim::scim_sync_post, super::v1_scim::scim_sync_get, super::v1_scim::scim_entry_id_get, + super::v1_scim::scim_person_id_get, super::v1::schema_get, super::v1::whoami, diff --git a/server/core/src/https/v1_scim.rs b/server/core/src/https/v1_scim.rs index 1573cebe2..bbae59d33 100644 --- a/server/core/src/https/v1_scim.rs +++ b/server/core/src/https/v1_scim.rs @@ -7,11 +7,13 @@ use super::v1::{ }; use super::ServerState; use crate::https::extractors::VerifiedClientInformation; -use axum::extract::{Path, State}; +use axum::extract::{Path, Query, State}; use axum::response::Html; use axum::routing::{get, post}; use axum::{Extension, Json, Router}; -use kanidm_proto::scim_v1::{server::ScimEntryKanidm, ScimSyncRequest, ScimSyncState}; +use kanidm_proto::scim_v1::{ + server::ScimEntryKanidm, ScimEntryGetQuery, ScimSyncRequest, ScimSyncState, +}; use kanidm_proto::v1::Entry as ProtoEntry; use kanidmd_lib::prelude::*; @@ -320,10 +322,49 @@ async fn scim_entry_id_get( Path(id): Path, Extension(kopid): Extension, VerifiedClientInformation(client_auth_info): VerifiedClientInformation, + Query(scim_entry_get_query): Query, ) -> Result, WebError> { state .qe_r_ref - .scim_entry_id_get(client_auth_info, kopid.eventid, id) + .scim_entry_id_get( + client_auth_info, + kopid.eventid, + id, + EntryClass::Object, + scim_entry_get_query, + ) + .await + .map(Json::from) + .map_err(WebError::from) +} + +#[utoipa::path( + get, + path = "/scim/v1/Person/{id}", + responses( + (status = 200, content_type="application/json", body=ScimEntry), + ApiResponseWithout200, + ), + security(("token_jwt" = [])), + tag = "scim", + operation_id = "scim_entry_id_get" +)] +async fn scim_person_id_get( + State(state): State, + Path(id): Path, + Extension(kopid): Extension, + VerifiedClientInformation(client_auth_info): VerifiedClientInformation, + Query(scim_entry_get_query): Query, +) -> Result, WebError> { + state + .qe_r_ref + .scim_entry_id_get( + client_auth_info, + kopid.eventid, + id, + EntryClass::Person, + scim_entry_get_query, + ) .await .map(Json::from) .map_err(WebError::from) @@ -420,6 +461,10 @@ pub fn route_setup() -> Router { // of any kind from the database. // {id} is any unique id. .route("/scim/v1/Entry/:id", get(scim_entry_id_get)) + // Person /Person/{id} GET Retrieve a a person from the + // database. + // {id} is any unique id. + .route("/scim/v1/Person/:id", get(scim_person_id_get)) // // Sync /Sync GET Retrieve the current // sync state associated diff --git a/server/lib/src/entry.rs b/server/lib/src/entry.rs index 3161d97df..58caa8458 100644 --- a/server/lib/src/entry.rs +++ b/server/lib/src/entry.rs @@ -47,7 +47,6 @@ use kanidm_proto::internal::ImageValue; use kanidm_proto::internal::{ ConsistencyError, Filter as ProtoFilter, OperationError, SchemaError, UiHint, }; -use kanidm_proto::scim_v1::server::ScimReference; use kanidm_proto::v1::Entry as ProtoEntry; use ldap3_proto::simple::{LdapPartialAttribute, LdapSearchResultEntry}; use openssl::ec::EcKey; @@ -64,7 +63,7 @@ use crate::value::{ ApiToken, CredentialType, IndexType, IntentTokenState, Oauth2Session, PartialValue, Session, SyntaxType, Value, }; -use crate::valueset::{self, ScimResolveStatus, ScimValueIntermediate, ValueSet}; +use crate::valueset::{self, ScimResolveStatus, ValueSet}; pub type EntryInitNew = Entry; pub type EntryInvalidNew = Entry; @@ -2232,7 +2231,7 @@ impl Entry { pub fn to_scim_kanidm( &self, - mut read_txn: QueryServerReadTransaction, + read_txn: &mut QueryServerReadTransaction, ) -> Result { let result: Result, OperationError> = self .attrs @@ -2245,7 +2244,7 @@ impl Entry { None => Ok(None), Some(ScimResolveStatus::Resolved(scim_value_kani)) => Ok(Some(scim_value_kani)), Some(ScimResolveStatus::NeedsResolution(scim_value_interim)) => { - resolve_scim_interim(scim_value_interim, &mut read_txn) + read_txn.resolve_scim_interim(scim_value_interim) } }; res_opt_scim_value @@ -2370,37 +2369,6 @@ impl Entry { } } -fn resolve_scim_interim( - scim_value_intermediate: ScimValueIntermediate, - read_txn: &mut QueryServerReadTransaction, -) -> Result, OperationError> { - match scim_value_intermediate { - ScimValueIntermediate::Refer(uuid) => { - if let Some(option) = read_txn.uuid_to_spn(uuid)? { - Ok(Some(ScimValueKanidm::EntryReference(ScimReference { - uuid, - value: option.to_proto_string_clone(), - }))) - } else { - // TODO: didn't have spn, fallback to uuid.to_string ? - Ok(None) - } - } - ScimValueIntermediate::ReferMany(uuids) => { - let mut scim_references = vec![]; - for uuid in uuids { - if let Some(option) = read_txn.uuid_to_spn(uuid)? { - scim_references.push(ScimReference { - uuid, - value: option.to_proto_string_clone(), - }) - } - } - Ok(Some(ScimValueKanidm::EntryReferences(scim_references))) - } - } -} - // impl Entry { impl Entry { /// This internally adds an AVA to the entry. If the entry was newly added, then true is returned. diff --git a/server/lib/src/server/mod.rs b/server/lib/src/server/mod.rs index 513b86e5e..0aa61ba5b 100644 --- a/server/lib/src/server/mod.rs +++ b/server/lib/src/server/mod.rs @@ -1,20 +1,30 @@ //! `server` contains the query server, which is the main high level construction //! to coordinate queries and operations in the server. -use std::str::FromStr; -use std::sync::Arc; - +use crate::be::{Backend, BackendReadTransaction, BackendTransaction, BackendWriteTransaction}; use concread::arcache::{ARCacheBuilder, ARCacheReadTxn}; use concread::cowcell::*; use hashbrown::{HashMap, HashSet}; +use kanidm_proto::internal::{DomainInfo as ProtoDomainInfo, ImageValue, UiHint}; +use kanidm_proto::scim_v1::server::ScimReference; +use kanidm_proto::scim_v1::ScimEntryGetQuery; use std::collections::BTreeSet; +use std::str::FromStr; +use std::sync::Arc; use tokio::sync::{Semaphore, SemaphorePermit}; use tracing::trace; - -use kanidm_proto::internal::{DomainInfo as ProtoDomainInfo, ImageValue, UiHint}; - -use crate::be::{Backend, BackendReadTransaction, BackendTransaction, BackendWriteTransaction}; // We use so many, we just import them all ... +use self::access::{ + profiles::{ + AccessControlCreate, AccessControlDelete, AccessControlModify, AccessControlSearch, + }, + AccessControls, AccessControlsReadTransaction, AccessControlsTransaction, + AccessControlsWriteTransaction, +}; +use self::keys::{ + KeyObject, KeyProvider, KeyProviders, KeyProvidersReadTransaction, KeyProvidersTransaction, + KeyProvidersWriteTransaction, +}; use crate::filter::{ Filter, FilterInvalid, FilterValid, FilterValidResolved, ResolveFilterCache, ResolveFilterCacheReadTxn, @@ -31,19 +41,7 @@ use crate::schema::{ }; use crate::value::{CredentialType, EXTRACT_VAL_DN}; use crate::valueset::uuid_to_proto_string; - -use self::access::{ - profiles::{ - AccessControlCreate, AccessControlDelete, AccessControlModify, AccessControlSearch, - }, - AccessControls, AccessControlsReadTransaction, AccessControlsTransaction, - AccessControlsWriteTransaction, -}; - -use self::keys::{ - KeyObject, KeyProvider, KeyProviders, KeyProvidersReadTransaction, KeyProvidersTransaction, - KeyProvidersWriteTransaction, -}; +use crate::valueset::ScimValueIntermediate; pub(crate) mod access; pub mod batch_modify; @@ -838,6 +836,37 @@ pub trait QueryServerTransaction<'a> { } } + fn resolve_scim_interim( + &mut self, + scim_value_intermediate: ScimValueIntermediate, + ) -> Result, OperationError> { + match scim_value_intermediate { + ScimValueIntermediate::Refer(uuid) => { + if let Some(option) = self.uuid_to_spn(uuid)? { + Ok(Some(ScimValueKanidm::EntryReference(ScimReference { + uuid, + value: option.to_proto_string_clone(), + }))) + } else { + // TODO: didn't have spn, fallback to uuid.to_string ? + Ok(None) + } + } + ScimValueIntermediate::ReferMany(uuids) => { + let mut scim_references = vec![]; + for uuid in uuids { + if let Some(option) = self.uuid_to_spn(uuid)? { + scim_references.push(ScimReference { + uuid, + value: option.to_proto_string_clone(), + }) + } + } + Ok(Some(ScimValueKanidm::EntryReferences(scim_references))) + } + } + } + // In the opposite direction, we can resolve values for presentation fn resolve_valueset(&mut self, value: &ValueSet) -> Result, OperationError> { if let Some(r_set) = value.as_refer_set() { @@ -1206,6 +1235,50 @@ impl<'a> QueryServerReadTransaction<'a> { results } + + #[instrument(level = "debug", skip_all)] + pub fn scim_entry_id_get_ext( + &mut self, + uuid: Uuid, + class: EntryClass, + query: ScimEntryGetQuery, + ident: Identity, + ) -> Result { + let filter_intent = filter!(f_and!([ + f_eq(Attribute::Uuid, PartialValue::Uuid(uuid)), + f_eq(Attribute::Class, class.into()) + ])); + + let f_intent_valid = filter_intent + .validate(self.get_schema()) + .map_err(OperationError::SchemaViolation)?; + + let f_valid = f_intent_valid.clone().into_ignore_hidden(); + + let r_attrs = query + .attributes + .map(|attr_set| attr_set.into_iter().collect()); + + let se = SearchEvent { + ident, + filter: f_valid, + filter_orig: f_intent_valid, + attrs: r_attrs, + }; + + let mut vs = self.search_ext(&se)?; + match vs.pop() { + Some(entry) if vs.is_empty() => entry.to_scim_kanidm(self), + _ => { + if vs.is_empty() { + Err(OperationError::NoMatchingEntries) + } else { + // Multiple entries matched, should not be possible! + Err(OperationError::UniqueConstraintViolation) + } + } + } + } } impl<'a> QueryServerTransaction<'a> for QueryServerWriteTransaction<'a> { @@ -2626,7 +2699,7 @@ mod tests { // Convert entry into scim let reduced = entry.as_ref().clone().into_reduced(); - let scim_entry = reduced.to_scim_kanidm(read_txn).unwrap(); + let scim_entry = reduced.to_scim_kanidm(&mut read_txn).unwrap(); // Assert scim entry attributes are as expected assert_eq!(scim_entry.header.id, UUID_IDM_PEOPLE_SELF_NAME_WRITE); diff --git a/server/testkit/tests/scim_test.rs b/server/testkit/tests/scim_test.rs index 37d8ae518..9ee7c2d98 100644 --- a/server/testkit/tests/scim_test.rs +++ b/server/testkit/tests/scim_test.rs @@ -1,6 +1,7 @@ use compact_jwt::{traits::JwsVerifiable, JwsCompact, JwsEs256Verifier, JwsVerifier}; use kanidm_client::KanidmClient; use kanidm_proto::internal::ScimSyncToken; +use kanidm_proto::scim_v1::ScimEntryGetQuery; use kanidmd_lib::prelude::{Attribute, BUILTIN_GROUP_IDM_ADMINS_V1}; use kanidmd_testkit::{ADMIN_TEST_PASSWORD, ADMIN_TEST_USER}; use reqwest::header::HeaderValue; @@ -180,7 +181,10 @@ async fn test_scim_sync_entry_get(rsclient: KanidmClient) { // This will be as raw json, not the strongly typed version the server sees // internally. - let scim_entry = rsclient.scim_v1_entry_get("demo_account").await.unwrap(); + let scim_entry = rsclient + .scim_v1_entry_get("demo_account", None) + .await + .unwrap(); tracing::info!("{:#?}", scim_entry); @@ -194,4 +198,56 @@ async fn test_scim_sync_entry_get(rsclient: KanidmClient) { .unwrap(), "demo_account".to_string() ); + + // Limit the attributes we want. + let query = ScimEntryGetQuery { + attributes: Some(vec![Attribute::Name]), + }; + + let scim_entry = rsclient + .scim_v1_entry_get("demo_account", Some(query)) + .await + .unwrap(); + + tracing::info!("{:#?}", scim_entry); + + // Should not be present now. + assert!(!scim_entry.attrs.contains_key(&Attribute::Class)); + assert!(scim_entry.attrs.contains_key(&Attribute::Name)); + + // ========================================== + // Same, but via the Person API + let scim_entry = rsclient + .scim_v1_person_get("demo_account", None) + .await + .unwrap(); + + tracing::info!("{:#?}", scim_entry); + + assert!(scim_entry.attrs.contains_key(&Attribute::Class)); + assert!(scim_entry.attrs.contains_key(&Attribute::Name)); + assert_eq!( + scim_entry + .attrs + .get(&Attribute::Name) + .and_then(|v| v.as_str()) + .unwrap(), + "demo_account".to_string() + ); + + // Limit the attributes we want. + let query = ScimEntryGetQuery { + attributes: Some(vec![Attribute::Name]), + }; + + let scim_entry = rsclient + .scim_v1_person_get("demo_account", Some(query)) + .await + .unwrap(); + + tracing::info!("{:#?}", scim_entry); + + // Should not be present now. + assert!(!scim_entry.attrs.contains_key(&Attribute::Class)); + assert!(scim_entry.attrs.contains_key(&Attribute::Name)); } diff --git a/tools/cli/src/cli/person.rs b/tools/cli/src/cli/person.rs index fb2006937..52a57f9a0 100644 --- a/tools/cli/src/cli/person.rs +++ b/tools/cli/src/cli/person.rs @@ -6,9 +6,8 @@ use dialoguer::theme::ColorfulTheme; use dialoguer::{Confirm, Input, Password, Select}; use kanidm_client::ClientError::Http as ClientErrorHttp; use kanidm_client::KanidmClient; -use kanidm_proto::constants::{ - ATTR_ACCOUNT_EXPIRE, ATTR_ACCOUNT_VALID_FROM, ATTR_GIDNUMBER, ATTR_SSH_PUBLICKEY, -}; +use kanidm_proto::attribute::Attribute; +use kanidm_proto::constants::{ATTR_ACCOUNT_EXPIRE, ATTR_ACCOUNT_VALID_FROM, ATTR_GIDNUMBER}; use kanidm_proto::internal::OperationError::{ DuplicateKey, DuplicateLabel, InvalidLabel, NoMatchingEntries, PasswordQuality, }; @@ -18,6 +17,7 @@ use kanidm_proto::internal::{ }; use kanidm_proto::internal::{CredentialDetail, CredentialDetailType}; use kanidm_proto::messages::{AccountChangeMessage, ConsoleOutputMode, MessageStatus}; +use kanidm_proto::scim_v1::{client::ScimSshPublicKeys, ScimEntryGetQuery}; use qrcode::render::unicode; use qrcode::QrCode; use time::format_description::well_known::Rfc3339; @@ -233,15 +233,31 @@ impl PersonOpt { AccountSsh::List(aopt) => { let client = aopt.copt.to_client(OpType::Read).await; - match client - .idm_person_account_get_attr( + let mut entry = match client + .scim_v1_person_get( aopt.aopts.account_id.as_str(), - ATTR_SSH_PUBLICKEY, + Some(ScimEntryGetQuery { + attributes: Some(vec![Attribute::SshPublicKey]), + }), ) .await { - Ok(pkeys) => pkeys.iter().flatten().for_each(|pkey| println!("{}", pkey)), - Err(e) => handle_client_error(e, aopt.copt.output_mode), + Ok(entry) => entry, + Err(e) => return handle_client_error(e, aopt.copt.output_mode), + }; + + let Some(pkeys) = entry.attrs.remove(&Attribute::SshPublicKey) else { + println!("No ssh public keys"); + return; + }; + + let Ok(keys) = serde_json::from_value::(pkeys) else { + eprintln!("Invalid ssh public key format"); + return; + }; + + for key in keys { + println!("{}: {}", key.label, key.value); } } AccountSsh::Add(aopt) => {