Skip to content

Commit

Permalink
Avoid X509IdentityProvider overloaded methods
Browse files Browse the repository at this point in the history
While using a `X509IdentityProvider` in a generic context, I
discovered that it has a few methods which I found confusing:

- X509IdentityProvider::identity takes 1 argument, but
  IdentityProvider::identity takes 2 arguments!

- X509IdentityProvider::valid_successor takes 2 arguments,
  but IdentityProvider::valid_successor takes 3 arguments!

I ended up solving this with the fully-qualified `<Type as
Trait>::method` syntax, but I think the situation can be avoided all
together by just implementing the trait directly.
  • Loading branch information
mgeisler committed Jan 12, 2025
1 parent 66d6717 commit 71bae7d
Showing 1 changed file with 27 additions and 52 deletions.
79 changes: 27 additions & 52 deletions mls-rs-identity-x509/src/provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,24 @@ where
validator,
}
}
}

#[cfg_attr(not(mls_build_async), maybe_async::must_be_sync)]
#[cfg_attr(mls_build_async, maybe_async::must_be_async)]
impl<IE, V> IdentityProvider for X509IdentityProvider<IE, V>
where
IE: X509IdentityExtractor + Send + Sync,
V: X509CredentialValidator + Send + Sync,
{
type Error = X509IdentityError;

/// Determine if a certificate is valid based on the behavior of the
/// underlying validator provided.
pub fn validate(
async fn validate_member(
&self,
signing_identity: &mls_rs_core::identity::SigningIdentity,
timestamp: Option<mls_rs_core::time::MlsTime>,
_context: MemberValidationContext<'_>,
) -> Result<(), X509IdentityError> {
let chain = credential_to_chain(&signing_identity.credential)?;

Expand All @@ -101,9 +112,10 @@ where

/// Produce a unique identity value to represent the entity controlling a
/// certificate credential within an MLS group.
pub fn identity(
async fn identity(
&self,
signing_id: &mls_rs_core::identity::SigningIdentity,
_extensions: &ExtensionList,
) -> Result<Vec<u8>, X509IdentityError> {
self.identity_extractor
.identity(&credential_to_chain(&signing_id.credential)?)
Expand All @@ -113,10 +125,11 @@ where
/// Determine if `successor` is controlled by the same entity as
/// `predecessor` based on the behavior of the underlying identity
/// extractor provided.
pub fn valid_successor(
async fn valid_successor(
&self,
predecessor: &mls_rs_core::identity::SigningIdentity,
successor: &mls_rs_core::identity::SigningIdentity,
_extensions: &ExtensionList,
) -> Result<bool, X509IdentityError> {
self.identity_extractor
.valid_successor(
Expand All @@ -126,65 +139,27 @@ where
.map_err(|e| X509IdentityError::IdentityExtractorError(e.into_any_error()))
}

/// Supported credential types.
///
/// Only [`CredentialType::X509`] is supported.
pub fn supported_types(&self) -> Vec<mls_rs_core::identity::CredentialType> {
vec![CredentialType::X509]
}
}

#[cfg_attr(not(mls_build_async), maybe_async::must_be_sync)]
#[cfg_attr(mls_build_async, maybe_async::must_be_async)]
impl<IE, V> IdentityProvider for X509IdentityProvider<IE, V>
where
IE: X509IdentityExtractor + Send + Sync,
V: X509CredentialValidator + Send + Sync,
{
type Error = X509IdentityError;

async fn validate_member(
&self,
signing_identity: &mls_rs_core::identity::SigningIdentity,
timestamp: Option<MlsTime>,
_: MemberValidationContext<'_>,
) -> Result<(), Self::Error> {
self.validate(signing_identity, timestamp)
}

async fn validate_external_sender(
&self,
signing_identity: &mls_rs_core::identity::SigningIdentity,
timestamp: Option<MlsTime>,
_extensions: Option<&ExtensionList>,
) -> Result<(), Self::Error> {
self.validate(signing_identity, timestamp)
self.validate_member(signing_identity, timestamp, MemberValidationContext::None)
}

async fn identity(
&self,
signing_id: &mls_rs_core::identity::SigningIdentity,
_extensions: &ExtensionList,
) -> Result<Vec<u8>, Self::Error> {
self.identity(signing_id)
}

async fn valid_successor(
&self,
predecessor: &mls_rs_core::identity::SigningIdentity,
successor: &mls_rs_core::identity::SigningIdentity,
_extensions: &ExtensionList,
) -> Result<bool, Self::Error> {
self.valid_successor(predecessor, successor)
}

fn supported_types(&self) -> Vec<CredentialType> {
self.supported_types()
/// Supported credential types.
///
/// Only [`CredentialType::X509`] is supported.
async fn supported_types(&self) -> Vec<CredentialType> {
vec![CredentialType::X509]
}
}

#[cfg(all(test, feature = "std"))]
mod tests {
use super::*;
use mls_rs_core::identity::MemberValidationContext::None as NO_CONTEXT;
use mls_rs_core::{crypto::SignaturePublicKey, identity::CredentialType, time::MlsTime};

use crate::{
Expand Down Expand Up @@ -249,7 +224,7 @@ mod tests {
});

test_provider
.validate(&test_signing_identity, Some(test_timestamp))
.validate_member(&test_signing_identity, Some(test_timestamp), NO_CONTEXT)
.unwrap();
}

Expand All @@ -266,7 +241,7 @@ mod tests {
});

assert_matches!(
test_provider.validate(&test_signing_identity, None),
test_provider.validate_member(&test_signing_identity, None, NO_CONTEXT),
Err(X509IdentityError::SignatureKeyMismatch)
);
}
Expand All @@ -280,7 +255,7 @@ mod tests {
});

assert_matches!(
test_provider.validate(&test_signing_identity(), None),
test_provider.validate_member(&test_signing_identity(), None, NO_CONTEXT),
Err(X509IdentityError::X509ValidationError(_))
)
}
Expand Down

0 comments on commit 71bae7d

Please sign in to comment.