Make shield work with D8 core basic_auth (1)

Here is a hacky patch that will disable the variables that cause basic_auth to execute. This will allow you to use basic_auth and shield independently of each other:

aka: use shield on a dev environment, but not basic_auth, basic_auth on production but not shield (by disabling the username in the shield settings).

Not really the solution for this issue but its a bandaid workaround for some use cases.

After spending some time on this with Chx, we realized that how shield works as middleware won't work for this use case. Its too soon in the request process, which means we cannot authenticate the user.

chx said something about rewriting shield to be a subscriber. #overmyhead ;)

The root cause is this: #2842858: Basic Auth module conflicts with server-level "Site Lock" implementations. Basically, core's basic_auth does not specify a realm. If there are multiple levels of basic authentication, you need different realms, otherwise they'll conflict.

(If Shield is not yet specifying a realm, then it should fix that too.)

The Basic realm is being set with whatever is in Shield's print config. (screenshot below)

In light of comment #6, we may need to remove the print config and simply change this to a hard coded value such as Basic realm="Shield".

From what I can see #3 was in the right direction, but I think we should only null the username and password if they've been used to authenticate.

Although eventually I think it'd be good to make shield an authentication_provider rather than middleware.

Although eventually I think it'd be good to make shield an authentication_provider rather than middleware.

I first agreed with this, but then I realized this is wrong: Shield does not authenticate a Drupal user, it's merely an extra protection layer, an extra shield.

@wimleers right, I just noticed that when putting together a proof of concept, I'm wondering if we could return user account 0, but that seems a bit hacky.

Heh, clever!

That might work. It also might violate one of the most basic assumptions of authentication providers. IDK, we'll have to dive in to figure that out. It would definitely be a hack though.

The patch #8 works for me. You've done a great job to work around what seems to be more a fault of the basic_auth core module.

I was surprised and annoyed to be hit by this problem. And once hit it's remarkably hard to clear it - chrome holds onto the basic auth credentials until you quit and restart.

#12: this is a bug in core's basic_auth module. See #2842858: Basic Auth module conflicts with server-level "Site Lock" implementations, which is linked as a related issue. Please help with getting that fix in!

Re-rolled patch for 8.1.1 release of Shield Module: https://www.drupal.org/project/shield/releases/8.x-1.1

this patch allows use endpoint rest and web service calls

I am using Drupal 8.5.1 and Shield 8.x-1.2 versions. After enabling and configuring SHIELD module, my site is showing an error message on all the pages.

Error message

*~*~*~*~*
You are not authorized to access this page.
*~*~*~*~*

