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
Change view_ permissions to edit_ permissions for elements in uberbar search #13095
Conversation
Makes sense! Looking good from my seat, so 👍 |
Was thinking if the permission logic should not also apply for resources, currently the code looks like if (!empty($this->query)) {
if (strpos($this->query, ':') === 0) {
// upcoming "launch actions"
//$this->searchActions();
} else {
// Search elements & resources
$this->searchResources();
if ($this->modx->hasPermission('edit_chunk')) {
$this->searchChunks();
}
if ($this->modx->hasPermission('edit_template')) {
$this->searchTemplates();
}
if ($this->modx->hasPermission('edit_tv')) {
$this->searchTVs();
}
if ($this->modx->hasPermission('edit_snippet')) {
$this->searchSnippets();
}
if ($this->modx->hasPermission('edit_plugin')) {
$this->searchPlugins();
}
if ($this->modx->hasPermission('edit_user')) {
$this->searchUsers();
}
}
} but why should it not look like if (!empty($this->query)) {
if (strpos($this->query, ':') === 0) {
// upcoming "launch actions"
//$this->searchActions();
} else {
// Search elements & resources
if ($this->modx->hasPermission('edit_document')) {
$this->searchResources();
}
if ($this->modx->hasPermission('edit_chunk')) {
$this->searchChunks();
}
if ($this->modx->hasPermission('edit_template')) {
$this->searchTemplates();
}
if ($this->modx->hasPermission('edit_tv')) {
$this->searchTVs();
}
if ($this->modx->hasPermission('edit_snippet')) {
$this->searchSnippets();
}
if ($this->modx->hasPermission('edit_plugin')) {
$this->searchPlugins();
}
if ($this->modx->hasPermission('edit_user')) {
$this->searchUsers();
}
}
} any idea why that permission check was left out there? putting it in didn't have any negative effect if the correct permissions are set, but otherwise nothing is shown, as I'd expect |
Not sure if there was a purpose of not checking permissions for resources (probably a left over from the initial PoC). All in all, makes sense too (again, from my seat ;p). |
+1 – stumbled upon it many times. With this fix the search-bar would make much more sense for editors. |
Updated PR with the discussed permission for resources |
What does it do?
Actually quite a simple change, all it does is changing the view_ permissions in the search processor to edit_ permissions.
Why is it needed?
If we assume you have some access policies in place where users cannot and should not be confronted with elements nor edit them, so they don't have the permissions to edit, but to view they have, as otherwise they wouldn't be able to fill content into templates, or drag chunks into the content field etc., but for these things, no edit permission is allowed.
So the problem appears when searching with the uberbar and then elements with the search sting in their name pop up as results and the user is tempted to click on them sometimes (at least once...), but what they get then is a nasty, ugly looking "Access denied"-white-error-page-that-looks-broken. And consequentially they assume the system is broken...and I get unnecessary calls over and over (nobody else has that??) =)
The (IMHO) simple solution is to adjust the needed permissions to "edit_" as the search-result-links only function is to EDIT the result right? Hopefully I'm not overlooking something critical here...please enlighten me!
For the more visual types:
Related issue(s)/PR(s)
none that I know of, but will update if I find one!