Refactor tentative_device_registration to use v2 types instead. (dfinity#3225)

* Refactor `tentative_device_registration` to use v2 types instead. This moves the mapping between types from the v2 methods (v1 -> v2) to the v1 methods (v2 -> v1).

* Refactor `tentative_device_registration` to use v2 types instead. This moves the mapping between types from the v2 methods (v1 -> v2) to the v1 methods (v2 -> v1).

* Refactor `tentative_device_registration` to use v2 types instead. This moves the mapping between types from the v2 methods (v1 -> v2) to the v1 methods (v2 -> v1).

Author: sea-snake

src/internet_identity/src/anchor_management/tentative_device_registration.rs

@@ -10,7 +10,6 @@ use ic_cdk::api::time;
 use ic_cdk::{call, trap};
 use internet_identity_interface::internet_identity::types::*;
 use std::collections::{hash_map, HashMap};
-use TentativeDeviceRegistrationError::{AnotherDeviceTentativelyAdded, DeviceRegistrationModeOff};
 
 // 15 mins
 const REGISTRATION_MODE_DURATION: u64 = secs_to_nanos(900);
@@ -76,28 +75,10 @@ pub fn exit_device_registration_mode(anchor_number: AnchorNumber) {
     });
 }
 
-pub struct TentativeRegistrationInfo {
-    pub verification_code: DeviceConfirmationCode,
-    pub device_registration_timeout: Timestamp,
-}
-
-#[derive(Debug)]
-pub enum TentativeDeviceRegistrationError {
-    DeviceRegistrationModeOff,
-    AnotherDeviceTentativelyAdded,
-    IdentityUpdateError(IdentityUpdateError),
-}
-
-impl From<IdentityUpdateError> for TentativeDeviceRegistrationError {
-    fn from(err: IdentityUpdateError) -> Self {
-        TentativeDeviceRegistrationError::IdentityUpdateError(err)
-    }
-}
-
 pub async fn add_tentative_device(
     anchor_number: AnchorNumber,
     tentative_device: DeviceData,
-) -> Result<TentativeRegistrationInfo, TentativeDeviceRegistrationError> {
+) -> Result<AuthnMethodConfirmationCode, AuthnMethodRegisterError> {
     let confirmation_code = new_confirmation_code().await;
     let now = time();
 
@@ -108,12 +89,12 @@ pub async fn add_tentative_device(
             // Get registration else throw error
             let registration = registrations
                 .get_mut(&anchor_number)
-                .ok_or(DeviceRegistrationModeOff)?;
+                .ok_or(AuthnMethodRegisterError::RegistrationModeOff)?;
 
             match registration {
                 // Make sure registration isn't expired
                 TentativeDeviceRegistration { expiration, .. } if *expiration <= now => {
-                    Err(DeviceRegistrationModeOff)
+                    Err(AuthnMethodRegisterError::RegistrationModeOff)
                 }
                 // Add tentative session if registration mode is active
                 TentativeDeviceRegistration {
@@ -125,25 +106,18 @@ pub async fn add_tentative_device(
                         failed_attempts: 0,
                         confirmation_code: confirmation_code.clone(),
                     };
-                    Ok(TentativeRegistrationInfo {
-                        device_registration_timeout: registration.expiration,
-                        verification_code: confirmation_code,
+                    Ok(AuthnMethodConfirmationCode {
+                        expiration: registration.expiration,
+                        confirmation_code,
                     })
                 }
                 // Else return error
-                _ => Err(AnotherDeviceTentativelyAdded),
+                _ => Err(AuthnMethodRegisterError::RegistrationAlreadyInProgress),
             }
         })
     })
 }
 
-#[derive(Debug)]
-pub enum VerifyTentativeDeviceError {
-    WrongCode { retries_left: u8 },
-    DeviceRegistrationModeOff,
-    NoDeviceToVerify,
-}
-
 /// Confirm the tentative device or session using the submitted `user_confirmation_code`.
 ///
 /// # Returns
