Basic Auth module conflicts with server-level "Site Lock" implementations

I think the best and only sane solution is to not use the basic_auth module when you use such a Site lock feature on your hosting platform. Once we have OAuth2 in core (#2834718: New experimental module: OAuth2), that'd be realistic.

That being said: couldn't we solve this by using a different realm? And perhaps the problem is that the basic_auth module is not validating the received realm?

See http://stackoverflow.com/questions/12701085/what-is-the-realm-in-basic-a....

That sounds good :)

Do you perhaps have the capacity to take this on?

Hey! I need to test correctly this patch but my local tests don't work

This is apparently also a problem with the Shield contrib module: #2815945: Make shield work with D8 core basic_auth (1).

Tested with my site. this patch works for me

This will need test coverage before it can go in.

It was working fine before drupal latest release 8.2.6. but now again causing problem.

I believe the patch in #6 only works if the site name is Drupal. I created a new function to generate the basic realm string since it needs to be used in the exception and the verification.

None of the patches here work $request->headers->get('www-authenticate'); returns null for me.

I think a better solution would be checking (at least) the username is valid before going any further.

Note: Shield really needs the patches from #2822720: Exclude/Include Pages from Shield Protection (D8 Feature Port) and #2815945: Make shield work with D8 core basic_auth (1) then basic auth paths (such as Rest or Relaxed Web Services) need to be white-listed in shield.

Just to bring a bit of insight into those hunting down why Relaxed along with Workspace might be throwing Access Denied with Server-side Basic Auth setups, the patches in #14 must be applied for you to dig yourself out of that hole.

I also recommend, as a way to speed up your development process, use a Header add-on and place the following in two modify statements:

  Authorization: Basic base64_encode(username:password)
  www-authenticate: Access restricted

This should make your lives a bit easier

