Skip to content

Commit

Permalink
Collect origin information on device registration (#1243)
Browse files Browse the repository at this point in the history
This PR adds origin information to newly registered devices, which
can be used to facilitate the upcoming domain migration.

* 🤖 npm run generate auto-update

* Add origin to front-end calls

* Fix tests

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
1 parent a1fd19d commit f597cee
Show file tree
Hide file tree
Showing 18 changed files with 249 additions and 18 deletions.
2 changes: 2 additions & 0 deletions src/archive/archive.did
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ type DeviceDataWithoutAlias = record {
purpose: Purpose;
key_type: KeyType;
protection: DeviceProtection;
origin: opt text;
};

type DeviceDataUpdate = record {
Expand All @@ -64,6 +65,7 @@ type DeviceDataUpdate = record {
purpose: opt Purpose;
key_type: opt KeyType;
protection: opt DeviceProtection;
origin: opt opt text;
};

type Private = variant {
Expand Down
24 changes: 20 additions & 4 deletions src/archive/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use ic_test_state_machine_client::ErrorCode::CanisterCalledTrap;
use internet_identity_interface::archive::*;
use internet_identity_interface::*;
use regex::Regex;
use serde_bytes::ByteBuf;
use std::time::{Duration, SystemTime};

/// Verifies that the canister can be installed successfully.
Expand Down Expand Up @@ -113,10 +114,16 @@ mod rollback_tests {
// rollback
upgrade_archive_canister(&env, canister_id, ARCHIVE_WASM_PREVIOUS.clone());

let logs = api::get_entries(&env, canister_id, None, None)?;
let logs = api::compat::get_entries(&env, canister_id, None, None)?;
assert_eq!(logs.entries.len(), 2);
assert_eq!(logs.entries.get(0).unwrap().as_ref().unwrap(), &entry1);
assert_eq!(logs.entries.get(1).unwrap().as_ref().unwrap(), &entry2);
assert_eq!(
logs.entries.get(0).unwrap().as_ref().unwrap(),
&api::compat::Entry::from(entry1)
);
assert_eq!(
logs.entries.get(1).unwrap().as_ref().unwrap(),
&api::compat::Entry::from(entry2)
);
Ok(())
}
}
Expand Down Expand Up @@ -729,6 +736,7 @@ mod stable_memory_tests {
purpose: Purpose::Authentication,
key_type: KeyType::Unknown,
protection: DeviceProtection::Unprotected,
origin: None,
},
},
timestamp: TIMESTAMP,
Expand All @@ -743,7 +751,14 @@ mod stable_memory_tests {
let add_entry = Entry {
anchor: ANCHOR,
operation: Operation::AddDevice {
device: DeviceDataWithoutAlias::from(device_data_2()),
device: DeviceDataWithoutAlias {
pubkey: ByteBuf::from(PUBKEY_2),
credential_id: None,
purpose: Purpose::Authentication,
key_type: KeyType::Unknown,
protection: DeviceProtection::Unprotected,
origin: None,
},
},
timestamp: TIMESTAMP,
caller: principal_1(),
Expand All @@ -764,6 +779,7 @@ mod stable_memory_tests {
purpose: Some(Purpose::Recovery),
key_type: None,
protection: None,
origin: None,
},
},
timestamp: TIMESTAMP,
Expand Down
146 changes: 145 additions & 1 deletion src/canister_tests/src/api/archive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,148 @@ pub fn status(env: &StateMachine, canister_id: CanisterId) -> Result<ArchiveStat
}

/// A "compatibility" module for the previous version of the archive to handle API changes.
pub mod compat {}
pub mod compat {
use super::*;
use candid::{CandidType, Deserialize};

#[derive(Eq, PartialEq, Clone, Debug, CandidType, Deserialize)]
pub struct Entry {
// store anchor in LogEntry, such that anchor operations can be attributed to an anchor without consulting the index.
pub anchor: AnchorNumber,
pub operation: Operation,
pub timestamp: Timestamp,
pub caller: Principal,
// global sequence number to detect lost messages (if any)
pub sequence_number: u64,
}

impl From<archive::Entry> for Entry {
fn from(value: archive::Entry) -> Self {
Self {
anchor: value.anchor,
operation: Operation::from(value.operation),
timestamp: value.timestamp,
caller: value.caller,
sequence_number: value.sequence_number,
}
}
}

#[derive(Eq, PartialEq, Clone, Debug, CandidType, Deserialize)]
pub enum Operation {
#[serde(rename = "register_anchor")]
RegisterAnchor { device: DeviceDataWithoutAlias },
#[serde(rename = "add_device")]
AddDevice { device: DeviceDataWithoutAlias },
#[serde(rename = "update_device")]
UpdateDevice {
device: PublicKey,
new_values: DeviceDataUpdate,
},
#[serde(rename = "replace_device")]
ReplaceDevice {
old_device: PublicKey,
new_device: DeviceDataWithoutAlias,
},
#[serde(rename = "remove_device")]
RemoveDevice { device: PublicKey },
}

impl From<archive::Operation> for Operation {
fn from(value: archive::Operation) -> Self {
match value {
archive::Operation::RegisterAnchor { device } => Operation::RegisterAnchor {
device: DeviceDataWithoutAlias::from(device),
},
archive::Operation::AddDevice { device } => Operation::AddDevice {
device: DeviceDataWithoutAlias::from(device),
},
archive::Operation::UpdateDevice { device, new_values } => {
Operation::UpdateDevice {
device,
new_values: DeviceDataUpdate::from(new_values),
}
}
archive::Operation::ReplaceDevice {
old_device,
new_device,
} => Operation::ReplaceDevice {
old_device,
new_device: DeviceDataWithoutAlias::from(new_device),
},
archive::Operation::RemoveDevice { device } => Operation::RemoveDevice { device },
}
}
}

#[derive(Eq, PartialEq, Clone, Debug, CandidType, Deserialize)]
pub struct DeviceDataWithoutAlias {
pub pubkey: DeviceKey,
pub credential_id: Option<CredentialId>,
pub purpose: Purpose,
pub key_type: KeyType,
pub protection: DeviceProtection,
}

impl From<archive::DeviceDataWithoutAlias> for DeviceDataWithoutAlias {
fn from(value: archive::DeviceDataWithoutAlias) -> Self {
Self {
pubkey: value.pubkey,
credential_id: value.credential_id,
purpose: value.purpose,
key_type: value.key_type,
protection: value.protection,
}
}
}

impl From<DeviceData> for DeviceDataWithoutAlias {
fn from(device_data: DeviceData) -> Self {
Self {
pubkey: device_data.pubkey,
credential_id: device_data.credential_id,
purpose: device_data.purpose,
key_type: device_data.key_type,
protection: device_data.protection,
}
}
}

// If present, the attribute has been changed to the value given.
// Does not include the pubkey because it cannot be changed.
#[derive(Eq, PartialEq, Clone, Debug, CandidType, Deserialize)]
pub struct DeviceDataUpdate {
pub alias: Option<Private>,
pub credential_id: Option<CredentialId>,
pub purpose: Option<Purpose>,
pub key_type: Option<KeyType>,
pub protection: Option<DeviceProtection>,
}

impl From<archive::DeviceDataUpdate> for DeviceDataUpdate {
fn from(value: archive::DeviceDataUpdate) -> Self {
Self {
alias: value.alias,
credential_id: value.credential_id,
purpose: value.purpose,
key_type: value.key_type,
protection: value.protection,
}
}
}

#[derive(Clone, Debug, CandidType, Deserialize)]
pub struct Entries {
// make this a vec of options to keep Entry extensible
pub entries: Vec<Option<Entry>>,
}

pub fn get_entries(
env: &StateMachine,
canister_id: CanisterId,
idx: Option<u64>,
limit: Option<u16>,
) -> Result<Entries, CallError> {
query_candid(env, canister_id, "get_entries", (idx, limit)).map(|(x,)| x)
}
}
8 changes: 8 additions & 0 deletions src/canister_tests/src/framework.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,7 @@ pub fn device_data_1() -> DeviceData {
purpose: Purpose::Authentication,
key_type: KeyType::Unknown,
protection: DeviceProtection::Unprotected,
origin: Some("https://identity.internetcomputer.org".to_string()),
}
}

