API tests using permanent credentials should clear credentials left by previous tests
authorsihui_liu@apple.com <sihui_liu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 2 Aug 2019 21:19:51 +0000 (21:19 +0000)
committersihui_liu@apple.com <sihui_liu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 2 Aug 2019 21:19:51 +0000 (21:19 +0000)
https://bugs.webkit.org/show_bug.cgi?id=199729

Reviewed by Alex Christensen.

Source/WebCore:

Update existing API tests.

* platform/network/CredentialStorage.cpp:
(WebCore::CredentialStorage::clearSessionCredentials):
(WebCore::CredentialStorage::clearPermanentCredentialsForProtectionSpace): Deleted.
* platform/network/CredentialStorage.h:
* platform/network/mac/CredentialStorageMac.mm:
(WebCore::CredentialStorage::clearPermanentCredentialsForProtectionSpace): Deleted.

Source/WebKit:

Permanent password credentials currently are shared across processes, so we don't need to clear them from
network process.

* NetworkProcess/NetworkProcess.cpp:
(WebKit::NetworkProcess::clearPermanentCredentialsForProtectionSpace): Deleted.
* NetworkProcess/NetworkProcess.h:
* NetworkProcess/NetworkProcess.messages.in:
* UIProcess/API/Cocoa/WKProcessPool.mm:
(-[WKProcessPool _clearPermanentCredentialsForProtectionSpace:]):
(-[WKProcessPool _clearPermanentCredentialsForProtectionSpace:completionHandler:]): Deleted.
* UIProcess/API/Cocoa/WKProcessPoolPrivate.h:
* UIProcess/Cocoa/WebProcessPoolCocoa.mm:
(WebKit::WebProcessPool::clearPermanentCredentialsForProtectionSpace):
* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::clearPermanentCredentialsForProtectionSpace): Deleted.
* UIProcess/WebProcessPool.h:

Tools:

We used to clear the permanent credentials created by API tests at the end of the API tests, to ensure those
credentials will not affect tests running after. There is a case where permanent credentials were left on the
system, so those API tests were timing out themselves before reaching to the cleanup, which caused cascading
failure. To prevent this from happening again, add cleanup at the begining of the tests.

* TestWebKitAPI/Tests/WebKitCocoa/Challenge.mm:
(TEST):
* TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm:
(TestWebKitAPI::TEST):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@248181 268f45cc-cd09-0410-ab3c-d52691b4dbfc

16 files changed:
Source/WebCore/ChangeLog
Source/WebCore/platform/network/CredentialStorage.cpp
Source/WebCore/platform/network/CredentialStorage.h
Source/WebCore/platform/network/mac/CredentialStorageMac.mm
Source/WebKit/ChangeLog
Source/WebKit/NetworkProcess/NetworkProcess.cpp
Source/WebKit/NetworkProcess/NetworkProcess.h
Source/WebKit/NetworkProcess/NetworkProcess.messages.in
Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm
Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h
Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm
Source/WebKit/UIProcess/WebProcessPool.cpp
Source/WebKit/UIProcess/WebProcessPool.h
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/Challenge.mm
Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm

index 8351289..f79eded 100644 (file)
@@ -1,3 +1,19 @@
+2019-08-02  Sihui Liu  <sihui_liu@apple.com>
+
+        API tests using permanent credentials should clear credentials left by previous tests
+        https://bugs.webkit.org/show_bug.cgi?id=199729
+
+        Reviewed by Alex Christensen.
+
+        Update existing API tests.
+
+        * platform/network/CredentialStorage.cpp:
+        (WebCore::CredentialStorage::clearSessionCredentials):
+        (WebCore::CredentialStorage::clearPermanentCredentialsForProtectionSpace): Deleted.
+        * platform/network/CredentialStorage.h:
+        * platform/network/mac/CredentialStorageMac.mm:
+        (WebCore::CredentialStorage::clearPermanentCredentialsForProtectionSpace): Deleted.
+
 2019-08-02  Devin Rousso  <drousso@apple.com>
 
         Web Inspector: Storage: disable related agents when the tab is closed
index 0136b9e..26a3b9e 100644 (file)
@@ -200,10 +200,6 @@ void CredentialStorage::removeSessionCredentialsWithOrigins(const Vector<Securit
 void CredentialStorage::clearSessionCredentials()
 {
 }
-
-void CredentialStorage::clearPermanentCredentialsForProtectionSpace(const ProtectionSpace&)
-{
-}
 #endif
 
 } // namespace WebCore
index cd853c6..d0e16f7 100644 (file)
@@ -50,7 +50,6 @@ public:
     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();
 
index 6ea0bab..a1c508a 100644 (file)
@@ -91,16 +91,4 @@ void CredentialStorage::clearSessionCredentials()
     }
 }
 
