processwire / processwire-issues

ProcessWire issue reports.
45 stars 2 forks source link

Page Autocomplete not working when check_access is required #550

Closed adrianbj closed 1 year ago

adrianbj commented 6 years ago

Short description of the issue

If a Page reference field is set to the Page Autocomplete inputfield type and returns users ("template=user, check_access=0") in the selector, it returns nothing for non-superusers - even those who have the user-admin permission. If I change the inputfield to "Select" or "ASMSelect", then the results are returned as expected

Expected behavior

Results should be returned.

Actual behavior

No results are returned.

Steps to reproduce the issue

  1. Setup a page reference field that has the following selector: "template=user, check_access=0" and the Page Autocomplete inputfield type
  2. Add this field to the user template
  3. Try to select a user from this field when logged in as a non-superuser
ryancramerdesign commented 6 years ago

The selector for an autocomplete passes through user input (ajax), so "include=all" and "check_access=0" are filtered out for security by ProcessPageSearch, unless the user is a superuser. I'm not really sure of a good way around that. You might need to use a hook to capture the find() and add the "check_access=0" to it. I will marinate on it a bit to see if I can think of any better options that would be safe.

BitPoet commented 6 years ago

How about another execute method for ProcessPageSearch that takes the current field name, the page and the search term, instantiates the field in question (if editable for the page given, otherwise return an empty page array), appends the necessary selector work to $inputfield->findPagesSelector / $inputfield->findPagesSelect according to the inputfield's $operator and $searchFields settings and finally invokes $inputfield->getSelectablePages to retrieve the matching pages? This way, all the permission checks should be applied that would be there if this was regular pages select.

adrianbj commented 6 years ago

Hey @ryancramerdesign - any thoughts on @BitPoet 's idea? I need something on this very shortly and wondering if I need to look into the hook option or some other alternative?

Thanks!

adrianbj commented 6 years ago

Just so you know I went with hooking into Pages::find and have a solution that works, but I do think that leaving this issue open makes sense because I am sure I won't be the last person who needs a way to select users and have it work for non-superusers. I think that page autocomplete is the only viable option here when you have a substantial number of users.

BernhardBaumrock commented 5 years ago

+1 Got crazy today with this issue, thx adrian! I fixed it with a Hook provided by @adrianbj

$wire->addHookBefore('Pages::find', function(HookEvent $event) {
  $selector = $event->arguments(0);
  if(is_string($selector) && strpos($selector, 'template=user') !== false) {
    if($this->user->hasRole('physio')) $selector .= ', check_access=0';
  }
  $event->arguments(0, $selector);
});
adrianbj commented 5 years ago

Yeah, this one cost me a few hours also :)

teppokoivula commented 4 years ago

Just got bit by this as well. Ended up implementing a similar hook as the one Bernhard suggested above. Not sure if there's a better way to handle this, but as a quick fix PageAutocomplete should at least clearly state what gets removed and why, and perhaps suggest how to circumvent this issue.

Toutouwai commented 4 years ago

It would be great if InputfieldPageAutocomplete always included...

  1. the field name
  2. the ID of the page being edited

...in the data passed to ProcessPageSearch. That way it would be possible to modify the selector returned by ProcessPageSearch::findReady so it conforms to the rules for selectable pages for that field on that page.

And maybe make InputfieldPageAutocomplete::getAjaxUrl hookable so we can pass our own GET variables through to ProcessPageSearch?

PWaddict commented 3 years ago

I would like to use Page Auto Complete to get users as SUPERUSER but I'm not getting anything. If I changed it to "Select" it works fine. I tried the above hook from @BernhardBaumrock but it doesn't seem to work.

My selector string is: template=user, roles=login-register, sort=name

@teppokoivula can you please share your solution.

thetuningspoon commented 3 years ago

Why is check_access=0 required for non-superusers who have view access to the user pages? It seems like giving view access to the user should resolve this issue, but it doesn't.

duffnering commented 2 years ago

This issue is still present. See https://processwire.com/talk/topic/20829-cannot-select-users-in-page-field/#comment-222270 for more information.

Notanotherdotcom commented 1 year ago

+1 for a workaround for this @ryancramerdesign as I've also lost time to it. Nothing in the field's "custom PHP code" suggested markup suggests this wouldn't work but as others have said it doesn't. Perhaps until a solution is implemented there could be a note pointing here even just to save us some head-scratching?

matjazpotocnik commented 1 year ago

Just crossposting a workaround from Robin, if anyone need it:

