From d7503f119d53b9b00130458289e829ddfad0157c Mon Sep 17 00:00:00 2001 From: Frederik Rothenberger Date: Mon, 16 Sep 2024 14:58:24 +0200 Subject: [PATCH 1/3] Change registration on API v2 to flow with 3 steps The registration for new identities using API v2 is now split into 3 different steps: 1. start a flow, providing a principal to track progress on that flow 2. solve captcha 3. submit authn_method to tie the identity to This PR is a preparation to making the step 2 dynamically remove step 2 on low load. --- .github/workflows/canister-tests.yml | 15 + allowed_breaking_change.patch | 121 +++++++++ .../src/api/internet_identity/api_v2.rs | 39 ++- src/internet_identity/internet_identity.did | 94 +++++-- .../src/anchor_management/registration.rs | 7 +- .../anchor_management/registration/captcha.rs | 3 +- .../registration/rate_limit.rs | 6 +- .../registration/registration_flow_v2.rs | 172 ++++++++++++ src/internet_identity/src/main.rs | 28 +- src/internet_identity/src/state.rs | 11 + .../src/state/flow_states.rs | 131 +++++++++ .../v2_api/authn_method_test_helpers.rs | 32 +-- .../integration/v2_api/identity_register.rs | 257 ++++++++++++------ .../src/internet_identity/types/api_v2.rs | 51 ++++ 14 files changed, 817 insertions(+), 150 deletions(-) create mode 100644 allowed_breaking_change.patch create mode 100644 src/internet_identity/src/anchor_management/registration/registration_flow_v2.rs create mode 100644 src/internet_identity/src/state/flow_states.rs diff --git a/.github/workflows/canister-tests.yml b/.github/workflows/canister-tests.yml index c2b3d140a3..3c0d11fdcc 100644 --- a/.github/workflows/canister-tests.yml +++ b/.github/workflows/canister-tests.yml @@ -955,8 +955,23 @@ jobs: steps: - uses: actions/checkout@v4 - uses: ./.github/actions/setup-didc + - name: 'Get latest release' + uses: actions/github-script@v7 + id: latest-release-tag + with: + result-encoding: string + script: return (await github.rest.repos.getLatestRelease({owner:"dfinity", repo:"internet-identity"})).data.tag_name; - name: "Check canister interface compatibility" run: | + release="release-2024-09-17" + # undo the breaking changes that we _explicitly_ made + # remove after the next release + # if we accidentally introduced other breaking changes, the patch would no longer apply / fix them + # making this job fail. + if [ "${{ steps.latest-release-tag.outputs.result }}" == "$release" ]; then + echo "Rolling back intentionally made breaking changes $release" + git apply allowed_breaking_change.patch + fi curl -sSL https://github.com/dfinity/internet-identity/releases/latest/download/internet_identity.did -o internet_identity_previous.did didc check src/internet_identity/internet_identity.did internet_identity_previous.did diff --git a/allowed_breaking_change.patch b/allowed_breaking_change.patch new file mode 100644 index 0000000000..ff53ba55a3 --- /dev/null +++ b/allowed_breaking_change.patch @@ -0,0 +1,121 @@ +diff --git a/src/internet_identity/internet_identity.did b/src/internet_identity/internet_identity.did +index 500ef4d6..eb75b075 100644 +--- a/src/internet_identity/internet_identity.did ++++ b/src/internet_identity/internet_identity.did +@@ -435,6 +435,17 @@ type IdentityInfoError = variant { + InternalCanisterError: text; + }; + ++ ++ ++type IdentityRegisterError = variant { ++ // No more registrations are possible in this instance of the II service canister. ++ CanisterFull; ++ // The captcha check was not successful. ++ BadCaptcha; ++ // The metadata of the provided authentication method contains invalid entries. ++ InvalidMetadata: text; ++}; ++ + type AuthnMethodAddError = variant { + InvalidMetadata: text; + }; +@@ -521,72 +532,6 @@ type SignedIdAlias = record { + id_dapp : principal; + }; + +-type IdRegNextStepResult = record { +- // The next step in the registration flow +- next_step: RegistrationFlowNextStep; +-}; +- +-type IdRegStartError = variant { +- // The method was called anonymously, which is not supported. +- InvalidCaller; +- // Too many registrations. Please try again later. +- RateLimitExceeded; +- // A registration flow is already in progress. +- AlreadyInProgress; +-}; +- +-// The next step in the registration flow: +-// - CheckCaptcha: supply the solution to the captcha using `check_captcha` +-// - Finish: finish the registration using `identity_registration_finish` +-type RegistrationFlowNextStep = variant { +- // Supply the captcha solution using check_captcha +- CheckCaptcha: record { +- captcha_png_base64: text; +- }; +- // Finish the registration using identity_registration_finish +- Finish; +-}; +- +-type CheckCaptchaArg = record { +- solution : text; +-}; +- +-type CheckCaptchaError = variant { +- // The supplied solution was wrong. Try again with the new captcha. +- WrongSolution: record { +- new_captcha_png_base64: text; +- }; +- // This call is unexpected, see next_step. +- UnexpectedCall: record { +- next_step: RegistrationFlowNextStep; +- }; +- // No registration flow ongoing for the caller. +- NoRegistrationFlow; +-}; +- +-type IdRegFinishArg = record { +- authn_method: AuthnMethodData; +-}; +- +-type IdRegFinishResult = record { +- identity_number: nat64; +-}; +- +-type IdRegFinishError = variant { +- // The configured maximum number of identities has been reached. +- IdentityLimitReached; +- // This call is unexpected, see next_step. +- UnexpectedCall: record { +- next_step: RegistrationFlowNextStep; +- }; +- // No registration flow ongoing for the caller. +- NoRegistrationFlow; +- // The supplied authn_method is not valid. +- InvalidAuthnMethod: text; +- // Error while persisting the new identity. +- StorageError: text; +-}; +- + service : (opt InternetIdentityInit) -> { + // Legacy identity management API + // ============================== +@@ -613,17 +558,16 @@ service : (opt InternetIdentityInit) -> { + + // V2 Identity Management API + // ========================== +- // WARNING: The following methods are experimental and may ch 0ange in the future. +- +- // Starts the identity registration flow to create a new identity. +- identity_registration_start: () -> (variant {Ok: IdRegNextStepResult; Err: IdRegStartError;}); ++ // WARNING: The following methods are experimental and may change in the future. + +- // Check the captcha challenge +- // If successful, the registration can be finished with `identity_registration_finish`. +- check_captcha: (CheckCaptchaArg) -> (variant {Ok: IdRegNextStepResult; Err: CheckCaptchaError;}); ++ // Creates a new captcha. The solution needs to be submitted using the ++ // `identity_register` call. ++ captcha_create: () -> (variant {Ok: Challenge; Err;}); + +- // Starts the identity registration flow to create a new identity. +- identity_registration_finish: (IdRegFinishArg) -> (variant {Ok: IdRegFinishResult; Err: IdRegFinishError;}); ++ // Registers a new identity with the given authn_method. ++ // A valid captcha solution to a previously generated captcha (using create_captcha) must be provided. ++ // The sender needs to match the supplied authn_method. ++ identity_register: (AuthnMethodData, CaptchaResult, opt principal) -> (variant {Ok: IdentityNumber; Err: IdentityRegisterError;}); + + // Returns information about the authentication methods of the identity with the given number. + // Only returns the minimal information required for authentication without exposing any metadata such as aliases. diff --git a/src/canister_tests/src/api/internet_identity/api_v2.rs b/src/canister_tests/src/api/internet_identity/api_v2.rs index 344ad7f59c..bbee60639d 100644 --- a/src/canister_tests/src/api/internet_identity/api_v2.rs +++ b/src/canister_tests/src/api/internet_identity/api_v2.rs @@ -5,35 +5,54 @@ use pocket_ic::common::rest::RawEffectivePrincipal; use pocket_ic::{call_candid, call_candid_as, query_candid, CallError, PocketIc}; use std::collections::HashMap; -pub fn captcha_create( +pub fn identity_registration_start( env: &PocketIc, canister_id: CanisterId, -) -> Result, CallError> { - call_candid( + sender: Principal, +) -> Result, CallError> { + call_candid_as( env, canister_id, RawEffectivePrincipal::None, - "captcha_create", + sender, + "identity_registration_start", (), ) .map(|(x,)| x) } -pub fn identity_register( +pub fn check_captcha( + env: &PocketIc, + canister_id: CanisterId, + sender: Principal, + solution: String, +) -> Result, CallError> { + call_candid_as( + env, + canister_id, + RawEffectivePrincipal::None, + sender, + "check_captcha", + (CheckCaptchaArg { solution },), + ) + .map(|(x,)| x) +} + +pub fn identity_registration_finish( env: &PocketIc, canister_id: CanisterId, sender: Principal, authn_method: &AuthnMethodData, - challenge_attempt: &ChallengeAttempt, - temp_key: Option, -) -> Result, CallError> { +) -> Result, CallError> { call_candid_as( env, canister_id, RawEffectivePrincipal::None, sender, - "identity_register", - (authn_method, challenge_attempt, temp_key), + "identity_registration_finish", + (IdRegFinishArg { + authn_method: authn_method.clone(), + },), ) .map(|(x,)| x) } diff --git a/src/internet_identity/internet_identity.did b/src/internet_identity/internet_identity.did index eb75b075d2..500ef4d6e8 100644 --- a/src/internet_identity/internet_identity.did +++ b/src/internet_identity/internet_identity.did @@ -435,17 +435,6 @@ type IdentityInfoError = variant { InternalCanisterError: text; }; - - -type IdentityRegisterError = variant { - // No more registrations are possible in this instance of the II service canister. - CanisterFull; - // The captcha check was not successful. - BadCaptcha; - // The metadata of the provided authentication method contains invalid entries. - InvalidMetadata: text; -}; - type AuthnMethodAddError = variant { InvalidMetadata: text; }; @@ -532,6 +521,72 @@ type SignedIdAlias = record { id_dapp : principal; }; +type IdRegNextStepResult = record { + // The next step in the registration flow + next_step: RegistrationFlowNextStep; +}; + +type IdRegStartError = variant { + // The method was called anonymously, which is not supported. + InvalidCaller; + // Too many registrations. Please try again later. + RateLimitExceeded; + // A registration flow is already in progress. + AlreadyInProgress; +}; + +// The next step in the registration flow: +// - CheckCaptcha: supply the solution to the captcha using `check_captcha` +// - Finish: finish the registration using `identity_registration_finish` +type RegistrationFlowNextStep = variant { + // Supply the captcha solution using check_captcha + CheckCaptcha: record { + captcha_png_base64: text; + }; + // Finish the registration using identity_registration_finish + Finish; +}; + +type CheckCaptchaArg = record { + solution : text; +}; + +type CheckCaptchaError = variant { + // The supplied solution was wrong. Try again with the new captcha. + WrongSolution: record { + new_captcha_png_base64: text; + }; + // This call is unexpected, see next_step. + UnexpectedCall: record { + next_step: RegistrationFlowNextStep; + }; + // No registration flow ongoing for the caller. + NoRegistrationFlow; +}; + +type IdRegFinishArg = record { + authn_method: AuthnMethodData; +}; + +type IdRegFinishResult = record { + identity_number: nat64; +}; + +type IdRegFinishError = variant { + // The configured maximum number of identities has been reached. + IdentityLimitReached; + // This call is unexpected, see next_step. + UnexpectedCall: record { + next_step: RegistrationFlowNextStep; + }; + // No registration flow ongoing for the caller. + NoRegistrationFlow; + // The supplied authn_method is not valid. + InvalidAuthnMethod: text; + // Error while persisting the new identity. + StorageError: text; +}; + service : (opt InternetIdentityInit) -> { // Legacy identity management API // ============================== @@ -558,16 +613,17 @@ service : (opt InternetIdentityInit) -> { // V2 Identity Management API // ========================== - // WARNING: The following methods are experimental and may change in the future. + // WARNING: The following methods are experimental and may ch 0ange in the future. + + // Starts the identity registration flow to create a new identity. + identity_registration_start: () -> (variant {Ok: IdRegNextStepResult; Err: IdRegStartError;}); - // Creates a new captcha. The solution needs to be submitted using the - // `identity_register` call. - captcha_create: () -> (variant {Ok: Challenge; Err;}); + // Check the captcha challenge + // If successful, the registration can be finished with `identity_registration_finish`. + check_captcha: (CheckCaptchaArg) -> (variant {Ok: IdRegNextStepResult; Err: CheckCaptchaError;}); - // Registers a new identity with the given authn_method. - // A valid captcha solution to a previously generated captcha (using create_captcha) must be provided. - // The sender needs to match the supplied authn_method. - identity_register: (AuthnMethodData, CaptchaResult, opt principal) -> (variant {Ok: IdentityNumber; Err: IdentityRegisterError;}); + // Starts the identity registration flow to create a new identity. + identity_registration_finish: (IdRegFinishArg) -> (variant {Ok: IdRegFinishResult; Err: IdRegFinishError;}); // Returns information about the authentication methods of the identity with the given number. // Only returns the minimal information required for authentication without exposing any metadata such as aliases. diff --git a/src/internet_identity/src/anchor_management/registration.rs b/src/internet_identity/src/anchor_management/registration.rs index f194067c57..0514f9f445 100644 --- a/src/internet_identity/src/anchor_management/registration.rs +++ b/src/internet_identity/src/anchor_management/registration.rs @@ -1,4 +1,3 @@ -use crate::anchor_management::registration::captcha::Base64; use crate::anchor_management::{activity_bookkeeping, post_operation_bookkeeping}; use crate::state; use crate::state::temp_keys::TempKeyId; @@ -12,6 +11,9 @@ use internet_identity_interface::internet_identity::types::*; mod captcha; mod rate_limit; +pub mod registration_flow_v2; + +pub use captcha::Base64; pub async fn create_challenge() -> Challenge { let mut rng = captcha::make_rng().await; @@ -65,7 +67,8 @@ pub fn register( // The key is optional for backwards compatibility temp_key: Option, ) -> RegisterResponse { - rate_limit::process_rate_limit(); + rate_limit::process_rate_limit() + .unwrap_or_else(|_| trap("rate limit reached, try again later")); if let Err(()) = captcha::check_challenge(challenge_result) { return RegisterResponse::BadChallenge; } diff --git a/src/internet_identity/src/anchor_management/registration/captcha.rs b/src/internet_identity/src/anchor_management/registration/captcha.rs index f4954c3ff1..71bcac4cd4 100644 --- a/src/internet_identity/src/anchor_management/registration/captcha.rs +++ b/src/internet_identity/src/anchor_management/registration/captcha.rs @@ -9,6 +9,7 @@ use lazy_static::lazy_static; use rand_core::{RngCore, SeedableRng}; use std::collections::{HashMap, HashSet}; +#[derive(Clone, Debug)] pub struct Base64(pub String); lazy_static! { @@ -143,7 +144,7 @@ pub fn check_challenge(res: ChallengeAttempt) -> Result<(), ()> { /// Check whether the supplied CAPTCHA solution attempt matches the expected solution (after /// normalizing ambiguous characters). -fn check_captcha_solution(solution_attempt: String, solution: String) -> Result<(), ()> { +pub fn check_captcha_solution(solution_attempt: String, solution: String) -> Result<(), ()> { // avoid processing too many characters if solution_attempt.len() > CAPTCHA_LENGTH { return Err(()); diff --git a/src/internet_identity/src/anchor_management/registration/rate_limit.rs b/src/internet_identity/src/anchor_management/registration/rate_limit.rs index d71437d0d8..eddb821316 100644 --- a/src/internet_identity/src/anchor_management/registration/rate_limit.rs +++ b/src/internet_identity/src/anchor_management/registration/rate_limit.rs @@ -1,7 +1,6 @@ use crate::state; use crate::state::RateLimitState; use ic_cdk::api::time; -use ic_cdk::trap; use internet_identity_interface::internet_identity::types::RateLimitConfig; use std::cmp::min; @@ -15,7 +14,7 @@ use std::cmp::min; /// tokens have replenished. /// There is a maximum of `max_tokens` tokens, when reached the tokens not increase any further. /// This is the maximum number of calls that can be handled in a burst. -pub fn process_rate_limit() { +pub fn process_rate_limit() -> Result<(), ()> { let config = state::persistent_state(|ps| ps.registration_rate_limit.clone()); state::registration_rate_limit_mut(|state_opt| { @@ -35,8 +34,9 @@ pub fn process_rate_limit() { if state.tokens > 0 { state.tokens -= 1; } else { - trap("rate limit reached, try again later"); + return Err(()); } + Ok(()) }) } diff --git a/src/internet_identity/src/anchor_management/registration/registration_flow_v2.rs b/src/internet_identity/src/anchor_management/registration/registration_flow_v2.rs new file mode 100644 index 0000000000..fa24d4cb5a --- /dev/null +++ b/src/internet_identity/src/anchor_management/registration/registration_flow_v2.rs @@ -0,0 +1,172 @@ +use crate::anchor_management::registration::captcha::{ + check_captcha_solution, create_captcha, make_rng, Base64, +}; +use crate::anchor_management::registration::rate_limit::process_rate_limit; +use crate::anchor_management::{activity_bookkeeping, post_operation_bookkeeping}; +use crate::state; +use crate::state::flow_states::RegistrationFlowState; +use crate::state::temp_keys::TempKeyId; +use crate::storage::anchor::Device; +use candid::Principal; +use ic_cdk::api::time; +use ic_cdk::caller; +use internet_identity_interface::archive::types::{DeviceDataWithoutAlias, Operation}; +use internet_identity_interface::internet_identity::types::IdRegFinishError::IdentityLimitReached; +use internet_identity_interface::internet_identity::types::{ + CheckCaptchaArg, CheckCaptchaError, DeviceData, DeviceWithUsage, IdRegFinishArg, + IdRegFinishError, IdRegFinishResult, IdRegNextStepResult, IdRegStartError, IdentityNumber, + RegistrationFlowNextStep, +}; + +impl From for RegistrationFlowNextStep { + fn from(value: RegistrationFlowState) -> Self { + match value { + RegistrationFlowState::CheckCaptcha { + captcha_png_base64, .. + } => RegistrationFlowNextStep::CheckCaptcha { + captcha_png_base64: captcha_png_base64.0, + }, + RegistrationFlowState::FinishRegistration { .. } => RegistrationFlowNextStep::Finish, + } + } +} + +pub async fn identity_registration_start() -> Result { + process_rate_limit().map_err(|_| IdRegStartError::RateLimitExceeded)?; + + let caller = caller(); + if caller == Principal::anonymous() { + return Err(IdRegStartError::InvalidCaller); + } + + let (captcha_png_base64, flow_state) = captcha_flow_state(time()).await; + state::with_flow_states_mut(|flow_states| { + flow_states.new_registration_flow(caller, flow_state) + }) + .map_err(|_| IdRegStartError::AlreadyInProgress)?; + + Ok(IdRegNextStepResult { + next_step: RegistrationFlowNextStep::CheckCaptcha { + captcha_png_base64: captcha_png_base64.0, + }, + }) +} + +async fn captcha_flow_state(flow_created_timestamp_ns: u64) -> (Base64, RegistrationFlowState) { + let mut rng = &mut make_rng().await; + let (captcha_png_base64, captcha_solution) = create_captcha(&mut rng); + let flow_state = RegistrationFlowState::CheckCaptcha { + flow_created_timestamp_ns, + captcha_solution, + captcha_png_base64: captcha_png_base64.clone(), + }; + (captcha_png_base64, flow_state) +} + +pub async fn check_captcha(arg: CheckCaptchaArg) -> Result { + let Some(current_state) = state::with_flow_states(|s| s.registration_flow_state(&caller())) + else { + return Err(CheckCaptchaError::NoRegistrationFlow); + }; + + let RegistrationFlowState::CheckCaptcha { + flow_created_timestamp_ns, + captcha_solution, + .. + } = current_state + else { + return Err(CheckCaptchaError::UnexpectedCall { + next_step: RegistrationFlowNextStep::from(current_state), + }); + }; + + let caller = caller(); + if check_captcha_solution(arg.solution, captcha_solution).is_err() { + let (captcha_png_base64, flow_state) = captcha_flow_state(flow_created_timestamp_ns).await; + state::with_flow_states_mut(|flow_states| { + flow_states.update_registration_flow(caller, flow_state) + }) + // If we fail to update the flow, then the flow has expired in the time it took to create + // the captcha. + .map_err(|_| CheckCaptchaError::NoRegistrationFlow)?; + + return Err(CheckCaptchaError::WrongSolution { + new_captcha_png_base64: captcha_png_base64.0, + }); + } + + let flow_state = RegistrationFlowState::FinishRegistration { + flow_created_timestamp_ns, + }; + + state::with_flow_states_mut(|flow_states| { + flow_states.update_registration_flow(caller, flow_state) + }) + // The update_registration_flow triggers a clean-up as well, which could lead to a situation where + // the flow being processed here is cleaned up. In that case, abort with an error. + .map_err(|_| CheckCaptchaError::NoRegistrationFlow)?; + + Ok(IdRegNextStepResult { + next_step: RegistrationFlowNextStep::Finish, + }) +} + +pub fn identity_registration_finish( + arg: IdRegFinishArg, +) -> Result { + let Some(current_state) = state::with_flow_states(|s| s.registration_flow_state(&caller())) + else { + return Err(IdRegFinishError::NoRegistrationFlow); + }; + + let RegistrationFlowState::FinishRegistration { .. } = current_state else { + return Err(IdRegFinishError::UnexpectedCall { + next_step: RegistrationFlowNextStep::from(current_state), + }); + }; + + let identity_number = create_identity(&arg)?; + + let caller = caller(); + // flow completed --> remove flow state + state::with_flow_states_mut(|flow_states| flow_states.remove_registration_flow(&caller)); + + // add temp key so the user can keep using the identity used for the registration flow + state::with_temp_keys_mut(|temp_keys| { + temp_keys.add_temp_key( + TempKeyId::IdentityAuthnMethod { + identity_number, + authn_method_pubkey: arg.authn_method.public_key(), + }, + caller, + ) + }); + + Ok(IdRegFinishResult { identity_number }) +} + +fn create_identity(arg: &IdRegFinishArg) -> Result { + let Some(mut identity) = state::storage_borrow_mut(|s| s.allocate_anchor()) else { + return Err(IdentityLimitReached); + }; + let device = DeviceWithUsage::try_from(arg.authn_method.clone()) + .map(|device| Device::from(DeviceData::from(device))) + .map_err(|err| IdRegFinishError::InvalidAuthnMethod(err.to_string()))?; + + identity + .add_device(device.clone()) + .map_err(|err| IdRegFinishError::InvalidAuthnMethod(err.to_string()))?; + activity_bookkeeping(&mut identity, &device.pubkey); + + let identity_number = identity.anchor_number(); + + state::storage_borrow_mut(|s| s.write(identity)) + .map_err(|err| IdRegFinishError::StorageError(err.to_string()))?; + + let operation = Operation::RegisterAnchor { + device: DeviceDataWithoutAlias::from(device), + }; + post_operation_bookkeeping(identity_number, operation); + + Ok(identity_number) +} diff --git a/src/internet_identity/src/main.rs b/src/internet_identity/src/main.rs index d32e5d9d56..a709b4d34f 100644 --- a/src/internet_identity/src/main.rs +++ b/src/internet_identity/src/main.rs @@ -6,6 +6,7 @@ use crate::archive::ArchiveState; use crate::assets::init_assets; use crate::state::persistent_state; use crate::stats::event_stats::all_aggregations_top_n; +use anchor_management::registration; use authz_utils::{ anchor_operation_with_authz_check, check_authorization, check_authz_and_record_activity, }; @@ -135,7 +136,7 @@ fn verify_tentative_device( #[update] async fn create_challenge() -> Challenge { - anchor_management::registration::create_challenge().await + registration::create_challenge().await } #[update] @@ -514,25 +515,20 @@ mod v2_api { } #[update] - async fn captcha_create() -> Result { - let challenge = anchor_management::registration::create_challenge().await; - Ok(challenge) + async fn identity_registration_start() -> Result { + registration::registration_flow_v2::identity_registration_start().await } #[update] - fn identity_register( - authn_method: AuthnMethodData, - challenge_result: ChallengeAttempt, - temp_key: Option, - ) -> Result { - let device = DeviceWithUsage::try_from(authn_method) - .map_err(|err| IdentityRegisterError::InvalidMetadata(err.to_string()))?; + async fn check_captcha(arg: CheckCaptchaArg) -> Result { + registration::registration_flow_v2::check_captcha(arg).await + } - match register(DeviceData::from(device), challenge_result, temp_key) { - RegisterResponse::Registered { user_number } => Ok(user_number), - RegisterResponse::CanisterFull => Err(IdentityRegisterError::CanisterFull), - RegisterResponse::BadChallenge => Err(IdentityRegisterError::BadCaptcha), - } + #[update] + fn identity_registration_finish( + arg: IdRegFinishArg, + ) -> Result { + registration::registration_flow_v2::identity_registration_finish(arg) } #[update] diff --git a/src/internet_identity/src/state.rs b/src/internet_identity/src/state.rs index fd8295e9ad..1e8d383a37 100644 --- a/src/internet_identity/src/state.rs +++ b/src/internet_identity/src/state.rs @@ -1,4 +1,5 @@ use crate::archive::{ArchiveData, ArchiveState, ArchiveStatusCache}; +use crate::state::flow_states::FlowStates; use crate::state::temp_keys::TempKeys; use crate::stats::activity_stats::activity_counter::active_anchor_counter::ActiveAnchorCounter; use crate::stats::activity_stats::activity_counter::authn_method_counter::AuthnMethodCounter; @@ -19,6 +20,7 @@ use std::collections::HashMap; use std::ops::{Deref, DerefMut}; use std::time::Duration; +pub mod flow_states; pub mod temp_keys; /// Default captcha config @@ -169,6 +171,7 @@ struct State { sigs: RefCell, // Temporary keys that can be used in lieu of a particular device temp_keys: RefCell, + flow_states: RefCell, last_upgrade_timestamp: Cell, // note: we COULD persist this through upgrades, although this is currently NOT persisted // through upgrades @@ -350,6 +353,14 @@ pub fn with_temp_keys(f: impl FnOnce(&TempKeys) -> R) -> R { STATE.with(|s| f(&mut s.temp_keys.borrow())) } +pub fn with_flow_states_mut(f: impl FnOnce(&mut FlowStates) -> R) -> R { + STATE.with(|s| f(&mut s.flow_states.borrow_mut())) +} + +pub fn with_flow_states(f: impl FnOnce(&FlowStates) -> R) -> R { + STATE.with(|s| f(&mut s.flow_states.borrow())) +} + pub fn usage_metrics(f: impl FnOnce(&UsageMetrics) -> R) -> R { STATE.with(|s| f(&s.usage_metrics.borrow())) } diff --git a/src/internet_identity/src/state/flow_states.rs b/src/internet_identity/src/state/flow_states.rs new file mode 100644 index 0000000000..ab2f47e0f2 --- /dev/null +++ b/src/internet_identity/src/state/flow_states.rs @@ -0,0 +1,131 @@ +use crate::anchor_management::registration::Base64; +use crate::MINUTE_NS; +use candid::Principal; +use ic_cdk::api::time; +use internet_identity_interface::internet_identity::types::Timestamp; +use std::collections::{HashMap, VecDeque}; + +// Expiration for a registration flow, the same as the captcha expiry +const FLOW_EXPIRATION_NS: u64 = 5 * MINUTE_NS; + +#[derive(Default)] +pub struct FlowStates { + /// In-progress registration flows tied to the respective principal. The principal will become + /// a temp key upon successful completion of the registration flow. + /// Registration flows are removed on three conditions: + /// * The registration flow is completed successfully + /// * Once the flow expires + /// * The Internet Identity canister is upgraded + /// + /// The total number of flow states is limited by the registration rate limit. + registration_flows: HashMap, + + /// Deque to efficiently prune expired registration flows + /// + /// The total number of flow state expirations is limited by the registration rate limit. + expirations: VecDeque, +} + +/// State of a registration flow. The state expresses which action is expected next. +#[derive(Debug, Clone)] +pub enum RegistrationFlowState { + /// Client is expected to supply the `captcha_solution` using + /// `check_captcha`. + CheckCaptcha { + flow_created_timestamp_ns: u64, + captcha_solution: String, + captcha_png_base64: Base64, + }, + /// Client is expected to finish the registration (supply an [authn_method](internet_identity_interface::internet_identity::types::AuthnMethodData)) + /// using `identity_registration_finish`. + FinishRegistration { flow_created_timestamp_ns: u64 }, +} + +impl RegistrationFlowState { + pub fn create_at_timestamp_ns(&self) -> u64 { + match self { + RegistrationFlowState::CheckCaptcha { + flow_created_timestamp_ns, + .. + } + | RegistrationFlowState::FinishRegistration { + flow_created_timestamp_ns, + } => *flow_created_timestamp_ns, + } + } +} + +impl FlowStates { + pub fn new_registration_flow( + &mut self, + principal: Principal, + flow_state: RegistrationFlowState, + ) -> Result<(), String> { + self.prune_expired_flows(); + if self.registration_flows.contains_key(&principal) { + return Err("already exists".to_string()); + } + + self.expirations.push_back(FlowExpiration { + principal, + expiration: flow_state.create_at_timestamp_ns() + FLOW_EXPIRATION_NS, + }); + + self.registration_flows.insert(principal, flow_state); + Ok(()) + } + + pub fn update_registration_flow( + &mut self, + principal: Principal, + flow_state: RegistrationFlowState, + ) -> Result<(), String> { + self.prune_expired_flows(); + if self.registration_flows.remove(&principal).is_none() { + return Err("no flow_state to update".to_string()); + } + self.registration_flows.insert(principal, flow_state); + Ok(()) + } + + pub fn remove_registration_flow(&mut self, principal: &Principal) { + self.prune_expired_flows(); + self.registration_flows.remove(principal); + } + + pub fn registration_flow_state(&self, principal: &Principal) -> Option { + self.registration_flows + .get(principal) + // filter out expired flow + .filter(|flow_state| flow_state.create_at_timestamp_ns() + FLOW_EXPIRATION_NS > time()) + .cloned() + } + + fn prune_expired_flows(&mut self) { + const MAX_TO_PRUNE: usize = 100; + + let now = time(); + for _ in 0..MAX_TO_PRUNE { + let Some(expiration) = self.expirations.front() else { + break; + }; + + // The expirations are sorted by expiration time because the expiration is constant + // and time() is monotonic + // -> we can stop pruning once we reach a flow that is not expired + if expiration.expiration > now { + break; + } + self.registration_flows.remove(&expiration.principal); + self.expirations.pop_front(); + } + } +} + +#[derive(Clone, Debug, Eq, PartialEq)] +struct FlowExpiration { + /// Principal which the flow is tied to. + pub principal: Principal, + /// The expiration timestamp of the temp key + pub expiration: Timestamp, +} diff --git a/src/internet_identity/tests/integration/v2_api/authn_method_test_helpers.rs b/src/internet_identity/tests/integration/v2_api/authn_method_test_helpers.rs index 0ff005ca3d..f8cac91b4d 100644 --- a/src/internet_identity/tests/integration/v2_api/authn_method_test_helpers.rs +++ b/src/internet_identity/tests/integration/v2_api/authn_method_test_helpers.rs @@ -1,9 +1,9 @@ use canister_tests::api::internet_identity::api_v2; +use canister_tests::framework::{test_principal, time}; use ic_cdk::api::management_canister::main::CanisterId; use internet_identity_interface::internet_identity::types::{ AuthnMethod, AuthnMethodData, AuthnMethodProtection, AuthnMethodPurpose, - AuthnMethodSecuritySettings, ChallengeAttempt, IdentityNumber, MetadataEntryV2, PublicKeyAuthn, - WebAuthn, + AuthnMethodSecuritySettings, IdentityNumber, MetadataEntryV2, PublicKeyAuthn, WebAuthn, }; use pocket_ic::PocketIc; use serde_bytes::ByteBuf; @@ -57,24 +57,20 @@ pub fn create_identity_with_authn_method( canister_id: CanisterId, authn_method: &AuthnMethodData, ) -> IdentityNumber { - let challenge = api_v2::captcha_create(env, canister_id) + // unique flow principal as the time changes every round + let flow_principal = test_principal(time(env)); + api_v2::identity_registration_start(env, canister_id, flow_principal) .expect("API call failed") - .expect("captcha_create failed"); + .expect("registration start failed"); - let challenge_attempt = ChallengeAttempt { - chars: "a".to_string(), - key: challenge.challenge_key, - }; - api_v2::identity_register( - env, - canister_id, - authn_method.principal(), - authn_method, - &challenge_attempt, - None, - ) - .expect("API call failed") - .expect("identity_register failed") + api_v2::check_captcha(env, canister_id, flow_principal, "a".to_string()) + .expect("API call failed") + .expect("check_captcha failed"); + + api_v2::identity_registration_finish(env, canister_id, flow_principal, authn_method) + .expect("API call failed") + .expect("registration finish failed") + .identity_number } pub fn create_identity_with_authn_methods( diff --git a/src/internet_identity/tests/integration/v2_api/identity_register.rs b/src/internet_identity/tests/integration/v2_api/identity_register.rs index ee4885b996..5bea2521ff 100644 --- a/src/internet_identity/tests/integration/v2_api/identity_register.rs +++ b/src/internet_identity/tests/integration/v2_api/identity_register.rs @@ -4,14 +4,15 @@ use crate::v2_api::authn_method_test_helpers::{ use candid::Principal; use canister_tests::api::internet_identity::api_v2; use canister_tests::framework::{ - arg_with_anchor_range, env, expect_user_error_with_message, install_ii_canister, - install_ii_canister_with_arg, II_WASM, + arg_with_anchor_range, arg_with_rate_limit, assert_metric, env, get_metrics, + install_ii_canister, install_ii_canister_with_arg, test_principal, time, II_WASM, }; +use internet_identity_interface::internet_identity::types::IdentityInfoError::Unauthorized; use internet_identity_interface::internet_identity::types::{ - ChallengeAttempt, IdentityRegisterError, MetadataEntryV2, + CheckCaptchaError, IdRegFinishError, IdRegStartError, MetadataEntryV2, RateLimitConfig, + RegistrationFlowNextStep, }; -use pocket_ic::ErrorCode::CanisterCalledTrap; -use regex::Regex; +use pocket_ic::CallError; use serde_bytes::ByteBuf; use std::time::Duration; @@ -37,6 +38,40 @@ fn should_register_multiple_identities() { assert_ne!(identity_number_1, identity_number_2); } +#[test] +fn should_transition_flow_principal_to_temp_key() { + let env = env(); + let canister_id = install_ii_canister(&env, II_WASM.clone()); + let authn_method = test_authn_method(); + let flow_principal = test_principal(time(&env)); + + api_v2::identity_registration_start(&env, canister_id, flow_principal) + .expect("API call failed") + .expect("registration start failed"); + + api_v2::check_captcha(&env, canister_id, flow_principal, "a".to_string()) + .expect("API call failed") + .expect("check_captcha failed"); + + let identity_nr = + api_v2::identity_registration_finish(&env, canister_id, flow_principal, &authn_method) + .expect("API call failed") + .expect("registration finish failed") + .identity_number; + + // authenticated call + let result = api_v2::identity_info(&env, canister_id, flow_principal, identity_nr) + .expect("API call failed"); + assert!(result.is_ok()); + + env.advance_time(Duration::from_secs(601)); + + // temp_key is expired now + let result = api_v2::identity_info(&env, canister_id, flow_principal, identity_nr) + .expect("API call failed"); + assert!(matches!(result, Err(Unauthorized(_)))); +} + #[test] fn should_not_exceed_configured_identity_range() { let env = env(); @@ -47,102 +82,122 @@ fn should_not_exceed_configured_identity_range() { create_identity_with_authn_method(&env, canister_id, &authn_method); create_identity_with_authn_method(&env, canister_id, &authn_method); - let challenge = api_v2::captcha_create(&env, canister_id) + let flow_principal = test_principal(0); + api_v2::identity_registration_start(&env, canister_id, flow_principal) + .expect("API call failed") + .expect("registration start failed"); + + api_v2::check_captcha(&env, canister_id, flow_principal, "a".to_string()) + .expect("API call failed") + .expect("check_captcha failed"); + + let result = + api_v2::identity_registration_finish(&env, canister_id, flow_principal, &authn_method) + .expect("API call failed"); + assert!(matches!( + result, + Err(IdRegFinishError::IdentityLimitReached) + )); +} + +#[test] +fn should_not_allow_wrong_captcha() { + let env = env(); + let canister_id = install_ii_canister(&env, II_WASM.clone()); + + let flow_principal = test_principal(0); + api_v2::identity_registration_start(&env, canister_id, flow_principal) .expect("API call failed") - .expect("captcha_create failed"); + .expect("registration start failed"); - let result = api_v2::identity_register( + let result = api_v2::check_captcha( &env, canister_id, - authn_method.principal(), - &authn_method, - &ChallengeAttempt { - chars: "a".to_string(), - key: challenge.challenge_key, - }, - None, + flow_principal, + "wrong solution".to_string(), ) .expect("API call failed"); - assert!(matches!(result, Err(IdentityRegisterError::CanisterFull))); + + assert!(matches!( + result, + Err(CheckCaptchaError::WrongSolution { .. }) + )); +} + +#[test] +fn should_not_allow_anonymous_principal_to_start_registration() { + let env = env(); + let canister_id = install_ii_canister(&env, II_WASM.clone()); + + let result = api_v2::identity_registration_start(&env, canister_id, Principal::anonymous()) + .expect("API call failed"); + + assert!(matches!(result, Err(IdRegStartError::InvalidCaller))); } #[test] -fn should_verify_sender_matches_authn_method() { +fn should_not_allow_different_principal() { let env = env(); let canister_id = install_ii_canister(&env, II_WASM.clone()); - let challenge = api_v2::captcha_create(&env, canister_id) + let flow_principal1 = test_principal(1); + let flow_principal2 = test_principal(2); + api_v2::identity_registration_start(&env, canister_id, flow_principal1) .expect("API call failed") - .expect("captcha_create failed"); + .expect("registration start failed"); - let result = api_v2::identity_register( - &env, - canister_id, - Principal::anonymous(), - &test_authn_method(), - &ChallengeAttempt { - chars: "a".to_string(), - key: challenge.challenge_key, - }, - None, - ); - expect_user_error_with_message( - result, - CanisterCalledTrap, - Regex::new("[a-z\\d-]+ could not be authenticated against").unwrap(), - ); + let result = api_v2::check_captcha(&env, canister_id, flow_principal2, "a".to_string()) + .expect("API call failed"); + + assert!(matches!(result, Err(CheckCaptchaError::NoRegistrationFlow))); } #[test] -fn should_not_allow_wrong_captcha() { +fn should_allow_captcha_retries() { let env = env(); let canister_id = install_ii_canister(&env, II_WASM.clone()); - let authn_method = test_authn_method(); - let challenge = api_v2::captcha_create(&env, canister_id) + let flow_principal = test_principal(0); + api_v2::identity_registration_start(&env, canister_id, flow_principal) .expect("API call failed") - .expect("captcha_create failed"); + .expect("registration start failed"); - let result = api_v2::identity_register( + let result = api_v2::check_captcha( &env, canister_id, - authn_method.principal(), - &authn_method, - &ChallengeAttempt { - chars: "wrong solution".to_string(), - key: challenge.challenge_key, - }, - None, + flow_principal, + "wrong solution".to_string(), ) .expect("API call failed"); - assert!(matches!(result, Err(IdentityRegisterError::BadCaptcha))); + + assert!(matches!( + result, + Err(CheckCaptchaError::WrongSolution { .. }) + )); + + let result = api_v2::check_captcha(&env, canister_id, flow_principal, "a".to_string()) + .expect("API call failed") + .expect("check_captcha failed"); + assert!(matches!(result.next_step, RegistrationFlowNextStep::Finish)) } #[test] -fn should_not_allow_expired_captcha() { +fn should_not_allow_to_continue_expired_flow() { let env = env(); let canister_id = install_ii_canister(&env, II_WASM.clone()); - let authn_method = test_authn_method(); - let challenge = api_v2::captcha_create(&env, canister_id) + let flow_principal = test_principal(0); + api_v2::identity_registration_start(&env, canister_id, flow_principal) .expect("API call failed") - .expect("captcha_create failed"); + .expect("registration start failed"); - env.advance_time(Duration::from_secs(301)); // one second longer than captcha validity + env.advance_time(Duration::from_secs(301)); // one second longer than the flow validity - let result = api_v2::identity_register( - &env, - canister_id, - authn_method.principal(), - &authn_method, - &ChallengeAttempt { - chars: "a".to_string(), - key: challenge.challenge_key, - }, - None, - ) - .expect("API call failed"); - assert!(matches!(result, Err(IdentityRegisterError::BadCaptcha))); + let result = api_v2::check_captcha(&env, canister_id, flow_principal, "a".to_string()) + .expect("API call failed"); + + // flow is not known anymore, as it has been pruned + assert!(matches!(result, Err(CheckCaptchaError::NoRegistrationFlow))); } #[test] @@ -155,24 +210,64 @@ fn should_fail_on_invalid_metadata() { MetadataEntryV2::Bytes(ByteBuf::from("invalid")), ); - let challenge = api_v2::captcha_create(&env, canister_id) + let flow_principal = test_principal(0); + api_v2::identity_registration_start(&env, canister_id, flow_principal) .expect("API call failed") - .expect("captcha_create failed"); + .expect("registration start failed"); - let result = api_v2::identity_register( - &env, - canister_id, - authn_method.principal(), - &authn_method, - &ChallengeAttempt { - chars: "a".to_string(), - key: challenge.challenge_key, - }, - None, - ) - .expect("API call failed"); + api_v2::check_captcha(&env, canister_id, flow_principal, "a".to_string()) + .expect("API call failed") + .expect("check_captcha failed"); + + let result = + api_v2::identity_registration_finish(&env, canister_id, flow_principal, &authn_method) + .expect("API call failed"); assert!(matches!( result, - Err(IdentityRegisterError::InvalidMetadata(_)) + Err(IdRegFinishError::InvalidAuthnMethod(_)) )); } + +#[test] +fn should_trigger_rate_limit_on_too_many_flows() -> Result<(), CallError> { + let env = env(); + let canister_id = install_ii_canister_with_arg( + &env, + II_WASM.clone(), + arg_with_rate_limit(RateLimitConfig { + time_per_token_ns: Duration::from_secs(1).as_nanos() as u64, + max_tokens: 3, + }), + ); + + // the canister starts out with max_tokens, so use some to make the replenish actually do something + for i in 0..3 { + api_v2::identity_registration_start(&env, canister_id, test_principal(i)) + .expect("API call failed") + .expect("registration start failed"); + } + + assert_metric( + &get_metrics(&env, canister_id), + "internet_identity_register_rate_limit_current_tokens", + 0f64, + ); + + let flow_principal = test_principal(3); + let result = api_v2::identity_registration_start(&env, canister_id, flow_principal) + .expect("API call failed"); + assert!(matches!(result, Err(IdRegStartError::RateLimitExceeded))); + + env.advance_time(Duration::from_secs(100)); + + let result = api_v2::identity_registration_start(&env, canister_id, flow_principal) + .expect("API call failed"); + assert!(result.is_ok()); + + assert_metric( + &get_metrics(&env, canister_id), + "internet_identity_register_rate_limit_current_tokens", + 2f64, + ); + Ok(()) +} diff --git a/src/internet_identity_interface/src/internet_identity/types/api_v2.rs b/src/internet_identity_interface/src/internet_identity/types/api_v2.rs index 0fce71c545..09415c10f0 100644 --- a/src/internet_identity_interface/src/internet_identity/types/api_v2.rs +++ b/src/internet_identity_interface/src/internet_identity/types/api_v2.rs @@ -149,3 +149,54 @@ pub enum IdentityMetadataReplaceError { }, InternalCanisterError(String), } + +#[derive(Clone, Debug, CandidType, Deserialize, Eq, PartialEq)] +pub struct IdRegNextStepResult { + pub next_step: RegistrationFlowNextStep, +} + +#[derive(Clone, Debug, CandidType, Deserialize, Eq, PartialEq)] +pub enum IdRegStartError { + RateLimitExceeded, + InvalidCaller, + AlreadyInProgress, +} + +#[derive(Clone, Debug, CandidType, Deserialize, Eq, PartialEq)] +pub enum RegistrationFlowNextStep { + /// Supply the captcha solution using check_captcha + CheckCaptcha { captcha_png_base64: String }, + /// Finish the registration using identity_registration_finish + Finish, +} + +#[derive(Clone, Debug, CandidType, Deserialize, Eq, PartialEq)] +pub struct CheckCaptchaArg { + pub solution: String, +} + +#[derive(Clone, Debug, CandidType, Deserialize, Eq, PartialEq)] +pub enum CheckCaptchaError { + WrongSolution { new_captcha_png_base64: String }, + UnexpectedCall { next_step: RegistrationFlowNextStep }, + NoRegistrationFlow, +} + +#[derive(Clone, Debug, CandidType, Deserialize, Eq, PartialEq)] +pub struct IdRegFinishArg { + pub authn_method: AuthnMethodData, +} + +#[derive(Clone, Debug, CandidType, Deserialize, Eq, PartialEq)] +pub struct IdRegFinishResult { + pub identity_number: IdentityNumber, +} + +#[derive(Clone, Debug, CandidType, Deserialize, Eq, PartialEq)] +pub enum IdRegFinishError { + IdentityLimitReached, + UnexpectedCall { next_step: RegistrationFlowNextStep }, + NoRegistrationFlow, + InvalidAuthnMethod(String), + StorageError(String), +} From 28aebf022feba7e17e8ace5288798fb4badeeffd Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 26 Sep 2024 12:54:26 +0000 Subject: [PATCH 2/3] =?UTF-8?q?=F0=9F=A4=96=20npm=20run=20generate=20auto-?= =?UTF-8?q?update?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../generated/internet_identity_idl.js | 59 ++++++++++++++----- .../generated/internet_identity_types.d.ts | 41 ++++++++++--- 2 files changed, 78 insertions(+), 22 deletions(-) diff --git a/src/frontend/generated/internet_identity_idl.js b/src/frontend/generated/internet_identity_idl.js index baa1177d0f..5f1c4e1dc6 100644 --- a/src/frontend/generated/internet_identity_idl.js +++ b/src/frontend/generated/internet_identity_idl.js @@ -148,6 +148,19 @@ export const idlFactory = ({ IDL }) => { const AuthnMethodSecuritySettingsReplaceError = IDL.Variant({ 'AuthnMethodNotFound' : IDL.Null, }); + const CheckCaptchaArg = IDL.Record({ 'solution' : IDL.Text }); + const RegistrationFlowNextStep = IDL.Variant({ + 'CheckCaptcha' : IDL.Record({ 'captcha_png_base64' : IDL.Text }), + 'Finish' : IDL.Null, + }); + const IdRegNextStepResult = IDL.Record({ + 'next_step' : RegistrationFlowNextStep, + }); + const CheckCaptchaError = IDL.Variant({ + 'NoRegistrationFlow' : IDL.Null, + 'UnexpectedCall' : IDL.Record({ 'next_step' : RegistrationFlowNextStep }), + 'WrongSolution' : IDL.Record({ 'new_captcha_png_base64' : IDL.Text }), + }); const ChallengeKey = IDL.Text; const Challenge = IDL.Record({ 'png_base64' : IDL.Text, @@ -283,15 +296,19 @@ export const idlFactory = ({ IDL }) => { 'space_available' : IDL.Nat64, }), }); - const ChallengeResult = IDL.Record({ - 'key' : ChallengeKey, - 'chars' : IDL.Text, + const IdRegFinishArg = IDL.Record({ 'authn_method' : AuthnMethodData }); + const IdRegFinishResult = IDL.Record({ 'identity_number' : IDL.Nat64 }); + const IdRegFinishError = IDL.Variant({ + 'NoRegistrationFlow' : IDL.Null, + 'UnexpectedCall' : IDL.Record({ 'next_step' : RegistrationFlowNextStep }), + 'InvalidAuthnMethod' : IDL.Text, + 'IdentityLimitReached' : IDL.Null, + 'StorageError' : IDL.Text, }); - const CaptchaResult = ChallengeResult; - const IdentityRegisterError = IDL.Variant({ - 'BadCaptcha' : IDL.Null, - 'CanisterFull' : IDL.Null, - 'InvalidMetadata' : IDL.Text, + const IdRegStartError = IDL.Variant({ + 'InvalidCaller' : IDL.Null, + 'AlreadyInProgress' : IDL.Null, + 'RateLimitExceeded' : IDL.Null, }); const UserKey = PublicKey; const PrepareIdAliasRequest = IDL.Record({ @@ -308,6 +325,10 @@ export const idlFactory = ({ IDL }) => { 'InternalCanisterError' : IDL.Text, 'Unauthorized' : IDL.Principal, }); + const ChallengeResult = IDL.Record({ + 'key' : ChallengeKey, + 'chars' : IDL.Text, + }); const RegisterResponse = IDL.Variant({ 'bad_challenge' : IDL.Null, 'canister_full' : IDL.Null, @@ -411,9 +432,14 @@ export const idlFactory = ({ IDL }) => { ], [], ), - 'captcha_create' : IDL.Func( - [], - [IDL.Variant({ 'Ok' : Challenge, 'Err' : IDL.Null })], + 'check_captcha' : IDL.Func( + [CheckCaptchaArg], + [ + IDL.Variant({ + 'Ok' : IdRegNextStepResult, + 'Err' : CheckCaptchaError, + }), + ], [], ), 'config' : IDL.Func([], [InternetIdentityInit], ['query']), @@ -465,9 +491,14 @@ export const idlFactory = ({ IDL }) => { ], [], ), - 'identity_register' : IDL.Func( - [AuthnMethodData, CaptchaResult, IDL.Opt(IDL.Principal)], - [IDL.Variant({ 'Ok' : IdentityNumber, 'Err' : IdentityRegisterError })], + 'identity_registration_finish' : IDL.Func( + [IdRegFinishArg], + [IDL.Variant({ 'Ok' : IdRegFinishResult, 'Err' : IdRegFinishError })], + [], + ), + 'identity_registration_start' : IDL.Func( + [], + [IDL.Variant({ 'Ok' : IdRegNextStepResult, 'Err' : IdRegStartError })], [], ), 'init_salt' : IDL.Func([], [], []), diff --git a/src/frontend/generated/internet_identity_types.d.ts b/src/frontend/generated/internet_identity_types.d.ts index 1dc8ea53d6..10fcdb7a89 100644 --- a/src/frontend/generated/internet_identity_types.d.ts +++ b/src/frontend/generated/internet_identity_types.d.ts @@ -89,6 +89,10 @@ export interface Challenge { } export type ChallengeKey = string; export interface ChallengeResult { 'key' : ChallengeKey, 'chars' : string } +export interface CheckCaptchaArg { 'solution' : string } +export type CheckCaptchaError = { 'NoRegistrationFlow' : null } | + { 'UnexpectedCall' : { 'next_step' : RegistrationFlowNextStep } } | + { 'WrongSolution' : { 'new_captcha_png_base64' : string } }; export type CredentialId = Uint8Array | number[]; export interface Delegation { 'pubkey' : PublicKey, @@ -158,6 +162,17 @@ export interface IdAliasCredentials { 'rp_id_alias_credential' : SignedIdAlias, 'issuer_id_alias_credential' : SignedIdAlias, } +export interface IdRegFinishArg { 'authn_method' : AuthnMethodData } +export type IdRegFinishError = { 'NoRegistrationFlow' : null } | + { 'UnexpectedCall' : { 'next_step' : RegistrationFlowNextStep } } | + { 'InvalidAuthnMethod' : string } | + { 'IdentityLimitReached' : null } | + { 'StorageError' : string }; +export interface IdRegFinishResult { 'identity_number' : bigint } +export interface IdRegNextStepResult { 'next_step' : RegistrationFlowNextStep } +export type IdRegStartError = { 'InvalidCaller' : null } | + { 'AlreadyInProgress' : null } | + { 'RateLimitExceeded' : null }; export interface IdentityAnchorInfo { 'devices' : Array, 'device_registration' : [] | [DeviceRegistrationInfo], @@ -184,9 +199,6 @@ export type IdentityMetadataReplaceError = { } }; export type IdentityNumber = bigint; -export type IdentityRegisterError = { 'BadCaptcha' : null } | - { 'CanisterFull' : null } | - { 'InvalidMetadata' : string }; export interface InternetIdentityInit { 'assigned_user_number_range' : [] | [[bigint, bigint]], 'archive_config' : [] | [ArchiveConfig], @@ -246,6 +258,10 @@ export interface RateLimitConfig { export type RegisterResponse = { 'bad_challenge' : null } | { 'canister_full' : null } | { 'registered' : { 'user_number' : UserNumber } }; +export type RegistrationFlowNextStep = { + 'CheckCaptcha' : { 'captcha_png_base64' : string } + } | + { 'Finish' : null }; export type SessionKey = PublicKey; export interface SignedDelegation { 'signature' : Uint8Array | number[], @@ -333,7 +349,11 @@ export interface _SERVICE { { 'Ok' : null } | { 'Err' : AuthnMethodSecuritySettingsReplaceError } >, - 'captcha_create' : ActorMethod<[], { 'Ok' : Challenge } | { 'Err' : null }>, + 'check_captcha' : ActorMethod< + [CheckCaptchaArg], + { 'Ok' : IdRegNextStepResult } | + { 'Err' : CheckCaptchaError } + >, 'config' : ActorMethod<[], InternetIdentityInit>, 'create_challenge' : ActorMethod<[], Challenge>, 'deploy_archive' : ActorMethod<[Uint8Array | number[]], DeployArchiveResult>, @@ -369,10 +389,15 @@ export interface _SERVICE { { 'Ok' : null } | { 'Err' : IdentityMetadataReplaceError } >, - 'identity_register' : ActorMethod< - [AuthnMethodData, CaptchaResult, [] | [Principal]], - { 'Ok' : IdentityNumber } | - { 'Err' : IdentityRegisterError } + 'identity_registration_finish' : ActorMethod< + [IdRegFinishArg], + { 'Ok' : IdRegFinishResult } | + { 'Err' : IdRegFinishError } + >, + 'identity_registration_start' : ActorMethod< + [], + { 'Ok' : IdRegNextStepResult } | + { 'Err' : IdRegStartError } >, 'init_salt' : ActorMethod<[], undefined>, 'lookup' : ActorMethod<[UserNumber], Array>, From ca17ae5f13cd18cef678b2b3da3d7a6f9855f3e1 Mon Sep 17 00:00:00 2001 From: Frederik Rothenberger Date: Fri, 27 Sep 2024 10:51:09 +0200 Subject: [PATCH 3/3] Small review fixes --- .../registration/registration_flow_v2.rs | 8 ++++---- src/internet_identity/src/state/flow_states.rs | 12 ++++++------ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/internet_identity/src/anchor_management/registration/registration_flow_v2.rs b/src/internet_identity/src/anchor_management/registration/registration_flow_v2.rs index fa24d4cb5a..37c39c167b 100644 --- a/src/internet_identity/src/anchor_management/registration/registration_flow_v2.rs +++ b/src/internet_identity/src/anchor_management/registration/registration_flow_v2.rs @@ -64,7 +64,8 @@ async fn captcha_flow_state(flow_created_timestamp_ns: u64) -> (Base64, Registra } pub async fn check_captcha(arg: CheckCaptchaArg) -> Result { - let Some(current_state) = state::with_flow_states(|s| s.registration_flow_state(&caller())) + let caller = caller(); + let Some(current_state) = state::with_flow_states(|s| s.registration_flow_state(&caller)) else { return Err(CheckCaptchaError::NoRegistrationFlow); }; @@ -80,7 +81,6 @@ pub async fn check_captcha(arg: CheckCaptchaArg) -> Result Result Result { - let Some(current_state) = state::with_flow_states(|s| s.registration_flow_state(&caller())) + let caller = caller(); + let Some(current_state) = state::with_flow_states(|s| s.registration_flow_state(&caller)) else { return Err(IdRegFinishError::NoRegistrationFlow); }; @@ -127,7 +128,6 @@ pub fn identity_registration_finish( let identity_number = create_identity(&arg)?; - let caller = caller(); // flow completed --> remove flow state state::with_flow_states_mut(|flow_states| flow_states.remove_registration_flow(&caller)); diff --git a/src/internet_identity/src/state/flow_states.rs b/src/internet_identity/src/state/flow_states.rs index ab2f47e0f2..36cacdabc9 100644 --- a/src/internet_identity/src/state/flow_states.rs +++ b/src/internet_identity/src/state/flow_states.rs @@ -42,7 +42,7 @@ pub enum RegistrationFlowState { } impl RegistrationFlowState { - pub fn create_at_timestamp_ns(&self) -> u64 { + pub fn created_at_timestamp_ns(&self) -> u64 { match self { RegistrationFlowState::CheckCaptcha { flow_created_timestamp_ns, @@ -68,7 +68,7 @@ impl FlowStates { self.expirations.push_back(FlowExpiration { principal, - expiration: flow_state.create_at_timestamp_ns() + FLOW_EXPIRATION_NS, + expiration: flow_state.created_at_timestamp_ns() + FLOW_EXPIRATION_NS, }); self.registration_flows.insert(principal, flow_state); @@ -97,7 +97,7 @@ impl FlowStates { self.registration_flows .get(principal) // filter out expired flow - .filter(|flow_state| flow_state.create_at_timestamp_ns() + FLOW_EXPIRATION_NS > time()) + .filter(|flow_state| flow_state.created_at_timestamp_ns() + FLOW_EXPIRATION_NS > time()) .cloned() } @@ -106,13 +106,13 @@ impl FlowStates { let now = time(); for _ in 0..MAX_TO_PRUNE { + // The expirations are sorted by expiration time because the expiration is constant + // and time() is monotonic + // -> we can stop pruning once we reach a flow that is not expired let Some(expiration) = self.expirations.front() else { break; }; - // The expirations are sorted by expiration time because the expiration is constant - // and time() is monotonic - // -> we can stop pruning once we reach a flow that is not expired if expiration.expiration > now { break; }