Bug 186870

Summary: NSURLAuthenticationMethodOAuth challenges are surfaced to clients in -didReceiveAuthenticationChallenge as NSURLAuthenticationMethodDefault
Product: WebKit Reporter: Ansh Shukla <ansh_shukla>
Component: WebKit APIAssignee: Alex Christensen <achristensen>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, beidson, cdumez, commit-queue, ews-watchlist, ggaren, realdawei, rniwa, ryanhaddad, tsavell, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: All   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Archive of layout-test-results from ews103 for mac-sierra
none
Patch
none
Patch
none
Archive of layout-test-results from ews103 for mac-sierra
none
Fix layout test
none
Archive of layout-test-results from ews115 for mac-sierra
none
Archive of layout-test-results from ews104 for mac-sierra-wk2
none
Archive of layout-test-results from ews122 for ios-simulator-wk2
none
Patch
none
Archive of layout-test-results from ews103 for mac-sierra
none
Archive of layout-test-results from ews117 for mac-sierra
none
Patch none

Description Ansh Shukla 2018-06-20 19:08:38 PDT
When WebKit receives an authentication request header of the form: WWW-Authenticate	OAuth realm="https%3A%2F%2Fauthenticate.apple.com", it converts it to an internal representation of type ProtectionSpaceAuthenticationSchemeUnknown. However, when this authentication challenge is eventually sent to clients with -didReceiveAuthenticationChallenge, its cocoa representation has authentication type default because the implementation of nsSpace() doesn't handle the unknown case:

    default:
        break;
        ASSERT_NOT_REACHED();
    }
    
    m_nsSpace = adoptNS(proxyType
        ? [[NSURLProtectionSpace alloc] initWithProxyHost:host() port:port() type:proxyType realm:realm() authenticationMethod:method]
        : [[NSURLProtectionSpace alloc] initWithHost:host() port:port() protocol:protocol realm:realm() authenticationMethod:method]);


Passing in nil to the NSURLProtectionSpace initializer will give it type default.

To clients, this manifests as an authentication challenge of type HTTP basic auth, since the protocol will be HTTP.

In the WK2 C++ API, there was a custom object which did have the enum type: kWKProtectionSpaceAuthenticationSchemeUnknown.
Comment 1 Radar WebKit Bug Importer 2018-06-20 19:09:00 PDT
<rdar://problem/41314410>
Comment 2 Ansh Shukla 2018-06-20 19:39:50 PDT
Created attachment 343201 [details]
Patch
Comment 3 Ansh Shukla 2018-08-09 13:25:22 PDT
Created attachment 346858 [details]
Patch
Comment 4 Ansh Shukla 2018-08-09 13:25:51 PDT
New patch to see if it compiles with CFNetwork SPI.
Comment 5 Ansh Shukla 2018-08-14 14:25:33 PDT
Created attachment 347110 [details]
Patch
Comment 6 Alex Christensen 2018-08-14 14:55:26 PDT
Comment on attachment 347110 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=347110&action=review

> Source/WebCore/ChangeLog:17
> +2018-08-09  Ansh Shukla  <ansh_shukla@apple.com>

duplicate ChangeLog entry

> Source/WebKit/ChangeLog:12
> +2018-08-09  Ansh Shukla  <ansh_shukla@apple.com>

ditto

> Tools/WebKitTestRunner/TestController.cpp:1843
> +        m_currentInvocation->outputText(String::format("canAuthenticateAgainstProtectionSpace: %s\n", toString(authenticationScheme)));

You'll either need to remove this change and its log in your one expected file, or update all tests that log a canAuthenticateAgainstProtectionSpace message.  I suggest the former.
Comment 7 Ansh Shukla 2018-08-14 15:01:38 PDT
Created attachment 347114 [details]
Patch
Comment 8 EWS Watchlist 2018-08-14 15:56:33 PDT
Comment on attachment 347114 [details]
Patch

Attachment 347114 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/8860087

New failing tests:
http/tests/loading/oauth.html
Comment 9 EWS Watchlist 2018-08-14 15:56:35 PDT
Created attachment 347120 [details]
Archive of layout-test-results from ews103 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 10 Alex Christensen 2018-08-14 16:42:17 PDT
Created attachment 347128 [details]
Patch
Comment 11 WebKit Commit Bot 2018-08-14 17:10:18 PDT
Comment on attachment 347128 [details]
Patch

Clearing flags on attachment: 347128

Committed r234870: <https://trac.webkit.org/changeset/234870>
Comment 12 WebKit Commit Bot 2018-08-14 17:10:20 PDT
All reviewed patches have been landed.  Closing bug.
Comment 13 Dawei Fenton (:realdawei) 2018-08-15 09:19:02 PDT
test http/tests/loading/oauth.html has been pretty flaky since it was added:

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=http%2Ftests%2Floading%2Foauth.html