Expand All @@ -270,6 +271,7 @@ pub fn device_data_2() -> DeviceData {
purpose: Purpose::Authentication,
key_type: KeyType::Unknown,
protection: DeviceProtection::Unprotected,
origin: Some("https://identity.ic0.app".to_string()),
}
}

Expand All @@ -281,6 +283,7 @@ pub fn max_size_device() -> DeviceData {
purpose: Purpose::Authentication,
key_type: KeyType::Unknown,
protection: DeviceProtection::Unprotected,
origin: Some("https://rdmx6-jaaaa-aaaaa-aaadq-cai.foobar.icp0.io".to_string()),
}
}

Expand All @@ -292,6 +295,7 @@ pub fn recovery_device_data_1() -> DeviceData {
purpose: Purpose::Recovery,
key_type: KeyType::SeedPhrase,
protection: DeviceProtection::Unprotected,
origin: None,
}
}

Expand All @@ -303,6 +307,7 @@ pub fn recovery_device_data_2() -> DeviceData {
purpose: Purpose::Recovery,
key_type: KeyType::SeedPhrase,
protection: DeviceProtection::Unprotected,
origin: None,
}
}

Expand Down Expand Up @@ -526,6 +531,7 @@ pub fn log_entry_1() -> Entry {
purpose: Purpose::Authentication,
key_type: KeyType::Unknown,
protection: DeviceProtection::Unprotected,
origin: None,
},
},
sequence_number: 0,
Expand All @@ -544,6 +550,7 @@ pub fn log_entry_2() -> Entry {
purpose: Purpose::Authentication,
key_type: KeyType::Unknown,
protection: DeviceProtection::Unprotected,
origin: Some("foo.bar".to_string()),
},
},
sequence_number: 1,
Expand All @@ -563,6 +570,7 @@ pub fn log_entry(idx: u64, timestamp: u64, anchor: AnchorNumber) -> Entry {
purpose: Some(Purpose::Authentication),
key_type: None,
protection: Some(DeviceProtection::Unprotected),
origin: Some(Some("foo.bar".to_string())),
},
},
sequence_number: idx,
Expand Down
1 change: 1 addition & 0 deletions src/frontend/generated/internet_identity_idl.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export const idlFactory = ({ IDL }) => {
const CredentialId = IDL.Vec(IDL.Nat8);
const DeviceData = IDL.Record({
'alias' : IDL.Text,
'origin' : IDL.Opt(IDL.Text),
'protection' : DeviceProtection,
'pubkey' : DeviceKey,
'key_type' : KeyType,
Expand Down
1 change: 1 addition & 0 deletions src/frontend/generated/internet_identity_types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export type DeployArchiveResult = { 'creation_in_progress' : null } |
{ 'failed' : string };
export interface DeviceData {
'alias' : string,
'origin' : [] | [string],
'protection' : DeviceProtection,
'pubkey' : DeviceKey,
'key_type' : KeyType,
Expand Down
4 changes: 4 additions & 0 deletions src/frontend/src/showcase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ const recoveryPhrase: DeviceData = {
key_type: { seed_phrase: null },
purpose: { recovery: null },
credential_id: [],
origin: [],
};

const recoveryPhraseText =
Expand All @@ -70,6 +71,7 @@ const recoveryDevice: DeviceData = {
key_type: { unknown: null },
purpose: { recovery: null },
credential_id: [],
origin: [],
};

const simpleDevices: [DeviceData, DeviceData] = [
Expand All @@ -80,6 +82,7 @@ const simpleDevices: [DeviceData, DeviceData] = [
key_type: { unknown: null },
purpose: { authentication: null },
credential_id: [],
origin: [],
},

{
Expand All @@ -89,6 +92,7 @@ const simpleDevices: [DeviceData, DeviceData] = [
key_type: { unknown: null },
purpose: { authentication: null },
credential_id: [],
origin: [],
},
];

Expand Down
3 changes: 3 additions & 0 deletions src/frontend/src/utils/iiConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ export class Connection {
key_type: { unknown: null },
purpose: { authentication: null },
protection: { unprotected: null },
origin: window?.origin === undefined ? [] : [window.origin],
},
challengeResult
);
Expand Down Expand Up @@ -380,6 +381,7 @@ export class Connection {
key_type: keyType,
purpose,
protection: { unprotected: null },
origin: window?.origin === undefined ? [] : [window.origin],
});
};

Expand Down Expand Up @@ -496,6 +498,7 @@ export class AuthenticatedConnection extends Connection {
key_type: keyType,
purpose,
protection,
origin: window?.origin === undefined ? [] : [window.origin],
});
};

Expand Down
1 change: 1 addition & 0 deletions src/internet_identity/internet_identity.did
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ type DeviceData = record {
purpose: Purpose;
key_type: KeyType;
protection: DeviceProtection;
origin: opt text;
};

type RegisterResponse = variant {
Expand Down
5 changes: 5 additions & 0 deletions src/internet_identity/src/archive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -430,5 +430,10 @@ pub fn device_diff(old: &Device, new: &Device) -> DeviceDataUpdate {
} else {
Some(new.protection.clone())
},
origin: if old.origin == new.origin {
None
} else {
Some(new.origin.clone())
},
}
}
Loading

0 comments on commit f597cee

Please sign in to comment.