mirror of
https://github.com/HeyPuter/puter.git
synced 2026-08-29 01:16:43 +00:00
Stop offering an access level a delegate cannot grant
A recipient given "can edit & share" could not pass that level on: the dialog offered it, the server refused it, and the refusal was a bare 403 Forbidden that reads as a bug. Handing out manage needs authority over manage, which only the owner has — the refusal is right, the dead end and the silence were not. The dropdown now withholds it from anyone who does not own the item; a row already set to it keeps it, so opening the dialog cannot downgrade the owner's own grant, and a mixed selection follows its strictest item. The server says why, and only to someone who can already share the item — a stranger still gets the ACL's own safe error, which does not admit the node exists. Verified against a running server: a delegate grants read and write as before, and manage now answers cannot_delegate_manage with a sentence naming the owner as the one who can.
This commit is contained in:
Juan Castro
@@ -806,7 +806,35 @@ describe('ShareService', () => {
|
||||
recipient: { email: third.email },
|
||||
mode: 'manage',
|
||||
}),
|
||||
).rejects.toMatchObject({ statusCode: 403 });
|
||||
).rejects.toMatchObject({
|
||||
statusCode: 403,
|
||||
legacyCode: 'cannot_delegate_manage',
|
||||
});
|
||||
|
||||
// What they can do is unchanged.
|
||||
await expect(
|
||||
share(delegate.actor, {
|
||||
uid: file.uuid,
|
||||
recipient: { email: third.email },
|
||||
mode: 'write',
|
||||
}),
|
||||
).resolves.toMatchObject({ mode: 'write' });
|
||||
});
|
||||
|
||||
it('tells a stranger nothing when they ask to grant `manage`', async () => {
|
||||
const owner = await makeUser();
|
||||
const stranger = await makeUser();
|
||||
const third = await makeUser();
|
||||
const file = await makeFile(owner.user);
|
||||
|
||||
// No access at all, so the refusal must not confirm the file exists.
|
||||
await expect(
|
||||
share(stranger.actor, {
|
||||
uid: file.uuid,
|
||||
recipient: { email: third.email },
|
||||
mode: 'manage',
|
||||
}),
|
||||
).rejects.not.toMatchObject({ legacyCode: 'cannot_delegate_manage' });
|
||||
});
|
||||
|
||||
it('leaves a delegate alone when their authority survives another issuer', async () => {
|
||||
|
||||
@@ -1720,6 +1720,22 @@ export class ShareService extends PuterService {
|
||||
// given, rather than everything its user owns.
|
||||
if (allowed && (await this.#hasOwnReach(actor, entry, mode))) return;
|
||||
|
||||
// Only for someone who can already share here, so it leaks nothing.
|
||||
if (mode === MANAGE_PERM_PREFIX) {
|
||||
const canDelegateAccess =
|
||||
await this.services.permission.canManagePermission(
|
||||
userRelatedActor(actor),
|
||||
entryPermissionForMode(entry.uuid, 'write'),
|
||||
);
|
||||
if (canDelegateAccess) {
|
||||
throw new HttpError(
|
||||
403,
|
||||
'Only the owner can grant edit & share access',
|
||||
{ legacyCode: 'cannot_delegate_manage' },
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
const safe = await this.services.acl.getSafeAclError(
|
||||
actor,
|
||||
this.#descriptorFor(entry),
|
||||
|
||||
@@ -19,7 +19,7 @@
|
||||
|
||||
import path from '../../lib/path.js';
|
||||
import item_icon from '../../helpers/item_icon.js';
|
||||
import { owner_of_path } from '../../helpers/path_owner.js';
|
||||
import { is_owned_by_me, owner_of_path } from '../../helpers/path_owner.js';
|
||||
import { invalidate_shared_roots } from '../../helpers/shared_access.js';
|
||||
import { icons } from '../../helpers/actionIcons.js';
|
||||
import { mode_label, options_for } from '../../helpers/share_modes.js';
|
||||
@@ -115,6 +115,8 @@ export default function UIShareModal ({ items, path: item_path, name, owner, fse
|
||||
}));
|
||||
const target_paths = targets.map((item) => item.path);
|
||||
const total = targets.length;
|
||||
// Strictest item decides: one borrowed item withholds it for the rest.
|
||||
const allow_manage = target_paths.every((p) => is_owned_by_me(p));
|
||||
const is_multi = total > 1;
|
||||
// Nothing to share: an empty selection is a caller's mistake, not a dialog.
|
||||
if ( total === 0 ) return { close: () => {} };
|
||||
@@ -159,7 +161,7 @@ export default function UIShareModal ({ items, path: item_path, name, owner, fse
|
||||
<div class="share-modal-add-row">
|
||||
<input type="text" class="share-modal-recipient" autocomplete="off" autocapitalize="off" spellcheck="false" enterkeyhint="send"
|
||||
placeholder="${i18n('share_add_people')}" aria-label="${i18n('share_add_people')}" />
|
||||
<select class="share-modal-mode" aria-label="${i18n('share_access_level')}">${options_for('read')}</select>
|
||||
<select class="share-modal-mode" aria-label="${i18n('share_access_level')}">${options_for('read', { allow_manage })}</select>
|
||||
</div>
|
||||
<button type="submit" class="share-modal-submit" disabled>
|
||||
<span class="share-modal-spinner" aria-hidden="true"></span>
|
||||
@@ -318,7 +320,7 @@ export default function UIShareModal ({ items, path: item_path, name, owner, fse
|
||||
// The accessible names carry the person: a list where every row
|
||||
// reads as bare "Access level" / "Remove access" leaves a screen
|
||||
// reader user unable to tell whose grant a control changes.
|
||||
row += `<select class="share-modal-row-mode" data-key="${key}" aria-label="${i18n('share_access_level_for', { recipient: group.name })}">${options_for(group.mode)}</select>`;
|
||||
row += `<select class="share-modal-row-mode" data-key="${key}" aria-label="${i18n('share_access_level_for', { recipient: group.name })}">${options_for(group.mode, { allow_manage })}</select>`;
|
||||
} else {
|
||||
const fixed_mode = group.pending ? group.pendingMode : group.inheritedMode;
|
||||
row += `<span class="share-modal-row-tag">${fixed_mode ? mode_label(fixed_mode) : i18n('share_access_mixed')}</span>`;
|
||||
|
||||
@@ -20,7 +20,7 @@
|
||||
import UIWindow from './UIWindow.js';
|
||||
import UIAlert from './UIAlert.js';
|
||||
import path from '../lib/path.js';
|
||||
import { owner_of_path } from '../helpers/path_owner.js';
|
||||
import { is_owned_by_me, owner_of_path } from '../helpers/path_owner.js';
|
||||
import { invalidate_shared_roots } from '../helpers/shared_access.js';
|
||||
import { icons } from '../helpers/actionIcons.js';
|
||||
import { mode_label, options_for } from '../helpers/share_modes.js';
|
||||
@@ -41,6 +41,8 @@ async function UIWindowShare (options) {
|
||||
const item_name = options.name ?? path.basename(item_path);
|
||||
const item_owner =
|
||||
options.owner ?? owner_of_path(item_path) ?? window.user.username;
|
||||
// A delegate passes on access, never the authority to pass it on.
|
||||
const allow_manage = is_owned_by_me(item_path);
|
||||
|
||||
let h = '';
|
||||
h += '<div class="share-dialog">';
|
||||
@@ -51,7 +53,7 @@ async function UIWindowShare (options) {
|
||||
h += '<div class="share-dialog-row">';
|
||||
h += `<input class="share-recipient" id="share-recipient" type="text" autocomplete="off" spellcheck="false"
|
||||
placeholder="${html_encode(i18n('share_add_people'))}" />`;
|
||||
h += `<select class="share-mode">${options_for('read')}</select>`;
|
||||
h += `<select class="share-mode">${options_for('read', { allow_manage })}</select>`;
|
||||
h += '</div>';
|
||||
h += `<button class="share-btn button button-primary button-block button-normal">${i18n('share')}</button>`;
|
||||
|
||||
@@ -143,7 +145,7 @@ async function UIWindowShare (options) {
|
||||
}
|
||||
rows += '<div class="share-row">';
|
||||
rows += `<span class="share-row-who">${holder}</span>`;
|
||||
rows += `<select class="share-row-mode-select" data-holder="${holder}">${options_for(share.mode)}</select>`;
|
||||
rows += `<select class="share-row-mode-select" data-holder="${holder}">${options_for(share.mode, { allow_manage })}</select>`;
|
||||
rows += `<button class="share-revoke" data-holder="${holder}" title="${html_encode(i18n('share_remove_access'))}" aria-label="${html_encode(i18n('share_remove_access'))}">${icons.trash}</button>`;
|
||||
rows += '</div>';
|
||||
}
|
||||
|
||||
@@ -48,11 +48,17 @@ export const mode_label = (mode) => {
|
||||
* unselectable placeholder, so a batch of mixed modes can't be read as one of
|
||||
* them, and picking a real mode is what levels them.
|
||||
*
|
||||
* `allow_manage: false` drops "Can edit & share", bar a row already on it.
|
||||
*
|
||||
* @param {string|null} current
|
||||
* @param {{ allow_manage?: boolean }} [options]
|
||||
* @returns {string} HTML-safe markup
|
||||
*/
|
||||
export const options_for = (current) => {
|
||||
const listed = MODES
|
||||
export const options_for = (current, { allow_manage = true } = {}) => {
|
||||
const offered = MODES.filter(
|
||||
(mode) => allow_manage || mode !== 'manage' || mode === current,
|
||||
);
|
||||
const listed = offered
|
||||
.map(
|
||||
(mode) =>
|
||||
`<option value="${html_encode(mode)}"${mode === current ? ' selected' : ''}>${mode_label(mode)}</option>`,
|
||||
|
||||
@@ -57,6 +57,19 @@ describe('options_for', () => {
|
||||
expect(html).not.toContain('<option value="read" selected>');
|
||||
});
|
||||
|
||||
it('withholds `manage` from someone who cannot grant it', () => {
|
||||
// Offering it to a delegate is a dead end the server refuses.
|
||||
const html = options_for('read', { allow_manage: false });
|
||||
expect(values(html)).toEqual(['read', 'write']);
|
||||
expect(html).not.toContain('value="manage"');
|
||||
});
|
||||
|
||||
it('still shows a row already set to `manage`, so opening the dialog does not downgrade it', () => {
|
||||
const html = options_for('manage', { allow_manage: false });
|
||||
expect(values(html)).toEqual(MODES);
|
||||
expect(html).toContain('<option value="manage" selected>');
|
||||
});
|
||||
|
||||
it('keeps an out-of-band mode instead of rounding it to read', () => {
|
||||
const html = options_for('see');
|
||||
expect(values(html)).toEqual(['see', ...MODES]);
|
||||
|
||||
Reference in New Issue
Block a user