From 814de0d879b731db3024d3cacbde60447b016ecf Mon Sep 17 00:00:00 2001 From: David Calavera Date: Tue, 2 Apr 2024 21:10:11 -0700 Subject: [PATCH 1/2] Cleanup IAM definitions We have two different definitions of IAM policies and statement. These changes centralize them both into one. These changes also add the `Condition` field that was missing from both implementations. These changes also make `Effect` to be an enum with the default `Allow`, instead of an optional string. This is more ergonomic than checking whether the effect is none or `allow`. Signed-off-by: David Calavera --- lambda-events/Cargo.toml | 2 +- lambda-events/src/custom_serde/iam.rs | 127 ++++++++++++++++++ lambda-events/src/custom_serde/mod.rs | 19 +-- lambda-events/src/event/apigw/mod.rs | 42 +++--- lambda-events/src/event/iam/mod.rs | 38 ++++-- ...w-custom-auth-response-with-condition.json | 30 +++++ .../example-apigw-custom-auth-response.json | 40 +++--- 7 files changed, 240 insertions(+), 58 deletions(-) create mode 100644 lambda-events/src/custom_serde/iam.rs create mode 100644 lambda-events/src/fixtures/example-apigw-custom-auth-response-with-condition.json diff --git a/lambda-events/Cargo.toml b/lambda-events/Cargo.toml index eb5d9c31..8f2fca99 100644 --- a/lambda-events/Cargo.toml +++ b/lambda-events/Cargo.toml @@ -85,7 +85,7 @@ default = [ activemq = [] alb = ["bytes", "http", "http-body", "http-serde", "query_map"] -apigw = ["bytes", "http", "http-body", "http-serde", "query_map"] +apigw = ["bytes", "http", "http-body", "http-serde", "iam", "query_map"] appsync = [] autoscaling = ["chrono"] bedrock_agent_runtime = [] diff --git a/lambda-events/src/custom_serde/iam.rs b/lambda-events/src/custom_serde/iam.rs new file mode 100644 index 00000000..ef256ee0 --- /dev/null +++ b/lambda-events/src/custom_serde/iam.rs @@ -0,0 +1,127 @@ +use std::{borrow::Cow, collections::HashMap, fmt}; + +use serde::{ + de::{Error as DeError, MapAccess, Visitor}, + Deserialize, Deserializer, Serialize, +}; + +use super::StringOrSlice; + +pub type IamPolicyCondition = HashMap>>; + +#[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)] +pub enum IamPolicyEffect { + #[default] + Allow, + Deny, +} + +pub(crate) fn deserialize_policy_condition<'de, D>(de: D) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + de.deserialize_option(IamPolicyConditionVisitor) +} + +struct IamPolicyConditionVisitor; + +impl<'de> Visitor<'de> for IamPolicyConditionVisitor { + type Value = Option; + + // Format a message stating what data this Visitor expects to receive. + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str("lots of things can go wrong with a IAM Policy Condition") + } + + fn visit_unit(self) -> Result + where + E: DeError, + { + Ok(None) + } + + fn visit_none(self) -> Result + where + E: DeError, + { + Ok(None) + } + + fn visit_some(self, deserializer: D) -> Result + where + D: Deserializer<'de>, + { + deserializer.deserialize_map(self) + } + + fn visit_map(self, mut access: M) -> Result + where + M: MapAccess<'de>, + { + let mut map = HashMap::with_capacity(access.size_hint().unwrap_or(0)); + + while let Some((key, val)) = access.next_entry::, HashMap, StringOrSlice>>()? { + let mut value = HashMap::with_capacity(val.len()); + for (val_key, string_or_slice) in val { + let val = match string_or_slice { + StringOrSlice::Slice(slice) => slice, + StringOrSlice::String(s) => vec![s], + }; + value.insert(val_key.into_owned(), val); + } + + map.insert(key.into_owned(), value); + } + + Ok(Some(map)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_deserialize_string_condition() { + let data = serde_json::json!({ + "condition": { + "StringEquals": { + "iam:RegisterSecurityKey": "Activate", + "iam:FIDO-certification": "L1plus" + } + } + }); + + #[derive(Deserialize)] + struct Test { + #[serde(deserialize_with = "deserialize_policy_condition")] + condition: Option, + } + + let test: Test = serde_json::from_value(data).unwrap(); + let condition = test.condition.unwrap(); + assert_eq!(1, condition.len()); + + assert_eq!(vec!["Activate"], condition["StringEquals"]["iam:RegisterSecurityKey"]); + assert_eq!(vec!["L1plus"], condition["StringEquals"]["iam:FIDO-certification"]); + } + + #[test] + fn test_deserialize_slide_condition() { + let data = serde_json::json!({ + "condition": {"StringLike": {"s3:prefix": ["janedoe/*"]}} + }); + + #[derive(Deserialize)] + struct Test { + #[serde(deserialize_with = "deserialize_policy_condition")] + condition: Option, + } + + let test: Test = serde_json::from_value(data).unwrap(); + let condition = test.condition.unwrap(); + assert_eq!(1, condition.len()); + + assert_eq!(vec!["janedoe/*"], condition["StringLike"]["s3:prefix"]); + } +} diff --git a/lambda-events/src/custom_serde/mod.rs b/lambda-events/src/custom_serde/mod.rs index 9d68c8d3..1e403915 100644 --- a/lambda-events/src/custom_serde/mod.rs +++ b/lambda-events/src/custom_serde/mod.rs @@ -31,6 +31,9 @@ pub(crate) mod float_unix_epoch; #[cfg(any(feature = "alb", feature = "apigw"))] pub(crate) mod http_method; +#[cfg(any(feature = "iam", test))] +pub(crate) mod iam; + pub(crate) fn deserialize_base64<'de, D>(deserializer: D) -> Result, D::Error> where D: Deserializer<'de>, @@ -92,19 +95,19 @@ where Ok(opt.unwrap_or_default()) } +#[derive(serde::Deserialize)] +#[serde(untagged)] +pub(crate) enum StringOrSlice { + String(String), + Slice(Vec), +} + /// Deserializes `Vec`, from a JSON `string` or `[string]`. -#[cfg(any(feature = "apigw", test))] +#[cfg(any(feature = "iam", test))] pub(crate) fn deserialize_string_or_slice<'de, D>(deserializer: D) -> Result, D::Error> where D: Deserializer<'de>, { - #[derive(serde::Deserialize)] - #[serde(untagged)] - enum StringOrSlice { - String(String), - Slice(Vec), - } - let string_or_slice = StringOrSlice::deserialize(deserializer)?; match string_or_slice { diff --git a/lambda-events/src/event/apigw/mod.rs b/lambda-events/src/event/apigw/mod.rs index 1777cf76..533bb77e 100644 --- a/lambda-events/src/event/apigw/mod.rs +++ b/lambda-events/src/event/apigw/mod.rs @@ -1,8 +1,9 @@ use crate::custom_serde::{ - deserialize_headers, deserialize_lambda_map, deserialize_nullish_boolean, deserialize_string_or_slice, http_method, - serialize_headers, serialize_multi_value_headers, + deserialize_headers, deserialize_lambda_map, deserialize_nullish_boolean, http_method, serialize_headers, + serialize_multi_value_headers, }; use crate::encodings::Body; +use crate::iam::IamPolicyStatement; use http::{HeaderMap, Method}; use query_map::QueryMap; use serde::{de::DeserializeOwned, ser::SerializeMap, Deserialize, Deserializer, Serialize, Serializer}; @@ -723,30 +724,13 @@ where /// `ApiGatewayCustomAuthorizerPolicy` represents an IAM policy #[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)] -#[serde(rename_all = "camelCase")] +#[serde(rename_all = "PascalCase")] pub struct ApiGatewayCustomAuthorizerPolicy { #[serde(default)] - #[serde(rename = "Version")] pub version: Option, - #[serde(rename = "Statement")] pub statement: Vec, } -/// `IamPolicyStatement` represents one statement from IAM policy with action, effect and resource -#[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)] -#[serde(rename_all = "camelCase")] -pub struct IamPolicyStatement { - #[serde(rename = "Action")] - #[serde(deserialize_with = "deserialize_string_or_slice")] - pub action: Vec, - #[serde(default)] - #[serde(rename = "Effect")] - pub effect: Option, - #[serde(rename = "Resource")] - #[serde(deserialize_with = "deserialize_string_or_slice")] - pub resource: Vec, -} - fn default_http_method() -> Method { Method::GET } @@ -1045,4 +1029,22 @@ mod test { assert_eq!(Some(1), fields.get("clientId").unwrap().as_u64()); assert_eq!(Some("Exata"), fields.get("clientName").unwrap().as_str()); } + + #[test] + #[cfg(feature = "apigw")] + fn example_apigw_custom_auth_response_with_statement_condition() { + use crate::iam::IamPolicyEffect; + + let data = include_bytes!("../../fixtures/example-apigw-custom-auth-response-with-condition.json"); + let parsed: ApiGatewayCustomAuthorizerResponse = serde_json::from_slice(data).unwrap(); + let output: String = serde_json::to_string(&parsed).unwrap(); + let reparsed: ApiGatewayCustomAuthorizerResponse = serde_json::from_slice(output.as_bytes()).unwrap(); + assert_eq!(parsed, reparsed); + + let statement = parsed.policy_document.statement.first().unwrap(); + assert_eq!(IamPolicyEffect::Deny, statement.effect); + + let condition = statement.condition.as_ref().unwrap(); + assert_eq!(vec!["xxx"], condition["StringEquals"]["aws:SourceIp"]); + } } diff --git a/lambda-events/src/event/iam/mod.rs b/lambda-events/src/event/iam/mod.rs index 12bf7ba9..55e55106 100644 --- a/lambda-events/src/event/iam/mod.rs +++ b/lambda-events/src/event/iam/mod.rs @@ -1,25 +1,41 @@ +use std::collections::HashMap; + use serde::{Deserialize, Serialize}; +use crate::custom_serde::{deserialize_string_or_slice, iam::deserialize_policy_condition}; + /// `IamPolicyDocument` represents an IAM policy document. #[derive(Debug, Clone, Eq, PartialEq, Deserialize, Serialize)] -#[serde(rename_all = "camelCase")] +#[serde(rename_all = "PascalCase")] pub struct IamPolicyDocument { #[serde(default)] - #[serde(rename = "Version")] pub version: Option, - #[serde(rename = "Statement")] pub statement: Vec, } -/// `IamPolicyStatement` represents one statement from IAM policy with action, effect and resource. -#[derive(Debug, Clone, Eq, PartialEq, Deserialize, Serialize)] -#[serde(rename_all = "camelCase")] +/// `IamPolicyStatement` represents one statement from IAM policy with action, effect and resource +#[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)] +#[serde(rename_all = "PascalCase")] pub struct IamPolicyStatement { - #[serde(rename = "Action")] + #[serde(deserialize_with = "deserialize_string_or_slice")] pub action: Vec, - #[serde(default)] - #[serde(rename = "Effect")] - pub effect: Option, - #[serde(rename = "Resource")] + #[serde(default = "default_statement_effect")] + pub effect: IamPolicyEffect, + #[serde(deserialize_with = "deserialize_string_or_slice")] pub resource: Vec, + #[serde(default, deserialize_with = "deserialize_policy_condition")] + pub condition: Option, +} + +pub type IamPolicyCondition = HashMap>>; + +#[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)] +pub enum IamPolicyEffect { + #[default] + Allow, + Deny, +} + +fn default_statement_effect() -> IamPolicyEffect { + IamPolicyEffect::Allow } diff --git a/lambda-events/src/fixtures/example-apigw-custom-auth-response-with-condition.json b/lambda-events/src/fixtures/example-apigw-custom-auth-response-with-condition.json new file mode 100644 index 00000000..53a09b39 --- /dev/null +++ b/lambda-events/src/fixtures/example-apigw-custom-auth-response-with-condition.json @@ -0,0 +1,30 @@ +{ + "principalId": "yyyyyyyy", + "policyDocument": { + "Version": "2012-10-17", + "Statement": [ + { + "Action": [ + "execute-api:Invoke" + ], + "Effect": "Deny", + "Resource": [ + "arn:aws:execute-api:{regionId}:{accountId}:{appId}/{stage}/{httpVerb}/[{resource}/[child-resources]]" + ], + "Condition": { + "StringEquals": { + "aws:SourceIp": [ + "xxx" + ] + } + } + } + ] + }, + "context": { + "stringKey": "value", + "numberKey": "1", + "booleanKey": "true" + }, + "usageIdentifierKey": "{api-key}" +} \ No newline at end of file diff --git a/lambda-events/src/fixtures/example-apigw-custom-auth-response.json b/lambda-events/src/fixtures/example-apigw-custom-auth-response.json index 9b624141..b1502cde 100644 --- a/lambda-events/src/fixtures/example-apigw-custom-auth-response.json +++ b/lambda-events/src/fixtures/example-apigw-custom-auth-response.json @@ -1,19 +1,23 @@ { - "principalId": "yyyyyyyy", - "policyDocument": { - "Version": "2012-10-17", - "Statement": [ - { - "Action": ["execute-api:Invoke"], - "Effect": "Allow|Deny", - "Resource": ["arn:aws:execute-api:{regionId}:{accountId}:{appId}/{stage}/{httpVerb}/[{resource}/[child-resources]]"] - } - ] - }, - "context": { - "stringKey": "value", - "numberKey": "1", - "booleanKey": "true" - }, - "usageIdentifierKey": "{api-key}" -} + "principalId": "yyyyyyyy", + "policyDocument": { + "Version": "2012-10-17", + "Statement": [ + { + "Action": [ + "execute-api:Invoke" + ], + "Effect": "Deny", + "Resource": [ + "arn:aws:execute-api:{regionId}:{accountId}:{appId}/{stage}/{httpVerb}/[{resource}/[child-resources]]" + ] + } + ] + }, + "context": { + "stringKey": "value", + "numberKey": "1", + "booleanKey": "true" + }, + "usageIdentifierKey": "{api-key}" +} \ No newline at end of file From 61c3d9ce55eed88d7147ec9b83a66eac374efd0c Mon Sep 17 00:00:00 2001 From: David Calavera Date: Wed, 3 Apr 2024 07:56:13 -0700 Subject: [PATCH 2/2] Remove unnecesary module. Keep all deserialization logic together, it's easier to manage. Signed-off-by: David Calavera --- lambda-events/src/custom_serde/iam.rs | 127 ----------------------- lambda-events/src/custom_serde/mod.rs | 24 ----- lambda-events/src/event/iam/mod.rs | 139 +++++++++++++++++++++++++- 3 files changed, 135 insertions(+), 155 deletions(-) delete mode 100644 lambda-events/src/custom_serde/iam.rs diff --git a/lambda-events/src/custom_serde/iam.rs b/lambda-events/src/custom_serde/iam.rs deleted file mode 100644 index ef256ee0..00000000 --- a/lambda-events/src/custom_serde/iam.rs +++ /dev/null @@ -1,127 +0,0 @@ -use std::{borrow::Cow, collections::HashMap, fmt}; - -use serde::{ - de::{Error as DeError, MapAccess, Visitor}, - Deserialize, Deserializer, Serialize, -}; - -use super::StringOrSlice; - -pub type IamPolicyCondition = HashMap>>; - -#[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)] -pub enum IamPolicyEffect { - #[default] - Allow, - Deny, -} - -pub(crate) fn deserialize_policy_condition<'de, D>(de: D) -> Result, D::Error> -where - D: Deserializer<'de>, -{ - de.deserialize_option(IamPolicyConditionVisitor) -} - -struct IamPolicyConditionVisitor; - -impl<'de> Visitor<'de> for IamPolicyConditionVisitor { - type Value = Option; - - // Format a message stating what data this Visitor expects to receive. - fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - formatter.write_str("lots of things can go wrong with a IAM Policy Condition") - } - - fn visit_unit(self) -> Result - where - E: DeError, - { - Ok(None) - } - - fn visit_none(self) -> Result - where - E: DeError, - { - Ok(None) - } - - fn visit_some(self, deserializer: D) -> Result - where - D: Deserializer<'de>, - { - deserializer.deserialize_map(self) - } - - fn visit_map(self, mut access: M) -> Result - where - M: MapAccess<'de>, - { - let mut map = HashMap::with_capacity(access.size_hint().unwrap_or(0)); - - while let Some((key, val)) = access.next_entry::, HashMap, StringOrSlice>>()? { - let mut value = HashMap::with_capacity(val.len()); - for (val_key, string_or_slice) in val { - let val = match string_or_slice { - StringOrSlice::Slice(slice) => slice, - StringOrSlice::String(s) => vec![s], - }; - value.insert(val_key.into_owned(), val); - } - - map.insert(key.into_owned(), value); - } - - Ok(Some(map)) - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_deserialize_string_condition() { - let data = serde_json::json!({ - "condition": { - "StringEquals": { - "iam:RegisterSecurityKey": "Activate", - "iam:FIDO-certification": "L1plus" - } - } - }); - - #[derive(Deserialize)] - struct Test { - #[serde(deserialize_with = "deserialize_policy_condition")] - condition: Option, - } - - let test: Test = serde_json::from_value(data).unwrap(); - let condition = test.condition.unwrap(); - assert_eq!(1, condition.len()); - - assert_eq!(vec!["Activate"], condition["StringEquals"]["iam:RegisterSecurityKey"]); - assert_eq!(vec!["L1plus"], condition["StringEquals"]["iam:FIDO-certification"]); - } - - #[test] - fn test_deserialize_slide_condition() { - let data = serde_json::json!({ - "condition": {"StringLike": {"s3:prefix": ["janedoe/*"]}} - }); - - #[derive(Deserialize)] - struct Test { - #[serde(deserialize_with = "deserialize_policy_condition")] - condition: Option, - } - - let test: Test = serde_json::from_value(data).unwrap(); - let condition = test.condition.unwrap(); - assert_eq!(1, condition.len()); - - assert_eq!(vec!["janedoe/*"], condition["StringLike"]["s3:prefix"]); - } -} diff --git a/lambda-events/src/custom_serde/mod.rs b/lambda-events/src/custom_serde/mod.rs index 1e403915..030cb5b3 100644 --- a/lambda-events/src/custom_serde/mod.rs +++ b/lambda-events/src/custom_serde/mod.rs @@ -31,9 +31,6 @@ pub(crate) mod float_unix_epoch; #[cfg(any(feature = "alb", feature = "apigw"))] pub(crate) mod http_method; -#[cfg(any(feature = "iam", test))] -pub(crate) mod iam; - pub(crate) fn deserialize_base64<'de, D>(deserializer: D) -> Result, D::Error> where D: Deserializer<'de>, @@ -95,27 +92,6 @@ where Ok(opt.unwrap_or_default()) } -#[derive(serde::Deserialize)] -#[serde(untagged)] -pub(crate) enum StringOrSlice { - String(String), - Slice(Vec), -} - -/// Deserializes `Vec`, from a JSON `string` or `[string]`. -#[cfg(any(feature = "iam", test))] -pub(crate) fn deserialize_string_or_slice<'de, D>(deserializer: D) -> Result, D::Error> -where - D: Deserializer<'de>, -{ - let string_or_slice = StringOrSlice::deserialize(deserializer)?; - - match string_or_slice { - StringOrSlice::Slice(slice) => Ok(slice), - StringOrSlice::String(s) => Ok(vec![s]), - } -} - #[cfg(test)] #[allow(deprecated)] mod test { diff --git a/lambda-events/src/event/iam/mod.rs b/lambda-events/src/event/iam/mod.rs index 55e55106..ee35bbc8 100644 --- a/lambda-events/src/event/iam/mod.rs +++ b/lambda-events/src/event/iam/mod.rs @@ -1,8 +1,9 @@ -use std::collections::HashMap; +use std::{borrow::Cow, collections::HashMap, fmt}; -use serde::{Deserialize, Serialize}; - -use crate::custom_serde::{deserialize_string_or_slice, iam::deserialize_policy_condition}; +use serde::{ + de::{Error as DeError, MapAccess, Visitor}, + Deserialize, Deserializer, Serialize, +}; /// `IamPolicyDocument` represents an IAM policy document. #[derive(Debug, Clone, Eq, PartialEq, Deserialize, Serialize)] @@ -39,3 +40,133 @@ pub enum IamPolicyEffect { fn default_statement_effect() -> IamPolicyEffect { IamPolicyEffect::Allow } + +#[derive(serde::Deserialize)] +#[serde(untagged)] +enum StringOrSlice { + String(String), + Slice(Vec), +} + +/// Deserializes `Vec`, from a JSON `string` or `[string]`. +fn deserialize_string_or_slice<'de, D>(deserializer: D) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + let string_or_slice = StringOrSlice::deserialize(deserializer)?; + + match string_or_slice { + StringOrSlice::Slice(slice) => Ok(slice), + StringOrSlice::String(s) => Ok(vec![s]), + } +} + +fn deserialize_policy_condition<'de, D>(de: D) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + de.deserialize_option(IamPolicyConditionVisitor) +} + +struct IamPolicyConditionVisitor; + +impl<'de> Visitor<'de> for IamPolicyConditionVisitor { + type Value = Option; + + // Format a message stating what data this Visitor expects to receive. + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str("lots of things can go wrong with a IAM Policy Condition") + } + + fn visit_unit(self) -> Result + where + E: DeError, + { + Ok(None) + } + + fn visit_none(self) -> Result + where + E: DeError, + { + Ok(None) + } + + fn visit_some(self, deserializer: D) -> Result + where + D: Deserializer<'de>, + { + deserializer.deserialize_map(self) + } + + fn visit_map(self, mut access: M) -> Result + where + M: MapAccess<'de>, + { + let mut map = HashMap::with_capacity(access.size_hint().unwrap_or(0)); + + while let Some((key, val)) = access.next_entry::, HashMap, StringOrSlice>>()? { + let mut value = HashMap::with_capacity(val.len()); + for (val_key, string_or_slice) in val { + let val = match string_or_slice { + StringOrSlice::Slice(slice) => slice, + StringOrSlice::String(s) => vec![s], + }; + value.insert(val_key.into_owned(), val); + } + + map.insert(key.into_owned(), value); + } + + Ok(Some(map)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_deserialize_string_condition() { + let data = serde_json::json!({ + "condition": { + "StringEquals": { + "iam:RegisterSecurityKey": "Activate", + "iam:FIDO-certification": "L1plus" + } + } + }); + + #[derive(Deserialize)] + struct Test { + #[serde(deserialize_with = "deserialize_policy_condition")] + condition: Option, + } + + let test: Test = serde_json::from_value(data).unwrap(); + let condition = test.condition.unwrap(); + assert_eq!(1, condition.len()); + + assert_eq!(vec!["Activate"], condition["StringEquals"]["iam:RegisterSecurityKey"]); + assert_eq!(vec!["L1plus"], condition["StringEquals"]["iam:FIDO-certification"]); + } + + #[test] + fn test_deserialize_slide_condition() { + let data = serde_json::json!({ + "condition": {"StringLike": {"s3:prefix": ["janedoe/*"]}} + }); + + #[derive(Deserialize)] + struct Test { + #[serde(deserialize_with = "deserialize_policy_condition")] + condition: Option, + } + + let test: Test = serde_json::from_value(data).unwrap(); + let condition = test.condition.unwrap(); + assert_eq!(1, condition.len()); + + assert_eq!(vec!["janedoe/*"], condition["StringLike"]["s3:prefix"]); + } +}