From 3430a1c31daa451350da6e2073bb3ea1a789406a Mon Sep 17 00:00:00 2001 From: Firstyear Date: Sat, 4 Jan 2025 13:09:48 +1000 Subject: [PATCH] Ignore anonymous in oauth2 read allow access (#3336) Administrators will sometimes configure oauth2 clients with `idm_all_accounts` as an allowed scope group. Despite anonymous being *unable* to interact with oauth2, this still allowed oauth2 clients to be read by anonymous in this configuration. For some users, this may be considered a public info disclosure. --- server/lib/src/server/access/mod.rs | 22 +++++++++++++++++----- server/lib/src/server/access/search.rs | 5 +++++ 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/server/lib/src/server/access/mod.rs b/server/lib/src/server/access/mod.rs index 212b18c98..e57222b40 100644 --- a/server/lib/src/server/access/mod.rs +++ b/server/lib/src/server/access/mod.rs @@ -2964,6 +2964,7 @@ mod tests { let r_set = vec![Arc::new(ev1.clone()), Arc::new(ev2)]; + // Check the authorisation search event, and that it reduces correctly. let se_a = SearchEvent::new_impersonate_entry( E_TEST_ACCOUNT_1.clone(), filter_all!(f_pres(Attribute::Name)), @@ -2971,17 +2972,28 @@ mod tests { let ex_a = vec![Arc::new(ev1)]; let ex_a_reduced = vec![ev1_reduced]; + test_acp_search!(&se_a, vec![], r_set.clone(), ex_a); + test_acp_search_reduce!(&se_a, vec![], r_set.clone(), ex_a_reduced); + + // Check that anonymous is denied even though it's a member of the group. + let anon: EntryInitNew = BUILTIN_ACCOUNT_ANONYMOUS_DL6.clone().into(); + let mut anon = anon.into_invalid_new(); + anon.set_ava_set(&Attribute::MemberOf, ValueSetRefer::new(UUID_TEST_GROUP_1)); + + let anon = Arc::new(anon.into_sealed_committed()); + + let se_anon = + SearchEvent::new_impersonate_entry(anon, filter_all!(f_pres(Attribute::Name))); + let ex_anon = vec![]; + test_acp_search!(&se_anon, vec![], r_set.clone(), ex_anon); + + // Check the deny case. let se_b = SearchEvent::new_impersonate_entry( E_TEST_ACCOUNT_2.clone(), filter_all!(f_pres(Attribute::Name)), ); let ex_b = vec![]; - // Check the authorisation search event, and that it reduces correctly. - test_acp_search!(&se_a, vec![], r_set.clone(), ex_a); - test_acp_search_reduce!(&se_a, vec![], r_set.clone(), ex_a_reduced); - - // Check the deny case. test_acp_search!(&se_b, vec![], r_set, ex_b); } diff --git a/server/lib/src/server/access/search.rs b/server/lib/src/server/access/search.rs index 023cc099c..96e30a86b 100644 --- a/server/lib/src/server/access/search.rs +++ b/server/lib/src/server/access/search.rs @@ -168,6 +168,11 @@ fn search_oauth2_filter_entry(ident: &Identity, entry: &Arc AccessResult::Ignore, IdentType::User(iuser) => { + if iuser.entry.get_uuid() == UUID_ANONYMOUS { + debug!("Anonymous can't access OAuth2 entries, ignoring"); + return AccessResult::Ignore; + } + let contains_o2_rs = entry .get_ava_as_iutf8(Attribute::Class) .map(|set| {