The same connection.peerIdentity promise can be resolved multiple times

[dontcallmedom]

Raised by @soareschen

In 9.4 Verifying Identity Assertions:

10. The RTCPeerConnection resolves the peerIdentity attribute with a new instance of RTCIdentityAssertion that includes the IdP domain and peer identity.

Identity assertion validation is performed every time setRemoteDescription() is called with SDP containing a=identity line. Whether the validation succeed or failed, 9.4 attempts to resolve connection.peerIdentity promise without considering if it has previously been resolved.

The only time the peerIdentity promise is replaced is when identity validation fails and there is no a target peer identity. So what happen in other permutations of promise fulfillment/rejection? e.g.:

const pc = new RTCPeerConnection({ peerIdentity });
pc.setRemoteDescription(sdpWithInvalidAssertion)
.catch(() => pc.setRemoteDescription(sdpWithValidAssertion));

const pc = new RTCPeerConnection({ peerIdentity });
pc.setRemoteDescription(sdpWithValidAssertion)
.then(() => pc.setRemoteDescription(sdpWithInvalidAssertion));

[dontcallmedom]

Comment by @martinthomson

If there is a target peer identity or the promise resolves (those are the two cases), then the promise should be left alone if it is already resolved. There is no new information to provide. In the former case, setRemoteDescription() will fail; in the latter, the value would only resolve to the same value (but it would create a separate promise with a different resolution time).

[dontcallmedom]

Comment by @soareschen

I think the wordings can be more explicit instead of relying on the implicit behavior of promises. e.g. "If connection.peerIdentity is not yet resolved, resolve it with ...".

Still, it doesn't solve the first case where peer identity is set but first call to identity validation fails.

// target peer identity is established during construction
const pc = new RTCPeerConnection({ peerIdentity: ... }) 
pc.setRemoteDescription(sdpWithInvalidAssertion) // first SRD fails
.catch(() => pc.setRemoteDescription(sdpWithValidAssertion)) // second SRD succeeds
.then(pc.peerIdentity)
.catch(err => ...); // pc.peerIdentity stays rejected forever

Relevant paragraphs:

If identity validation fails, the peerIdentity promise is rejected with a newly created OperationError.

If identity validation fails and there is no a target peer identity, the value of the peerIdentity MUST be set to a new, unresolved promise instance.

So if validation fails and there is a target peer identity, the rejected peerIdentity promise is never replaced.

Probably a better way to cover all failure cases is:

• If identity validation fails and the connection.peerIdentity promise is not yet resolved, reject it with a newly created OperationError and set connection.peerIdentity to a new, unresolved promise instance.

[dontcallmedom]

Comment by @martinthomson

Hmm, that changes things subtly. It means that if you set a target peer identity, then you can keep trying to set a remote description. I can't see a problem with that right now.

[dontcallmedom]

Comment by @soareschen

Does the original definition not allow setting remote description multiple times if target peer identity is set?

[dontcallmedom]

Comment by @martinthomson

I just looked at the spec and while my understanding was that resolving or rejecting a promise was a noop if it was settled, that is only true for the resolution and rejection functions. So I've refactored this code a little.