Attachment 373722 Details for Bug 199385 – Patch for landing

[patch] Patch for landing

bug-199385-20190709092900.patch (text/plain), 34.97 KB, created by Sihui Liu on 2019-07-09 09:29:01 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Sihui Liu

Created: 2019-07-09 09:29:01 PDT

Size: 34.97 KB

patch

obsolete

>Subversion Revision: 247207
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index ad415c97f98ac9b08d3aabe279a27aa566ed9b50..42568a58d3af7c1ade15113e50bc818a7bd82d79 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,29 @@
>+2019-07-09  Sihui Liu  <sihui_liu@apple.com>
>+
>+        Only allow fetching and removing session credentials from WebsiteDataStore
>+        https://bugs.webkit.org/show_bug.cgi?id=199385
>+        <rdar://problem/52622080>
>+
>+        Reviewed by Alex Christensen.
>+
>+        Fetch and remove only session credentials from NSURLCredentialStorage.
>+
>+        Modified existing API tests: WKWebsiteDataStore.FetchPersistentCredentials
>+
>+        * platform/network/CredentialStorage.cpp:
>+        (WebCore::CredentialStorage::originsWithCredentials const):
>+        (WebCore::CredentialStorage::originsWithSessionCredentials):
>+        (WebCore::CredentialStorage::removeSessionCredentialsWithOrigins):
>+        (WebCore::CredentialStorage::clearSessionCredentials):
>+        (WebCore::CredentialStorage::clearPermanentCredentialsForProtectionSpace):
>+        * platform/network/CredentialStorage.h:
>+        * platform/network/mac/CredentialStorageMac.mm:
>+        (WebCore::CredentialStorage::originsWithSessionCredentials):
>+        (WebCore::CredentialStorage::removeSessionCredentialsWithOrigins):
>+        (WebCore::CredentialStorage::clearSessionCredentials):
>+        (WebCore::CredentialStorage::clearPermanentCredentialsForProtectionSpace):
>+        (WebCore::CredentialStorage::originsWithPersistentCredentials): Deleted.
>+
> 2019-07-08  Antoine Quint  <graouts@apple.com>
> 
>         [Pointer Events] Enable only on the most recent version of the supported iOS family
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 03ef68d1c17eda36b0d281685377e41ef4079f03..b98013e65fe08904883cac108b5e37742afc51ef 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,39 @@
>+2019-07-09  Sihui Liu  <sihui_liu@apple.com>
>+
>+        Only allow fetching and removing session credentials from WebsiteDataStore
>+        https://bugs.webkit.org/show_bug.cgi?id=199385
>+        <rdar://problem/52622080>
>+
>+        Reviewed by Alex Christensen.
>+
>+        Stop sending an extra message to network process for fetching or removing persistent credentials.
>+
>+        Also introduce a new SPI for clearing persistent credentials created in test.
>+
>+        * NetworkProcess/NetworkProcess.cpp:
>+        (WebKit::NetworkProcess::clearPermanentCredentialsForProtectionSpace):
>+        (WebKit::NetworkProcess::fetchWebsiteData):
>+        (WebKit::NetworkProcess::deleteWebsiteData):
>+        (WebKit::NetworkProcess::deleteWebsiteDataForOrigins):
>+        (WebKit::NetworkProcess::deleteWebsiteDataForRegistrableDomains):
>+        (WebKit::NetworkProcess::originsWithPersistentCredentials): Deleted.
>+        (WebKit::NetworkProcess::removeCredentialsWithOrigins): Deleted.
>+        * NetworkProcess/NetworkProcess.h:
>+        * NetworkProcess/NetworkProcess.messages.in:
>+        * NetworkProcess/cocoa/NetworkProcessCocoa.mm:
>+        (WebKit::NetworkProcess::originsWithPersistentCredentials): Deleted.
>+        (WebKit::NetworkProcess::removeCredentialsWithOrigins): Deleted.
>+        * UIProcess/API/Cocoa/WKProcessPool.mm:
>+        (-[WKProcessPool _clearPermanentCredentialsForProtectionSpace:completionHandler:]):
>+        * UIProcess/API/Cocoa/WKProcessPoolPrivate.h:
>+        * UIProcess/WebProcessPool.cpp:
>+        (WebKit::WebProcessPool::clearPermanentCredentialsForProtectionSpace):
>+        * UIProcess/WebProcessPool.h:
>+        * UIProcess/WebsiteData/WebsiteDataStore.cpp:
>+        (WebKit::WebsiteDataStore::fetchDataAndApply):
>+        (WebKit::computeWebProcessAccessTypeForDataRemoval):
>+        (WebKit::WebsiteDataStore::removeData):
>+
> 2019-07-07  Antoine Quint  <graouts@apple.com>
> 
>         [Pointer Events] touch-action should affect the behavior of pinch-to-zoom to show tabs in Safari
>diff --git a/Source/WebCore/platform/network/CredentialStorage.cpp b/Source/WebCore/platform/network/CredentialStorage.cpp
>index 98e9426fc27adbb2bb60bed46dd5667faf7c3962..0136b9ecdf9f2f0a5a2aeb74cf4082b81b1afd42 100644
>--- a/Source/WebCore/platform/network/CredentialStorage.cpp
>+++ b/Source/WebCore/platform/network/CredentialStorage.cpp
>@@ -102,9 +102,9 @@ void CredentialStorage::removeCredentialsWithOrigin(const SecurityOriginData& or
>         remove(key.first, key.second);
> }
> 
>-Vector<SecurityOriginData> CredentialStorage::originsWithCredentials() const
>+HashSet<SecurityOriginData> CredentialStorage::originsWithCredentials() const
> {
>-    Vector<SecurityOriginData> origins;
>+    HashSet<SecurityOriginData> origins;
>     for (auto& keyValuePair : m_protectionSpaceToCredentialMap) {
>         auto& protectionSpace = keyValuePair.key.second;
>         if (protectionSpace.isProxy())
>@@ -129,7 +129,7 @@ Vector<SecurityOriginData> CredentialStorage::originsWithCredentials() const
>         }
> 
>         SecurityOriginData origin { protocol, protectionSpace.host(), static_cast<uint16_t>(protectionSpace.port())};
>-        origins.append(WTFMove(origin));
>+        origins.add(WTFMove(origin));
>     }
>     return origins;
> }
>@@ -187,4 +187,23 @@ void CredentialStorage::clearCredentials()
>     m_pathToDefaultProtectionSpaceMap.clear();
> }
> 
>+#if !PLATFORM(COCOA)
>+HashSet<SecurityOriginData> CredentialStorage::originsWithSessionCredentials()
>+{
>+    return { };
>+}
>+
>+void CredentialStorage::removeSessionCredentialsWithOrigins(const Vector<SecurityOriginData>&)
>+{
>+}
>+
>+void CredentialStorage::clearSessionCredentials()
>+{
>+}
>+
>+void CredentialStorage::clearPermanentCredentialsForProtectionSpace(const ProtectionSpace&)
>+{
>+}
>+#endif
>+
> } // namespace WebCore
>diff --git a/Source/WebCore/platform/network/CredentialStorage.h b/Source/WebCore/platform/network/CredentialStorage.h
>index f8c75ae929ab2d5425a24a6267cd42738d75c0d7..cd853c64f024c5a0f78ec5d27c70305d4d95ab75 100644
>--- a/Source/WebCore/platform/network/CredentialStorage.h
>+++ b/Source/WebCore/platform/network/CredentialStorage.h
>@@ -45,9 +45,12 @@ public:
>     WEBCORE_EXPORT void remove(const String&, const ProtectionSpace&);
>     WEBCORE_EXPORT void removeCredentialsWithOrigin(const SecurityOriginData&);
> 
>-    // OS persistent storage.
>+    // OS credential storage.
>     WEBCORE_EXPORT static Credential getFromPersistentStorage(const ProtectionSpace&);
>-    WEBCORE_EXPORT static Vector<SecurityOriginData> originsWithPersistentCredentials();
>+    WEBCORE_EXPORT static HashSet<SecurityOriginData> originsWithSessionCredentials();
>+    WEBCORE_EXPORT static void removeSessionCredentialsWithOrigins(const Vector<SecurityOriginData>& origins);
>+    WEBCORE_EXPORT static void clearSessionCredentials();
>+    WEBCORE_EXPORT static void clearPermanentCredentialsForProtectionSpace(const ProtectionSpace&);
> 
>     WEBCORE_EXPORT void clearCredentials();
> 
>@@ -56,7 +59,7 @@ public:
>     WEBCORE_EXPORT bool set(const String&, const Credential&, const URL&); // Returns true if the URL corresponds to a known protection space, so credentials could be updated.
>     WEBCORE_EXPORT Credential get(const String&, const URL&);
> 
>-    WEBCORE_EXPORT Vector<SecurityOriginData> originsWithCredentials() const;
>+    WEBCORE_EXPORT HashSet<SecurityOriginData> originsWithCredentials() const;
> 
> private:
>     HashMap<std::pair<String /* partitionName */, ProtectionSpace>, Credential> m_protectionSpaceToCredentialMap;
>diff --git a/Source/WebCore/platform/network/mac/CredentialStorageMac.mm b/Source/WebCore/platform/network/mac/CredentialStorageMac.mm
>index 65b51511f9a9b2b8b12f63edae36fef35e76d39c..6ea0bab2b07b8b76091caf37b390871893d629c7 100644
>--- a/Source/WebCore/platform/network/mac/CredentialStorageMac.mm
>+++ b/Source/WebCore/platform/network/mac/CredentialStorageMac.mm
>@@ -38,13 +38,69 @@ Credential CredentialStorage::getFromPersistentStorage(const ProtectionSpace& pr
>     return credential ? Credential(credential) : Credential();
> }
> 
>-Vector<WebCore::SecurityOriginData> CredentialStorage::originsWithPersistentCredentials()
>+HashSet<SecurityOriginData> CredentialStorage::originsWithSessionCredentials()
> {
>-    Vector<WebCore::SecurityOriginData> origins;
>+    HashSet<SecurityOriginData> origins;
>     auto allCredentials = [[NSURLCredentialStorage sharedCredentialStorage] allCredentials];
>-    for (NSURLProtectionSpace* key in allCredentials.keyEnumerator)
>-        origins.append(WebCore::SecurityOriginData { String(key.protocol), String(key.host), key.port });
>+    for (NSURLProtectionSpace* key in allCredentials.keyEnumerator) {
>+        for (NSURLProtectionSpace* space in allCredentials) {
>+            auto credentials = allCredentials[space];
>+            for (NSString* user in credentials) {
>+                if (credentials[user].persistence == NSURLCredentialPersistenceForSession) {
>+                    origins.add(WebCore::SecurityOriginData { String(key.protocol), String(key.host), key.port });
>+                    break;
>+                }
>+            }
>+        }
>+    }
>     return origins;
> }
> 
>+void CredentialStorage::removeSessionCredentialsWithOrigins(const Vector<SecurityOriginData>& origins)
>+{
>+    auto sharedStorage = [NSURLCredentialStorage sharedCredentialStorage];
>+    auto allCredentials = [sharedStorage allCredentials];
>+    for (auto& origin : origins) {
>+        for (NSURLProtectionSpace* space in allCredentials) {
>+            if (origin.protocol == String(space.protocol)
>+                && origin.host == String(space.host)
>+                && origin.port
>+                && *origin.port == space.port) {
>+                    auto credentials = allCredentials[space];
>+                    for (NSString* user in credentials) {
>+                        auto credential = credentials[user];
>+                        if (credential.persistence == NSURLCredentialPersistenceForSession)
>+                            [sharedStorage removeCredential:credential forProtectionSpace:space];
>+                }
>+            }
>+        }
>+    }
>+}
>+
>+void CredentialStorage::clearSessionCredentials()
>+{
>+    auto sharedStorage = [NSURLCredentialStorage sharedCredentialStorage];
>+    auto allCredentials = [sharedStorage allCredentials];
>+    for (NSURLProtectionSpace* space in allCredentials.keyEnumerator) {
>+        auto credentials = allCredentials[space];
>+        for (NSString* user in credentials) {
>+            auto credential = credentials[user];
>+            if (credential.persistence == NSURLCredentialPersistenceForSession)
>+                [sharedStorage removeCredential:credential forProtectionSpace:space];
>+        }
>+    }
>+}
>+
>+void CredentialStorage::clearPermanentCredentialsForProtectionSpace(const ProtectionSpace& protectionSpace)
>+{
>+    auto sharedStorage = [NSURLCredentialStorage sharedCredentialStorage];
>+    auto allCredentials = [sharedStorage allCredentials];
>+    auto credentials = allCredentials[protectionSpace.nsSpace()];
>+    for (NSString* user in credentials) {
>+        auto credential = credentials[user];
>+        if (credential.persistence == NSURLCredentialPersistencePermanent)
>+            [sharedStorage removeCredential:credentials[user] forProtectionSpace:protectionSpace.nsSpace()];
>+    }
>+}
>+
> } // namespace WebCore
>diff --git a/Source/WebKit/NetworkProcess/NetworkProcess.cpp b/Source/WebKit/NetworkProcess/NetworkProcess.cpp
>index e8e15ca087fc8210644716bbded60333470d87f5..f8bb7678ee38684b2294d18409915028e4379da0 100644
>--- a/Source/WebKit/NetworkProcess/NetworkProcess.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkProcess.cpp
>@@ -455,6 +455,12 @@ void NetworkProcess::clearCachedCredentials()
>         ASSERT_NOT_REACHED();
> }
> 
>+void NetworkProcess::clearPermanentCredentialsForProtectionSpace(const ProtectionSpace& protectionSpace, CompletionHandler<void()>&& completionHandler)
>+{
>+    WebCore::CredentialStorage::clearPermanentCredentialsForProtectionSpace(protectionSpace);
>+    completionHandler();
>+}
>+
> void NetworkProcess::addWebsiteDataStore(WebsiteDataStoreParameters&& parameters)
> {
> #if ENABLE(INDEXED_DATABASE)
>@@ -1298,6 +1304,9 @@ void NetworkProcess::fetchWebsiteData(PAL::SessionID sessionID, OptionSet<Websit
>             for (auto& securityOrigin : securityOrigins)
>                 callbackAggregator->m_websiteData.entries.append({ securityOrigin, WebsiteDataType::Credentials, 0 });
>         }
>+        auto securityOrigins = WebCore::CredentialStorage::originsWithSessionCredentials();
>+        for (auto& securityOrigin : securityOrigins)
>+            callbackAggregator->m_websiteData.entries.append({ securityOrigin, WebsiteDataType::Credentials, 0 });
>     }
> 
>     if (websiteDataTypes.contains(WebsiteDataType::DOMCache)) {
>@@ -1379,6 +1388,7 @@ void NetworkProcess::deleteWebsiteData(PAL::SessionID sessionID, OptionSet<Websi
>     if (websiteDataTypes.contains(WebsiteDataType::Credentials)) {
>         if (auto* session = storageSession(sessionID))
>             session->credentialStorage().clearCredentials();
>+        WebCore::CredentialStorage::clearSessionCredentials();
>     }
> 
>     auto clearTasksHandler = WTF::CallbackAggregator::create([this, callbackID] {
>@@ -1516,6 +1526,7 @@ void NetworkProcess::deleteWebsiteDataForOrigins(PAL::SessionID sessionID, Optio
>             for (auto& originData : originDatas)
>                 session->credentialStorage().removeCredentialsWithOrigin(originData);
>         }
>+        WebCore::CredentialStorage::removeSessionCredentialsWithOrigins(originDatas);
>     }
> 
>     // FIXME: Implement storage quota clearing for these origins.
>@@ -1660,14 +1671,18 @@ void NetworkProcess::deleteWebsiteDataForRegistrableDomains(PAL::SessionID sessi
>     }
> #endif
> 
>-    /*
>-    // FIXME: No API to delete credentials by origin
>-    HashSet<String> originsWithCredentials;
>     if (websiteDataTypes.contains(WebsiteDataType::Credentials)) {
>-        if (storageSession(sessionID))
>-            originsWithCredentials = storageSession(sessionID)->credentialStorage().originsWithCredentials();
>+        if (auto* session = storageSession(sessionID)) {
>+            auto origins = session->credentialStorage().originsWithCredentials();
>+            auto originsToDelete = filterForRegistrableDomains(origins, domainsToDeleteAllButCookiesFor, callbackAggregator->m_domains);
>+            for (auto& origin : originsToDelete)
>+                session->credentialStorage().removeCredentialsWithOrigin(origin);
>+        }
>+
>+        auto origins = WebCore::CredentialStorage::originsWithSessionCredentials();
>+        auto originsToDelete = filterForRegistrableDomains(origins, domainsToDeleteAllButCookiesFor, callbackAggregator->m_domains);
>+        WebCore::CredentialStorage::removeSessionCredentialsWithOrigins(originsToDelete);
>     }
>-    */
>     
>     if (websiteDataTypes.contains(WebsiteDataType::DOMCache)) {
>         CacheStorage::Engine::fetchEntries(*this, sessionID, fetchOptions.contains(WebsiteDataFetchOption::ComputeSizes), [this, domainsToDeleteAllButCookiesFor, sessionID, callbackAggregator = callbackAggregator.copyRef()](auto entries) mutable {
>@@ -2563,16 +2578,6 @@ StorageQuotaManager& NetworkProcess::storageQuotaManager(PAL::SessionID sessionI
> }
> 
> #if !PLATFORM(COCOA)
>-void NetworkProcess::originsWithPersistentCredentials(CompletionHandler<void(Vector<WebCore::SecurityOriginData>)>&& completionHandler)
>-{
>-    completionHandler(Vector<WebCore::SecurityOriginData>());
>-}
>-
>-void NetworkProcess::removeCredentialsWithOrigins(const Vector<WebCore::SecurityOriginData>&, CompletionHandler<void()>&& completionHandler)
>-{
>-    completionHandler();
>-}
>-
> void NetworkProcess::initializeProcess(const AuxiliaryProcessInitializationParameters&)
> {
> }
>diff --git a/Source/WebKit/NetworkProcess/NetworkProcess.h b/Source/WebKit/NetworkProcess/NetworkProcess.h
>index 83b4f3484640fbb3c456d71bb54f17b020f84761..db86f249d42a049eff3254dc79f1c7d2d9005bfd 100644
>--- a/Source/WebKit/NetworkProcess/NetworkProcess.h
>+++ b/Source/WebKit/NetworkProcess/NetworkProcess.h
>@@ -393,6 +393,7 @@ private:
>     void deleteWebsiteDataForOrigins(PAL::SessionID, OptionSet<WebsiteDataType>, const Vector<WebCore::SecurityOriginData>& origins, const Vector<String>& cookieHostNames, const Vector<String>& HSTSCacheHostnames, uint64_t callbackID);
> 
>     void clearCachedCredentials();
>+    void clearPermanentCredentialsForProtectionSpace(const WebCore::ProtectionSpace&, CompletionHandler<void()>&&);
> 
>     void setCacheStorageParameters(PAL::SessionID, String&& cacheStorageDirectory, SandboxExtension::Handle&&);
>     void initializeQuotaUsers(WebCore::StorageQuotaManager&, PAL::SessionID, const WebCore::ClientOrigin&);
>@@ -436,9 +437,6 @@ private:
> #endif
> 
>     void platformSyncAllCookies(CompletionHandler<void()>&&);
>-
>-    void originsWithPersistentCredentials(CompletionHandler<void(Vector<WebCore::SecurityOriginData>)>&&);
>-    void removeCredentialsWithOrigins(const Vector<WebCore::SecurityOriginData>& origins, CompletionHandler<void()>&&);
>     
>     void registerURLSchemeAsSecure(const String&) const;
>     void registerURLSchemeAsBypassingContentSecurityPolicy(const String&) const;
>diff --git a/Source/WebKit/NetworkProcess/NetworkProcess.messages.in b/Source/WebKit/NetworkProcess/NetworkProcess.messages.in
>index 6d0624083f75f332726ab3fb21dc1d2cff90c21c..9be16d8cdf6f1e99dda18befd5ee6deecfdd58af 100644
>--- a/Source/WebKit/NetworkProcess/NetworkProcess.messages.in
>+++ b/Source/WebKit/NetworkProcess/NetworkProcess.messages.in
>@@ -38,6 +38,7 @@ messages -> NetworkProcess LegacyReceiver {
> #endif
> 
>     ClearCachedCredentials()
>+    ClearPermanentCredentialsForProtectionSpace(WebCore::ProtectionSpace protectionSpace) -> () Async
> 
>     AddWebsiteDataStore(struct WebKit::WebsiteDataStoreParameters websiteDataStoreParameters);
>     DestroySession(PAL::SessionID sessionID)
>@@ -168,7 +169,5 @@ messages -> NetworkProcess LegacyReceiver {
>     SetAdClickAttributionOverrideTimerForTesting(PAL::SessionID sessionID, bool value) -> () Async
>     SetAdClickAttributionConversionURLForTesting(PAL::SessionID sessionID, URL url) -> () Async
>     MarkAdClickAttributionsAsExpiredForTesting(PAL::SessionID sessionID) -> () Async
>-    OriginsWithPersistentCredentials() -> (Vector<WebCore::SecurityOriginData> origins) Async
>-    RemoveCredentialsWithOrigins(Vector<WebCore::SecurityOriginData> origins) -> () Async
>     GetLocalStorageOriginDetails(PAL::SessionID sessionID) -> (Vector<WebKit::LocalStorageDatabaseTracker::OriginDetails> details) Async
> }
>diff --git a/Source/WebKit/NetworkProcess/cocoa/NetworkProcessCocoa.mm b/Source/WebKit/NetworkProcess/cocoa/NetworkProcessCocoa.mm
>index bdefe9e4fa59006d5eb597bb7c05d361dd7918a6..5b52cabc0d997ec589a37a5089c9145b28aead3a 100644
>--- a/Source/WebKit/NetworkProcess/cocoa/NetworkProcessCocoa.mm
>+++ b/Source/WebKit/NetworkProcess/cocoa/NetworkProcessCocoa.mm
>@@ -212,31 +212,6 @@ void NetworkProcess::clearDiskCache(WallTime modifiedSince, CompletionHandler<vo
>     }).get());
> }
> 
>-void NetworkProcess::originsWithPersistentCredentials(CompletionHandler<void(Vector<WebCore::SecurityOriginData>)>&& completionHandler)
>-{
>-    completionHandler(WebCore::CredentialStorage::originsWithPersistentCredentials());
>-}
>-
>-void NetworkProcess::removeCredentialsWithOrigins(const Vector<WebCore::SecurityOriginData>& origins, CompletionHandler<void()>&& completionHandler)
>-{
>-    for (auto& origin : origins) {
>-        auto allCredentials = [[NSURLCredentialStorage sharedCredentialStorage] allCredentials];
>-        for (NSURLProtectionSpace* space in allCredentials) {
>-            if (origin.protocol == String(space.protocol)
>-                && origin.host == String(space.host)
>-                && origin.port
>-                && *origin.port == space.port) {
>-                auto credentials = allCredentials[space];
>-                for (NSString* user in credentials) {
>-                    auto credential = credentials[user];
>-                    [[NSURLCredentialStorage sharedCredentialStorage] removeCredential:credential forProtectionSpace:space];
>-                }
>-            }
>-        }
>-    }
>-    completionHandler();
>-}
>-
> #if PLATFORM(MAC)
> void NetworkProcess::setSharedHTTPCookieStorage(const Vector<uint8_t>& identifier)
> {
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm b/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm
>index c06e1afb33ffa318b7e29dfdc4f067957a838bf2..cae9d7896bc9b123e4c7c066a633229b23b03a18 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm
>@@ -631,4 +631,11 @@ - (BOOL)_networkProcessHasEntitlementForTesting:(NSString *)entitlement
>     return _processPool->networkProcessHasEntitlementForTesting(entitlement);
> }
> 
>+- (void)_clearPermanentCredentialsForProtectionSpace:(NSURLProtectionSpace *)protectionSpace completionHandler:(void(^)())completionHandler
>+{
>+    _processPool->clearPermanentCredentialsForProtectionSpace(WebCore::ProtectionSpace(protectionSpace), [completionHandler = makeBlockPtr(completionHandler)] {
>+        completionHandler();
>+    });
>+}
>+
> @end
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h b/Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h
>index 397321de800980e37395b2d2e14f7602a0f905b2..cb15a3e4f44d7a3e8cdbbbfe9bd1fc23f766eceb 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h
>@@ -120,6 +120,7 @@
> - (void)_registerURLSchemeServiceWorkersCanHandle:(NSString *)scheme WK_API_AVAILABLE(macos(10.13.4), ios(11.3));
> - (void)_getActivePagesOriginsInWebProcessForTesting:(pid_t)pid completionHandler:(void(^)(NSArray<NSString *> *))completionHandler WK_API_AVAILABLE(macos(10.14.4), ios(12.2));
> - (BOOL)_networkProcessHasEntitlementForTesting:(NSString *)entitlement WK_API_AVAILABLE(macos(10.14.4), ios(12.2));
>+- (void)_clearPermanentCredentialsForProtectionSpace:(NSURLProtectionSpace *)protectionSpace completionHandler:(void(^)(void))completionHandler WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
> 
> @property (nonatomic, getter=_isCookieStoragePartitioningEnabled, setter=_setCookieStoragePartitioningEnabled:) BOOL _cookieStoragePartitioningEnabled WK_API_DEPRECATED("Partitioned cookies are no longer supported", macos(10.12.3, 10.14.4), ios(10.3, 12.2));
> @property (nonatomic, getter=_isStorageAccessAPIEnabled, setter=_setStorageAccessAPIEnabled:) BOOL _storageAccessAPIEnabled WK_API_AVAILABLE(macos(10.13.4), ios(11.3));
>diff --git a/Source/WebKit/UIProcess/WebProcessPool.cpp b/Source/WebKit/UIProcess/WebProcessPool.cpp
>index 47036b3d2781ac1fe3f6be695285c71bf9ad457f..a20fa29249e9241212a25ec6d8aea72c865858cf 100644
>--- a/Source/WebKit/UIProcess/WebProcessPool.cpp
>+++ b/Source/WebKit/UIProcess/WebProcessPool.cpp
>@@ -1744,6 +1744,12 @@ void WebProcessPool::clearCachedCredentials()
>         m_networkProcess->send(Messages::NetworkProcess::ClearCachedCredentials(), 0);
> }
> 
>+void WebProcessPool::clearPermanentCredentialsForProtectionSpace(WebCore::ProtectionSpace&& protectionSpace, CompletionHandler<void()>&& completionHandler)
>+{
>+    if (m_networkProcess)
>+        m_networkProcess->sendWithAsyncReply(Messages::NetworkProcess::ClearPermanentCredentialsForProtectionSpace(protectionSpace), WTFMove(completionHandler));
>+}
>+
> void WebProcessPool::terminateNetworkProcess()
> {
>     if (!m_networkProcess)
>diff --git a/Source/WebKit/UIProcess/WebProcessPool.h b/Source/WebKit/UIProcess/WebProcessPool.h
>index 9969112d69b3c38c42c9f7a60a93a2cccd10e453..06027b6a8e6061806eb649fc675982187203d20c 100644
>--- a/Source/WebKit/UIProcess/WebProcessPool.h
>+++ b/Source/WebKit/UIProcess/WebProcessPool.h
>@@ -310,6 +310,7 @@ public:
>     void setAllowsAnySSLCertificateForWebSocket(bool);
> 
>     void clearCachedCredentials();
>+    void clearPermanentCredentialsForProtectionSpace(WebCore::ProtectionSpace&&, CompletionHandler<void()>&&);
>     void terminateNetworkProcess();
>     void sendNetworkProcessWillSuspendImminently();
>     void sendNetworkProcessDidResume();
>diff --git a/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp b/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
>index 0eec80127a61502e3fb0318a976fa95ba36fa24c..5ab53da0d26eefb4304cd692ee814ce8e3870bf9 100644
>--- a/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
>+++ b/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
>@@ -520,24 +520,6 @@ void WebsiteDataStore::fetchDataAndApply(OptionSet<WebsiteDataType> dataTypes, O
>         });
>     }
> 
>-#if PLATFORM(COCOA)
>-    if (dataTypes.contains(WebsiteDataType::Credentials) && isPersistent()) {
>-        for (auto& processPool : processPools()) {
>-            if (!processPool->networkProcess())
>-                continue;
>-            
>-            callbackAggregator->addPendingCallback();
>-            WTF::CompletionHandler<void(Vector<WebCore::SecurityOriginData>&&)> completionHandler = [callbackAggregator](Vector<WebCore::SecurityOriginData>&& origins) mutable {
>-                WebsiteData websiteData;
>-                for (auto& origin : origins)
>-                    websiteData.entries.append(WebsiteData::Entry { origin, WebsiteDataType::Credentials, 0 });
>-                callbackAggregator->removePendingCallback(WTFMove(websiteData));
>-            };
>-            processPool->networkProcess()->sendWithAsyncReply(Messages::NetworkProcess::OriginsWithPersistentCredentials(), WTFMove(completionHandler));
>-        }
>-    }
>-#endif
>-
> #if ENABLE(NETSCAPE_PLUGIN_API)
>     if (dataTypes.contains(WebsiteDataType::PlugInData) && isPersistent()) {
>         class State {
>@@ -644,9 +626,6 @@ static ProcessAccessType computeWebProcessAccessTypeForDataRemoval(OptionSet<Web
>     if (dataTypes.contains(WebsiteDataType::MemoryCache))
>         processAccessType = std::max(processAccessType, ProcessAccessType::OnlyIfLaunched);
> 
>-    if (dataTypes.contains(WebsiteDataType::Credentials))
>-        processAccessType = std::max(processAccessType, ProcessAccessType::OnlyIfLaunched);
>-
>     return processAccessType;
> }
> 
>@@ -1093,19 +1072,6 @@ void WebsiteDataStore::removeData(OptionSet<WebsiteDataType> dataTypes, const Ve
>         });
>     }
> 
>-    if (dataTypes.contains(WebsiteDataType::Credentials) && isPersistent()) {
>-        for (auto& processPool : processPools()) {
>-            if (!processPool->networkProcess())
>-                continue;
>-            
>-            callbackAggregator->addPendingCallback();
>-            WTF::CompletionHandler<void()> completionHandler = [callbackAggregator]() mutable {
>-                callbackAggregator->removePendingCallback();
>-            };
>-            processPool->networkProcess()->sendWithAsyncReply(Messages::NetworkProcess::RemoveCredentialsWithOrigins(origins), WTFMove(completionHandler));
>-        }
>-    }
>-
> #if ENABLE(NETSCAPE_PLUGIN_API)
>     if (dataTypes.contains(WebsiteDataType::PlugInData) && isPersistent()) {
>         Vector<String> hostNames;
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 64f079d784818e71add312992451f8e53c7e7c64..72103cea7bbd47e5645bbbe370d70c043ed030c6 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,20 @@
>+2019-07-09  Sihui Liu  <sihui_liu@apple.com>
>+
>+        Only allow fetching and removing session credentials from WebsiteDataStore
>+        https://bugs.webkit.org/show_bug.cgi?id=199385
>+        <rdar://problem/52622080>
>+
>+        Reviewed by Alex Christensen.
>+
>+        removeDataOfTypes will no longer remove persistent credentials. We should clear persistent credentials using
>+        the new SPI after each test that creates persistent credentials, otherwise the later tests may use credentials
>+        left by previous tests and didReceiveAuthenticationChallenge will not be invoked.
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/Challenge.mm:
>+        (TEST):
>+        * TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm:
>+        (TestWebKitAPI::TEST):
>+
> 2019-07-08  Antoine Quint  <graouts@apple.com>
> 
>         [Pointer Events] Enable only on the most recent version of the supported iOS family
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/Challenge.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/Challenge.mm
>index a7db07795ac1b708e20bdf8aa59ba7857aed288e..99835df12d80abf2433e48abc0cbf18e492a3b46 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/Challenge.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/Challenge.mm
>@@ -103,6 +103,14 @@ TEST(Challenge, SecIdentity)
>     [webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:[NSString stringWithFormat:@"http://127.0.0.1:%d/", server.port()]]]];
> 
>     Util::run(&navigationFinished);
>+
>+    // Clear persistent credentials created by this test.
>+    NSURLProtectionSpace *protectionSpace = [[[NSURLProtectionSpace alloc] initWithHost:@"127.0.0.1" port:server.port() protocol:NSURLProtectionSpaceHTTP realm:@"testrealm" authenticationMethod:NSURLAuthenticationMethodHTTPBasic] autorelease];
>+    __block bool removedCredential = false;
>+    [[webView configuration].processPool _clearPermanentCredentialsForProtectionSpace:protectionSpace completionHandler:^{
>+        removedCredential = true;
>+    }];
>+    Util::run(&removedCredential);
> }
> 
> @interface ClientCertificateDelegate : NSObject <WKNavigationDelegate> {
>@@ -198,13 +206,12 @@ TEST(Challenge, BasicProposedCredential)
>     [webView loadRequest:request.get()];
>     Util::run(&navigationFinished);
>     EXPECT_TRUE(receivedSecondChallenge);
>-    
>+
>+    // Clear persistent credentials created by this test.
>+    NSURLProtectionSpace *protectionSpace = [[[NSURLProtectionSpace alloc] initWithHost:@"127.0.0.1" port:server.port() protocol:NSURLProtectionSpaceHTTP realm:@"testrealm" authenticationMethod:NSURLAuthenticationMethodHTTPBasic] autorelease];
>     __block bool removedCredential = false;
>-    WKWebsiteDataStore *websiteDataStore = [webView configuration].websiteDataStore;
>-    [websiteDataStore fetchDataRecordsOfTypes:[NSSet setWithObject:_WKWebsiteDataTypeCredentials] completionHandler:^(NSArray<WKWebsiteDataRecord *> *dataRecords) {
>-        [websiteDataStore removeDataOfTypes:[NSSet setWithObject:_WKWebsiteDataTypeCredentials] forDataRecords:dataRecords completionHandler:^(void) {
>-            removedCredential = true;
>-        }];
>+    [[webView configuration].processPool _clearPermanentCredentialsForProtectionSpace:protectionSpace completionHandler:^{
>+        removedCredential = true;
>     }];
>     Util::run(&removedCredential);
> }
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm
>index fae0a34bb33158767af536fa1377cfabdc471fe4..b94bc730a78222ea1100bb9aecc18e8c3e605f74 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm
>@@ -92,7 +92,7 @@ TEST(WKWebsiteDataStore, RemoveAndFetchData)
>     
>     readyToContinue = false;
>     [[WKWebsiteDataStore defaultDataStore] fetchDataRecordsOfTypes:[WKWebsiteDataStore _allWebsiteDataTypesIncludingPrivate] completionHandler:^(NSArray<WKWebsiteDataRecord *> *dataRecords) {
>-        ASSERT_EQ(0u, dataRecords.count);
>+        EXPECT_EQ(0u, dataRecords.count);
>         readyToContinue = true;
>     }];
>     TestWebKitAPI::Util::run(&readyToContinue);
>@@ -137,44 +137,6 @@ TEST(WKWebsiteDataStore, FetchNonPersistentCredentials)
> }
> 
> TEST(WKWebsiteDataStore, FetchPersistentCredentials)
>-{
>-    TCPServer server(TCPServer::respondWithChallengeThenOK);
>-    
>-    usePersistentCredentialStorage = true;
>-    auto websiteDataStore = [WKWebsiteDataStore defaultDataStore];
>-    auto navigationDelegate = adoptNS([[NavigationTestDelegate alloc] init]);
>-    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
>-    [webView setNavigationDelegate:navigationDelegate.get()];
>-    [webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:[NSString stringWithFormat:@"http://127.0.0.1:%d/", server.port()]]]];
>-    [navigationDelegate waitForDidFinishNavigation];
>-
>-    __block bool done = false;
>-    [websiteDataStore fetchDataRecordsOfTypes:[NSSet setWithObject:_WKWebsiteDataTypeCredentials] completionHandler:^(NSArray<WKWebsiteDataRecord *> *dataRecords) {
>-        int credentialCount = dataRecords.count;
>-        ASSERT_GT(credentialCount, 0);
>-        bool foundExpectedRecord = false;
>-        for (WKWebsiteDataRecord *record in dataRecords) {
>-            auto name = [record displayName];
>-            if ([name isEqualToString:@"127.0.0.1"]) {
>-                foundExpectedRecord = true;
>-                break;
>-            }
>-        }
>-        EXPECT_TRUE(foundExpectedRecord);
>-        done = true;
>-    }];
>-    TestWebKitAPI::Util::run(&done);
>-    
>-    __block bool removedCredential = false;
>-    [websiteDataStore fetchDataRecordsOfTypes:[NSSet setWithObject:_WKWebsiteDataTypeCredentials] completionHandler:^(NSArray<WKWebsiteDataRecord *> *dataRecords) {
>-        [websiteDataStore removeDataOfTypes:[NSSet setWithObject:_WKWebsiteDataTypeCredentials] forDataRecords:dataRecords completionHandler:^(void) {
>-            removedCredential = true;
>-        }];
>-    }];
>-    TestWebKitAPI::Util::run(&removedCredential);
>-}
>-
>-TEST(WKWebsiteDataStore, RemovePersistentCredentials)
> {
>     TCPServer server(TCPServer::respondWithChallengeThenOK);
> 
>@@ -187,42 +149,20 @@ TEST(WKWebsiteDataStore, RemovePersistentCredentials)
>     [navigationDelegate waitForDidFinishNavigation];
> 
>     __block bool done = false;
>-    __block RetainPtr<WKWebsiteDataRecord> expectedRecord;
>     [websiteDataStore fetchDataRecordsOfTypes:[NSSet setWithObject:_WKWebsiteDataTypeCredentials] completionHandler:^(NSArray<WKWebsiteDataRecord *> *dataRecords) {
>         int credentialCount = dataRecords.count;
>-        ASSERT_GT(credentialCount, 0);
>-        for (WKWebsiteDataRecord *record in dataRecords) {
>-            auto name = [record displayName];
>-            if ([name isEqualToString:@"127.0.0.1"]) {
>-                expectedRecord = record;
>-                break;
>-            }
>-        }
>-        EXPECT_TRUE(expectedRecord);
>-        done = true;
>-    }];
>-    TestWebKitAPI::Util::run(&done);
>-
>-    done = false;
>-    [websiteDataStore removeDataOfTypes:[NSSet setWithObject:_WKWebsiteDataTypeCredentials] forDataRecords:[NSArray arrayWithObject:expectedRecord.get()] completionHandler:^(void) {
>+        EXPECT_EQ(credentialCount, 0);
>         done = true;
>     }];
>     TestWebKitAPI::Util::run(&done);
> 
>-    done = false;
>-    [websiteDataStore fetchDataRecordsOfTypes:[NSSet setWithObject:_WKWebsiteDataTypeCredentials] completionHandler:^(NSArray<WKWebsiteDataRecord *> *dataRecords) {
>-        bool foundLocalHostRecord = false;
>-        for (WKWebsiteDataRecord *record in dataRecords) {
>-            auto name = [record displayName];
>-            if ([name isEqualToString:@"127.0.0.1"]) {
>-                foundLocalHostRecord = true;
>-                break;
>-            }
>-        }
>-        EXPECT_FALSE(foundLocalHostRecord);
>-        done = true;
>+    // Clear persistent credentials created by this test.
>+    NSURLProtectionSpace *protectionSpace = [[[NSURLProtectionSpace alloc] initWithHost:@"127.0.0.1" port:server.port() protocol:NSURLProtectionSpaceHTTP realm:@"testrealm" authenticationMethod:NSURLAuthenticationMethodHTTPBasic] autorelease];
>+    __block bool removedCredential = false;
>+    [[webView configuration].processPool _clearPermanentCredentialsForProtectionSpace:protectionSpace completionHandler:^{
>+        removedCredential = true;
>     }];
>-    TestWebKitAPI::Util::run(&done);
>+    Util::run(&removedCredential);
> }
> 
> TEST(WKWebsiteDataStore, RemoveNonPersistentCredentials)

Actions: View | Formatted Diff | Diff

Attachments on bug 199385: 373275 | 373281 | 373307 | 373310 | 373328 | 373436 | 373591 | 373722