API V2 Refactor

This refactors the existing API v2 to be consistent with new patterns
introduced in the issuer API:
* Use camel case variants
* Use `Result` return values

All return values are still wrapped in `opt` in order to give us
the freedom to do breaking changes without CI complaining about it.
This `opt` will be removed once the API v2 stabilizes.

The integration tests are already written in a way that assumes the
`opt` is not there. This is possible because in candid `opt T` coerces
to `T`. This means that no integration test needs to be touched once
we remove the `opt` to stabilize.

src/canister_tests/src/api/internet_identity/api_v2.rs

@@ -7,7 +7,7 @@ use std::collections::HashMap;
 pub fn captcha_create(
     env: &StateMachine,
     canister_id: CanisterId,
-) -> Result<Option<CaptchaCreateResponse>, CallError> {
+) -> Result<Result<Challenge, ()>, CallError> {
     call_candid(env, canister_id, "captcha_create", ()).map(|(x,)| x)
 }
 
@@ -18,7 +18,7 @@ pub fn identity_register(
     authn_method: &AuthnMethodData,
     challenge_attempt: &ChallengeAttempt,
     temp_key: Option<Principal>,
-) -> Result<Option<IdentityRegisterResponse>, CallError> {
+) -> Result<Result<IdentityNumber, IdentityRegisterError>, CallError> {
     call_candid_as(
         env,
         canister_id,
@@ -34,7 +34,7 @@ pub fn identity_info(
     canister_id: CanisterId,
     sender: Principal,
     identity_number: IdentityNumber,
-) -> Result<Option<IdentityInfoResponse>, CallError> {
+) -> Result<Result<IdentityInfo, ()>, CallError> {
     call_candid_as(
         env,
         canister_id,
@@ -51,7 +51,7 @@ pub fn identity_metadata_replace(
     sender: Principal,
     identity_number: IdentityNumber,
     metadata: &HashMap<String, MetadataEntry>,
-) -> Result<Option<IdentityMetadataReplaceResponse>, CallError> {
+) -> Result<Result<(), ()>, CallError> {
     call_candid_as(
         env,
         canister_id,
@@ -68,7 +68,7 @@ pub fn authn_method_add(
     sender: Principal,
     identity_number: IdentityNumber,
     authn_method: &AuthnMethodData,
-) -> Result<Option<AuthnMethodAddResponse>, CallError> {
+) -> Result<Result<(), AuthnMethodAddError>, CallError> {
     call_candid_as(
         env,
         canister_id,
@@ -85,7 +85,7 @@ pub fn authn_method_remove(
     sender: Principal,
     identity_number: IdentityNumber,
     public_key: &PublicKey,
-) -> Result<Option<AuthnMethodRemoveResponse>, CallError> {
+) -> Result<Result<(), ()>, CallError> {
     call_candid_as(
         env,
         canister_id,

src/canister_tests/src/framework.rs

@@ -684,27 +684,3 @@ pub fn test_principal(n: u64) -> Principal {
     bytes.push(0xfe); // internal marker for user test ids
     Principal::from_slice(&bytes[..])
 }
-
-/// Macro to easily match a value against a pattern, and panic if the match fails.
-///
-/// This makes API v2 return types easier to handle.
-/// API v2 calls all return variants, requiring a match on the result.
-/// This macro allows to write the match in terms of the expected variant, with a fallback
-/// on unexpected variants.
-/// Example:
-/// ```
-/// use canister_tests::match_value;
-/// match_value!(
-///     api_v2::identity_info(&env, canister_id, principal, identity_number)?, // value
-///     Some(IdentityInfoResponse::Ok(identity_info)) // expected pattern, with binding to identity_info
-/// );
-/// ```
-#[macro_export]
-#[rustfmt::skip] // cargo fmt seems to have a bug with this macro (it indents the panic! way too far)
-macro_rules! match_value {
-    ($target: expr, $pat: pat_param) => {
-        let $pat = $target else {
-            panic!("expected {}", stringify!($pat));
-        };
-    };
-}

src/internet_identity/internet_identity.did

@@ -313,16 +313,16 @@ type PublicKeyAuthn = record {
 
 // The authentication methods currently supported by II.
 type AuthnMethod = variant {
-    webauthn: WebAuthn;
-    pubkey: PublicKeyAuthn;
+    WebAuthn: WebAuthn;
+    Puubkey: PublicKeyAuthn;
 };
 
 // This describes whether an authentication method is "protected" or not.
 // When protected, a authentication method can only be updated or removed if the
 // user is authenticated with that very authentication method.
 type AuthnMethodProtection = variant {
-    protected;
-    unprotected;
+    Protected;
+    Unprotected;
 };
 
 type AuthnMethodData = record {
@@ -358,36 +358,17 @@ type IdentityInfo = record {
     metadata: MetadataMap;
 };
 
-type CaptchaCreateResponse = variant {
-    ok: Challenge;
-};
-
-type IdentityRegisterResponse = variant {
-    // Registration successful.
-    ok: IdentityNumber;
+type IdentityRegisterError = variant {
     // No more registrations are possible in this instance of the II service canister.
-    canister_full;
+    CanisterFull;
     // The captcha check was not successful.
-    bad_captcha;
+    BadCaptcha;
     // The metadata of the provided authentication method contains invalid entries.
-    invalid_metadata: text;
-};
-
-type IdentityInfoResponse = variant {
-    ok: IdentityInfo;
-};
-
-type AuthnMethodAddResponse = variant {
-    ok;
-    invalid_metadata: text;
-};
-
-type AuthnMethodRemoveResponse = variant {
-    ok;
+    InvalidMetadata: text;
 };
 
-type IdentityMetadataReplaceResponse = variant {
-    ok;
+type AuthnMethodAddError = variant {
+    InvalidMetadata: text;
 };
 
 type PrepareIdAliasRequest = record {
@@ -482,36 +463,35 @@ service : (opt InternetIdentityInit) -> {
     // WARNING: The following methods are experimental and may change in the future.
     //
     // Note: the responses of v2 API calls are `opt` for compatibility reasons
-    // with future variant extensions.
-    // A client decoding a response as `null` indicates outdated type information
-    // and should be treated as an error.
+    // while the API is still in development (there will breaking changes).
+    // When the API is stable, the `opt` will be removed.
 
 
     // Creates a new captcha. The solution needs to be submitted using the
     // `identity_register` call.
-    captcha_create: () -> (opt CaptchaCreateResponse);
+    captcha_create: () -> (opt variant {Ok: Challenge; Err;});
 
     // 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) -> (opt IdentityRegisterResponse);
+    identity_register: (AuthnMethodData, CaptchaResult, opt principal) -> (opt variant {Ok: IdentityNumber; Err: IdentityRegisterError;});
 
     // Returns information about the identity with the given number.
     // Requires authentication.
-    identity_info: (IdentityNumber) -> (opt IdentityInfoResponse);
+    identity_info: (IdentityNumber) -> (opt variant {Ok: IdentityInfo; Err;});
 
     // Replaces the authentication method independent metadata map.
     // The existing metadata map will be overwritten.
     // Requires authentication.
-    identity_metadata_replace: (IdentityNumber, MetadataMap) -> (opt IdentityMetadataReplaceResponse);
+    identity_metadata_replace: (IdentityNumber, MetadataMap) -> (opt variant {Ok; Err;});
 
     // Adds a new authentication method to the identity.
     // Requires authentication.
-    authn_method_add: (IdentityNumber, AuthnMethodData) -> (opt AuthnMethodAddResponse);
+    authn_method_add: (IdentityNumber, AuthnMethodData) -> (opt variant {Ok; Err: AuthnMethodAddError;});
 
     // Removes the authentication method associated with the public key from the identity.
     // Requires authentication.
-    authn_method_remove: (IdentityNumber, PublicKey) -> (opt AuthnMethodRemoveResponse);
+    authn_method_remove: (IdentityNumber, PublicKey) -> (opt variant {Ok; Err;});
 
     // Attribute Sharing MVP API
     // The methods below are used to generate ID-alias credentials during attribute sharing flow.

src/internet_identity/src/main.rs

@@ -511,8 +511,7 @@ fn check_authentication(anchor_number: AnchorNumber) -> Result<(Anchor, DeviceKe
 /// New v2 API that aims to eventually replace the current API.
 /// The v2 API:
 /// * uses terminology more aligned with the front-end and is more consistent in its naming.
-/// * uses opt variant return types consistently in order to by able to extend / change return types
-///   in the future without breaking changes.
+/// * uses [Result] return types consistently.
 ///
 /// TODO, API v2 rollout plan:
 /// 1. Develop API v2 far enough so that front-ends can switch to it.
@@ -525,9 +524,9 @@ mod v2_api {
 
     #[update]
     #[candid_method]
-    async fn captcha_create() -> Option<CaptchaCreateResponse> {
+    async fn captcha_create() -> Option<Result<Challenge, ()>> {
         let challenge = anchor_management::registration::create_challenge().await;
-        Some(CaptchaCreateResponse::Ok(challenge))
+        Some(Ok(challenge))
     }
 
     #[update]
@@ -536,23 +535,25 @@ mod v2_api {
         authn_method: AuthnMethodData,
         challenge_result: ChallengeAttempt,
         temp_key: Option<Principal>,
-    ) -> Option<IdentityRegisterResponse> {
-        let result = match DeviceWithUsage::try_from(authn_method)
-            .map_err(|err| IdentityRegisterResponse::InvalidMetadata(err.to_string()))
+    ) -> Option<Result<IdentityNumber, IdentityRegisterError>> {
+        let device = match DeviceWithUsage::try_from(authn_method)
+            .map_err(|err| IdentityRegisterError::InvalidMetadata(err.to_string()))
         {
-            Ok(device) => IdentityRegisterResponse::from(register(
-                DeviceData::from(device),
-                challenge_result,
-                temp_key,
-            )),
-            Err(err) => err,
+            Ok(device) => device,
+            Err(err) => return Some(Err(err)),
+        };
+
+        let result = 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),
         };
         Some(result)
     }
 
     #[update]
     #[candid_method]
-    fn identity_info(identity_number: IdentityNumber) -> Option<IdentityInfoResponse> {
+    fn identity_info(identity_number: IdentityNumber) -> Option<Result<IdentityInfo, ()>> {
         authenticate_and_record_activity(identity_number);
         let anchor_info = anchor_management::get_anchor_info(identity_number);
         let metadata = state::anchor(identity_number)
@@ -571,24 +572,21 @@ mod v2_api {
                 .map(AuthnMethodRegistration::from),
             metadata,
         };
-        Some(IdentityInfoResponse::Ok(identity_info))
+        Some(Ok(identity_info))
     }
 
     #[update]
     #[candid_method]
     fn authn_method_add(
         identity_number: IdentityNumber,
         authn_method: AuthnMethodData,
-    ) -> Option<AuthnMethodAddResponse> {
-        let result = match DeviceWithUsage::try_from(authn_method)
-            .map_err(|err| AuthnMethodAddResponse::InvalidMetadata(err.to_string()))
-        {
-            Ok(device) => {
+    ) -> Option<Result<(), AuthnMethodAddError>> {
+        let result = DeviceWithUsage::try_from(authn_method)
+            .map(|device| {
                 add(identity_number, DeviceData::from(device));
-                AuthnMethodAddResponse::Ok
-            }
-            Err(err) => err,
-        };
+                ()
+            })
+            .map_err(|err| AuthnMethodAddError::InvalidMetadata(err.to_string()));
         Some(result)
     }
 
@@ -597,24 +595,24 @@ mod v2_api {
     fn authn_method_remove(
         identity_number: IdentityNumber,
         public_key: PublicKey,
-    ) -> Option<AuthnMethodRemoveResponse> {
+    ) -> Option<Result<(), ()>> {
         remove(identity_number, public_key);
-        Some(AuthnMethodRemoveResponse::Ok)
+        Some(Ok(()))
     }
 
     #[update]
     #[candid_method]
     fn identity_metadata_replace(
         identity_number: IdentityNumber,
         metadata: HashMap<String, MetadataEntry>,
-    ) -> Option<IdentityMetadataReplaceResponse> {
+    ) -> Option<Result<(), ()>> {
         let result = authenticated_anchor_operation(identity_number, |anchor| {
             Ok((
-                IdentityMetadataReplaceResponse::Ok,
+                (),
                 anchor_management::identity_metadata_replace(anchor, metadata),
             ))
         });
-        Some(result)
+        Some(Ok(result))
     }
 }
 

src/internet_identity/tests/integration/activity_stats/authn_methods.rs

@@ -47,12 +47,12 @@ fn should_report_daily_active_authn_methods() -> Result<(), CallError> {
 
         // repeated activity with the same authn_method on the same identity within the 24h
         // collection period should not increase the counter
-        api_v2::identity_info(&env, canister_id, authn_method.principal(), identity_nr)?;
+        api_v2::identity_info(&env, canister_id, authn_method.principal(), identity_nr)?.unwrap();
 
         env.advance_time(Duration::from_secs(DAY_SECONDS));
 
         // some activity is required to update the stats
-        api_v2::identity_info(&env, canister_id, authn_method.principal(), identity_nr)?;
+        api_v2::identity_info(&env, canister_id, authn_method.principal(), identity_nr)?.unwrap();
 
         let metrics = get_metrics(&env, canister_id);
         assert_metric(
@@ -103,12 +103,12 @@ fn should_report_monthly_active_authn_methods() -> Result<(), CallError> {
 
         // repeated activity with the same authn_method on the same identity within the 30-day
         // collection period should not increase the counter
-        api_v2::identity_info(&env, canister_id, authn_method.principal(), identity_nr)?;
+        api_v2::identity_info(&env, canister_id, authn_method.principal(), identity_nr)?.unwrap();
 
         env.advance_time(Duration::from_secs(MONTH_SECONDS));
 
         // some activity is required to update the stats
-        api_v2::identity_info(&env, canister_id, authn_method.principal(), identity_nr)?;
+        api_v2::identity_info(&env, canister_id, authn_method.principal(), identity_nr)?.unwrap();
 
         let metrics = get_metrics(&env, canister_id);
         assert_metric(
@@ -180,7 +180,8 @@ fn should_only_count_ii_domain_authn_methods() -> Result<(), CallError> {
         canister_id,
         non_ii_authn_method.principal(),
         identity_nr,
-    )?;
+    )?
+    .unwrap();
     env.advance_time(Duration::from_secs(MONTH_SECONDS));
     // some activity on an II domain is required to update the stats
     create_identity_with_authn_method(&env, canister_id, &ii_authn_method);
@@ -223,7 +224,7 @@ fn should_keep_stats_across_upgrades() -> Result<(), CallError> {
     env.advance_time(Duration::from_secs(DAY_SECONDS));
 
     // some activity is required to update the stats
-    api_v2::identity_info(&env, canister_id, authn_method.principal(), identity_nr)?;
+    api_v2::identity_info(&env, canister_id, authn_method.principal(), identity_nr)?.unwrap();
 
     assert_metric(
         &get_metrics(&env, canister_id),

src/internet_identity/tests/integration/archive_integration.rs

@@ -485,7 +485,8 @@ mod pull_entries_tests {
             device.principal(),
             anchor,
             &metadata,
-        )?;
+        )?
+        .unwrap();
 
         // the archive polls for entries once per second
         env.advance_time(Duration::from_secs(2));