www-authenticate header is response header, not request and is populated by app, so none of previous patches will work. In shield (https://www.drupal.org/node/2822720#comment-12154615) I posted patch to whitelist all paths with _format query as I dot know if there is any other way to say that the request is rest.

I think the best and only sane solution is to not use the basic_auth module when you use such a Site lock feature on your hosting platform. Once we have OAuth2 in core (#2834718: New experimental module: OAuth2), that'd be realistic.

The OAuth2 specification supports using Basic Auth to provide the client id and password (see https://tools.ietf.org/html/rfc6749#section-2.3.1). Simple OAuth (the candidate in #2834718: New experimental module: OAuth2) supports that feature since #2852290: Support HTTP Basic Auth for client ID and secret. Given that, I am confident that Simple OAuth is also afflicted by this issue. There's already a support request for it #2882780: Simple OAuth on a site with access protected by basic auth.

Marked #2618780: Basic Authentication breaks site when used with another basic auth protection as a duplicate.

We currently don't consider ways of figuring out if a username is registered on a site a security issue, but this would allow to differentiate between non-existing usernames and valid username with invalid password fairly easily. Especially since it would also bypass flood protection.

Also, the login service is.. a service. That can be replaced. We do that in various project to allow e-mail or username for login, hardcoding the username check would break that.

I did some debugging and found out the following:

This filter denies access on routes that do not define an authentication provider unless the authentication method is global:

Looking at authentication providers, cookie is global:

But basic auth is not:

I think that the current setup in core is correct by filtering supported authentication methods. However, for cases where basic authentication at the web server level is needed, I have created https://www.drupal.org/project/basic_auth_global in order to allow navigation as an authenticated user.

Thoughts on this?

Hello,

I just ran into the same problem for a staging environment.

Thanks for the workaround with the basic_auth_global module.

For me the patch in 17 worked
the basic_auth_global module did not work

Can we define basic_auth.authentication.basic_auth with global: TRUE?

I don't think we can do that — that'd mean it's always allowed. That'd be a huge change.

In our case, https://www.drupal.org/project/basic_auth_global works like a charm as a workaround. Gracias @juampynr

https://www.drupal.org/project/basic_auth_global not working for me on pantheon with Drupal 8.6.4. I have to turn the site lock off in order for it to work.

#17 works for me i am using Drupal 8.6.4

Hi!

#17 works for me too in 8.6.13. Thanks for the patch :)

#17 worked for me too

Can anyone confirm if patch #17 is working with 8.6.15? I had it working on 8.5.x but after updating to 8.6.15, I'm back to the original issue of the site only showing the login page, no matter where you navigate.

Hi!

I've tested again the patch and it's still working for 8.7.0.

Thanks!

Good!

Just a recap: this is a problem with a conflicting setup. The Basic Auth Drupal core module has the same issues. We have a patch in #17 that seems to be working for some.

It does work for me too

I use Drupal 8.7.2 for now and still have a problem with basic_auth module. I get error 403 You are not authorized to access this page. I tried to use a patch #17 but this code doesn't solve the problem.

Hello guys,

I comfirm, patch #17 is working for me.

I needed to protect some dev sites (I do not have a need for the patch on production) and #17 worked as expected.

I will move this to RTBC so that it catches someones attention, otherwise it will be stuck here forever.
Feel free to update this as required.

@bserem something cannot be rtbc when the tests are not passing and there is not consensus that this is the correct approach. Making something rtbc to just get attention is not the best way to move something forward. Now, for example, if I look at this issue I'm rewarding the behaviour which I don't want to do.

#17 works for me on 8.7.1.

#17 works for me as well. Thanks!

This issue has gone unsolved for too long. Let's get it back on track.

1. I agree with @Berdir's concern in #24 about the patch in #17: We currently don't consider ways of figuring out if a username is registered on a site a security issue, but this would allow to differentiate between non-existing usernames and valid username with invalid password fairly easily. Especially since it would also bypass flood protection.
2. The problem with making \Drupal\basic_auth\Authentication\Provider\BasicAuth authentication provider "global" like @juampynr explained in detail in #26 and made possible via his https://www.drupal.org/project/basic_auth_global contrib module (thanks, @juampynr!) is that it allows HTTP Basic Authentication to be used on all routes by all users simply by installing the basic_auth module. That's what making an authentication provider "global" means.

This is very tricky to solve without negative security impact (definitely in #24, arguably also in #26/https://www.drupal.org/project/basic_auth_global) in Drupal itself.

Having come up with this summary of the current situation … I wonder why server-level basic auth doesn't just strip the incoming Authorization request header if that server has HTTP Basic Authentication enabled? Because if that header is targeted at the infrastructure (server) level, that infrastructure shouldn't also pass it on to the application (Drupal) level. That's a best practice in HTTP intermediaries anyway.

So … which hosting companies are running into this problem?

We also walked against this issue by combining the basic_auth module and the HTTP authentication shield of Platform.sh.

Stripping the Authorization header sounds like a good idea. I've passed this on to Platform.sh in a support ticket.

So … which hosting companies are running into this problem?

pantheon.io

I'm not entirely sure if this might help or not, but I'm currently working on a custom environment solution for a client (AFAICT it's LXC based with Apache2 webserver on the container).

The container which hosts the Drupal site is protected via Apache2 password (.htpass) and with Basic Auth module activated, all the pages (including the homepage) are returning 403 Forbidden - handled by Drupal, not Apache2, mind you.

I noticed this when a feature that uses Basic Auth for REST API calls was deployed to the staging server. It became normal (although the feature was unusable) when the client uninstalled the basic_auth module via drush.

Any ideas on how to circumvent this or maybe set up something different on Apache so that this doesn't happen?

works for me as well on 8.8.5 Thanks!

Same as #17 but with entityTypeManager for D9 Compatibility

Using #17 , works for me!
We also had a redirect loop. really tricky

Using #17 , works for me as well!
In our case we just had all of the petitions as "Access Denied"

I recently hit this wall with a migration from a dev location to the live environment. I am running 8.9.13 and used the modified patch in #57 and it seemed to work.

I didn't attempt the Basic Auth Global module.

Now, a question might be that since Drupal 8/9 run services, would this issue be worked around or solved (without creating potential security issues like the suggested patches/module) with a third party auth (Google, Facebook, Twitter, etc). Or am I missing something?

Patch #57 works for me on 9.1.9, thanks!
We were getting Access Denied on /user/login.

this patch does work for me

Tagging for the Bug Smash Initiative.

I ran into this as well and the patch in #57 fixed it. I'm on Drupal 9.5.8.

The solution from #57 still works fine on Drupal 10.2. Is there any reason not to (reroll and) commit this?

This was discussed at a Bug Smash meeting yesterday with larowlan, dsci, and pameeela. The conclusion was that this is a feature request. The issue summary took a while to understand so adding the tag for an IS update.

@dsci plans to come back to this issue later this week and do triage. Let's hope that happens.

Updated IS to make the use case a bit more clear, but it probably still needs further updates.

Discussing in Slack we are not sure this is a bug, as even the OP concedes.

As described in #70, the patch in #57 works for us with Drupal 10.3.5, it would be great if this issue is going further in progress.

Edit: ok unfortunately it does not always work, sometimes the theme crashes and access denied appears everywhere, although correctly logged in.

I think #24 is a valid concern (see #3241232: [policy] Treat username enumerations as security bugs that require Security Advisories).

Updated issue summary with plans to move forward with this.

Added the username check on the basic auth prompt (hidden behind a setting) and a simple test case for it.

> The basic auth provider will only act upon routes which have been explicitly tagged with the basic_auth _auth option.

If basic_auth is enabled, and basic auth is received on a route that is *not* tagged for basic auth, should we log or display a warning message?

> The basic auth provider will only act upon routes which have been explicitly tagged with the basic_auth _auth option.

If basic_auth is enabled, and basic auth is received on a route that is *not* tagged for basic auth, should we log or display a warning message?

That's an accidental entry to the issue summary, and actually meant for the #2839210: Basic auth returns 403 when username & password supplied but not needed. issue.

The changes so far only check for the username's existence.

But no, I don't think it's worth logging it as modules making use of basic_auth should already be tagged as such, and bad actors can use this as a means of filling up the logs without suffering the penalty of being banned by the flood control system.

Instead of having a settings.php entry, how about a submodule, e.g. "Basic Auth with Site Lock" (the name probably needs work) - this would act as the flag instead? The module description and hook_help can explain why you might want this enabled (or not!) if you are using Basic Auth.

We have something similar with "Layout Builder Expose All Field Blocks" where additional functionality is enabled via a submodule.

Instead of having a settings.php entry, how about a submodule, e.g. "Basic Auth with Site Lock" (the name probably needs work) - this would act as the flag instead?

The feature flag module idea is interesting, but seems like potential overhead (haven't benchmarked how expensive calls to the module handler checks are) for a feature that most sites would probably rarely need.

I took inspiration with how the phpinfo() status page behaviour was changed, and the setting seemed like the way to go for a functionality tightly coupled with the site's security.

However if that's the convention going forward for core to make it easier for non-technical people to work with it, then I have no objection. Would likely need input from members on the core team to provide further direction on this.

Yeah, it's never been clear to me where this sort of thing should go. Right now we have an awkward combination of:

• $settings array in settings.php
• $config array in settings.php
• config form with a UI
• container parameter in services.yml
• feature flag module
• some new API? #3423352: Add an API for feature flags

Also ideally it wouldn't be a moduleExists() check, instead the code to handle this feature would live self-contained in the contrib module, extending basic_auth as necessary.

Also ideally it wouldn't be a moduleExists() check, instead the code to handle this feature would live self-contained in the contrib module, extending basic_auth as necessary.

Trying to solve this in a separate module seems like it'd complicate the codebase a bit more since logic might need to be duplicated or services overridden/decorated.

Flagging as RTBC since the settings approach seems like the easiest way to address the edge-case without adding too much bloat to the codebase. But if anyone's able to achieve a similar solution without complicating the code, then that's more than welcome too.

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

#17 / #57 started causing an issue for us after upgrading to Drupal 11. We got the error

InvalidArgumentException: Class "domain_entity_query_alter" does not exist. in Drupal\Core\DependencyInjection\ClassResolver->getInstanceFromDefinition() (line 32 of core/lib/Drupal/Core/DependencyInjection/ClassResolver.php).

Drupal\Core\DependencyInjection\ClassResolver->getInstanceFromDefinition() (Line: 78)
Drupal\Core\Utility\CallableResolver->getCallableFromDefinition() (Line: 742)
Drupal\Core\Extension\ModuleHandler->getHookImplementationList()
array_map() (Line: 476)
Drupal\Core\Extension\ModuleHandler->getCombinedListeners() (Line: 457)
Drupal\Core\Extension\ModuleHandler->alter() (Line: 491)
Drupal\Core\Database\Query\Select->preExecute() (Line: 516)
Drupal\Core\Database\Query\Select->execute() (Line: 288)
Drupal\Core\Entity\Query\Sql\Query->result() (Line: 85)
Drupal\Core\Entity\Query\Sql\Query->execute() (Line: 646)
Drupal\Core\Entity\EntityStorageBase->loadByProperties() (Line: 81)
Drupal\basic_auth\Authentication\Provider\BasicAuth->applies() (Line: 340)
Drupal\shield\ShieldMiddleware->basicAuthRequestAuthenticate() (Line: 265)
Drupal\shield\ShieldMiddleware->bypass() (Line: 228)
Drupal\shield\ShieldMiddleware->handle() (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle() (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle() (Line: 53)
Drupal\Core\StackMiddleware\AjaxPageState->handle() (Line: 54)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle() (Line: 745)
Drupal\Core\DrupalKernel->handle() (Line: 23)

which lead us to #3514467: domain_entity_query_alter() is interpreted by core as an implementation of hook_entity_query_alter() but it turns out that is actually caused by #3277210: Shield middleware invokes hooks before modules are loaded, corrupting module_implements cache. So it turns out that to cover for multiple cases with different hooks we need to make sure that the module handler loads all enabled modules before loading the user. I am attaching a patch that does this in place.

Interesting, and does the patch in the shield module not address the issue for you?

As shield is the trigger for your specific issue, so loading all the modules before it calls other auth providers should've addressed it.

And as per catch's comment here, the middleware shouldn't be invoking hooks like that, and the shield logic probably needs updating to use a kernel request listener that runs after all the modules have been properly loaded.

Marking as Needs review.

The Needs Review Queue Bot tested this issue. The merge request has merge conflicts and cannot be merged. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.