Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(jans-cedarling): Policy Store: Parse Schema and Policies #9575

Merged
merged 18 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
97ea92a
feat(jans-cedarling): add PolicyStore and field schema. Also added de…
olehbozhok Sep 23, 2024
7dac32a
test(jans-cedarling): add unit tests to check `parse_cedar_schema` (d…
olehbozhok Sep 23, 2024
e01bc54
docs(jans-cedarling): add docs for PolicyStore
olehbozhok Sep 23, 2024
928b2bd
feat(jans-cedarling): add loading policy store based on config
olehbozhok Sep 23, 2024
bd0df57
feat(jans-cedarling): add loading policy store to Cedarling
olehbozhok Sep 23, 2024
3449b60
chore(jans-cedarling): rename LogType to LogTypeConfig
olehbozhok Sep 23, 2024
821cc2e
chore(jans-cedarling): fix `log_init` example after updating config
olehbozhok Sep 23, 2024
5cd1c79
chore(jans-cedarling): add allow(dead_code) on schema
olehbozhok Sep 23, 2024
d29218e
chore(jans-cedarling): add copyright notice
olehbozhok Sep 23, 2024
6b4ed70
docs(jans-cedarling): add README to init module
olehbozhok Sep 23, 2024
a1163bd
docs(jans-cedarling): add README to authz module
olehbozhok Sep 23, 2024
8d8cf8b
chore(jans-cedarling): update message in ErrorLoadPolicyStore::MoreTh…
olehbozhok Sep 23, 2024
bf89da3
chore(jans-cedarling): add comments to Cedarling::new
olehbozhok Sep 23, 2024
b6dacac
chore(jans-cedarling): remove unnecessary code
olehbozhok Sep 23, 2024
8cd92b3
docs(jans-cedarling): in README removed `Cedarling bindings` section
olehbozhok Sep 24, 2024
e724090
chore(jans-cedarling): move position of PolicyStoreMap to be first
olehbozhok Sep 24, 2024
cf20191
chore(jans-cedarling): refactor, move errors messages to the enum
olehbozhok Sep 24, 2024
df19054
Merge branch 'main' into jans-cedaling-issue-9568
moabu Sep 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
feat(jans-cedarling): add loading policy store to Cedarling
Signed-off-by: Oleh Bohzok <olehbozhok@gmail.com>
  • Loading branch information
olehbozhok committed Sep 23, 2024
commit bd0df572a0e4766da2b009cd6bf2f32e4105f20b
9 changes: 5 additions & 4 deletions jans-cedarling/cedarling/src/authz/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
use crate::log::{LogWriter, Logger};
use crate::models::authz_config::AuthzConfig;
use crate::models::log_entry::{LogEntry, LogType};
use uuid7::{uuid4, Uuid};
use crate::models::policy_store::PolicyStore;
use uuid7::Uuid;

/// Authorization Service
/// The primary service of the Cedarling application responsible for evaluating authorization requests.
Expand All @@ -22,13 +23,12 @@ pub struct Authz {
log_service: Logger,
pdp_id: Uuid,
application_name: String,
policy_store: PolicyStore,
}

impl Authz {
/// Create a new Authorization Service
pub fn new(config: AuthzConfig, log: Logger) -> Self {
// we use uuid v4 because it is generated based on random numbers.
let pdp_id = uuid4();
pub fn new(config: AuthzConfig, pdp_id: Uuid, log: Logger, policy_store: PolicyStore) -> Self {
let application_name = config.application_name;

log.log(
Expand All @@ -40,6 +40,7 @@ impl Authz {
log_service: log,
pdp_id,
application_name,
policy_store,
}
}
}
14 changes: 8 additions & 6 deletions jans-cedarling/cedarling/src/init/policy_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::models::policy_store_config::{PolicyStoreConfig, PolicyStoreSource};

/// Error cases for loading policy
#[derive(Debug, thiserror::Error)]
pub enum ErrorLoadPolicy {
pub enum ErrorLoadPolicyStore {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than creating different enums for showing error messages, can we just make a single enum for all error messages inside \jans-cedarling\cedarling\src\models\enum\error.

for Example:

pub enum CedarlingApplicationError {
 #[error("{0}")]
    LoadPolicyStoreJsonParseError(#[from] serde_json::Error),
    #[error("store policy is empty")]
    LoadPolicyStoreWithEmptyPolicy,
    #[error("the `store_key` is not specified and the count on policies more than 1")]
    LoadPolicyStoreMoreThanOnePolicy,
    #[error("could not found policy by id: {0}")]
    LoadPolicyStorePolicyNotFound(String),
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea to return the error struct in functions that cover all possible error cases for only the current situation. Because it will be easier to maintain when application grows.

And If we have somethinglike CedarlingApplicationError
It will have all possible errors for all application.

And when we will be wanted to handle the error cases with match statement, it will be hard to understand what exactly cases current function returns. Because CedarlingApplicationError have all possible cases in the application.

Copy link
Contributor

@duttarnab duttarnab Sep 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And If we have something like CedarlingApplicationError
It will have all possible errors for all application.

This is what we practise in most of applications. We do not clutter our project with multiple Enums for error messages.

And when we will be wanted to handle the error cases with match statement, it will be hard to understand what exactly cases current function returns. Because CedarlingApplicationError have all possible cases in the application.

The naming convention of enum items should be sufficient of make its purpose clear. We don't need to make multiple error enums for this.

And I could see ErrorLoadPolicyStore in cedar_schem.rs and policy_store.rs. Code duplication does not make sense.

#[error("{0}")]
JsonParce(#[from] serde_json::Error),
#[error("store policy is empty")]
Expand All @@ -17,22 +17,24 @@ pub enum ErrorLoadPolicy {
/// Load policy store based on config
//
// Unit tests will be added when will be implemented other types of sources
pub(crate) fn load_policy_store(config: PolicyStoreConfig) -> Result<PolicyStore, ErrorLoadPolicy> {
pub(crate) fn load_policy_store(
config: PolicyStoreConfig,
) -> Result<PolicyStore, ErrorLoadPolicyStore> {
let mut policy_store_map: PolicyStoreMap = match config.source {
PolicyStoreSource::Json(json_raw) => serde_json::from_str(json_raw.as_str())?,
};

if policy_store_map.policy_stores.is_empty() {
return Err(ErrorLoadPolicy::PolicyEmpty);
return Err(ErrorLoadPolicyStore::PolicyEmpty);
}

let policy: PolicyStore = match (config.store_id, policy_store_map.policy_stores.len()) {
(Some(store_id), _) => policy_store_map
.policy_stores
.remove(store_id.as_str())
.ok_or(ErrorLoadPolicy::FindPolicy(store_id))?,
.ok_or(ErrorLoadPolicyStore::FindPolicy(store_id))?,
(None, 0) => {
return Err(ErrorLoadPolicy::PolicyEmpty);
return Err(ErrorLoadPolicyStore::PolicyEmpty);
},
(None, 1) => {
// getting first element and we know it is save to use unwrap here,
Expand All @@ -45,7 +47,7 @@ pub(crate) fn load_policy_store(config: PolicyStoreConfig) -> Result<PolicyStore
.unwrap()
},
(None, 2..) => {
return Err(ErrorLoadPolicy::MoreThanOnePolicy);
return Err(ErrorLoadPolicyStore::MoreThanOnePolicy);
},
};

Expand Down
41 changes: 35 additions & 6 deletions jans-cedarling/cedarling/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,20 @@ mod tests;
use std::rc::Rc;

use authz::Authz;
use log::init_logger;
use init::policy_store::{load_policy_store, ErrorLoadPolicyStore};
pub use log::LogStorage;
use log::{init_logger, LogWriter};
pub use models::config::*;
use models::log_entry::{LogEntry, LogType};
use uuid7::uuid4;

/// Errors that can occur during initialization Cedarling.
#[derive(Debug, thiserror::Error)]
pub enum ErrorInitCedarling {
/// Error that may occur during loading the policy store.
#[error("Could not load policy :{0}")]
PolicyStore(#[from] ErrorLoadPolicyStore),
}

/// The instance of the Cedarling application.
#[derive(Clone)]
Expand All @@ -41,14 +52,32 @@ pub struct Cedarling {

impl Cedarling {
/// Create a new instance of the Cedarling application.
pub fn new(config: BootstrapConfig) -> Cedarling {
let log = init_logger(config.log_config);
let authz = Authz::new(config.authz_config, log.clone());
pub fn new(config: BootstrapConfig) -> Result<Cedarling, ErrorInitCedarling> {
let log: Rc<log::LogStrategy> = init_logger(config.log_config);
// we use uuid v4 because it is generated based on random numbers.
let pdp_id = uuid4();
let application_id = config.authz_config.application_name.clone();

let policy_store = load_policy_store(config.policy_store_config)
.inspect(|_| {
log.log(
LogEntry::new_with_data(pdp_id, application_id.clone(), LogType::System)
.set_message("PolicyStore loaded successfully".to_string()),
);
})
.inspect_err(|err| {
log.log(
LogEntry::new_with_data(pdp_id, application_id, LogType::System)
.set_message(format!("Could not load PolicyStore: {}", err.to_string())),
)
})?;

let authz = Authz::new(config.authz_config, pdp_id, log.clone(), policy_store);

Cedarling {
Ok(Cedarling {
log,
authz: Rc::new(authz),
}
})
}
}

Expand Down
3 changes: 3 additions & 0 deletions jans-cedarling/cedarling/src/models/bootstrap_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

use super::authz_config::AuthzConfig;
use super::log_config::LogConfig;
use super::policy_store_config::PolicyStoreConfig;

/// Bootstrap configuration
/// properties for configuration `Cedarling` application.
Expand All @@ -16,4 +17,6 @@ pub struct BootstrapConfig {
pub authz_config: AuthzConfig,
/// A set of properties used to configure logging in the `Cedarling` application.
pub log_config: LogConfig,
/// A set of properties used to load `PolicyStore` in the `Cedarling` application.
pub policy_store_config: PolicyStoreConfig,
}