The 2026 Drupal Business Survey is open. Your response will help shape the future of the Drupal business ecosystem.Problem/Motivation
When using the module in basic auth systems which is mostly only test but can only be in production produces false green and false red results.
The executablePhp check is always green because the basic auth and the header check is always red because of it.
Steps to reproduce
Run tests in basic auth protected system.
Proposed resolution
Step 1: Add a new check which at first tries to load a normal asset file via guzzle (e.g. on from the core). If this isn't possible there should be a warning that other guzzle based checks can not proceed useful (and maybe also stop). The warning should contain a link to documentation especially to the solution of step 2.
Step 2: Add basic auth config to httpClient calls and add a check of a file which should always be accessible to identify the basic auth problematic.
As far as I know there is no core standard config for local basic auth credentials. For example the entity_print.print_engine.dompdf can be used with basic auth by config and added by settings.php
$config['entity_print.print_engine.dompdf']['settings']['username'] = '{{ access_user }}';
$config['entity_print.print_engine.dompdf']['settings']['password'] = '{{ access_pass }}';
Especially as a security oriented module we should encourage people to also use settings.local.php outside web root. We can maybe avoid UI form to force.
Remaining tasks
Add code for step 1 and step 2.
User interface changes
Additional warning about guzzle access problems and link to documentation of step 2 solution.
API changes
-
Data model changes
-
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | pngout.png | 68.28 KB | andrew.wang |
Issue fork security_review-3454933
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
andrew.wang commentedI encountered the same issue - header check is always failing for my basic auth protected site:
Comment #3
smustgrave commentedHello @c-logemann since it's been a few months wonder if you want me to un-assign?
Comment #4
c-logemann@smustgrave Sorry I was to busy in last time. Especially this issue is easy to solve when maintainer decides how to manage credentials.
Comment #5
jurgenhaasWe run into a related issue. We don't have basic auth enabled, but we have intranets without any content for anonymous users. Therefore, the 403 setting in
/admin/config/system/site-informationis set to/user/login. But that 403 causes the header test to fail as well, although the response contains the headers. But I assume that the 403 is enough for not testing in more detail.This could be resolved by allowing the optional configuration of a patch that should be tested instead of the plain result of
$host = $this->requestStack->getCurrentRequest()->getSchemeAndHttpHost();Comment #6
smustgrave commentedThis plugin already uses a setting form so expanding to an a "Alternative host path to test" wouldn't be terribly difficult is that you were proposing?
Comment #7
jurgenhaas@smustgrave an alternative path would certainly help for the scenario reported in #5, but probably not for the IS with basic auth.
Comment #8
smustgrave commentedMaybe can address the basic auth if they can use a path with a username/password in the URL
Comment #9
smustgrave commentedComment #11
smustgrave commentedThoughts?
Comment #12
jurgenhaasThis is looking good. just added one suggestion and a related one.
Comment #13
c-logemann@smustgrave Guzzle has an official option to handle authentication. I think implementing an configuration option for this is still the cleaner way to solve this even if it could be done via alternative host. But primary this issue address wrong results of other tests when guzzle is blocked by basic auth.
Comment #14
c-logemannSo in any case we need a warning when tests are blocked via 403 which then hopefulle points on one ore maybe more options to solve this.
Comment #15
jurgenhaas@C-Logemann but authentication would solve the issue that's being addressed by the MR now. It's for scenarios where the front page is not accessible for anonymous users. Only the login page can be tested in that scenario, that's why an alternative path helps in that case.
However, basic auth is another issue that isn't fixed by the alternative path, and that could do with the guzzle auth options.
So, maybe both attempts need to be available?
Comment #16
smustgrave commentedGoing to say this is more a feature request now.
Comment #17
c-logemann@smustgrave The checks currently rise false green and false red results. So this part is an bug in my opinion. Allowing checks via basic auth and additional host can be seen as feature request.
@jurgenhaas I didn't mean that we cannot fix both scenarios at once. I think they can even come together. So we should do a check on 403 and offer/suggest at least the both solutions to fix security check on 403: basic auth and alternative host path.
Comment #18
smustgrave commentedAsked Claude this one and it's recommendation would be to do something like
$config['security_review.settings']['basic_auth']['username'] = getenv('SR_HTTP_USER');
$config['security_review.settings']['basic_auth']['password'] = getenv('SR_HTTP_PASS');
And if present try and use those values.
Comment #19
c-logemann@smustgrave Nice example for wasting energy with so called AI without delivering any useful help of it. How the password can be configured is already described in the summary.
It's not a problem to define a config. It's a question about do we want to create another config for something the core should have. But if the provided config is wrong or not present how to handle the situation because this won't fix the situation of "executablePhp check is always green" which is based on any 403 message. For this we need a solution. I think I should start with the easiest part before someone ask another "AI" Tool how to tell Guzzle to use password.
Comment #20
smustgrave commentedlol attitude isn’t needed but quick way to get no help or reviews going forward.
Best of luck
Comment #21
c-logemann@smustgrave I'm not a native speaker and I'm not sure if I got your message right. In any case I appreciate any help and a discussion about where so called "AI" is helpful or not doesn't belong to this issue and bringing this critic in my comment wasn't helpful as well.
I used this little conversation to think about this issue and came to the conclusion that the issue summary does currently do not reflect the main problem which would be better for humans and machines. But before I found time to improve the issue summary I already found an idea I like to share that others can also think about it if this can solve the current dilemma:
The current executablePhp test only tries to execute the PHP file. When protected via basic auth the check marked as passed without really checking the PHP.
My new idea is to add another test on a "normal" asset file to check if guzzle has normal access to the webserver. If this isn't possible there should be a warning (with a link to documentation) that guzzle based tests cannot be processed and maybe this processing should also be stopped.
Now with this idea I assigned myself again.
Comment #22
c-logemann