refactor: trait type system

[lonerapier]

Trait System

Currently MFKDF2 uses Factor as the base struct that contains each factor in an enum FactorType. Setup and Derive functionality is implemented using FactorSetup and FactorDerive traits.

pub trait FactorDerive: Send + Sync + std::fmt::Debug {
  fn include_params(&mut self, params: Value) -> MFKDF2Result<()>;
  fn params(&self, key: Key) -> MFKDF2Result<Value> { Ok(serde_json::json!({})) }
  fn output(&self) -> Value { serde_json::json!({}) }
}

This trait is implemented by all factor types and the usage looks like:

let factor = hotp(options)?;

let setup_params = factor.factor_type.setup().params(key.into())?;

let derive_params = factor.factor_type.derive().params(key.into())?;

setup and derive methods return a trait object (dyn FactorSetup/FactorDerive). Each new factor is required to implement these traits, and update the trait object method so that setup and derive work.

Solution

A better solution is to make both traits generic on params and output value, like this:

pub trait FactorSetup: Send + Sync + std::fmt::Debug {
  type Params: Serialize + DeserializeOwned + std::fmt::Debug + Default;
  type Output: Serialize + DeserializeOwned + std::fmt::Debug + Default;

  fn bytes(&self) -> Vec<u8>;
  fn params(&self, key: Key) -> MFKDF2Result<Self::Params> {
    Ok(serde_json::from_value(serde_json::json!({}))?)
  }
  fn output(&self, key: Key) -> Self::Output { Self::Output::default() }
}

Then each factor type can implement these traits with their own types, and user have a nice abstraction over the factors where each factor is encapsulated under the trait.

Current trait objects doesn't allow this, so the next step is to remove trait objects, and implement SetupParams, SetupOutput, DeriveParams, DeriveOutput enums (similar to FactorType enum). The usage then looks like:

let factor: MFKDF2Factor = hotp(options)?;
// critical modification here (setup_params instead of setup().params())
let params = factor.factor_type.setup_params(key.into())?;

match params {
    SetupParams::HOTP(p) => { /* p: hotp::Params */ }
    _ => unreachable!("we know it's HOTP here"),
}

let output = factor.factor_type.setup_output(key.into())?;
let hotp_out: <hotp::HOTP as FactorSetup>::Output = output.try_into().unwrap();

Each factor has to define the structs for these enums, and these enums will be used to implement methods on FactorType struct.

impl FactorType {
  pub fn bytes(&self) -> Vec<u8> {}
  pub fn setup_params(&self, key) -> SetupParams {}
  pub fn setup_output(&self, key) -> SetupOutput {}
  pub fn derive_params(&self, key) -> DeriveParams {}
  pub fn derive_output(&self, key) -> DeriveOutput {}
}

Proposal

• Remove trait object and move to individual enums that encapuslate types from each factor.
• Use declarative macros to define these types.

define_factors! {
    Password(password::Password),
    HOTP(hotp::HOTP),
    Question(question::Question),
    UUID(uuid::UUIDFactor),
    HmacSha1(hmacsha1::HmacSha1),
    TOTP(totp::TOTP),
    OOBA(ooba::Ooba),
    Passkey(passkey::Passkey),
    Stack(stack::Stack),
}

[lonerapier]

@Autoparallel need your opinion on this. My questions:

• Is previous api more preferable over proposed one?
• Is there a better solution for this that avoids dynamic dispatch and not convolute the type system? because in proposed solution, every factor will have to define four extra types (SetupParams, SetupOutput, DeriveParams, DeriveOutput). We can definitely reduce this to two as setup and derive will have same params and output types.
• Is macros the correct solution here?

[Autoparallel]

Do you need

  type Params: Serialize + DeserializeOwned + std::fmt::Debug + Default;
  type Output: Serialize + DeserializeOwned + std::fmt::Debug + Default;

and not just

  type Params: Serialize + for<'de>Deserialize<'de> + std::fmt::Debug + Default;
  type Output: Serialize + for<'de>Deserialize<'de> + std::fmt::Debug + Default;

?

[Autoparallel]

fwiw you could also:

  fn params(&self, key: impl Into<Key>) -> MFKDF2Result<Self::Params> {
    Ok(serde_json::from_value(serde_json::json!({}))?)
  }

[Autoparallel]

The macro is an interesting idea.

Could the trait itself not just be generic?

pub trait ParamsType {
    type Params: Serialize + for<'de>Deserialize<'de> + std::fmt::Debug + Default;
    fn params() -> T;
}

pub trait OutputType {
   fn Output: Serialize + for<'de>Deserialize<'de> + std::fmt::Debug + Default; 
   fn output() -> T;
}

pub trait FactorSetup<T>: Send + Sync + std::fmt::Debug 
where
    T: ParamsType + OutputType
{
  fn bytes(&self) -> Vec<u8>;
  fn params(&self, key: Key) -> MFKDF2Result<<T as ParamsType>::Params> {
    Ok(serde_json::from_value(serde_json::json!({}))?)
  }
  fn output(&self, key: Key) -> <T as OutputType>::Output { Self::Output::default() }
}

then you have FactorSetup<Password> for example