Allow wildcard path conditions

I ported the patch from here for version 8.x-1.0

@CShark The patch cannot be applied.

I updated the patch to apply to the latest dev, but it still looks like there needs some work on the form validation when saving new protected paths.

In addition, the original patch will throw an undefined variable error for $where_clause, and it's not referenced anywhere else, so I'm not entirely sure what it represents.

Hi, I've dig into this issue a bit. I have applied patch #5 and worked from there on.

I have added the option for wildcard * at the end of the path. I have removed path validation for this kinds of protected paths (@todo validation). I also did not bother with coding standards and best practices because they are parts of other issues (there will be rerolling to do either way).

Might also be good idea to use path.matcher service instead of making all these gymnastics with db querrying. What do you think? Something like getting all protected paths from db and than foreaching through them

$match = \Drupal::service('path.matcher')->matchPath($current_path, $protected_path);

Hi,

I updated patch from comment #6 and added validation for the edit form too.

Great work so far. My concern with the current approach is that it doesn't support paths that extend past one level deep. For example, /foo/* protects /foo/bar, but not /foo/bar/baz.

I think @useernamee's suggestion in #6 to try the path.matcher service is on the right track. Attaching a patch that explores that approach.

I added the patch from #11 (had to manually add certain lines), and also changed code very slightly so that the path /protected-page itself doesn't find itself "protected", which will cause an infinite number of redirects.

