From 6be84cbcea99518e8eca58c76259effd0dd992fd Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Thu, 22 Mar 2018 18:40:33 +0100 Subject: [PATCH] Improve activity pub actors implementation --- server/helpers/activitypub.ts | 9 ++++++++- server/helpers/custom-validators/activitypub/activity.ts | 2 +- server/helpers/custom-validators/activitypub/misc.ts | 2 +- server/lib/activitypub/actor.ts | 5 ++++- server/lib/activitypub/process/process-accept.ts | 4 +++- server/lib/activitypub/process/process-reject.ts | 4 +++- server/lib/activitypub/process/process-undo.ts | 9 ++++++--- server/lib/activitypub/process/process.ts | 7 +++++-- shared/models/activitypub/activity.ts | 2 +- 9 files changed, 32 insertions(+), 12 deletions(-) diff --git a/server/helpers/activitypub.ts b/server/helpers/activitypub.ts index d64a6dd78..1934fa0f0 100644 --- a/server/helpers/activitypub.ts +++ b/server/helpers/activitypub.ts @@ -1,5 +1,5 @@ import { ResultList } from '../../shared/models' -import { Activity } from '../../shared/models/activitypub' +import { Activity, ActivityPubActor } from '../../shared/models/activitypub' import { ACTIVITY_PUB } from '../initializers' import { ActorModel } from '../models/activitypub/actor' import { signObject } from './peertube-crypto' @@ -98,9 +98,16 @@ function buildSignedActivity (byActor: ActorModel, data: Object) { return signObject(byActor, activity) as Promise } +function getActorUrl (activityActor: string | ActivityPubActor) { + if (typeof activityActor === 'string') return activityActor + + return activityActor.id +} + // --------------------------------------------------------------------------- export { + getActorUrl, activityPubContextify, activityPubCollectionPagination, activityPubCollection, diff --git a/server/helpers/custom-validators/activitypub/activity.ts b/server/helpers/custom-validators/activitypub/activity.ts index 632f14223..7e4dccefb 100644 --- a/server/helpers/custom-validators/activitypub/activity.ts +++ b/server/helpers/custom-validators/activitypub/activity.ts @@ -26,7 +26,7 @@ function isRootActivityValid (activity: any) { ) || ( isActivityPubUrlValid(activity.id) && - isActivityPubUrlValid(activity.actor) + (isActivityPubUrlValid(activity.actor) || isActivityPubUrlValid(activity.actor.id)) ) } diff --git a/server/helpers/custom-validators/activitypub/misc.ts b/server/helpers/custom-validators/activitypub/misc.ts index 75d308e9d..2dac8e1ad 100644 --- a/server/helpers/custom-validators/activitypub/misc.ts +++ b/server/helpers/custom-validators/activitypub/misc.ts @@ -24,7 +24,7 @@ function isBaseActivityValid (activity: any, type: string) { return (activity['@context'] === undefined || Array.isArray(activity['@context'])) && activity.type === type && isActivityPubUrlValid(activity.id) && - isActivityPubUrlValid(activity.actor) && + (isActivityPubUrlValid(activity.actor) || isActivityPubUrlValid(activity.actor.id)) && ( activity.to === undefined || (Array.isArray(activity.to) && activity.to.every(t => isActivityPubUrlValid(t))) diff --git a/server/lib/activitypub/actor.ts b/server/lib/activitypub/actor.ts index b7114bbee..fa31e71c7 100644 --- a/server/lib/activitypub/actor.ts +++ b/server/lib/activitypub/actor.ts @@ -5,6 +5,7 @@ import * as url from 'url' import * as uuidv4 from 'uuid/v4' import { ActivityPubActor, ActivityPubActorType } from '../../../shared/models/activitypub' import { ActivityPubAttributedTo } from '../../../shared/models/activitypub/objects' +import { getActorUrl } from '../../helpers/activitypub' import { isActorObjectValid } from '../../helpers/custom-validators/activitypub/actor' import { isActivityPubUrlValid } from '../../helpers/custom-validators/activitypub/misc' import { retryTransactionWrapper, updateInstanceWithAnother } from '../../helpers/database-utils' @@ -34,7 +35,9 @@ function setAsyncActorKeys (actor: ActorModel) { }) } -async function getOrCreateActorAndServerAndModel (actorUrl: string, recurseIfNeeded = true) { +async function getOrCreateActorAndServerAndModel (activityActor: string | ActivityPubActor, recurseIfNeeded = true) { + const actorUrl = getActorUrl(activityActor) + let actor = await ActorModel.loadByUrl(actorUrl) // We don't have this actor in our database, fetch it on remote diff --git a/server/lib/activitypub/process/process-accept.ts b/server/lib/activitypub/process/process-accept.ts index 7db2f8ff0..c55b57820 100644 --- a/server/lib/activitypub/process/process-accept.ts +++ b/server/lib/activitypub/process/process-accept.ts @@ -1,4 +1,5 @@ import { ActivityAccept } from '../../../../shared/models/activitypub' +import { getActorUrl } from '../../../helpers/activitypub' import { ActorModel } from '../../../models/activitypub/actor' import { ActorFollowModel } from '../../../models/activitypub/actor-follow' import { addFetchOutboxJob } from '../fetch' @@ -6,7 +7,8 @@ import { addFetchOutboxJob } from '../fetch' async function processAcceptActivity (activity: ActivityAccept, inboxActor?: ActorModel) { if (inboxActor === undefined) throw new Error('Need to accept on explicit inbox.') - const targetActor = await ActorModel.loadByUrl(activity.actor) + const actorUrl = getActorUrl(activity.actor) + const targetActor = await ActorModel.loadByUrl(actorUrl) return processAccept(inboxActor, targetActor) } diff --git a/server/lib/activitypub/process/process-reject.ts b/server/lib/activitypub/process/process-reject.ts index b2de28d79..f06b03772 100644 --- a/server/lib/activitypub/process/process-reject.ts +++ b/server/lib/activitypub/process/process-reject.ts @@ -1,4 +1,5 @@ import { ActivityReject } from '../../../../shared/models/activitypub/activity' +import { getActorUrl } from '../../../helpers/activitypub' import { sequelizeTypescript } from '../../../initializers' import { ActorModel } from '../../../models/activitypub/actor' import { ActorFollowModel } from '../../../models/activitypub/actor-follow' @@ -6,7 +7,8 @@ import { ActorFollowModel } from '../../../models/activitypub/actor-follow' async function processRejectActivity (activity: ActivityReject, inboxActor?: ActorModel) { if (inboxActor === undefined) throw new Error('Need to reject on explicit inbox.') - const targetActor = await ActorModel.loadByUrl(activity.actor) + const actorUrl = getActorUrl(activity.actor) + const targetActor = await ActorModel.loadByUrl(actorUrl) return processReject(inboxActor, targetActor) } diff --git a/server/lib/activitypub/process/process-undo.ts b/server/lib/activitypub/process/process-undo.ts index 5a770bb97..565e70289 100644 --- a/server/lib/activitypub/process/process-undo.ts +++ b/server/lib/activitypub/process/process-undo.ts @@ -1,5 +1,6 @@ import { ActivityFollow, ActivityLike, ActivityUndo } from '../../../../shared/models/activitypub' import { DislikeObject } from '../../../../shared/models/activitypub/objects' +import { getActorUrl } from '../../../helpers/activitypub' import { retryTransactionWrapper } from '../../../helpers/database-utils' import { logger } from '../../../helpers/logger' import { sequelizeTypescript } from '../../../initializers' @@ -13,12 +14,14 @@ import { getOrCreateAccountAndVideoAndChannel } from '../videos' async function processUndoActivity (activity: ActivityUndo) { const activityToUndo = activity.object + const actorUrl = getActorUrl(activity.actor) + if (activityToUndo.type === 'Like') { - return processUndoLike(activity.actor, activity) + return processUndoLike(actorUrl, activity) } else if (activityToUndo.type === 'Create' && activityToUndo.object.type === 'Dislike') { - return processUndoDislike(activity.actor, activity) + return processUndoDislike(actorUrl, activity) } else if (activityToUndo.type === 'Follow') { - return processUndoFollow(activity.actor, activityToUndo) + return processUndoFollow(actorUrl, activityToUndo) } logger.warn('Unknown activity object type %s -> %s when undo activity.', activityToUndo.type, { activity: activity.id }) diff --git a/server/lib/activitypub/process/process.ts b/server/lib/activitypub/process/process.ts index 094219489..10d8ba189 100644 --- a/server/lib/activitypub/process/process.ts +++ b/server/lib/activitypub/process/process.ts @@ -1,4 +1,5 @@ import { Activity, ActivityType } from '../../../../shared/models/activitypub' +import { getActorUrl } from '../../../helpers/activitypub' import { logger } from '../../../helpers/logger' import { ActorModel } from '../../../models/activitypub/actor' import { processAcceptActivity } from './process-accept' @@ -25,9 +26,11 @@ const processActivity: { [ P in ActivityType ]: (activity: Activity, inboxActor? async function processActivities (activities: Activity[], signatureActor?: ActorModel, inboxActor?: ActorModel) { for (const activity of activities) { + const actorUrl = getActorUrl(activity.actor) + // When we fetch remote data, we don't have signature - if (signatureActor && activity.actor !== signatureActor.url) { - logger.warn('Signature mismatch between %s and %s.', activity.actor, signatureActor.url) + if (signatureActor && actorUrl !== signatureActor.url) { + logger.warn('Signature mismatch between %s and %s.', actorUrl, signatureActor.url) continue } diff --git a/shared/models/activitypub/activity.ts b/shared/models/activitypub/activity.ts index a28260ffd..f555f0118 100644 --- a/shared/models/activitypub/activity.ts +++ b/shared/models/activitypub/activity.ts @@ -22,7 +22,7 @@ export interface BaseActivity { id: string to?: string[] cc?: string[] - actor: string + actor: string | ActivityPubActor type: ActivityType signature?: ActivityPubSignature }