[FS-919] Support Basic Processing of External Commits

[mdimjasevic]

The PR introduces basic support for external commits in MLS. Note that this does not resubmit external pending proposals submitted by the backend; this is to be done in https://wearezeta.atlassian.net/browse/FS-920.

Tracked by https://wearezeta.atlassian.net/browse/FS-919.

Checklist

• Add a new entry in an appropriate subdirectory of changelog.d
• Read and follow the PR guidelines

[elland]

Would moving pure outside of the case match work?

[mdimjasevic]

I don't think it would. At first it looked weird to me as well. All the branches of the bigger pattern-match have to have the same type. As far as my understanding goes, this block is written this way so it can be executed later, but then I suppose the same could have been achieved if at the top we had a let binding instead of monadic binding and using one level of monads less.

[elland]

I'm fine either way, more of a nitpick :D

[pcapriotti]

This Maybe () is pretty weird. Can we use Any instead?

[mdimjasevic]

Sure, done in commit 20162df. But aren't these two types isomorphic as well, i.e., don't the same arguments you had yesterday in the call apply to Any as well?

[pcapriotti]

What arguments? My only argument here is that Maybe () is a circuitous way to express a boolean, while Any is more direct. Using Bool there is also fine. It's a minor thing, anyway.

[mdimjasevic]

Sure, I see your point.

[pcapriotti]

Shouldn't you validate the key package first? See for example applyProposal.

[mdimjasevic]

Your observation has seemed valid and with that in mind, I looked at other cases when a key package is updated or mapped and then some of them seemed equally wrong because they also just call updateKeyPackageMapping. For example, in the case of an existing member updating their key package via the update path we blindly accept the new key package without checking if it is for the same protocol, same cipher suite, etc., but this seems wrong, doesn't it?

I realized updateKeyPackageMapping does validation (the ciphersuite, identity matching, signature scheme, etc.) if the old key package is non-existent (i.e., we pass in Nothing to updateKeyPackageMapping), by calling an internal endpoint in Brig, for which there's a handler upsertKeyPackage.

So before making a call to updateKeyPackageMapping in the case of a NewMemberSender, I just added a call to addKeyPackageRef, which calls upsertKeyPackage in Brig and validates it before inserting into the database.

Does that seem OK?

[pcapriotti]

As long as validation is happening.