Skip to content

Commit

Permalink
Merge pull request #22 from xmtp/cv/mutable-metadata
Browse files Browse the repository at this point in the history
Remove test only distinction for group_context_ext_proposal
  • Loading branch information
cameronvoell authored Apr 10, 2024
2 parents 4eee1fc + ad48359 commit 9288932
Show file tree
Hide file tree
Showing 9 changed files with 301 additions and 58 deletions.
95 changes: 59 additions & 36 deletions .github/workflows/interop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,39 +26,39 @@ jobs:

# ---------------------------------------------------------------------------------------

- name: MLS++ | Checkout
run: |
git clone https://github.com/cisco/mlspp.git
cd mlspp
git checkout 623acd0839d1117e8665b6bd52eecad1ce05438d
- name: MLS++ | Install dependencies | 1/2
uses: lukka/run-vcpkg@v11
with:
vcpkgDirectory: "${{ github.workspace }}/vcpkg"
vcpkgGitCommitId: "70992f64912b9ab0e60e915ab7421faa197524b7"
vcpkgJsonGlob: "mlspp/vcpkg.json"
runVcpkgInstall: true

- name: MLS++ | Install dependencies | 2/2
uses: lukka/run-vcpkg@v11
with:
vcpkgDirectory: "${{ github.workspace }}/vcpkg"
vcpkgGitCommitId: "70992f64912b9ab0e60e915ab7421faa197524b7"
vcpkgJsonGlob: "mlspp/cmd/interop/vcpkg.json"
runVcpkgInstall: true

- name: MLS++ | Build | 1/2
working-directory: mlspp
run: |
cmake . -DCMAKE_TOOLCHAIN_FILE=${{ github.workspace }}/vcpkg/scripts/buildsystems/vcpkg.cmake
make
- name: MLS++ | Build | 2/2
working-directory: mlspp/cmd/interop
run: |
cmake . -DCMAKE_TOOLCHAIN_FILE=${{ github.workspace }}/vcpkg/scripts/buildsystems/vcpkg.cmake
make
#- name: MLS++ | Checkout
# run: |
# git clone https://github.com/cisco/mlspp.git
# cd mlspp
# git checkout 623acd0839d1117e8665b6bd52eecad1ce05438d

#- name: MLS++ | Install dependencies | 1/2
# uses: lukka/run-vcpkg@v11
# with:
# vcpkgDirectory: "${{ github.workspace }}/vcpkg"
# vcpkgGitCommitId: "70992f64912b9ab0e60e915ab7421faa197524b7"
# vcpkgJsonGlob: "mlspp/vcpkg.json"
# runVcpkgInstall: true

#- name: MLS++ | Install dependencies | 2/2
# uses: lukka/run-vcpkg@v11
# with:
# vcpkgDirectory: "${{ github.workspace }}/vcpkg"
# vcpkgGitCommitId: "70992f64912b9ab0e60e915ab7421faa197524b7"
# vcpkgJsonGlob: "mlspp/cmd/interop/vcpkg.json"
# runVcpkgInstall: true

#- name: MLS++ | Build | 1/2
# working-directory: mlspp
# run: |
# cmake . -DCMAKE_TOOLCHAIN_FILE=${{ github.workspace }}/vcpkg/scripts/buildsystems/vcpkg.cmake
# make

#- name: MLS++ | Build | 2/2
# working-directory: mlspp/cmd/interop
# run: |
# cmake . -DCMAKE_TOOLCHAIN_FILE=${{ github.workspace }}/vcpkg/scripts/buildsystems/vcpkg.cmake
# make

# ---------------------------------------------------------------------------------------

Expand Down Expand Up @@ -89,10 +89,32 @@ jobs:
# ---------------------------------------------------------------------------------------

- name: Test interoperability
# - name: Test interoperability
# run: |
# ./target/debug/interop_client&
# ./mlspp/cmd/interop/mlspp_client -live 12345&
#
# cd mls-implementations/interop
# # TODO(#1238):
# # * Add `commit.json` as soon as group context extensions proposals are supported.
# # Note: It's also possible to remove GCE proposals by hand from `commit.json`.
# # But let's not do this in CI for now and hope that it isn't needed anymore soon.
# for scenario in {welcome_join.json,external_join.json,application.json};
# do
# echo Running configs/$scenario
# errors=$(./test-runner/test-runner -fail-fast -client localhost:50051 -client localhost:12345 -config=configs/$scenario | grep error | wc -l)
# if [ "$errors" = "0" ];
# then
# echo "Success";
# else
# echo "Failed";
# exit 1;
# fi
# done

