diff --git a/v3/Cargo.lock b/v3/Cargo.lock index 6dc31b68f5965..fb790291a3b9d 100644 --- a/v3/Cargo.lock +++ b/v3/Cargo.lock @@ -2425,12 +2425,14 @@ version = "3.0.0" dependencies = [ "axum", "axum-core", + "derive_more 0.99.18", "http 1.1.0", "lang-graphql", "open-dds", "pretty_assertions", "schemars", "serde", + "serde_json", "thiserror", ] diff --git a/v3/changelog.md b/v3/changelog.md index 61e2e22b8297d..4035d779b25cc 100644 --- a/v3/changelog.md +++ b/v3/changelog.md @@ -9,6 +9,15 @@ ### Fixed +- When the `CompatibilityConfig` date is set to `2024-10-16` or newer, session + variables returned by webhooks, set in `noAuth` config in `AuthConfig` or set + in JWT claims are now correctly allowed to be full JSON values, not just JSON + strings. This fixes the bug where you were incorrectly required to JSON-encode + your JSON value inside a string. For example, you were previously incorrectly + required to return session variables like this + `{ "X-Hasura-AllowedUserIds": "[1,2,3]" }`, but now you can correctly return + them like this: `{ "X-Hasura-AllowedUserIds": [1,2,3] }`. + ### Changed ## [v2024.10.21] diff --git a/v3/crates/auth/hasura-authn-core/Cargo.toml b/v3/crates/auth/hasura-authn-core/Cargo.toml index dd117ddc7adfa..36938a375d735 100644 --- a/v3/crates/auth/hasura-authn-core/Cargo.toml +++ b/v3/crates/auth/hasura-authn-core/Cargo.toml @@ -13,9 +13,11 @@ open-dds = { path = "../../open-dds" } axum = { workspace = true } axum-core = { workspace = true } +derive_more = { workspace = true } http = { workspace = true } schemars = { workspace = true } serde = { workspace = true } +serde_json = { workspace = true } thiserror = { workspace = true } [dev-dependencies] diff --git a/v3/crates/auth/hasura-authn-core/src/lib.rs b/v3/crates/auth/hasura-authn-core/src/lib.rs index 0dfd610892f8f..9abf8d63ddaf9 100644 --- a/v3/crates/auth/hasura-authn-core/src/lib.rs +++ b/v3/crates/auth/hasura-authn-core/src/lib.rs @@ -26,16 +26,73 @@ pub use open_dds::{ session_variables::{SessionVariableName, SessionVariableReference, SESSION_VARIABLE_ROLE}, }; -#[derive(Clone, Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize, JsonSchema)] -/// Value of a session variable -pub struct SessionVariableValue(pub String); +#[derive(Clone, Debug, Eq, PartialEq, serde::Serialize, derive_more::Display)] +/// Value of a session variable, used to capture session variable input from parsed sources (jwt, webhook, etc) +/// and unparsed sources (http headers) +pub enum SessionVariableValue { + /// An unparsed session variable value as a string. Might be a raw string, might be a number, might be json. + /// How we interpret it depends on what type we're trying to coerce to from the string + #[display(fmt = "{_0}")] + Unparsed(String), + /// A parsed JSON session variable value. We know what the type is because we parsed it from JSON. + #[display(fmt = "{_0}")] + Parsed(serde_json::Value), +} impl SessionVariableValue { pub fn new(value: &str) -> Self { - SessionVariableValue(value.to_string()) + SessionVariableValue::Unparsed(value.to_string()) + } + + /// Assert that a session variable represents a string, regardless of encoding + pub fn as_str(&self) -> Option<&str> { + match self { + SessionVariableValue::Unparsed(s) => Some(s.as_str()), + SessionVariableValue::Parsed(value) => value.as_str(), + } + } + + pub fn as_i64(&self) -> Option { + match self { + SessionVariableValue::Unparsed(s) => s.parse::().ok(), + SessionVariableValue::Parsed(value) => value.as_i64(), + } + } + + pub fn as_f64(&self) -> Option { + match self { + SessionVariableValue::Unparsed(s) => s.parse::().ok(), + SessionVariableValue::Parsed(value) => value.as_f64(), + } + } + + pub fn as_bool(&self) -> Option { + match self { + SessionVariableValue::Unparsed(s) => s.parse::().ok(), + SessionVariableValue::Parsed(value) => value.as_bool(), + } + } + + pub fn as_value(&self) -> serde_json::Result { + match self { + SessionVariableValue::Unparsed(s) => serde_json::from_str(s), + SessionVariableValue::Parsed(value) => Ok(value.clone()), + } + } +} + +impl From for SessionVariableValue { + fn from(value: JsonSessionVariableValue) -> Self { + SessionVariableValue::Parsed(value.0) } } +/// JSON value of a session variable +// This is used instead of SessionVariableValue when only JSON session variable values are accepted +#[derive(Debug, serde::Serialize, serde::Deserialize, PartialEq, Clone, JsonSchema)] +#[schemars(rename = "SessionVariableValue")] // Renamed to keep json schema compatibility +pub struct JsonSessionVariableValue(pub serde_json::Value); + #[derive(Clone, Debug, Eq, PartialEq, serde::Serialize)] pub struct SessionVariables(HashMap); @@ -184,16 +241,17 @@ pub fn authorize_identity( // traverse through the headers and collect role and session variables for (header_name, header_value) in headers { if let Ok(session_variable) = SessionVariableName::from_str(header_name.as_str()) { - let variable_value = match header_value.to_str() { + let variable_value_str = match header_value.to_str() { Err(e) => Err(SessionError::InvalidHeaderValue { header_name: header_name.to_string(), error: e.to_string(), })?, - Ok(h) => SessionVariableValue::new(h), + Ok(h) => h, }; + let variable_value = SessionVariableValue::Unparsed(variable_value_str.to_string()); if session_variable == SESSION_VARIABLE_ROLE { - role = Some(Role::new(&variable_value.0)); + role = Some(Role::new(variable_value_str)); } else { // TODO: Handle the duplicate case? session_variables.insert(session_variable, variable_value); diff --git a/v3/crates/auth/hasura-authn-jwt/src/auth.rs b/v3/crates/auth/hasura-authn-jwt/src/auth.rs index 03269cbd7a213..4e9763e8ba7c9 100644 --- a/v3/crates/auth/hasura-authn-jwt/src/auth.rs +++ b/v3/crates/auth/hasura-authn-jwt/src/auth.rs @@ -16,7 +16,11 @@ fn build_allowed_roles( // Note: The same `custom_claims` is being cloned // for every role present in the allowed roles. // We should think of having common claims. - session_variables: hasura_claims.custom_claims.clone(), + session_variables: hasura_claims + .custom_claims + .iter() + .map(|(k, v)| (k.clone(), v.clone().into())) + .collect(), allowed_session_variables_from_request: auth_base::SessionVariableList::Some( HashSet::new(), ), @@ -75,7 +79,14 @@ pub async fn authenticate_request( let role = hasura_claims .custom_claims .get(&SESSION_VARIABLE_ROLE) - .map(|v| Role::new(v.0.as_str())); + .map(|v| { + Ok::<_, Error>(Role::new(v.0.as_str().ok_or_else( + || Error::ClaimMustBeAString { + claim_name: SESSION_VARIABLE_ROLE.to_string(), + }, + )?)) + }) + .transpose()?; match role { // `x-hasura-role` is found, check if it's the // role that can emulate by comparing it to @@ -111,6 +122,7 @@ mod tests { use std::str::FromStr; use auth_base::{RoleAuthorization, SessionVariableValue}; + use hasura_authn_core::JsonSessionVariableValue; use jsonwebtoken as jwt; use jsonwebtoken::Algorithm; use jwt::{encode, EncodingKey}; @@ -168,7 +180,7 @@ mod tests { let mut hasura_custom_claims = HashMap::new(); hasura_custom_claims.insert( SessionVariableName::from_str("x-hasura-user-id").unwrap(), - SessionVariableValue("1".to_string()), + JsonSessionVariableValue(json!("1")), ); HasuraClaims { default_role: Role::new("user"), @@ -225,7 +237,7 @@ mod tests { role_authorization_session_variables.insert( SessionVariableName::from_str("x-hasura-user-id").unwrap(), - SessionVariableValue::new("1"), + SessionVariableValue::Parsed(json!("1")), ); expected_allowed_roles.insert( test_role.clone(), @@ -274,7 +286,7 @@ mod tests { let mut hasura_claims = get_default_hasura_claims(); hasura_claims.custom_claims.insert( SessionVariableName::from_str("x-hasura-role").unwrap(), - SessionVariableValue::new("admin"), + JsonSessionVariableValue(json!("admin")), ); let encoded_claims = get_encoded_claims(Algorithm::HS256, &hasura_claims)?; diff --git a/v3/crates/auth/hasura-authn-jwt/src/jwt.rs b/v3/crates/auth/hasura-authn-jwt/src/jwt.rs index 18c3e713c664f..ff198808a594a 100644 --- a/v3/crates/auth/hasura-authn-jwt/src/jwt.rs +++ b/v3/crates/auth/hasura-authn-jwt/src/jwt.rs @@ -4,7 +4,7 @@ use std::time::Duration; use axum::http::{HeaderMap, HeaderValue}; use axum::response::IntoResponse; use cookie::{self, Cookie}; -use hasura_authn_core::{Role, SessionVariableValue}; +use hasura_authn_core::{JsonSessionVariableValue, Role}; use jsonptr::Pointer; use jsonwebtoken::{self as jwt, decode, DecodingKey, Validation}; use jwt::decode_header; @@ -40,6 +40,8 @@ pub enum Error { claim_name: String, err: serde_json::Error, }, + #[error("Expected string value for claim {claim_name}")] + ClaimMustBeAString { claim_name: String }, #[error("Required claim {claim_name} not found")] RequiredClaimNotFound { claim_name: String }, #[error("JWT Authorization token source: Header name {header_name} not found.")] @@ -114,7 +116,8 @@ impl Error { header_name: _, } | Error::CookieParseError { err: _ } - | Error::MissingCookieValue { cookie_name: _ } => StatusCode::BAD_REQUEST, + | Error::MissingCookieValue { cookie_name: _ } + | Error::ClaimMustBeAString { claim_name: _ } => StatusCode::BAD_REQUEST, } } } @@ -254,7 +257,7 @@ pub struct JWTClaimsMap { /// A dictionary of the custom claims, where the key is the name of the claim and the value /// is the JSON pointer to lookup the custom claims within the decoded JWT. pub custom_claims: - Option>>, + Option>>, } #[derive(Serialize, Deserialize, PartialEq, Clone, JsonSchema, Debug)] @@ -391,7 +394,7 @@ pub struct HasuraClaims { /// as per the user's defined permissions. /// For example, things like `x-hasura-user-id` can go here. #[serde(flatten)] - pub custom_claims: HashMap, + pub custom_claims: HashMap, } #[derive(Debug, Serialize, Deserialize)] @@ -735,7 +738,7 @@ mod tests { let mut hasura_custom_claims = HashMap::new(); hasura_custom_claims.insert( SessionVariableName::from_str("x-hasura-user-id").unwrap(), - SessionVariableValue("1".to_string()), + JsonSessionVariableValue(json!("1")), ); HasuraClaims { default_role: Role::new("user"), diff --git a/v3/crates/auth/hasura-authn-noauth/src/lib.rs b/v3/crates/auth/hasura-authn-noauth/src/lib.rs index 4a35c8ac3e13b..923ac3297d473 100644 --- a/v3/crates/auth/hasura-authn-noauth/src/lib.rs +++ b/v3/crates/auth/hasura-authn-noauth/src/lib.rs @@ -1,4 +1,4 @@ -use hasura_authn_core::{Role, SessionVariableName, SessionVariableValue}; +use hasura_authn_core::{JsonSessionVariableValue, Role, SessionVariableName}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use serde_json::json; @@ -15,7 +15,7 @@ pub struct NoAuthConfig { pub role: Role, /// static session variables to use whilst running the engine #[schemars(title = "SessionVariables")] - pub session_variables: HashMap, + pub session_variables: HashMap, } impl NoAuthConfig { @@ -40,7 +40,11 @@ pub fn identity_from_config(no_auth_config: &NoAuthConfig) -> hasura_authn_core: no_auth_config.role.clone(), hasura_authn_core::RoleAuthorization { role: no_auth_config.role.clone(), - session_variables: no_auth_config.session_variables.clone(), + session_variables: no_auth_config + .session_variables + .iter() + .map(|(k, v)| (k.clone(), v.clone().into())) + .collect(), allowed_session_variables_from_request: hasura_authn_core::SessionVariableList::Some( HashSet::new(), ), diff --git a/v3/crates/auth/hasura-authn-webhook/src/webhook.rs b/v3/crates/auth/hasura-authn-webhook/src/webhook.rs index 115079334c7ac..55a692817bc54 100644 --- a/v3/crates/auth/hasura-authn-webhook/src/webhook.rs +++ b/v3/crates/auth/hasura-authn-webhook/src/webhook.rs @@ -46,6 +46,8 @@ pub enum InternalError { ReqwestError(reqwest::Error), #[error("'x-hasura-role' session variable not found in the webhook response.")] RoleSessionVariableNotFound, + #[error("'x-hasura-role' session variable in the webhook response was not a string.")] + RoleSessionVariableMustBeString, } impl TraceableError for InternalError { @@ -202,14 +204,14 @@ async fn make_auth_hook_request( match response.status() { reqwest::StatusCode::UNAUTHORIZED => Err(Error::AuthenticationFailed), reqwest::StatusCode::OK => { - let auth_hook_response: HashMap = + let auth_hook_response: HashMap = response.json().await.map_err(InternalError::ReqwestError)?; let mut session_variables = HashMap::new(); for (k, v) in &auth_hook_response { match SessionVariableName::from_str(k) { Ok(session_variable) => { session_variables - .insert(session_variable, SessionVariableValue(v.to_string())); + .insert(session_variable, SessionVariableValue::Parsed(v.clone())); } Err(_e) => {} } @@ -218,8 +220,8 @@ async fn make_auth_hook_request( session_variables .get(&session_variables::SESSION_VARIABLE_ROLE) .ok_or(InternalError::RoleSessionVariableNotFound)? - .0 - .as_str(), + .as_str() + .ok_or_else(|| InternalError::RoleSessionVariableMustBeString)?, ); let role_authorization = RoleAuthorization { role: role.clone(), @@ -324,6 +326,7 @@ mod tests { use mockito; use rand::{thread_rng, Rng}; use reqwest::header::CONTENT_TYPE; + use serde_json::json; #[tokio::test] // This test emulates a successful authentication by the webhook using Get method @@ -367,11 +370,11 @@ mod tests { let mut role_authorization_session_variables = HashMap::new(); role_authorization_session_variables.insert( SessionVariableName::from_str("x-hasura-role").unwrap(), - SessionVariableValue::new("test-role"), + SessionVariableValue::Parsed(json!("test-role")), ); role_authorization_session_variables.insert( SessionVariableName::from_str("x-hasura-test-role-id").unwrap(), - SessionVariableValue::new("1"), + SessionVariableValue::Parsed(json!("1")), ); expected_allowed_roles.insert( test_role.clone(), @@ -438,11 +441,11 @@ mod tests { let mut role_authorization_session_variables = HashMap::new(); role_authorization_session_variables.insert( SessionVariableName::from_str("x-hasura-role").unwrap(), - SessionVariableValue::new("test-role"), + SessionVariableValue::Parsed(json!("test-role")), ); role_authorization_session_variables.insert( SessionVariableName::from_str("x-hasura-test-role-id").unwrap(), - SessionVariableValue::new("1"), + SessionVariableValue::Parsed(json!("1")), ); expected_allowed_roles.insert( test_role.clone(), @@ -510,15 +513,15 @@ mod tests { let mut role_authorization_session_variables = HashMap::new(); role_authorization_session_variables.insert( SessionVariableName::from_str("x-hasura-role").unwrap(), - SessionVariableValue::new("test-role"), + SessionVariableValue::Parsed(json!("test-role")), ); role_authorization_session_variables.insert( SessionVariableName::from_str("x-hasura-test-role-id").unwrap(), - SessionVariableValue::new("1"), + SessionVariableValue::Parsed(json!("1")), ); role_authorization_session_variables.insert( SessionVariableName::from_str("status").unwrap(), - SessionVariableValue::new("true"), + SessionVariableValue::Parsed(json!("true")), ); expected_allowed_roles.insert( test_role.clone(), @@ -638,11 +641,11 @@ mod tests { let mut role_authorization_session_variables = HashMap::new(); role_authorization_session_variables.insert( SessionVariableName::from_str("x-hasura-role").unwrap(), - SessionVariableValue::new("user"), + SessionVariableValue::Parsed(json!("user")), ); role_authorization_session_variables.insert( SessionVariableName::from_str("x-hasura-user-id").unwrap(), - SessionVariableValue::new("1"), + SessionVariableValue::Parsed(json!("1")), ); expected_allowed_roles.insert( test_role.clone(), diff --git a/v3/crates/auth/hasura-authn/tests/auth_config.jsonschema b/v3/crates/auth/hasura-authn/tests/auth_config.jsonschema index c45e4dd1c3494..0bc9e0df3e5b9 100644 --- a/v3/crates/auth/hasura-authn/tests/auth_config.jsonschema +++ b/v3/crates/auth/hasura-authn/tests/auth_config.jsonschema @@ -480,8 +480,7 @@ ] }, "SessionVariableValue": { - "description": "Value of a session variable", - "type": "string" + "description": "JSON value of a session variable" }, "JWTClaimsMappingPathEntry_for_SessionVariableValue": { "$id": "https://hasura.io/jsonschemas/metadata/JWTClaimsMappingPathEntry_for_SessionVariableValue", diff --git a/v3/crates/engine/tests/common.rs b/v3/crates/engine/tests/common.rs index 3702d1a455137..f4ee08502f170 100644 --- a/v3/crates/engine/tests/common.rs +++ b/v3/crates/engine/tests/common.rs @@ -3,7 +3,9 @@ use execute::{HttpContext, ProjectId}; use goldenfile::{differs::text_diff, Mint}; use graphql_frontend::execute_query; use graphql_schema::GDS; -use hasura_authn_core::{Identity, Role, Session, SessionError, SessionVariableValue}; +use hasura_authn_core::{ + Identity, JsonSessionVariableValue, Role, Session, SessionError, SessionVariableValue, +}; use lang_graphql::ast::common as ast; use lang_graphql::{http::RawRequest, schema::Schema}; use metadata_resolve::{data_connectors::NdcVersion, LifecyclePluginConfigs}; @@ -47,7 +49,15 @@ pub(crate) fn resolve_session( let authorization = Identity::admin(Role::new("admin")); let role = session_variables .get(&SESSION_VARIABLE_ROLE) - .map(|v| Role::new(&v.0)); + .map(|v| { + Ok(Role::new(v.as_str().ok_or_else(|| { + SessionError::InvalidHeaderValue { + header_name: SESSION_VARIABLE_ROLE.to_string(), + error: "session variable value is not a string".to_owned(), + } + })?)) + }) + .transpose()?; let role_authorization = authorization.get_role_authorization(role.as_ref())?; let session = role_authorization.build_session(session_variables); Ok(session) @@ -107,11 +117,18 @@ pub(crate) fn test_introspection_expectation( let request_headers = reqwest::header::HeaderMap::new(); let session_vars_path = &test_path.join("session_variables.json"); - let sessions: Vec> = + let sessions: Vec> = json::from_str(read_to_string(session_vars_path)?.as_ref())?; let sessions: Vec = sessions .into_iter() - .map(resolve_session) + .map(|session_vars| { + resolve_session( + session_vars + .into_iter() + .map(|(k, v)| (k, v.into())) + .collect(), + ) + }) .collect::>()?; let raw_request = RawRequest { @@ -266,11 +283,18 @@ pub fn test_execution_expectation_for_multiple_ndc_versions( let request_headers = reqwest::header::HeaderMap::new(); let session_vars_path = &test_path.join("session_variables.json"); - let sessions: Vec> = + let sessions: Vec> = json::from_str(read_to_string(session_vars_path)?.as_ref())?; let sessions: Vec = sessions .into_iter() - .map(resolve_session) + .map(|session_vars| { + resolve_session( + session_vars + .into_iter() + .map(|(k, v)| (k, v.into())) + .collect(), + ) + }) .collect::>()?; // expected response headers are a `Vec`; one set for each @@ -469,11 +493,11 @@ pub fn test_execute_explain( let schema = GDS::build_schema(&gds)?; let request_headers = reqwest::header::HeaderMap::new(); let session = { - let session_variables_raw = r#"{ - "x-hasura-role": "admin" - }"#; let session_variables: HashMap = - serde_json::from_str(session_variables_raw)?; + HashMap::from_iter([( + SESSION_VARIABLE_ROLE.clone(), + SessionVariableValue::Unparsed("admin".to_owned()), + )]); resolve_session(session_variables) }?; let query = read_to_string(&root_test_dir.join(gql_request_file_path))?; @@ -579,9 +603,14 @@ pub(crate) fn test_sql(test_path_string: &str) -> anyhow::Result<()> { let session = Arc::new({ let session_vars_path = &test_path.join("session_variables.json"); - let session_variables: HashMap = + let session_variables: HashMap = serde_json::from_str(read_to_string(session_vars_path)?.as_ref())?; - resolve_session(session_variables) + resolve_session( + session_variables + .into_iter() + .map(|(k, v)| (k, v.into())) + .collect(), + ) }?); let catalog = Arc::new(sql::catalog::Catalog::from_metadata(gds.metadata)); diff --git a/v3/crates/engine/tests/execute/models/select_many/array_session_variable/expected.json b/v3/crates/engine/tests/execute/session_variables/json_disabled/integer_session_variable/expected.json similarity index 100% rename from v3/crates/engine/tests/execute/models/select_many/array_session_variable/expected.json rename to v3/crates/engine/tests/execute/session_variables/json_disabled/integer_session_variable/expected.json diff --git a/v3/crates/engine/tests/execute/session_variables/json_disabled/integer_session_variable/metadata.json b/v3/crates/engine/tests/execute/session_variables/json_disabled/integer_session_variable/metadata.json new file mode 100644 index 0000000000000..fbb739172e70e --- /dev/null +++ b/v3/crates/engine/tests/execute/session_variables/json_disabled/integer_session_variable/metadata.json @@ -0,0 +1,192 @@ +{ + "version": "v2", + "flags": { + "json_session_variables": false + }, + "subgraphs": [ + { + "name": "default", + "objects": [ + { + "kind": "BooleanExpressionType", + "version": "v1", + "definition": { + "name": "int_bool_exp", + "operand": { + "scalar": { + "type": "Int", + "comparisonOperators": [ + { + "name": "_eq", + "argumentType": "Int!" + }, + { + "name": "_in", + "argumentType": "[Int!]" + } + ], + "dataConnectorOperatorMapping": [ + { + "dataConnectorName": "db", + "dataConnectorScalarType": "int4", + "operatorMapping": {} + } + ] + } + }, + "logicalOperators": { + "enable": true + }, + "isNull": { + "enable": true + }, + "graphql": { + "typeName": "Int_Filter" + } + } + }, + { + "kind": "ObjectType", + "version": "v1", + "definition": { + "name": "author", + "fields": [ + { + "name": "author_id", + "type": "Int!" + }, + { + "name": "first_name", + "type": "String!" + }, + { + "name": "last_name", + "type": "String!" + } + ], + "graphql": { + "typeName": "Author" + }, + "dataConnectorTypeMapping": [ + { + "dataConnectorName": "db", + "dataConnectorObjectType": "author", + "fieldMapping": { + "author_id": { + "column": { + "name": "id" + } + }, + "first_name": { + "column": { + "name": "first_name" + } + }, + "last_name": { + "column": { + "name": "last_name" + } + } + } + } + ] + } + }, + { + "kind": "BooleanExpressionType", + "version": "v1", + "definition": { + "name": "author_bool_exp", + "operand": { + "object": { + "type": "author", + "comparableFields": [ + { + "fieldName": "author_id", + "booleanExpressionType": "int_bool_exp" + } + ], + "comparableRelationships": [] + } + }, + "logicalOperators": { + "enable": true + }, + "isNull": { + "enable": true + }, + "graphql": { + "typeName": "Author_Filter" + } + } + }, + { + "kind": "Model", + "version": "v1", + "definition": { + "name": "Authors", + "objectType": "author", + "source": { + "dataConnectorName": "db", + "collection": "author" + }, + "graphql": { + "selectUniques": [], + "selectMany": { + "queryRootField": "AuthorMany" + }, + "orderByExpressionType": "Author_Order_By" + }, + "filterExpressionType": "author_bool_exp", + "orderableFields": [ + { + "fieldName": "author_id", + "orderByDirections": { + "enableAll": true + } + } + ] + } + }, + { + "kind": "TypePermissions", + "version": "v1", + "definition": { + "typeName": "author", + "permissions": [ + { + "role": "user", + "output": { + "allowedFields": ["author_id", "first_name", "last_name"] + } + } + ] + } + }, + { + "kind": "ModelPermissions", + "version": "v1", + "definition": { + "modelName": "Authors", + "permissions": [ + { + "role": "user", + "select": { + "filter": { + "fieldComparison": { + "field": "author_id", + "operator": "_eq", + "value": { + "sessionVariable": "x-hasura-allowed-author-id" + } + } + } + } + } + ] + } + } + ] + } + ] +} diff --git a/v3/crates/engine/tests/execute/models/select_many/array_session_variable/request.gql b/v3/crates/engine/tests/execute/session_variables/json_disabled/integer_session_variable/request.gql similarity index 100% rename from v3/crates/engine/tests/execute/models/select_many/array_session_variable/request.gql rename to v3/crates/engine/tests/execute/session_variables/json_disabled/integer_session_variable/request.gql diff --git a/v3/crates/engine/tests/execute/models/select_many/array_session_variable/session_variables.json b/v3/crates/engine/tests/execute/session_variables/json_disabled/integer_session_variable/session_variables.json similarity index 50% rename from v3/crates/engine/tests/execute/models/select_many/array_session_variable/session_variables.json rename to v3/crates/engine/tests/execute/session_variables/json_disabled/integer_session_variable/session_variables.json index c81ca1a21f20f..5ee49fdcec44f 100644 --- a/v3/crates/engine/tests/execute/models/select_many/array_session_variable/session_variables.json +++ b/v3/crates/engine/tests/execute/session_variables/json_disabled/integer_session_variable/session_variables.json @@ -1,6 +1,6 @@ [ { "x-hasura-role": "user", - "x-hasura-allowed-author-ids": "[1]" + "x-hasura-allowed-author-id": "1" } ] diff --git a/v3/crates/engine/tests/execute/session_variables/json_enabled/array_session_variable/expected.json b/v3/crates/engine/tests/execute/session_variables/json_enabled/array_session_variable/expected.json new file mode 100644 index 0000000000000..92b2c8daeb822 --- /dev/null +++ b/v3/crates/engine/tests/execute/session_variables/json_enabled/array_session_variable/expected.json @@ -0,0 +1,11 @@ +[ + { + "data": { + "AuthorMany": [ + { + "author_id": 1 + } + ] + } + } +] diff --git a/v3/crates/engine/tests/execute/models/select_many/array_session_variable/metadata.json b/v3/crates/engine/tests/execute/session_variables/json_enabled/array_session_variable/metadata.json similarity index 100% rename from v3/crates/engine/tests/execute/models/select_many/array_session_variable/metadata.json rename to v3/crates/engine/tests/execute/session_variables/json_enabled/array_session_variable/metadata.json diff --git a/v3/crates/engine/tests/execute/session_variables/json_enabled/array_session_variable/request.gql b/v3/crates/engine/tests/execute/session_variables/json_enabled/array_session_variable/request.gql new file mode 100644 index 0000000000000..70c0f295d99f8 --- /dev/null +++ b/v3/crates/engine/tests/execute/session_variables/json_enabled/array_session_variable/request.gql @@ -0,0 +1,5 @@ +query MyQuery { + AuthorMany { + author_id + } +} diff --git a/v3/crates/engine/tests/execute/session_variables/json_enabled/array_session_variable/session_variables.json b/v3/crates/engine/tests/execute/session_variables/json_enabled/array_session_variable/session_variables.json new file mode 100644 index 0000000000000..39e2b431d5fb8 --- /dev/null +++ b/v3/crates/engine/tests/execute/session_variables/json_enabled/array_session_variable/session_variables.json @@ -0,0 +1,6 @@ +[ + { + "x-hasura-role": "user", + "x-hasura-allowed-author-ids": [1] + } +] diff --git a/v3/crates/engine/tests/execute/session_variables/json_enabled/integer_session_variable/expected.json b/v3/crates/engine/tests/execute/session_variables/json_enabled/integer_session_variable/expected.json new file mode 100644 index 0000000000000..92b2c8daeb822 --- /dev/null +++ b/v3/crates/engine/tests/execute/session_variables/json_enabled/integer_session_variable/expected.json @@ -0,0 +1,11 @@ +[ + { + "data": { + "AuthorMany": [ + { + "author_id": 1 + } + ] + } + } +] diff --git a/v3/crates/engine/tests/execute/session_variables/json_enabled/integer_session_variable/metadata.json b/v3/crates/engine/tests/execute/session_variables/json_enabled/integer_session_variable/metadata.json new file mode 100644 index 0000000000000..1d110b0db306c --- /dev/null +++ b/v3/crates/engine/tests/execute/session_variables/json_enabled/integer_session_variable/metadata.json @@ -0,0 +1,192 @@ +{ + "version": "v2", + "flags": { + "json_session_variables": true + }, + "subgraphs": [ + { + "name": "default", + "objects": [ + { + "kind": "BooleanExpressionType", + "version": "v1", + "definition": { + "name": "int_bool_exp", + "operand": { + "scalar": { + "type": "Int", + "comparisonOperators": [ + { + "name": "_eq", + "argumentType": "Int!" + }, + { + "name": "_in", + "argumentType": "[Int!]" + } + ], + "dataConnectorOperatorMapping": [ + { + "dataConnectorName": "db", + "dataConnectorScalarType": "int4", + "operatorMapping": {} + } + ] + } + }, + "logicalOperators": { + "enable": true + }, + "isNull": { + "enable": true + }, + "graphql": { + "typeName": "Int_Filter" + } + } + }, + { + "kind": "ObjectType", + "version": "v1", + "definition": { + "name": "author", + "fields": [ + { + "name": "author_id", + "type": "Int!" + }, + { + "name": "first_name", + "type": "String!" + }, + { + "name": "last_name", + "type": "String!" + } + ], + "graphql": { + "typeName": "Author" + }, + "dataConnectorTypeMapping": [ + { + "dataConnectorName": "db", + "dataConnectorObjectType": "author", + "fieldMapping": { + "author_id": { + "column": { + "name": "id" + } + }, + "first_name": { + "column": { + "name": "first_name" + } + }, + "last_name": { + "column": { + "name": "last_name" + } + } + } + } + ] + } + }, + { + "kind": "BooleanExpressionType", + "version": "v1", + "definition": { + "name": "author_bool_exp", + "operand": { + "object": { + "type": "author", + "comparableFields": [ + { + "fieldName": "author_id", + "booleanExpressionType": "int_bool_exp" + } + ], + "comparableRelationships": [] + } + }, + "logicalOperators": { + "enable": true + }, + "isNull": { + "enable": true + }, + "graphql": { + "typeName": "Author_Filter" + } + } + }, + { + "kind": "Model", + "version": "v1", + "definition": { + "name": "Authors", + "objectType": "author", + "source": { + "dataConnectorName": "db", + "collection": "author" + }, + "graphql": { + "selectUniques": [], + "selectMany": { + "queryRootField": "AuthorMany" + }, + "orderByExpressionType": "Author_Order_By" + }, + "filterExpressionType": "author_bool_exp", + "orderableFields": [ + { + "fieldName": "author_id", + "orderByDirections": { + "enableAll": true + } + } + ] + } + }, + { + "kind": "TypePermissions", + "version": "v1", + "definition": { + "typeName": "author", + "permissions": [ + { + "role": "user", + "output": { + "allowedFields": ["author_id", "first_name", "last_name"] + } + } + ] + } + }, + { + "kind": "ModelPermissions", + "version": "v1", + "definition": { + "modelName": "Authors", + "permissions": [ + { + "role": "user", + "select": { + "filter": { + "fieldComparison": { + "field": "author_id", + "operator": "_eq", + "value": { + "sessionVariable": "x-hasura-allowed-author-id" + } + } + } + } + } + ] + } + } + ] + } + ] +} diff --git a/v3/crates/engine/tests/execute/session_variables/json_enabled/integer_session_variable/request.gql b/v3/crates/engine/tests/execute/session_variables/json_enabled/integer_session_variable/request.gql new file mode 100644 index 0000000000000..70c0f295d99f8 --- /dev/null +++ b/v3/crates/engine/tests/execute/session_variables/json_enabled/integer_session_variable/request.gql @@ -0,0 +1,5 @@ +query MyQuery { + AuthorMany { + author_id + } +} diff --git a/v3/crates/engine/tests/execute/session_variables/json_enabled/integer_session_variable/session_variables.json b/v3/crates/engine/tests/execute/session_variables/json_enabled/integer_session_variable/session_variables.json new file mode 100644 index 0000000000000..964588e67cdd4 --- /dev/null +++ b/v3/crates/engine/tests/execute/session_variables/json_enabled/integer_session_variable/session_variables.json @@ -0,0 +1,6 @@ +[ + { + "x-hasura-role": "user", + "x-hasura-allowed-author-id": 1 + } +] diff --git a/v3/crates/engine/tests/execution.rs b/v3/crates/engine/tests/execution.rs index f4ef87d3c141f..fb7a70207d792 100644 --- a/v3/crates/engine/tests/execution.rs +++ b/v3/crates/engine/tests/execution.rs @@ -129,17 +129,6 @@ fn test_model_select_many_empty_select() -> anyhow::Result<()> { ) } -#[test] -fn test_model_select_many_array_session_variable() -> anyhow::Result<()> { - let test_path_string = "execute/models/select_many/array_session_variable"; - let common_metadata_path_string = "execute/common_metadata/postgres_connector_schema.json"; - common::test_execution_expectation( - test_path_string, - &[common_metadata_path_string], - common::TestOpenDDPipeline::TestNDCResponses, - ) -} - #[test] fn test_model_select_many_field_arguments() -> anyhow::Result<()> { common::test_execution_expectation_for_multiple_ndc_versions( @@ -2096,3 +2085,38 @@ fn test_command_query_forwarded_headers() -> anyhow::Result<()> { common::TestOpenDDPipeline::TestNDCResponses, ) } + +// Tests of session variables + +#[test] +fn test_session_variables_json_enabled_array_session_variable() -> anyhow::Result<()> { + let test_path_string = "execute/session_variables/json_enabled/array_session_variable"; + let common_metadata_path_string = "execute/common_metadata/postgres_connector_schema.json"; + common::test_execution_expectation( + test_path_string, + &[common_metadata_path_string], + common::TestOpenDDPipeline::TestNDCResponses, + ) +} + +#[test] +fn test_session_variables_json_enabled_integer_session_variable() -> anyhow::Result<()> { + let test_path_string = "execute/session_variables/json_enabled/integer_session_variable"; + let common_metadata_path_string = "execute/common_metadata/postgres_connector_schema.json"; + common::test_execution_expectation( + test_path_string, + &[common_metadata_path_string], + common::TestOpenDDPipeline::TestNDCResponses, + ) +} + +#[test] +fn test_session_variables_json_disabled_integer_session_variable() -> anyhow::Result<()> { + let test_path_string = "execute/session_variables/json_disabled/integer_session_variable"; + let common_metadata_path_string = "execute/common_metadata/postgres_connector_schema.json"; + common::test_execution_expectation( + test_path_string, + &[common_metadata_path_string], + common::TestOpenDDPipeline::TestNDCResponses, + ) +} diff --git a/v3/crates/engine/tests/snapshots/execution__common__ir_execute__models__select_many__array_session_variable.snap b/v3/crates/engine/tests/snapshots/execution__common__ir_execute__session_variables__json_disabled__integer_session_variable_user.snap similarity index 100% rename from v3/crates/engine/tests/snapshots/execution__common__ir_execute__models__select_many__array_session_variable.snap rename to v3/crates/engine/tests/snapshots/execution__common__ir_execute__session_variables__json_disabled__integer_session_variable_user.snap diff --git a/v3/crates/engine/tests/snapshots/execution__common__ir_execute__models__select_many__array_session_variable_user.snap b/v3/crates/engine/tests/snapshots/execution__common__ir_execute__session_variables__json_enabled__array_session_variable_user.snap similarity index 100% rename from v3/crates/engine/tests/snapshots/execution__common__ir_execute__models__select_many__array_session_variable_user.snap rename to v3/crates/engine/tests/snapshots/execution__common__ir_execute__session_variables__json_enabled__array_session_variable_user.snap diff --git a/v3/crates/engine/tests/snapshots/execution__common__ir_execute__session_variables__json_enabled__integer_session_variable_user.snap b/v3/crates/engine/tests/snapshots/execution__common__ir_execute__session_variables__json_enabled__integer_session_variable_user.snap new file mode 100644 index 0000000000000..370b9285c6b02 --- /dev/null +++ b/v3/crates/engine/tests/snapshots/execution__common__ir_execute__session_variables__json_enabled__integer_session_variable_user.snap @@ -0,0 +1,52 @@ +--- +source: crates/engine/tests/common.rs +expression: query_ir +--- +V1( + QueryRequestV1 { + queries: { + Alias( + Identifier( + "AuthorMany", + ), + ): Model( + ModelSelection { + target: ModelTarget { + subgraph: SubgraphName( + "default", + ), + model_name: ModelName( + Identifier( + "Authors", + ), + ), + arguments: {}, + filter: None, + order_by: [], + limit: None, + offset: None, + }, + selection: { + Alias( + Identifier( + "author_id", + ), + ): Field( + ObjectFieldSelection { + target: ObjectFieldTarget { + field_name: FieldName( + Identifier( + "author_id", + ), + ), + arguments: {}, + }, + selection: None, + }, + ), + }, + }, + ), + }, + }, +) diff --git a/v3/crates/engine/tests/snapshots/execution__common__rowsets_execute__models__select_many__array_session_variable.snap b/v3/crates/engine/tests/snapshots/execution__common__rowsets_execute__models__select_many__array_session_variable.snap deleted file mode 100644 index 3123daf5f3ba6..0000000000000 --- a/v3/crates/engine/tests/snapshots/execution__common__rowsets_execute__models__select_many__array_session_variable.snap +++ /dev/null @@ -1,13 +0,0 @@ ---- -source: crates/engine/tests/common.rs -expression: rowsets ---- -[ - { - "rows": [ - { - "author_id": 1 - } - ] - } -] diff --git a/v3/crates/engine/tests/snapshots/execution__common__rowsets_execute__models__select_many__array_session_variable_user.snap b/v3/crates/engine/tests/snapshots/execution__common__rowsets_execute__session_variables__json_disabled__integer_session_variable_user.snap similarity index 100% rename from v3/crates/engine/tests/snapshots/execution__common__rowsets_execute__models__select_many__array_session_variable_user.snap rename to v3/crates/engine/tests/snapshots/execution__common__rowsets_execute__session_variables__json_disabled__integer_session_variable_user.snap diff --git a/v3/crates/engine/tests/snapshots/execution__common__rowsets_execute__session_variables__json_enabled__array_session_variable_user.snap b/v3/crates/engine/tests/snapshots/execution__common__rowsets_execute__session_variables__json_enabled__array_session_variable_user.snap new file mode 100644 index 0000000000000..901db20111a35 --- /dev/null +++ b/v3/crates/engine/tests/snapshots/execution__common__rowsets_execute__session_variables__json_enabled__array_session_variable_user.snap @@ -0,0 +1,15 @@ +--- +source: crates/engine/tests/common.rs +expression: rowsets +--- +{ + "Ok": [ + { + "rows": [ + { + "author_id": 1 + } + ] + } + ] +} diff --git a/v3/crates/engine/tests/snapshots/execution__common__rowsets_execute__session_variables__json_enabled__integer_session_variable_user.snap b/v3/crates/engine/tests/snapshots/execution__common__rowsets_execute__session_variables__json_enabled__integer_session_variable_user.snap new file mode 100644 index 0000000000000..901db20111a35 --- /dev/null +++ b/v3/crates/engine/tests/snapshots/execution__common__rowsets_execute__session_variables__json_enabled__integer_session_variable_user.snap @@ -0,0 +1,15 @@ +--- +source: crates/engine/tests/common.rs +expression: rowsets +--- +{ + "Ok": [ + { + "rows": [ + { + "author_id": 1 + } + ] + } + ] +} diff --git a/v3/crates/frontends/graphql/src/lib.rs b/v3/crates/frontends/graphql/src/lib.rs index f26a09191d1cd..f3f2f9fdb499d 100644 --- a/v3/crates/frontends/graphql/src/lib.rs +++ b/v3/crates/frontends/graphql/src/lib.rs @@ -162,7 +162,7 @@ mod tests { // TODO: remove duplication between this function and 'add_session' fn resolve_session(session_vars_path: PathBuf) -> Session { let authorization = Identity::admin(Role::new("admin")); - let session_variables: HashMap = { + let session_variables: HashMap = { if session_vars_path.exists() { json::from_str(fs::read_to_string(session_vars_path).unwrap().as_ref()).unwrap() } else { @@ -172,7 +172,11 @@ mod tests { let role = session_variables .get(&SESSION_VARIABLE_ROLE) - .map(|v| Role::new(&v.0)); + .map(|v| Role::new(v)); + let session_variables = session_variables + .into_iter() + .map(|(k, v)| (k, SessionVariableValue::Unparsed(v))) + .collect(); authorization .get_role_authorization(role.as_ref()) .unwrap() diff --git a/v3/crates/graphql/ir/src/error.rs b/v3/crates/graphql/ir/src/error.rs index 804e1f7ae66a3..50cd3dde2092e 100644 --- a/v3/crates/graphql/ir/src/error.rs +++ b/v3/crates/graphql/ir/src/error.rs @@ -122,11 +122,28 @@ pub enum InternalDeveloperError { session_variable: SessionVariableName, }, - #[error("Unable to typecast session variable. Expected: {expected:}, but found: {found:}")] - VariableTypeCast { expected: String, found: String }, + #[error("The session variables {session_variable} is not encoded as a string. JSON-typed session variables are not supported unless you update your compatibility date")] + VariableJsonNotSupported { + session_variable: SessionVariableName, + }, + + #[error("Session variable {session_variable} value is of an unexpected type. Expected: {expected}, but found: {found}")] + VariableTypeCast { + session_variable: SessionVariableName, + expected: String, + found: String, + }, - #[error("Typecasting to array is not supported.")] - VariableArrayTypeCast, + #[error("Typecasting session variable {session_variable} to an array is not supported. Update your compatibility date to enable JSON session variables")] + VariableArrayTypeCastNotSupported { + session_variable: SessionVariableName, + }, + + #[error("Expected session variable {session_variable} to be a valid JSON value, but encountered a JSON parsing error: {parse_error}")] + VariableExpectedJson { + session_variable: SessionVariableName, + parse_error: serde_json::Error, + }, #[error("Mapping for the {mapping_kind} typename {type_name:} not found")] TypenameMappingNotFound { diff --git a/v3/crates/graphql/ir/src/permissions.rs b/v3/crates/graphql/ir/src/permissions.rs index b627fa3656e7a..4b45e39c42bd8 100644 --- a/v3/crates/graphql/ir/src/permissions.rs +++ b/v3/crates/graphql/ir/src/permissions.rs @@ -1,4 +1,4 @@ -use hasura_authn_core::{SessionVariableValue, SessionVariables}; +use hasura_authn_core::{SessionVariableName, SessionVariableValue, SessionVariables}; use lang_graphql::normalized_ast; use std::collections::BTreeMap; @@ -229,7 +229,12 @@ pub(crate) fn make_argument_from_value_expression( } })?; - typecast_session_variable(session_var.passed_as_json, value, value_type) + typecast_session_variable( + &session_var.name, + value, + session_var.passed_as_json, + value_type, + ) } } } @@ -254,7 +259,12 @@ pub(crate) fn make_argument_from_value_expression_or_predicate<'s>( })?; Ok(Argument::Literal { - value: typecast_session_variable(session_var.passed_as_json, value, value_type)?, + value: typecast_session_variable( + &session_var.name, + value, + session_var.passed_as_json, + value_type, + )?, }) } metadata_resolve::ValueExpressionOrPredicate::BooleanExpression(model_predicate) => { @@ -274,22 +284,31 @@ pub(crate) fn make_argument_from_value_expression_or_predicate<'s>( /// Typecast a stringified session variable into a given type, but as a serde_json::Value fn typecast_session_variable( - passed_as_json: bool, + session_var_name: &SessionVariableName, session_var_value_wrapped: &SessionVariableValue, + passed_as_json: bool, to_type: &QualifiedTypeReference, ) -> Result { if passed_as_json { - typecast_session_variable_v2(session_var_value_wrapped, to_type) + typecast_session_variable_v2(session_var_name, session_var_value_wrapped, to_type) } else { - typecast_session_variable_v1(session_var_value_wrapped, to_type) + typecast_session_variable_v1(session_var_name, session_var_value_wrapped, to_type) } } fn typecast_session_variable_v1( + session_var_name: &SessionVariableName, session_var_value_wrapped: &SessionVariableValue, to_type: &QualifiedTypeReference, ) -> Result { - let session_var_value = &session_var_value_wrapped.0; + // In v1 (ie before json type support in session variables), we expect every session + // variable to arrive as a string and then we parse that string into whatever type we need + let session_var_value = &session_var_value_wrapped + .as_str() + .ok_or(error::InternalDeveloperError::VariableJsonNotSupported { + session_variable: session_var_name.clone(), + })? + .to_string(); match &to_type.underlying_type { QualifiedBaseType::Named(type_name) => { match type_name { @@ -297,6 +316,7 @@ fn typecast_session_variable_v1( InbuiltType::Int => { let value: i32 = session_var_value.parse().map_err(|_| { error::InternalDeveloperError::VariableTypeCast { + session_variable: session_var_name.clone(), expected: "int".into(), found: session_var_value.clone(), } @@ -306,6 +326,7 @@ fn typecast_session_variable_v1( InbuiltType::Float => { let value: f32 = session_var_value.parse().map_err(|_| { error::InternalDeveloperError::VariableTypeCast { + session_variable: session_var_name.clone(), expected: "float".into(), found: session_var_value.clone(), } @@ -316,6 +337,7 @@ fn typecast_session_variable_v1( "true" => Ok(serde_json::Value::Bool(true)), "false" => Ok(serde_json::Value::Bool(false)), _ => Err(error::InternalDeveloperError::VariableTypeCast { + session_variable: session_var_name.clone(), expected: "true or false".into(), found: session_var_value.clone(), })?, @@ -337,76 +359,107 @@ fn typecast_session_variable_v1( } } } - QualifiedBaseType::List(_) => Err(error::InternalDeveloperError::VariableArrayTypeCast)?, + QualifiedBaseType::List(_) => Err( + error::InternalDeveloperError::VariableArrayTypeCastNotSupported { + session_variable: session_var_name.clone(), + }, + )?, } } fn typecast_session_variable_v2( + session_var_name: &SessionVariableName, session_var_value: &SessionVariableValue, to_type: &QualifiedTypeReference, ) -> Result { - let value = serde_json::from_str(&session_var_value.0)?; - typecheck_session_variable(&value, to_type)?; - Ok(value) -} - -fn typecheck_session_variable( - value: &serde_json::Value, - to_type: &QualifiedTypeReference, -) -> Result<(), crate::Error> { match &to_type.underlying_type { QualifiedBaseType::Named(type_name) => match type_name { QualifiedTypeName::Inbuilt(primitive) => match primitive { InbuiltType::Int => { - if !value.is_i64() { - Err(error::InternalDeveloperError::VariableTypeCast { + let value: i64 = session_var_value.as_i64().ok_or_else(|| { + error::InternalDeveloperError::VariableTypeCast { + session_variable: session_var_name.clone(), expected: "int".into(), - found: value.to_string(), - })?; - } - Ok(()) + found: session_var_value.to_string(), + } + })?; + Ok(serde_json::Value::Number(value.into())) } InbuiltType::Float => { - if !value.is_f64() { - Err(error::InternalDeveloperError::VariableTypeCast { + let value: f64 = session_var_value.as_f64().ok_or_else(|| { + error::InternalDeveloperError::VariableTypeCast { + session_variable: session_var_name.clone(), + expected: "float".into(), + found: session_var_value.to_string(), + } + })?; + serde_json::to_value(value).map_err(|_error| { + // f64 may not cleanly go into JSON numbers if the value is a NaN or infinite + error::InternalDeveloperError::VariableTypeCast { + session_variable: session_var_name.clone(), expected: "float".into(), found: value.to_string(), - })?; - } - Ok(()) + } + .into() + }) } InbuiltType::Boolean => { - if !value.is_boolean() { - Err(error::InternalDeveloperError::VariableTypeCast { + let value: bool = session_var_value.as_bool().ok_or_else(|| { + error::InternalDeveloperError::VariableTypeCast { + session_variable: session_var_name.clone(), expected: "true or false".into(), - found: value.to_string(), - })?; - } - Ok(()) + found: session_var_value.to_string(), + } + })?; + Ok(serde_json::Value::Bool(value)) } InbuiltType::ID | InbuiltType::String => { - if !value.is_string() { - Err(error::InternalDeveloperError::VariableTypeCast { + let value: &str = session_var_value.as_str().ok_or_else(|| { + error::InternalDeveloperError::VariableTypeCast { + session_variable: session_var_name.clone(), expected: "string".into(), - found: value.to_string(), - })?; - } - Ok(()) + found: session_var_value.to_string(), + } + })?; + Ok(serde_json::Value::String(value.to_string())) } }, - QualifiedTypeName::Custom(_) => Ok(()), + QualifiedTypeName::Custom(_) => { + let value = session_var_value.as_value().map_err(|parse_error| { + error::InternalDeveloperError::VariableExpectedJson { + session_variable: session_var_name.clone(), + parse_error, + } + })?; + Ok(value) + } }, QualifiedBaseType::List(element_type) => { + let value = session_var_value.as_value().map_err(|parse_error| { + error::InternalDeveloperError::VariableExpectedJson { + session_variable: session_var_name.clone(), + parse_error, + } + })?; let elements = value.as_array().ok_or_else(|| { error::InternalDeveloperError::VariableTypeCast { + session_variable: session_var_name.clone(), expected: "array".into(), found: value.to_string(), } })?; - for element in elements { - typecheck_session_variable(element, element_type)?; - } - Ok(()) + let typecasted_elements = elements + .iter() + .map(|element| { + typecast_session_variable_v2( + session_var_name, + &SessionVariableValue::Parsed(element.clone()), + element_type, + ) + }) + .collect::, _>>()?; + + Ok(serde_json::Value::Array(typecasted_elements)) } } } diff --git a/v3/justfile b/v3/justfile index b9c2599be75f8..b6b81d6a56bfa 100644 --- a/v3/justfile +++ b/v3/justfile @@ -100,7 +100,7 @@ machete: # update golden tests update-golden-files: start-docker-test-deps - UPDATE_GOLDENFILES=1 cargo test + UPDATE_GOLDENFILES=1 just test just fix-format update-custom-connector-schema-in-test-metadata: