WebKit Bugzilla
Attachment 372142 Details for
Bug 198854
: -[WKWebsiteDataStore removeDataOfTypes:modifiedSince:completionHandler:] doesn't delete _WKWebsiteDataTypeCredentials
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-198854-20190614144538.patch (text/plain), 19.69 KB, created by
Sihui Liu
on 2019-06-14 14:45:38 PDT
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Sihui Liu
Created:
2019-06-14 14:45:38 PDT
Size:
19.69 KB
patch
obsolete
>Subversion Revision: 246439 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index 0aa6d087c2688334bc7ea2a4639804faf5527e9f..532f30f8dfb30925f28811e50526cfedc442e575 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,26 @@ >+2019-06-14 Sihui Liu <sihui_liu@apple.com> >+ >+ -[WKWebsiteDataStore removeDataOfTypes:modifiedSince:completionHandler:] doesn't delete _WKWebsiteDataTypeCredentials >+ https://bugs.webkit.org/show_bug.cgi?id=198854 >+ <rdar://problem/51386058> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Add option NSURLCredentialStorageRemoveSynchronizableCredentials when removing persistent credential so >+ credentials from same account will be removed from all devices. >+ >+ Test: WKWebsiteDataStore.RemoveAllPersistentCredentials >+ >+ * platform/network/CredentialStorage.cpp: >+ (WebCore::CredentialStorage::originsWithPersistentCredentials): >+ (WebCore::CredentialStorage::removePersistentCredentialsWithOrigins): >+ (WebCore::CredentialStorage::clearPersistentCredentials): >+ * platform/network/CredentialStorage.h: >+ * platform/network/mac/CredentialStorageMac.mm: >+ (WebCore::CredentialStorage::originsWithPersistentCredentials): >+ (WebCore::CredentialStorage::removePersistentCredentialsWithOrigins): >+ (WebCore::CredentialStorage::clearPersistentCredentials): >+ > 2019-06-14 Saam Barati <sbarati@apple.com> > > Unreviewed. Follow up to r246438. This removes a debug assert until >diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog >index be102fd6f527c897fee6b4ee4f6da1a16187e712..f118bb8ee28e8096cb0e5a41c13e65d7dac09bd2 100644 >--- a/Source/WebKit/ChangeLog >+++ b/Source/WebKit/ChangeLog >@@ -1,3 +1,32 @@ >+2019-06-14 Sihui Liu <sihui_liu@apple.com> >+ >+ -[WKWebsiteDataStore removeDataOfTypes:modifiedSince:completionHandler:] doesn't delete _WKWebsiteDataTypeCredentials >+ https://bugs.webkit.org/show_bug.cgi?id=198854 >+ <rdar://problem/51386058> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Clear persistent credentials in deleteWebsiteData of network process. >+ >+ Also, merge originsWithPersistentCredentials and removeCredentialsWithOrigins into fetchWebsiteData and >+ deleteWebsiteData, and move credentials handling to WebCore. >+ >+ * NetworkProcess/NetworkProcess.cpp: >+ (WebKit::NetworkProcess::fetchWebsiteData): >+ (WebKit::NetworkProcess::deleteWebsiteData): >+ (WebKit::NetworkProcess::deleteWebsiteDataForOrigins): >+ (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/WebsiteData/WebsiteDataStore.cpp: >+ (WebKit::WebsiteDataStore::fetchDataAndApply): >+ (WebKit::computeWebProcessAccessTypeForDataRemoval): >+ (WebKit::WebsiteDataStore::removeData): >+ > 2019-06-13 Megan Gardner <megan_gardner@apple.com> > > Fix selection hang in Mail >diff --git a/Source/WebCore/platform/network/CredentialStorage.cpp b/Source/WebCore/platform/network/CredentialStorage.cpp >index 98e9426fc27adbb2bb60bed46dd5667faf7c3962..1be12e7b6f8b49e52e69bbac299dbb36ffbb2076 100644 >--- a/Source/WebCore/platform/network/CredentialStorage.cpp >+++ b/Source/WebCore/platform/network/CredentialStorage.cpp >@@ -187,4 +187,20 @@ void CredentialStorage::clearCredentials() > m_pathToDefaultProtectionSpaceMap.clear(); > } > >+#if !PLATFORM(COCOA) >+Vector<WebCore::SecurityOriginData> CredentialStorage::originsWithPersistentCredentials() >+{ >+ return { }; >+} >+ >+void CredentialStorage::removePersistentCredentialsWithOrigins(const Vector<WebCore::SecurityOriginData>& origins) >+{ >+ return; >+} >+ >+void CredentialStorage::clearPersistentCredentials() >+{ >+ return; >+} >+#endif > } // namespace WebCore >diff --git a/Source/WebCore/platform/network/CredentialStorage.h b/Source/WebCore/platform/network/CredentialStorage.h >index f8c75ae929ab2d5425a24a6267cd42738d75c0d7..9758b0447e12dbd66591d51e62f036377d49653a 100644 >--- a/Source/WebCore/platform/network/CredentialStorage.h >+++ b/Source/WebCore/platform/network/CredentialStorage.h >@@ -47,7 +47,9 @@ public: > > // OS persistent storage. > WEBCORE_EXPORT static Credential getFromPersistentStorage(const ProtectionSpace&); >- WEBCORE_EXPORT static Vector<SecurityOriginData> originsWithPersistentCredentials(); >+ WEBCORE_EXPORT static Vector<WebCore::SecurityOriginData> originsWithPersistentCredentials(); >+ WEBCORE_EXPORT static void removePersistentCredentialsWithOrigins(const Vector<WebCore::SecurityOriginData>& origins); >+ WEBCORE_EXPORT static void clearPersistentCredentials(); > > WEBCORE_EXPORT void clearCredentials(); > >diff --git a/Source/WebCore/platform/network/mac/CredentialStorageMac.mm b/Source/WebCore/platform/network/mac/CredentialStorageMac.mm >index 65b51511f9a9b2b8b12f63edae36fef35e76d39c..d7837de04054a9f12b95372119c28912cf1b587a 100644 >--- a/Source/WebCore/platform/network/mac/CredentialStorageMac.mm >+++ b/Source/WebCore/platform/network/mac/CredentialStorageMac.mm >@@ -41,10 +41,43 @@ Credential CredentialStorage::getFromPersistentStorage(const ProtectionSpace& pr > Vector<WebCore::SecurityOriginData> CredentialStorage::originsWithPersistentCredentials() > { > Vector<WebCore::SecurityOriginData> origins; >- auto allCredentials = [[NSURLCredentialStorage sharedCredentialStorage] allCredentials]; >- for (NSURLProtectionSpace* key in allCredentials.keyEnumerator) >+ auto allCredentials = adoptNS([[NSURLCredentialStorage sharedCredentialStorage] allCredentials]); >+ for (NSURLProtectionSpace* key in allCredentials.get().keyEnumerator) > origins.append(WebCore::SecurityOriginData { String(key.protocol), String(key.host), key.port }); > return origins; > } > >+void CredentialStorage::removePersistentCredentialsWithOrigins(const Vector<WebCore::SecurityOriginData>& origins) >+{ >+ auto sharedStorage = [NSURLCredentialStorage sharedCredentialStorage]; >+ auto allCredentials = adoptNS([sharedStorage allCredentials]); >+ for (auto& origin : origins) { >+ for (NSURLProtectionSpace* space in allCredentials.get()) { >+ if (origin.protocol == String(space.protocol) >+ && origin.host == String(space.host) >+ && origin.port >+ && *origin.port == space.port) { >+ auto credentials = allCredentials.get()[space]; >+ for (NSString* user in credentials) { >+ auto credential = credentials[user]; >+ [sharedStorage removeCredential:credential forProtectionSpace:space options:@{ NSURLCredentialStorageRemoveSynchronizableCredentials : @YES }]; >+ } >+ } >+ } >+ } >+} >+ >+void CredentialStorage::clearPersistentCredentials() >+{ >+ auto sharedStorage = [NSURLCredentialStorage sharedCredentialStorage]; >+ auto allCredentials = adoptNS([sharedStorage allCredentials]); >+ for (NSURLProtectionSpace* space in allCredentials.get().keyEnumerator) { >+ auto credentials = allCredentials.get()[space]; >+ for (NSString* user in credentials) { >+ auto credential = credentials[user]; >+ [sharedStorage removeCredential:credential forProtectionSpace:space options:@{ NSURLCredentialStorageRemoveSynchronizableCredentials : @YES }]; >+ } >+ } >+} >+ > } // namespace WebCore >diff --git a/Source/WebKit/NetworkProcess/NetworkProcess.cpp b/Source/WebKit/NetworkProcess/NetworkProcess.cpp >index dd9a1e3a8c5d76d2dd435303b7b8365ccae092e2..7fdda1d9e169adb1c94ec802d9487f04e86eafb2 100644 >--- a/Source/WebKit/NetworkProcess/NetworkProcess.cpp >+++ b/Source/WebKit/NetworkProcess/NetworkProcess.cpp >@@ -1282,6 +1282,11 @@ void NetworkProcess::fetchWebsiteData(PAL::SessionID sessionID, OptionSet<Websit > for (auto& securityOrigin : securityOrigins) > callbackAggregator->m_websiteData.entries.append({ securityOrigin, WebsiteDataType::Credentials, 0 }); > } >+ if (!sessionID.isEphemeral()) { >+ auto securityOrigins = WebCore::CredentialStorage::originsWithPersistentCredentials(); >+ for (auto& securityOrigin : securityOrigins) >+ callbackAggregator->m_websiteData.entries.append({ securityOrigin, WebsiteDataType::Credentials, 0 }); >+ } > } > > if (websiteDataTypes.contains(WebsiteDataType::DOMCache)) { >@@ -1359,6 +1364,8 @@ void NetworkProcess::deleteWebsiteData(PAL::SessionID sessionID, OptionSet<Websi > if (websiteDataTypes.contains(WebsiteDataType::Credentials)) { > if (auto* session = storageSession(sessionID)) > session->credentialStorage().clearCredentials(); >+ if (!sessionID.isEphemeral()) >+ WebCore::CredentialStorage::clearPersistentCredentials(); > } > > auto clearTasksHandler = WTF::CallbackAggregator::create([this, callbackID] { >@@ -1496,6 +1503,8 @@ void NetworkProcess::deleteWebsiteDataForOrigins(PAL::SessionID sessionID, Optio > for (auto& originData : originDatas) > session->credentialStorage().removeCredentialsWithOrigin(originData); > } >+ if (!sessionID.isEphemeral()) >+ WebCore::CredentialStorage::removePersistentCredentialsWithOrigins(originDatas); > } > > // FIXME: Implement storage quota clearing for these origins. >@@ -2541,16 +2550,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 0e1dabd00ee4ca94e22b2d25e0c32fbbc4054fdd..b5d6f0c940880e67fc2dc95d5062b2517af4d601 100644 >--- a/Source/WebKit/NetworkProcess/NetworkProcess.h >+++ b/Source/WebKit/NetworkProcess/NetworkProcess.h >@@ -436,9 +436,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..91fe69d57a2a36de246c42ab7d778f2bda3d978f 100644 >--- a/Source/WebKit/NetworkProcess/NetworkProcess.messages.in >+++ b/Source/WebKit/NetworkProcess/NetworkProcess.messages.in >@@ -168,7 +168,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/WebsiteData/WebsiteDataStore.cpp b/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp >index c28a640f7773d40e2089094db68ca13ba76e3f52..c4d9758ac42a195bf9b8a210e50e7be58a5aad8e 100644 >--- a/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp >+++ b/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp >@@ -512,24 +512,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 { >@@ -636,9 +618,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; > } > >@@ -1085,19 +1064,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 e3aa4e9379b70b74b726ca9930cf354aaef9b7ce..cd26a8c8a5f4d0ec1edefcc805de7ddebe11cbe5 100644 >--- a/Tools/ChangeLog >+++ b/Tools/ChangeLog >@@ -1,3 +1,13 @@ >+2019-06-14 Sihui Liu <sihui_liu@apple.com> >+ >+ -[WKWebsiteDataStore removeDataOfTypes:modifiedSince:completionHandler:] doesn't delete _WKWebsiteDataTypeCredentials >+ https://bugs.webkit.org/show_bug.cgi?id=198854 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ * TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm: >+ (TestWebKitAPI::TEST): >+ > 2019-06-14 Youenn Fablet <youenn@apple.com> > > import-w3c-tests should respect WEBKIT_OUTPUTDIR >diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm >index f9787880e274f3461ffc6fd6a45452499d23dcd3..1e0b1f4d590773554814607089ab97c0648c0d68 100644 >--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm >+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm >@@ -252,6 +252,51 @@ TEST(WKWebsiteDataStore, RemovePersistentCredentials) > TestWebKitAPI::Util::run(&done); > } > >+TEST(WKWebsiteDataStore, RemoveAllPersistentCredentials) >+{ >+ usePersistentCredentialStorage = true; >+ >+ TCPServer server(respondWithChallengeThenOK); >+ 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]; >+ >+ readyToContinue = false; >+ [websiteDataStore fetchDataRecordsOfTypes:[NSSet setWithObject:_WKWebsiteDataTypeCredentials] completionHandler:^(NSArray<WKWebsiteDataRecord *> *dataRecords) { >+ bool foundRecord = false; >+ for (WKWebsiteDataRecord *record in dataRecords) { >+ auto name = [record displayName]; >+ if ([name isEqualToString:@"127.0.0.1"]) >+ foundRecord = true; >+ } >+ EXPECT_TRUE(foundRecord); >+ readyToContinue = true; >+ }]; >+ TestWebKitAPI::Util::run(&readyToContinue); >+ >+ readyToContinue = false; >+ [websiteDataStore removeDataOfTypes:[NSSet setWithObject:_WKWebsiteDataTypeCredentials] modifiedSince:[NSDate distantPast] completionHandler:^() { >+ readyToContinue = true; >+ }]; >+ TestWebKitAPI::Util::run(&readyToContinue); >+ >+ readyToContinue = false; >+ [websiteDataStore fetchDataRecordsOfTypes:[NSSet setWithObject:_WKWebsiteDataTypeCredentials] completionHandler:^(NSArray<WKWebsiteDataRecord *> *dataRecords) { >+ bool foundRecord = false; >+ for (WKWebsiteDataRecord *record in dataRecords) { >+ auto name = [record displayName]; >+ if ([name isEqualToString:@"127.0.0.1"]) >+ foundRecord = true; >+ } >+ EXPECT_FALSE(foundRecord); >+ readyToContinue = true; >+ }]; >+ TestWebKitAPI::Util::run(&readyToContinue); >+} >+ > TEST(WKWebsiteDataStore, RemoveNonPersistentCredentials) > { > TCPServer server(respondWithChallengeThenOK);
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=372142">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 198854
:
372113
|
372126
|
372142
|
372306