fix: close two gaps the review found in the invite-address fix

- A full-access token was denied the address while `shared-by-me` still
  handed it the same rows, so the clause bought no privacy and cost an
  API client the address `unshare()` takes. `isAccountContext` is the
  boundary the rest of the codebase already uses for 'acting as the
  account': plain session or full-access token, never a scoped one.
- The Dashboard share modal dropped a withheld invite entirely, since
  its aggregate keys a pending row on the address — so a delegate saw no
  sign of an outstanding invite and accessCount under-reported who could
  reach the item. It is now kept, keyed on the share uid, labelled, and
  without the controls that would need a recipient to address.
This commit is contained in:
Juan Castro
2026-09-17 13:21:13 -04:00
parent 3e3d02bafe
commit 461cb7d0a9
5 changed files with 68 additions and 5 deletions
@@ -26,6 +26,7 @@ import {
type PuterTestEnv,
} from '../../testUtil.js';
import { ShareController } from './ShareController.js';
import { FULL_API_ACCESS } from '../../services/permission/consts.js';
/**
* Route-level coverage for the sharing endpoints. The service unit tests drive
@@ -1169,6 +1170,30 @@ describe('share endpoints over HTTP', () => {
expect(JSON.parse(body).shares).toEqual([]);
});
// Withholding here would buy nothing: `shared-by-me` hands the same
// addresses to the same credential.
it('tells a full-access token what it could read from shared-by-me anyway', async () => {
const owner = env.users.user;
const { file, email } = await invitedFile(owner);
const user = await env.server.stores.user.getByUsername(
owner.username,
);
const actor = makeActor({ user: user! });
const pat = await runWithContext({ actor }, () =>
env.server.services.auth.createAccessToken(actor, [
[FULL_API_ACCESS],
]),
);
const outbound = await get('/share/shared-by-me', pat, {});
expect(outbound.status).toBe(200);
expect(await outbound.text()).toContain(email);
const listed = await get('/share/shares', pat, { uid: file.uid });
expect(listed.status).toBe(200);
expect(await listed.text()).toContain(email);
});
it('answers an app that was handed the item to manage', async () => {
const owner = env.users.user;
const { file, email } = await invitedFile(owner);
+7 -2
View File
@@ -19,7 +19,12 @@
import { contentType as contentTypeFromMime } from 'mime-types';
import { posix as pathPosix } from 'node:path';
import { makeActor, userRelatedActor, type Actor } from '../../core/actor';
import {
isAccountContext,
makeActor,
userRelatedActor,
type Actor,
} from '../../core/actor';
import { HttpError, isHttpError } from '../../core/http/HttpError.js';
import { runWithConcurrencyLimitSettled } from '../../util/concurrency.js';
import { isUniqueViolation } from '../../util/dbError.js';
@@ -3270,7 +3275,7 @@ export class ShareService extends PuterService {
entry: FSEntry,
row: OutboundShareRow,
): boolean {
if (actor.app || actor.accessToken) return false;
if (!isAccountContext(actor)) return false;
const userId = actor.user?.id;
if (typeof userId !== 'number') return false;
return (
+4 -2
View File
@@ -324,8 +324,10 @@ export default function UIShareModal ({ items, path: item_path, name, owner, fse
: '';
// Only direct grants live on the items themselves; an inherited one
// belongs to the ancestor folder and has to be changed there.
const can_change = group.directPaths.length > 0;
const can_revoke = can_change || group.pendingPaths.length > 0;
// Every action addresses a person by name; a withheld invite has none.
const can_change = group.directPaths.length > 0 && ! group.anonymous;
const can_revoke = ! group.anonymous
&& (can_change || group.pendingPaths.length > 0);
// Extending someone needs a mode to extend; a person whose grants
// disagree levels them with the select first.
const missing = missingPathsFor(target_paths, group).length;
+12 -1
View File
@@ -97,6 +97,15 @@ const identify = (share, bucket) => {
const name = bucket === 'pending'
? (share.recipientEmail ?? '')
: (share.holder ?? '');
// Withheld address: still reaches someone, and has no name to group on.
if ( name === '' && bucket === 'pending' ) {
return {
key: `invite:${share.uid}`,
name: i18n('share_invited_someone'),
teamUid: null,
anonymous: true,
};
}
if ( name === '' ) return null;
return {
key: `${bucket === 'pending' ? 'invite' : 'user'}:${name}`,
@@ -129,7 +138,7 @@ export const aggregateShares = (paths, sharesByPath) => {
const bucket = bucket_of(share);
const identity = identify(share, bucket);
if ( ! identity ) continue;
const { key, name, teamUid } = identity;
const { key, name, teamUid, anonymous } = identity;
if ( counted.has(`${key}|${bucket}`) ) continue;
counted.add(`${key}|${bucket}`);
@@ -139,6 +148,7 @@ export const aggregateShares = (paths, sharesByPath) => {
key,
name,
teamUid,
anonymous: anonymous === true,
pending: bucket === 'pending',
directPaths: [],
pendingPaths: [],
@@ -175,6 +185,7 @@ export const aggregateShares = (paths, sharesByPath) => {
key: group.key,
name: group.name,
teamUid: group.teamUid,
anonymous: group.anonymous,
pending: group.pending,
directPaths: group.directPaths,
pendingPaths: group.pendingPaths,
@@ -84,6 +84,26 @@ describe('aggregateShares', () => {
expect(groups[1].directPaths).toEqual(['/me/b']);
});
it('keeps an invitation whose address was withheld, unnamed and unactionable', () => {
// Hiding the row would under-report who reaches the item.
const groups = aggregateShares(['/me/a', '/me/b'], new Map([
['/me/a', [grant(null, 'read', { pending: true, uid: 's1' })]],
['/me/b', [grant(null, 'read', { pending: true, uid: 's2' })]],
]));
// Two invites, not one row folded together on the empty name.
expect(groups).toHaveLength(2);
expect(groups.map((g) => g.key)).toEqual(['invite:s1', 'invite:s2']);
for ( const group of groups ) {
expect(group).toMatchObject({
anonymous: true,
pending: true,
name: 'share_invited_someone',
accessCount: 1,
});
}
});
it('counts an item once when two grants on it name the same person', () => {
// Two holders can grant the same access; the item is still one item.
const groups = aggregateShares(['/me/a'], new Map([