openvibers/openvibe.community Commits

Accept attachments from item owning services (#375)

OP OpenVibers committed 2 days ago · 1d7bdc754c · 4 files, +79 −23

* Accept owning service comment attachments

* Allow active unlisted picture discussions

* Accept picture comment uploads with person token

Changes

ddiff --git a/server/comments/service.js b/server/comments/service.js
iindex 019c99a..2378638 100644
---- a/server/comments/service.js
++++ b/server/comments/service.js
@@@ -42,7 +42,7 @@ const sectionReactions = require('./section-reactions');
const { fail, isoTime } = require('../http/v1');
const { createAuthors } = require('../identity/authors');
const { createPersonLimiter } = require('../limits');
--const { discussionModerator: moderator } = require('../identity/capabilities');
++const { discussionModerator: moderator, checkCapability } = require('../identity/capabilities');
const blocks = require('../identity/blocks');
const commentAttachments = require('./attachments');
const forumLinks = require('./forum-links');
@@@ -160,22 +160,22 @@ function createCommentService({ db, network = null, limits = {}, canViewPaste =
if (canViewPaste) return await canViewPaste(v, p);
return !(p.visibility === 'private' && !(v && (v.staff || (v.subject && p.owner_subject && v.subject === p.owner_subject))));
}
-- /** Whether the viewer may see the entity a thread belongs to (only Community's pastes can be checked here). */
-- async function publicPicture(id) {
++ /** An active Pics image is visible to anyone holding its unlisted link and id. */
++ async function activePicture(id) {
if (!/^pic_[0-9A-HJKMNP-TV-Z]{26}$/.test(String(id))) return false;
try {
const response = await fetchImpl(`https://openvibe.pics/api/v1/pictures/${encodeURIComponent(id)}`, { redirect: 'error', signal: AbortSignal.timeout(3000), headers: { Accept: 'application/json' } });
if (!response.ok) return false;
const picture = await response.json();
-- return picture.id === id && picture.listed === true && picture.state === 'active' && typeof picture.views === 'number';
++ return picture.id === id && picture.state === 'active' && typeof picture.slug === 'string';
} catch { return false; }
}
-- const entityVisible = async (v, t) => t.ref_service === 'pics' && t.ref_type === 'picture' ? publicPicture(t.ref_id)
++ const entityVisible = async (v, t) => t.ref_service === 'pics' && t.ref_type === 'picture' ? activePicture(t.ref_id)
: !(t.ref_service === 'community' && t.ref_type === 'paste') || await pasteVisibleTo(v, t.ref_id);
/** Community's own entities must exist (and be visible to the viewer) before anyone opens a thread on them. */
async function checkCommunityRef(ref, v) {
-- if (ref.service === 'pics' && ref.type === 'picture' && !await publicPicture(ref.id)) fail(404, 'ref.not_found', 'No such public picture');
++ if (ref.service === 'pics' && ref.type === 'picture' && !await activePicture(ref.id)) fail(404, 'ref.not_found', 'No such active picture');
if (ref.service !== 'community') return;
if (ref.type === 'paste') {
if (!await pasteVisibleTo(v, ref.id)) fail(404, 'ref.not_found', 'No such paste');
@@@ -229,11 +229,16 @@ function createCommentService({ db, network = null, limits = {}, canViewPaste =
return { thread: shapeThread(thread, v), created };
},
-- /** Upload one attachment to Community, which owns Community paste comments. */
++ /** Stage an attachment received by the service that owns the item. */
async uploadAttachment(v, ref, file) {
if (!(v && v.subject)) fail(401, 'auth.required', 'Sign in to attach a file');
-- if (!ref || ref.service !== 'community' || ref.type !== 'paste') fail(403, 'attachment.owner_service', 'Upload this attachment through the service that owns the item');
-- await checkCommunityRef(ref, v);
++ const paste = ref && ref.service === 'community' && ref.type === 'paste';
++ const owner = ref && ((ref.service === 'live' && ['vod', 'clip'].includes(ref.type) && v.service === 'svc:live')
++ || (ref.service === 'pics' && ref.type === 'picture' && v.service === 'svc:media-hub'));
++ const picture = ref && ref.service === 'pics' && ref.type === 'picture' && v.kind === 'user';
++ if (!paste && !picture && !(owner && v.kind === 'service' && checkCapability(v.claims, 'community.comment.write').allowed))
++ fail(403, 'attachment.owner_service', 'Upload this attachment through the service that owns the item');
++ if (paste || picture) await checkCommunityRef(ref, v);
if (!file || !Buffer.isBuffer(file.buffer) || !file.buffer.length) fail(400, 'attachment.missing', 'Choose a file');
if (file.buffer.length > 16 * 1024 * 1024) fail(413, 'attachment.too_large', 'Files are limited to 16 MB');
if (!media || !media.configured || typeof media.uploadImage !== 'function') fail(503, 'attachment.unavailable', 'Files cannot be attached right now');
@@@ -338,7 +343,6 @@ function createCommentService({ db, network = null, limits = {}, canViewPaste =
if (rawAttachments.length && !author) fail(401, 'auth.required', 'Sign in to attach a file');
let attachmentIds = [];
if (rawAttachments.length) {
-- if (t.ref_service !== 'community' || t.ref_type !== 'paste') fail(403, 'attachment.owner_service', 'Upload attachments through the service that owns the item');
const refs = rawAttachments.map((raw) => {
try { return commentAttachments.cleanReference(raw, author); }
catch (err) { fail(err.status || 400, err.code || 'comment.attachment_invalid', err.message); }
@@@ -388,7 +392,7 @@ function createCommentService({ db, network = null, limits = {}, canViewPaste =
if (!isAuthor && !moderator(v)) fail(403, 'comment.not_yours', 'Not authorized to delete this comment');
await store.softDeleteComment(db, c.id, v.subject || v.service || null);
for (const a of c.attachments || []) {
-- if (a.ref_service === 'community' && media && typeof media.deleteObject === 'function') {
++ if (media && typeof media.deleteObject === 'function') {
try { await media.deleteObject(a.media_id); }
catch (err) { console.warn(`[Comments] removed attachment ${a.media_id} from comment; Media cleanup failed: ${err.message}`); }
}
ddiff --git a/test/comments.test.js b/test/comments.test.js
iindex e58f426..e651184 100644
---- a/test/comments.test.js
++++ b/test/comments.test.js
@@@ -427,6 +427,35 @@ const { checkCapability } = require('../server/identity/capabilities');
assert.strictEqual(byBrowser.status, 201, 'people with the access id may comment');
});
++ await check('Live stages a file for its VOD; the author claims it once and removal retires it', async () => {
++ const ref = { service: 'live', type: 'vod', id: '7001' };
++ const form = new FormData();
++ form.set('ref', JSON.stringify(ref));
++ form.set('file', new Blob([Buffer.from('live picture')], { type: 'image/png' }), 'live.png');
++ const url = '/api/v1/comments/attachments';
++ const browser = await t.get(url, { method: 'POST', headers: { authorization: `Bearer ${alexJwt}` }, body: form });
++ assert.strictEqual(browser.status, 403);
++ const token = svc([WRITE], { sub: 'svc:live' });
++ const uploaded = await t.get(url, { method: 'POST', headers: { authorization: `Bearer ${token}`, 'x-ov-subject': alex.subject_id }, body: form });
++ assert.strictEqual(uploaded.status, 201, uploaded.text);
++ const attachment = uploaded.json().attachment;
++ const added = await callL(`/api/v1/comments/threads/${liveVod.id}/comments`, {
++ method: 'POST', json: { message: 'A picture', attachments: [attachment] }, token,
++ headers: { 'x-ov-subject': alex.subject_id },
++ });
++ assert.strictEqual(added.status, 201, added.text);
++ assert.deepStrictEqual(added.json().comment.attachments.map((a) => a.id), [attachment.id]);
++ const stolen = await callL(`/api/v1/comments/threads/${liveVod.id}/comments`, {
++ method: 'POST', json: { message: 'Reused picture', attachments: [attachment] }, token,
++ headers: { 'x-ov-subject': alex.subject_id },
++ });
++ assert.strictEqual(stolen.status, 400);
++ const removed = await callL(`/api/v1/comments/${added.json().comment.id}`, { method: 'DELETE', token,
++ headers: { 'x-ov-subject': alex.subject_id } });
++ assert.strictEqual(removed.status, 200);
++ assert.ok(t.media.deletes.includes(attachment.id));
++ });
++
await check('one comment with its thread (GET /:commentId): services only, so comment ids cannot be walked to refs', async () => {
const c = (await callL(`/api/v1/comments/threads/${liveVod.id}/comments`, { method: 'POST', json: { message: 'look me up' }, ...asLive(sam.subject_id) })).json().comment;
const r = await callL(`/api/v1/comments/${c.id}`, asLive(sam.subject_id));
ddiff --git a/test/pics-comments.test.js b/test/pics-comments.test.js
iindex 07cbecb..afed219 100644
---- a/test/pics-comments.test.js
++++ b/test/pics-comments.test.js
@@@ -2,42 +2,65 @@
/**
* Comments on an OpenVibe.Pics picture, and a picture embedded in a paste.
*
-- * A picture's thread is opened by the picture's own page, so Community asks Pics whether the picture is public
-- * right now: listed, active, and with public counts. Anything else fails closed (404, never a hint that the
-- * picture exists). A paste may embed a picture from the fixed Pics origin, raster formats only.
++ * A picture's thread is opened by its unlisted page, so Community asks Pics whether the picture is active
++ * right now. Anything hidden or gone fails closed (404). A paste may embed a picture from the fixed
++ * Pics origin, raster formats only.
*/
const assert = require('assert');
const { createCommentService } = require('../server/comments/service');
const { renderMarkdown } = require('../server/render/markdown');
const PICTURE = 'pic_01ARZ3NDEKTSV4RRFFQ69G5FAV';
--const listed = (over = {}) => ({ id: PICTURE, listed: true, state: 'active', views: 3, ...over });
++const picture = (over = {}) => ({ id: PICTURE, slug: 'Abcdef23', state: 'active', ...over });
(async () => {
const answers = [];
let requests = 0;
-- const service = createCommentService({ db: {}, fetchImpl: async () => { requests++; const body = answers.shift() || { ok: false }; return { ok: body.ok !== false, json: async () => body.body || body }; } });
++ const service = createCommentService({ db: { tx: async () => { throw new Error('reached store'); } },
++ fetchImpl: async () => { requests++; const body = answers.shift() || { ok: false }; return { ok: body.ok !== false, json: async () => body.body || body }; } });
await assert.rejects(service.resolve({ kind: 'anonymous' }, { ref: { service: 'pics', type: 'picture', id: 'not-a-picture' } }),
(e) => e.code === 'ref.not_found', 'an id that is not a picture is refused');
assert.equal(requests, 0, 'and nothing was asked of Pics');
-- answers.push({ body: listed({ listed: false }) });
++ answers.push({ body: picture() });
await assert.rejects(service.resolve({ kind: 'anonymous' }, { ref: { service: 'pics', type: 'picture', id: PICTURE } }),
-- (e) => e.code === 'ref.not_found', 'an unlisted picture has no thread');
++ (e) => e.message === 'reached store', 'an active unlisted picture may open a thread');
-- answers.push({ body: listed({ state: 'hidden' }) });
++ answers.push({ body: picture({ state: 'hidden' }) });
await assert.rejects(service.resolve({ kind: 'anonymous' }, { ref: { service: 'pics', type: 'picture', id: PICTURE } }),
(e) => e.code === 'ref.not_found', 'a hidden picture has no thread');
-- answers.push({ body: listed({ views: undefined }) });
++ answers.push({ body: picture({ slug: undefined }) });
await assert.rejects(service.resolve({ kind: 'anonymous' }, { ref: { service: 'pics', type: 'picture', id: PICTURE } }),
-- (e) => e.code === 'ref.not_found', 'a picture without public counts is not a public picture');
++ (e) => e.code === 'ref.not_found', 'a picture without a public slug is not exposed');
answers.push({ ok: false });
await assert.rejects(service.resolve({ kind: 'anonymous' }, { ref: { service: 'pics', type: 'picture', id: PICTURE } }),
(e) => e.code === 'ref.not_found', 'and Pics saying nothing fails closed');
++ let staged;
++ const mediaId = 'med_01JAB2C3D4E5F6G7H8J9K0MNP6';
++ const owner = 'usr_01JAB2C3D4E5F6G7H8J9K0MNPQ';
++ const uploadService = createCommentService({
++ db: { prepare: (sql) => ({
++ run: async (...args) => { if (sql.includes('INSERT INTO comment_attachments')) staged = {
++ media_id: args[0], ref_service: args[1], ref_type: args[2], ref_id: args[3], owner_subject: args[4],
++ filename: args[5], mime: args[6], size_bytes: args[7], visibility: args[8],
++ }; return { changes: 1 }; },
++ get: async () => staged,
++ }) },
++ media: { configured: true, uploadImage: async () => ({ id: mediaId }), object: async () => ({
++ owner: { subject: owner }, lifecycle_status: 'ready', metadata: { filename: 'look.png' }, mime_type: 'image/png', size_bytes: 4,
++ }) },
++ fetchImpl: async () => ({ ok: true, json: async () => picture() }),
++ });
++ const uploaded = await uploadService.uploadAttachment({ kind: 'user', subject: owner }, { service: 'pics', type: 'picture', id: PICTURE },
++ { buffer: Buffer.from('look'), originalname: 'look.png', mimetype: 'image/png' });
++ assert.equal(uploaded.attachment.id, mediaId);
++ assert.equal(staged.ref_service, 'pics');
++ assert.equal(staged.owner_subject, owner);
++
// A paste may embed a picture: the fixed origin, raster formats only.
const embed = renderMarkdown('![forest](https://openvibe.pics/i/Abcdef23.png)');
assert.ok(embed.includes('<img'), 'a Pics picture embeds as an image');
@@@ -47,5 +70,5 @@ const listed = (over = {}) => ({ id: PICTURE, listed: true, state: 'active', vie
assert.ok(!renderMarkdown('![x](https://openvibe.pics/i/Abcdef23.png?x=1)').includes('<img'), 'no query strings');
assert.ok(!renderMarkdown('![x](https://openvibe.pics/a/Abcdef23.png)').includes('<img'), 'no album paths');
-- process.stdout.write('Pics comments fail closed; fixed-origin raster embeds pass\n');
++ process.stdout.write('Active unlisted Pics comments and fixed-origin raster embeds pass\n');
})().catch((e) => { process.stderr.write(e.stack + '\n'); process.exit(1); });
ddiff --git a/test/security-ssrf.test.js b/test/security-ssrf.test.js
iindex 9b7415b..d870fc1 100644
---- a/test/security-ssrf.test.js
++++ b/test/security-ssrf.test.js
@@@ -31,7 +31,7 @@ const INVENTORY = {
'server/render/pages.js': [1, 'not a call: the home page shows a fetch() sample as text (never executed)'],
'server/identity/account-data.js': [2, 'Network /oauth/token and /internal/account-exports and /internal/account-deletions (fixed paths) with a service token'],
'server/identity/profile-module.js': [3, 'Network modules API (community.profile) with a service token'],
-- 'server/comments/service.js': [1, 'OpenVibe.Pics GET /api/v1/pictures/<id> at the fixed https://openvibe.pics origin, no redirect, to ask whether a picture is public; the id must match the picture-ID shape and is encoded'],
++ 'server/comments/service.js': [1, 'OpenVibe.Pics GET /api/v1/pictures/<id> at the fixed https://openvibe.pics origin, no redirect, to ask whether a picture is active; the id must match the picture-ID shape and is encoded'],
'server/comments/forum-links.js': [1, 'Public item lookup at fixed OpenVibe.Live or OpenVibe.Pics origins; the validated item id is encoded in a fixed API path and redirects are rejected'],
'server/media/files.js': [2, 'OV_MEDIA_INTERNAL_URL v1 file store, service token'],
'server/media/objects.js': [3, 'OV_MEDIA_INTERNAL_URL Object API v2 and normalize-image paths, service token; fixed Media base, never a user URL'],