From 78a8001225eb5b872b24031bd78d19d1819ea1de Mon Sep 17 00:00:00 2001 From: Tommy Yu Date: Fri, 27 Sep 2024 15:53:56 +1200 Subject: [PATCH] Formalize generated policy, preparing user role - Completely refactor of all relevant structs, renaming them more appropriately, and moved into their own module. - Include a new user role relationship, which represents the role that the user has at the system, such that when a resource is at a workflow state, the permissions associated with that role associated with that state will become activated for the user. - Clarified that Reviewer role doesn't need the global role by default at the base model level, as that type of grant is wider than what was intended. With the user role being defined this type of grant is much better fit for that, and will achieve the goal of enabling the access whenever ``State::Pending`` is set for those resources. - Various backend methods have been renamed to be more precise. - Still need to provide backend methods for management of user roles. --- pmrac/src/platform.rs | 28 +-- pmrac/tests/test_platform.rs | 45 +++-- pmrcore/src/ac.rs | 1 + pmrcore/src/ac/genpolicy.rs | 47 +++++ pmrcore/src/ac/permit.rs | 35 +--- pmrcore/src/ac/traits.rs | 11 +- ...872eaee5226bba34a7194cd7a90e8b058f3e6.json | 26 +++ .../migrations/pmrac/20240912185559_pmrac.sql | 27 ++- pmrmodel/src/model/db/sqlite/ac/policy.rs | 16 +- pmrmodel/src/model/db/sqlite/ac/resource.rs | 176 ++++++++++++------ pmrrbac/src/lib.rs | 159 ++++++++++------ 11 files changed, 382 insertions(+), 189 deletions(-) create mode 100644 pmrcore/src/ac/genpolicy.rs create mode 100644 pmrmodel/.sqlx/query-e3572c1a4e184b624955de0ca48872eaee5226bba34a7194cd7a90e8b058f3e6.json diff --git a/pmrac/src/platform.rs b/pmrac/src/platform.rs index 8c832a0..23222f1 100644 --- a/pmrac/src/platform.rs +++ b/pmrac/src/platform.rs @@ -1,7 +1,7 @@ use pmrcore::{ ac::{ agent::Agent, - permit::ResourcePolicy, + genpolicy::Policy, role::Role, workflow::State, }, @@ -179,26 +179,26 @@ impl Platform { // Agent Policy management impl Platform { - pub async fn grant_role_to_agent( + pub async fn grant_res_role_to_agent( &self, res: &str, agent: impl Into, role: Role, ) -> Result<(), Error> { - Ok(self.ac_platform.grant_role_to_agent( + Ok(self.ac_platform.grant_res_role_to_agent( res, &agent.into(), role ).await?) } - pub async fn revoke_role_from_agent( + pub async fn revoke_res_role_from_agent( &self, res: &str, agent: impl Into, role: Role, ) -> Result<(), Error> { - Ok(self.ac_platform.revoke_role_from_agent( + Ok(self.ac_platform.revoke_res_role_from_agent( res, &agent.into(), role, @@ -250,12 +250,14 @@ impl Platform { ).await?) } - pub async fn generate_policy_for_res( + pub async fn generate_policy_for_agent_res( &self, + agent: &Agent, res: String, - ) -> Result { - Ok(self.ac_platform.generate_policy_for_res( - res + ) -> Result { + Ok(self.ac_platform.generate_policy_for_agent_res( + agent, + res, ).await?) } } @@ -270,13 +272,17 @@ impl Platform { endpoint_group: impl AsRef, http_method: &str, ) -> Result { + let agent = agent.into(); Ok(self.pmrrbac_builder .build_with_resource_policy( - self.generate_policy_for_res(res.to_string()).await?, + self.generate_policy_for_agent_res( + &agent, + res.to_string(), + ).await?, ) .await? .enforce( - >>::into(agent.into()), + >>::into(agent), res, endpoint_group, http_method, diff --git a/pmrac/tests/test_platform.rs b/pmrac/tests/test_platform.rs index a8f2447..d74cb00 100644 --- a/pmrac/tests/test_platform.rs +++ b/pmrac/tests/test_platform.rs @@ -1,4 +1,5 @@ use pmrcore::ac::{ + agent::Agent, role::Role, workflow::State, }; @@ -141,8 +142,8 @@ async fn policy() -> anyhow::Result<()> { let state = State::Private; let role = Role::Manager; - platform.grant_role_to_agent("/", &user, role).await?; - platform.revoke_role_from_agent("/", &user, role).await?; + platform.grant_res_role_to_agent("/", &user, role).await?; + platform.revoke_res_role_from_agent("/", &user, role).await?; platform.assign_policy_to_wf_state(state, role, "", "GET").await?; platform.remove_policy_from_wf_state(state, role, "", "GET").await?; @@ -155,12 +156,12 @@ async fn resource_wf_state() -> anyhow::Result<()> { let admin = platform.create_user("admin").await?; let user = platform.create_user("test_user").await?; - platform.grant_role_to_agent( + platform.grant_res_role_to_agent( "/*", admin, Role::Manager, ).await?; - platform.grant_role_to_agent( + platform.grant_res_role_to_agent( "/item/1", user, Role::Owner, @@ -195,16 +196,18 @@ async fn resource_wf_state() -> anyhow::Result<()> { State::Private, ).await?; - let mut policy = platform.generate_policy_for_res("/item/1".into()).await?; - policy.grants.sort_unstable(); - policy.policies.sort_unstable(); + let mut policy = platform.generate_policy_for_agent_res(&Agent::Anonymous, "/item/1".into()).await?; + policy.res_grants.sort_unstable(); + policy.role_permits.sort_unstable(); assert_eq!(policy, serde_json::from_str(r#"{ "resource": "/item/1", - "grants": [ + "user_roles": [ + ], + "res_grants": [ {"res": "/*", "agent": "admin", "role": "Manager"}, {"res": "/item/1", "agent": "test_user", "role": "Owner"} ], - "policies": [ + "role_permits": [ {"role": "Owner", "endpoint_group": "edit", "method": "GET"}, {"role": "Owner", "endpoint_group": "edit", "method": "POST"} ] @@ -214,16 +217,18 @@ async fn resource_wf_state() -> anyhow::Result<()> { "/item/1", State::Published, ).await?; - let mut policy = platform.generate_policy_for_res("/item/1".into()).await?; - policy.grants.sort_unstable(); - policy.policies.sort_unstable(); + let mut policy = platform.generate_policy_for_agent_res(&Agent::Anonymous, "/item/1".into()).await?; + policy.res_grants.sort_unstable(); + policy.role_permits.sort_unstable(); assert_eq!(policy, serde_json::from_str(r#"{ "resource": "/item/1", - "grants": [ + "user_roles": [ + ], + "res_grants": [ {"res": "/*", "agent": "admin", "role": "Manager"}, {"res": "/item/1", "agent": "test_user", "role": "Owner"} ], - "policies": [ + "role_permits": [ {"role": "Owner", "endpoint_group": "edit", "method": "GET"}, {"role": "Reader", "endpoint_group": "", "method": "GET"} ] @@ -249,21 +254,21 @@ async fn policy_enforcement() -> anyhow::Result<()> { let admin = platform.create_user("admin").await?; admin.reset_password("admin", "admin").await?; - platform.grant_role_to_agent("/*", admin, Role::Manager).await?; + platform.grant_res_role_to_agent("/*", admin, Role::Manager).await?; let reviewer = platform.create_user("reviewer").await?; reviewer.reset_password("reviewer", "reviewer").await?; // this makes the reviewer being able to review globally - // platform.grant_role_to_agent("/*", &reviewer, Role::Reviewer).await?; + // platform.grant_res_role_to_agent("/*", &reviewer, Role::Reviewer).await?; // we need something actually // Or is this something that can be expressed with casbin as part of the base model/policy? // platform.enable_role_at_state_for_resource(Role::Reviewer, State::Pending, "/*").await?; - platform.grant_role_to_agent("/profile/reviewer", reviewer, Role::Owner).await?; + platform.grant_res_role_to_agent("/profile/reviewer", reviewer, Role::Owner).await?; platform.set_wf_state_for_res("/profile/reviewer", State::Private).await?; let user = platform.create_user("user").await?; user.reset_password("user", "user").await?; - platform.grant_role_to_agent("/profile/user", user, Role::Owner).await?; + platform.grant_res_role_to_agent("/profile/user", user, Role::Owner).await?; platform.set_wf_state_for_res("/profile/user", State::Private).await?; let admin = platform.authenticate_user("admin", "admin").await?; @@ -275,7 +280,7 @@ async fn policy_enforcement() -> anyhow::Result<()> { assert!(!platform.enforce(&reviewer, "/profile/user", "", "GET").await?); // create content owned by user - platform.grant_role_to_agent("/news/post/1", &user, Role::Owner).await?; + platform.grant_res_role_to_agent("/news/post/1", &user, Role::Owner).await?; // editable by the user while private platform.set_wf_state_for_res("/news/post/1", State::Private).await?; @@ -289,7 +294,7 @@ async fn policy_enforcement() -> anyhow::Result<()> { // moment, rather than the role in a more general way // That said, this address the use case for assigning _specific_ reviewer for the // task and they will have the rights required - platform.grant_role_to_agent("/news/post/1", &reviewer, Role::Reviewer).await?; + platform.grant_res_role_to_agent("/news/post/1", &reviewer, Role::Reviewer).await?; assert!(platform.enforce(&admin, "/news/post/1", "edit", "POST").await?); assert!(!platform.enforce(&user, "/news/post/1", "edit", "POST").await?); diff --git a/pmrcore/src/ac.rs b/pmrcore/src/ac.rs index 964b2f2..b27cdc8 100644 --- a/pmrcore/src/ac.rs +++ b/pmrcore/src/ac.rs @@ -1,4 +1,5 @@ pub mod agent; +pub mod genpolicy; pub mod permit; pub mod role; pub mod traits; diff --git a/pmrcore/src/ac/genpolicy.rs b/pmrcore/src/ac/genpolicy.rs new file mode 100644 index 0000000..9a23c5e --- /dev/null +++ b/pmrcore/src/ac/genpolicy.rs @@ -0,0 +1,47 @@ +//! Generated Policy +//! +//! The structs provided by this module represents policies generated +//! for consumption by some security enforcer, and is not meant to be +//! persisted in some datastore. + +use serde::{Deserialize, Serialize}; +use crate::ac::role::Role; + +/// Grants, roles and permissions associated with the given resource +/// to be passed into the security enforcer as a complete policy. +#[derive(Clone, Debug, PartialEq, Deserialize, Serialize)] +pub struct Policy { + pub resource: String, + pub user_roles: Vec, + pub res_grants: Vec, + pub role_permits: Vec, +} + +/// A resource grant - the agent will have the stated role at the given +/// resource. +#[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd, Deserialize, Serialize)] +pub struct ResGrant { + // this may feel redundant later, but this line signifies the exact + // res this was granted for, which may be at a higher level. + pub res: String, + pub agent: Option, + pub role: Role, +} + +/// This represents the endpoint_group and HTTP method that the role is +/// given the permit for. +#[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd, Deserialize, Serialize)] +pub struct RolePermit { + pub role: Role, + pub endpoint_group: String, + pub method: String, +} + +/// Represents the role granted to the user for the system. Roles +/// granted this way is only applicable for resources at some +/// appropriate state. +#[derive(Clone, Debug, PartialEq, Deserialize, Serialize)] +pub struct UserRole { + pub user: String, + pub role: Role, +} diff --git a/pmrcore/src/ac/permit.rs b/pmrcore/src/ac/permit.rs index 834816c..a9ef8e7 100644 --- a/pmrcore/src/ac/permit.rs +++ b/pmrcore/src/ac/permit.rs @@ -1,8 +1,6 @@ use serde::{Deserialize, Serialize}; -use crate::ac::{ - role::Role, - user::User, -}; + +// TODO use these structs to represent the underlying for editing and presentation /// Resource grant /// @@ -16,7 +14,7 @@ pub struct ResGrant { pub role: String, } -/// Workfolow policy +/// Workflow policy /// /// For each workflow state there may be multiple roles associated with /// the different endpoint groups and HTTP methods. This struct will @@ -29,30 +27,3 @@ pub struct WorkflowPolicy { pub endpoint_group: String, pub method: String, } - -/// A grant that will be passed onto the enforcer. -#[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd, Deserialize, Serialize)] -pub struct Grant { - // this may feel redundant later, but this line signifies the exact - // res this was granted for, which may be at a higher level. - pub res: String, - pub agent: Option, - pub role: String, -} - -/// A policy entry that will be passed onto the enforcer. -#[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd, Deserialize, Serialize)] -pub struct Policy { - pub role: Role, - pub endpoint_group: String, - pub method: String, -} - -/// Grants and resources associated with the resource ready to be passed -/// into the enforcer. -#[derive(Clone, Debug, PartialEq, Deserialize, Serialize)] -pub struct ResourcePolicy { - pub resource: String, - pub grants: Vec, - pub policies: Vec, -} diff --git a/pmrcore/src/ac/traits.rs b/pmrcore/src/ac/traits.rs index b37b35a..2869cc0 100644 --- a/pmrcore/src/ac/traits.rs +++ b/pmrcore/src/ac/traits.rs @@ -2,7 +2,7 @@ use async_trait::async_trait; use crate::error::BackendError; use super::{ agent::Agent, - permit::ResourcePolicy, + genpolicy::Policy, role::Role, user::User, workflow::State, @@ -39,13 +39,13 @@ pub trait UserBackend { #[async_trait] pub trait PolicyBackend { - async fn grant_role_to_agent( + async fn grant_res_role_to_agent( &self, res: &str, agent: &Agent, role: Role, ) -> Result<(), BackendError>; - async fn revoke_role_from_agent( + async fn revoke_res_role_from_agent( &self, res: &str, agent: &Agent, @@ -74,8 +74,9 @@ pub trait ResourceBackend { res: &str, wf_state: State, ) -> Result<(), BackendError>; - async fn generate_policy_for_res( + async fn generate_policy_for_agent_res( &self, + agent: &Agent, res: String, - ) -> Result; + ) -> Result; } diff --git a/pmrmodel/.sqlx/query-e3572c1a4e184b624955de0ca48872eaee5226bba34a7194cd7a90e8b058f3e6.json b/pmrmodel/.sqlx/query-e3572c1a4e184b624955de0ca48872eaee5226bba34a7194cd7a90e8b058f3e6.json new file mode 100644 index 0000000..39b99d1 --- /dev/null +++ b/pmrmodel/.sqlx/query-e3572c1a4e184b624955de0ca48872eaee5226bba34a7194cd7a90e8b058f3e6.json @@ -0,0 +1,26 @@ +{ + "db_name": "SQLite", + "query": "SELECT\n 'user'.name as user,\n user_role.role AS role\nFROM\n user_role\nJOIN\n 'user' ON user_role.user_id == 'user'.id\nWHERE\n user_role.user_id = ?1\n", + "describe": { + "columns": [ + { + "name": "user", + "ordinal": 0, + "type_info": "Text" + }, + { + "name": "role", + "ordinal": 1, + "type_info": "Text" + } + ], + "parameters": { + "Right": 1 + }, + "nullable": [ + false, + false + ] + }, + "hash": "e3572c1a4e184b624955de0ca48872eaee5226bba34a7194cd7a90e8b058f3e6" +} diff --git a/pmrmodel/migrations/pmrac/20240912185559_pmrac.sql b/pmrmodel/migrations/pmrac/20240912185559_pmrac.sql index db1819e..d885e93 100644 --- a/pmrmodel/migrations/pmrac/20240912185559_pmrac.sql +++ b/pmrmodel/migrations/pmrac/20240912185559_pmrac.sql @@ -26,12 +26,27 @@ CREATE TABLE IF NOT EXISTS user_password ( -- old passwords when reset. CREATE INDEX IF NOT EXISTS user_password__user_id ON user_email(user_id); --- This would group the grants for a given resource for the rbac model --- if we really want this to be very robust, the role will be an --- identifier, there would be annotations associating the role name to --- the role id and then the end points may also associate, but that's --- too complicated for what we are currently doing, so a simple --- descriptive string token will suffice for now. +-- This provides a user with the given role that only becomes active for +-- any given resource at a workflow state where the role is enabled for +-- the specific combination of endpoint_group and http method. +-- +-- In effect, this grants the user the specific role in the system, and +-- the system will only grant this role to the user for applicable +-- resources. +CREATE TABLE IF NOT EXISTS user_role ( + id INTEGER PRIMARY KEY NOT NULL, + user_id INTEGER, + role TEXT NOT NULL, + FOREIGN KEY(user_id) REFERENCES 'user'(id) +); +CREATE INDEX IF NOT EXISTS user_role__user_id ON user_role(user_id); + +-- This grants the user a specific role at the resource. Currently, the +-- role is a enum encoded in the underlying application; a future +-- refinement may allow this to be fully user definable. The grant will +-- be applied through the Casbin model that implements a form of RBAC +-- when used in conjunction with the workflow policy and the resource +-- state. CREATE TABLE IF NOT EXISTS res_grant ( id INTEGER PRIMARY KEY NOT NULL, res TEXT NOT NULL, diff --git a/pmrmodel/src/model/db/sqlite/ac/policy.rs b/pmrmodel/src/model/db/sqlite/ac/policy.rs index 75776a3..2d405f9 100644 --- a/pmrmodel/src/model/db/sqlite/ac/policy.rs +++ b/pmrmodel/src/model/db/sqlite/ac/policy.rs @@ -13,7 +13,7 @@ use crate::{ backend::db::SqliteBackend, }; -async fn grant_role_to_agent_sqlite( +async fn grant_res_role_to_agent_sqlite( backend: &SqliteBackend, res: &str, agent: &Agent, @@ -40,7 +40,7 @@ VALUES ( ?1, ?2, ?3 ) Ok(()) } -async fn revoke_role_from_agent_sqlite( +async fn revoke_res_role_from_agent_sqlite( backend: &SqliteBackend, res: &str, agent: &Agent, @@ -127,13 +127,13 @@ WHERE #[async_trait] impl PolicyBackend for SqliteBackend { - async fn grant_role_to_agent( + async fn grant_res_role_to_agent( &self, res: &str, agent: &Agent, role: Role, ) -> Result<(), BackendError> { - grant_role_to_agent_sqlite( + grant_res_role_to_agent_sqlite( &self, res, agent, @@ -141,13 +141,13 @@ impl PolicyBackend for SqliteBackend { ).await } - async fn revoke_role_from_agent( + async fn revoke_res_role_from_agent( &self, res: &str, agent: &Agent, role: Role, ) -> Result<(), BackendError> { - revoke_role_from_agent_sqlite( + revoke_res_role_from_agent_sqlite( &self, res, agent, @@ -214,8 +214,8 @@ pub(crate) mod testing { let agent: Agent = UserBackend::get_user_by_id(&backend, user_id).await?.into(); let state = State::Published; let role = Role::Reader; - PolicyBackend::grant_role_to_agent(&backend, "/", &agent, role).await?; - PolicyBackend::revoke_role_from_agent(&backend, "/", &agent, role).await?; + PolicyBackend::grant_res_role_to_agent(&backend, "/", &agent, role).await?; + PolicyBackend::revoke_res_role_from_agent(&backend, "/", &agent, role).await?; PolicyBackend::assign_policy_to_wf_state(&backend, state, role, "", "GET").await?; PolicyBackend::remove_policy_from_wf_state(&backend, state, role, "", "GET").await?; Ok(()) diff --git a/pmrmodel/src/model/db/sqlite/ac/resource.rs b/pmrmodel/src/model/db/sqlite/ac/resource.rs index de7224d..73a1ac6 100644 --- a/pmrmodel/src/model/db/sqlite/ac/resource.rs +++ b/pmrmodel/src/model/db/sqlite/ac/resource.rs @@ -1,10 +1,12 @@ use async_trait::async_trait; use pmrcore::{ ac::{ - permit::{ - Grant, + agent::Agent, + genpolicy::{ + UserRole, + ResGrant, + RolePermit, Policy, - ResourcePolicy, }, role::Role, traits::ResourceBackend, @@ -43,16 +45,45 @@ DO UPDATE SET Ok(()) } -async fn generate_policy_for_res_sqlite( +async fn generate_policy_for_agent_res_sqlite( backend: &SqliteBackend, + agent: &Agent, res: impl Into + Send, -) -> Result { +) -> Result { let resource = res.into(); - let res_str = resource.as_str(); + + let user_roles = match agent { + Agent::User(user) => { + sqlx::query!( + "\ +SELECT + 'user'.name as user, + user_role.role AS role +FROM + user_role +JOIN + 'user' ON user_role.user_id == 'user'.id +WHERE + user_role.user_id = ?1 +\ + ", + user.id, + ) + .map(|row| UserRole { + user: row.user, + role: Role::from_str(&row.role).unwrap_or(Role::default()), + }) + .fetch_all(&*backend.pool) + .await? + } + Agent::Anonymous => vec![], + }; + // FIXME Eventually we may need to support all levels of wildcards, // so the shortcut `OR res = "/*"` will no longer be sufficient. // Well, or whatever way this will be structured. - let grants = sqlx::query!( + let res_str = resource.as_str(); + let res_grants = sqlx::query!( r#" SELECT res_grant.res as res, @@ -67,15 +98,15 @@ WHERE "#, res_str, ) - .map(|row| Grant { + .map(|row| ResGrant { res: row.res, agent: row.user, - role: row.role, + role: Role::from_str(&row.role).unwrap_or(Role::default()), }) .fetch_all(&*backend.pool) .await?; - let policies = sqlx::query!( + let role_permits = sqlx::query!( r#" SELECT wf_policy.role AS role, @@ -90,7 +121,7 @@ WHERE "#, res_str, ) - .map(|row| Policy { + .map(|row| RolePermit { role: Role::from_str(&row.role).unwrap_or_default(), endpoint_group: row.endpoint_group, method: row.method, @@ -98,10 +129,11 @@ WHERE .fetch_all(&*backend.pool) .await?; - Ok(ResourcePolicy { + Ok(Policy { resource, - grants, - policies, + user_roles, + res_grants, + role_permits, }) } @@ -119,12 +151,14 @@ impl ResourceBackend for SqliteBackend { ).await } - async fn generate_policy_for_res( + async fn generate_policy_for_agent_res( &self, + agent: &Agent, res: String, - ) -> Result { - generate_policy_for_res_sqlite( + ) -> Result { + generate_policy_for_agent_res_sqlite( &self, + agent, res, ).await } @@ -134,7 +168,7 @@ impl ResourceBackend for SqliteBackend { pub(crate) mod testing { use pmrcore::ac::{ agent::Agent, - permit::ResourcePolicy, + genpolicy::Policy, role::Role, traits::{ PolicyBackend, @@ -154,21 +188,31 @@ pub(crate) mod testing { .await? .run_migration_profile(MigrationProfile::Pmrac) .await?; - let policy = ResourceBackend::generate_policy_for_res(&backend, "/".into()).await?; - assert_eq!(policy, ResourcePolicy { + let policy = ResourceBackend::generate_policy_for_agent_res( + &backend, + &Agent::Anonymous, + "/".into(), + ).await?; + assert_eq!(policy, Policy { resource: "/".to_string(), - grants: vec![], - policies: vec![], + user_roles: vec![], + res_grants: vec![], + role_permits: vec![], }); - // we only publish here, but no policies/users attached + // we only publish here, but no role_permits/users attached let state = State::Published; ResourceBackend::set_wf_state_for_res(&backend, "/", state).await?; - let policy = ResourceBackend::generate_policy_for_res(&backend, "/".into()).await?; - assert_eq!(policy, ResourcePolicy { + let policy = ResourceBackend::generate_policy_for_agent_res( + &backend, + &Agent::Anonymous, + "/".into(), + ).await?; + assert_eq!(policy, Policy { resource: "/".to_string(), - grants: vec![], - policies: vec![], + user_roles: vec![], + res_grants: vec![], + role_permits: vec![], }); Ok(()) } @@ -185,29 +229,41 @@ pub(crate) mod testing { let state = State::Published; let role = Role::Reader; let agent: Agent = user.into(); - PolicyBackend::grant_role_to_agent(&backend, "/", &agent, role).await?; + PolicyBackend::grant_res_role_to_agent(&backend, "/", &agent, role).await?; PolicyBackend::assign_policy_to_wf_state(&backend, state, role, "", "GET").await?; ResourceBackend::set_wf_state_for_res(&backend, "/", state).await?; - let policy = ResourceBackend::generate_policy_for_res(&backend, "/".into()).await?; + let policy = ResourceBackend::generate_policy_for_agent_res( + &backend, + &agent, + "/".into(), + ).await?; assert_eq!(policy, serde_json::from_str(r#"{ "resource": "/", - "grants": [ + "user_roles": [ + ], + "res_grants": [ {"res": "/", "agent": "test_user", "role": "Reader"} ], - "policies": [ + "role_permits": [ {"role": "Reader", "endpoint_group": "", "method": "GET"} ] }"#)?); - PolicyBackend::revoke_role_from_agent(&backend, "/", &agent, role).await?; + PolicyBackend::revoke_res_role_from_agent(&backend, "/", &agent, role).await?; PolicyBackend::remove_policy_from_wf_state(&backend, state, role, "", "GET").await?; - let policy = ResourceBackend::generate_policy_for_res(&backend, "/".into()).await?; + let policy = ResourceBackend::generate_policy_for_agent_res( + &backend, + &agent, + "/".into(), + ).await?; assert_eq!(policy, serde_json::from_str(r#"{ "resource": "/", - "grants": [ + "user_roles": [ ], - "policies": [ + "res_grants": [ + ], + "role_permits": [ ] }"#)?); @@ -220,18 +276,19 @@ pub(crate) mod testing { .await? .run_migration_profile(MigrationProfile::Pmrac) .await?; - // we only publish here, but no policies/users attached + // we only publish here, but no role_permits/users attached let state = State::Published; let role = Role::Reader; let agent = Agent::Anonymous; ResourceBackend::set_wf_state_for_res(&backend, "/", state).await?; - PolicyBackend::grant_role_to_agent(&backend, "/", &agent, role).await?; + PolicyBackend::grant_res_role_to_agent(&backend, "/", &agent, role).await?; - let policy = ResourceBackend::generate_policy_for_res(&backend, "/".into()).await?; - assert_eq!(policy, ResourcePolicy { + let policy = ResourceBackend::generate_policy_for_agent_res(&backend, &agent, "/".into()).await?; + assert_eq!(policy, Policy { resource: "/".to_string(), - grants: serde_json::from_str(r#"[{"res": "/", "user": null, "role": "Reader"}]"#)?, - policies: vec![], + user_roles: vec![], + res_grants: serde_json::from_str(r#"[{"res": "/", "user": null, "role": "Reader"}]"#)?, + role_permits: vec![], }); Ok(()) } @@ -251,13 +308,13 @@ pub(crate) mod testing { let admin_id = UserBackend::add_user(&backend, "admin").await?; let admin = UserBackend::get_user_by_id(&backend, admin_id).await?; let agent_admin: Agent = admin.into(); - PolicyBackend::grant_role_to_agent( + PolicyBackend::grant_res_role_to_agent( &backend, "/*", &agent_admin, Role::Manager, ).await?; - PolicyBackend::grant_role_to_agent( + PolicyBackend::grant_res_role_to_agent( &backend, "/item/1", &agent_user, @@ -297,16 +354,23 @@ pub(crate) mod testing { "/item/1", State::Private, ).await?; - let mut policy = ResourceBackend::generate_policy_for_res(&backend, "/item/1".into()).await?; - policy.grants.sort_unstable(); - policy.policies.sort_unstable(); + // TODO should generate policy for agent_user also + let mut policy = ResourceBackend::generate_policy_for_agent_res( + &backend, + &agent_admin, + "/item/1".into(), + ).await?; + policy.res_grants.sort_unstable(); + policy.role_permits.sort_unstable(); assert_eq!(policy, serde_json::from_str(r#"{ "resource": "/item/1", - "grants": [ + "user_roles": [ + ], + "res_grants": [ {"res": "/*", "agent": "admin", "role": "Manager"}, {"res": "/item/1", "agent": "test_user", "role": "Owner"} ], - "policies": [ + "role_permits": [ {"role": "Owner", "endpoint_group": "edit", "method": "GET"}, {"role": "Owner", "endpoint_group": "edit", "method": "POST"} ] @@ -317,16 +381,22 @@ pub(crate) mod testing { "/item/1", State::Published, ).await?; - let mut policy = ResourceBackend::generate_policy_for_res(&backend, "/item/1".into()).await?; - policy.grants.sort_unstable(); - policy.policies.sort_unstable(); + let mut policy = ResourceBackend::generate_policy_for_agent_res( + &backend, + &agent_admin, + "/item/1".into(), + ).await?; + policy.res_grants.sort_unstable(); + policy.role_permits.sort_unstable(); assert_eq!(policy, serde_json::from_str(r#"{ "resource": "/item/1", - "grants": [ + "user_roles": [ + ], + "res_grants": [ {"res": "/*", "agent": "admin", "role": "Manager"}, {"res": "/item/1", "agent": "test_user", "role": "Owner"} ], - "policies": [ + "role_permits": [ {"role": "Owner", "endpoint_group": "edit", "method": "GET"}, {"role": "Reader", "endpoint_group": "", "method": "GET"} ] diff --git a/pmrrbac/src/lib.rs b/pmrrbac/src/lib.rs index bdec0d7..22cadb5 100644 --- a/pmrrbac/src/lib.rs +++ b/pmrrbac/src/lib.rs @@ -7,10 +7,11 @@ use casbin::{ }; use pmrcore::ac::{ role::Role, - permit::{ - Grant, + genpolicy::{ Policy, - ResourcePolicy, + ResGrant, + RolePermit, + UserRole, }, }; @@ -74,11 +75,6 @@ Editor, /*, edit, POST Editor, /*, grant, GET Editor, /*, protocol, GET Editor, /*, protocol, POST - -# Reviewers can only view and edit. -Reviewer, /*, , GET -Reviewer, /*, edit, GET -Reviewer, /*, edit, POST "; /// An alternative policy @@ -105,11 +101,6 @@ Editor, /*, edit, POST Editor, /*, grant, GET Editor, /*, protocol, GET Editor, /*, protocol, POST - -# Reviewers can only view and edit. -Reviewer, /*, , GET -Reviewer, /*, edit, GET -Reviewer, /*, edit, POST "; /// Builds a role-based access controller (RBAC) for PMR. @@ -127,7 +118,7 @@ pub struct Builder { anonymous_reader: bool, base_policy: Box, default_model: Box, - resource_policy: Option, + resource_policy: Option, } impl Builder { @@ -162,7 +153,7 @@ impl Builder { self } - pub fn resource_policy(mut self, val: ResourcePolicy) -> Self { + pub fn resource_policy(mut self, val: Policy) -> Self { self.resource_policy = Some(val); self } @@ -178,7 +169,7 @@ impl Builder { pub async fn build_with_resource_policy( &self, - resource_policy: ResourcePolicy, + resource_policy: Policy, ) -> Result { PmrRbac::new( self.anonymous_reader, @@ -198,7 +189,7 @@ impl PmrRbac { anonymous_reader: bool, policies: &str, model: &str, - resource_policy: Option, + resource_policy: Option, ) -> Result { let m = DefaultModel::from_str(model).await?; let a = MemoryAdapter::default(); @@ -221,7 +212,7 @@ impl PmrRbac { log::info!("new enforcer set up with {n} policies"); let mut result = Self { enforcer }; if anonymous_reader { - result.create_agent(None::<&str>).await?; + result.grant_agent_role(None::<&str>, Role::Reader).await?; } if let Some(resource_policy) = resource_policy { result.set_resource_policy(resource_policy).await?; @@ -234,35 +225,37 @@ impl PmrRbac { .unwrap_or("-".to_string()) } - /// Create agent assigns the reader role to the agent through the - /// second grouping policy - pub async fn create_agent( + /// Grant agent the role, which will enable the agent the role for + /// resources that have a policy attached for the role. + pub async fn grant_agent_role( &mut self, agent: Option + std::fmt::Display>, + role: Role, ) -> Result { self.enforcer.add_named_grouping_policy("g2", vec![ Self::to_agent(agent), - Role::Reader.into(), + role.into(), ]).await } - /// Remove agent removes the reader role for the second grouping. - pub async fn remove_agent( + /// Revoke an implicit role from agent. + pub async fn revoke_agent_role( &mut self, agent: impl AsRef + std::fmt::Display, + role: Role, ) -> Result { self.enforcer.remove_named_grouping_policy("g2", vec![ Self::to_agent(Some(agent)), - Role::Reader.into(), + role.into(), ]).await } /// Grant agent specified role at resource. /// Creates the relevant casbin grouping policy. - pub async fn grant( + pub async fn grant_res( &mut self, agent: Option + std::fmt::Display>, - role: impl Into, + role: Role, resource: impl Into, ) -> Result { self.enforcer.add_named_grouping_policy("g", vec![ @@ -274,10 +267,10 @@ impl PmrRbac { /// Revokes agent specified role at resource. /// Removes the relevant casbin grouping policy. - pub async fn revoke( + pub async fn revoke_res( &mut self, agent: Option + std::fmt::Display>, - role: impl Into, + role: Role, resource: impl Into, ) -> Result { self.enforcer.remove_named_grouping_policy("g", vec![ @@ -321,14 +314,16 @@ impl PmrRbac { pub async fn set_resource_policy( &mut self, - policy: ResourcePolicy, + policy: Policy, ) -> Result<(), Error> { - for Grant { res, agent, role } in policy.grants.iter() { - self.create_agent(agent.as_ref()).await?; - self.grant(agent.as_ref(), role, res).await?; + for UserRole { user, role } in policy.user_roles.into_iter() { + self.grant_agent_role(Some(user), role).await?; + } + for ResGrant { res, agent, role } in policy.res_grants.into_iter() { + self.grant_res(agent.as_ref(), role, res).await?; } - for Policy { role, endpoint_group, method } in policy.policies.iter() { - self.attach_policy(*role, policy.resource.clone(), endpoint_group, method).await?; + for RolePermit { role, endpoint_group, method } in policy.role_permits.into_iter() { + self.attach_policy(role, policy.resource.clone(), endpoint_group, method).await?; } Ok(()) } @@ -361,7 +356,7 @@ mod test { .build() .await?; // the rules don't actually work without the default - assert!(security.grant(Some("admin"), Role::Manager, "/*").await?); + assert!(security.grant_res(Some("admin"), Role::Manager, "/*").await?); assert!(!security.enforce(Some("admin"), "/exposure/1", "", "GET")?); Ok(()) } @@ -372,17 +367,17 @@ mod test { let not_logged_in: Option<&str> = None; // admin account has access to every part of the application - assert!(security.grant(Some("admin"), Role::Manager, "/*").await?); + assert!(security.grant_res(Some("admin"), Role::Manager, "/*").await?); // alice is the owner of exposure 1 - assert!(security.create_agent(Some("alice")).await?); - assert!(security.grant(Some("alice"), Role::Owner, "/exposure/1").await?); + assert!(security.grant_agent_role(Some("alice"), Role::Reader).await?); + assert!(security.grant_res(Some("alice"), Role::Owner, "/exposure/1").await?); // bob is the owner of exposure 2 - assert!(security.create_agent(Some("bob")).await?); - assert!(security.grant(Some("bob"), Role::Owner, "/exposure/2").await?); + assert!(security.grant_agent_role(Some("bob"), Role::Reader).await?); + assert!(security.grant_res(Some("bob"), Role::Owner, "/exposure/2").await?); // cathy is the editor of exposure 2 - assert!(security.grant(Some("cathy"), Role::Editor, "/exposure/2").await?); + assert!(security.grant_res(Some("cathy"), Role::Editor, "/exposure/2").await?); // create the anonymous agent also - assert!(security.create_agent(not_logged_in).await?); + assert!(security.grant_agent_role(not_logged_in, Role::Reader).await?); // make site root public assert!(security.attach_policy(Role::Reader, "/", "", "GET").await?); // make /exposure/1 public @@ -464,11 +459,12 @@ mod test { // a rough approximation of a private resource security.set_resource_policy(serde_json::from_str(r#"{ "resource": "/item/1", - "grants": [ + "user_roles": [], + "res_grants": [ {"res": "/*", "agent": "admin", "role": "Manager"}, {"res": "/item/1", "agent": "alice", "role": "Owner"} ], - "policies": [ + "role_permits": [ {"role": "Owner", "endpoint_group": "edit", "method": "GET"}, {"role": "Owner", "endpoint_group": "edit", "method": "POST"} ] @@ -490,15 +486,20 @@ mod test { #[tokio::test] async fn policy_usage_reviewer() -> anyhow::Result<()> { let not_logged_in: Option<&str> = None; - // a rough approximation of a resource under review - let policy: ResourcePolicy = serde_json::from_str(r#"{ + // a rough approximation of a resource under review, with reviewer + // having **global** access to everything + let policy: Policy = serde_json::from_str(r#"{ "resource": "/item/1", - "grants": [ + "user_roles": [ + {"user": "reviewer", "role": "Reader"}, + {"user": "reviewer", "role": "Reviewer"} + ], + "res_grants": [ {"res": "/*", "agent": "reviewer", "role": "Reviewer"}, {"res": "/item/1", "agent": "alice", "role": "Owner"} ], - "policies": [ - {"role": "Reviewer", "endpoint_group": "grant", "method": "POST"}, + "role_permits": [ + {"role": "Reviewer", "endpoint_group": "", "method": "GET"}, {"role": "Reviewer", "endpoint_group": "edit", "method": "GET"}, {"role": "Reviewer", "endpoint_group": "edit", "method": "POST"} ] @@ -520,6 +521,10 @@ mod test { // this wasn't granted globally by the model, so this should have no effect. assert!(!security.enforce(Some("reviewer"), "/item/1", "grant", "POST")?); + assert!(!security.enforce(Some("reviewer"), "/item/2", "", "GET")?); + assert!(!security.enforce(Some("reviewer"), "/item/2", "edit", "GET")?); + assert!(!security.enforce(Some("reviewer"), "/item/2", "edit", "POST")?); + let security = Builder::new_limited() .anonymous_reader(true) .resource_policy(policy.clone()) @@ -538,6 +543,50 @@ mod test { // this wasn't granted globally by the model, so this should have no effect. assert!(!security.enforce(Some("reviewer"), "/item/1", "grant", "POST")?); + assert!(!security.enforce(Some("reviewer"), "/item/2", "", "GET")?); + assert!(!security.enforce(Some("reviewer"), "/item/2", "edit", "GET")?); + assert!(!security.enforce(Some("reviewer"), "/item/2", "edit", "POST")?); + + Ok(()) + } + + #[tokio::test] + async fn policy_usage_reviewer_unconstrained() -> anyhow::Result<()> { + // a rough approximation of a resource under review + let policy: Policy = serde_json::from_str(r#"{ + "resource": "/item/1", + "user_roles": [], + "res_grants": [ + {"res": "/*", "agent": "reviewer", "role": "Reviewer"}, + {"res": "/item/1", "agent": "alice", "role": "Owner"} + ], + "role_permits": [ + {"role": "Reviewer", "endpoint_group": "", "method": "GET"}, + {"role": "Reviewer", "endpoint_group": "edit", "method": "GET"}, + {"role": "Reviewer", "endpoint_group": "edit", "method": "POST"} + ] + }"#)?; + + + let mut security = Builder::new() + .anonymous_reader(true) + .resource_policy(policy.clone()) + .build() + .await?; + // the resource policy doesn't provide additional underlying policies, so this is + // manually unconstrained to emulate the default policies defined for editor/manager + security.attach_policy(Role::Reviewer, "/*", "", "GET").await?; + security.attach_policy(Role::Reviewer, "/*", "edit", "GET").await?; + security.attach_policy(Role::Reviewer, "/*", "edit", "POST").await?; + + assert!(security.enforce(Some("reviewer"), "/item/1", "", "GET")?); + assert!(security.enforce(Some("reviewer"), "/item/1", "edit", "GET")?); + assert!(security.enforce(Some("reviewer"), "/item/1", "edit", "POST")?); + // note that /item/2 will also be permitted despite the resource only has /item/1 + assert!(security.enforce(Some("reviewer"), "/item/2", "", "GET")?); + assert!(security.enforce(Some("reviewer"), "/item/2", "edit", "GET")?); + assert!(security.enforce(Some("reviewer"), "/item/2", "edit", "POST")?); + // anyway, the model may be possible to have further simplification. Ok(()) } @@ -548,11 +597,12 @@ mod test { .anonymous_reader(true) .resource_policy(serde_json::from_str(r#"{ "resource": "/item/1", - "grants": [ + "user_roles": [], + "res_grants": [ {"res": "/*", "agent": "admin", "role": "Manager"}, {"res": "/item/1", "agent": "alice", "role": "Owner"} ], - "policies": [ + "role_permits": [ {"role": "Owner", "endpoint_group": "edit", "method": "GET"}, {"role": "Reader", "endpoint_group": "", "method": "GET"} ] @@ -581,11 +631,12 @@ mod test { .anonymous_reader(true) .resource_policy(serde_json::from_str(r#"{ "resource": "/item/1", - "grants": [ + "user_roles": [], + "res_grants": [ {"res": "/*", "agent": "admin", "role": "Manager"}, {"res": "/item/1", "agent": "alice", "role": "Owner"} ], - "policies": [ + "role_permits": [ {"role": "Owner", "endpoint_group": "edit", "method": "GET"}, {"role": "Reader", "endpoint_group": "", "method": "GET"} ]