Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clientExtensionResults to be optional #109

Closed
Mikescops opened this issue Mar 15, 2021 · 3 comments
Closed

clientExtensionResults to be optional #109

Mikescops opened this issue Mar 15, 2021 · 3 comments
Labels
package:browser @simplewebauthn/browser package:server @simplewebauthn/server

Comments

@Mikescops
Copy link
Contributor

Hello,

In the following types, it would be nice to have clientExtensionResults optional as it is not used in the verify attestation nor assertion:

export interface AttestationCredentialJSON extends Omit<AttestationCredential, 'response' | 'rawId' | 'getClientExtensionResults'> {
    rawId: Base64URLString;
    response: AuthenticatorAttestationResponseJSON;
    clientExtensionResults: AuthenticationExtensionsClientOutputs;
    transports?: AuthenticatorTransport[];
}

export interface AssertionCredentialJSON extends Omit<AssertionCredential, 'response' | 'rawId' | 'getClientExtensionResults'> {
    rawId: Base64URLString;
    response: AuthenticatorAssertionResponseJSON;
    clientExtensionResults: AuthenticationExtensionsClientOutputs;
}
@MasterKale
Copy link
Owner

Extension results could contain responses to extensions specified in attestation or assertion options that the RP is interested in. Getting the results to the RP is the responsibility of @simplewebauthn/browser; what the RP does with the values afterwards is beyond scope.

PublicKeyCredential, which AttestationCredential and AssertionCredential are sub-interfaces of, has a method getClientExtensionResults() defined on it in TypeScript's DOM lib, which is typed to always return something, even if it's an empty object. I want to define as much as I can in the context of the DOM lib, so I don't think it's appropriate to layer on additional logic of when to include these extension results - if TypeScript says something will always be returned (as it should given the definition of the method in the spec: https://w3c.github.io/webauthn/#iface-pkcredential) then I'm choosing to follow that.

Is specifying an empty object for clientExtensionResults not feasible in your situation? That should be sufficient to get past any typing issues that come up as a result of this.

@Mikescops
Copy link
Contributor Author

What I meant is that similarly to transports there are just optional values that may be used by the relying party. What I'm doing now is passing an empty object manually to the verify function which looks pretty useless code ^^

@MasterKale
Copy link
Owner

I keep going back and forth on this one. I think right now I'm going to leave things as-is because to make results optional would technically involve a breaking API change for implementations that rely on that value being populated.

I'm closing this ticket for now, but I'll revisit this decision in the future when next I have a more substantial release that will make breaking chances.

@MasterKale MasterKale added package:browser @simplewebauthn/browser package:server @simplewebauthn/server labels Mar 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:browser @simplewebauthn/browser package:server @simplewebauthn/server
Projects
None yet
Development

No branches or pull requests

2 participants