`$file->changeSort()` doesn't have permission setting

[texnixe]

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:

1. In the album template, add `$page->file()->changeSort(5);
2. Open the animal page or another sibling in a browser.
3. 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

[lukasbestle]

$page->changeNum(5) in the template doesn't require auth for me either... 🤔

[texnixe]

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.

[lukasbestle]

Yes, it's a bit strange. I'd expect all methods to check for permissions.

[bastianallgeier]

Hm, changeNum and changeSort should indeed be protected by permissions. We need to check this.

[afbora]

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.

[afbora]

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 🤔

[lukasbestle]

You are right – changeStatus = true implies (or should imply) changeNum = true as well. A user cannot really have the changeStatus permission without changeNum.

[distantnative]

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.

[distantnative]

I think we need a decision here if/how to proceed.

[lukasbestle]

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.

[distantnative]

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?

[lukasbestle]

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).

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[lukasbestle]

I think we should pick this up again after the blueprint refactoring.

[github-actions[bot]]

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.