Allow WebFinger discovery for localhost/private hosts by default

src/config.ts

@@ -11,13 +11,19 @@ const config = {
     conflict: true
   },
   cordovaRedirectUri: undefined,
+  discovery: {
+    // Allow WebFinger discovery to target localhost / private-IP hosts. Set to
+    // false in non-browser embedders that want the SSRF guard back.
+    allowPrivateAddresses: true,
+    // WebFinger lookup timeout, in milliseconds.
+    timeout: 5000
+  },
   logging: false,
   modules: [],
   // the following are not public and will probably be moved away from the
   // default config
   backgroundSyncInterval: 60000,
   disableFeatures: [],
-  discoveryTimeout: 5000,
   isBackground: false,
   requestTimeout: 30000,
   syncInterval: 10000

src/discover.ts

@@ -37,14 +37,15 @@ const Discover = function Discover(userAddress: string): Promise<StorageInfo> {
   const webFinger = new WebFinger({
     tls_only: false,
     uri_fallback: true,
-    request_timeout: config.discoveryTimeout
+    request_timeout: config.discovery.timeout,
+    allow_private_addresses: config.discovery.allowPrivateAddresses
   });
 
   let timer;
   const timeoutPromise = new Promise<never>((_, reject) => {
     timer = setTimeout(() => {
       reject(new Error('timed out'));
-    }, config.discoveryTimeout);
+    }, config.discovery.timeout);
   });
 
   return Promise.race([

src/remotestorage.ts

@@ -160,6 +160,10 @@ enum ApiKeyType {
  *     conflict: true
  *   },
  *   cordovaRedirectUri: undefined,
+ *   discovery: {
+ *     allowPrivateAddresses: true,
+ *     timeout: 5000
+ *   },
  *   logging: false,
  *   modules: []
  * });
@@ -428,7 +432,17 @@ export class RemoteStorage {
   constructor (cfg?: object) {
     // Initial configuration property settings.
     // TODO use modern JS to merge object properties
-    if (typeof cfg === 'object') { extend(config, cfg); }
+    if (typeof cfg === 'object' && cfg !== null) {
+      // Pull out nested option groups so the shallow `extend` below doesn't
+      // overwrite the whole sub-object when the caller only sets a subset
+      // (e.g. `{ discovery: { timeout: 10 } }` keeps the default
+      // `allowPrivateAddresses`).
+      const { discovery, ...rest } = cfg as { discovery?: object };
+      extend(config, rest);
+      if (discovery && typeof discovery === 'object') {
+        extend(config.discovery, discovery);
+      }
+    }
 
     this.addEvents([
       'ready', 'authing', 'connecting', 'connected', 'disconnected',

test/unit/discover.test.mjs

@@ -48,6 +48,20 @@ const jrdJimmy = {
   ]
 };
 
+const jrdLocalhost = {
+  "subject": "acct:alice@localhost:8000",
+  "links": [
+    {
+      "rel": "http://tools.ietf.org/id/draft-dejong-remotestorage",
+      "href": "http://localhost:8000/storage/alice",
+      "properties": {
+        "http://remotestorage.io/spec/version": "draft-dejong-remotestorage-13",
+        "http://tools.ietf.org/html/rfc6749#section-4.2": "http://localhost:8000/oauth/alice"
+      }
+    }
+  ]
+};
+
 const jrdJimbo = {
   "subject": "acct:jimmy@kosmos.social",
   "aliases": [
@@ -80,13 +94,13 @@ const jrdJimbo = {
 describe('Webfinger discovery', () => {
   before(() => {
     config.requestTimeout = 500;
-    config.discoveryTimeout = 500;
+    config.discovery.timeout = 500;
   });
 
   after(() => {
     localStorage.clear();
     config.requestTimeout = 30000;
-    config.discoveryTimeout = 5000;
+    config.discovery.timeout = 5000;
   });
 
   describe('successful lookup', () => {
@@ -118,6 +132,36 @@ describe('Webfinger discovery', () => {
     after(() => fetchMock.reset());
   });
 
+  describe('localhost / private addresses (regression for #1384)', () => {
+    before(() => {
+      fetchMock.mock(
+        'http://localhost:8000/.well-known/webfinger?resource=acct:alice@localhost:8000',
+        { status: 200, body: jrdLocalhost, headers: {
+          'Content-Type': 'application/jrd+json; charset=utf-8'
+        }},
+      );
+    });
+
+    it("resolves storage info for a localhost address by default", async () => {
+      const info = await Discover('alice@localhost:8000');
+      expect(info.href).to.equal('http://localhost:8000/storage/alice');
+      expect(info.authURL).to.equal('http://localhost:8000/oauth/alice');
+    });
+
+    it("rejects localhost addresses when discovery.allowPrivateAddresses is false", async () => {
+      const previous = config.discovery.allowPrivateAddresses;
+      config.discovery.allowPrivateAddresses = false;
+      try {
+        // Use a fresh address so the in-module cache does not short-circuit the lookup.
+        await expect(Discover('bob@localhost:9000')).to.be.rejectedWith(/private or internal addresses/);
+      } finally {
+        config.discovery.allowPrivateAddresses = previous;
+      }
+    });
+
+    after(() => fetchMock.reset());
+  });
+
   describe('record missing', () => {
     before(() => {
       fetchMock.mock(

test/unit/remotestorage.test.mjs

@@ -5,6 +5,7 @@ import chaiAsPromised from 'chai-as-promised';
 import sinon from 'sinon';
 import fetchMock from 'fetch-mock';
 
+import config from '../../build/config.js';
 import Dropbox from '../../build/dropbox.js';
 import { EventHandling } from '../../build/eventhandling.js';
 import { RemoteStorage } from '../../build/remotestorage.js';
@@ -41,6 +42,27 @@ describe("RemoteStorage", function() {
     sinon.reset();
   });
 
+  describe('constructor config', function() {
+    it("merges nested `discovery` options with the defaults", function() {
+      const previousTimeout = config.discovery.timeout;
+      const previousAllow = config.discovery.allowPrivateAddresses;
+      try {
+        // Setting only `timeout` must not clobber the default `allowPrivateAddresses`.
+        new RemoteStorage({ cache: false, discovery: { timeout: 1234 } }).disconnect();
+        expect(config.discovery.timeout).to.equal(1234);
+        expect(config.discovery.allowPrivateAddresses).to.equal(previousAllow);
+
+        // Setting only `allowPrivateAddresses` must not clobber `timeout`.
+        new RemoteStorage({ cache: false, discovery: { allowPrivateAddresses: false } }).disconnect();
+        expect(config.discovery.timeout).to.equal(1234);
+        expect(config.discovery.allowPrivateAddresses).to.equal(false);
+      } finally {
+        config.discovery.timeout = previousTimeout;
+        config.discovery.allowPrivateAddresses = previousAllow;
+      }
+    });
+  });
+
   describe('#addModule', function() {
     it('creates a module', function() {
       this.rs.addModule({ name: 'foo', builder: function() {
@@ -84,7 +106,7 @@ describe("RemoteStorage", function() {
 
       this.rs = new RemoteStorage({
         cache: false,
-        discoveryTimeout: 10
+        discovery: { timeout: 10 }
       });
 
       sinon.stub(this.rs, 'authorize');