Diff
Modified: trunk/Source/WebCore/ChangeLog (274745 => 274746)
--- trunk/Source/WebCore/ChangeLog 2021-03-19 23:25:07 UTC (rev 274745)
+++ trunk/Source/WebCore/ChangeLog 2021-03-19 23:27:45 UTC (rev 274746)
@@ -1,3 +1,56 @@
+2021-03-19 Kate Cheney <[email protected]>
+
+ Cannot login to microsoftonline.com without allowing storage access
+ https://bugs.webkit.org/show_bug.cgi?id=223510
+ <rdar://problem/75619058>
+
+ Reviewed by John Wilander.
+
+ This patch introduces two changes:
+
+ 1. Activate the storage access quirks for default browsers only. This
+ is the main place we test them, and can lead to unexpected behavior in
+ apps otherwise.
+
+ 2. Doesn't cancel the click even if the user denies storage access.
+ Previously we we were not allowing the click because it produces
+ unexpected behavior. But, sites like login.microsoftonline.com are used
+ by multiple Microsoft login flows. Since only a subset require storage
+ access, canceling the click across the board could be considered
+ regressing behavior.
+
+ Since storage access data is stored in the network process, we
+ currently cancel the user's click on quirked sites while we wait
+ asyncronously for the IPC response. If the user grants storage access,
+ we store this in the web content process and dispatch a synthetic
+ click. In this case, when the click triggers another storage access
+ check, we can allow the click because we no longer need to wait for
+ information from the network process.
+
+ We need some heuristic in the web content process to handle the case
+ of the user denying storage access to know whether to allow a
+ synthetic click to happen. This patch introduces
+ hasDeniedCrossPageStorageAccess which stores domains in the web
+ content process which have been denied storage access via quirk. If
+ a user has previously denied storage access in that web content
+ process, we don't prompt again. A new web content process will give them
+ another opportunity to allow/deny storage access.
+
+ * dom/Element.cpp:
+ (WebCore::Element::dispatchMouseEvent):
+ * loader/FrameLoaderClient.h:
+ Add a client function to check if the parent process is a full web
+ browser, and pass this to the place we apply the quirks.
+
+ * loader/ResourceLoadObserver.h:
+ (WebCore::ResourceLoadObserver::setHasDeniedCrossPageStorageAccess):
+ (WebCore::ResourceLoadObserver::hasDeniedCrossPageStorageAccess const):
+ * page/Quirks.cpp:
+ (WebCore::hasDeniedCrossPageStorageAccess):
+ (WebCore::Quirks::requestStorageAccessAndHandleClick const):
+ (WebCore::Quirks::triggerOptionalStorageAccessQuirk const):
+ * page/Quirks.h:
+
2021-03-19 Zalan Bujtas <[email protected]>
Unreviewed, reverting r274596.
Modified: trunk/Source/WebCore/dom/Element.cpp (274745 => 274746)
--- trunk/Source/WebCore/dom/Element.cpp 2021-03-19 23:25:07 UTC (rev 274745)
+++ trunk/Source/WebCore/dom/Element.cpp 2021-03-19 23:27:45 UTC (rev 274746)
@@ -390,8 +390,15 @@
if (dispatchPointerEventIfNeeded(*this, mouseEvent.get(), platformEvent, didNotSwallowEvent) == ShouldIgnoreMouseEvent::Yes)
return false;
-
- if (Quirks::StorageAccessResult::ShouldCancelEvent == document().quirks().triggerOptionalStorageAccessQuirk(*this, platformEvent, eventType, detail, relatedTarget))
+
+ auto isParentProcessAFullWebBrowser = false;
+#if PLATFORM(IOS_FAMILY)
+ if (Frame* frame = document().frame())
+ isParentProcessAFullWebBrowser = frame->loader().client().isParentProcessAFullWebBrowser();
+#elif PLATFORM(MAC)
+ isParentProcessAFullWebBrowser = MacApplication::isSafari();
+#endif
+ if (Quirks::StorageAccessResult::ShouldCancelEvent == document().quirks().triggerOptionalStorageAccessQuirk(*this, platformEvent, eventType, detail, relatedTarget, isParentProcessAFullWebBrowser))
return false;
ASSERT(!mouseEvent->target() || mouseEvent->target() != relatedTarget);
Modified: trunk/Source/WebCore/loader/FrameLoaderClient.h (274745 => 274746)
--- trunk/Source/WebCore/loader/FrameLoaderClient.h 2021-03-19 23:25:07 UTC (rev 274745)
+++ trunk/Source/WebCore/loader/FrameLoaderClient.h 2021-03-19 23:27:45 UTC (rev 274746)
@@ -384,6 +384,8 @@
#if ENABLE(PDFKIT_PLUGIN)
virtual bool shouldUsePDFPlugin(const String&, StringView) const { return false; }
#endif
+
+ virtual bool isParentProcessAFullWebBrowser() const { return false; }
};
} // namespace WebCore
Modified: trunk/Source/WebCore/loader/ResourceLoadObserver.h (274745 => 274746)
--- trunk/Source/WebCore/loader/ResourceLoadObserver.h 2021-03-19 23:25:07 UTC (rev 274745)
+++ trunk/Source/WebCore/loader/ResourceLoadObserver.h 2021-03-19 23:27:45 UTC (rev 274746)
@@ -71,6 +71,9 @@
virtual void setDomainsWithCrossPageStorageAccess(HashMap<TopFrameDomain, SubResourceDomain>&&, CompletionHandler<void()>&& completionHandler) { completionHandler(); }
virtual bool hasCrossPageStorageAccess(const SubResourceDomain&, const TopFrameDomain&) const { return false; }
virtual bool hasHadUserInteraction(const RegistrableDomain&) const { return false; }
+
+ virtual void setHasDeniedCrossPageStorageAccess(HashMap<TopFrameDomain, SubResourceDomain>&&, CompletionHandler<void()>&& completionHandler) { completionHandler(); }
+ virtual bool hasDeniedCrossPageStorageAccess(const SubResourceDomain&, const TopFrameDomain&) const { return false; }
};
} // namespace WebCore
Modified: trunk/Source/WebCore/page/Quirks.cpp (274745 => 274746)
--- trunk/Source/WebCore/page/Quirks.cpp 2021-03-19 23:25:07 UTC (rev 274745)
+++ trunk/Source/WebCore/page/Quirks.cpp 2021-03-19 23:27:45 UTC (rev 274746)
@@ -1037,6 +1037,15 @@
return true;
}
+static bool hasDeniedCrossPageStorageAccess(const HashSet<RegistrableDomain>& loginDomains, const RegistrableDomain& topFrameDomain)
+{
+ for (auto& loginDomain : loginDomains) {
+ if (ResourceLoadObserver::shared().hasDeniedCrossPageStorageAccess(loginDomain, topFrameDomain))
+ return true;
+ }
+ return false;
+}
+
const String& Quirks::BBCRadioPlayerURLString()
{
static NeverDestroyed<String> BBCRadioPlayerURLString = "https://www.bbc.co.uk/sounds/player/bbc_world_service"_s;
@@ -1064,16 +1073,17 @@
return element.parentElement()->classNames().contains("p_audioButton_buttonInner") && parentElement->parentElement()->classNames().contains("hidden");
}
-Quirks::StorageAccessResult Quirks::requestStorageAccessAndHandleClick(CompletionHandler<void(StorageAccessWasGranted)>&& completionHandler) const
+Quirks::StorageAccessResult Quirks::requestStorageAccessAndHandleClick(CompletionHandler<void(ShouldDispatchClick)>&& completionHandler) const
{
auto firstPartyDomain = mapToTopDomain(m_document->topDocument().url());
auto domainsInNeedOfStorageAccess = NetworkStorageSession::subResourceDomainsInNeedOfStorageAccessForFirstParty(firstPartyDomain);
if (!domainsInNeedOfStorageAccess || domainsInNeedOfStorageAccess.value().isEmpty()) {
- completionHandler(StorageAccessWasGranted::No);
+ completionHandler(ShouldDispatchClick::No);
return Quirks::StorageAccessResult::ShouldNotCancelEvent;
}
- if (hasStorageAccessForAllLoginDomains(*domainsInNeedOfStorageAccess, firstPartyDomain)) {
- completionHandler(StorageAccessWasGranted::No);
+ if (hasStorageAccessForAllLoginDomains(*domainsInNeedOfStorageAccess, firstPartyDomain)
+ || hasDeniedCrossPageStorageAccess(*domainsInNeedOfStorageAccess, firstPartyDomain)) {
+ completionHandler(ShouldDispatchClick::No);
return Quirks::StorageAccessResult::ShouldNotCancelEvent;
}
@@ -1080,18 +1090,20 @@
auto domainInNeedOfStorageAccess = RegistrableDomain(*domainsInNeedOfStorageAccess.value().begin().get());
if (!m_document) {
- completionHandler(StorageAccessWasGranted::No);
+ completionHandler(ShouldDispatchClick::No);
return Quirks::StorageAccessResult::ShouldNotCancelEvent;
}
DocumentStorageAccess::requestStorageAccessForNonDocumentQuirk(*m_document, WTFMove(domainInNeedOfStorageAccess), [firstPartyDomain, domainInNeedOfStorageAccess, completionHandler = WTFMove(completionHandler)](StorageAccessWasGranted storageAccessGranted) mutable {
if (storageAccessGranted == StorageAccessWasGranted::No) {
- completionHandler(storageAccessGranted);
+ ResourceLoadObserver::shared().setHasDeniedCrossPageStorageAccess({{ firstPartyDomain, domainInNeedOfStorageAccess }}, [completionHandler = WTFMove(completionHandler)] () mutable {
+ completionHandler(ShouldDispatchClick::Yes);
+ });
return;
}
- ResourceLoadObserver::shared().setDomainsWithCrossPageStorageAccess({{ firstPartyDomain, domainInNeedOfStorageAccess }}, [storageAccessGranted, completionHandler = WTFMove(completionHandler)] () mutable {
- completionHandler(storageAccessGranted);
+ ResourceLoadObserver::shared().setDomainsWithCrossPageStorageAccess({{ firstPartyDomain, domainInNeedOfStorageAccess }}, [completionHandler = WTFMove(completionHandler)] () mutable {
+ completionHandler(ShouldDispatchClick::Yes);
});
});
return Quirks::StorageAccessResult::ShouldCancelEvent;
@@ -1106,9 +1118,9 @@
}
#endif
-Quirks::StorageAccessResult Quirks::triggerOptionalStorageAccessQuirk(Element& element, const PlatformMouseEvent& platformEvent, const AtomString& eventType, int detail, Element* relatedTarget) const
+Quirks::StorageAccessResult Quirks::triggerOptionalStorageAccessQuirk(Element& element, const PlatformMouseEvent& platformEvent, const AtomString& eventType, int detail, Element* relatedTarget, bool isParentProcessAFullWebBrowser) const
{
- if (!DeprecatedGlobalSettings::resourceLoadStatisticsEnabled())
+ if (!DeprecatedGlobalSettings::resourceLoadStatisticsEnabled() || !isParentProcessAFullWebBrowser)
return Quirks::StorageAccessResult::ShouldNotCancelEvent;
#if ENABLE(RESOURCE_LOAD_STATISTICS)
@@ -1183,11 +1195,11 @@
}
if (isStorageAccessQuirkDomainAndElement(m_document->url(), element)) {
- return requestStorageAccessAndHandleClick([element = makeWeakPtr(element), platformEvent, eventType, detail, relatedTarget] (StorageAccessWasGranted storageAccessWasGranted) mutable {
+ return requestStorageAccessAndHandleClick([element = makeWeakPtr(element), platformEvent, eventType, detail, relatedTarget] (ShouldDispatchClick shouldDispatchClick) mutable {
if (!element)
return;
- if (storageAccessWasGranted == StorageAccessWasGranted::Yes)
+ if (shouldDispatchClick == ShouldDispatchClick::Yes)
element->dispatchMouseEvent(platformEvent, eventType, detail, relatedTarget);
});
}
@@ -1197,8 +1209,8 @@
// BBC RadioPlayer case.
if (isBBCDomain(domain) && isBBCPopUpPlayerElement(element)) {
- return requestStorageAccessAndHandleClick([document = m_document] (StorageAccessWasGranted storageAccessWasGranted) mutable {
- if (!document || storageAccessWasGranted == StorageAccessWasGranted::No)
+ return requestStorageAccessAndHandleClick([document = m_document] (ShouldDispatchClick shouldDispatchClick) mutable {
+ if (!document || shouldDispatchClick == ShouldDispatchClick::No)
return;
auto domWindow = document->domWindow();
Modified: trunk/Source/WebCore/page/Quirks.h (274745 => 274746)
--- trunk/Source/WebCore/page/Quirks.h 2021-03-19 23:25:07 UTC (rev 274745)
+++ trunk/Source/WebCore/page/Quirks.h 2021-03-19 23:27:45 UTC (rev 274746)
@@ -119,7 +119,8 @@
bool shouldAvoidPastingImagesAsWebContent() const;
enum StorageAccessResult : bool { ShouldNotCancelEvent, ShouldCancelEvent };
- StorageAccessResult triggerOptionalStorageAccessQuirk(Element&, const PlatformMouseEvent&, const AtomString& eventType, int, Element*) const;
+ enum ShouldDispatchClick : bool { No, Yes };
+ StorageAccessResult triggerOptionalStorageAccessQuirk(Element&, const PlatformMouseEvent&, const AtomString& eventType, int, Element*, bool isParentProcessAFullWebBrowser) const;
bool needsVP9FullRangeFlagQuirk() const;
bool needsHDRPixelDepthQuirk() const;
@@ -139,7 +140,7 @@
static bool hasStorageAccessForAllLoginDomains(const HashSet<RegistrableDomain>&, const RegistrableDomain&);
static const String& BBCRadioPlayerURLString();
WEBCORE_EXPORT static const String& staticRadioPlayerURLString();
- StorageAccessResult requestStorageAccessAndHandleClick(CompletionHandler<void(StorageAccessWasGranted)>&&) const;
+ StorageAccessResult requestStorageAccessAndHandleClick(CompletionHandler<void(ShouldDispatchClick)>&&) const;
static RegistrableDomain mapToTopDomain(const URL&);
#endif
Modified: trunk/Source/WebKit/ChangeLog (274745 => 274746)
--- trunk/Source/WebKit/ChangeLog 2021-03-19 23:25:07 UTC (rev 274745)
+++ trunk/Source/WebKit/ChangeLog 2021-03-19 23:27:45 UTC (rev 274746)
@@ -1,3 +1,27 @@
+2021-03-19 Kate Cheney <[email protected]>
+
+ Cannot login to microsoftonline.com without allowing storage access
+ https://bugs.webkit.org/show_bug.cgi?id=223510
+ <rdar://problem/75619058>
+
+ Reviewed by John Wilander.
+
+ Disable the storage access quirk for non-default web browsers and
+ don't cancel the login click on sites even if the user denies storage
+ access. See WebCore changelog for more details.
+
+ * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
+ (WebKit::WebFrameLoaderClient::isParentProcessAFullWebBrowser const):
+ * WebProcess/WebCoreSupport/WebFrameLoaderClient.h:
+ * WebProcess/WebCoreSupport/WebResourceLoadObserver.cpp:
+ (WebKit::WebResourceLoadObserver::hasDeniedCrossPageStorageAccess const):
+ (WebKit::WebResourceLoadObserver::setHasDeniedCrossPageStorageAccess):
+ * WebProcess/WebCoreSupport/WebResourceLoadObserver.h:
+ * WebProcess/WebPage/WebPage.cpp:
+ (WebKit::WebPage::pageIsParentProcessAFullWebBrowser):
+ (WebKit::WebPage::updatePreferences):
+ * WebProcess/WebPage/WebPage.h:
+
2021-03-19 Brent Fulgham <[email protected]>
[macOS] Allow opendirectoryd access in base system
Modified: trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp (274745 => 274746)
--- trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp 2021-03-19 23:25:07 UTC (rev 274745)
+++ trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp 2021-03-19 23:27:45 UTC (rev 274746)
@@ -1957,6 +1957,12 @@
}
#endif
+bool WebFrameLoaderClient::isParentProcessAFullWebBrowser() const
+{
+ auto* page = m_frame->page();
+ return page && page->isParentProcessAWebBrowser();
+}
+
} // namespace WebKit
#undef PREFIX_PARAMETERS
Modified: trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.h (274745 => 274746)
--- trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.h 2021-03-19 23:25:07 UTC (rev 274745)
+++ trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.h 2021-03-19 23:27:45 UTC (rev 274746)
@@ -294,6 +294,7 @@
bool shouldUsePDFPlugin(const String& contentType, StringView path) const final;
#endif
+ bool isParentProcessAFullWebBrowser() const final;
};
// As long as EmptyFrameLoaderClient exists in WebCore, this can return nullptr.
Modified: trunk/Source/WebKit/WebProcess/WebCoreSupport/WebResourceLoadObserver.cpp (274745 => 274746)
--- trunk/Source/WebKit/WebProcess/WebCoreSupport/WebResourceLoadObserver.cpp 2021-03-19 23:25:07 UTC (rev 274745)
+++ trunk/Source/WebKit/WebProcess/WebCoreSupport/WebResourceLoadObserver.cpp 2021-03-19 23:27:45 UTC (rev 274746)
@@ -443,6 +443,25 @@
completionHandler();
}
+bool WebResourceLoadObserver::hasDeniedCrossPageStorageAccess(const SubFrameDomain& subDomain, const TopFrameDomain& topDomain) const
+{
+ auto it = m_domainsWithDeniedStorageAccess.find(topDomain);
+
+ if (it != m_domainsWithDeniedStorageAccess.end())
+ return it->value.contains(subDomain);
+
+ return false;
+}
+
+void WebResourceLoadObserver::setHasDeniedCrossPageStorageAccess(HashMap<TopFrameDomain, SubFrameDomain>&& domains, CompletionHandler<void()>&& completionHandler)
+{
+ for (auto& topDomain : domains.keys()) {
+ m_domainsWithDeniedStorageAccess.ensure(topDomain, [] { return HashSet<RegistrableDomain> { };
+ }).iterator->value.add(domains.get(topDomain));
+ }
+ completionHandler();
+}
+
} // namespace WebKit
#endif // ENABLE(RESOURCE_LOAD_STATISTICS)
Modified: trunk/Source/WebKit/WebProcess/WebCoreSupport/WebResourceLoadObserver.h (274745 => 274746)
--- trunk/Source/WebKit/WebProcess/WebCoreSupport/WebResourceLoadObserver.h 2021-03-19 23:25:07 UTC (rev 274745)
+++ trunk/Source/WebKit/WebProcess/WebCoreSupport/WebResourceLoadObserver.h 2021-03-19 23:27:45 UTC (rev 274746)
@@ -69,6 +69,8 @@
void setDomainsWithCrossPageStorageAccess(HashMap<TopFrameDomain, SubFrameDomain>&&, CompletionHandler<void()>&&) final;
bool hasHadUserInteraction(const WebCore::RegistrableDomain&) const final;
bool hasCrossPageStorageAccess(const SubFrameDomain&, const TopFrameDomain&) const final;
+ void setHasDeniedCrossPageStorageAccess(HashMap<TopFrameDomain, SubFrameDomain>&&, CompletionHandler<void()>&&) final;
+ bool hasDeniedCrossPageStorageAccess(const SubFrameDomain&, const TopFrameDomain&) const final;
private:
WebCore::ResourceLoadStatistics& ensureResourceStatisticsForRegistrableDomain(const WebCore::RegistrableDomain&);
@@ -88,6 +90,7 @@
HashSet<WebCore::RegistrableDomain> m_domainsWithUserInteraction;
HashMap<TopFrameDomain, HashSet<SubFrameDomain>> m_domainsWithCrossPageStorageAccess;
+ HashMap<TopFrameDomain, HashSet<SubFrameDomain>> m_domainsWithDeniedStorageAccess;
#if !RELEASE_LOG_DISABLED
uint64_t m_loggingCounter { 0 };
static bool shouldLogUserInteraction;
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp (274745 => 274746)
--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2021-03-19 23:25:07 UTC (rev 274745)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2021-03-19 23:27:45 UTC (rev 274746)
@@ -3826,6 +3826,15 @@
updatePreferences(store);
}
+bool WebPage::isParentProcessAWebBrowser() const
+{
+#if HAVE(AUDIT_TOKEN)
+ if (auto* connection = WebProcess::singleton().parentProcessConnection())
+ return isParentProcessAFullWebBrowser(connection->getAuditToken());
+#endif
+ return false;
+}
+
void WebPage::updatePreferences(const WebPreferencesStore& store)
{
updatePreferencesGenerated(store);
@@ -3932,10 +3941,8 @@
#endif
#if ENABLE(WEB_AUTHN) && PLATFORM(IOS)
- if (auto* connection = WebProcess::singleton().parentProcessConnection()) {
- if (isParentProcessAFullWebBrowser(connection->getAuditToken()))
- settings.setWebAuthenticationEnabled(true);
- }
+ if (isParentProcessAWebBrowser())
+ settings.setWebAuthenticationEnabled(true);
#endif
#if ENABLE(WEBM_FORMAT_READER)
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.h (274745 => 274746)
--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.h 2021-03-19 23:25:07 UTC (rev 274745)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.h 2021-03-19 23:27:45 UTC (rev 274746)
@@ -1413,6 +1413,8 @@
void setLastNavigationWasAppBound(bool wasAppBound) { m_lastNavigationWasAppBound = wasAppBound; }
void lastNavigationWasAppBound(CompletionHandler<void(bool)>&&);
+ bool isParentProcessAWebBrowser() const;
+
private:
WebPage(WebCore::PageIdentifier, WebPageCreationParameters&&);
