Refactor II VC API (#2142)

This PR refactors the II VC API: * get rid of `opt` * restructure errors to be aligned with Rusts `Result` type * CamelCase error variants
dfinity · Dec 15, 2023 · ed19d81 · ed19d81
1 parent 2958671
commit ed19d81
Show file tree

Hide file tree

Showing 10 changed files with 234 additions and 395 deletions.
diff --git a/demos/vc_issuer/tests/issue_credential.rs b/demos/vc_issuer/tests/issue_credential.rs
@@ -16,7 +16,7 @@ use identity_core::common::Value;
 use identity_jose::jwt::JwtClaims;
 use internet_identity_interface::http_gateway::{HttpRequest, HttpResponse};
 use internet_identity_interface::internet_identity::types::vc_mvp::{
-    GetIdAliasRequest, GetIdAliasResponse, PrepareIdAliasRequest, PrepareIdAliasResponse,
+    GetIdAliasRequest, PrepareIdAliasRequest,
 };
 use internet_identity_interface::internet_identity::types::FrontendHostname;
 use lazy_static::lazy_static;
@@ -552,15 +552,10 @@ fn should_issue_credential_e2e() -> Result<(), CallError> {
         issuer: issuer.clone(),
     };
 
-    let prepare_response =
+    let prepared_id_alias =
         ii_api::prepare_id_alias(&env, ii_id, principal_1(), prepare_id_alias_req)?
-            .expect("Got 'None' from prepare_id_alias");
+            .expect("prepare id_alias failed");
 
-    let prepared_id_alias = if let PrepareIdAliasResponse::Ok(response) = prepare_response {
-        response
-    } else {
-        panic!("prepare id_alias failed")
-    };
     let canister_sig_pk =
         CanisterSigPublicKey::try_from(prepared_id_alias.canister_sig_pk_der.as_ref())
             .expect("failed parsing canister sig pk");
@@ -572,18 +567,8 @@ fn should_issue_credential_e2e() -> Result<(), CallError> {
         rp_id_alias_jwt: prepared_id_alias.rp_id_alias_jwt,
         issuer_id_alias_jwt: prepared_id_alias.issuer_id_alias_jwt,
     };
-    let id_alias_credentials =
-        match ii_api::get_id_alias(&env, ii_id, principal_1(), get_id_alias_req)?
-            .expect("Got 'None' from get_id_alias")
-        {
-            GetIdAliasResponse::Ok(credentials) => credentials,
-            GetIdAliasResponse::NoSuchCredentials(err) => {
-                panic!("{}", format!("failed to get id_alias credentials: {}", err))
-            }
-            GetIdAliasResponse::AuthenticationFailed(err) => {
-                panic!("{}", format!("failed authentication: {}", err))
-            }
-        };
+    let id_alias_credentials = ii_api::get_id_alias(&env, ii_id, principal_1(), get_id_alias_req)?
+        .expect("get id_alias failed");
 
     let root_pk_raw =
         extract_raw_root_pk_from_der(&env.root_key()).expect("Failed decoding IC root key.");

diff --git a/src/canister_tests/src/api/internet_identity/vc_mvp.rs b/src/canister_tests/src/api/internet_identity/vc_mvp.rs
@@ -2,15 +2,16 @@ use candid::Principal;
 use ic_cdk::api::management_canister::main::CanisterId;
 use ic_test_state_machine_client::{call_candid_as, query_candid_as, CallError, StateMachine};
 use internet_identity_interface::internet_identity::types::vc_mvp::{
-    GetIdAliasRequest, GetIdAliasResponse, PrepareIdAliasRequest, PrepareIdAliasResponse,
+    GetIdAliasError, GetIdAliasRequest, IdAliasCredentials, PrepareIdAliasError,
+    PrepareIdAliasRequest, PreparedIdAlias,
 };
 
 pub fn prepare_id_alias(
     env: &StateMachine,
     canister_id: CanisterId,
     sender: Principal,
     prepare_id_alias_req: PrepareIdAliasRequest,
-) -> Result<Option<PrepareIdAliasResponse>, CallError> {
+) -> Result<Result<PreparedIdAlias, PrepareIdAliasError>, CallError> {
     call_candid_as(
         env,
         canister_id,
@@ -26,7 +27,7 @@ pub fn get_id_alias(
     canister_id: CanisterId,
     sender: Principal,
     get_id_alias_req: GetIdAliasRequest,
-) -> Result<Option<GetIdAliasResponse>, CallError> {
+) -> Result<Result<IdAliasCredentials, GetIdAliasError>, CallError> {
     query_candid_as(
         env,
         canister_id,

diff --git a/src/frontend/generated/internet_identity_idl.js b/src/frontend/generated/internet_identity_idl.js
@@ -170,10 +170,9 @@ export const idlFactory = ({ IDL }) => {
     'rp_id_alias_credential' : SignedIdAlias,
     'issuer_id_alias_credential' : SignedIdAlias,
   });
-  const GetIdAliasResponse = IDL.Variant({
-    'ok' : IdAliasCredentials,
-    'authentication_failed' : IDL.Text,
-    'no_such_credentials' : IDL.Text,
+  const GetIdAliasError = IDL.Variant({
+    'NoSuchCredentials' : IDL.Text,
+    'AuthenticationFailed' : IDL.Text,
   });
   const HeaderField = IDL.Tuple(IDL.Text, IDL.Text);
   const HttpRequest = IDL.Record({
@@ -238,9 +237,8 @@ export const idlFactory = ({ IDL }) => {
     'issuer_id_alias_jwt' : IDL.Text,
     'canister_sig_pk_der' : PublicKey,
   });
-  const PrepareIdAliasResponse = IDL.Variant({
-    'ok' : PreparedIdAlias,
-    'authentication_failed' : IDL.Text,
+  const PrepareIdAliasError = IDL.Variant({
+    'AuthenticationFailed' : IDL.Text,
   });
   const RegisterResponse = IDL.Variant({
     'bad_challenge' : IDL.Null,
@@ -303,7 +301,7 @@ export const idlFactory = ({ IDL }) => {
       ),
     'get_id_alias' : IDL.Func(
         [GetIdAliasRequest],
-        [IDL.Opt(GetIdAliasResponse)],
+        [IDL.Variant({ 'Ok' : IdAliasCredentials, 'Err' : GetIdAliasError })],
         ['query'],
       ),
     'get_principal' : IDL.Func(
@@ -337,7 +335,7 @@ export const idlFactory = ({ IDL }) => {
       ),
     'prepare_id_alias' : IDL.Func(
         [PrepareIdAliasRequest],
-        [IDL.Opt(PrepareIdAliasResponse)],
+        [IDL.Variant({ 'Ok' : PreparedIdAlias, 'Err' : PrepareIdAliasError })],
         [],
       ),
     'register' : IDL.Func(

diff --git a/src/frontend/generated/internet_identity_types.d.ts b/src/frontend/generated/internet_identity_types.d.ts
@@ -99,16 +99,15 @@ export interface DeviceWithUsage {
 export type FrontendHostname = string;
 export type GetDelegationResponse = { 'no_such_delegation' : null } |
   { 'signed_delegation' : SignedDelegation };
+export type GetIdAliasError = { 'NoSuchCredentials' : string } |
+  { 'AuthenticationFailed' : string };
 export interface GetIdAliasRequest {
   'rp_id_alias_jwt' : string,
   'issuer' : FrontendHostname,
   'issuer_id_alias_jwt' : string,
   'relying_party' : FrontendHostname,
   'identity_number' : IdentityNumber,
 }
-export type GetIdAliasResponse = { 'ok' : IdAliasCredentials } |
-  { 'authentication_failed' : string } |
-  { 'no_such_credentials' : string };
 export type HeaderField = [string, string];
 export interface HttpRequest {
   'url' : string,
@@ -174,13 +173,12 @@ export type MetadataMap = Array<
       { 'bytes' : Uint8Array | number[] },
   ]
 >;
+export type PrepareIdAliasError = { 'AuthenticationFailed' : string };
 export interface PrepareIdAliasRequest {
   'issuer' : FrontendHostname,
   'relying_party' : FrontendHostname,
   'identity_number' : IdentityNumber,
 }
-export type PrepareIdAliasResponse = { 'ok' : PreparedIdAlias } |
-  { 'authentication_failed' : string };
 export interface PreparedIdAlias {
   'rp_id_alias_jwt' : string,
   'issuer_id_alias_jwt' : string,
@@ -259,7 +257,11 @@ export interface _SERVICE {
     [UserNumber, FrontendHostname, SessionKey, Timestamp],
     GetDelegationResponse
   >,
-  'get_id_alias' : ActorMethod<[GetIdAliasRequest], [] | [GetIdAliasResponse]>,
+  'get_id_alias' : ActorMethod<
+    [GetIdAliasRequest],
+    { 'Ok' : IdAliasCredentials } |
+      { 'Err' : GetIdAliasError }
+  >,
   'get_principal' : ActorMethod<[UserNumber, FrontendHostname], Principal>,
   'http_request' : ActorMethod<[HttpRequest], HttpResponse>,
   'http_request_update' : ActorMethod<[HttpRequest], HttpResponse>,
@@ -280,7 +282,8 @@ export interface _SERVICE {
   >,
   'prepare_id_alias' : ActorMethod<
     [PrepareIdAliasRequest],
-    [] | [PrepareIdAliasResponse]
+    { 'Ok' : PreparedIdAlias } |
+      { 'Err' : PrepareIdAliasError }
   >,
   'register' : ActorMethod<
     [DeviceData, ChallengeResult, [] | [Principal]],

diff --git a/src/frontend/src/utils/iiConnection.ts b/src/frontend/src/utils/iiConnection.ts
@@ -598,7 +598,7 @@ export class AuthenticatedConnection extends Connection {
     const rpOrigin = remapToLegacyDomain(rpOrigin_);
     const actor = await this.getActor();
     const userNumber = this.userNumber;
-    const [result] = await actor.prepare_id_alias({
+    const result = await actor.prepare_id_alias({
       issuer: issuerOrigin,
       relying_party: rpOrigin,
       identity_number: userNumber,
@@ -609,14 +609,25 @@ export class AuthenticatedConnection extends Connection {
       return { error: "internal_error" };
     }
 
-    if ("authentication_failed" in result) {
+    if ("Ok" in result) {
+      return result.Ok;
+    }
+
+    if (!("Err" in result)) {
       console.error(
-        ["Authentication failed", result.authentication_failed].join(": ")
+        "Expected property 'Ok' or 'Err', got: ",
+        JSON.stringify(result)
       );
+      return { error: "internal_error" };
+    }
+
+    const err = result.Err;
+    if ("AuthenticationFailed" in err) {
       return { error: "authentication_failed" };
     }
 
-    return result.ok;
+    console.error("Unknown error", err);
+    return { error: "internal_error" };
   };
 
   getIdAlias = async ({
@@ -637,7 +648,7 @@ export class AuthenticatedConnection extends Connection {
     const actor = await this.getActor();
     const userNumber = this.userNumber;
 
-    const [result] = await actor.get_id_alias({
+    const result = await actor.get_id_alias({
       issuer: issuerOrigin,
       relying_party: rpOrigin,
       identity_number: userNumber,
@@ -649,16 +660,29 @@ export class AuthenticatedConnection extends Connection {
       return { error: "internal_error" };
     }
 
-    if ("no_such_credentials" in result) {
-      console.error(["No credentials", result.no_such_credentials].join(": "));
+    if ("Ok" in result) {
+      return result.Ok;
+    }
+
+    if (!("Err" in result)) {
+      console.error(
+        "Expected property 'Ok' or 'Err', got: ",
+        JSON.stringify(result)
+      );
       return { error: "internal_error" };
     }
 
-    if ("authentication_failed" in result) {
+    const err = result.Err;
+    if ("NoSuchCredentials" in err) {
+      console.error(["No credentials", err.NoSuchCredentials].join(": "));
+      return { error: "internal_error" };
+    }
+    if ("AuthenticationFailed" in err) {
       return { error: "authentication_failed" };
     }
 
-    return result.ok;
+    console.error("Unknown error", err);
+    return { error: "internal_error" };
   };
 }
 

diff --git a/src/internet_identity/internet_identity.did b/src/internet_identity/internet_identity.did
@@ -399,11 +399,9 @@ type PrepareIdAliasRequest = record {
     identity_number : IdentityNumber;
 };
 
-type PrepareIdAliasResponse = variant {
-    /// Credentials prepared successfully, can be retrieved via `get_id_alias`
-    ok : PreparedIdAlias;
+type PrepareIdAliasError = variant {
     /// Caller authentication failed.
-    authentication_failed : text;
+    AuthenticationFailed : text;
 };
 
 /// The prepared id alias contains two (still unsigned) credentials in JWT format,
@@ -425,13 +423,11 @@ type GetIdAliasRequest = record {
     identity_number : IdentityNumber;
 };
 
-type GetIdAliasResponse = variant {
-    /// The signed id alias credentials
-    ok : IdAliasCredentials;
+type GetIdAliasError = variant {
     /// Caller authentication failed.
-    authentication_failed : text;
+    AuthenticationFailed : text;
     /// The credential(s) are not available: may be expired or not prepared yet (call prepare_id_alias to prepare).
-    no_such_credentials : text;
+    NoSuchCredentials : text;
 };
 
 /// The signed id alias credentials for each involved party.
@@ -519,6 +515,6 @@ service : (opt InternetIdentityInit) -> {
 
     // Attribute Sharing MVP API
     // The methods below are used to generate ID-alias credentials during attribute sharing flow.
-    prepare_id_alias : (PrepareIdAliasRequest) -> (opt PrepareIdAliasResponse);
-    get_id_alias : (GetIdAliasRequest) -> (opt GetIdAliasResponse) query;
+    prepare_id_alias : (PrepareIdAliasRequest) -> (variant {Ok: PreparedIdAlias; Err: PrepareIdAliasError;});
+    get_id_alias : (GetIdAliasRequest) -> (variant {Ok: IdAliasCredentials; Err: GetIdAliasError;}) query;
 }
diff --git a/src/internet_identity/src/main.rs b/src/internet_identity/src/main.rs
@@ -11,7 +11,8 @@ use ic_cdk_macros::{init, post_upgrade, pre_upgrade, query, update};
 use internet_identity_interface::archive::types::{BufferedEntry, Operation};
 use internet_identity_interface::http_gateway::{HttpRequest, HttpResponse};
 use internet_identity_interface::internet_identity::types::vc_mvp::{
-    GetIdAliasRequest, GetIdAliasResponse, PrepareIdAliasRequest, PrepareIdAliasResponse,
+    GetIdAliasError, GetIdAliasRequest, IdAliasCredentials, PrepareIdAliasError,
+    PrepareIdAliasRequest, PreparedIdAlias,
 };
 use internet_identity_interface::internet_identity::types::*;
 use serde_bytes::ByteBuf;
@@ -623,7 +624,9 @@ mod attribute_sharing_mvp {
 
     #[update]
     #[candid_method]
-    async fn prepare_id_alias(req: PrepareIdAliasRequest) -> Option<PrepareIdAliasResponse> {
+    async fn prepare_id_alias(
+        req: PrepareIdAliasRequest,
+    ) -> Result<PreparedIdAlias, PrepareIdAliasError> {
         let _maybe_ii_domain = authenticate_and_record_activity(req.identity_number);
         let prepared_id_alias = vc_mvp::prepare_id_alias(
             req.identity_number,
@@ -633,28 +636,27 @@ mod attribute_sharing_mvp {
             },
         )
         .await;
-        Some(PrepareIdAliasResponse::Ok(prepared_id_alias))
+        Ok(prepared_id_alias)
     }
 
     #[query]
     #[candid_method(query)]
-    fn get_id_alias(req: GetIdAliasRequest) -> Option<GetIdAliasResponse> {
+    fn get_id_alias(req: GetIdAliasRequest) -> Result<IdAliasCredentials, GetIdAliasError> {
         let Ok(_) = check_authentication(req.identity_number) else {
-            return Some(GetIdAliasResponse::AuthenticationFailed(format!(
+            return Err(GetIdAliasError::AuthenticationFailed(format!(
                 "{} could not be authenticated.",
                 caller()
             )));
         };
-        let response = vc_mvp::get_id_alias(
+        vc_mvp::get_id_alias(
             req.identity_number,
             vc_mvp::InvolvedDapps {
                 relying_party: req.relying_party,
                 issuer: req.issuer,
             },
             &req.rp_id_alias_jwt,
             &req.issuer_id_alias_jwt,
-        );
-        Some(response)
+        )
     }
 }