🇩🇪Germany @C-Logemann

Yes, it's true. With the "data-" prefix we get a little bit of more functional data to manage and serve. But it's so minimal that if you want to safe some data there are so many possibilities without getting invalid: Maybe w shorter IDs and classes. e.g. "m-item" instead of "menu-item" etc. Or we could move default public files folder from "/sites/default/files" to "/files" so all links to css etc. would be shorter. And as far as I understand it's not needed to add <meta name="Generator" content="Drupal 10 (https://www.drupal.org)"> to any page.

I started with a a serach prelace of " hx-" with " data-hx-" but didn't touched the two JS files. One reason is that I'm not so familiar with JS which is the reason why really like HTMX.

C-Logemann → created an issue.

C-Logemann → created an issue.

I work on a contrib module "signal" which will also provide a "bridge" between Browser UI and CLI to solve several similar problems/ CLI tasks. There will be a Drush command to "ask" Drupal for tasks, work on the ask an return a report which can be viewed in Browser UI. To keep things light every signal type will be an entity bundle. But I think the automatic update possibility is so important that I will help to configure this as easy as possible e.g. with additional config recipe and/or code if needed.

C-Logemann → created an issue.

Adding something to "oauth2_server.api.php" and a test was already planned. And I will think about the response situation.

setServerData() is currently triggered via cron. This could be an interface to run cron via http request. But in CI situations cron isn't always wanted depending on the system needs.

