🐛 PUT-1586 + PUT-1599: Share dialog says what it can grant, and what it did

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<Record<string, unknown>>;
+        }>);
+        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;

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,

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 () => {

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

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.
 

src/gui/src/UI/Dashboard/UIShareModal.js

@@ -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';
@@ -29,6 +29,7 @@ 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;
@@ -39,6 +40,14 @@ const chevronIcon = `<svg class="share-modal-chevron" viewBox="0 0 24 24" width=
 // How many item icons the header fans out before it stops adding to the pile.
 const MAX_STACKED_ICONS = 3;
 
+/** What each outcome of a share call is called on screen. */
+const SHARE_MESSAGE = {
+    invited: 'share_invited',
+    shared: 'share_shared_with',
+    updated: 'share_access_updated',
+    unchanged: 'share_already_shared_with',
+};
+
 // What one /share, /share/revoke or listing pass may cover, matching the
 // backend's documented cap (see doc: rate limits and quotas). Bigger
 // selections are shared in several requests, and skip the access list rather
@@ -115,6 +124,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 +170,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 +329,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>`;
@@ -533,9 +544,16 @@ export default function UIShareModal ({ items, path: item_path, name, owner, fse
             // "Shared with" would claim access an invite does not grant.
             const invited = created?.some((share) => share.pending);
             if ( ! is_multi ) {
-                show_success(invited
-                    ? i18n('share_invited', { recipient })
-                    : i18n('share_shared_with', { recipient }));
+                // One item, so the list on screen settles what changed.
+                const before = last_groups.map((group) => ({
+                    holder: group.name,
+                    mode: group.mode,
+                }));
+                show_success(
+                    i18n(SHARE_MESSAGE[share_outcome(created, before)], {
+                        recipient,
+                    }),
+                );
             } else if ( granted < total ) {
                 show_success(i18n('share_shared_with_partial', { recipient, count: granted, total }));
             } else {

src/gui/src/UI/UIWindowShare.js

@@ -20,11 +20,20 @@
 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';
 import { has_direct_share, mark_item_shared } from '../helpers/sharedBadge.js';
+import { share_outcome } from '../helpers/shareOutcome.js';
+
+/** What each outcome of a share call is called on screen. */
+const SHARE_MESSAGE = {
+    invited: 'share_invited',
+    shared: 'share_shared_with',
+    updated: 'share_access_updated',
+    unchanged: 'share_already_shared_with',
+};
 
 /**
  * Sharing dialog for one file or directory.
@@ -41,6 +50,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 +62,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>`;
 
@@ -112,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 += '<div class="share-row">';
@@ -143,7 +158,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>';
         }
@@ -176,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();

src/gui/src/helpers/shareOutcome.js

@@ -0,0 +1,47 @@
+/*
+ * Copyright (C) 2024-present Puter Technologies Inc.
+ *
+ * This file is part of Puter.
+ *
+ * Puter is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published
+ * by the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ */
+
+/**
+ * What a share call actually did, so the dialog can say so. A backend that
+ * omits `isNew` reads as `shared`, which is what these dialogs said before it.
+ *
+ * @param {Array<{ pending?: boolean, isNew?: boolean, mode?: string, holder?: string|null }>} created
+ * @param {Array<{ holder?: string|null, mode?: string, inheritedFrom?: string|null }>} [before]
+ * @returns {'invited' | 'shared' | 'updated' | 'unchanged'}
+ */
+export const share_outcome = (created, before = []) => {
+    const list = Array.isArray(created) ? created.filter(Boolean) : [];
+    if ( list.some((share) => share.pending) ) return 'invited';
+    if ( list.length === 0 || list.some((share) => share.isNew !== false) ) {
+        return 'shared';
+    }
+
+    // Matched on the resolved username rather than what was typed, so an email
+    // that belongs to a known account still finds their row.
+    const previous = (Array.isArray(before) ? before : []).find(
+        (share) =>
+            share?.holder &&
+            list.some((made) => made.holder === share.holder) &&
+            ! share.inheritedFrom,
+    );
+    if ( ! previous ) return 'unchanged';
+    return list.some((made) => made.mode !== previous.mode)
+        ? 'updated'
+        : 'unchanged';
+};

src/gui/src/helpers/shareOutcome.test.js

@@ -0,0 +1,63 @@
+import { describe, expect, it } from 'vitest';
+import { share_outcome } from './shareOutcome.js';
+
+describe('share_outcome', () => {
+    it('reports a first-time share', () => {
+        expect(share_outcome([{ holder: 'ann', mode: 'read', isNew: true }]))
+            .toBe('shared');
+    });
+
+    it('reports an invite ahead of anything else', () => {
+        expect(
+            share_outcome([
+                { recipientEmail: 'x@example.com', pending: true, isNew: true },
+            ]),
+        ).toBe('invited');
+    });
+
+    it('says nothing changed when the same access is granted twice', () => {
+        // The bug: this used to read as a fresh share.
+        expect(
+            share_outcome(
+                [{ holder: 'ann', mode: 'read', isNew: false }],
+                [{ holder: 'ann', mode: 'read' }],
+            ),
+        ).toBe('unchanged');
+    });
+
+    it('says the level changed when the mode differs', () => {
+        expect(
+            share_outcome(
+                [{ holder: 'ann', mode: 'write', isNew: false }],
+                [{ holder: 'ann', mode: 'read' }],
+            ),
+        ).toBe('updated');
+    });
+
+    it('matches on the resolved username, not what was typed', () => {
+        // The recipient was entered as an email; their row is keyed on the
+        // username the server resolved it to.
+        expect(
+            share_outcome(
+                [{ holder: 'ann', mode: 'write', isNew: false }],
+                [{ holder: 'ann', mode: 'read' }, { holder: 'bob', mode: 'read' }],
+            ),
+        ).toBe('updated');
+    });
+
+    it('ignores an inherited row, which this call cannot have changed', () => {
+        expect(
+            share_outcome(
+                [{ holder: 'ann', mode: 'read', isNew: false }],
+                [{ holder: 'ann', mode: 'write', inheritedFrom: '/bob/Docs' }],
+            ),
+        ).toBe('unchanged');
+    });
+
+    it('falls back to `shared` when the backend does not report isNew', () => {
+        // An older server, or the CDN SDK before this shipped.
+        expect(share_outcome([{ holder: 'ann', mode: 'read' }])).toBe('shared');
+        expect(share_outcome([])).toBe('shared');
+        expect(share_outcome(undefined)).toBe('shared');
+    });
+});