Merge pull request #49 from immers-space/reliability

Misc reliability fixes

README.md

@@ -137,6 +137,7 @@ store | Replace the default storage model & database backend with your own (see
 threadDepth | Controls how far up apex will follow links in incoming activities in order to display the conversation thread & check for inbox forwarding needs  (default 10)
 systemUser | Actor object representing system and used for signing GETs (see below)
 offlineMode | Disable delivery. Useful for running migrations and queueing deliveries to be sent when app is running
+requestTimeout | Timeout for requests to other servers, ms (default 5000)
 
 Blocked, rejections, and rejected: these routes must be defined in order to track
 these items internally for each actor, but they do not need to be exposed endpoints

index.js

@@ -40,6 +40,7 @@ module.exports = function (settings) {
   apex.systemUser = settings.systemUser
   apex.logger = settings.logger || console
   apex.offlineMode = settings.offlineMode
+  apex.requestTimeout = settings.requestTimeout ?? 5000
   apex.utils = {
     usernameToIRI: apex.idToIRIFactory(apex.domain, settings.routes.actor, apex.actorParam),
     objectIdToIRI: apex.idToIRIFactory(apex.domain, settings.routes.object, apex.objectParam),
@@ -56,7 +57,9 @@ module.exports = function (settings) {
   function onFinishedHandler (err, res) {
     if (err) return
     const apexLocal = res.locals.apex
-    Promise.all(apexLocal.postWork.map(task => task.call(res)))
+    // execute postWork tasks in sequence (not parallel)
+    apexLocal.postWork
+      .reduce((acc, task) => acc.then(() => task(res)), Promise.resolve())
       .then(() => {
         if (apexLocal.eventName) {
           res.app.emit(apexLocal.eventName, apexLocal.eventMessage)

net/validators.js

@@ -116,16 +116,15 @@ function inboxActivity (req, res, next) {
       return next()
     }
   } else if (type === 'accept') {
-    // the activity being accepted was sent to the actor trying to accept it
-    if (!object.to.includes(actor.id)) {
-      resLocal.status = 403
-      return next()
-    }
     // for follows, also confirm the follow object was the actor trying to accept it
     const isFollow = object.type.toLowerCase() === 'follow'
     if (isFollow && !apex.validateTarget(object, actor.id)) {
       resLocal.status = 403
       return next()
+    } else if (!isFollow && !object.to?.includes(actor.id)) {
+      // the activity being accepted was sent to the actor trying to accept it
+      resLocal.status = 403
+      return next()
     }
   }
   tasks.push(apex.embedCollections(activity))
@@ -360,16 +359,15 @@ function outboxActivity (req, res, next) {
     return next()
   }
   if (type === 'accept') {
-    // the activity being accepted was sent to the actor trying to accept it
-    if (!object.to.includes(actor.id)) {
-      resLocal.status = 403
-      return next()
-    }
-    // for follows, also confirm the follow object was the actor trying to accept it
+    // for follows, confirm the follow object was the actor trying to accept it
     const isFollow = object.type.toLowerCase() === 'follow'
     if (isFollow && !apex.validateTarget(object, actor.id)) {
       resLocal.status = 403
       return next()
+    } else if (!isFollow && !object.to?.includes(actor.id)) {
+      // for other accepts, check the activity being accepted was sent to the actor trying to accept it
+      resLocal.status = 403
+      return next()
     }
   } else if (type === 'create') {
     // per spec, ensure attributedTo and audience fields in object are correct

pub/federation.js

@@ -22,7 +22,8 @@ function requestObject (id) {
   const req = {
     url: id,
     headers: { Accept: 'application/activity+json' },
-    json: true
+    json: true,
+    timeout: this.requestTimeout
   }
   if (this.systemUser) {
     req.httpSignature = {
@@ -78,6 +79,7 @@ function deliver (actorId, activity, address, signingKey) {
     },
     resolveWithFullResponse: true,
     simple: false,
+    timeout: this.requestTimeout,
     body: activity
   })
 }

pub/utils.js

@@ -1,4 +1,6 @@
 'use strict'
+const fs = require('fs')
+const path = require('path')
 const jsonld = require('jsonld')
 const merge = require('deepmerge')
 const actorStreamNames = ['inbox', 'outbox', 'following', 'followers', 'liked', 'blocked', 'rejected', 'rejections']
@@ -336,10 +338,27 @@ function validateTarget (object, targetId, prop = 'object') {
   return false
 }
 
+// keep main contexts in memory for speedy access
+const coreContexts = {
+  'https://w3id.org/security/v1': {
+    contextUrl: null,
+    documentUrl: 'https://w3id.org/security/v1',
+    document: JSON.parse(fs.readFileSync(path.resolve(__dirname, '../vocab/security.json')))
+  },
+  'https://www.w3.org/ns/activitystreams': {
+    contextUrl: null,
+    documentUrl: 'https://www.w3.org/ns/activitystreams',
+    document: JSON.parse(fs.readFileSync(path.resolve(__dirname, '../vocab/as.json')))
+  }
+}
 // cached JSONLD contexts to reduce requests an eliminate
 // failures caused when context servers are unavailable
 const nodeDocumentLoader = jsonld.documentLoaders.node()
+
 async function jsonldContextLoader (url, options) {
+  if (coreContexts[url]) {
+    return coreContexts[url]
+  }
   try {
     const cached = await this.store.getContext(url)
     if (cached) {
@@ -351,12 +370,13 @@ async function jsonldContextLoader (url, options) {
   const context = await nodeDocumentLoader(url)
   if (context && context.document) {
     try {
+      // save original url in case of redirects
+      context.documentUrl = url
       await this.store.saveContext(context)
     } catch (err) {
       this.logger.error('Error saving jsonld contact cache', err.message)
     }
   }
-  // call the default documentLoader
   return context
 }
 

spec/functional/inbox.spec.js

@@ -331,7 +331,23 @@ describe('inbox', function () {
           .expect(200)
           .end(err => { if (err) done(err) })
       })
+      it('handles accept to follow without a to field', async function (done) {
+        delete follow.to
+        await apex.store.saveActivity(follow)
+        app.once('apex-inbox', msg => {
+          expect(msg.object.id).toEqual(follow.id)
+          expect(msg.object._meta.collection).toContain(testUser.following[0])
+          done()
+        })
+        request(app)
+          .post('/inbox/test')
+          .set('Content-Type', 'application/activity+json')
+          .send(accept)
+          .expect(200)
+          .end(err => { if (err) done(err) })
+      })
       it('rejects accept from non-recipients of original activity', async function (done) {
+        follow.type = 'Offer'
         follow.to = ['https://ignore.com/sally']
         await apex.store.saveActivity(follow)
         app.once('apex-inbox', msg => {

spec/functional/outbox.spec.js

@@ -525,6 +525,26 @@ describe('outbox', function () {
           .expect(201)
           .end(err => { if (err) done(err) })
       })
+      it('handles accept of follow without to field', async function (done) {
+        delete follow.to
+        await apex.store.saveActivity(follow)
+        app.once('apex-outbox', msg => {
+          expect(msg.actor).toEqual(testUser)
+          const exp = merge({ _meta: { collection: ['https://localhost/outbox/test'] } }, activityNormalized)
+          exp.type = 'Accept'
+          exp.object = [follow.id]
+          expect(global.stripIds(msg.activity)).toEqual(exp)
+          follow._meta.collection.push(testUser.followers[0])
+          expect(msg.object).toEqual(follow)
+          done()
+        })
+        request(app)
+          .post('/authorized/outbox/test')
+          .set('Content-Type', 'application/activity+json')
+          .send(accept)
+          .expect(201)
+          .end(err => { if (err) done(err) })
+      })
       it('publishes collection update', async function (done) {
         const mockedUser = 'https://mocked.com/user/mocked'
         nock('https://mocked.com')

spec/helpers/nocks.js

@@ -1,18 +1,6 @@
-/* global beforeAll, afterAll */
-const fs = require('fs')
+/* global beforeEach, afterEach */
 const nock = require('nock')
-const activities = fs.readFileSync('vocab/as.json')
-const security = fs.readFileSync('vocab/security.json')
-beforeAll(() => {
-  nock('https://www.w3.org')
-    .get('/ns/activitystreams')
-    .reply(200, activities)
-    .persist(true)
-  nock('https://w3id.org')
-    .get('/security/v1')
-    .reply(200, security)
-    .persist(true)
-  // block federation attempts
+beforeEach(() => {
   nock('https://ignore.com')
     .get(uri => uri.startsWith('/s/'))
     .reply(200, uri => ({ id: `https://ignore.com${uri}`, type: 'Activity', actor: 'https://ignore.com/u/bob' }))
@@ -24,6 +12,6 @@ beforeAll(() => {
     .reply(200)
     .persist()
 })
-afterAll(() => {
+afterEach(() => {
   nock.cleanAll()
 })

spec/unit/utils.spec.js

@@ -57,6 +57,34 @@ describe('utils', function () {
         contextUrl: null
       })
     })
+    it('caches redirected contexts by original url', async function () {
+      nock('https://mocked.com')
+        .get('/context/v1')
+        .reply(302, undefined, {
+          Location: 'http://redirect.com/context/v1'
+        })
+      nock('http://redirect.com')
+        .get('/context/v1')
+        .reply(200, context)
+      const doc = {
+        '@context': 'https://mocked.com/context/v1',
+        id: 'https://mocked.com/s/abc123',
+        customProp: 'https://mocked.com/s/123abc'
+      }
+      const ld = await apex.toJSONLD(doc)
+      expect(ld).toEqual({
+        '@context': apex.context,
+        id: 'https://mocked.com/s/abc123',
+        'https://mocked.com/context/v1#customProp': {
+          id: 'https://mocked.com/s/123abc'
+        }
+      })
+      expect(await apex.store.getContext('https://mocked.com/context/v1')).toEqual({
+        documentUrl: 'https://mocked.com/context/v1',
+        document: JSON.stringify(context),
+        contextUrl: null
+      })
+    })
     it('uses cached context', async function () {
       await apex.store.saveContext({
         documentUrl: 'https://mocked.com/context/v1',