diff --git a/src/backend/controllers/share/ShareController.http.test.ts b/src/backend/controllers/share/ShareController.http.test.ts index 94b3c6be1..45f9ceba2 100644 --- a/src/backend/controllers/share/ShareController.http.test.ts +++ b/src/backend/controllers/share/ShareController.http.test.ts @@ -109,6 +109,33 @@ describe('share endpoints over HTTP', () => { } }); + it('says whether a share created access or the recipient already had it', async () => { + const owner = env.users.user; + const recipient = env.users.other; + const file = await makeFile(owner); + const share = (mode: string) => + post('/share', owner.token, { + recipients: [recipient.username], + items: [{ uid: file.uid }], + mode, + }).then((r) => r.json() as Promise<{ + results: Array<{ is_new?: boolean }>; + }>); + + expect((await share('read')).results[0].is_new).toBe(true); + // Without this the dialog cannot tell a repeat from a first share. + expect((await share('read')).results[0].is_new).toBe(false); + expect((await share('write')).results[0].is_new).toBe(false); + + // A listing describes standing access, so it says nothing about it. + const listed = await get('/share/shares', owner.token, { + uid: file.uid, + }).then((r) => r.json() as Promise<{ + items: Array>; + }>); + expect(listed.items[0]).not.toHaveProperty('is_new'); + }); + it('shares an item, lists it for the recipient, then revokes it', async () => { const owner = env.users.user; const recipient = env.users.other; diff --git a/src/backend/controllers/share/clientShare.ts b/src/backend/controllers/share/clientShare.ts index e7eb9fe58..8f0e813df 100644 --- a/src/backend/controllers/share/clientShare.ts +++ b/src/backend/controllers/share/clientShare.ts @@ -57,6 +57,8 @@ export async function toClientShare( ...(share.pending ? { pending: true, recipient_email: share.recipientEmail } : {}), + // Set on a share call only, so a listing stays silent about it. + ...(share.isNew === undefined ? {} : { is_new: share.isNew }), uid_entry: share.entryUid, is_dir: share.isDir, issuer: share.issuer.username, diff --git a/src/backend/services/share/ShareService.test.ts b/src/backend/services/share/ShareService.test.ts index 759278c24..5a6089f01 100644 --- a/src/backend/services/share/ShareService.test.ts +++ b/src/backend/services/share/ShareService.test.ts @@ -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 () => { diff --git a/src/backend/services/share/ShareService.ts b/src/backend/services/share/ShareService.ts index 9e6ce17da..9a3bf1cc9 100644 --- a/src/backend/services/share/ShareService.ts +++ b/src/backend/services/share/ShareService.ts @@ -94,11 +94,9 @@ export interface ResolvedShare { issuedByApp?: string | null; modified: number; size: number | null; - /** - * Set by `share()` only, and never sent to a client: who to notify, and - * whether this call created reach that didn't exist before. - */ + /** Set by `share()` only: who to notify. Never sent to a client. */ holderId?: number; + /** Whether this call created reach that didn't exist before. */ isNew?: boolean; /** * An invite to an address with no confirmed account. No grant exists yet — @@ -1720,6 +1718,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), diff --git a/src/docs/src/FS/share.md b/src/docs/src/FS/share.md index 2ba1479b0..69a807bc6 100644 --- a/src/docs/src/FS/share.md +++ b/src/docs/src/FS/share.md @@ -69,6 +69,7 @@ A `Promise` that resolves to an array of share objects, one per recipient/item p - `recipientEmail` (String) - Address a pending share was sent to. Only set when `pending`. - `modified` (Number) - Last-modified time of the item, in unix seconds. - `size` (Number) - Size of the item in bytes; `null` for a directory. +- `isNew` (Boolean) - Whether this call created access that did not exist before. `false` means the recipient already had it, possibly at a different mode — sharing again is not an error, so this is how you tell the two apart. Only `share()` reports it; a listing leaves it undefined. Sharing the same item with the same person again **replaces** their access rather than adding a second share, so raising someone from `read` to `write` is just another call. diff --git a/src/gui/src/UI/Dashboard/TabFiles.js b/src/gui/src/UI/Dashboard/TabFiles.js index b2a2c5fa4..01ba9a7b3 100644 --- a/src/gui/src/UI/Dashboard/TabFiles.js +++ b/src/gui/src/UI/Dashboard/TabFiles.js @@ -2579,6 +2579,7 @@ const TabFiles = { row.setAttribute("data-is_dir", file.is_dir ? "1" : "0"); row.setAttribute("data-is_trash", file.is_trash ? "1" : "0"); row.setAttribute("data-shared_with_me", file.shared_with_me ? "1" : "0"); + row.setAttribute("data-is_shared", file.is_shared === true ? "1" : "0"); row.setAttribute("data-share_mode", file.share_mode ?? ''); row.setAttribute("data-shared_by", file.shared_by ?? ''); row.setAttribute("data-has_website", file.has_website ? "1" : "0"); @@ -2604,6 +2605,10 @@ const TabFiles = {
${icon} +
-
diff --git a/src/gui/src/UI/Dashboard/UIShareModal.js b/src/gui/src/UI/Dashboard/UIShareModal.js index 64f6ee90c..9a0b6ae92 100644 --- a/src/gui/src/UI/Dashboard/UIShareModal.js +++ b/src/gui/src/UI/Dashboard/UIShareModal.js @@ -19,12 +19,17 @@ import path from '../../lib/path.js'; import item_icon from '../../helpers/itemIcon.js'; -import { owner_of_path } from '../../helpers/pathOwner.js'; +import { is_owned_by_me, owner_of_path } from '../../helpers/pathOwner.js'; import { invalidate_shared_roots } from '../../helpers/sharedAccess.js'; import { icons } from '../../helpers/actionIcons.js'; import { mode_label, options_for } from '../../helpers/shareModes.js'; import { isTouchPrimaryDevice } from './ContextMenu/ContextMenu.js'; import { avatarHue, avatarInitial } from './shareAvatar.js'; +import { + has_direct_share, + mark_item_shared, +} from '../../helpers/sharedBadge.js'; +import { share_outcome } from '../../helpers/shareOutcome.js'; import { aggregateOwners, aggregateShares, missingPathsFor } from './shareAggregate.js'; const { html_encode } = window; @@ -35,6 +40,14 @@ const chevronIcon = ` 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: () => {} }; @@ -155,7 +170,7 @@ export default function UIShareModal ({ items, path: item_path, name, owner, fse
- +
`; @@ -111,7 +123,11 @@ async function UIWindowShare (options) { $success.html(message).show(); }; + /** The access list as last drawn, which is what a share call changes. */ + let shown_shares = []; + const render = (shares) => { + shown_shares = Array.isArray(shares) ? shares : []; let rows = ''; // The owner's access comes from owning the item, so it can't be revoked rows += '
'; @@ -142,7 +158,7 @@ async function UIWindowShare (options) { } rows += ''; } @@ -150,6 +166,8 @@ async function UIWindowShare (options) { rows += ``; } $list.html(rows); + // Every share, mode change and revoke lands here. + mark_item_shared(item_path, has_direct_share(shares)); }; const refresh = async () => { @@ -173,13 +191,12 @@ async function UIWindowShare (options) { }); $(el_window).find('.share-recipient').val(''); $error.hide(); - // "Shared with" would claim access an invite does not grant. // `i18n()` encodes its replacements; encoding first would show the // entities to anyone whose address or username contains one. show_success( - created.some((share) => share.pending) - ? i18n('share_invited', { recipient }) - : i18n('share_shared_with', { recipient }), + i18n(SHARE_MESSAGE[share_outcome(created, shown_shares)], { + recipient, + }), ); invalidate_shared_roots(); await refresh(); diff --git a/src/gui/src/css/dashboard.css b/src/gui/src/css/dashboard.css index 9ab7c7b9e..fc113a46c 100644 --- a/src/gui/src/css/dashboard.css +++ b/src/gui/src/css/dashboard.css @@ -4064,6 +4064,15 @@ body.myapps-reordering .myapps-tile { border-radius: 2px; } +/* Smaller than the desktop's, to match a 24px row icon. */ +.dashboard-section-files .files-tab .files .row .item-shared-marker { + width: 7px; + height: 7px; + right: 2px; + bottom: 2px; + box-shadow: 0 0 0 1px white; +} + .dashboard-section-files .files-tab .files.files-list-view .row .item-icon img { width: 18px; height: 18px; diff --git a/src/gui/src/css/style.css b/src/gui/src/css/style.css index 8ba76589b..7bf1d266f 100644 --- a/src/gui/src/css/style.css +++ b/src/gui/src/css/style.css @@ -533,6 +533,8 @@ span.header-sort-icon img { } .item-icon { + /* Anchors the shared marker to the icon's corner. */ + position: relative; display: block; margin: 0 auto; padding: 5px; @@ -674,6 +676,22 @@ span.header-sort-icon img { cursor: pointer; } +/* Shared, owner-side: a dot on the icon's lower-right. A glyph is unreadable + at the size a row icon allows, and colour is the signal that survives it. */ +.item-shared-marker { + position: absolute; + /* Inset onto the glyph. Sitting on the icon box's corner reads as clipped, + because the box is 5px wider than the artwork on every side. */ + right: 7px; + bottom: 7px; + width: 9px; + height: 9px; + border-radius: 50%; + background: #3b82f6; + box-shadow: 0 0 0 1.5px white; + pointer-events: all; +} + .item-name, .item-name-editor, .item-name-shadow { font-size: 12px; color: white; diff --git a/src/gui/src/helpers/applyItemAddedToContainers.js b/src/gui/src/helpers/applyItemAddedToContainers.js index da21e2a39..52695fd07 100644 --- a/src/gui/src/helpers/applyItemAddedToContainers.js +++ b/src/gui/src/helpers/applyItemAddedToContainers.js @@ -70,6 +70,7 @@ const apply_item_added_to_containers = async function (item) { is_shortcut: item.is_shortcut, shortcut_to: item.shortcut_to, shortcut_to_path: item.shortcut_to_path, + is_shared: item.is_shared, }); } diff --git a/src/gui/src/helpers/refreshItemContainer.js b/src/gui/src/helpers/refreshItemContainer.js index c68772174..668dcbabe 100644 --- a/src/gui/src/helpers/refreshItemContainer.js +++ b/src/gui/src/helpers/refreshItemContainer.js @@ -254,6 +254,7 @@ const refresh_item_container = function (el_item_container, options) { shared_with_me: fsentry.shared_with_me, share_mode: fsentry.share_mode, shared_by: fsentry.shared_by, + is_shared: fsentry.is_shared, owner: fsentry.owner?.username ?? fsentry.owner, }); } diff --git a/src/gui/src/helpers/shareModes.js b/src/gui/src/helpers/shareModes.js index 8415da8df..1c4d7f1f8 100644 --- a/src/gui/src/helpers/shareModes.js +++ b/src/gui/src/helpers/shareModes.js @@ -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) => ``, diff --git a/src/gui/src/helpers/shareModes.test.js b/src/gui/src/helpers/shareModes.test.js index d3ff19465..97c705213 100644 --- a/src/gui/src/helpers/shareModes.test.js +++ b/src/gui/src/helpers/shareModes.test.js @@ -57,6 +57,19 @@ describe('options_for', () => { expect(html).not.toContain('