-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
index e672a8b..a2b5143 100644 (file)
@@ -1,3 +1,27 @@
+2019-08-02  Sihui Liu  <sihui_liu@apple.com>
+
+        API tests using permanent credentials should clear credentials left by previous tests
+        https://bugs.webkit.org/show_bug.cgi?id=199729
+
+        Reviewed by Alex Christensen.
+
+        Permanent password credentials currently are shared across processes, so we don't need to clear them from 
+        network process.
+
+        * NetworkProcess/NetworkProcess.cpp:
+        (WebKit::NetworkProcess::clearPermanentCredentialsForProtectionSpace): Deleted.
+        * NetworkProcess/NetworkProcess.h:
+        * NetworkProcess/NetworkProcess.messages.in:
+        * UIProcess/API/Cocoa/WKProcessPool.mm:
+        (-[WKProcessPool _clearPermanentCredentialsForProtectionSpace:]):
+        (-[WKProcessPool _clearPermanentCredentialsForProtectionSpace:completionHandler:]): Deleted.
+        * UIProcess/API/Cocoa/WKProcessPoolPrivate.h:
+        * UIProcess/Cocoa/WebProcessPoolCocoa.mm:
+        (WebKit::WebProcessPool::clearPermanentCredentialsForProtectionSpace):
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::clearPermanentCredentialsForProtectionSpace): Deleted.
+        * UIProcess/WebProcessPool.h:
+
 2019-08-02  Devin Rousso  <drousso@apple.com>
 
         Web Inspector: Timelines: Develop > Start Timeline Recording doesn't work when focused on a detached inspector window
index acd9551..c333957 100644 (file)
@@ -458,12 +458,6 @@ 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)
index 605112c..546d5ef 100644 (file)
@@ -395,7 +395,6 @@ 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&);
index 0700679..b7e01f5 100644 (file)
@@ -38,7 +38,6 @@ messages -> NetworkProcess LegacyReceiver {
 #endif
 
     ClearCachedCredentials()
-    ClearPermanentCredentialsForProtectionSpace(WebCore::ProtectionSpace protectionSpace) -> () Async
 
     AddWebsiteDataStore(struct WebKit::WebsiteDataStoreParameters websiteDataStoreParameters);
     DestroySession(PAL::SessionID sessionID)
index 9b5f7b5..bace5ac 100644 (file)
@@ -643,11 +643,9 @@ static NSDictionary *policiesHashMapToDictionary(const HashMap<String, HashMap<S
     return _processPool->networkProcessHasEntitlementForTesting(entitlement);
 }
 
-- (void)_clearPermanentCredentialsForProtectionSpace:(NSURLProtectionSpace *)protectionSpace completionHandler:(void(^)())completionHandler
+- (void)_clearPermanentCredentialsForProtectionSpace:(NSURLProtectionSpace *)protectionSpace
 {
-    _processPool->clearPermanentCredentialsForProtectionSpace(WebCore::ProtectionSpace(protectionSpace), [completionHandler = makeBlockPtr(completionHandler)] {
-        completionHandler();
-    });
+    _processPool->clearPermanentCredentialsForProtectionSpace(WebCore::ProtectionSpace(protectionSpace));
 }
 
 @end
index 806cbc4..efff405 100644 (file)
 - (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));
+- (void)_clearPermanentCredentialsForProtectionSpace:(NSURLProtectionSpace *)protectionSpace 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));
index 31cdfba..3070d57 100644 (file)
@@ -548,6 +548,17 @@ void WebProcessPool::setStorageAccessAPIEnabled(bool enabled)
     sendToNetworkingProcess(Messages::NetworkProcess::SetStorageAccessAPIEnabled(enabled));
 }
 
+void WebProcessPool::clearPermanentCredentialsForProtectionSpace(WebCore::ProtectionSpace&& protectionSpace)
+{
+    auto sharedStorage = [NSURLCredentialStorage sharedCredentialStorage];
+    auto credentials = [sharedStorage credentialsForProtectionSpace:protectionSpace.nsSpace()];
+    for (NSString* user in credentials) {
+        auto credential = credentials[user];
+        if (credential.persistence == NSURLCredentialPersistencePermanent)
+            [sharedStorage removeCredential:credentials[user] forProtectionSpace:protectionSpace.nsSpace()];
+    }
+}
+
 int networkProcessLatencyQOS()
 {
     static const int qos = [[NSUserDefaults standardUserDefaults] integerForKey:@"WebKitNetworkProcessLatencyQOS"];
index f6bbd2b..cf8cf69 100644 (file)
@@ -1768,12 +1768,6 @@ 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)
index 226cf36..2ccb47f 100644 (file)
@@ -310,7 +310,6 @@ public:
     void setAllowsAnySSLCertificateForWebSocket(bool);
 
     void clearCachedCredentials();
-    void clearPermanentCredentialsForProtectionSpace(WebCore::ProtectionSpace&&, CompletionHandler<void()>&&);
     void terminateNetworkProcess();
     void sendNetworkProcessWillSuspendImminentlyForTesting();
     void sendNetworkProcessDidResume();
@@ -468,6 +467,8 @@ public:
     void setCookieStoragePartitioningEnabled(bool);
     bool storageAccessAPIEnabled() const { return m_storageAccessAPIEnabled; }
     void setStorageAccessAPIEnabled(bool);
