967 oauth2 implicit search (#1382)

This commit is contained in:
Firstyear 2023-02-16 09:40:16 +10:00 committed by GitHub
parent 8bedb55548
commit a02337a07a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 221 additions and 19 deletions

View file

@ -1270,6 +1270,9 @@ pub const JSON_IDM_HP_ACP_SERVICE_ACCOUNT_INTO_PERSON_MIGRATE_V1: &str = r#"{
}
}"#;
/*
// Removed in favour of a dynamic check inside of access.rs that is based on membership to an
// oauth2 rs.
pub const JSON_IDM_ACP_OAUTH2_READ_PRIV_V1: &str = r#"{
"attrs": {
"class": [
@ -1294,6 +1297,7 @@ pub const JSON_IDM_ACP_OAUTH2_READ_PRIV_V1: &str = r#"{
]
}
}"#;
*/
pub const JSON_IDM_HP_ACP_SYNC_ACCOUNT_MANAGE_PRIV_V1: &str = r#"{
"attrs": {

View file

@ -300,7 +300,7 @@ pub const _UUID_IDM_PEOPLE_SELF_ACP_WRITE_MAIL_V1: Uuid =
uuid!("00000000-0000-0000-0000-ffffff000041");
pub const _UUID_IDM_HP_ACP_SERVICE_ACCOUNT_INTO_PERSON_MIGRATE_V1: Uuid =
uuid!("00000000-0000-0000-0000-ffffff000042");
pub const _UUID_IDM_ACP_OAUTH2_READ_PRIV_V1: Uuid = uuid!("00000000-0000-0000-0000-ffffff000043");
pub const UUID_IDM_ACP_OAUTH2_READ_PRIV_V1: Uuid = uuid!("00000000-0000-0000-0000-ffffff000043");
pub const _UUID_IDM_HP_ACP_SYNC_ACCOUNT_MANAGE_PRIV_V1: Uuid =
uuid!("00000000-0000-0000-0000-ffffff000044");
pub const UUID_IDM_ACP_ACCOUNT_MAIL_READ_PRIV_V1: Uuid =

View file

@ -13,30 +13,26 @@ impl<'a> IdmServerProxyReadTransaction<'a> {
}
};
// Do an internal search
// ⚠️ Safety Notes - We perform an internal search here which bypasses
// access controls. Why? Users normally can't read the oauth2_rs_scope_maps
// since that could (?) disclose access rules. It's probably not a risk, but
// we just don't show them by default.
// Formerly we did an internal search here, but we no longer need to since we have
// the access control module setup so that we can search for and see rs that we
// have access to.
//
// This IS safe because we control *all* inputs (the uuids and memberof) and
// they come from the cryptographically verified UAT. we also control all
// outputs and ONLY output data that IS visible by default for an oauth2
// resource server.
//
// This is kind of a limitation of the kani search system, where the ability to
// compare an attribute, also allows you to read it. In this case we want compare
// without read, but it's not really possible, and it's a silly concept generally
// anyway because publicly allowing that allows retrieval of the values to bruteforce.
let f = filter!(f_or(
// We do this weird looking f_executed/f_intent shenanigans to actually search
// on what we have access to, but we apply access as though we did a search on
// class=oauth2_resource_server instead, and we still apply access here.
let f_executed = filter!(f_or(
ident_mo
.iter()
.copied()
.map(|uuid| { f_eq("oauth2_rs_scope_map", PartialValue::Refer(uuid)) })
.collect()
));
let f_intent = filter!(f_eq("class", PVCLASS_OAUTH2_RS.clone()));
let oauth2_related = self.qs_read.internal_search(f)?;
// _ext reduces the entries based on access.
let oauth2_related = self
.qs_read
.impersonate_search_ext(f_executed, f_intent, ident)?;
trace!(?oauth2_related);
// Aggregate results to a Vec of AppLink

View file

@ -2603,4 +2603,124 @@ mod tests {
// Test reject purge
test_acp_modify!(&me_purge, vec![acp_allow], &r2_set, false);
}
#[test]
fn test_access_ouath2_dyn_search() {
sketching::test_init();
// Test that an account that is granted a scope to an oauth2 rs is granted
// the ability to search that rs.
let rs_uuid = Uuid::new_v4();
let ev1 = unsafe {
entry_init!(
("class", CLASS_OBJECT.clone()),
("class", Value::new_class("oauth2_resource_server")),
("class", Value::new_class("oauth2_resource_server_basic")),
("uuid", Value::Uuid(rs_uuid)),
("oauth2_rs_name", Value::new_iname("test_resource_server")),
("displayname", Value::new_utf8s("test_resource_server")),
(
"oauth2_rs_origin",
Value::new_url_s("https://demo.example.com").unwrap()
),
(
"oauth2_rs_scope_map",
Value::new_oauthscopemap(UUID_TEST_GROUP_1, btreeset!["groups".to_string()])
.expect("invalid oauthscope")
),
(
"oauth2_rs_sup_scope_map",
Value::new_oauthscopemap(
UUID_TEST_GROUP_1,
btreeset!["supplement".to_string()]
)
.expect("invalid oauthscope")
),
(
"oauth2_allow_insecure_client_disable_pkce",
Value::new_bool(true)
),
("oauth2_jwt_legacy_crypto_enable", Value::new_bool(false)),
("oauth2_prefer_short_username", Value::new_bool(false))
)
.into_sealed_committed()
};
let ev1_reduced = unsafe {
entry_init!(
("class", CLASS_OBJECT.clone()),
("class", Value::new_class("oauth2_resource_server")),
("class", Value::new_class("oauth2_resource_server_basic")),
("uuid", Value::Uuid(rs_uuid)),
("oauth2_rs_name", Value::new_iname("test_resource_server")),
("displayname", Value::new_utf8s("test_resource_server")),
(
"oauth2_rs_origin",
Value::new_url_s("https://demo.example.com").unwrap()
)
)
.into_sealed_committed()
};
let ev2 = unsafe {
entry_init!(
("class", CLASS_OBJECT.clone()),
("class", Value::new_class("oauth2_resource_server")),
("class", Value::new_class("oauth2_resource_server_basic")),
("uuid", Value::Uuid(Uuid::new_v4())),
("oauth2_rs_name", Value::new_iname("second_resource_server")),
("displayname", Value::new_utf8s("second_resource_server")),
(
"oauth2_rs_origin",
Value::new_url_s("https://noaccess.example.com").unwrap()
),
(
"oauth2_rs_scope_map",
Value::new_oauthscopemap(UUID_SYSTEM_ADMINS, btreeset!["groups".to_string()])
.expect("invalid oauthscope")
),
(
"oauth2_rs_sup_scope_map",
Value::new_oauthscopemap(
// This is NOT the scope map that is access checked!
UUID_TEST_GROUP_1,
btreeset!["supplement".to_string()]
)
.expect("invalid oauthscope")
),
(
"oauth2_allow_insecure_client_disable_pkce",
Value::new_bool(true)
),
("oauth2_jwt_legacy_crypto_enable", Value::new_bool(false)),
("oauth2_prefer_short_username", Value::new_bool(false))
)
.into_sealed_committed()
};
let r_set = vec![Arc::new(ev1.clone()), Arc::new(ev2)];
let se_a = unsafe {
SearchEvent::new_impersonate_entry(
E_TEST_ACCOUNT_1.clone(),
filter_all!(f_pres("oauth2_rs_name")),
)
};
let ex_a = vec![Arc::new(ev1)];
let ex_a_reduced = vec![ev1_reduced];
let se_b = unsafe {
SearchEvent::new_impersonate_entry(
E_TEST_ACCOUNT_2.clone(),
filter_all!(f_pres("oauth2_rs_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);
}
}

View file

@ -34,6 +34,14 @@ pub(super) fn apply_search_access<'a>(
AccessResult::Allow(mut set) => allow.append(&mut set),
};
match search_oauth2_filter_entry(ident, entry) {
AccessResult::Denied => denied = true,
AccessResult::Grant => grant = true,
AccessResult::Ignore => {}
AccessResult::Constrain(mut set) => constrain.append(&mut set),
AccessResult::Allow(mut set) => allow.append(&mut set),
};
// We'll add more modules later.
// Now finalise the decision.
@ -102,3 +110,41 @@ fn search_filter_entry<'a>(
AccessResult::Allow(allowed_attrs)
}
fn search_oauth2_filter_entry<'a>(
ident: &Identity,
entry: &'a Arc<EntrySealedCommitted>,
) -> AccessResult<'a> {
match &ident.origin {
IdentType::Internal | IdentType::Synch(_) => AccessResult::Ignore,
IdentType::User(iuser) => {
if entry
.get_ava_as_iutf8("class")
.map(|set| {
trace!(?set);
set.contains("oauth2_resource_server")
})
.unwrap_or(false)
{
if entry
.get_ava_as_oauthscopemaps("oauth2_rs_scope_map")
.and_then(|maps| ident.get_memberof().map(|mo| (maps, mo)))
.map(|(maps, mo)| maps.keys().any(|k| mo.contains(k)))
.unwrap_or(false)
{
security_access!(entry = ?entry.get_uuid(), ident = ?iuser.entry.get_uuid2rdn(), "ident is a memberof a group granted an oauth2 scope by this entry");
return AccessResult::Allow(btreeset!(
"class",
"displayname",
"uuid",
"oauth2_rs_name",
"oauth2_rs_origin",
"oauth2_rs_origin_landing"
));
}
}
AccessResult::Ignore
}
}
}

View file

@ -156,6 +156,25 @@ impl<'a> QueryServerWriteTransaction<'a> {
let de = DeleteEvent::new_internal(f_valid);
self.delete(&de)
}
#[instrument(level = "debug", skip_all)]
pub fn internal_delete_uuid_if_exists(
&mut self,
target_uuid: Uuid,
) -> Result<(), OperationError> {
let filter = filter!(f_eq("uuid", PartialValue::Uuid(target_uuid)));
let f_valid = filter
.validate(self.get_schema())
.map_err(OperationError::SchemaViolation)?;
let ee = ExistsEvent::new_internal(f_valid.clone());
// Submit it
if self.exists(&ee)? {
let de = DeleteEvent::new_internal(f_valid);
self.delete(&de)?;
}
Ok(())
}
}
#[cfg(test)]

