Avoid public fields

Makes it hard to change internal details without breaking API. Instead expose getters where needed;
We might need to go through the publicly exposed API and only keep the bare minimum

src/lib.rs

@@ -54,8 +54,8 @@ pub struct OathSession {
     salt: Vec<u8>,
     challenge: Option<Vec<u8>>,
     transaction_context: TransactionContext,
-    pub locked: bool,
+    locked: bool,
-    pub name: String,
+    name: String,
 }
 
 impl OathSession {
@@ -192,7 +192,7 @@ impl OathSession {
             Instruction::Delete as u8,
             0,
             0,
-            Some(&cred.id_data.as_tlv()),
+            Some(&cred.id_data().as_tlv()),
         )?;
         Ok(())
     }
@@ -212,11 +212,11 @@ impl OathSession {
             .copy_from_slice(&secret_short[..len_to_copy]);
 
         let mut data = [
-            cred.id_data.as_tlv(),
+            cred.id_data().as_tlv(),
             to_tlv(
                 Tag::Key,
                 &[
-                    [(cred.id_data.oath_type as u8) | (algo as u8), digits].to_vec(),
+                    [(cred.id_data().oath_type() as u8) | (algo as u8), digits].to_vec(),
                     secret_padded.to_vec(),
                 ]
                 .concat(),
@@ -224,7 +224,7 @@ impl OathSession {
         ]
         .concat();
 
-        if cred.touch_required {
+        if cred.is_touch_required() {
             data.extend([Tag::Property as u8, 2u8]); // FIXME: python impl does *not* send this to tlv, which seems to work but feels wrong. See also: https://github.com/Yubico/yubikey-manager/issues/660
         }
 
@@ -260,17 +260,17 @@ impl OathSession {
         cred: &OathCredential,
         timestamp_sys: Option<SystemTime>,
     ) -> Result<OathCodeDisplay, Error> {
-        if self.name != cred.device_id {
+        if self.name != cred.device_id() {
             return Err(Error::DeviceMismatch);
         }
 
         let timestamp = timestamp_sys.unwrap_or_else(SystemTime::now);
 
-        let mut data = cred.id_data.as_tlv();
+        let mut data = cred.id_data().as_tlv();
-        if cred.id_data.oath_type == OathType::Totp {
+        if cred.id_data().oath_type() == OathType::Totp {
             data.extend(to_tlv(
                 Tag::Challenge,
-                &time_challenge(Some(timestamp), cred.id_data.period()),
+                &time_challenge(Some(timestamp), cred.id_data().period()),
             ));
         }
 
@@ -310,15 +310,11 @@ impl OathSession {
         let mut key_buffer = Vec::new();
 
         for (cred_id, meta) in TlvZipIter::from_vec(response?) {
-            let touch = Into::<u8>::into(meta.tag()) == (Tag::Touch as u8); // touch only works with totp, this is intended
+            let touch_required = Into::<u8>::into(meta.tag()) == (Tag::Touch as u8); // touch only works with totp, this is intended
             let id_data = CredentialIDData::from_tlv(cred_id.value(), meta.tag());
             let code = OathCodeDisplay::from_tlv(meta);
 
-            let cred = OathCredential {
+            let cred = OathCredential::new(&self.name, id_data, touch_required);
-                device_id: self.name.clone(),
-                id_data,
-                touch_required: touch,
-            };
 
             let mut refreshable_cred = RefreshableOathCredential::new(cred, self);
             refreshable_cred.force_update(code, timestamp);

src/oath_credential.rs

@@ -7,29 +7,48 @@ use crate::CredentialIDData;
 
 #[derive(Debug, Clone)]
 pub struct OathCredential {
-    pub device_id: String,
+    device_id: String,
-    pub id_data: CredentialIDData,
+    id_data: CredentialIDData,
-    pub touch_required: bool,
+    touch_required: bool,
+}
+
+impl OathCredential {
+    pub fn new(name: &str, id_data: CredentialIDData, touch_required: bool) -> Self {
+        Self {
+            device_id: name.to_owned(),
+            id_data,
+            touch_required,
+        }
+    }
+    pub fn device_id(&self) -> &str {
+        &self.device_id
+    }
+
+    pub fn id_data(&self) -> &CredentialIDData {
+        &self.id_data
+    }
+
+    pub fn is_touch_required(&self) -> bool {
+        self.touch_required
+    }
 }
 
 impl PartialOrd for OathCredential {
     fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
         let a = (
             self.id_data
-                .issuer
+                .issuer()
-                .clone()
+                .unwrap_or_else(|| self.id_data.name())
-                .unwrap_or_else(|| self.id_data.name.clone())
                 .to_lowercase(),
-            self.id_data.name.to_lowercase(),
+            self.id_data.name().to_lowercase(),
         );
         let b = (
             other
                 .id_data
-                .issuer
+                .issuer()
-                .clone()
+                .unwrap_or_else(|| other.id_data.name())
-                .unwrap_or_else(|| other.id_data.name.clone())
                 .to_lowercase(),
-            other.id_data.name.to_lowercase(),
+            other.id_data.name().to_lowercase(),
         );
         Some(a.cmp(&b))
     }

src/oath_credential_id.rs

@@ -14,15 +14,15 @@ use crate::{to_tlv, OathType, Tag, DEFAULT_PERIOD};
 pub struct CredentialIDData {
     /// name of a credential.
     /// Typically specifies an account.
-    pub name: String,
+    name: String,
 
     /// One of `OathType::Totp` or `OathType::Hotp`.
     /// Specifies the type of OTP used represented by this credential.
-    pub oath_type: OathType,
+    oath_type: OathType,
 
     /// issuer of the credential.
     /// Typically specifies the platform.
-    pub issuer: Option<String>,
+    issuer: Option<String>,
 
     /// validity period of each generated code.
     period: Option<Duration>,
@@ -55,6 +55,18 @@ impl CredentialIDData {
         to_tlv(Tag::Name, &self.format_cred_id())
     }
 
+    pub fn name(&self) -> &str {
+        &self.name
+    }
+
+    pub fn issuer(&self) -> Option<&str> {
+        self.issuer.as_deref()
+    }
+
+    pub fn oath_type(&self) -> OathType {
+        self.oath_type
+    }
+
     /// Returns the defined period or default
     pub fn period(&self) -> Duration {
         self.period.unwrap_or(DEFAULT_PERIOD)

src/refreshable_oath_credential.rs

@@ -7,18 +7,18 @@ use std::{
 use crate::{OathCodeDisplay, OathCredential, OathSession, OathType};
 
 pub struct RefreshableOathCredential<'a> {
-    pub cred: OathCredential,
+    cred: OathCredential,
-    pub code: Option<OathCodeDisplay>,
+    code: Option<OathCodeDisplay>,
-    pub valid_timeframe: Range<SystemTime>,
+    valid_timeframe: Range<SystemTime>,
     refresh_provider: &'a OathSession,
 }
 
 impl Display for RefreshableOathCredential<'_> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         if let Some(c) = self.code {
-            f.write_fmt(format_args!("{}: {}", self.cred.id_data, c))
+            f.write_fmt(format_args!("{}: {}", self.cred.id_data(), c))
         } else {
-            f.write_fmt(format_args!("{}", self.cred.id_data))
+            f.write_fmt(format_args!("{}", self.cred.id_data()))
         }
     }
 }
@@ -60,17 +60,24 @@ impl<'a> RefreshableOathCredential<'a> {
     }
 
     fn format_validity_time_frame(&self, timestamp: SystemTime) -> Range<SystemTime> {
-        match self.cred.id_data.oath_type {
+        match self.cred.id_data().oath_type() {
             OathType::Totp => {
                 let timestamp_seconds = timestamp
                     .duration_since(SystemTime::UNIX_EPOCH)
                     .as_ref()
                     .map_or(0, Duration::as_secs);
-                let time_step = timestamp_seconds / (self.cred.id_data.period().as_secs());
+                let time_step = timestamp_seconds / (self.cred.id_data().period().as_secs());
                 let valid_from = SystemTime::UNIX_EPOCH
-                    .checked_add(self.cred.id_data.period().saturating_mul(time_step as u32))
+                    .checked_add(
+                        self.cred
+                            .id_data()
+                            .period()
+                            .saturating_mul(time_step as u32),
+                    )
+                    .unwrap();
+                let valid_to = valid_from
+                    .checked_add(self.cred.id_data().period())
                     .unwrap();
-                let valid_to = valid_from.checked_add(self.cred.id_data.period()).unwrap();
                 valid_from..valid_to
             }
             OathType::Hotp => {