If I disable the SHIELD module then site works perfectly. The patch [https://www.drupal.org/files/issues/shield-basic-auth_2923801_8.x_2.patch] did not help me too.

Any help is appreciated.

Attached is the patch with proper formatting which is accepted by composer too.

It looks like these patches convert spaced indent to tabs?

This patch has not solved my problem.

The sign in window keeps popping up.

I did apply this patch after having first experienced the problem so have not been able to log into my site and change any of the settings. I'm not sure if this makes a difference.

I have normalized the spacings in the patch.

The patch worked on my environment.

Don't forget to reenable shield and basic_auth and run drush cr after patching.

Also, very important, refresh, restart or clear the browser cache.

Patch in #20 allows me to have both shield and basic_auth enabled.

I have inspected the code and I think this could be better implemented by calling the basic_auth service to get the benefit of its flood mechanism.

I am preparing a new patch.

The attached patch should be more robust. It uses the basic auth service if it exists to check the credentials if they are not defined in shield.

It could be interesting to have a flood control feature for shield in the future.

Sorry I forgot the interdiff.

I'll review the patch now. This is truely critical and we should have a new dev and stable release.

I sadly cant' confirm #24 to work. Then basic_auth is enabled shield still blocks all requests with access denied. Or do we need a further module / patch to make it work?

@Anybody: did you clear your browser cache prior to test?

edit: and run drush cr?

Yes I even tried a different browser, deleted cookies, drush cr multiple times, etc... but let's get further feedback, perhaps it's a different reason for me.

Okay! I am currently using this version of shield: 8.x-1.2 on Drupal 8.5.5. If I have some time, I will try to look on a fresh install.

I looked into the patch and it is not clear to me who comes first, and why, shield and/or basic_auth. Is it even determined or left to chance? Do they both use the middleware api? Whatever the answer is, let's write it into the source.

Hello axel.rutz, shield comes first, and if not positive, basic auth is called. As to why, I have just kept the order from the previous commits patches, but we can think about a configuration + permission to let the admin choose which method is preferred over the other(s?).

I didn't know about the middleware API but shield seems to have implemented the HttpKernelInterface in the ShieldMiddleware.php file. That said, I am not sure about how to improve the code to make better use of the middleware API... ^^'

The patch in #23 worked for me.
When using the Deploy Suite basic_auth is a dependency early on. With the patch this can coexist on a dev site with shield enabled.
I tried before https://www.drupal.org/project/basic_auth_global which is supposed as I understand to solve the same problem, but that did not work.

Small modification to the patch to ensure that $user always has a default declaration. Without this, when I was running a suite of Behat tests on my site my tests were failing due to Undefined variable: user. This was previously addressed by issue #2990069, but that patch does not apply cleanly on top of this new patch.

Missed the patch for shield.services.yml.

This looks similar to the related issue. Please consolidate.
Also going to commit patches touching this code, so probably needs reroll.

Hm, the patch in #36 doesn't seem to apply anymore :(

I've changed the patch so it works with the Shield 1.3-rc1 version.

shield$ patch -p1 < 2815945-shield-1.3-rc1_basic_auth_compatibility-39.patch
patching file shield.services.yml
patching file src/ShieldMiddleware.php

For me the patch still doesn't work. I changed the patch to remove PHP_AUTH_USER and PHP_AUTH_PW headers rather than set them to null.
Not entirely sure is the right thing to do but is working for me now.

Recreated patch #40

re-rolled the patch in comment #41.

Tested patch from #43 and now able to have both Shield and Basic Auth enabled. RTBC for me, thanks!

I've rerolled the patch for 1.4

Tested with patch #43. working as expected in my case which includes PDB Vue module and entity share Module.

Rerolled patch for 1.4

Updated patch #48, Since basic auth module enable check should be applied when shield is disabled.

Rework of #49 based on 1.x-dev

Thanks for the patch. It works as long as the 'user'-field is not empty in the Shield 'Credentials' configuration section. When I leave user and password empty the basic_auth endpoints break again.

Additionally I have the issue that when the user enters the login data of a drupal user account he seems to be logged in but isnt really. It seems like the user and pass in the header get carried with every request which basically authenticates the request but the user is not really logged in.

I was able to fix the issue by removing the lines of the exception handling from the patch.

+++ b/src/ShieldMiddleware.php
@@ -141,8 +156,37 @@ class ShieldMiddleware implements HttpKernelInterface {
+        // Basic auth second chance.
+        /* @var $basicAuthService \Drupal\basic_auth\Authentication\Provider\BasicAuth */
+        $basicAuthService = \Drupal::service('basic_auth.authentication.basic_auth');
+
+        /* @var $requestStackService \Symfony\Component\HttpFoundation\RequestStack */
+        $requestStackService = \Drupal::service('request_stack');

Are these services intentionally not injected? If so, let's document why.

Rerolled the patch for version 1.4

Sorry everyone, the path I provided in the previous comment is invalid. Providing a working one.

Continuing work from #52 and fixed code issues as suggested in #53

Patch #52, failed in following setup :-

• Shield
• Basic auth
• JsonAPI

1. Failed while accessing unpublished content by jsonapi with shield ON or OFF
2. Access denied while shield is OFF

Fix failed test case.

#59 works for me on 1.x-dev.

As patch #59 suggested by Bunty is working fine for me

RTBC
Drupal ver: 9.x
DB: Mariadb

The latest patch does not apply anymore against the 8.x-1.x branch. Because I suspect there may be cases where the patch is not 100% accurate with basic_auth, I added an hidden settings (no UI) to handle basic_auth headers or not. By default, this option is turned on so basic_auth headers are unset as per the patch.

I will try to create a unit test for this use case.

A new patch which adds a unit test to validate we get a 403 when basic_auth is enabled and basic_auth headers are not unset by shield.

Given the previous patch (#59) was RTBC and that my recent changes are just adding test and hidden setting, I will merge it into 8.x-1.x.

It has been merged into 8.x-1.x. Thanks for the discussion, the patch and the reviews.

@vbouchet: I was using Shield in the latest stable release (8.x-1.4) and ran into this problem.
Since none of the recent patches were applicable to the 1.4 code base, I've switched to using Shield from the dev branch, fixed to the latest commit (a2c0b8845eb7c4546407d3e48fb0098235d1cc4c).

I've noticed that I still had this issue. While there is a new config "unset_basic_auth_headers" which is also in the installation schema, sites where Shield is already enabled won't have this new config.
I've fixed this by manually fixing the shield config and running drush cim -y but I guess we want to add an update hook here to add this new config?

@dpacassi, thanks for the feedback. I realised it earlier today and missed time to do so. Please find a patch with the code I will release immediately on 8.x-1.x to avoid more site being broken. I will keep this issue open for some time to get feedbacks on the fix.

The "fix/workaround" has been release with 1.5 ~2months ago. Closing it.