Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added check for ACL in Header of pages when searching #102

Merged
merged 1 commit into from Apr 11, 2017
Merged

Added check for ACL in Header of pages when searching #102

merged 1 commit into from Apr 11, 2017

Conversation

NilsEngelbach
Copy link
Contributor

The SimpleSearch Plugin returns results for pages that are protected by ACL Rules like

access:
    site.login: true

It is not enough to filter the results in the twig template for search results, since there is also an json formatted endpoint (/search.json/query:Example).

@rbukovansky
Copy link
Contributor

rbukovansky commented Mar 15, 2017

@flaviocopes Can we please get this merged ASAP and release of new version of this plugin done? Thank you very much.

@NilsEngelbach
Copy link
Contributor Author

@rbukovansky I am not really into PHP and grav plugins yet, so you might want to improve/refactor the code. I think there could be couple of improvements, especially for the more less duplicated code from login plugin. Maybe there should be a general function to do this filtering in the "core?!", if this would be possible...

@rbukovansky
Copy link
Contributor

@NilsEngelbach Well, I'm not a dev at all... I've just asked @flaviocopes to merge the changes and please to do a proper release... That's all.

@flaviocopes flaviocopes merged commit 9e0627d into getgrav:develop Apr 11, 2017
@flaviocopes
Copy link
Contributor

Looks good, thanks!

@lulis
Copy link
Contributor

lulis commented Apr 15, 2017

Hi,
This patch breaks my simplesearch usage, now i got the following error (on this line):

Identifier "user" is not defined.

I'm not using any kind of login in grav. Have to comment the call of checkForPermissions (here) as workaround.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants