Bug 201439 - [WebAuthn] Make WebAuthn default off and let clients turn it on at will
Summary: [WebAuthn] Make WebAuthn default off and let clients turn it on at will
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jiewen Tan
URL:
Keywords: InRadar
Depends on:
Blocks: 181943
  Show dependency treegraph
 
Reported: 2019-09-03 16:57 PDT by Jiewen Tan
Modified: 2019-09-06 17:08 PDT (History)
10 users (show)

See Also:


Attachments
Patch (7.37 KB, patch)
2019-09-03 17:24 PDT, Jiewen Tan
no flags Details | Formatted Diff | Diff
Patch (7.38 KB, patch)
2019-09-03 17:55 PDT, Jiewen Tan
no flags Details | Formatted Diff | Diff
Patch (3.02 KB, patch)
2019-09-05 19:46 PDT, Jiewen Tan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jiewen Tan 2019-09-03 16:57:26 PDT
Add a SPI to enable WebAuthentication in WKPreferences.
Comment 1 Radar WebKit Bug Importer 2019-09-03 16:58:22 PDT
<rdar://problem/54998154>
Comment 2 Jiewen Tan 2019-09-03 17:24:21 PDT
Created attachment 377937 [details]
Patch
Comment 3 mitz 2019-09-03 17:28:28 PDT
Comment on attachment 377937 [details]
Patch

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

> Source/WebKit/UIProcess/API/Cocoa/WKPreferencesPrivate.h:160
> +@property (nonatomic, setter=_setWebAuthenticationEnabled:) BOOL _webAuthenticationEnabled WK_API_AVAILABLE(macos(10.13), ios(13.0));

macOS 10.13 has already shipped without this SPI, and I suspect that iOS 13.0 is also incorrect here. We typically use WK_MAC_TBA and WK_IOS_TBA until after the GM seed of the OS is available to developers.
Comment 4 Jiewen Tan 2019-09-03 17:38:17 PDT
Comment on attachment 377937 [details]
Patch

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

Thanks Dan for reviewing the patch.

>> Source/WebKit/UIProcess/API/Cocoa/WKPreferencesPrivate.h:160
>> +@property (nonatomic, setter=_setWebAuthenticationEnabled:) BOOL _webAuthenticationEnabled WK_API_AVAILABLE(macos(10.13), ios(13.0));
> 
> macOS 10.13 has already shipped without this SPI, and I suspect that iOS 13.0 is also incorrect here. We typically use WK_MAC_TBA and WK_IOS_TBA until after the GM seed of the OS is available to developers.

WebAuthentication is enabled for Safari 13 and we are shipping Safari 13 on High Sierra, Mojave, and Catalina. I think specify WK_MAC_TBA for macOS might be risky as someone might later change it to 10.15 and breaks Safari.

Fixed iOS.
Comment 5 mitz 2019-09-03 17:41:34 PDT
(In reply to Jiewen Tan from comment #4)
> Comment on attachment 377937 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=377937&action=review
> 
> Thanks Dan for reviewing the patch.
> 
> >> Source/WebKit/UIProcess/API/Cocoa/WKPreferencesPrivate.h:160
> >> +@property (nonatomic, setter=_setWebAuthenticationEnabled:) BOOL _webAuthenticationEnabled WK_API_AVAILABLE(macos(10.13), ios(13.0));
> > 
> > macOS 10.13 has already shipped without this SPI, and I suspect that iOS 13.0 is also incorrect here. We typically use WK_MAC_TBA and WK_IOS_TBA until after the GM seed of the OS is available to developers.
> 
> WebAuthentication is enabled for Safari 13 and we are shipping Safari 13 on
> High Sierra, Mojave, and Catalina. I think specify WK_MAC_TBA for macOS
> might be risky as someone might later change it to 10.15 and breaks Safari.
> 
> Fixed iOS.

The SPI that’s available to Safari when deployed to an already-shipping macOS release is different from what’s in those release. Please use WK_MAC_TBA or consult folks who are familiar with how Apple manages WebKit API availability.
Comment 6 Jiewen Tan 2019-09-03 17:53:38 PDT
Comment on attachment 377937 [details]
Patch

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

>>>> Source/WebKit/UIProcess/API/Cocoa/WKPreferencesPrivate.h:160
>>>> +@property (nonatomic, setter=_setWebAuthenticationEnabled:) BOOL _webAuthenticationEnabled WK_API_AVAILABLE(macos(10.13), ios(13.0));
>>> 
>>> macOS 10.13 has already shipped without this SPI, and I suspect that iOS 13.0 is also incorrect here. We typically use WK_MAC_TBA and WK_IOS_TBA until after the GM seed of the OS is available to developers.
>> 
>> WebAuthentication is enabled for Safari 13 and we are shipping Safari 13 on High Sierra, Mojave, and Catalina. I think specify WK_MAC_TBA for macOS might be risky as someone might later change it to 10.15 and breaks Safari.
>> 
>> Fixed iOS.
> 
> The SPI that’s available to Safari when deployed to an already-shipping macOS release is different from what’s in those release. Please use WK_MAC_TBA or consult folks who are familiar with how Apple manages WebKit API availability.

I see. Fixed macOS as well.
Comment 7 Jiewen Tan 2019-09-03 17:55:11 PDT
Created attachment 377944 [details]
Patch
Comment 8 Jiewen Tan 2019-09-05 19:46:10 PDT
Created attachment 378153 [details]
Patch
Comment 9 Jiewen Tan 2019-09-06 15:07:39 PDT
Comment on attachment 378153 [details]
Patch

Thanks Youenn for r+ the patch.
Comment 10 WebKit Commit Bot 2019-09-06 17:08:19 PDT
Comment on attachment 378153 [details]
Patch

Clearing flags on attachment: 378153

Committed r249600: <https://trac.webkit.org/changeset/249600>
Comment 11 WebKit Commit Bot 2019-09-06 17:08:20 PDT
All reviewed patches have been landed.  Closing bug.