[webkit-reviews] review granted: [Bug 233371] Add headers for [_WKWebAuthenticationPanel makeCredentialWithClientDataHash] and [_WKWebAuthenticationPanel getAssertionWithClientDataHash] : [Attachment 444828] Patch

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has granted j_pascoe at apple.com
<j_pascoe at apple.com>'s request for review:
Bug 233371: Add headers for [_WKWebAuthenticationPanel
makeCredentialWithClientDataHash] and [_WKWebAuthenticationPanel
getAssertionWithClientDataHash]
https://bugs.webkit.org/show_bug.cgi?id=233371

Attachment 444828: Patch

https://bugs.webkit.org/attachment.cgi?id=444828&action=review




--- Comment #4 from Brent Fulgham <bfulgham at webkit.org> ---
Comment on attachment 444828
  --> https://bugs.webkit.org/attachment.cgi?id=444828
Patch

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

r=me if you switch to WK_<MAC/IOS>_TBA in the availability macros.

> Source/WebKit/UIProcess/API/Cocoa/_WKWebAuthenticationPanel.h:129
> +- (void)makeCredentialWithClientDataHash:(NSData *)clientDataHash
options:(_WKPublicKeyCredentialCreationOptions *)options
completionHandler:(void (^)(_WKAuthenticatorAttestationResponse *, NSError
*))handler WK_API_AVAILABLE(macos(12.0), ios(15.0));

This isn't true -- we already shipped macOS 12 and iOS 15 without these
methods. Usually we use "WK_MAC_TBA" and "WK_IOS_TBA" until we are close to a
release.

> Source/WebKit/UIProcess/API/Cocoa/_WKWebAuthenticationPanel.h:131
> +- (void)getAssertionWithClientDataHash:(NSData *)clientDataHash
options:(_WKPublicKeyCredentialRequestOptions *)options completionHandler:(void
(^)(_WKAuthenticatorAssertionResponse *, NSError *))handler
WK_API_AVAILABLE(macos(12.0), ios(15.0));

Ditto.

> Tools/TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm:1932
> +	   EXPECT_WK_STREQ([response.rawId base64EncodedStringWithOptions:0],
"SMSXHngF7hEOsElA73C3RY+8bR4=");

Curious if this rawId has a meaning you could reference, similar to the
comments in the GetAssertionLAClientDataHash test, below.

> Tools/TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm:2117
> +	   // echo -n "example.com" | shasum -a 256 | xxd -r -p | base64

These comments are super helpful. I wonder if there is a similar thing you
could say about the raw ID above?