Skip to content

Commit

Permalink
Cleanup in frontend code (#740)
Browse files Browse the repository at this point in the history 
Signed-off-by: Kesha Hietala <khieta@amazon.com>
  • Loading branch information
@khieta
khieta authored Mar 25, 2024
1 parent c16e932 commit cecb2c3
Show file tree
Hide file tree
Showing 3 changed files with 207 additions and 215 deletions.
130 changes: 38 additions & 92 deletions cedar-policy/src/frontend/is_authorized.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ use crate::api::EntityTypeName;
use crate::api::PartialResponse;
use crate::PolicyId;
use crate::{
Authorizer, Context, Decision, Entities, EntityUid, Policy, PolicySet, Request, Response,
Schema, SlotId, Template,
Authorizer, Context, Decision, Entities, EntityUid, PolicySet, Request, Response, Schema,
SlotId,
};
use cedar_policy_core::jsonvalue::JsonValueWithNoDuplicateKeys;
use itertools::Itertools;
Expand Down Expand Up @@ -55,7 +55,7 @@ fn is_authorized(call: AuthorizationCall) -> AuthorizationAnswer {
.into(),
})
}
Err(errors) => AuthorizationAnswer::ParseFailed { errors },
Err(errors) => AuthorizationAnswer::Failure { errors },
}
}

Expand All @@ -68,9 +68,7 @@ pub fn json_is_authorized(input: &str) -> InterfaceResult {
|e| InterfaceResult::fail_internally(format!("error parsing call: {e:}")),
|call| match is_authorized(call) {
answer @ AuthorizationAnswer::Success { .. } => InterfaceResult::succeed(answer),
AuthorizationAnswer::ParseFailed { errors } => {
InterfaceResult::fail_bad_request(errors)
}
AuthorizationAnswer::Failure { errors } => InterfaceResult::fail_bad_request(errors),
},
)
}
Expand All @@ -83,18 +81,18 @@ fn is_authorized_partial(call: AuthorizationCall) -> PartialAuthorizationAnswer
concrete_response @ PartialResponse::Concrete(_) => {
match concrete_response.try_into() {
Ok(response) => PartialAuthorizationAnswer::Concrete { response },
Err(errors) => PartialAuthorizationAnswer::ParseFailed { errors },
Err(errors) => PartialAuthorizationAnswer::Failure { errors },
}
}
residual_response @ PartialResponse::Residual(_) => {
match residual_response.try_into() {
Ok(response) => PartialAuthorizationAnswer::Residuals { response },
Err(errors) => PartialAuthorizationAnswer::ParseFailed { errors },
Err(errors) => PartialAuthorizationAnswer::Failure { errors },
}
}
}
}),
Err(errors) => PartialAuthorizationAnswer::ParseFailed { errors },
Err(errors) => PartialAuthorizationAnswer::Failure { errors },
}
}

Expand All @@ -110,7 +108,7 @@ pub fn json_is_authorized_partial(input: &str) -> InterfaceResult {
|call| match is_authorized_partial(call) {
answer @ (PartialAuthorizationAnswer::Concrete { .. }
| PartialAuthorizationAnswer::Residuals { .. }) => InterfaceResult::succeed(answer),
PartialAuthorizationAnswer::ParseFailed { errors } => {
PartialAuthorizationAnswer::Failure { errors } => {
InterfaceResult::fail_bad_request(errors)
}
},
Expand Down Expand Up @@ -262,35 +260,43 @@ impl TryFrom<PartialResponse> for InterfaceResidualResponse {
}
}

/// Answer struct from authorization call
#[derive(Debug, Serialize, Deserialize)]
#[serde(untagged)]
#[cfg_attr(feature = "wasm", derive(tsify::Tsify))]
#[cfg_attr(feature = "wasm", tsify(into_wasm_abi, from_wasm_abi))]
enum AuthorizationAnswer {
ParseFailed { errors: Vec<String> },
/// Represents a failure to parse or call the authorizer
Failure { errors: Vec<String> },
/// Represents a successful authorization call
Success { response: InterfaceResponse },
}

#[cfg(feature = "partial-eval")]
#[derive(Debug, Serialize, Deserialize)]
#[serde(untagged)]
enum PartialAuthorizationAnswer {
ParseFailed { errors: Vec<String> },
Failure { errors: Vec<String> },
Concrete { response: InterfaceResponse },
Residuals { response: InterfaceResidualResponse },
}

/// Struct containing the input data for authorization
#[serde_as]
#[derive(Debug, Serialize, Deserialize)]
#[cfg_attr(feature = "wasm", derive(tsify::Tsify))]
#[cfg_attr(feature = "wasm", tsify(into_wasm_abi, from_wasm_abi))]
struct AuthorizationCall {
/// The principal taking action
#[cfg_attr(feature = "wasm", tsify(type = "string|{type: string, id: string}"))]
principal: Option<JsonValueWithNoDuplicateKeys>,
/// The action the principal is taking
#[cfg_attr(feature = "wasm", tsify(type = "string|{type: string, id: string}"))]
action: JsonValueWithNoDuplicateKeys,
/// The resource being acted on by the principal
#[cfg_attr(feature = "wasm", tsify(type = "string|{type: string, id: string}"))]
resource: Option<JsonValueWithNoDuplicateKeys>,
/// The context details specific to the request
#[serde_as(as = "MapPreventDuplicates<_, _>")]
#[cfg_attr(
feature = "wasm",
Expand All @@ -310,6 +316,7 @@ struct AuthorizationCall {
/// If a schema is not provided, this option has no effect.
#[serde(default = "constant_true")]
enable_request_validation: bool,
/// The slice containing entities and policies
slice: RecvdSlice,
}

Expand Down Expand Up @@ -557,19 +564,7 @@ impl RecvdSlice {
template_instantiations,
} = self;

let policy_set = match policies {
PolicySpecification::Concatenated(policies) => match PolicySet::from_str(&policies) {
Ok(ps) => Ok(ps),
Err(parse_errors) => Err(std::iter::once(
"couldn't parse concatenated policies string".to_string(),
)
.chain(parse_errors.errors_as_strings())
.collect()),
},
PolicySpecification::Map(policies) => {
parse_policy_set_from_individual_policies(&policies, templates)
}
};
let policy_set: Result<PolicySet, Vec<String>> = policies.try_into(templates);

let mut errs = Vec::new();

Expand Down Expand Up @@ -610,51 +605,6 @@ impl RecvdSlice {
}
}

fn parse_policy_set_from_individual_policies(
policies: &HashMap<String, String>,
templates: Option<HashMap<String, String>>,
) -> Result<PolicySet, Vec<String>> {
let mut policy_set = PolicySet::new();
let mut errs = Vec::new();
for (id, policy_src) in policies {
match Policy::parse(Some(id.clone()), policy_src) {
Ok(p) => match policy_set.add(p) {
Ok(()) => {}
Err(err) => {
errs.push(format!("couldn't add policy to set due to error: {err}"));
}
},
Err(pes) => errs.extend(
std::iter::once(format!("couldn't parse policy with id `{id}`"))
.chain(pes.errors_as_strings().into_iter()),
),
}
}

if let Some(templates) = templates {
for (id, policy_src) in templates {
match Template::parse(Some(id.clone()), policy_src) {
Ok(p) => match policy_set.add_template(p) {
Ok(()) => {}
Err(err) => {
errs.push(format!("couldn't add policy to set due to error: {err}"));
}
},
Err(pes) => errs.extend(
std::iter::once(format!("couldn't parse policy with id `{id}`"))
.chain(pes.errors_as_strings().into_iter()),
),
}
}
}

if errs.is_empty() {
Ok(policy_set)
} else {
Err(errs)
}
}

// PANIC SAFETY unit tests
#[allow(clippy::panic)]
#[cfg(test)]
Expand Down Expand Up @@ -723,7 +673,7 @@ mod test {
assert_is_failure(
&json_is_authorized("iefjieoafiaeosij"),
true,
"expected value",
"error parsing call: expected value",
);
}

Expand Down Expand Up @@ -1271,11 +1221,7 @@ mod test {
]
}
}"#;
assert_is_failure(
&json_is_authorized(call),
false,
"Error instantiating template: unable to link template: template-linked policy id `ID1` conflicts with an existing policy id",
);
assert_is_failure(&json_is_authorized(call), false, "Error instantiating template: unable to link template: template-linked policy id `ID1` conflicts with an existing policy id");
}

