kirby
kirby copied to clipboard
`$file->changeSort()` doesn't have permission setting
Describe the bug
While $page->changeNum()
requires authentication, $file->changeSort()
doesn't and there also doesn't seem to be a permission setting for this. Is there are reason why?
To Reproduce Steps to reproduce the behavior:
- In the album template, add `$page->file()->changeSort(5);
- Open the
animal
page or another sibling in a browser. - See how there is no error message and the file is updated.
Expected behavior I'd argue that there should also be permission to prevent sorting on a per user basis and required authentication from the front-end.
Kirby 3.2.3-rc1
$page->changeNum(5)
in the template doesn't require auth for me either... 🤔
Oh, ok, then I mixed that up, but nevertheless, the question is then why both actions do not require authentication when called from a template/script.
There seems to be a sort
permission for pages (mentioned here: https://getkirby.com/docs/guide/users/permissions#role-based-permissions-in-user-blueprints) but at least according to the docs it doesn't have an equivalent in the page blueprint options or this option is just missing from the docs and should probably be called changeNum
anyway.
I was just wondering if this is intended and if so, why.
Yes, it's a bit strange. I'd expect all methods to check for permissions.
Hm, changeNum and changeSort should indeed be protected by permissions. We need to check this.
I was thinking about this issue. A great and simple solution came to my mind 💡
In the Kirby system, the permissions are managed by templates
, not by page or file directly. So instead of looking at all the pages, if we check the templates in the collection by caching, the process is completed very quickly. So even if there are 1000 pages, if all of them belong to a single template, the process will be completed as a result of one check.
As I was dealing with this again, I realized that changeNum
and changeStatus
are linked. If user have the changeStatus
permission and haven't changeNum
permission, when user try to update the page status as a listed
, it will be updated as a listed
, but the page num
cannot be set 🤔
You are right – changeStatus = true
implies (or should imply) changeNum = true
as well. A user cannot really have the changeStatus
permission without changeNum
.
I think the tricky part is that changeStatus = true
needs to imply changeNum = true
as well. But not the other way around. A user might have permission to change the order of pages but not to publish one.
And regarding @texnixe's initial question: I think there is no equivalent for file because changing the sort order for a file means just updating its content file. So it is tied to the update permission. But could be a question whether three should be an additional permission to prevent file sorting even if a user can update the content file.
I think we need a decision here if/how to proceed.
I think the "if" is pretty clear: Having more fine-grained permissions here is pretty valuable.
We are missing the following:
- [ ]
pages.changeNum
- [ ]
files.changeNum
- [ ]
files.changeStatus
changeStatus
should imply changeNum
as discussed. Otherwise the UX just won't work.
Should they be tief to files.update
as well? Or do we treat it as if this isn't really an update of the content file?
Do you mean because the sorting number is stored in the content file for files? To be honest I wouldn't tie the permissions together because of this, otherwise it will be confusing compared to pages.update
(that wouldn't be tied to pages.changeNum
just because the sorting number isn't stored in the content file).
Also I think that devs should be able to control the permissions independently as much as possible. For changeStatus
and changeNum
it isn't possible for the mentioned UX reasons, but besides that it is (IMO).
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
I think we should pick this up again after the blueprint refactoring.
This issue has been automatically marked as stale because it has not had recent activity. This is for us to prioritize issues that are still relevant to our community. It will be closed if no further activity occurs within the next 14 days. If this issue is still relevant to you, please leave a comment.