Problem
Drupal\Core\Authentication\AuthenticationManager::challengeException only allows one authentication provider to send the challenge. For example, Basic Auth uses it to set WWW-Authenticate and the exception to UnauthorizedHttpException. This limits it to one authentication provider, if another one wishes to send it's challenge alongside BasicAuth, it cannot do so.

Proposed Resolution
Allow authentication provider to return string so that the manager and collect challenges from all providers and create the UnauthorizedHttpException itself having WWW-Authenticate. If a provider returns an exception, pass that exception as it is back.

Remaining Tasks
* Tests
* Figure out a better solution, if any?

API Changes
* Drupal\Core\Authentication\AuthenticationProviderChallenegeInterface::challengeException() renamed to challenge() since it is no longer exclusive for exceptions

Comments

Dragooon created an issue. See original summary.

skyredwang’s picture

Version: 9.x-dev » 8.0.x-dev
Component: other » base system
Dragooon’s picture

FileSize
6 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] Unable to apply patch 0003-Unit%20tests.patch. Unable to apply patch. See the log in the details link for more information. View
10.47 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 106,845 pass(es), 1 fail(s), and 0 exception(s). View

I've attached unit tests patch and combined patch

Dragooon’s picture

Status: Active » Needs review

The last submitted patch, 3: 0003-Unit tests.patch, failed testing.

The last submitted patch, 0001-Allow multiple WWW-Authenticate challenges.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 3: 0003-Allow multiple www-authenticate challeneges.patch, failed testing.

Dragooon’s picture

Status: Needs work » Needs review
FileSize
10.47 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 106,850 pass(es). View

Ah, I missed a period which caused the test to fail. Sorry about that.

skyredwang’s picture

Status: Needs review » Needs work
  1. +++ b/core/modules/system/src/Tests/Authentication/AuthenticationProviderHeaderTest.php
    @@ -0,0 +1,38 @@
    \ No newline at end of file
    
  2. +++ b/core/modules/system/tests/modules/auth_provider_test/src/Authentication/Provider/TestAuthProvider.php
    @@ -0,0 +1,41 @@
    \ No newline at end of file
    
Dragooon’s picture

Status: Needs work » Needs review
FileSize
11.71 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 106,880 pass(es). View

Oops, added newlines at the end of both the files.

dawehner’s picture

+++ b/core/lib/Drupal/Core/Authentication/AuthenticationManager.php
@@ -117,16 +132,20 @@ protected function getProvider(Request $request) {
+   * @return array(string)

we use string[] for that

skyredwang’s picture

Status: Needs review » Needs work
Dragooon’s picture

Status: Needs work » Needs review
FileSize
11.71 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 106,873 pass(es). View

Okay, updated the patch to use string[] instead of array(string).

skyredwang’s picture

Status: Needs review » Needs work

In order to ensure the core prints the correct www-authenticate string. It's useful to create a unit test to compare the 403 page header against the enabled authentication methods list. But, I don't see it in #13 patch.

The last submitted patch, 13: 0001-Allow-multiple-WWW-Authenticate-header-values.patch, failed testing.

Dragooon’s picture

Version: 8.0.x-dev » 8.1.x-dev
Dragooon’s picture

It should send WWW-Authenticate on 401 (Unauthorised) and not 403 (Forbidden), for which I have a unit test. Should it be different? I've fixed the patch to apply on 8.1.x.

Dragooon’s picture

Status: Needs work » Needs review

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.