#[test]
Expand Down Expand Up @@ -1312,11 +1258,7 @@ mod test {
]
}
}"#;
assert_is_failure(
&json_is_authorized(call),
false,
"Error instantiating template: unable to link template: template-linked policy id `ID0` conflicts with an existing policy id",
);
assert_is_failure(&json_is_authorized(call), false, "Error instantiating template: unable to link template: template-linked policy id `ID0` conflicts with an existing policy id");
}

#[test]
Expand Down Expand Up @@ -1353,11 +1295,7 @@ mod test {
]
}
}"#;
assert_is_failure(
&json_is_authorized(call),
false,
"Error instantiating template: unable to link template: template-linked policy id `ID1` conflicts with an existing policy id",
);
assert_is_failure(&json_is_authorized(call), false, "Error instantiating template: unable to link template: template-linked policy id `ID1` conflicts with an existing policy id");
}

#[track_caller] // report the caller's location as the location of the panic, not the location in this function
Expand Down Expand Up @@ -1401,7 +1339,7 @@ mod test {
"template_instantiations" : [ ]
}
}"#;
assert_is_failure(&json_is_authorized(call), true, "no duplicate IDs");
assert_is_failure(&json_is_authorized(call), true, "error parsing call: policies as a concatenated string or multiple policies as a hashmap where the policy id is the key");
}

#[test]
Expand All @@ -1421,7 +1359,11 @@ mod test {
"template_instantiations" : [ ]
}
}"#;
assert_is_failure(&json_is_authorized(call), true, "found duplicate key");
assert_is_failure(
&json_is_authorized(call),
true,
"error parsing call: invalid entry: found duplicate key",
);
}

#[test]
Expand Down Expand Up @@ -1456,7 +1398,7 @@ mod test {
assert_is_failure(
&json_is_authorized(call),
true,
"duplicate instantiations of the slot(s): `?principal`",
"error parsing call: duplicate instantiations of the slot(s): `?principal`",
);
}

Expand Down Expand Up @@ -1496,7 +1438,7 @@ mod test {
assert_is_failure(
&json_is_authorized(call),
true,
"duplicate instantiations of the slot(s): `?principal`",
"error parsing call: duplicate instantiations of the slot(s): `?principal`",
);
}

Expand Down Expand Up @@ -1540,7 +1482,7 @@ mod test {
assert_is_failure(
&json_is_authorized(call),
true,
"duplicate instantiations of the slot(s): `?principal`, `?resource`",
"error parsing call: duplicate instantiations of the slot(s): `?principal`, `?resource`",
);
}

Expand Down Expand Up @@ -1617,7 +1559,11 @@ mod test {
"template_instantiations" : []
}
}"#;
assert_is_failure(&json_is_authorized(call), true, "found duplicate key");
assert_is_failure(
&json_is_authorized(call),
true,
"error parsing call: invalid entry: found duplicate key",
);
}

#[cfg(feature = "partial-eval")]
Expand Down
Loading

0 comments on commit cecb2c3

Please sign in to comment.