Bug 223168

Summary: Add internal additions for WebAuthn compatibility
Product: WebKit Reporter: Kate Cheney <katherine_cheney>
Component: WebKit Misc.Assignee: Kate Cheney <katherine_cheney>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, cdumez, ews-watchlist, jiewen_tan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Kate Cheney
Reported 2021-03-14 16:21:19 PDT
com.apple.webkit.webauthn should actually be com.apple.WebKit.WebAuthn for internal-compatibility reasons.
Attachments
Patch (4.43 KB, patch)
2021-03-14 16:25 PDT, Kate Cheney 		no flags
DetailsFormatted DiffDiff
Patch (1.88 KB, patch)
2021-03-15 12:48 PDT, Kate Cheney 		no flags
DetailsFormatted DiffDiff
Patch (2.04 KB, patch)
2021-03-15 15:42 PDT, Kate Cheney 		no flags
DetailsFormatted DiffDiff
Patch (2.12 KB, patch)
2021-03-15 20:47 PDT, Kate Cheney 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Kate Cheney
Comment 1 2021-03-14 16:21:46 PDT
<rdar://problem/74890060>
Kate Cheney
Comment 2 2021-03-14 16:25:14 PDT
Created attachment 423137 [details] Patch
Brent Fulgham
Comment 3 2021-03-15 09:47:58 PDT
Comment on attachment 423137 [details] Patch Case sensitivity. *sigh*. r=me
Jiewen Tan
Comment 4 2021-03-15 12:36:41 PDT
Comment on attachment 423137 [details] Patch Can we just add the WKA part and remove the entitlement part?
Kate Cheney
Comment 5 2021-03-15 12:48:58 PDT
Created attachment 423224 [details] Patch
Kate Cheney
Comment 6 2021-03-15 12:49:13 PDT
(In reply to Jiewen Tan from comment #4) > Comment on attachment 423137 [details] > Patch > > Can we just add the WKA part and remove the entitlement part? Yes, I updated the patch.
Jiewen Tan
Comment 7 2021-03-15 13:14:52 PDT
Comment on attachment 423224 [details] Patch LGTM.
Kate Cheney
Comment 8 2021-03-15 15:42:00 PDT
Created attachment 423255 [details] Patch
Brent Fulgham
Comment 9 2021-03-15 16:41:20 PDT
Comment on attachment 423255 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423255&action=review r=me > Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm:54 > +static NSDictionary *createAlternateQuery(const String&) { return [NSDictionary alloc]; } Could this just be return nil, or perhapss ASSERT_NOT_REACHED? I don't think we should ever hit this code path.
Chris Dumez
Comment 10 2021-03-15 16:46:21 PDT
Comment on attachment 423255 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423255&action=review >> Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm:54 >> +static NSDictionary *createAlternateQuery(const String&) { return [NSDictionary alloc]; } > > Could this just be return nil, or perhapss ASSERT_NOT_REACHED? I don't think we should ever hit this code path. Please don't call [NSDictionary alloc] here, this would leak.
Chris Dumez
Comment 11 2021-03-15 16:54:44 PDT
Comment on attachment 423255 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423255&action=review > Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm:142 > + query = createAlternateQuery(rpId); Based on how this is used, createAlternateQuery() seems to either leak or return an autoreleased value. If it returns an autoreleased value, then I think we should not use "create" in the name. If it returns a value that is not autoreleased, then I think createAlternateQuery() should return a RetainPtr<>.
Kate Cheney
Comment 12 2021-03-15 20:47:24 PDT
Created attachment 423292 [details] Patch
Brent Fulgham
Comment 13 2021-03-16 09:23:55 PDT
Comment on attachment 423292 [details] Patch r=me
EWS
Comment 14 2021-03-16 10:34:00 PDT
Committed r274490: <https://commits.webkit.org/r274490> All reviewed patches have been landed. Closing bug and clearing flags on attachment 423292 [details].
Note You need to log in before you can comment on or make changes to this bug.