diff --git a/backend/canisters/identity/CHANGELOG.md b/backend/canisters/identity/CHANGELOG.md index 0d1aa0d40b..7fdff743a5 100644 --- a/backend/canisters/identity/CHANGELOG.md +++ b/backend/canisters/identity/CHANGELOG.md @@ -16,6 +16,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Expose `liquid_cycles_balance` in metrics ([#8350](https://github.com/open-chat-labs/open-chat/pull/8350)) - Ensure new account linking codes are always unique ([#8393](https://github.com/open-chat-labs/open-chat/pull/8393)) +- Only allow generating account linking code if re-authenticated within 5 mins ([#8402](https://github.com/open-chat-labs/open-chat/pull/8402)) ### Fixed diff --git a/backend/canisters/identity/api/src/queries/check_auth_principal_v2.rs b/backend/canisters/identity/api/src/queries/check_auth_principal_v2.rs index 36a01c899e..11490bd87c 100644 --- a/backend/canisters/identity/api/src/queries/check_auth_principal_v2.rs +++ b/backend/canisters/identity/api/src/queries/check_auth_principal_v2.rs @@ -7,14 +7,14 @@ use types::{Empty, UserId}; pub type Args = Empty; #[ts_export(identity, check_auth_principal_v2)] -#[derive(CandidType, Serialize, Deserialize)] +#[derive(CandidType, Serialize, Deserialize, Debug)] pub enum Response { Success(SuccessResult), NotFound, } #[ts_export(identity, check_auth_principal_v2)] -#[derive(CandidType, Serialize, Deserialize)] +#[derive(CandidType, Serialize, Deserialize, Debug)] pub struct SuccessResult { pub user_id: Option, pub originating_canister: Principal, diff --git a/backend/canisters/identity/api/src/updates/create_account_linking_code.rs b/backend/canisters/identity/api/src/updates/create_account_linking_code.rs index 60b4d0c70d..25db04be12 100644 --- a/backend/canisters/identity/api/src/updates/create_account_linking_code.rs +++ b/backend/canisters/identity/api/src/updates/create_account_linking_code.rs @@ -2,13 +2,20 @@ use crate::account_linking_code::AccountLinkingCode; use candid::{CandidType, Deserialize}; use serde::Serialize; use ts_export::ts_export; -use types::Empty; +use types::SignedDelegation; -pub type Args = Empty; +#[ts_export(identity, create_account_linking_code)] +#[derive(CandidType, Serialize, Deserialize, Debug)] +pub struct Args { + #[serde(with = "serde_bytes")] + pub public_key: Vec, + pub delegation: SignedDelegation, +} #[ts_export(identity, create_account_linking_code)] #[derive(CandidType, Serialize, Deserialize, Debug)] pub enum Response { Success(AccountLinkingCode), UserNotFound, + DelegationTooOld, } diff --git a/backend/canisters/identity/impl/src/lib.rs b/backend/canisters/identity/impl/src/lib.rs index c9686ff565..3299f244f5 100644 --- a/backend/canisters/identity/impl/src/lib.rs +++ b/backend/canisters/identity/impl/src/lib.rs @@ -148,14 +148,6 @@ impl RuntimeState { } } - pub fn get_user_id_by_caller(&self) -> Option { - let caller = self.caller_auth_principal(); - self.data - .user_principals - .get_by_auth_principal(&caller) - .and_then(|u| u.user_id) - } - pub fn metrics(&self) -> Metrics { Metrics { heap_memory_used: utils::memory::heap(), diff --git a/backend/canisters/identity/impl/src/model/user_principals.rs b/backend/canisters/identity/impl/src/model/user_principals.rs index c5b39ebe31..7303326959 100644 --- a/backend/canisters/identity/impl/src/model/user_principals.rs +++ b/backend/canisters/identity/impl/src/model/user_principals.rs @@ -273,7 +273,7 @@ impl UserPrincipals { .and_then(|p| self.user_principals.get_mut(p.user_principal_index as usize)) } - fn user_principal_by_index(&self, user_principal_index: u32) -> Option { + pub fn user_principal_by_index(&self, user_principal_index: u32) -> Option { self.user_principals .get(usize::try_from(user_principal_index).unwrap()) .map(|u| UserPrincipal { diff --git a/backend/canisters/identity/impl/src/updates/create_account_linking_code.rs b/backend/canisters/identity/impl/src/updates/create_account_linking_code.rs index 23c3950180..14bf90fd7d 100644 --- a/backend/canisters/identity/impl/src/updates/create_account_linking_code.rs +++ b/backend/canisters/identity/impl/src/updates/create_account_linking_code.rs @@ -6,16 +6,35 @@ use identity_canister::create_account_linking_code::{Response::*, *}; #[update(msgpack = true, candid = true)] #[trace] -fn create_account_linking_code(_args: Args) -> Response { - mutate_state(create_account_linking_code_impl) +fn create_account_linking_code(args: Args) -> Response { + mutate_state(|state| create_account_linking_code_impl(args, state)) } -fn create_account_linking_code_impl(state: &mut RuntimeState) -> Response { - let Some(user_id) = state.get_user_id_by_caller() else { +fn create_account_linking_code_impl(args: Args, state: &mut RuntimeState) -> Response { + let caller = state.caller_auth_principal(); + + let Some(auth_principal) = state.data.user_principals.get_auth_principal(&caller) else { + return UserNotFound; + }; + + let Some(user_id) = state + .data + .user_principals + .user_principal_by_index(auth_principal.user_principal_index) + .and_then(|p| p.user_id) + else { return UserNotFound; }; let now = state.env.now(); + if state + .data + .verify_certificate_time(&auth_principal, &args.delegation.signature, now, 5 * MINUTE_IN_MS) + .is_err() + { + return DelegationTooOld; + } + let rng = state.env.rng(); // Clean up expired codes, keeps the memory footprint smaller in diff --git a/backend/integration_tests/src/account_linking_tests.rs b/backend/integration_tests/src/account_linking_tests.rs index 51c13b33a9..5a291f31f4 100644 --- a/backend/integration_tests/src/account_linking_tests.rs +++ b/backend/integration_tests/src/account_linking_tests.rs @@ -15,6 +15,11 @@ fn test_account_linking_create_link_code() { // Original user that will create the linking code let (user, user_auth) = register_user_and_include_auth(env, canister_ids); + let create_account_linking_code_args = identity_canister::create_account_linking_code::Args { + public_key: user.public_key, + delegation: user_auth.delegation, + }; + // // ---- Create a linking code ---- // @@ -22,7 +27,7 @@ fn test_account_linking_create_link_code() { env, user_auth.auth_principal, canister_ids.identity, - &identity_canister::create_account_linking_code::Args {}, + &create_account_linking_code_args, ); let created_linking_code = match create_linking_code_response { @@ -30,7 +35,7 @@ fn test_account_linking_create_link_code() { response => panic!("'create_account_linking_code' error: {response:?}"), }; - assert!(created_linking_code.value.len() == 6); + assert_eq!(created_linking_code.value.len(), 6); assert!(created_linking_code.is_valid(now_millis(env))); assert_eq!(created_linking_code.user_id, user.user_id); @@ -42,7 +47,7 @@ fn test_account_linking_create_link_code() { env, user_auth.auth_principal, canister_ids.identity, - &identity_canister::create_account_linking_code::Args {}, + &create_account_linking_code_args, ); match repeated_linking_code_response { @@ -79,9 +84,8 @@ fn test_account_linking_create_link_code() { // // ---- Verify that the linking is successful by querying for it ---- // - let auth_principals_response = client::identity::happy_path::auth_principals(env, new_principal, canister_ids.identity); + let check_auth_principal_response = + client::identity::happy_path::check_auth_principal(env, new_principal, canister_ids.identity); - assert_eq!(auth_principals_response.len(), 2); - assert!(auth_principals_response.first().unwrap().is_ii_principal); - assert!(!auth_principals_response.last().unwrap().is_ii_principal); + assert_eq!(check_auth_principal_response.user_id, Some(user.user_id)); } diff --git a/backend/integration_tests/src/client/identity.rs b/backend/integration_tests/src/client/identity.rs index 82eb04f81b..7f2a09a6c5 100644 --- a/backend/integration_tests/src/client/identity.rs +++ b/backend/integration_tests/src/client/identity.rs @@ -22,7 +22,7 @@ pub mod happy_path { use candid::Principal; use identity_canister::auth_principals::UserPrincipal; use pocket_ic::PocketIc; - use types::{CanisterId, SignedDelegation, TimestampMillis}; + use types::{CanisterId, Empty, SignedDelegation, TimestampMillis}; pub fn create_identity( env: &mut PocketIc, @@ -214,6 +214,19 @@ pub mod happy_path { } } + pub fn check_auth_principal( + env: &PocketIc, + sender: Principal, + identity_canister_id: CanisterId, + ) -> identity_canister::check_auth_principal_v2::SuccessResult { + let response = super::check_auth_principal_v2(env, sender, identity_canister_id, &Empty {}); + + match response { + identity_canister::check_auth_principal_v2::Response::Success(result) => result, + response => panic!("'check_auth_principal_v2' error: {response:?}"), + } + } + pub fn auth_principals(env: &mut PocketIc, sender: Principal, identity_canister_id: CanisterId) -> Vec { let response = super::auth_principals( env,