feature/SDK-59

[sanderPostma]

No description provided.

[ronal2do]

I only miss to have unit tests for this module, to verify regressions

[nklomp]

Has this change been verified to be correct to begin with in the specs @sanderPostma? The fact the underlying Veramo functions allow for it doesn't mean this change is correct. I cannot remember a need ever to not have both.

Actually I do not even want this change, as it would promote not binding a domain to a proof

[nklomp]

Needs to be checked against spec, but this change is dangerous in itself as it would start promoting not using domain bindings for proofs

[sanderPostma]

!challenge && !domain

@nklomp
what would be the alternative?
!challenge || !domain instead of !challenge && !domain
?
Or throw an error when we have a challenge but not a domain?

Because with did:web we have a domain but not a challenge.
We could add a challenge, but it's not mandatory by spec, so we need to be able to deal with it

The domain property within a proof is optional. It specifies the operational sphere or context in which the proof is intended to be valid, often used to prevent replay attacks by restricting the proof's validity to a particular domain.
Omitting the domain property can have implications:
Increased Risk of Replay Attacks: Without a specified domain, the proof might be accepted in unintended contexts, potentially allowing malicious actors to reuse the proof elsewhere.
Broader Validity Scope: The proof may be considered valid across multiple domains, which might not align with the issuer's intent.

[nklomp]

We should make sure that our RP asks for a challenge to begin with. See the warning. That is the reason why I am hesitant to even allow it. Since it is in the spec I guess we should, but IMO I rather have an option to allow for the omission of a challenge, that defaults to false. I rather run into issues with certain RPs that do not require it, as they really should use it. So first and foremost we should fix it in our RP.