Skip to content

feat: add permissions for public link shares#2483

Open
benjaminfrueh wants to merge 1 commit intomainfrom
feat/public-share-permissions
Open

feat: add permissions for public link shares#2483
benjaminfrueh wants to merge 1 commit intomainfrom
feat/public-share-permissions

Conversation

@benjaminfrueh
Copy link
Copy Markdown
Contributor

Adds view, create, update and delete permissions for public table shares.

  • UI changes are kept minimal for now, as the sharing sidebar should be updated to the unified sharing component in the future
  • Rows created/edited via public link shares use public-<token> for the created_by and last_edit_byvalue.
  • If a public link share has only create permission, the UI handles it as a public form with adjusted icon and text

Closes #2389

@benjaminfrueh benjaminfrueh added the enhancement New feature or request label Apr 15, 2026
@benjaminfrueh benjaminfrueh added the 3. to review Waiting for reviews label Apr 15, 2026
@github-project-automation github-project-automation Bot moved this to 🧭 Planning evaluation (don't pick) in 📝 Productivity team Apr 15, 2026
@benjaminfrueh benjaminfrueh moved this from 🧭 Planning evaluation (don't pick) to 👀 In review in 📝 Productivity team Apr 15, 2026
@benjaminfrueh benjaminfrueh force-pushed the feat/public-share-permissions branch from 07381c4 to 9754375 Compare April 15, 2026 21:37
@samin-z samin-z self-requested a review April 16, 2026 08:13
Comment thread lib/Service/PermissionsService.php
Comment thread lib/Controller/ShareController.php
Comment thread lib/Controller/ShareController.php Outdated
Comment thread lib/Controller/PublicRowOCSController.php
Comment thread src/store/data.js
@benjaminfrueh benjaminfrueh force-pushed the feat/public-share-permissions branch from 35509c9 to 7a3163c Compare April 22, 2026 08:59

if ($userId === '') {
return true;
return $this->isCli || $this->isPublicContext;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i wonder if this breaks existing shares 🤔

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@enjeck enjeck force-pushed the feat/public-share-permissions branch from 7a3163c to c826307 Compare April 23, 2026 07:12
Copy link
Copy Markdown
Contributor

@enjeck enjeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 } : {}),
    },
  },
)

@benjaminfrueh benjaminfrueh force-pushed the feat/public-share-permissions branch from 4936a9a to 94530a7 Compare April 23, 2026 19:56
@benjaminfrueh benjaminfrueh requested a review from enjeck April 23, 2026 19:58
@benjaminfrueh benjaminfrueh force-pushed the feat/public-share-permissions branch from 94530a7 to 6374a90 Compare April 23, 2026 20:18
@benjaminfrueh
Copy link
Copy Markdown
Contributor Author

benjaminfrueh commented Apr 23, 2026

Hi @enjeck and thank you for catching the delete issue, I updated the PR. For now to keep it consistent with the updateSet method, I passed the tableId to validate that the row belongs to the table.

@benjaminfrueh benjaminfrueh force-pushed the feat/public-share-permissions branch 3 times, most recently from 4677a1d to 1ef5c23 Compare April 23, 2026 20:53
Signed-off-by: Benjamin Frueh <benjamin.frueh@gmail.com>
@benjaminfrueh benjaminfrueh force-pushed the feat/public-share-permissions branch from 1ef5c23 to c61cf0c Compare April 23, 2026 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews enhancement New feature or request

Projects

Status: 👀 In review

Development

Successfully merging this pull request may close these issues.

Permissions for Public link shares

3 participants