diff --git a/docs/nodejs.md b/docs/nodejs.md index 0e2198b68..086a7bd1e 100644 --- a/docs/nodejs.md +++ b/docs/nodejs.md @@ -27,14 +27,14 @@ token that you acquired beforehand: remoteStorage.connect('user@example.com', 'abcdef123456') ``` -This will skip the entire OAuth process, because you did that before in -some other way, of course. +This will skip the entire OAuth process in favor of acquiring a token +beforehand, without the browser redirect. ## Obtaining a token -For some programs, like e.g. a server daemon, you can usually acquire -the token from your server manually, and then just configure it for -example as environment variable, when running your program. +For some programs, for example a server daemon, you can acquire the token from +your RS server manually, and then use it from a config file or environment +variable in your application. For CLI programs, and if you actually want to integrate the OAuth flow in your program, one possible solution is the following: @@ -64,7 +64,19 @@ rs-backup is not using remoteStorage.js at all, which you might also want to consider as an option when writing non-browser applications. ::: -## Caveats +## Other considerations + +### Discovery + +In most cases, you will want to set `discovery.allowPrivateAddresses` to +`false` when creating your [RemoteStorage][1] instance. + +This setting controls whether WebFinger may target localhost/private-IP hosts. +It defaults to `true` because cross-origin requests in browsers are already +gated by the same-origin policy / CORS. Setting it to `false` in non-browser +embedders will re-enable webfinger.js's SSRF guard. + +### Caveats - IndexedDB and localStorage are not supported by default in Node.js, so the library will fall back to in-memory storage for caching data @@ -89,3 +101,5 @@ fact cache your data similar to how a browser would do it. accounts using the [chat-messages](https://www.npmjs.com/package/remotestorage-module-chat-messages) module + +[1]: ../api/remotestorage/classes/RemoteStorage.html diff --git a/src/config.ts b/src/config.ts index 71eb5e9f3..1fe96a969 100644 --- a/src/config.ts +++ b/src/config.ts @@ -11,13 +11,16 @@ const config = { conflict: true }, cordovaRedirectUri: undefined, + discovery: { + allowPrivateAddresses: true, + 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 diff --git a/src/discover.ts b/src/discover.ts index 1b908873b..f66a1cbd3 100644 --- a/src/discover.ts +++ b/src/discover.ts @@ -37,14 +37,15 @@ const Discover = function Discover(userAddress: string): Promise { 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((_, reject) => { timer = setTimeout(() => { reject(new Error('timed out')); - }, config.discoveryTimeout); + }, config.discovery.timeout); }); return Promise.race([ diff --git a/src/remotestorage.ts b/src/remotestorage.ts index 08f738fdd..a1075d337 100644 --- a/src/remotestorage.ts +++ b/src/remotestorage.ts @@ -141,7 +141,7 @@ enum ApiKeyType { } /** - * Create a `remoteStorage` class instance so: + * Create a `remoteStorage` class instance: * * ```js * const remoteStorage = new RemoteStorage(); @@ -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 `Object.assign` 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 }; + Object.assign(config, rest); + if (discovery && typeof discovery === 'object') { + Object.assign(config.discovery, discovery); + } + } this.addEvents([ 'ready', 'authing', 'connecting', 'connected', 'disconnected', diff --git a/test/unit/discover.test.mjs b/test/unit/discover.test.mjs index 5cb6f13b0..60b3ef2dd 100644 --- a/test/unit/discover.test.mjs +++ b/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( diff --git a/test/unit/remotestorage.test.mjs b/test/unit/remotestorage.test.mjs index 5a2b922e4..cf19568c4 100644 --- a/test/unit/remotestorage.test.mjs +++ b/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');