I also added verbiage when adding a path that you can use /* to protect every page on the site, which is now true because of the above edit.

I did not have time to figure out how to make this into a pretty patch file, so apologies please-- but I am just going to attach the entire module zip file I ended up with. To maintainer, please feel free to create into a patch, and *please* apply to dev or put out a new release!

I rerolled the patch in #11 for 8.x-1.x-dev and added the tweaks from #13.

I like this approach. I think it's cleaner than the solution currently being worked on for #3005788: Allow multiple pages with same validation and one page with multiple validations.

However, that solution has the advantage of adding pages unrelated by path to the same authentication. It would be great to see that combined here, preferably in a multivalue field.

Here's a new version of the patch that I think works with the latest dev

Reviewed patch in #16, added dependency injection to it and fixed some minor CS issues.

Hi, @kbrodej I applied the patch and reviewed the code and everything is OK. I also tested the solution and it works as it should so I will set this to RTBC.
Regards, strozx.

I second the RTBC. We've been using the patch for months with no issue.

Patch #17 is missing wildcard check for private file paths. This patch fixes it.

If it helps #20 - this word for me protecting files, and pages
Thanks

Unfortunately the patch no longer applies to the lastest dev, due to a bunch of new commits. We'll have to create a new patch. I'll take a look if I get an opportunity soon.

OK here's an attempt at rerolling the patch from #20 for the latest dev version.

Hi. I was testing this and when applying patch from #23 there was a warning: 2 lines add whitespace errors.
I have just quickly fixed this, so patch applies without warnings.

Also I have tested the functionality and it seems this is working as described. I had no issues and also logs are clear of any warnings.

As these updates work, apply cleanly, and contain no substantive changes to the previous patches, I'm setting this back as RTBC.

I just manually applied this to 8.x-1.2 . Seems to work.

thanks.

Be good to see this wrapped up.

#24 patch works on 8.x-1.4

This patch doesn't apply to latest dev

Re-rolled #24 for MR!2

Cleaned this issue and added related task https://www.drupal.org/project/protected_pages/issues/3005788 (which seems more robust, but introduce too much changes at once)

I don't know the state of this patch currently (we've continued to successfully use #24 on D9.5) and I have my doubts, given the maintenance of this project, that it will make it into production. However, I've discovered a worrisome bug when using the patch.

When you have more than 20 passwords set up, the interface becomes paged, which is normal. However, any wildcard page passwords you have setup that are on the second page are NO LONGER password restricted.

Looking through the code of the patch, one of the first additions is pulling from loadAllProtectedPages:

// Check all protected pages entries for path match including wildcards.
$all_protected_pages = $protected_pages_storage->loadAllProtectedPages();
foreach ($all_protected_pages as $protected_page) {
  if ($path_matcher->matchPath($file_path, $protected_page->path)) {
    $pid = $protected_page->pid;
    break;
  }
}

Unfortunately, the loadAllProtectedPages function contains a pager/limit for 20 items. Fine for listing the pages, bad for checking all of the passwords:

public function loadAllProtectedPages() {
 $results = $this->connection->select('protected_pages', 'p')
   ->extend('Drupal\Core\Database\Query\PagerSelectExtender')
   ->fields('p')
   ->orderBy('p.pid', 'DESC')
   ->limit(20)
   ->execute()
   ->fetchAll();

A quick fix for this, and you need one if your pages are now unprotected, is to just up the limit or even drop the pager completely. I ended up duplicating the loadAllProtectedPages function, calling it listAllProtectedPages, and using it where the results are called. I left alone all the references to loadAllProtectedPages in the patch and just removed the pager from that function.

I would add this to the patch, but again, I'm not sure where it stands given the merge request, etc.

Here's a patch for the latest dev based on #24

And here's a version based on my comment in #34, it adds a new function to the patch in #35 for listing the Protected Pages vs loading them.

I made some changes based on the test failures in #35, so here's an updated patch for the latest dev based on #24. I'm not familiar with writing tests so you're on your own there.

This is an updated version of #36, based on my comment in #34.

I used patch #24 with version 1.4 of this module.
Now I updated the module to 1.6 but the patch can not be applied.

Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/2020-05-19/2890122-24.patch

an suggestions ?

it seems to work with wildcards e.g. * and version 1.6, but the description hints from patch #24 are missing.
Please enter the relative path and its corresponding password. When user opens this url, they will asked to enter password to view this page. For example, "/node/5", "/new-events" etc.
I thought, that indicates that patch did not make it to the release...
Can someone confirm wildcards are supported with V. 1.6 ?

a quick look into the module code, it seems the patch mostly made it to V1.6, only the descriptions for wildcard usage seem to be missing.
can someone confirm this is the case?

edit: seems there was still code from the patch existing in my local installation when looking into it.

@nojj, I haven't looked at V1.6, but I'd be surprised if these changes made it in unannounced. In any case, the latest patches - 38 & 39 - work with the latest dev version only.

@mxwright, thanks for your response.
in a local test with V1.6 it seems with wildcard * the protection is not applied.
regarding latest dev and patches: is the security issue fixed in V1.6 included in the latest dev?

@nojj, yes it looks like the security issue is addressed in both 1.6 and the latest dev.

so you are are using the latest dev with patch #39 in your current projects?
and is the security-fix what you describe in #34 or something else?

@nojj, here is the current state of things:

• v1.6 and 8.x-1.x-dev have the Drupal security update
• Patch in #38 updates the patch in #24 for the latest dev version
• Patch in #39 updates the patch in #38 with the fix for the issue in #34, which I think is necessary to address, and is for the latest dev version

I'm using #39 with the latest dev successfully on my projects

@mxwright thank you very much for the detailed explanation!
curious if this makes it into the stable release...

@mxwright did another test. coming from V1.4 with patch #24, I could successfully update to V1.6 (stable version) and then apply patch #39. And wildcards seem to work.
that would be nice, because DEV-versions of modules are not listed in Drupal for pending updates.

is eher an option to use the wildcards for a "section global password"
we would like to use one global password for all URL beginning with
/xxx/*
but have different passwords for individual urls with a more specific url like
/xxx/abc

at the moment if the password for /xxx/* is not accepted for urls like
/xxx/abc

if a different password is set to this urls

any ideas to solve this?

Rerolled patch 39 for protected_pages 1.7.0

Rerolled patch 39 for protected_pages 1.7.0 Now with AliasManagerInterface in the constructor

Patch in 53 appears to work for 1.8.0

also patch #53 fixes this issue for 1.8.0
https://www.drupal.org/project/protected_pages/issues/3543502

Rerolled MR again.
This has been RTBC for almost 2 years, can we get this committed please?

It's a great addition to the module and I believe it's mostly ready to be merged.

However, it would be nice to see a test that covers this new functionality before merging.

Added tests and merged (has to come back to add credits... forgot on the first pass).
Thank you all!

Thanks for incorporating this feature!

Unfortunately, the issue I reported in #34 still persists. Not sure why, since it was fixed in the patches. This particular change was not committed.

The issue is that if you have more than 20 entries in your protected pages list, the entries that spill over to the second page (21+) are no longer protected. The protection rules only apply to the items on the first page. This is a pretty big problem!

If I recall correctly, the solution is to just remove the limit from this code from /src/ProtectedPagesStorage.php

public function loadAllProtectedPages() {
 $results = $this->connection->select('protected_pages', 'p')
   ->extend('Drupal\Core\Database\Query\PagerSelectExtender')
   ->fields('p')
   ->orderBy('p.pid', 'DESC')
   ->limit(20)
   ->execute()
   ->fetchAll();

I may make a new patch just for this, or open a new issue, but in the meantime I'd recommend downgrading to 1.8 and patching with #53

I haven't opened a new issue yet for what I wrote in #65, but I made a new patch that should add back the missing pieces of the patch in #53 that keeps this wildcard function working when you have more than 20 entries.

#3557159: Wildcard path conditions don't apply after 20 entries