sample diff:
--- /Volumes/Data/slave/highsierra-debug-tests-wk2/build/layout-test-results/http/tests/loading/oauth-expected.txt
+++ /Volumes/Data/slave/highsierra-debug-tests-wk2/build/layout-test-results/http/tests/loading/oauth-actual.txt
@@ -2,7 +2,7 @@
 main frame - didCommitLoadForFrame
 main frame - didFinishDocumentLoadForFrame
 main frame - didHandleOnloadEventsForFrame
-main frame - didFinishLoadForFrame
 canAuthenticateAgainstProtectionSpace
 127.0.0.1:8000 - didReceiveAuthenticationChallenge - ProtectionSpaceAuthenticationSchemeOAuth - Simulating cancelled authentication sheet
+main frame - didFinishLoadForFrame
Comment 14 Ryan Haddad 2018-08-15 11:58:06 PDT
Also, as iOS-sim EWS predicted, http/tests/loading/basic-auth-load-URL-with-consecutive-slashes.html is now failing on the bots. It looks like it just needs a rebaseline, though.

https://build.webkit.org/results/Apple%20iOS%2011%20Simulator%20Release%20WK2%20(Tests)/r234889%20(6801)/results.html
Comment 15 Ryan Haddad 2018-08-15 13:22:19 PDT
Reverted r234870 for reason:

The test introduced with this change is a flaky failure.

Committed r234897: <https://trac.webkit.org/changeset/234897>
Comment 16 Alex Christensen 2018-08-15 14:16:36 PDT
Created attachment 347209 [details]
Patch
Comment 17 EWS Watchlist 2018-08-15 15:14:20 PDT
Comment on attachment 347209 [details]
Patch

Attachment 347209 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/8872102

New failing tests:
http/tests/loading/oauth.html
http/tests/loading/basic-auth-load-URL-with-consecutive-slashes.html
Comment 18 EWS Watchlist 2018-08-15 15:14:22 PDT
Created attachment 347213 [details]
Archive of layout-test-results from ews103 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 19 Ansh Shukla 2018-08-15 15:33:58 PDT
I think ``runtest`` needs to have a capital 'T' in the onload attribute. We also need to re-baseline http/tests/loading/basic-auth-load-URL-with-consecutive-slashes.
Comment 20 Ansh Shukla 2018-08-15 15:56:24 PDT
Created attachment 347217 [details]
Fix layout test

Fix layout test. Trailing slash test isn't failing locally for me.
Comment 21 EWS Watchlist 2018-08-15 16:26:31 PDT
Comment on attachment 347209 [details]
Patch

Attachment 347209 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/8872506

New failing tests:
http/tests/loading/oauth.html
http/tests/loading/basic-auth-load-URL-with-consecutive-slashes.html
Comment 22 EWS Watchlist 2018-08-15 16:26:33 PDT
Created attachment 347221 [details]
Archive of layout-test-results from ews115 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 23 EWS Watchlist 2018-08-15 16:31:56 PDT
Comment on attachment 347209 [details]
Patch

Attachment 347209 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/8872859

New failing tests:
http/tests/loading/oauth.html
Comment 24 EWS Watchlist 2018-08-15 16:31:58 PDT
Created attachment 347222 [details]
Archive of layout-test-results from ews104 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 25 EWS Watchlist 2018-08-15 17:17:37 PDT
Comment on attachment 347209 [details]
Patch

Attachment 347209 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/8873008

New failing tests:
http/tests/loading/oauth.html
Comment 26 EWS Watchlist 2018-08-15 17:17:40 PDT
Created attachment 347230 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4
Comment 27 Alex Christensen 2018-08-15 17:29:25 PDT
Created attachment 347231 [details]
Patch
Comment 28 EWS Watchlist 2018-08-15 18:38:50 PDT
Comment on attachment 347231 [details]
Patch

Attachment 347231 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/8874500