$wire->addHookAfter('ProcessPageSearch::findReady', function(HookEvent $event) {
    $selector = $event->return;
    if($event->wire()->user->isSuperuser()) return;
    // If the selector looks like it comes from a particular autocomplete Page Reference field
    // (I wish this method provided a better way of working out where the selector is coming from)
    if(strpos($selector, 'parent_id=1329, templates_id=62,') === 0) {
        $selector .= ", include=all";
        $event->return = $selector;
    }
});
PWaddict commented 1 year ago

I have a page reference field on the user template and I would like to use the "Page Auto Complete". My selector string is "template=user, roles=login-register, sort=name". As SUPERUSER when I try to type any user on that field it can't find anything. If I change the input field type to "Select" it works fine.

matjazpotocnik commented 1 year ago

Try adding include=all to your selector or parent.id=29, include=all to get all users

PWaddict commented 1 year ago

Try adding include=all to your selector or parent.id=29, include=all to get all users

It doesn't work. I'm using the latest PW master version 3.0.210.

adrianbj commented 1 year ago

@PWaddict - this is how I handle this:

$this->wire()->addHookAfter('ProcessPageSearch::findReady', function(HookEvent $event) {
    $selector = $event->arguments(0);
    if(strpos($selector, ) { // whatever you need here
        $selector .= ', check_access=0';
    }
    $event->return = $selector;
});

The key thing is to add back in the check_access=0 which gets stripped out from the AJAX request.

PWaddict commented 1 year ago

@adrianbj it doesn't work. Have you tested it? It works for you as superuser? Which PW version you're using?

adrianbj commented 1 year ago

Yep, it works for my sites. PW version shouldn't matter - been using it for several years.

PWaddict commented 1 year ago

@adrianbj where did you placed the hook? On site/ready.php?

adrianbj commented 1 year ago

Yep, in ready.php or templates/admin.php

Make sure you adjust line#3 as needed for your use case.

PWaddict commented 1 year ago

Here is what I'm using on the hook and it doesn't work.

$this->wire()->addHookAfter('ProcessPageSearch::findReady', function(HookEvent $event) {
    $selector = $event->arguments(0);
    if(strpos($selector, 'template=user, roles=login-register, sort=name')) {
        $selector .= ', check_access=0';
    }
    $event->return = $selector;
});

The selector "template=user, roles=login-register, sort=name" is the same that I'm using on the page referenced field. What I'm I missing?

adrianbj commented 1 year ago

Have you checked that $selector actually contains template=user, roles=login-register, sort=name in that order?

If so and the final $result of $selector where you return it includes the check_access=0, then I'm sorry, I don't know.

PWaddict commented 1 year ago

Have you checked that $selector actually contains template=user, roles=login-register, sort=name in that order?

How can I check that?

adrianbj commented 1 year ago

Do a bd() call :)

PWaddict commented 1 year ago

Do a bd() call :)

It's been years since I used Tracy Debugger and I think I never used that bd() call. I will search for a video or text tutorial.

PWaddict commented 1 year ago

Ok, so on the hook I did this bd() call: bd($event->return = $selector); then when I typed "gmail" on the page referenced field I got the following results on the Dumps (ajax): 'template=user, roles=login-register, sort=name, status<2048, title%=gmail, limit=50'

So the hook doesn't include the "check_access=0" :(

matjazpotocnik commented 1 year ago

My pageref field selector string is: parent.id=1756|29, include=all, limit=100 (29 is $config->usersPageID, and 1756 is my custom user branch/parent id). You don't need check_access=0, as this is only stripped if you are not superuser.

image

It's working without a hook.

matjazpotocnik commented 1 year ago

Are you searching for email field? You have to specify what fields to search for, that's on the input tab, click on Settings sepcific to Page Auto Complete.

image

PWaddict commented 1 year ago

I'm searching for the name field of the user, as the users pages don't have a title field. LoginRegisterPro creates user's name field from the email field. I checked the "fields to query for autocomplete" and I noticed that it had the title field only. I changed it to email field and now when I'm searching for gmail I'm getting 2 results which are correct but they're both displayed as null.

image

matjazpotocnik commented 1 year ago

Under Label Field you can select custom format where you can enter fieldnames in curly brackets what fields will be shown in the results:

image

PWaddict commented 1 year ago

THANK YOU

So, 2 things had to changed for this to properly work as superuser on user template:

adrianbj commented 1 year ago

Sorry if I was confusing things - the original issue here was about when the user isn't a superuser, hence the need for check_access=0

ryancramerdesign commented 1 year ago

@adrianbj I've pushed an update that should remove the limitations in what you can search in InputfieldPageAutocomplete.

adrianbj commented 1 year ago

Thanks @ryancramerdesign - looks good!