Summary: | [Cocoa] Expose SPI to opt out of Extensible SSO authentication | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Brent Fulgham <bfulgham> | ||||||
Component: | WebKit Misc. | Assignee: | Brent Fulgham <bfulgham> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | bfulgham, ews-watchlist, jiewen_tan, simon.fraser, thorton, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=227925 | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 227974, 227925 | ||||||||
Attachments: |
|
Description
Brent Fulgham
2021-07-06 17:06:12 PDT
Created attachment 433053 [details]
Patch
Comment on attachment 433053 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=433053&action=review > Source/WebKit/UIProcess/API/Cocoa/WKPreferences.h:70 > +@property (nonatomic) BOOL extensibleSSOEnabled WK_API_AVAILABLE(macos(12.0), ios(15.0)); Might be good to get this API reviewed before landing to avoid any mishaps. Also, I think you want an 'is'-prefixed getter. > Source/WebKit/UIProcess/Cocoa/SOAuthorization/NavigationSOAuthorizationSession.mm:60 > + AUTHORIZATIONSESSION_RELEASE_LOG("shouldStartInternal: Starting Extensible SSO authentication for a web view that is not attached to a window. Loading will pause until a window is attached. If this is unexpected, set the WKPreference.extensibleSSOEnabled property to NO to disable Extenstible SSO operations."); "Extenstible" sp. > Source/WebKit/UIProcess/Cocoa/SOAuthorization/PopUpSOAuthorizationSession.mm:189 > [m_secretWebView setUIDelegate:m_secretDelegate.get()]; Wow, we make a WKWebView of our own, and copy the configuration of the parent? When do we do this?! How do bundle clients not get confused about the spurious web view?? "Make me a new web view" is usually delegated to the client. And its navigation delegate is custom, and ignores the app's policies? Very odd. ... all that said, not new in this patch! Comment on attachment 433053 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=433053&action=review >> Source/WebKit/UIProcess/API/Cocoa/WKPreferences.h:70 >> +@property (nonatomic) BOOL extensibleSSOEnabled WK_API_AVAILABLE(macos(12.0), ios(15.0)); > > Might be good to get this API reviewed before landing to avoid any mishaps. > > Also, I think you want an 'is'-prefixed getter. Please expand "SSO" at least in the comment. Comment on attachment 433053 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=433053&action=review >>> Source/WebKit/UIProcess/API/Cocoa/WKPreferences.h:70 >>> +@property (nonatomic) BOOL extensibleSSOEnabled WK_API_AVAILABLE(macos(12.0), ios(15.0)); >> >> Might be good to get this API reviewed before landing to avoid any mishaps. >> >> Also, I think you want an 'is'-prefixed getter. > > Please expand "SSO" at least in the comment. Will do (for both). >> Source/WebKit/UIProcess/Cocoa/SOAuthorization/NavigationSOAuthorizationSession.mm:60 >> + AUTHORIZATIONSESSION_RELEASE_LOG("shouldStartInternal: Starting Extensible SSO authentication for a web view that is not attached to a window. Loading will pause until a window is attached. If this is unexpected, set the WKPreference.extensibleSSOEnabled property to NO to disable Extenstible SSO operations."); > > "Extenstible" sp. Doh! >> Source/WebKit/UIProcess/Cocoa/SOAuthorization/PopUpSOAuthorizationSession.mm:189 >> [m_secretWebView setUIDelegate:m_secretDelegate.get()]; > > Wow, we make a WKWebView of our own, and copy the configuration of the parent? When do we do this?! How do bundle clients not get confused about the spurious web view?? "Make me a new web view" is usually delegated to the client. And its navigation delegate is custom, and ignores the app's policies? Very odd. > > ... all that said, not new in this patch! Yeah, I forgot the tortured situation that led to Jiewen needing to do this. After discussing more, we decided this should be API. We instead need to design a delegate method so the client can be part of the decision process of whether the wait for the view to be attached to a window, or fail quickly. For now, we will just expose the existing property we were using for internal and test purposes as SPI. Created attachment 433149 [details]
Patch
Comment on attachment 433149 [details]
Patch
Maybe double-check the API tests bot, since it's orange.
(In reply to Tim Horton from comment #7) > Comment on attachment 433149 [details] > Patch > > Maybe double-check the API tests bot, since it's orange. I won't land until both api bots are green. There are some differences in iOS and macOS that I might not have captured in my new assertion. The bots are green, so cq+. Committed r279755 (239528@main): <https://commits.webkit.org/239528@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 433149 [details]. |