Bug 182771 – [WebAuthN] Implement PublicKeyCredential.isUserVerifyingPlatformAuthenticatorAvailable()

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Jiewen Tan]

Implement PublicKeyCredential.isUserVerifyingPlatformAuthenticatorAvailable() per https://www.w3.org/TR/webauthn/#isUserVerifyingPlatformAuthenticatorAvailable.

[Jiewen Tan]

<rdar://problem/36459988>

[Jiewen Tan]

Created attachment 333769 [details]
Patch

[Brent Fulgham]

Comment on attachment 333769 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=333769&action=review

This looks good, but I think you should try to implement on macOS as well. The SDKs exist! r- because of lack of macOS support.

> Source/WebCore/ChangeLog:13
> +        Besides, it changes DeferredPromise to DOMPromiseDeferred<> for all CredentialsManagement and

I would say, "In addition, it changes ..."

> Source/WebKit/ChangeLog:9
> +        This patch uitlizes LocalAuthentication Framework to determine if biometrics

utilizes

> Source/WebKit/ChangeLog:14
> +        Corresponding macOS implementations are marked as unimplemented as a result.

I don't think this is true. <https://developer.apple.com/documentation/localauthentication?language=objc> claims that it is present in iOS 8+ and macOS 10.10+

> Source/WebKit/UIProcess/CredentialManagement/ios/WebCredentialsMessengerProxyIOS.mm:44
> +        LOG_ERROR("Couldn't evaluate policy: %@", error);

This might be better as "Couldn't evaluate authentication with biometrics policy: %@"

> Source/WebKit/UIProcess/CredentialManagement/mac/WebCredentialsMessengerProxyMac.mm:37
> +    notImplemented();

I think this can be implemented, based on <https://developer.apple.com/documentation/localauthentication?language=objc>.

[Jiewen Tan]

Comment on attachment 333769 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=333769&action=review

Thanks Brent for reviewing the patch.

>> Source/WebCore/ChangeLog:13
>> +        Besides, it changes DeferredPromise to DOMPromiseDeferred<> for all CredentialsManagement and
> 
> I would say, "In addition, it changes ..."

Fixed.

>> Source/WebKit/ChangeLog:9
>> +        This patch uitlizes LocalAuthentication Framework to determine if biometrics
> 
> utilizes

Fixed.

>> Source/WebKit/ChangeLog:14
>> +        Corresponding macOS implementations are marked as unimplemented as a result.
> 
> I don't think this is true. <https://developer.apple.com/documentation/localauthentication?language=objc> claims that it is present in iOS 8+ and macOS 10.10+

Sure. I will try a TouchID equipped device and see what the real effect is.

>> Source/WebKit/UIProcess/CredentialManagement/ios/WebCredentialsMessengerProxyIOS.mm:44
>> +        LOG_ERROR("Couldn't evaluate policy: %@", error);
> 
> This might be better as "Couldn't evaluate authentication with biometrics policy: %@"

Fixed.

>> Source/WebKit/UIProcess/CredentialManagement/mac/WebCredentialsMessengerProxyMac.mm:37
>> +    notImplemented();
> 
> I think this can be implemented, based on <https://developer.apple.com/documentation/localauthentication?language=objc>.

Ditto.

[Jiewen Tan]

Confirmed LocalAuthentication does distinguish TouchID equipped macs and TouchIDless ones.

[Jiewen Tan]

Created attachment 333949 [details]
Patch

[Jiewen Tan]

Created attachment 333970 [details]
Patch

[Brent Fulgham]

Comment on attachment 333970 [details]
Patch

Looks great! r=me.

[Jiewen Tan]

(In reply to Brent Fulgham from comment #8)
> Comment on attachment 333970 [details]
> Patch
> 
> Looks great! r=me.

Thanks Brent for r+ the patch.

[WebKit Commit Bot]

Comment on attachment 333970 [details]
Patch

Clearing flags on attachment: 333970

Committed r228572: <https://trac.webkit.org/changeset/228572>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.