- name: Test interoperability (OpenMLS only)
run: |
./target/debug/interop_client&
./mlspp/cmd/interop/mlspp_client -live 12345&
cd mls-implementations/interop
# TODO(#1238):
Expand All @@ -102,7 +124,7 @@ jobs:
for scenario in {welcome_join.json,external_join.json,application.json};
do
echo Running configs/$scenario
errors=$(./test-runner/test-runner -fail-fast -client localhost:50051 -client localhost:12345 -config=configs/$scenario | grep error | wc -l)
errors=$(./test-runner/test-runner -fail-fast -client localhost:50051 -config=configs/$scenario | grep error | wc -l)
if [ "$errors" = "0" ];
then
echo "Success";
Expand All @@ -111,3 +133,4 @@ jobs:
exit 1;
fi
done
3 changes: 2 additions & 1 deletion openmls/src/framing/mls_content.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,8 @@ pub(crate) fn framed_content_tbs_serialized<'context, W: Write>(
// NewMemberCommit.
written += match serialized_context.into() {
Some(context) if matches!(sender, Sender::Member(_) | Sender::NewMemberCommit) => {
writer.write(context)?
writer.write_all(context)?;
context.len()
}
_ => 0,
};
Expand Down
13 changes: 7 additions & 6 deletions openmls/src/group/core_group/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1043,17 +1043,14 @@ impl CoreGroup {
group_info: group_info.filter(|_| self.use_ratchet_tree_extension),
})
}
}