New failing tests:
http/tests/security/credentials-iframes-allowCrossOriginSubresourcesToAskForCredentials.html
http/tests/misc/authentication-redirect-4/authentication-sent-to-redirect-same-origin-url.html
http/tests/security/mixedContent/secure-redirect-to-secure-redirect-to-basic-auth-insecure-image-allowCrossOriginSubresourcesToAskForCredentials.https.html
http/tests/security/mixedContent/secure-redirect-to-insecure-redirect-to-basic-auth-secure-image-allowCrossOriginSubresourcesToAskForCredentials.https.html
http/tests/misc/authentication-redirect-1/authentication-sent-to-redirect-cross-origin.html
http/tests/security/mixedContent/insecure-image-redirects-to-basic-auth-secure-image-allowCrossOriginSubresourcesToAskForCredentials.html
http/tests/security/private-browsing-http-auth.html
http/tests/security/basic-auth-subresource.html
http/tests/security/mixedContent/secure-page-navigates-to-basic-auth-insecure-page.https.html
http/tests/media/video-auth-with-allowCrossOriginSubresourcesToAskForCredentials.html
http/tests/loading/basic-auth-resend-wrong-credentials.html
http/tests/misc/401-alternative-content.php
http/tests/loading/authentication-after-redirect-stores-wrong-credentials/authentication-after-redirect-stores-wrong-credentials.html
http/tests/xmlhttprequest/remember-bad-password.html
http/tests/misc/authentication-redirect-3/authentication-sent-to-redirect-same-origin-with-location-credentials.html
http/tests/security/mixedContent/insecure-basic-auth-image-allowCrossOriginSubresourcesToAskForCredentials.https.html
http/tests/security/mixedContent/secure-redirect-to-secure-redirect-to-basic-auth-secure-image-allowCrossOriginSubresourcesToAskForCredentials.https.html
http/tests/misc/authentication-redirect-2/authentication-sent-to-redirect-same-origin.html
http/tests/security/401-logout/401-logout.php
http/tests/xmlhttprequest/failed-auth.html
http/tests/security/mixedContent/secure-page-navigates-to-basic-auth-secure-page-via-insecure-redirect.https.html
http/tests/media/video-auth.html
http/tests/loading/basic-credentials-sent-automatically.html
Comment 29 EWS Watchlist 2018-08-15 18:38:52 PDT
Created attachment 347235 [details]
Archive of layout-test-results from ews103 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 30 EWS Watchlist 2018-08-15 19:34:22 PDT
Comment on attachment 347231 [details]
Patch

Attachment 347231 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/8874700

New failing tests:
http/tests/security/credentials-iframes-allowCrossOriginSubresourcesToAskForCredentials.html
http/tests/misc/authentication-redirect-4/authentication-sent-to-redirect-same-origin-url.html
http/tests/security/mixedContent/secure-redirect-to-secure-redirect-to-basic-auth-insecure-image-allowCrossOriginSubresourcesToAskForCredentials.https.html
http/tests/security/mixedContent/secure-redirect-to-insecure-redirect-to-basic-auth-secure-image-allowCrossOriginSubresourcesToAskForCredentials.https.html
http/tests/misc/authentication-redirect-1/authentication-sent-to-redirect-cross-origin.html
http/tests/security/mixedContent/insecure-image-redirects-to-basic-auth-secure-image-allowCrossOriginSubresourcesToAskForCredentials.html
http/tests/security/private-browsing-http-auth.html
http/tests/security/basic-auth-subresource.html
http/tests/security/mixedContent/secure-page-navigates-to-basic-auth-insecure-page.https.html
http/tests/media/video-auth-with-allowCrossOriginSubresourcesToAskForCredentials.html
http/tests/loading/basic-auth-resend-wrong-credentials.html
http/tests/misc/401-alternative-content.php
http/tests/loading/authentication-after-redirect-stores-wrong-credentials/authentication-after-redirect-stores-wrong-credentials.html
http/tests/xmlhttprequest/remember-bad-password.html
http/tests/misc/authentication-redirect-3/authentication-sent-to-redirect-same-origin-with-location-credentials.html
http/tests/security/mixedContent/insecure-basic-auth-image-allowCrossOriginSubresourcesToAskForCredentials.https.html
http/tests/security/mixedContent/secure-redirect-to-secure-redirect-to-basic-auth-secure-image-allowCrossOriginSubresourcesToAskForCredentials.https.html
http/tests/misc/authentication-redirect-2/authentication-sent-to-redirect-same-origin.html
http/tests/security/401-logout/401-logout.php
http/tests/xmlhttprequest/failed-auth.html
http/tests/security/mixedContent/secure-page-navigates-to-basic-auth-secure-page-via-insecure-redirect.https.html
http/tests/media/video-auth.html
http/tests/loading/basic-credentials-sent-automatically.html
Comment 31 EWS Watchlist 2018-08-15 19:34:24 PDT
Created attachment 347237 [details]
Archive of layout-test-results from ews117 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 32 Alex Christensen 2018-08-15 20:56:31 PDT
Created attachment 347238 [details]
Patch
Comment 33 WebKit Commit Bot 2018-08-15 22:38:46 PDT
Comment on attachment 347238 [details]
Patch

Clearing flags on attachment: 347238

Committed r234912: <https://trac.webkit.org/changeset/234912>
Comment 34 WebKit Commit Bot 2018-08-15 22:38:48 PDT
All reviewed patches have been landed.  Closing bug.