Add API v2 authn_method_remove method (#1809)

* Add API v2 authn_method_remove method

Add the authn_method_remove method to slowly build the
API v2.

* 🤖 npm run generate auto-update

* Add API v2 todos

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

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

@@ -2,8 +2,8 @@ use candid::Principal;
 use ic_cdk::api::management_canister::main::CanisterId;
 use ic_test_state_machine_client::{call_candid_as, CallError, StateMachine};
 use internet_identity_interface::internet_identity::types::{
-    AuthnMethodAddResponse, AuthnMethodData, IdentityInfoResponse, IdentityMetadataReplaceResponse,
-    IdentityNumber, MetadataEntry,
+    AuthnMethodAddResponse, AuthnMethodData, AuthnMethodRemoveResponse, IdentityInfoResponse,
+    IdentityMetadataReplaceResponse, IdentityNumber, MetadataEntry, PublicKey,
 };
 use std::collections::HashMap;
 
@@ -23,6 +23,23 @@ pub fn identity_info(
     .map(|(x,)| x)
 }
 
+pub fn identity_metadata_replace(
+    env: &StateMachine,
+    canister_id: CanisterId,
+    sender: Principal,
+    identity_number: IdentityNumber,
+    metadata: &HashMap<String, MetadataEntry>,
+) -> Result<Option<IdentityMetadataReplaceResponse>, CallError> {
+    call_candid_as(
+        env,
+        canister_id,
+        sender,
+        "identity_metadata_replace",
+        (identity_number, metadata),
+    )
+    .map(|(x,)| x)
+}
+
 pub fn authn_method_add(
     env: &StateMachine,
     canister_id: CanisterId,
@@ -40,19 +57,19 @@ pub fn authn_method_add(
     .map(|(x,)| x)
 }
 
-pub fn identity_metadata_replace(
+pub fn authn_method_remove(
     env: &StateMachine,
     canister_id: CanisterId,
     sender: Principal,
     identity_number: IdentityNumber,
-    metadata: &HashMap<String, MetadataEntry>,
-) -> Result<Option<IdentityMetadataReplaceResponse>, CallError> {
+    public_key: &PublicKey,
+) -> Result<Option<AuthnMethodRemoveResponse>, CallError> {
     call_candid_as(
         env,
         canister_id,
         sender,
-        "identity_metadata_replace",
-        (identity_number, metadata),
+        "authn_method_remove",
+        (identity_number, public_key),
     )
     .map(|(x,)| x)
 }

src/frontend/generated/internet_identity_idl.js

@@ -92,6 +92,7 @@ export const idlFactory = ({ IDL }) => {
     'ok' : IDL.Null,
     'invalid_metadata' : IDL.Text,
   });
+  const AuthnMethodRemoveResponse = IDL.Variant({ 'ok' : IDL.Null });
   const ChallengeKey = IDL.Text;
   const Challenge = IDL.Record({
     'png_base64' : IDL.Text,
@@ -234,6 +235,11 @@ export const idlFactory = ({ IDL }) => {
         [IDL.Opt(AuthnMethodAddResponse)],
         [],
       ),
+    'authn_method_remove' : IDL.Func(
+        [IdentityNumber, PublicKey],
+        [IDL.Opt(AuthnMethodRemoveResponse)],
+        [],
+      ),
     'create_challenge' : IDL.Func([], [Challenge], []),
     'deploy_archive' : IDL.Func([IDL.Vec(IDL.Nat8)], [DeployArchiveResult], []),
     'enter_device_registration_mode' : IDL.Func([UserNumber], [Timestamp], []),

src/frontend/generated/internet_identity_types.d.ts

@@ -43,6 +43,7 @@ export interface AuthnMethodRegistrationInfo {
   'expiration' : Timestamp,
   'authn_method' : [] | [AuthnMethodData],
 }
+export type AuthnMethodRemoveResponse = { 'ok' : null };
 export interface BufferedArchiveEntry {
   'sequence_number' : bigint,
   'entry' : Uint8Array | number[],
@@ -203,6 +204,10 @@ export interface _SERVICE {
     [IdentityNumber, AuthnMethodData],
     [] | [AuthnMethodAddResponse]
   >,
+  'authn_method_remove' : ActorMethod<
+    [IdentityNumber, PublicKey],
+    [] | [AuthnMethodRemoveResponse]
+  >,
   'create_challenge' : ActorMethod<[], Challenge>,
   'deploy_archive' : ActorMethod<[Uint8Array | number[]], DeployArchiveResult>,
   'enter_device_registration_mode' : ActorMethod<[UserNumber], Timestamp>,

src/internet_identity/internet_identity.did

@@ -363,6 +363,10 @@ type AuthnMethodAddResponse = variant {
     invalid_metadata: text;
 };
 
+type AuthnMethodRemoveResponse = variant {
+    ok;
+};
+
 type IdentityMetadataReplaceResponse = variant {
     ok;
 };
@@ -423,4 +427,8 @@ service : (opt InternetIdentityInit) -> {
     // Adds a new authentication method to the identity.
     // Requires authentication.
     authn_method_add: (IdentityNumber, AuthnMethodData) -> (opt AuthnMethodAddResponse);
+
+    // Removes the authentication method associated with the public key from the identity.
+    // Requires authentication.
+    authn_method_remove: (IdentityNumber, PublicKey) -> (opt AuthnMethodRemoveResponse);
 }

src/internet_identity/src/main.rs

@@ -489,6 +489,13 @@ fn check_authentication(anchor_number: AnchorNumber) -> Result<(Anchor, DeviceKe
 /// * 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.
+///
+/// TODO, API v2 rollout plan:
+/// 1. Develop API v2 far enough so that front-ends can switch to it.
+/// 2. Deprecate the old API.
+/// 3. Add additional errors to the API v2 return type. The canister should no longer trap on invalid
+///    input.
+/// 4. Add additional features to the API v2, that were not possible with the old API.
 mod v2_api {
     use super::*;
 
@@ -534,6 +541,16 @@ mod v2_api {
         Some(result)
     }
 
+    #[update]
+    #[candid_method]
+    fn authn_method_remove(
+        identity_number: IdentityNumber,
+        public_key: PublicKey,
+    ) -> Option<AuthnMethodRemoveResponse> {
+        remove(identity_number, public_key);
+        Some(AuthnMethodRemoveResponse::Ok)
+    }
+
     #[update]
     #[candid_method]
     fn identity_metadata_replace(

src/internet_identity/tests/integration/v2_api/authn_method_add.rs

@@ -1,5 +1,5 @@
 use crate::v2_api::authn_method_test_helpers::{
-    create_identity_with_authn_method, eq_ignoring_last_authentication, test_authn_method,
+    create_identity_with_authn_method, eq_ignoring_last_authentication, sample_authn_method,
 };
 use candid::Principal;
 use canister_tests::api::internet_identity::api_v2;
@@ -10,8 +10,7 @@ use canister_tests::match_value;
 use ic_test_state_machine_client::CallError;
 use ic_test_state_machine_client::ErrorCode::CanisterCalledTrap;
 use internet_identity_interface::internet_identity::types::{
-    AuthnMethod, AuthnMethodAddResponse, AuthnMethodData, IdentityInfoResponse, MetadataEntry,
-    PublicKeyAuthn,
+    AuthnMethodAddResponse, IdentityInfoResponse, MetadataEntry,
 };
 use regex::Regex;
 use serde_bytes::ByteBuf;
@@ -98,12 +97,3 @@ fn should_report_error_on_failed_conversion() -> Result<(), CallError> {
 
     Ok(())
 }
-
-fn sample_authn_method(i: u8) -> AuthnMethodData {
-    AuthnMethodData {
-        authn_method: AuthnMethod::PubKey(PublicKeyAuthn {
-            pubkey: ByteBuf::from(vec![i; 32]),
-        }),
-        ..test_authn_method()
-    }
-}

src/internet_identity/tests/integration/v2_api/authn_method_remove.rs

@@ -0,0 +1,79 @@
+use crate::v2_api::authn_method_test_helpers::{
+    create_identity_with_authn_method, sample_authn_method,
+};
+use candid::Principal;
+use canister_tests::api::internet_identity::api_v2;
+use canister_tests::framework::{
+    env, expect_user_error_with_message, install_ii_canister, II_WASM,
+};
+use canister_tests::match_value;
+use ic_test_state_machine_client::CallError;
+use ic_test_state_machine_client::ErrorCode::CanisterCalledTrap;
+use internet_identity_interface::internet_identity::types::IdentityInfoResponse;
+use internet_identity_interface::internet_identity::types::{
+    AuthnMethodAddResponse, AuthnMethodRemoveResponse,
+};
+use regex::Regex;
+
+#[test]
+fn should_remove_authn_method() -> Result<(), CallError> {
+    let env = env();
+    let canister_id = install_ii_canister(&env, II_WASM.clone());
+    let authn_method_1 = sample_authn_method(1);
+    let principal = authn_method_1.principal();
+    let authn_method_2 = sample_authn_method(2);
+
+    let identity_number = create_identity_with_authn_method(&env, canister_id, &authn_method_1);
+    let result = api_v2::authn_method_add(
+        &env,
+        canister_id,
+        principal,
+        identity_number,
+        &authn_method_2,
+    )?
+    .unwrap();
+
+    assert!(matches!(result, AuthnMethodAddResponse::Ok));
+
+    match_value!(
+        api_v2::identity_info(&env, canister_id, principal, identity_number)?,
+        Some(IdentityInfoResponse::Ok(identity_info))
+    );
+
+    assert_eq!(identity_info.authn_methods.len(), 2);
+
+    match_value!(
+        api_v2::authn_method_remove(
+            &env,
+            canister_id,
+            principal,
+            identity_number,
+            &authn_method_2.public_key(),
+        )?,
+        Some(AuthnMethodRemoveResponse::Ok)
+    );
+    Ok(())
+}
+
+#[test]
+fn should_require_authentication_to_remove_authn_method() -> Result<(), CallError> {
+    let env = env();
+    let canister_id = install_ii_canister(&env, II_WASM.clone());
+    let authn_method = sample_authn_method(1);
+
+    let identity_number = create_identity_with_authn_method(&env, canister_id, &authn_method);
+
+    let result = api_v2::authn_method_remove(
+        &env,
+        canister_id,
+        Principal::anonymous(),
+        identity_number,
+        &authn_method.public_key(),
+    );
+    expect_user_error_with_message(
+        result,
+        CanisterCalledTrap,
+        Regex::new("[a-z\\d-]+ could not be authenticated.").unwrap(),
+    );
+    Ok(())
+}

src/internet_identity/tests/integration/v2_api/authn_method_test_helpers.rs

@@ -47,3 +47,12 @@ pub fn create_identity_with_authn_method(
     };
     user_number
 }
+
+pub fn sample_authn_method(i: u8) -> AuthnMethodData {
+    AuthnMethodData {
+        authn_method: AuthnMethod::PubKey(PublicKeyAuthn {
+            pubkey: ByteBuf::from(vec![i; 32]),
+        }),
+        ..test_authn_method()
+    }
+}

src/internet_identity/tests/integration/v2_api/mod.rs

@@ -1,4 +1,5 @@
 mod authn_method_add;
+mod authn_method_remove;
 pub mod authn_method_test_helpers;
 mod identity_info;
 mod identity_metadata;

src/internet_identity_interface/src/internet_identity/authn_method.rs

@@ -1,14 +1,21 @@
-use crate::internet_identity::types::{AuthnMethod, AuthnMethodData, PublicKeyAuthn, WebAuthn};
+use crate::internet_identity::types::{
+    AuthnMethod, AuthnMethodData, PublicKey, PublicKeyAuthn, WebAuthn,
+};
 use candid::Principal;
 
 impl AuthnMethodData {
     /// Returns the principal of this authentication method (i.e. the caller principal observed in
     /// the II canister when signing canister calls with the given authentication method).
     pub fn principal(&self) -> Principal {
-        let pubkey = match self.authn_method {
+        Principal::self_authenticating(self.public_key())
+    }
+
+    /// Returns the public key of this authentication method.
+    pub fn public_key(&self) -> PublicKey {
+        match self.authn_method {
             AuthnMethod::WebAuthn(WebAuthn { ref pubkey, .. }) => pubkey,
             AuthnMethod::PubKey(PublicKeyAuthn { ref pubkey }) => pubkey,
-        };
-        Principal::self_authenticating(pubkey)
+        }
+        .clone()
     }
 }

src/internet_identity_interface/src/internet_identity/types/api_v2.rs

@@ -73,6 +73,12 @@ pub enum AuthnMethodAddResponse {
     InvalidMetadata(String),
 }
 
+#[derive(Clone, Debug, CandidType, Deserialize, Eq, PartialEq)]
+pub enum AuthnMethodRemoveResponse {
+    #[serde(rename = "ok")]
+    Ok,
+}
+
 #[derive(Clone, Debug, CandidType, Deserialize, Eq, PartialEq)]
 pub enum IdentityMetadataReplaceResponse {
     #[serde(rename = "ok")]