feat: add permissions for public link shares#2483
Conversation
07381c4 to
9754375
Compare
35509c9 to
7a3163c
Compare
|
|
||
| if ($userId === '') { | ||
| return true; | ||
| return $this->isCli || $this->isPublicContext; |
There was a problem hiding this comment.
i wonder if this breaks existing shares 🤔
There was a problem hiding this comment.
It should not break existing shares, anything going through PublicRowOCSController, so also the existing getRows() will have isPublicContext set to true. I just wanted to be more explicit and safe here, in what cases a empty userId should be allowed.
Was there any other case than CLI and isPublicContext when a empty userId should bypass the permission check? We can of course also revert this line and always return true here again, just to be sure.
I think we should think about a way to refactor this later and maybe use something else than an empty userId to bypass the permission checks.
7a3163c to
c826307
Compare
There was a problem hiding this comment.
When a public share has delete permissions, the guest is able to delete rows from other tables 🙀 😱
To see, create a public share, open in an incognito/private tab and run in the console:
const TOKEN = location.pathname.match(/\/s\/([A-Za-z0-9]{16})\/?$/)?.[1] || 'TOKEN_HERE'
const ROW_ID = 16 // row from some other table
const res = await fetch(
`${location.origin}/ocs/v2.php/apps/tables/api/2/public/${TOKEN}/rows/${ROW_ID}?format=json`,
{
method: 'DELETE',
headers: {
'OCS-APIRequest': 'true',
...(window.OC?.requestToken ? { requesttoken: OC.requestToken } : {}),
},
},
)
4936a9a to
94530a7
Compare
94530a7 to
6374a90
Compare
|
Hi @enjeck and thank you for catching the delete issue, I updated the PR. For now to keep it consistent with the |
4677a1d to
1ef5c23
Compare
Signed-off-by: Benjamin Frueh <benjamin.frueh@gmail.com>
1ef5c23 to
c61cf0c
Compare
Adds view, create, update and delete permissions for public table shares.
public-<token>for thecreated_byandlast_edit_byvalue.Closes #2389