WebKit Bugzilla
Attachment 372306 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 for landing
bug-198854-20190617180048.patch (text/plain), 19.70 KB, created by
Sihui Liu
on 2019-06-17 18:00:48 PDT
(
hide
)
Description:
Patch for landing
Filename:
MIME Type:
Creator:
Sihui Liu
Created:
2019-06-17 18:00:48 PDT
Size:
19.70 KB
patch
obsolete
>Subversion Revision: 246527 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index 4f25d9e5578dcc3aef179192e096a7f48c4d659e..23775f51b28880d977d105980eea3deb171db9b3 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,26 @@ >+2019-06-17 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 Geoffrey Garen. >+ >+ 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-17 Alex Christensen <achristensen@webkit.org> > > Fix iOS crash when starting loads with no active DocumentLoader >diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog >index 3a22dd44190e1e185346fa94eea2a4fdca26aef1..29126aebc5e9f32e1b0b82a3ae2248639c3120db 100644 >--- a/Source/WebKit/ChangeLog >+++ b/Source/WebKit/ChangeLog >@@ -1,3 +1,32 @@ >+2019-06-17 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 Geoffrey Garen. >+ >+ 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-17 Jiewen Tan <jiewen_tan@apple.com> > > Unreviewed, build fix after r246514 >diff --git a/Source/WebCore/platform/network/CredentialStorage.cpp b/Source/WebCore/platform/network/CredentialStorage.cpp >index 98e9426fc27adbb2bb60bed46dd5667faf7c3962..0cbf94b28070402ae71fb57a69b81771d64c0cef 100644 >--- a/Source/WebCore/platform/network/CredentialStorage.cpp >+++ b/Source/WebCore/platform/network/CredentialStorage.cpp >@@ -187,4 +187,21 @@ void CredentialStorage::clearCredentials() > m_pathToDefaultProtectionSpaceMap.clear(); > } > >+#if !PLATFORM(COCOA) >+Vector<SecurityOriginData> CredentialStorage::originsWithPersistentCredentials() >+{ >+ return { }; >+} >+ >+void CredentialStorage::removePersistentCredentialsWithOrigins(const Vector<SecurityOriginData>&) >+{ >+ 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..ffb1e7ca57f93a108747d0e95cc26daf39013a20 100644 >--- a/Source/WebCore/platform/network/CredentialStorage.h >+++ b/Source/WebCore/platform/network/CredentialStorage.h >@@ -48,6 +48,8 @@ public: > // OS persistent storage. > WEBCORE_EXPORT static Credential getFromPersistentStorage(const ProtectionSpace&); > WEBCORE_EXPORT static Vector<SecurityOriginData> originsWithPersistentCredentials(); >+ WEBCORE_EXPORT static void removePersistentCredentialsWithOrigins(const Vector<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..47ff82eb3d95adea029d99942c555f582a3c92a5 100644 >--- a/Source/WebCore/platform/network/mac/CredentialStorageMac.mm >+++ b/Source/WebCore/platform/network/mac/CredentialStorageMac.mm >@@ -38,13 +38,46 @@ Credential CredentialStorage::getFromPersistentStorage(const ProtectionSpace& pr > return credential ? Credential(credential) : Credential(); > } > >-Vector<WebCore::SecurityOriginData> CredentialStorage::originsWithPersistentCredentials() >+Vector<SecurityOriginData> CredentialStorage::originsWithPersistentCredentials() > { >- Vector<WebCore::SecurityOriginData> origins; >+ Vector<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 }); >+ origins.append(SecurityOriginData { String(key.protocol), String(key.host), key.port }); > return origins; > } > >+void CredentialStorage::removePersistentCredentialsWithOrigins(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]; >+ [sharedStorage removeCredential:credential forProtectionSpace:space options:@{ NSURLCredentialStorageRemoveSynchronizableCredentials : @YES }]; >+ } >+ } >+ } >+ } >+} >+ >+void CredentialStorage::clearPersistentCredentials() >+{ >+ 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]; >+ [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 f25e7e602d0964652e681f53ab1ef9b4797bf2ef..5d65508e3b5fd69f2517d0f17dee8fa470ceea79 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 8bc0805e25257aace29c7ad0df2d6959f6b43144..3db893c976750d4d554fc91380f25ec8a7fcece4 100644 >--- a/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp >+++ b/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp >@@ -516,24 +516,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 { >@@ -640,9 +622,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; > } > >@@ -1089,19 +1068,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 e8ca79f7c379911a1db4896b67b7813b8a368222..f5f940b90a2aaaceb96325e8ca4b9555cb731255 100644 >--- a/Tools/ChangeLog >+++ b/Tools/ChangeLog >@@ -1,3 +1,14 @@ >+2019-06-17 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 Geoffrey Garen. >+ >+ * TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm: >+ (TestWebKitAPI::TEST): >+ > 2019-06-17 Adrian Perez de Castro <aperez@igalia.com> > > [Flatpak][JHBuild] Update build environments to use WPEBackend-fdo 1.3.1 >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=372306">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