@@ -154,20 +128,21 @@ pub enum VerifyTentativeDeviceError {
 pub fn confirm_tentative_device_or_session(
     anchor_number: AnchorNumber,
     user_confirmation_code: DeviceConfirmationCode,
-) -> Result<Option<DeviceData>, VerifyTentativeDeviceError> {
+) -> Result<Option<DeviceData>, AuthnMethodConfirmationError> {
     state::tentative_device_registrations_mut(|registrations| {
         state::lookup_tentative_device_registration_mut(|lookup| {
             prune_expired_tentative_device_registrations(registrations, lookup);
 
             // Get registration else throw error
             let registration = registrations
                 .get_mut(&anchor_number)
-                .ok_or(VerifyTentativeDeviceError::DeviceRegistrationModeOff)?;
+                .ok_or(AuthnMethodConfirmationError::RegistrationModeOff)?;
 
             let (should_remove, response) = match &mut registration.state {
-                DeviceRegistrationModeActive | SessionTentativelyConfirmed { .. } => {
-                    (false, Err(VerifyTentativeDeviceError::NoDeviceToVerify))
-                }
+                DeviceRegistrationModeActive | SessionTentativelyConfirmed { .. } => (
+                    false,
+                    Err(AuthnMethodConfirmationError::NoAuthnMethodToConfirm),
+                ),
                 DeviceTentativelyAdded {
                     failed_attempts,
                     confirmation_code,
@@ -183,7 +158,7 @@ pub fn confirm_tentative_device_or_session(
                         // Remove registration if max attempts reached
                         (
                             *failed_attempts >= MAX_DEVICE_REGISTRATION_ATTEMPTS,
-                            Err(VerifyTentativeDeviceError::WrongCode {
+                            Err(AuthnMethodConfirmationError::WrongCode {
                                 retries_left: (MAX_DEVICE_REGISTRATION_ATTEMPTS - *failed_attempts),
                             }),
                         )
@@ -209,7 +184,7 @@ pub fn confirm_tentative_device_or_session(
                         // Remove registration if max attempts reached
                         (
                             *failed_attempts >= MAX_DEVICE_REGISTRATION_ATTEMPTS,
-                            Err(VerifyTentativeDeviceError::WrongCode {
+                            Err(AuthnMethodConfirmationError::WrongCode {
                                 retries_left: (MAX_DEVICE_REGISTRATION_ATTEMPTS - *failed_attempts),
                             }),
                         )

src/internet_identity/src/main.rs

@@ -1,7 +1,4 @@
 use crate::anchor_management::tentative_device_registration;
-use crate::anchor_management::tentative_device_registration::{
-    TentativeDeviceRegistrationError, TentativeRegistrationInfo, VerifyTentativeDeviceError,
-};
 use crate::archive::ArchiveState;
 use crate::assets::init_assets;
 use crate::state::persistent_state;
@@ -98,23 +95,24 @@ async fn add_tentative_device(
     let result =
         tentative_device_registration::add_tentative_device(anchor_number, device_data).await;
     match result {
-        Ok(TentativeRegistrationInfo {
-            verification_code,
-            device_registration_timeout,
+        Ok(AuthnMethodConfirmationCode {
+            confirmation_code,
+            expiration,
         }) => AddTentativeDeviceResponse::AddedTentatively {
-            verification_code,
-            device_registration_timeout,
+            verification_code: confirmation_code,
+            device_registration_timeout: expiration,
         },
         Err(err) => match err {
-            TentativeDeviceRegistrationError::DeviceRegistrationModeOff => {
+            AuthnMethodRegisterError::RegistrationModeOff => {
                 AddTentativeDeviceResponse::DeviceRegistrationModeOff
             }
-            TentativeDeviceRegistrationError::AnotherDeviceTentativelyAdded => {
+            AuthnMethodRegisterError::RegistrationAlreadyInProgress => {
                 AddTentativeDeviceResponse::AnotherDeviceTentativelyAdded
             }
-            TentativeDeviceRegistrationError::IdentityUpdateError(err) => {
-                // Legacy API traps instead of returning an error.
-                trap(String::from(err).as_str())
+            AuthnMethodRegisterError::InvalidMetadata(_) => {
+                // Unreachable since we don't convert from `AuthnMethodData` to `DeviceWithUsage`
+                // in this legacy method in comparison to the newer `authn_method_register` method.
+                trap("Unreachable error");
             }
         },
     }
@@ -144,15 +142,22 @@ fn verify_tentative_device(
             VerifyTentativeDeviceResponse::Verified
         }
         Err(err) => match err {
-            VerifyTentativeDeviceError::DeviceRegistrationModeOff => {
+            AuthnMethodConfirmationError::RegistrationModeOff => {
                 VerifyTentativeDeviceResponse::DeviceRegistrationModeOff
             }
-            VerifyTentativeDeviceError::NoDeviceToVerify => {
+            AuthnMethodConfirmationError::NoAuthnMethodToConfirm => {
                 VerifyTentativeDeviceResponse::NoDeviceToVerify
             }
-            VerifyTentativeDeviceError::WrongCode { retries_left } => {
+            AuthnMethodConfirmationError::WrongCode { retries_left } => {
                 VerifyTentativeDeviceResponse::WrongCode { retries_left }
             }
+            // Unreachable since these two errors already result in a trap in this legacy method.
+            AuthnMethodConfirmationError::Unauthorized(principal) => {
+                trap(&format!("{principal} could not be authenticated."))
+            }
+            AuthnMethodConfirmationError::InternalCanisterError(err) => {
+                trap(&err.to_string());
+            }
         },
     }
 }
@@ -915,22 +920,12 @@ mod v2_api {
     ) -> Result<AuthnMethodConfirmationCode, AuthnMethodRegisterError> {
         let device = DeviceWithUsage::try_from(authn_method)
             .map_err(|err| AuthnMethodRegisterError::InvalidMetadata(err.to_string()))?;
-        let result = add_tentative_device(identity_number, DeviceData::from(device)).await;
-        match result {
-            AddTentativeDeviceResponse::AddedTentatively {
-                device_registration_timeout,
-                verification_code,
-            } => Ok(AuthnMethodConfirmationCode {
-                expiration: device_registration_timeout,
-                confirmation_code: verification_code,
-            }),
-            AddTentativeDeviceResponse::DeviceRegistrationModeOff => {
-                Err(AuthnMethodRegisterError::RegistrationModeOff)
-            }
-            AddTentativeDeviceResponse::AnotherDeviceTentativelyAdded => {
-                Err(AuthnMethodRegisterError::RegistrationAlreadyInProgress)
-            }
-        }
+
+        tentative_device_registration::add_tentative_device(
+            identity_number,
+            DeviceData::from(device),
+        )
+        .await
     }
 
     #[update]
@@ -945,18 +940,7 @@ mod v2_api {
             tentative_device_registration::confirm_tentative_device_or_session(
                 identity_number,
                 confirmation_code,
-            )
-            .map_err(|err| match err {
-                VerifyTentativeDeviceError::WrongCode { retries_left } => {
-                    AuthnMethodConfirmationError::WrongCode { retries_left }
-                }
-                VerifyTentativeDeviceError::DeviceRegistrationModeOff => {
-                    AuthnMethodConfirmationError::RegistrationModeOff
-                }
-                VerifyTentativeDeviceError::NoDeviceToVerify => {
-                    AuthnMethodConfirmationError::NoAuthnMethodToConfirm
-                }
-            })?;
+            )?;
 
         if let Some(confirmed_device) = maybe_confirmed_device {
             // Add device to anchor with bookkeeping if it has been confirmed