View file

@ -502,7 +502,7 @@ impl<'a> QueryServerWriteTransaction<'a> {
JSON_IDM_ACP_RADIUS_SECRET_READ_PRIV_V1,
JSON_IDM_ACP_RADIUS_SECRET_WRITE_PRIV_V1,
JSON_IDM_HP_ACP_SERVICE_ACCOUNT_INTO_PERSON_MIGRATE_V1,
JSON_IDM_ACP_OAUTH2_READ_PRIV_V1,
// JSON_IDM_ACP_OAUTH2_READ_PRIV_V1,
JSON_IDM_HP_ACP_SYNC_ACCOUNT_MANAGE_PRIV_V1,
];
@ -534,6 +534,20 @@ impl<'a> QueryServerWriteTransaction<'a> {
debug_assert!(res.is_ok());
res?;
// Delete entries that no longer need to exist.
let delete_entries = [UUID_IDM_ACP_OAUTH2_READ_PRIV_V1];
let res: Result<(), _> = delete_entries
.into_iter()
.try_for_each(|entry_uuid| self.internal_delete_uuid_if_exists(entry_uuid));
if res.is_ok() {
admin_debug!("initialise_idm -> result Ok!");
} else {
admin_error!(?res, "initialise_idm p3 -> result");
}
debug_assert!(res.is_ok());
res?;
self.changed_schema = true;
self.changed_acp = true;

View file

@ -224,7 +224,10 @@ impl Component for Oauth2App {
let query: Option<AuthorisationRequest> = location
.query()
.map_err(|e| {
let e_msg = format!("failed to decode authorisation request url parameters -> {:?}", e);
let e_msg = format!(
"failed to decode authorisation request url parameters -> {:?}",
e
);
console::error!(e_msg.as_str());
})
.ok()