Currently setServerData() is just returning null on cli usage (see also 🐛 Get server uid function return unexpected type null Active . This is avoids corrects tests but even if we just open it would get wrong values of the file owner and they would be store in state api.
Even if the current code will not mess up with the state API storage and cli can get correct serverdata.- This only would be available after a successful check via GUI. Beside the basic auth problematic (see related issue) this cann not work on a fresh test system e.g. in CI-Workflow.

C-Logemann → created an issue.

C-Logemann → created an issue.

@smustgrave The null comes from a a cli check with empty return on setServerData().

As pointed out on duplicate issue I suggest to return a posix compatible "-1" so we don't need to extend the type. See MR on 🐛 [error] TypeError: Drupal\security_review\SecurityReview::getServerUid(): Return value must be of type int, null returned Closed: duplicate .

@smustgrave, just saw the other issue 🐛 Get server uid function return unexpected type null Active . Let's discuss there.

C-Logemann → created an issue.

Because "SecurityReviewData->findWritableFiles" is already checking the fileowner I think it would be a good idea to move this code to another function which could return two the values writable and possibly writeable because of ownership. For backward compatiblity we can keep the old function as wrapper.

The related issue is more important because it directly tests the possibilty to change the ability to change permissions. The file owner check should maybe be separate because there could be server strategies to avoid changes of permissions like chattr and the ownership of a file cannot be used for changing permissions. And if this is the case it would be helpful to deactivate this check.

@smustgrave OK, we need a test which creates an unsave situation to check if the file permission tests is still working correctly? Or is it only to make sure the test will not cause problems?

On my old concept I was only thinking about checking if webserver is the owner of the code. This can also be a good test additionally to the already working search on writeable folder. But this should be handle in different issues.

3.x work is now addressed in this issue: 🐛 file check is problematic "green" when not test with chmod Needs work

Just found my old issue about this: ✨ "Safe file system permissions" should test the ownership of files and directories Closed: outdated

On my test system (D 10.2, nginx, fpm) I changed the file ownership to secured user and the chmod part was just ignored.

I added a file perm read and chmod reset to the code.

I commit a first code which makes the test "real" with

chmod($directory, 0755);

.
But depending on the webserver this can cause problems related to group and others permissions. Maybe we can read the permissions first and just change the user permission. And like good hackers we could clean up by ourself with resetting the permissions as before.

C-Logemann → created an issue.

At first I thought this changed IGNOREME.txt caused my red test. But in my case it's still there after removing an Now I see that the check is on writing process itself not the result. So my problem is currently maybe a problem of my test system or another issue. In any case it looks better when the file is clear at the control line as described in the beginning of the file.

C-Logemann → created an issue.

Firs of all I like to thank @bradjones1 for all the work you've already done on this issue. As soon as I can I like to help with testing in our MariaDB setup.

I have a similar plan like @gapple for the "signal" module with unknown mixed structured data which also like to preserve at it comes from external data sources. With unknown I want to say that this should not be limited to a specific external system as long as it can "report" with json. I will check Reporting API module and maybe integrate.

Additionally I think about some modules where I like to enable some kind of config options content which can be managed by group members. Beside to avoid giving normal users access to Field API it would be a nightmare of multiple field groups with a complete overhead of field schemas to manage this flexibility. Just using a json base field type could just store this user-config as json and it would be searchable when this feature is rolled out.

This mapping helps to determine if a drupal 8+ cookie is available:

map $http_cookie $drupal_cookie {
  default 0;
  ~SSESS 1; # PHP session
}

See this post for example (with "~SESS" for older drupal versions: https://www.webfoobar.com/node/28

The official wiki repo is still available:
https://github.com/nginxinc/nginx-wiki/blob/master/source/start/topics/r...

Because there so many links to the nginx.com wiki I think it's a bad move to just shut it down.
Maybe they need a little bit help to configure a proper redirect.

@solideogloria I don't know if unit fit's all the things I do with nginx config for now.

@andypost Especially angie seems to be very interesting. I planned to try it ASAP.

In our company we already use a cookie check to switch auth request on our daily used test systems. I will work on solution / example with a kind of "@"-section switch I think.

C-Logemann → created an issue.

Because I currently use apache2 not often it's better when somebody else will d. I created this Issue first to relate on it on the next one for nginx and its forks.

C-Logemann → created an issue.

The old nginx.com recipe is gone. Here is a copy in wayback machine:
https://web.archive.org/web/20240511055421/https://www.nginx.com/resourc...

It seems that it's now more important that we have a kind of official nginx example on d.o maybe not only in code.

The related issue is solved because there was just an overseen "minimum-stability" value in project composer.json. After changing to "dev" the grant module was installing. But the tome module still has the same confusing report. Comparing to grant module there is a big difference which probably the cause. Grant has a working module in main namespace which has a dependency on grant:grant_base defined in grant.info.yml. Because there is nothing defined in composer.json I wonder how composer "knows" about the sub modules in modules folder. I think in this composer interpretation of the situation there is an answer an maybe a composer based solution to fix this behavior in tome without changing the current sub module strategy.

Because there was a change in the following file about catch command I looked int this file:
modules/tome_static/src/EventSubscriber/RoutePathSubscriber.php
and found this:
catch (\Throwable $e) {

So it seems this correct dev version code but composer shows installing current stable version "1.12.0". So it' still confusing.

I had over seen "minimum-stability" setting and was than confused about related issue on tome module which is still confusing.

C-Logemann → created an issue.

The tome module does also use submodules. The difference is that tome.info .yml is empty. But the composer result is a little bit strange where only the main project is marked as dev and composer seems to install version 1.12.0 (the current stable) :

 composer require 'drupal/tome:1.x-dev@dev'
./composer.json has been updated
Running composer update drupal/tome
Gathering patches for root package.
> DrupalProject\composer\ScriptHandler::checkComposerVersion
Loading composer repositories with package information
Updating dependencies
Lock file operations: 4 installs, 0 updates, 0 removals
  - Locking drupal/tome (dev-1.x ec6aae3)
  - Locking drupal/tome_base (1.12.0)
  - Locking drupal/tome_static (1.12.0)
  - Locking drupal/tome_sync (1.12.0)
Writing lock file
Installing dependencies from lock file (including require-dev)
Package operations: 4 installs, 0 updates, 0 removals
  - Syncing drupal/tome (dev-1.x ec6aae3) into cache
Gathering patches for root package.
Gathering patches for dependencies. This might take a minute.
  - Installing drupal/tome_base (1.12.0)
  - Installing drupal/tome (dev-1.x ec6aae3): Cloning ec6aae332f from cache
  - Installing drupal/tome_sync (1.12.0)
  - Installing drupal/tome_static (1.12.0)

C-Logemann → created an issue.

C-Logemann → created an issue.

@Joachim: Es war nur noch ein String unstranslated, den ich approved habe. Issue kann geschlossen werden.

I'm also not a native speaker. But I now have an idea what you mean.
Yes, the most problems cam from forms and there we should operate to make Drupal safer.
I believe also captcha modules are doing some logging. And especially when using syslog core module you can handle this with fail2ban: https://en.wikipedia.org/wiki/Fail2ban
If fail2ban is not usable there is the core module "ban" which can block IP addresses:
https://www.drupal.org/docs/8/core/modules/ban/overview →

If not yes available you need a connection between ban and your preferred captcha solution. But this is not in ficus of this module.

I won't suggest to organize clients with roles. In our own project we will organize clients with the upcoming module grant → . And on our client projects I try to reduce roles.

> The verification code is triggered

Which code you mean? Do you think about an additional setting of firewall or something else? And generally on which way this code is or should be provided to the request?

Generally about IP blocking and the firewall module. On a first step we only operate on a layer where we won't communicate with the database. There are already core and contrib modules doings such things like "flood".

In any case there will be a way to log IPs when blocked by this module. So the linux tool fail2ban can recognize and can manage blocking on linux firewall which is way more effective than blocking IPs at Drupal.

Just made a visual check of the #6 patch so I didn't know if this will work:

$dr = $_SERVER['HTTP_REFERER'] ?? : NULL;

The correct form is without ":"

$dr = $_SERVER['HTTP_REFERER'] ?? NULL;

Also, falls der implizite Hinweis in den "Remaining tasks", das nicht deutlich macht. Ich habe die vier gefundenen Fehler bereits mit einem korrigierten Übersetzungsvorschlag versehen. Und das System zwingt uns ja (sinnvollerweise meiner Meinung nach) zu einem Vieraugen-Vorgehen. D.h. trotz grundsätzlicher Permission kann ich meine eigenen Vorschhäge ja nicht selbst freigeben.

C-Logemann → created an issue.

By starting with a proof of concept I figured out that it would be easy to get also control of "Automatic authorization".
I wanted to keep I simple but also allow to react between modules so I started now with an "alter" Hook "hook_oauth2_server_authorize_alter" where &$authorization can be modified based on $client and $current_user. Next I just added a simple "AccessDeniedHttpException();" because I think if someone wants a more sophisticated reaction on a "deny" situation this can also be manged in the hook implementation like error messages and/or redirect etc.

I don't believe that this will come for D7 code but this would be easy possible with custom code when ✨ Add hook to control account client access Active will be accepted.

According to hook_entity_access I think "hook_oauth2_server_authorize" would be a good name. I currently see only the need of $client and maybe the current user which is already loaded in Class OAuth2Controller. If somebody want to decide on server settings can this be done via loading serve from $client object.

I will create a custom implementation before suggesting a change for the hook invocation.

C-Logemann → created an issue.

Successfully tested MR !1 on D10.2 / php 8.2 and array_filter() error is gone. So +1 for RTBC.

Increased priority because the editor config cannot be saved with this bug.

There suggested changes in this request where errors are (re?)introduced like moving from "$this->t()" to "t()". And arrays with multiple lines are reduced to one line which make it less readable etc. This shows that a "phpcs" Issue isn't always as simple, So there is a big +1 from me for the status "needs work".

Moved already existing D8+ documentation link to first place.

Changed "Drupal 8/9" to "Drupal 8+" to reflect D10+

> But in logs I see Retrieve route public://boost/.html can not be done.

This is already fixed and available in dev release.

@paulrad Many thanks for fixing this issue.

C-Logemann → made their first commit to this issue’s fork.

@Orkut Murat Yılmaz Please open an extra issue for this and please specify in which way you like to integrate. For me that has currently not a high priority especially because so many basic feature are still not stable.

Because the core logic to display blocks is based on regex this will not be the suggested method in future. Maybe there will be an API/UI change to get this working.
But there will be an additional method for excluding/including logic I currently also develop for the firewall module → . A partial paths would be possible ('/some/path/*'). But I also have ideas to check paths like "/dynamic/*/something" faster as core is currently doing with regex.

For everybody who already want's the small change +core_version_requirement: ^9 || ^10 || ^11 to info file. Please first help to get this module stable on D10.

@sarwan_verma I believe "Project Update Bot" will not answer but made a more important improvement on duplacte issue: 📌 Automated Drupal 11 compatibility fixes for boost Needs review

C-Logemann → created an issue.

I just successfully tested patch #117 with core 10.2 and it fixed a view with entity references when using a filter on multiple value field.

Improved description part and moved "caution" section nearer to the top

Added tag "Drupal 10"

Changed some Core mentions to 8+ (because it's still in D10) / improved and extended contrib module example list.

I think I also created a goof example of a situation where a code change can bring an problematic error. This shows that this module needs tests especially for the allow deny logic. But the development of this module was just started by one single developer. That means there is also a need of peer review and already asked for this in the security discussion channel on community slack.

C-Logemann → changed the visibility of the branch 1.0.x to hidden.

When testing this part I found some other logic problems on parameter allow check which is hard to explain. But it's easier for me to fix both things at once.

Code is fixed and will be committed first to company gitlab and will soon be merged here.