+
+    void clearPermanentCredentialsForProtectionSpace(WebCore::ProtectionSpace&&);
 #endif
 
 #if ENABLE(SERVICE_WORKER)
index a594457..3725281 100644 (file)
@@ -1,3 +1,20 @@
+2019-08-02  Sihui Liu  <sihui_liu@apple.com>
+
+        API tests using permanent credentials should clear credentials left by previous tests
+        https://bugs.webkit.org/show_bug.cgi?id=199729
+
+        Reviewed by Alex Christensen.
+
+        We used to clear the permanent credentials created by API tests at the end of the API tests, to ensure those 
+        credentials will not affect tests running after. There is a case where permanent credentials were left on the 
+        system, so those API tests were timing out themselves before reaching to the cleanup, which caused cascading 
+        failure. To prevent this from happening again, add cleanup at the begining of the tests.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/Challenge.mm:
+        (TEST):
+        * TestWebKitAPI/Tests/WebKitCocoa/WKWebsiteDatastore.mm:
+        (TestWebKitAPI::TEST):
+
 2019-08-02  Andres Gonzalez  <andresg_22@apple.com>
 
         Add accessibility object method to determine whether an element is inside a table cell. Needed for iOS accessibility client.
index 4248f34..c0c2639 100644 (file)
@@ -100,17 +100,14 @@ TEST(Challenge, SecIdentity)
     auto webView = adoptNS([WKWebView new]);
     auto delegate = adoptNS([ChallengeDelegate new]);
     [webView setNavigationDelegate:delegate.get()];
+
+    // Make sure no credential left by previous tests.
+    NSURLProtectionSpace *protectionSpace = [[[NSURLProtectionSpace alloc] initWithHost:@"127.0.0.1" port:server.port() protocol:NSURLProtectionSpaceHTTP realm:@"testrealm" authenticationMethod:NSURLAuthenticationMethodHTTPBasic] autorelease];
+    [[webView configuration].processPool _clearPermanentCredentialsForProtectionSpace:protectionSpace];
+
     [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> {
@@ -199,6 +196,11 @@ TEST(Challenge, BasicProposedCredential)
     auto webView = adoptNS([[WKWebView alloc] initWithFrame:CGRectZero configuration:configuration.get()]);
     auto delegate = adoptNS([ProposedCredentialDelegate new]);
     [webView setNavigationDelegate:delegate.get()];
+
+    // Make sure no credential left by previous tests.
+    NSURLProtectionSpace *protectionSpace = [[[NSURLProtectionSpace alloc] initWithHost:@"127.0.0.1" port:server.port() protocol:NSURLProtectionSpaceHTTP realm:@"testrealm" authenticationMethod:NSURLAuthenticationMethodHTTPBasic] autorelease];
+    [[webView configuration].processPool _clearPermanentCredentialsForProtectionSpace:protectionSpace];
+
     RetainPtr<NSURLRequest> request = [NSURLRequest requestWithURL:[NSURL URLWithString:[NSString stringWithFormat:@"http://127.0.0.1:%d/", server.port()]]];
     [webView loadRequest:request.get()];
     Util::run(&navigationFinished);
@@ -208,12 +210,7 @@ TEST(Challenge, BasicProposedCredential)
     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;
-    [[webView configuration].processPool _clearPermanentCredentialsForProtectionSpace:protectionSpace completionHandler:^{
-        removedCredential = true;
-    }];
-    Util::run(&removedCredential);
+    [[webView configuration].processPool _clearPermanentCredentialsForProtectionSpace:protectionSpace];
 }
 
 #if HAVE(SSL)
index b94bc73..48a1047 100644 (file)
@@ -145,6 +145,11 @@ TEST(WKWebsiteDataStore, FetchPersistentCredentials)
     auto navigationDelegate = adoptNS([[NavigationTestDelegate alloc] init]);
     auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
     [webView setNavigationDelegate:navigationDelegate.get()];
+
+    // Make sure no credential left by previous tests.
+    NSURLProtectionSpace *protectionSpace = [[[NSURLProtectionSpace alloc] initWithHost:@"127.0.0.1" port:server.port() protocol:NSURLProtectionSpaceHTTP realm:@"testrealm" authenticationMethod:NSURLAuthenticationMethodHTTPBasic] autorelease];
+    [[webView configuration].processPool _clearPermanentCredentialsForProtectionSpace:protectionSpace];
+
     [webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:[NSString stringWithFormat:@"http://127.0.0.1:%d/", server.port()]]]];
     [navigationDelegate waitForDidFinishNavigation];
 
@@ -157,12 +162,7 @@ TEST(WKWebsiteDataStore, FetchPersistentCredentials)
     TestWebKitAPI::Util::run(&done);
 
     // 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);
+    [[webView configuration].processPool _clearPermanentCredentialsForProtectionSpace:protectionSpace];
 }
 
 TEST(WKWebsiteDataStore, RemoveNonPersistentCredentials)