Generalise CredentialValidator, PresentationValidator to support arbitrary DID Documents

[cycraig]

Description of change

Abstracts the CredentialValidator and PresentationValidator interfaces to support any DID Document that implements the Document trait (added by this PR).

Added

• Add Document trait.
• Add sealed ValidatorDocument trait.
• Add CredentialValidator::extract_issuer.
• Add PresentationValidator::extract_holder.
• Add Document::resolve_service.
• Move IotaDocument::(un)revoke_credentials logic to CoreDocument.

Changed

• Change CredentialValidator, PresentationValidator function parameters from AsRef<IotaDocument> to ValidatorDocument.
• Move CredentialValidator, PresentationValidator from identity_iota_client to identity_credential under a new validator feature.
• Change (un)revoke_credentials parameter from &str to Into<DIDUrlQuery>.

Note that using Document and ValidatorDocument mean ResolvedIotaDocument no longer works as an input parameter; it previously worked with AsRef<IotaDocument>.

Approach Explanation

The abstraction is achieved through dynamic dispatch, where concrete Document implementations are cast to a sealed, object-safe trait ValidatorDocument. This is done to work around limitations of Rust when it comes to collections of different structs, which we use in several function parameters for issuer DID Documents.

E.g.

fn verify_signature<T: Serialize, D: AsRef<IotaDocument>>(credential: &Credential<T>, trusted_issuers: &[D], options: &VerifierOptions) -> ValidationUnitResult

is now

fn verify_signature<T: Serialize>(credential: &Credential<T>, trusted_issuers: &[&dyn ValidatorDocument], options: &VerifierOptions) -> ValidationUnitResult

The issuers of credentials in a presentation could all use different DID Methods (did:iota, did:key, etc.), which we want to support. Monomorphisation means we cannot use D: Document, since trusted_issuers: &[D] would be a slice of the same concrete type. Using dynamic dispatch with trusted_issuers: &[&dyn Document] or trusted_issuers: &[Box<dyn Document>] is not possible because the trait needs to be object-safe. Document itself cannot be made object-safe (without significant loss of functionality) due to generic type parameters in structs like Service<D,U>, and VerificationMethod<D, V>. This is why the ValidatorDocument trait is introduced, to provide an object-safe wrapper trait with a blanket implementation for Document. Note that associated types are used for Document rather than generics, otherwise the compiler complains about unconstrained type parameters in the blanket implementation.

Another approach would have been to use CoreDocument, but even that has limitations due to generics. I.e. all DID Documents would need to be converted to CoreDocument<CoreDID, Object, Object, Object>. That would be a less ergonomic interface and likely have more overhead due to the conversion to CoreDocument require transforming the types with .map or going through serde.

Unfortunately, the Document/ValidatorDocument approach chosen still requires some work when passing in collections due to explicit casts to &dyn Document being necessary. E.g.

let resolved_issuers: Vec<ResolvedIotaDocument> = resolver.resolve_presentation_issuers(presentation).await?;
let issuers: Vec<&dyn ValidatorDocument> = resolved_issuers
  .iter()
  .map(|resolved| &resolved.document as &dyn ValidatorDocument)
  .collect();
PresentationValidator::validate(presentation, holder, issuers.as_slice(), options, fail_fast)

This can be slightly shortened with the provided ValidatorDocument::as_validator function:

let issuers: Vec<&dyn ValidatorDocument> = resolved_issuers
  .iter()
  .map(|resolved| resolved.document.as_validator())
  .collect();

Not ideal but I rate this is still better, ergonomically and performance-wise, than converting to CoreDocument. To be clear: I would greatly welcome any alternative that could eliminate this clunkiness.

Open Questions

1. Should Document be extended with more functions, like accessing method or service collections directly (service(), authentication(), and so on)? This would make it more difficult for downstream implementers not building on CoreDocument and its underlying structs, like OrderedSet. But the methods return Service<D, U> and VerificationMethod<D, V>, so it would be very difficult if not using CoreDocument anyway.
2. Should we split sign_data, and verify_data out of Document into one or more separate traits? E.g. SignerDocument, VerifierDocument? Named to avoid existing Signer and Verifier traits in identity_core. We could make a blanket implementation for VerifierDocument using Document, but that may make custom signature schemes more difficult to support in the future.
3. Should we add a DocumentMut trait for methods like id_mut, controller_mut, insert_method, insert_service etc.?
4. Do we need blanket implementations of Document for &T, &mut T, Box<T> etc.?
5. Should we keep the mixed use of generics and dynamic traits in the validator functions? I.e. both D: ValidatorDocument in addition to &dyn ValidatorDocument? The benefit is that developers can choose either static or dynamic dispatch but only with generic type parameter functions (D: ValidatorDocument).
6. There is still a potential problem with the generic type parameter for the properties in Credential<T>, since it means all credentials in a Presentation must have the same properties type T (though one could always circumvent that restriction by deserializing to Credential<Object>). I think it's fine to leave as-is for now since it has a relatively simple workaround.
7. The Wasm bindings are left using Document | ResolvedDocument for now, since currently every relevant type is assumed to be specific to IOTA DID. The generalisation of the Wasm bindings is left for a future PR, but it's undecided what approach would be best there. My ideas are either:
   • Using Typescript union types e.g. IotaDocument | KeyDocument | IotanxDocument etc. and deserialising to CoreDocument internally.
   • Create a Typescript interface equivalent for the Document trait, then Document can be implemented for said TypescriptDocumentInterface which would call resolve_service etc. via Javascript. Unclear whether that performance impact would be worse than deserializing to CoreDocument via JSON.

