Skip to content

Commit

Permalink
Add anchor number to Anchor type (#2256)
Browse files Browse the repository at this point in the history
* Add anchor number to Anchor type

This refactoring includes the anchor number in the `Anchor` business
logic struct, making it impossible to mix up different anchors in storage.

* Readd out of bound anchor nr test
  • Loading branch information
Frederik Rothenberger authored Feb 7, 2024
1 parent c5b1826 commit 83e8f88
Show file tree
Hide file tree
Showing 7 changed files with 111 additions and 89 deletions.
17 changes: 11 additions & 6 deletions src/internet_identity/src/anchor_management/registration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,20 +244,25 @@ pub fn register(
verify_caller_is_device_or_temp_key(&temp_key, &device_principal);

let allocation = state::storage_borrow_mut(|storage| storage.allocate_anchor());
let Some((anchor_number, mut anchor)) = allocation else {
let Some(mut anchor) = allocation else {
return RegisterResponse::CanisterFull;
};
let anchor_number = anchor.anchor_number();

anchor
.add_device(device.clone())
.unwrap_or_else(|err| trap(&format!("failed to register anchor {anchor_number}: {err}")));
anchor.add_device(device.clone()).unwrap_or_else(|err| {
trap(&format!(
"failed to register anchor {}: {err}",
anchor_number
))
});
activity_bookkeeping(&mut anchor, &device.pubkey);

// write anchor to stable memory
state::storage_borrow_mut(|storage| {
storage.write(anchor_number, anchor).unwrap_or_else(|err| {
storage.write(anchor).unwrap_or_else(|err| {
trap(&format!(
"failed to write data of anchor {anchor_number}: {err}"
"failed to write data of anchor {}: {err}",
anchor_number
))
});
});
Expand Down
4 changes: 2 additions & 2 deletions src/internet_identity/src/authz_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ where
let result = op(&mut anchor);

// write back anchor
state::storage_borrow_mut(|storage| storage.write(anchor_number, anchor))
state::storage_borrow_mut(|storage| storage.write(anchor))
.map_err(|err| E::from(IdentityUpdateError::StorageError(anchor_number, err)))?;

match result {
Expand Down Expand Up @@ -126,7 +126,7 @@ pub fn check_authz_and_record_activity(
check_authorization(anchor_number).map_err(IdentityUpdateError::from)?;
let maybe_domain = anchor.device(&device_key).unwrap().ii_domain();
anchor_management::activity_bookkeeping(&mut anchor, &device_key);
state::storage_borrow_mut(|storage| storage.write(anchor_number, anchor))
state::storage_borrow_mut(|storage| storage.write(anchor))
.map_err(|err| IdentityUpdateError::StorageError(anchor_number, err))?;
Ok(maybe_domain)
}
41 changes: 23 additions & 18 deletions src/internet_identity/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,27 +203,28 @@ fn remove(anchor_number: AnchorNumber, device_key: DeviceKey) {
#[query]
#[candid_method(query)]
fn lookup(anchor_number: AnchorNumber) -> Vec<DeviceData> {
state::storage_borrow(|storage| {
storage
.read(anchor_number)
.unwrap_or_default()
.into_devices()
.into_iter()
.map(DeviceData::from)
.map(|mut d| {
// Remove non-public fields.
d.alias = "".to_string();
d.metadata = None;
d
})
.collect()
})
let Ok(anchor) = state::storage_borrow(|storage| storage.read(anchor_number)) else {
return vec![];
};
anchor
.into_devices()
.into_iter()
.map(DeviceData::from)
.map(|mut d| {
// Remove non-public fields.
d.alias = "".to_string();
d.metadata = None;
d
})
.collect()
}

#[query]
#[candid_method(query)]
fn get_anchor_credentials(anchor_number: AnchorNumber) -> AnchorCredentials {
let anchor = state::storage_borrow(|storage| storage.read(anchor_number).unwrap_or_default());
let Ok(anchor) = state::storage_borrow(|storage| storage.read(anchor_number)) else {
return AnchorCredentials::default();
};

anchor.into_devices().into_iter().fold(
AnchorCredentials {
Expand Down Expand Up @@ -506,8 +507,12 @@ mod v2_api {
#[query]
#[candid_method(query)]
fn identity_authn_info(identity_number: IdentityNumber) -> Result<IdentityAuthnInfo, ()> {
let anchor =
state::storage_borrow(|storage| storage.read(identity_number).unwrap_or_default());
let Ok(anchor) = state::storage_borrow(|storage| storage.read(identity_number)) else {
return Ok(IdentityAuthnInfo {
authn_methods: vec![],
recovery_authn_methods: vec![],
});
};

let authn_info = anchor.into_devices().into_iter().fold(
IdentityAuthnInfo {
Expand Down
9 changes: 5 additions & 4 deletions src/internet_identity/src/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,18 +270,19 @@ impl<M: Memory + Clone> Storage<M> {
///
/// Returns None if the range of Identity Anchor assigned to this
/// storage is exhausted.
pub fn allocate_anchor(&mut self) -> Option<(AnchorNumber, Anchor)> {
pub fn allocate_anchor(&mut self) -> Option<Anchor> {
let anchor_number = self.header.id_range_lo + self.header.num_anchors as u64;
if anchor_number >= self.header.id_range_hi {
return None;
}
self.header.num_anchors += 1;
self.flush();
Some((anchor_number, Anchor::new()))
Some(Anchor::new(anchor_number))
}

/// Writes the data of the specified anchor to stable memory.
pub fn write(&mut self, anchor_number: AnchorNumber, data: Anchor) -> Result<(), StorageError> {
pub fn write(&mut self, data: Anchor) -> Result<(), StorageError> {
let anchor_number = data.anchor_number();
let storable_anchor = StorableAnchor::from(data);
let buf = storable_anchor.to_bytes();
if buf.len() > self.header.entry_size as usize {
Expand Down Expand Up @@ -310,7 +311,7 @@ impl<M: Memory + Clone> Storage<M> {

reader.read_exact(&mut buf).expect("failed to read memory");
let storable_anchor = StorableAnchor::from_bytes(Cow::Owned(buf));
Ok(Anchor::from(storable_anchor))
Ok(Anchor::from((anchor_number, storable_anchor)))
}

/// Make sure all the required metadata is recorded to stable memory.
Expand Down
15 changes: 11 additions & 4 deletions src/internet_identity/src/storage/anchor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ mod tests;
/// The anchor has limited visibility for the constructor to make sure it is loaded from storage.
/// The devices can only be modified by the exposed functions which keeps invariant checking local
/// to this module.
#[derive(Clone, Debug, Default, Eq, PartialEq)]
#[derive(Clone, Debug, Eq, PartialEq)]
pub struct Anchor {
anchor_number: AnchorNumber,
devices: Vec<Device>,
metadata: Option<HashMap<String, MetadataEntry>>,
}
Expand Down Expand Up @@ -107,9 +108,10 @@ impl From<Anchor> for StorableAnchor {
}
}

impl From<StorableAnchor> for Anchor {
fn from(storable_anchor: StorableAnchor) -> Self {
impl From<(AnchorNumber, StorableAnchor)> for Anchor {
fn from((anchor_number, storable_anchor): (AnchorNumber, StorableAnchor)) -> Self {
Anchor {
anchor_number,
devices: storable_anchor.devices,
metadata: storable_anchor.metadata,
}
Expand All @@ -119,13 +121,18 @@ impl From<StorableAnchor> for Anchor {
impl Anchor {
/// Creation of new anchors is restricted in order to make sure that the device checks are
/// not accidentally bypassed.
pub(super) fn new() -> Anchor {
pub(super) fn new(anchor_number: AnchorNumber) -> Anchor {
Self {
anchor_number,
devices: vec![],
metadata: None,
}
}

pub fn anchor_number(&self) -> AnchorNumber {
self.anchor_number
}

pub fn add_device(&mut self, device: Device) -> Result<(), AnchorError> {
if self.devices.iter().any(|e| e.pubkey == device.pubkey) {
return Err(AnchorError::DuplicateDevice {
Expand Down
Loading

0 comments on commit 83e8f88

Please sign in to comment.