diff --git a/packages/account-wasm/src/account.rs b/packages/account-wasm/src/account.rs index d4c45cdaf..42ea3e01d 100644 --- a/packages/account-wasm/src/account.rs +++ b/packages/account-wasm/src/account.rs @@ -287,8 +287,8 @@ impl CartridgeAccount { .is_some()) } - #[wasm_bindgen(js_name = session)] - pub async fn session_metadata( + #[wasm_bindgen(js_name = getAuthorizedSessionMetadata)] + pub async fn authorized_session_metadata( &self, policies: Vec, public_key: Option, @@ -302,10 +302,28 @@ impl CartridgeAccount { .controller .lock() .await - .session_metadata(&policies, public_key.map(|f| f.0)) + .authorized_session_metadata(&policies, public_key.map(|f| f.0)) .map(|(_, metadata)| SessionMetadata::from(metadata))) } + #[wasm_bindgen(js_name = isRequestedSession)] + pub async fn is_requested_session( + &self, + policies: Vec, + public_key: Option, + ) -> std::result::Result { + let policies = policies + .into_iter() + .map(TryFrom::try_from) + .collect::, _>>()?; + + Ok(self + .controller + .lock() + .await + .is_requested_session(&policies, public_key.map(|f| f.0))) + } + #[wasm_bindgen(js_name = revokeSession)] pub fn revoke_session(&self) -> Result<()> { unimplemented!("Revoke Session not implemented"); diff --git a/packages/account-wasm/src/types/policy.rs b/packages/account-wasm/src/types/policy.rs index 7271d77cf..3794ca0d4 100644 --- a/packages/account-wasm/src/types/policy.rs +++ b/packages/account-wasm/src/types/policy.rs @@ -15,12 +15,16 @@ use super::EncodingError; pub struct CallPolicy { pub target: String, pub method: String, + #[tsify(optional)] + pub authorized: Option, } #[derive(Tsify, Serialize, Deserialize, Debug, Clone)] #[tsify(into_wasm_abi, from_wasm_abi)] pub struct TypedDataPolicy { pub scope_hash: String, + #[tsify(optional)] + pub authorized: Option, } #[allow(non_snake_case)] @@ -45,15 +49,22 @@ impl TryFrom for SdkPolicy { fn try_from(value: Policy) -> Result { match value { - Policy::Call(CallPolicy { target, method }) => Ok(SdkPolicy::Call(SdkCallPolicy { + Policy::Call(CallPolicy { + target, + method, + authorized, + }) => Ok(SdkPolicy::Call(SdkCallPolicy { contract_address: Felt::from_str(&target)?, selector: get_selector_from_name(&method).unwrap(), + authorized, + })), + Policy::TypedData(TypedDataPolicy { + scope_hash, + authorized, + }) => Ok(SdkPolicy::TypedData(SdkTypedDataPolicy { + scope_hash: Felt::from_str(&scope_hash)?, + authorized, })), - Policy::TypedData(TypedDataPolicy { scope_hash }) => { - Ok(SdkPolicy::TypedData(SdkTypedDataPolicy { - scope_hash: Felt::from_str(&scope_hash)?, - })) - } } } } @@ -64,9 +75,11 @@ impl From for Policy { SdkPolicy::Call(call_policy) => Policy::Call(CallPolicy { target: call_policy.contract_address.to_string(), method: call_policy.selector.to_string(), + authorized: call_policy.authorized, }), SdkPolicy::TypedData(typed_data_policy) => Policy::TypedData(TypedDataPolicy { scope_hash: typed_data_policy.scope_hash.to_string(), + authorized: typed_data_policy.authorized, }), } } diff --git a/packages/account-wasm/src/types/session.rs b/packages/account-wasm/src/types/session.rs index a51ea45ff..d235e2b12 100644 --- a/packages/account-wasm/src/types/session.rs +++ b/packages/account-wasm/src/types/session.rs @@ -58,7 +58,7 @@ impl From for Session { fn from(value: account_sdk::account::session::hash::Session) -> Self { Session { policies: value - .policies + .proved_policies .into_iter() .map(|p| p.policy.into()) .collect::>(), diff --git a/packages/account_sdk/src/account/session/hash.rs b/packages/account_sdk/src/account/session/hash.rs index 71b92a61d..6d7bad72f 100644 --- a/packages/account_sdk/src/account/session/hash.rs +++ b/packages/account_sdk/src/account/session/hash.rs @@ -26,7 +26,8 @@ use super::policy::ProvedPolicy; #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct Session { pub inner: crate::abigen::controller::Session, - pub policies: Vec, + pub requested_policies: Vec, + pub proved_policies: Vec, pub metadata: String, } @@ -37,23 +38,32 @@ impl Session { session_signer: &Signer, guardian_guid: Felt, ) -> Result { - if policies.is_empty() { - return Err(SignError::NoAllowedSessionMethods); - } let metadata = json!({ "metadata": "metadata", "max_fee": 0 }); - let hashes = policies + + // Only include authorized policies in the merkle tree + let authorized_policies: Vec<_> = policies.iter().filter(|&p| p.is_authorized()).collect(); + + let hashes = authorized_policies .iter() - .map(Policy::as_merkle_leaf) + .map(|&p| Policy::as_merkle_leaf(p)) .collect::>(); - let policies: Vec<_> = policies + + let proved_policies: Vec<_> = authorized_policies + .clone() .into_iter() .enumerate() - .map(|(i, method)| ProvedPolicy { - policy: method, + .map(|(i, policy)| ProvedPolicy { + policy: policy.clone(), proof: MerkleTree::compute_proof(hashes.clone(), i), }) .collect(); - let root = MerkleTree::compute_root(hashes[0], policies[0].proof.clone()); + + let root = if authorized_policies.is_empty() { + Felt::ZERO + } else { + MerkleTree::compute_root(hashes[0], proved_policies[0].proof.clone()) + }; + Ok(Self { inner: crate::abigen::controller::Session { expires_at, @@ -62,7 +72,8 @@ impl Session { guardian_key_guid: guardian_guid, metadata_hash: Felt::ZERO, }, - policies, + requested_policies: policies, + proved_policies, metadata: serde_json::to_string(&metadata).unwrap(), }) } @@ -77,16 +88,16 @@ impl Session { } pub fn single_proof(&self, policy: &Policy) -> Option> { - self.policies + self.proved_policies .iter() - .find(|ProvedPolicy { policy: method, .. }| method == policy) + .find(|ProvedPolicy { policy: p, .. }| p == policy) .map(|ProvedPolicy { proof, .. }| proof.clone()) } pub fn is_authorized(&self, policy: &Policy) -> bool { - self.policies - .iter() - .any(|proved_policy| proved_policy.policy == *policy) + self.proved_policies.iter().any(|proved_policy| { + proved_policy.policy == *policy && proved_policy.policy.is_authorized() + }) } pub fn is_expired(&self) -> bool { @@ -103,6 +114,10 @@ impl Session { self.inner.session_key_guid == session_key_guid } + + pub fn is_requested(&self, policy: &Policy) -> bool { + self.requested_policies.iter().any(|p| p == policy) + } } impl StructHashRev1 for abigen::controller::Session { diff --git a/packages/account_sdk/src/account/session/policy.rs b/packages/account_sdk/src/account/session/policy.rs index 1abf471c8..c60b40d36 100644 --- a/packages/account_sdk/src/account/session/policy.rs +++ b/packages/account_sdk/src/account/session/policy.rs @@ -24,22 +24,48 @@ impl Policy { Policy::Call(CallPolicy { contract_address, selector, + authorized: Some(true), }) } + pub fn new_typed_data(scope_hash: Felt) -> Self { - Policy::TypedData(TypedDataPolicy { scope_hash }) + Policy::TypedData(TypedDataPolicy { + scope_hash, + authorized: Some(true), + }) + } + + pub fn is_authorized(&self) -> bool { + match self { + Policy::Call(call) => call.authorized.unwrap_or(false), + Policy::TypedData(typed_data) => typed_data.authorized.unwrap_or(false), + } } } -#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +impl PartialEq for CallPolicy { + fn eq(&self, other: &Self) -> bool { + self.contract_address == other.contract_address && self.selector == other.selector + } +} + +impl PartialEq for TypedDataPolicy { + fn eq(&self, other: &Self) -> bool { + self.scope_hash == other.scope_hash + } +} + +#[derive(Clone, Debug, Serialize, Deserialize)] pub struct CallPolicy { pub contract_address: Felt, pub selector: Felt, + pub authorized: Option, } -#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +#[derive(Clone, Debug, Serialize, Deserialize)] pub struct TypedDataPolicy { pub scope_hash: Felt, + pub authorized: Option, } impl From<&Call> for Policy { @@ -47,6 +73,7 @@ impl From<&Call> for Policy { Policy::Call(CallPolicy { contract_address: call.to, selector: call.selector, + authorized: Some(true), }) } } @@ -55,6 +82,7 @@ impl From<&TypedData> for Policy { fn from(typed_data: &TypedData) -> Self { Self::TypedData(TypedDataPolicy { scope_hash: typed_data.scope_hash, + authorized: Some(true), }) } } @@ -90,8 +118,15 @@ pub trait MerkleLeaf { impl MerkleLeaf for Policy { fn as_merkle_leaf(&self) -> Felt { match self { - Policy::Call(call_policy) => call_policy.as_merkle_leaf(), - Policy::TypedData(typed_data_policy) => typed_data_policy.as_merkle_leaf(), + Policy::Call(call_policy) if call_policy.authorized.unwrap_or(false) => { + call_policy.as_merkle_leaf() + } + Policy::TypedData(typed_data_policy) + if typed_data_policy.authorized.unwrap_or(false) => + { + typed_data_policy.as_merkle_leaf() + } + _ => Felt::ZERO, } } } diff --git a/packages/account_sdk/src/controller.rs b/packages/account_sdk/src/controller.rs index b640ac5d6..c742f8e5b 100644 --- a/packages/account_sdk/src/controller.rs +++ b/packages/account_sdk/src/controller.rs @@ -188,7 +188,7 @@ impl Controller { match est { Ok(mut fee_estimate) => { if self - .session_metadata(&Policy::from_calls(&calls), None) + .authorized_session_metadata(&Policy::from_calls(&calls), None) .map_or(true, |(_, metadata)| !metadata.is_registered) { fee_estimate.overall_fee += WEBAUTHN_GAS * fee_estimate.gas_price; @@ -252,7 +252,7 @@ impl Controller { // Update is_registered to true after successful execution with a session if let Some((key, metadata)) = - self.session_metadata(&Policy::from_calls(&calls), None) + self.authorized_session_metadata(&Policy::from_calls(&calls), None) { if !metadata.is_registered { let mut updated_metadata = metadata; diff --git a/packages/account_sdk/src/execute_from_outside.rs b/packages/account_sdk/src/execute_from_outside.rs index 48074459b..45497dfd8 100644 --- a/packages/account_sdk/src/execute_from_outside.rs +++ b/packages/account_sdk/src/execute_from_outside.rs @@ -105,7 +105,9 @@ impl Controller { .map_err(ControllerError::PaymasterError)?; // Update is_registered to true after successful execution with a session - if let Some((key, metadata)) = self.session_metadata(&Policy::from_calls(&calls), None) { + if let Some((key, metadata)) = + self.authorized_session_metadata(&Policy::from_calls(&calls), None) + { if !metadata.is_registered { let mut updated_metadata = metadata; updated_metadata.is_registered = true; diff --git a/packages/account_sdk/src/execute_from_outside_test.rs b/packages/account_sdk/src/execute_from_outside_test.rs index 14cfec294..49a67a264 100644 --- a/packages/account_sdk/src/execute_from_outside_test.rs +++ b/packages/account_sdk/src/execute_from_outside_test.rs @@ -94,7 +94,7 @@ async fn test_execute_from_outside_with_session() { // Check that the session is not registered initially let (_, initial_metadata) = controller - .session_metadata(&Policy::from_calls(&[]), None) + .authorized_session_metadata(&Policy::from_calls(&[]), None) .expect("Failed to get session metadata"); assert!( !initial_metadata.is_registered, @@ -140,7 +140,7 @@ async fn test_execute_from_outside_with_session() { // Check that the session is registered let (_, metadata) = controller - .session_metadata(&Policy::from_calls(&[]), None) + .authorized_session_metadata(&Policy::from_calls(&[]), None) .expect("Failed to get session metadata"); assert!(metadata.is_registered, "Session should be registered"); } diff --git a/packages/account_sdk/src/session.rs b/packages/account_sdk/src/session.rs index ccfc7d54d..71d75b183 100644 --- a/packages/account_sdk/src/session.rs +++ b/packages/account_sdk/src/session.rs @@ -127,23 +127,37 @@ impl Controller { Ok(txn) } - pub fn session_metadata( + pub fn authorized_session_metadata( &self, policies: &[Policy], public_key: Option, ) -> Option<(String, SessionMetadata)> { - let key: String = Selectors::session(&self.address, &self.app_id, &self.chain_id); + let key = self.session_key(); self.storage .session(&key) .ok() .flatten() - .filter(|metadata| metadata.is_valid(policies, public_key)) + .filter(|metadata| metadata.is_authorized(policies, public_key)) .map(|metadata| (key, metadata)) } + pub fn is_requested_session(&self, policies: &[Policy], public_key: Option) -> bool { + let key = self.session_key(); + self.storage + .session(&key) + .ok() + .flatten() + .filter(|metadata| metadata.is_requested(policies, public_key)) + .is_some() + } + + pub fn session_key(&self) -> String { + Selectors::session(&self.address, &self.app_id, &self.chain_id) + } + pub fn session_account(&self, policies: &[Policy]) -> Option { // Check if there's a valid session stored - let (_, metadata) = self.session_metadata(policies, None)?; + let (_, metadata) = self.authorized_session_metadata(policies, None)?; let credentials = metadata.credentials.as_ref()?; let session_signer = Signer::Starknet(SigningKey::from_secret_scalar(credentials.private_key)); diff --git a/packages/account_sdk/src/storage/mod.rs b/packages/account_sdk/src/storage/mod.rs index 63516299f..78d5f6677 100644 --- a/packages/account_sdk/src/storage/mod.rs +++ b/packages/account_sdk/src/storage/mod.rs @@ -177,7 +177,7 @@ pub struct SessionMetadata { } impl SessionMetadata { - pub fn is_valid(&self, policies: &[Policy], public_key: Option) -> bool { + pub fn is_authorized(&self, policies: &[Policy], public_key: Option) -> bool { let public_key = if let Some(public_key) = public_key { let pubkey = VerifyingKey::from_scalar(public_key); pubkey.scalar() @@ -194,6 +194,24 @@ impl SessionMetadata { .iter() .all(|policy| self.session.is_authorized(policy)) } + + pub fn is_requested(&self, policies: &[Policy], public_key: Option) -> bool { + let public_key = if let Some(public_key) = public_key { + let pubkey = VerifyingKey::from_scalar(public_key); + pubkey.scalar() + } else if let Some(credentials) = &self.credentials { + let signer = SigningKey::from_secret_scalar(credentials.private_key); + signer.verifying_key().scalar() + } else { + return false; + }; + + !self.session.is_expired() + && self.session.is_session_key(public_key) + && policies + .iter() + .all(|policy| self.session.is_requested(policy)) + } } #[derive(Debug, Clone, Serialize, Deserialize, Default, PartialEq)] diff --git a/packages/keychain/src/components/ConfirmTransaction.tsx b/packages/keychain/src/components/ConfirmTransaction.tsx index 6d65d90a3..113ba9e41 100644 --- a/packages/keychain/src/components/ConfirmTransaction.tsx +++ b/packages/keychain/src/components/ConfirmTransaction.tsx @@ -1,5 +1,5 @@ import { useEffect, useMemo, useState } from "react"; -import { ResponseCodes, SessionPolicies, toArray } from "@cartridge/controller"; +import { ResponseCodes, toArray } from "@cartridge/controller"; import { Content, FOOTER_MIN_HEIGHT } from "@/components/layout"; import { TransactionDuoIcon } from "@cartridge/ui-next"; import { useConnection } from "@/hooks/connection"; @@ -8,6 +8,7 @@ import { ExecuteCtx } from "@/utils/connection"; import { getChecksumAddress, num } from "starknet"; import { ExecutionContainer } from "@/components/ExecutionContainer"; import { CreateSession } from "./connect"; +import { ParsedSessionPolicies } from "@/hooks/session"; export function ConfirmTransaction() { const { controller, context, origin, policies, setContext } = useConnection(); @@ -35,7 +36,7 @@ export function ConfirmTransaction() { setContext(undefined); }; - const callPolicies = useMemo( + const callPolicies = useMemo( () => ({ contracts: Object.fromEntries( toArray(ctx.transactions).map((c) => [ @@ -43,6 +44,7 @@ export function ConfirmTransaction() { { methods: [{ entrypoint: c.entrypoint }] }, ]), ), + verified: false, }), [ctx.transactions], ); @@ -65,8 +67,8 @@ export function ConfirmTransaction() { // If calls are approved by dapp specified policies but not stored session // then prompt user to update session. This also accounts for expired sessions. if (txnsApproved && account) { - account.session(callPolicies).then((hasSession) => { - setUpdateSession(!hasSession); + account.isRequestedSession(callPolicies).then((isRequestedSession) => { + setUpdateSession(!isRequestedSession); }); } else { setUpdateSession(false); diff --git a/packages/keychain/src/components/ErrorAlert.tsx b/packages/keychain/src/components/ErrorAlert.tsx index 764117d6d..0d53c876a 100644 --- a/packages/keychain/src/components/ErrorAlert.tsx +++ b/packages/keychain/src/components/ErrorAlert.tsx @@ -143,7 +143,7 @@ export function ControllerErrorAlert({ error, isPaymaster = false, }: { - error: ControllerError; + error: ControllerError | Error; isPaymaster?: boolean; }) { let title = "An error occurred"; @@ -152,6 +152,21 @@ export function ControllerErrorAlert({ let variant = "error"; let copyText: string | undefined; + if (!isControllerError(error)) { + title = "Unknown error"; + description = error.message; + + return ( + + ); + } + switch (error.code) { case ErrorCode.SignError: title = "Signing Error"; @@ -403,3 +418,9 @@ function StackTraceDisplay({ ); } + +function isControllerError( + error: ControllerError | Error, +): error is ControllerError { + return !!(error as ControllerError).code; +} diff --git a/packages/keychain/src/components/connect/CreateSession.tsx b/packages/keychain/src/components/connect/CreateSession.tsx index 6c5d929ca..4d2784312 100644 --- a/packages/keychain/src/components/connect/CreateSession.tsx +++ b/packages/keychain/src/components/connect/CreateSession.tsx @@ -71,6 +71,37 @@ export function CreateSession({ }, [chainId, policies]); const onCreateSession = useCallback(async () => { + if (!controller || !policies) return; + try { + setError(undefined); + setIsConnecting(true); + // Set all contract policies to authorized + if (policies.contracts) { + Object.keys(policies.contracts).forEach((address) => { + if (policies.contracts![address]) { + policies.contracts![address].methods.forEach((method) => { + method.authorized = true; + }); + } + }); + } + + // Set all message policies to authorized + if (policies.messages) { + policies.messages.forEach((message) => { + message.authorized = true; + }); + } + + await controller.createSession(expiresAt, policies, maxFee); + onConnect(); + } catch (e) { + setError(e as unknown as Error); + setIsConnecting(false); + } + }, [controller, expiresAt, policies, maxFee, onConnect]); + + const onSkipSession = useCallback(async () => { if (!controller || !policies) return; try { setError(undefined); @@ -155,14 +186,12 @@ export function CreateSession({ )} - {error && isControllerError(error) && ( - - )} + {error && }