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.

Sign up for GitHub

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

Jump to bottom

Use PE to implement GetLists #121

Merged
merged 6 commits into from
Jun 17, 2024
Merged

Use PE to implement GetLists #121

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fd43075
Use PE to implement `GetLists`
shaobo-he-aws May 13, 2024
ae8e8a2
Merge remote-tracking branch 'origin/main' into feature/shaobo/pe
shaobo-he-aws Jun 3, 2024
a0131ea
cargo fmt
shaobo-he-aws Jun 3, 2024
ee4da36
restored GetLists check
shaobo-he-aws Jun 3, 2024
fbd21fa
minor fix
shaobo-he-aws Jun 3, 2024
21bf560
Merge remote-tracking branch 'origin/main' into feature/shaobo/pe
shaobo-he-aws Jun 14, 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
1 change: 1 addition & 0 deletions tinytodo/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ notify = { version = "5.1.0", default-features = false, features = ["macos_kqueu
use-templates = []

[dependencies.cedar-policy]
features = ["partial-eval"]
version = "4.0.0"
git = "https://github.com/cedar-policy/cedar"
branch = "main"
Expand Down
47 changes: 34 additions & 13 deletions tinytodo/src/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,13 @@

use itertools::Itertools;
use lazy_static::lazy_static;
use std::path::PathBuf;
use std::{collections::HashMap, path::PathBuf};
use tracing::{error, info, trace};

use cedar_policy::{
schema_error::SchemaError, Authorizer, Context, Decision, Diagnostics, HumanSchemaError,
ParseErrors, PolicySet, PolicySetError, Request, Schema, ValidationMode, Validator,
schema_error::SchemaError, Authorizer, Context, Decision, Diagnostics, Entities,
HumanSchemaError, ParseErrors, PolicySet, PolicySetError, Request, RequestBuilder,
RestrictedExpression, Schema, ValidationMode, Validator,
};

use thiserror::Error;
Expand All @@ -45,8 +46,6 @@ use crate::{
use crate::{api::ShareRole, util::UserOrTeamUid};
#[cfg(feature = "use-templates")]
use cedar_policy::{PolicyId, SlotId};
#[cfg(feature = "use-templates")]
use std::collections::HashMap;

// There's almost certainly a nicer way to do this than having separate `sender` fields

Expand Down Expand Up @@ -512,17 +511,39 @@ impl AppContext {

fn get_lists(&self, r: GetLists) -> Result<AppResponse> {
self.is_authorized(&r.uid, &*ACTION_GET_LISTS, &*APPLICATION_TINY_TODO)?;
let entities: Entities = self.entities.as_entities(&self.schema);
let partial_request = RequestBuilder::default()
.action(Some(ACTION_GET_LIST.as_ref().clone().into()))
.principal(Some(cedar_policy::EntityUid::from(EntityUid::from(
r.uid.clone(),
))))
.build();
let partial_response =
self.authorizer
.is_authorized_partial(&partial_request, &self.policies, &entities);
Comment on lines +521 to +523
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add a comment explain what's going on here, because readers might be confused.

What we are trying to simulate is the following:

  • For all entities L of type List, is_authorized(principal,GET_LIST, L ) ? If so, add to collection

What we are doing here instead is

  • Let res = is_authorized_partial(principal,GET_LIST, unknown ).
  • For all entities L of type List, is res .reauthorize( unknown = L ) ? If so, add to collection

The benefit is that we are partially evaluating some of the policies so the reauthorization requires less work. Even more, we imagine that a future implementation of the entity store can implement the reauthorize logic more directly/efficiently, e.g., as a DB query.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Noted. Will fix it in another PR.


let slice = self.entities.get_lists();

Ok(AppResponse::Lists(
self.entities
.get_lists()
.filter(|t| {
self.is_authorized(&r.uid, &*ACTION_GET_LIST, t.uid())
.is_ok()
})
slice
.filter(|t| matches!(
partial_response.reauthorize(
HashMap::from_iter(
std::iter::once(
("resource".into(), RestrictedExpression::new_entity_uid(EntityUid::from(t.uid().clone()).into()))
)
),
&self.authorizer,
Request::new(
None,
None,
None,
Context::empty(),
None).expect("should be a valid request"),
&entities),
Ok(r) if matches!(r.decision(), Some(Decision::Allow))))
.cloned()
.collect::<Vec<List>>()
.into(),
.collect::<Vec<List>>(),
))
}

Expand Down
Loading