// Test functions
#[cfg(test)]
impl CoreGroup {
pub(crate) fn create_group_context_ext_proposal(
/// Create a new group context extension proposal
pub(crate) fn create_group_context_ext_proposal<KeyStore: OpenMlsKeyStore>(
&self,
framing_parameters: FramingParameters,
extensions: Extensions,
signer: &impl Signer,
) -> Result<AuthenticatedContent, CreateGroupContextExtProposalError> {
) -> Result<AuthenticatedContent, CreateGroupContextExtProposalError<KeyStore::Error>> {
// Ensure that the group supports all the extensions that are wanted.
let required_extension = extensions
.iter()
Expand Down Expand Up @@ -1082,7 +1079,11 @@ impl CoreGroup {
)
.map_err(|e| e.into())
}
}

// Test functions
#[cfg(test)]
impl CoreGroup {
pub(crate) fn use_ratchet_tree_extension(&self) -> bool {
self.use_ratchet_tree_extension
}
Expand Down
8 changes: 4 additions & 4 deletions openmls/src/group/core_group/test_proposals.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use openmls_rust_crypto::OpenMlsRustCrypto;
use openmls_traits::{types::Ciphersuite, OpenMlsProvider};
use openmls_traits::{key_store::OpenMlsKeyStore, types::Ciphersuite, OpenMlsProvider};

use super::CoreGroup;
use crate::{
Expand Down Expand Up @@ -543,9 +543,9 @@ fn test_group_context_extension_proposal_fails(
}

#[apply(ciphersuites_and_providers)]
fn test_group_context_extension_proposal(
fn test_group_context_extension_proposal<KeyStore: OpenMlsKeyStore>(
ciphersuite: Ciphersuite,
provider: &impl OpenMlsProvider,
provider: &impl OpenMlsProvider<KeyStoreProvider = KeyStore>,
) {
// Basic group setup.
let group_aad = b"Alice's test group";
Expand Down Expand Up @@ -617,7 +617,7 @@ fn test_group_context_extension_proposal(
&[CredentialType::Basic],
));
let gce_proposal = alice_group
.create_group_context_ext_proposal(
.create_group_context_ext_proposal::<KeyStore>(
framing_parameters,
Extensions::single(required_application_id),
&alice_signer,
Expand Down
8 changes: 7 additions & 1 deletion openmls/src/group/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,7 @@ pub(crate) enum CoreGroupParseMessageError {

/// Create group context ext proposal error
#[derive(Error, Debug, PartialEq, Clone)]
pub enum CreateGroupContextExtProposalError {
pub enum CreateGroupContextExtProposalError<KeyStoreError> {
/// See [`LibraryError`] for more details.
#[error(transparent)]
LibraryError(#[from] LibraryError),
Expand All @@ -507,6 +507,12 @@ pub enum CreateGroupContextExtProposalError {
/// See [`LeafNodeValidationError`] for more details.
#[error(transparent)]
LeafNodeValidation(#[from] LeafNodeValidationError),
/// See [`MlsGroupStateError`] for more details.
#[error(transparent)]
MlsGroupStateError(#[from] MlsGroupStateError),
/// See [`CreateCommitError`] for more details.
#[error(transparent)]
CreateCommitError(#[from] CreateCommitError<KeyStoreError>),
}

/// Error merging a commit.
Expand Down
2 changes: 1 addition & 1 deletion openmls/src/group/mls_group/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,5 +322,5 @@ pub enum ProposalError<KeyStoreError> {
ValidationError(#[from] ValidationError),
/// See [`CreateGroupContextExtProposalError`] for more details.
#[error(transparent)]
CreateGroupContextExtProposalError(#[from] CreateGroupContextExtProposalError),
CreateGroupContextExtProposalError(#[from] CreateGroupContextExtProposalError<KeyStoreError>),
}
64 changes: 57 additions & 7 deletions openmls/src/group/mls_group/proposal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ use openmls_traits::{
};

use super::{
core_group::create_commit_params::CreateCommitParams,
errors::{ProposalError, ProposeAddMemberError, ProposeRemoveMemberError},
MlsGroup,
CreateGroupContextExtProposalError, GroupContextExtensionProposal, MlsGroup, MlsGroupState,
PendingCommitState, Proposal,
};
use crate::{
binary_tree::LeafNodeIndex,
Expand All @@ -14,7 +16,7 @@ use crate::{
framing::MlsMessageOut,
group::{errors::CreateAddProposalError, GroupId, QueuedProposal},
key_packages::KeyPackage,
messages::proposals::ProposalOrRefType,
messages::{group_info::GroupInfo, proposals::ProposalOrRefType},
prelude::LibraryError,
schedule::PreSharedKeyId,
treesync::LeafNode,
Expand Down Expand Up @@ -319,16 +321,19 @@ impl MlsGroup {
}
}

#[cfg(test)]
pub fn propose_group_context_extensions(
/// Creates a proposals with a new set of `extensions` for the group context.
///
/// Returns an error when the group does not support all the required capabilities
/// in the new `extensions`.
pub fn propose_group_context_extensions<KeyStore: OpenMlsKeyStore>(
&mut self,
provider: &impl OpenMlsProvider,
provider: &impl OpenMlsProvider<KeyStoreProvider = KeyStore>,
extensions: Extensions,
signer: &impl Signer,
) -> Result<(MlsMessageOut, ProposalRef), ProposalError<()>> {
) -> Result<(MlsMessageOut, ProposalRef), ProposalError<KeyStore::Error>> {
self.is_operational()?;

let proposal = self.group.create_group_context_ext_proposal(
let proposal = self.group.create_group_context_ext_proposal::<KeyStore>(
self.framing_parameters(),
extensions,
signer,
Expand All @@ -350,4 +355,49 @@ impl MlsGroup {

Ok((mls_message, proposal_ref))
}

/// Updates group context extensions
///
/// Returns an error when the group does not support all the required capabilities
/// in the new `extensions`.
#[allow(clippy::type_complexity)]
pub fn update_group_context_extensions<KeyStore: OpenMlsKeyStore>(
&mut self,
provider: &impl OpenMlsProvider<KeyStoreProvider = KeyStore>,
extensions: Extensions,
signer: &impl Signer,
) -> Result<
(MlsMessageOut, Option<MlsMessageOut>, Option<GroupInfo>),
CreateGroupContextExtProposalError<KeyStore::Error>,
> {
self.is_operational()?;

// Create group context extension proposals
let inline_proposals = vec![Proposal::GroupContextExtensions(
GroupContextExtensionProposal { extensions },
)];

let params = CreateCommitParams::builder()
.framing_parameters(self.framing_parameters())
.proposal_store(&self.proposal_store)
.inline_proposals(inline_proposals)
.build();
let create_commit_result = self.group.create_commit(params, provider, signer)?;

let mls_messages = self.content_to_mls_message(create_commit_result.commit, provider)?;
self.group_state = MlsGroupState::PendingCommit(Box::new(PendingCommitState::Member(
create_commit_result.staged_commit,
)));

// Since the state of the group might be changed, arm the state flag
self.flag_state_change();

Ok((
mls_messages,
create_commit_result
.welcome_option
.map(|w| MlsMessageOut::from_welcome(w, self.group.version())),
create_commit_result.group_info,
))
}
}
Loading

0 comments on commit 9288932

Please sign in to comment.