Skip to content

Commit

Permalink
Refactor: make PWS data struct fields optional
Browse files Browse the repository at this point in the history
This will allow to remove PWS fields from the FlatCredential on its update
  • Loading branch information
szszszsz committed Aug 16, 2023
1 parent 12fe825 commit 495d17b
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 49 deletions.
38 changes: 13 additions & 25 deletions src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -551,15 +551,7 @@ impl<'l, const C: usize> TryFrom<&'l Data<C>> for CredentialUpdate<'l> {
..Default::default()
};

// TODO This would be better modeled with Option, than empty slice,
// to allow fields removal by setting the request fields to empty slice.
// Currently only setting is possible. A workaround for now is to
// set the value to remove to " ".
let mut pws = PasswordSafeData {
login: &[],
password: &[],
metadata: &[],
};
let mut pws: PasswordSafeData = Default::default();
// FIXME remove the need for this additional buffer - requires flexiber modification to
// to access the tag byte as u8, without unpacking completely
let mut buf = [0u8; 255];
Expand All @@ -581,13 +573,13 @@ impl<'l, const C: usize> TryFrom<&'l Data<C>> for CredentialUpdate<'l> {
res.new_label = Some(tag_data);
}
Tag::PwsLogin => {
pws.login = tag_data;
pws.login = Some(tag_data);
}
Tag::PwsPassword => {
pws.password = tag_data;
pws.password = Some(tag_data);
}
Tag::PwsMetadata => {
pws.metadata = tag_data;
pws.metadata = Some(tag_data);
}
_ => {
// Unmatched tags should return error
Expand Down Expand Up @@ -642,16 +634,16 @@ impl<'l> HmacData<'l> {
}
}

#[derive(Clone, Copy, Eq, PartialEq, Debug)]
#[derive(Clone, Copy, Eq, PartialEq, Debug, Default)]
pub struct PasswordSafeData<'l> {
pub login: &'l [u8],
pub password: &'l [u8],
pub metadata: &'l [u8],
pub login: Option<&'l [u8]>,
pub password: Option<&'l [u8]>,
pub metadata: Option<&'l [u8]>,
}

impl<'l> PasswordSafeData<'l> {
pub fn non_empty(&self) -> bool {
!self.login.is_empty() || !self.password.is_empty() || !self.metadata.is_empty()
self.login.is_some() || self.password.is_some() || self.metadata.is_some()
}
}

Expand Down Expand Up @@ -809,11 +801,7 @@ impl<'l, const C: usize> TryFrom<&'l Data<C>> for Register<'l> {
};

let pws_data = {
let mut pws = PasswordSafeData {
login: &[],
password: &[],
metadata: &[],
};
let mut pws: PasswordSafeData = Default::default();

let mut next_decoded: Option<TaggedSlice> = decoder.decode().ok();
while let Some(next) = next_decoded {
Expand All @@ -831,13 +819,13 @@ impl<'l, const C: usize> TryFrom<&'l Data<C>> for Register<'l> {

match tag {
Tag::PwsLogin => {
pws.login = tag_data;
pws.login = Some(tag_data);
}
Tag::PwsPassword => {
pws.password = tag_data;
pws.password = Some(tag_data);
}
Tag::PwsMetadata => {
pws.metadata = tag_data;
pws.metadata = Some(tag_data);
}
_ => {
// Unmatched tags should return error
Expand Down
42 changes: 18 additions & 24 deletions src/credential.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,19 +118,19 @@ impl CredentialFlat {
res.bits()
}

fn get_bytes_or_none_if_empty(x: &[u8]) -> Result<Option<ShortData>, ()> {
Ok(if !x.is_empty() {
fn get_bytes_or_none(xo: Option<&[u8]>) -> Result<Option<ShortData>, ()> {
Ok(if let Some(x) = xo {
Some(ShortData::from_slice(x)?)
} else {
None
})
}

fn get_or_empty_slice_if_none(x: &Option<ShortData>) -> &[u8] {
if let Some(x) = x {
x.as_slice()
fn get_ref_or_none(xo: &Option<ShortData>) -> Option<&[u8]> {
if let Some(x) = xo {
Some(x.as_slice())
} else {
&[]
None
}
}

Expand Down Expand Up @@ -163,9 +163,9 @@ impl CredentialFlat {
};

let p = PasswordSafeData {
login: Self::get_or_empty_slice_if_none(&self.login),
password: Self::get_or_empty_slice_if_none(&self.password),
metadata: Self::get_or_empty_slice_if_none(&self.metadata),
login: Self::get_ref_or_none(&self.login),
password: Self::get_ref_or_none(&self.password),
metadata: Self::get_ref_or_none(&self.metadata),
};
if p.non_empty() {
cred.password_safe = Some(p);
Expand Down Expand Up @@ -202,9 +202,9 @@ impl CredentialFlat {
}

if let Some(pass) = credential.password_safe {
cred.login = Self::get_bytes_or_none_if_empty(pass.login)?;
cred.password = Self::get_bytes_or_none_if_empty(pass.password)?;
cred.metadata = Self::get_bytes_or_none_if_empty(pass.metadata)?;
cred.login = Self::get_bytes_or_none(pass.login)?;
cred.password = Self::get_bytes_or_none(pass.password)?;
cred.metadata = Self::get_bytes_or_none(pass.metadata)?;
}

Ok(cred)
Expand All @@ -219,18 +219,12 @@ impl CredentialFlat {
self.touch_required = p.touch_required();
}
if let Some(pws) = update_req.password_safe {
if pws.login.len() > 0 {
self.login = Self::get_bytes_or_none_if_empty(pws.login)
.map_err(|_| Status::UnspecifiedNonpersistentExecutionError)?;
}
if pws.password.len() > 0 {
self.password = Self::get_bytes_or_none_if_empty(pws.password)
.map_err(|_| Status::UnspecifiedNonpersistentExecutionError)?;
}
if pws.metadata.len() > 0 {
self.metadata = Self::get_bytes_or_none_if_empty(pws.metadata)
.map_err(|_| Status::UnspecifiedNonpersistentExecutionError)?;
}
self.login = Self::get_bytes_or_none(pws.login)
.map_err(|_| Status::UnspecifiedNonpersistentExecutionError)?;
self.password = Self::get_bytes_or_none(pws.password)
.map_err(|_| Status::UnspecifiedNonpersistentExecutionError)?;
self.metadata = Self::get_bytes_or_none(pws.metadata)
.map_err(|_| Status::UnspecifiedNonpersistentExecutionError)?;
}
Ok(())
}
Expand Down

0 comments on commit 495d17b

Please sign in to comment.