Bug 201439

Summary: [WebAuthn] Make WebAuthn default off and let clients turn it on at will
Product: WebKit Reporter: Jiewen Tan <jiewen_tan>
Component: WebKit Misc.Assignee: Jiewen Tan <jiewen_tan>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, alex.gaynor, bfulgham, commit-queue, jiewen_tan, mitz, sam, thorton, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=201369
Bug Depends on:    
Bug Blocks: 181943    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

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.