Bug 186870 - NSURLAuthenticationMethodOAuth challenges are surfaced to clients in -didReceiveAuthenticationChallenge as NSURLAuthenticationMethodDefault
Summary: NSURLAuthenticationMethodOAuth challenges are surfaced to clients in -didRece...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit API (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified All
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-06-20 19:08 PDT by Ansh Shukla
Modified: 2018-08-15 22:38 PDT (History)
12 users (show)

See Also:


Attachments
Patch (6.54 KB, patch)
2018-06-20 19:39 PDT, Ansh Shukla
no flags Details | Formatted Diff | Diff
Patch (7.72 KB, patch)
2018-08-09 13:25 PDT, Ansh Shukla
no flags Details | Formatted Diff | Diff
Patch (46.89 KB, patch)
2018-08-14 14:25 PDT, Ansh Shukla
no flags Details | Formatted Diff | Diff
Patch (45.34 KB, patch)
2018-08-14 15:01 PDT, Ansh Shukla
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-sierra (2.59 MB, application/zip)
2018-08-14 15:56 PDT, Build Bot
no flags Details
Patch (42.03 KB, patch)
2018-08-14 16:42 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (44.23 KB, patch)
2018-08-15 14:16 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-sierra (2.98 MB, application/zip)
2018-08-15 15:14 PDT, Build Bot
no flags Details
Fix layout test (44.00 KB, patch)
2018-08-15 15:56 PDT, Ansh Shukla
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews115 for mac-sierra (3.53 MB, application/zip)
2018-08-15 16:26 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews104 for mac-sierra-wk2 (3.22 MB, application/zip)
2018-08-15 16:31 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews122 for ios-simulator-wk2 (8.49 MB, application/zip)
2018-08-15 17:17 PDT, Build Bot
no flags Details
Patch (47.86 KB, patch)
2018-08-15 17:29 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-sierra (2.74 MB, application/zip)
2018-08-15 18:38 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews117 for mac-sierra (3.34 MB, application/zip)
2018-08-15 19:34 PDT, Build Bot
no flags Details
Patch (43.07 KB, patch)
2018-08-15 20:56 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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 Build Bot 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 Build Bot 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 Build Bot 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 Build Bot 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 Build Bot 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 Build Bot 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 Build Bot 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 Build Bot 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 Build Bot 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 Build Bot 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 Build Bot 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 Build Bot 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 Build Bot 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 Build Bot 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.