Links to any relevant issues

Task of #908.

Type of change

• Bug fix (a non-breaking change which fixes an issue)
• Enhancement (a non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation Fix

How the change has been tested

Added new unit tests in resolver.rs, also the Rust compiler would shout at me if this didn't work.

Tests passed with and without default-features, i.e.

cargo test --all
cargo test --all-features --all
cargo test --no-default-features --all

Wasm bindings are mostly unchanged but all tests pass there too.

Change checklist

• I have followed the contribution guidelines for this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[PhilippGackstatter]

I like the approach in general.

I'm mainly wondering about the need for generic parameters which are practically always (?) Object.

In regards to the open questions:

1, 3, 4: Not sure if we can answer these questions well now. Can't we add them on-demand, when we encounter the need? In general they sound useful, though.

2: Didn't have time to look into this more yet, apologies.

5: Yes, I think we should use D: ValidatorDocument wherever possible.

7: Yeah seems fine to leave it as-is for now. The first option would be my preference, I think.

[PhilippGackstatter]

Is there actually a use case where we or our users use a generic parameter other than Object for Service or VerificationMethod? It seems Iota{Service, Document, VerificationMethod} all work fine for us with Object being fixed, so I'm wondering whether the core types need to be that flexible and in turn this trait and everything building on top.
I suppose if we want to implement option 3 from your proposed serde tagged enum approach for different service types, then we would likely want it, at least for Service. But if we don't go with that, I'm wondering whether the complexity that this generates in the Document trait and elsewhere is warranted.

This would also "address" open question 6.

[cycraig]

Yes, it was the previous design direction and makes it convenient to add extra fields to Service<D, U> or VerificationMethod<D, U> by injecting an arbitrary struct for U, and convert between them cheaply without duplicating field definitions.
Note that even if we eliminate U we still rely on the D: DID generic parameter everywhere, though we might want to reconsider it and go with CoreDID to enable storing identifiers from other methods in our structs for whatever reason. I still don't really comprehend why the fully-qualified DID Url is required for the id fields instead of just a fragment in the specification.

The tagged enum approach still has its drawbacks, I would actually consider a Service trait to be a potentially better avenue now.

Specifically regarding open question 6: I would be fine removing T from Credential<T> and Presentation<T>, but we need to come up with a canonical way for extending credentials and presentation contents easily.

Overall this is just something to consider during the bigger refactors, will leave the generics as-is for now.

[cycraig]

1, 3, 4: Not sure if we can answer these questions well now. Can't we add them on-demand, when we encounter the need? In general they sound useful, though.

I think it would be good to decide so we can utilise it for generalising the Wasm bindings, if we go with interface/trait-based approaches there. Perhaps this is better discussed when it becomes necessary though.

[olivereanderson]

Please fix doc generation on this branch before merging.

cargo doc --open generates loads of warnings when called from the identity_credential directory.

[cycraig]

@eike-hass what is needed to merge #925 to catch broken documentation links in CI? Edit: ah, Rust docs not wiki documentation. We should probably add that to CI too then.

Edit: opened #937 to catch this in the future.

[olivereanderson]

The crate level error in identity_credential is documented as follows:

/// This type represents all possible errors that can occur in the library.

With the changes brought here this is no longer correct. Hence the documentation either needs to be reworded, or the error needs to be updated to include variants that wrap the various validation errors, or alternatively one can remove the crate level error. I am fine with either option as long as something gets done to avoid misleading documentation.

[olivereanderson]

With regards to the open questions I would agree with @PhilippGackstatter that it would be preferable to wait with answering 1,3,4. With regards to 2. I would say yes that would be useful, but would not go with a blanket implementation. In fact if we could get away with using a VerifierDocument trait instead of Document in this PR I would strongly prefer that (perhaps especially keeping in mind answering 1,3 and 4 at a later point if possible).

I guess 5. is less relevant now (after re-introducing generic parameters in the validation methods)?

Number 6. can be left as is for now and 7. can probably be left for later.

[cycraig]

get away with using a VerifierDocument trait instead of Document in this PR I would strongly prefer that

That would mean each DID Document method would have to implement ValidatorDocument instead of a generic Document. Document is intended to be the way forward for handling different DID methods in this library.

[PhilippGackstatter]

In fact if we could get away with using a VerifierDocument trait instead of Document in this PR I would strongly prefer that

I would also prefer to have just one trait, at least eventually. But considering this is the first attempt at such an abstraction, I find it hard to judge where we might go with the trait or which approach is really better. Going with both for now might allow for easier exploration for which approach is better suited, so I'm still fine with merging it as-is.

[olivereanderson]

This looks good, thanks for the changes.

The Document trait introduced here will almost certainly need to be revisited before the next release, but that is OK.