208533 – [WebAuthn] Implement -[_WKWebAuthenticationPanelDelegate panel:decidePolicyForLocalAuthenticatorWithCompletionHandler:] SPI

Bug 208533 - [WebAuthn] Implement -[_WKWebAuthenticationPanelDelegate panel:decidePolicyForLocalAuthenticatorWithCompletionHandler:] SPI

Summary: [WebAuthn] Implement -[_WKWebAuthenticationPanelDelegate panel:decidePolicyFo...

Status:	RESOLVED FIXED

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	WebKit Misc. (show other bugs)
Version:	WebKit Nightly Build
Hardware:	Unspecified Unspecified

Importance:	P2 Normal
Assignee:	Jiewen Tan

URL:
Keywords:	InRadar

Depends on:
Blocks:	181943
	Show dependency tree / graph

Reported:	2020-03-03 13:19 PST by Jiewen Tan
Modified:	2020-03-04 17:03 PST (History)
CC List:	5 users (show)

See Also:

Attachments
Patch (53.88 KB, patch) 2020-03-03 13:40 PST, Jiewen Tan	no flags	Details \| Formatted Diff \| Diff
Patch (54.22 KB, patch) 2020-03-03 13:54 PST, Jiewen Tan	no flags	Details \| Formatted Diff \| Diff
Patch (54.22 KB, patch) 2020-03-03 14:38 PST, Jiewen Tan	no flags	Details \| Formatted Diff \| Diff
Patch (54.23 KB, patch) 2020-03-04 11:34 PST, Jiewen Tan	no flags	Details \| Formatted Diff \| Diff
Patch (55.95 KB, patch) 2020-03-04 12:15 PST, Jiewen Tan	achristensen: review+ commit-queue: commit-queue-	Details \| Formatted Diff \| Diff
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Jiewen Tan 2020-03-03 13:19:02 PST

Implement -[_WKWebAuthenticationPanelDelegate panel:decidePolicyForLocalAuthenticatorWithCompletionHandler:] SPI.

Comment 1 Radar WebKit Bug Importer 2020-03-03 13:19:39 PST

<rdar://problem/60010184>

Comment 2 Jiewen Tan 2020-03-03 13:40:22 PST

Created attachment 392320 [details]
Patch

Comment 3 Jiewen Tan 2020-03-03 13:43:02 PST

Comment on attachment 392320 [details]
Patch

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

> Source/WebKit/UIProcess/API/Cocoa/_WKWebAuthenticationPanel.h:93
> +@property (nonatomic, readonly, copy) NSSet *transports;

Should mention this change in the change log as well.

Comment 4 Jiewen Tan 2020-03-03 13:54:29 PST

Created attachment 392325 [details]
Patch

Comment 5 Jiewen Tan 2020-03-03 14:38:39 PST

Created attachment 392333 [details]
Patch

Comment 6 Alex Christensen 2020-03-04 09:22:34 PST

Comment on attachment 392333 [details]
Patch

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

> Source/WebCore/platform/LocalizedStrings.cpp:1213
> +    return WEB_UI_STRING("Touch ID to sign into this website.", "Use Touch ID to sign into this website");

What about Face ID?

> Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalConnection.h:53
> +    enum class UserVerification {

: bool

Comment 7 Jiewen Tan 2020-03-04 11:31:47 PST

Comment on attachment 392333 [details]
Patch

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

Thanks Alex for the review.

>> Source/WebCore/platform/LocalizedStrings.cpp:1213
>> +    return WEB_UI_STRING("Touch ID to sign into this website.", "Use Touch ID to sign into this website");
> 
> What about Face ID?

FaceID prompt doesn't show any title/message strings.

>> Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalConnection.h:53
>> +    enum class UserVerification {
> 
> : bool

Fixed.

Comment 8 Jiewen Tan 2020-03-04 11:34:08 PST

Created attachment 392446 [details]
Patch

Comment 9 Alex Christensen 2020-03-04 11:44:20 PST

Comment on attachment 392333 [details]
Patch

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

> Source/WebKit/UIProcess/API/APIWebAuthenticationPanelClient.h:54
> +    // FIXME: Ideally, LocalAuthenticatorPolicy::Disallow should be used. This is a quirk to bypass C API for WebKitTestRunner.

Let's not commit this.  Make the C API.  The default should be safe.
You don't have to make it fully asynchronous with a listener object and new object type, but please add a new C API client type with this callback that returns a bool, then WebKitTestRunner's implementation will return true and the default will be false.

Comment 10 Jiewen Tan 2020-03-04 12:10:21 PST

(In reply to Alex Christensen from comment #9)
> Comment on attachment 392333 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=392333&action=review
> 
> > Source/WebKit/UIProcess/API/APIWebAuthenticationPanelClient.h:54
> > +    // FIXME: Ideally, LocalAuthenticatorPolicy::Disallow should be used. This is a quirk to bypass C API for WebKitTestRunner.
> 
> Let's not commit this.  Make the C API.  The default should be safe.
> You don't have to make it fully asynchronous with a listener object and new
> object type, but please add a new C API client type with this callback that
> returns a bool, then WebKitTestRunner's implementation will return true and
> the default will be false.

Fixed.

Comment 11 Jiewen Tan 2020-03-04 12:15:13 PST

Created attachment 392455 [details]
Patch

Comment 12 Jiewen Tan 2020-03-04 12:36:48 PST

(In reply to Jiewen Tan from comment #11)
> Created attachment 392455 [details]
> Patch

Thanks, Alex.

Comment 13 WebKit Commit Bot 2020-03-04 13:16:20 PST

The commit-queue encountered the following flaky tests while processing attachment 392455 [details]:

editing/spelling/spellcheck-paste-continuous-disabled.html bug 208016 (authors: g.czajkowski@samsung.com and mark.lam@apple.com)
The commit-queue is continuing to process your patch.

Comment 14 WebKit Commit Bot 2020-03-04 13:17:03 PST

The commit-queue encountered the following flaky tests while processing attachment 392455 [details]:

editing/spelling/spellcheck-async-remove-frame.html bug 158401 (authors: morrita@google.com, rniwa@webkit.org, and tony@chromium.org)
svg/custom/object-sizing-explicit-height.xhtml bug 208592 (author: zimmermann@kde.org)
The commit-queue is continuing to process your patch.

Comment 15 WebKit Commit Bot 2020-03-04 13:35:18 PST

Comment on attachment 392455 [details]
Patch

Rejecting attachment 392455 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 392455, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 5000 characters of output:
coa/LocalAuthenticator.h
	M	Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm
	M	Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalConnection.h
	M	Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalConnection.mm
	M	Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalService.mm
	M	Source/WebKit/UIProcess/WebAuthentication/Cocoa/WebAuthenticationPanelClient.h
	M	Source/WebKit/UIProcess/WebAuthentication/Cocoa/WebAuthenticationPanelClient.mm
	M	Source/WebKit/UIProcess/WebAuthentication/Mock/MockLocalConnection.h
	M	Source/WebKit/UIProcess/WebAuthentication/Mock/MockLocalConnection.mm
	M	Source/WebKit/UIProcess/WebAuthentication/WebAuthenticationFlags.h
	M	Tools/ChangeLog
	M	Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
	M	Tools/TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm

ERROR from SVN:
Merge conflict during commit: Conflict at '/trunk/Source/WebKit/ChangeLog'
W: 937f80b4205f4987cb9a9d2ec8958d431ccbf351 and refs/remotes/origin/master differ, using rebase:
:040000 040000 a84a575ecae8477dd96f98fad91114e27723d018 6109000d1db81b6f0d5dc73d51b1fdea817f4f32 M	LayoutTests
:040000 040000 81cb632c531721fd1a234479ec9a2a896999554c 531e3b2f1c1c5c8c6b674937f3171e70ba8c7fa6 M	Source
:040000 040000 5443204dbe643da13513ba0c30cbe2ebd8a36bd2 1cbfe2da399663568c33343d7e846427f6383142 M	Tools
Current branch master is up to date.
ERROR: Not all changes have been committed into SVN, however the committed
ones (if any) seem to be successfully integrated into the working tree.
Please see the above messages for details.


Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	D	Tools/TestWebKitAPI/Tests/WebKitCocoa/web-authentication-get-assertion-la.html
	M	LayoutTests/http/wpt/webauthn/public-key-credential-create-failure-local.https.html
	M	LayoutTests/http/wpt/webauthn/public-key-credential-get-failure-local.https.html
	M	Source/WebCore/ChangeLog
	M	Source/WebCore/en.lproj/Localizable.strings
	M	Source/WebCore/platform/LocalizedStrings.cpp
	M	Source/WebCore/platform/LocalizedStrings.h
	M	Source/WebKit/ChangeLog
	M	Source/WebKit/Platform/spi/Cocoa/LocalAuthenticationSPI.h
	M	Source/WebKit/UIProcess/API/APIWebAuthenticationPanelClient.h
	M	Source/WebKit/UIProcess/API/C/WKPage.cpp
	M	Source/WebKit/UIProcess/API/Cocoa/_WKWebAuthenticationPanel.h
	M	Source/WebKit/UIProcess/API/Cocoa/_WKWebAuthenticationPanel.mm
	M	Source/WebKit/UIProcess/WebAuthentication/Authenticator.h
	M	Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp
	M	Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.h
	M	Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.h
	M	Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm
	M	Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalConnection.h
	M	Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalConnection.mm
	M	Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalService.mm
	M	Source/WebKit/UIProcess/WebAuthentication/Cocoa/WebAuthenticationPanelClient.h
	M	Source/WebKit/UIProcess/WebAuthentication/Cocoa/WebAuthenticationPanelClient.mm
	M	Source/WebKit/UIProcess/WebAuthentication/Mock/MockLocalConnection.h
	M	Source/WebKit/UIProcess/WebAuthentication/Mock/MockLocalConnection.mm
	M	Source/WebKit/UIProcess/WebAuthentication/WebAuthenticationFlags.h
	M	Tools/ChangeLog
	M	Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
	M	Tools/TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm

ERROR from SVN:
Merge conflict during commit: Conflict at '/trunk/Source/WebKit/ChangeLog'
W: 937f80b4205f4987cb9a9d2ec8958d431ccbf351 and refs/remotes/origin/master differ, using rebase:
:040000 040000 a84a575ecae8477dd96f98fad91114e27723d018 6109000d1db81b6f0d5dc73d51b1fdea817f4f32 M	LayoutTests
:040000 040000 81cb632c531721fd1a234479ec9a2a896999554c 531e3b2f1c1c5c8c6b674937f3171e70ba8c7fa6 M	Source
:040000 040000 5443204dbe643da13513ba0c30cbe2ebd8a36bd2 1cbfe2da399663568c33343d7e846427f6383142 M	Tools
Current branch master is up to date.
ERROR: Not all changes have been committed into SVN, however the committed
ones (if any) seem to be successfully integrated into the working tree.
Please see the above messages for details.


Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit
Updating OpenSource
From https://git.webkit.org/git/WebKit
   0a6a1caeff1..19142dbcd32  master     -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 257873 = 0a6a1caeff1415c65c45148a6b570c2e464b64c3
r257874 = 26e4328157b5c42de5d895971532024d34f9c9a9
r257875 = 19142dbcd32c2c760f38ad4fc7c0023f88f72d7a
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.

Full output: https://webkit-queues.webkit.org/results/13333637

Comment 16 Jiewen Tan 2020-03-04 13:42:44 PST

Committed r257877: <https://trac.webkit.org/changeset/257877>

Comment 17 Jiewen Tan 2020-03-04 17:03:21 PST

A build fix:
Committed r257891: <https://trac.webkit.org/changeset/257891>