WebKit Bugzilla
Attachment 373556 Details for
Bug 197093
: REGRESSION (r230681): Do not use stored credentials if WKBundlePageResourceLoadClient.shouldUseCredentialStorage returns false
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-197093-20190705174642.patch (text/plain), 34.99 KB, created by
Sihui Liu
on 2019-07-05 17:46:43 PDT
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Sihui Liu
Created:
2019-07-05 17:46:43 PDT
Size:
34.99 KB
patch
obsolete
>Subversion Revision: 247173 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index a31221fe7f6ad9dd7bb1b2fc0078c6838b56f6ae..87832841aade60a6ca46babd50fa5b5a756b77a1 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,29 @@ >+2019-07-05 Sihui Liu <sihui_liu@apple.com> >+ >+ REGRESSION (r230681): Do not use stored credentials if WKBundlePageResourceLoadClient.shouldUseCredentialStorage returns false >+ https://bugs.webkit.org/show_bug.cgi?id=197093 >+ <rdar://problem/50111468> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ 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-05 Youenn Fablet <youenn@apple.com> > > Add fetch quirk for www.bnz.co.nz >diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog >index ac2ecedfd155c32c15894f379b4e4e15b2daa00b..d5cd33e4979ae9ad07cc7676804e3ba5d47803bc 100644 >--- a/Source/WebKit/ChangeLog >+++ b/Source/WebKit/ChangeLog >@@ -1,3 +1,37 @@ >+2019-07-05 Sihui Liu <sihui_liu@apple.com> >+ >+ REGRESSION (r230681): Do not use stored credentials if WKBundlePageResourceLoadClient.shouldUseCredentialStorage returns false >+ https://bugs.webkit.org/show_bug.cgi?id=197093 >+ <rdar://problem/50111468> >+ >+ 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-05 Zalan Bujtas <zalan@apple.com> > > [ContentChangeObserver] REGRESSION (r247015): facebook photo/video upload button is unresponsive to user interaction. >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 be2c80368c1c86d515774eb0cc4accdea94800e9..c34600302df2435dfc5b914c6a4a5fbc6853bb27 100644 >--- a/Tools/ChangeLog >+++ b/Tools/ChangeLog >@@ -1,3 +1,20 @@ >+2019-07-05 Sihui Liu <sihui_liu@apple.com> >+ >+ REGRESSION (r230681): Do not use stored credentials if WKBundlePageResourceLoadClient.shouldUseCredentialStorage returns false >+ https://bugs.webkit.org/show_bug.cgi?id=197093 >+ <rdar://problem/50111468> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ 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-05 Ryan Haddad <ryanhaddad@apple.com> > > Unreviewed, rolling out r247115. >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)
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=373556">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 197093
:
367788
|
367805
|
373556