Skip to content

Commit

Permalink
Provide proper error for identity_info method (#2232)
Browse files Browse the repository at this point in the history
* Provide proper error for identity_info method

This PR relays the error back to the client consistent with
the errors used in other new API calls.

* 🤖 npm run generate auto-update

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
Frederik Rothenberger and github-actions[bot] authored Jan 24, 2024
1 parent f741dcf commit 1b3519a
Show file tree
Hide file tree
Showing 9 changed files with 56 additions and 25 deletions.
2 changes: 1 addition & 1 deletion src/canister_tests/src/api/internet_identity/api_v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ pub fn identity_info(
canister_id: CanisterId,
sender: Principal,
identity_number: IdentityNumber,
) -> Result<Result<IdentityInfo, ()>, CallError> {
) -> Result<Result<IdentityInfo, IdentityInfoError>, CallError> {
call_candid_as(
env,
canister_id,
Expand Down
6 changes: 5 additions & 1 deletion src/frontend/generated/internet_identity_idl.js
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,10 @@ export const idlFactory = ({ IDL }) => {
'metadata' : MetadataMapV2,
'authn_method_registration' : IDL.Opt(AuthnMethodRegistrationInfo),
});
const IdentityInfoError = IDL.Variant({
'InternalCanisterError' : IDL.Text,
'Unauthorized' : IDL.Principal,
});
const IdentityMetadataReplaceError = IDL.Variant({
'InternalCanisterError' : IDL.Text,
'Unauthorized' : IDL.Principal,
Expand Down Expand Up @@ -433,7 +437,7 @@ export const idlFactory = ({ IDL }) => {
),
'identity_info' : IDL.Func(
[IdentityNumber],
[IDL.Variant({ 'Ok' : IdentityInfo, 'Err' : IDL.Null })],
[IDL.Variant({ 'Ok' : IdentityInfo, 'Err' : IdentityInfoError })],
[],
),
'identity_metadata_replace' : IDL.Func(
Expand Down
4 changes: 3 additions & 1 deletion src/frontend/generated/internet_identity_types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,8 @@ export interface IdentityInfo {
'metadata' : MetadataMapV2,
'authn_method_registration' : [] | [AuthnMethodRegistrationInfo],
}
export type IdentityInfoError = { 'InternalCanisterError' : string } |
{ 'Unauthorized' : Principal };
export type IdentityMetadataReplaceError = {
'InternalCanisterError' : string
} |
Expand Down Expand Up @@ -350,7 +352,7 @@ export interface _SERVICE {
'identity_info' : ActorMethod<
[IdentityNumber],
{ 'Ok' : IdentityInfo } |
{ 'Err' : null }
{ 'Err' : IdentityInfoError }
>,
'identity_metadata_replace' : ActorMethod<
[IdentityNumber, MetadataMapV2],
Expand Down
14 changes: 10 additions & 4 deletions src/internet_identity/breaking_change_exceptions.patch
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
diff --git a/src/internet_identity/internet_identity.did b/src/internet_identity/internet_identity.did
index 6ef07401..a9883f61 100644
index dd464712..266039e1 100644
--- a/src/internet_identity/internet_identity.did
+++ b/src/internet_identity/internet_identity.did
@@ -454,9 +454,7 @@ type PrepareIdAliasRequest = record {
@@ -463,9 +463,7 @@ type PrepareIdAliasRequest = record {

type PrepareIdAliasError = variant {
/// The principal is not authorized to call this method with the given arguments.
Expand All @@ -13,7 +13,7 @@ index 6ef07401..a9883f61 100644
};

/// The prepared id alias contains two (still unsigned) credentials in JWT format,
@@ -480,11 +478,9 @@ type GetIdAliasRequest = record {
@@ -489,11 +487,9 @@ type GetIdAliasRequest = record {

type GetIdAliasError = variant {
/// The principal is not authorized to call this method with the given arguments.
Expand All @@ -26,7 +26,13 @@ index 6ef07401..a9883f61 100644
};

/// The signed id alias credentials for each involved party.
@@ -558,7 +554,7 @@ service : (opt InternetIdentityInit) -> {
@@ -562,12 +558,12 @@ service : (opt InternetIdentityInit) -> {

// Returns information about the identity with the given number.
// Requires authentication.
- identity_info: (IdentityNumber) -> (variant {Ok: IdentityInfo; Err: IdentityInfoError;});
+ identity_info: (IdentityNumber) -> (variant {Ok: IdentityInfo; Err;});

// Replaces the authentication method independent metadata map.
// The existing metadata map will be overwritten.
// Requires authentication.
Expand Down
11 changes: 10 additions & 1 deletion src/internet_identity/internet_identity.did
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,15 @@ type IdentityInfo = record {
metadata: MetadataMapV2;
};

type IdentityInfoError = variant {
/// The principal is not authorized to call this method with the given arguments.
Unauthorized: principal;
/// Internal canister error. See the error message for details.
InternalCanisterError: text;
};



type IdentityRegisterError = variant {
// No more registrations are possible in this instance of the II service canister.
CanisterFull;
Expand Down Expand Up @@ -553,7 +562,7 @@ service : (opt InternetIdentityInit) -> {

// Returns information about the identity with the given number.
// Requires authentication.
identity_info: (IdentityNumber) -> (variant {Ok: IdentityInfo; Err;});
identity_info: (IdentityNumber) -> (variant {Ok: IdentityInfo; Err: IdentityInfoError;});

// Replaces the authentication method independent metadata map.
// The existing metadata map will be overwritten.
Expand Down
15 changes: 14 additions & 1 deletion src/internet_identity/src/conversions/api_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ use crate::storage::StorageError;
use internet_identity_interface::internet_identity::types::vc_mvp::{
GetIdAliasError, PrepareIdAliasError,
};
use internet_identity_interface::internet_identity::types::IdentityMetadataReplaceError;
use internet_identity_interface::internet_identity::types::{
IdentityInfoError, IdentityMetadataReplaceError,
};

impl From<IdentityUpdateError> for IdentityMetadataReplaceError {
fn from(value: IdentityUpdateError) -> Self {
Expand Down Expand Up @@ -53,6 +55,17 @@ impl From<IdentityUpdateError> for PrepareIdAliasError {
}
}

impl From<IdentityUpdateError> for IdentityInfoError {
fn from(value: IdentityUpdateError) -> Self {
match value {
IdentityUpdateError::Unauthorized(principal) => {
IdentityInfoError::Unauthorized(principal)
}
err => IdentityInfoError::InternalCanisterError(err.to_string()),
}
}
}

impl From<AuthorizationError> for GetIdAliasError {
fn from(value: AuthorizationError) -> Self {
match value {
Expand Down
5 changes: 2 additions & 3 deletions src/internet_identity/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -563,9 +563,8 @@ mod v2_api {

#[update]
#[candid_method]
fn identity_info(identity_number: IdentityNumber) -> Result<IdentityInfo, ()> {
authenticate_and_record_activity(identity_number)
.unwrap_or_else(|err| trap(&format!("{err}")));
fn identity_info(identity_number: IdentityNumber) -> Result<IdentityInfo, IdentityInfoError> {
authenticate_and_record_activity(identity_number).map_err(IdentityInfoError::from)?;
let anchor_info = anchor_management::get_anchor_info(identity_number);

let metadata = state::anchor(identity_number)
Expand Down
18 changes: 5 additions & 13 deletions src/internet_identity/tests/integration/v2_api/identity_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,12 @@ use crate::v2_api::authn_method_test_helpers::{
use candid::Principal;
use canister_tests::api::internet_identity as api;
use canister_tests::api::internet_identity::api_v2;
use canister_tests::framework::{
env, expect_user_error_with_message, install_ii_canister, time, II_WASM,
};
use canister_tests::framework::{env, install_ii_canister, time, II_WASM};
use ic_test_state_machine_client::CallError;
use ic_test_state_machine_client::ErrorCode::CanisterCalledTrap;
use internet_identity_interface::internet_identity::types::{
AuthnMethodData, AuthnMethodRegistration, DeviceData, DeviceWithUsage, MetadataEntry,
AuthnMethodData, AuthnMethodRegistration, DeviceData, DeviceWithUsage, IdentityInfoError,
MetadataEntry,
};
use regex::Regex;
use serde_bytes::ByteBuf;
use std::collections::HashMap;
use std::time::Duration;
Expand Down Expand Up @@ -55,13 +52,8 @@ fn should_require_authentication_for_identity_info() -> Result<(), CallError> {
let authn_methods = sample_authn_methods();
let identity_number = create_identity_with_authn_methods(&env, canister_id, &authn_methods);

let result = api_v2::identity_info(&env, canister_id, Principal::anonymous(), identity_number);

expect_user_error_with_message(
result,
CanisterCalledTrap,
Regex::new("[a-z\\d-]+ could not be authenticated.").unwrap(),
);
let result = api_v2::identity_info(&env, canister_id, Principal::anonymous(), identity_number)?;
assert!(matches!(result, Err(IdentityInfoError::Unauthorized(_))));
Ok(())
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,12 @@ pub struct IdentityInfo {
pub metadata: HashMap<String, MetadataEntryV2>,
}

#[derive(Clone, Debug, CandidType, Deserialize, Eq, PartialEq)]
pub enum IdentityInfoError {
Unauthorized(Principal),
InternalCanisterError(String),
}

#[derive(Clone, Debug, CandidType, Deserialize, Eq, PartialEq)]
pub enum IdentityRegisterError {
CanisterFull,
Expand Down

0 comments on commit 1b3519a

Please sign in to comment.