diff --git a/test/server/lib/DocApi.ts b/test/server/lib/DocApi.ts index e4caa131c5..b327cabfb2 100644 --- a/test/server/lib/DocApi.ts +++ b/test/server/lib/DocApi.ts @@ -15,7 +15,7 @@ import { getDocApiUsageKeysToIncr, WebhookSubscription } from 'app/server/lib/DocApi'; -import {delayAbort} from 'app/server/lib/serverUtils'; +import {delayAbort, getAvailablePort} from 'app/server/lib/serverUtils'; import axios, {AxiosRequestConfig, AxiosResponse} from 'axios'; import {delay} from 'bluebird'; import {assert} from 'chai'; @@ -77,6 +77,21 @@ function makeConfig(username: string): AxiosRequestConfig { }; } +// Much like home.makeUserApi, except it injects extraHeadersForConfig (for tests with reverse-proxy) +function makeUserApi( + org: string, + username: string, + options?: { + baseUrl?: string + } +) { + return new UserAPIImpl(`${options?.baseUrl ?? homeUrl}/o/${org}`, { + headers: makeConfig(username).headers as Record, + fetch: fetch as unknown as typeof globalThis.fetch, + newFormData: () => new FormData() as any, + }); +} + describe('DocApi', function () { const webhooksTestPort = Number(process.env.WEBHOOK_TEST_PORT || 34365); @@ -160,33 +175,47 @@ describe('DocApi', function () { }; home = await TestServer.startServer('home', tmpDir, suitename, additionalEnvConfiguration); - docs = await TestServer.startServer('docs', tmpDir, suitename, additionalEnvConfiguration, home.serverUrl); homeUrl = serverUrl = home.serverUrl; + docs = await TestServer.startServer('docs', tmpDir, suitename, additionalEnvConfiguration, homeUrl); hasHomeApi = true; }); testDocApi({webhooksTestPort}); }); describe('behind a reverse-proxy', function () { - async function setupServersWithProxy(suitename: string, overrideEnvConf?: NodeJS.ProcessEnv) { - const proxy = new TestServerReverseProxy(); + async function setupServersWithProxy( + suitename: string, + { withAppHomeInternalUrl }: { withAppHomeInternalUrl: boolean } + ) { + const proxy = await TestServerReverseProxy.build(); + + const homePort = await getAvailablePort(parseInt(process.env.GET_AVAILABLE_PORT_START || '8080', 10)); + const home = new TestServer('home', homePort, tmpDir, suitename); + const additionalEnvConfiguration = { ALLOWED_WEBHOOK_DOMAINS: `example.com,localhost:${webhooksTestPort}`, GRIST_DATA_DIR: dataDir, - APP_HOME_URL: await proxy.getServerUrl(), + APP_HOME_URL: proxy.serverUrl, GRIST_ORG_IN_PATH: 'true', GRIST_SINGLE_PORT: '0', - ...overrideEnvConf + APP_HOME_INTERNAL_URL: withAppHomeInternalUrl ? home.serverUrl : '', }; - const home = await TestServer.startServer('home', tmpDir, suitename, additionalEnvConfiguration); - const docs = await TestServer.startServer( - 'docs', tmpDir, suitename, additionalEnvConfiguration, home.serverUrl - ); + + await home.start(home.serverUrl, additionalEnvConfiguration); + + const docPort = await getAvailablePort(parseInt(process.env.GET_AVAILABLE_PORT_START || '8080', 10)); + const docs = new TestServer('docs', docPort, tmpDir, suitename); + await docs.start(home.serverUrl, { + ...additionalEnvConfiguration, + APP_DOC_URL: `${proxy.serverUrl}/dw/dw1`, + APP_DOC_INTERNAL_URL: docs.serverUrl, + }); + proxy.requireFromOutsideHeader(); - await proxy.start(home, docs); + proxy.start(home, docs); - homeUrl = serverUrl = await proxy.getServerUrl(); + homeUrl = serverUrl = proxy.serverUrl; hasHomeApi = true; extraHeadersForConfig = { Origin: serverUrl, @@ -206,9 +235,9 @@ describe('DocApi', function () { let proxy: TestServerReverseProxy; - describe('should run usual DocApi test', function () { + describe('with APP_HOME_INTERNAL_URL set', function () { setup('behind-proxy-testdocs', async () => { - ({proxy, home, docs} = await setupServersWithProxy(suitename)); + ({proxy, home, docs} = await setupServersWithProxy(suitename, {withAppHomeInternalUrl: true})); }); after(() => tearDown(proxy, [home, docs])); @@ -217,11 +246,7 @@ describe('DocApi', function () { }); async function testCompareDocs(proxy: TestServerReverseProxy, home: TestServer) { - const chimpy = makeConfig('chimpy'); - const userApiServerUrl = await proxy.getServerUrl(); - const chimpyApi = home.makeUserApi( - ORG_NAME, 'chimpy', { serverUrl: userApiServerUrl, headers: chimpy.headers as Record } - ); + const chimpyApi = makeUserApi(ORG_NAME, 'chimpy'); const ws1 = (await chimpyApi.getOrgWorkspaces('current'))[0].id; const docId1 = await chimpyApi.newDoc({name: 'testdoc1'}, ws1); const docId2 = await chimpyApi.newDoc({name: 'testdoc2'}, ws1); @@ -230,11 +255,12 @@ describe('DocApi', function () { return doc1.compareDoc(docId2); } - describe('with APP_HOME_INTERNAL_URL', function () { + describe('specific tests with APP_HOME_INTERNAL_URL', function () { setup('behind-proxy-with-apphomeinternalurl', async () => { // APP_HOME_INTERNAL_URL will be set by TestServer. - ({proxy, home, docs} = await setupServersWithProxy(suitename)); + ({proxy, home, docs} = await setupServersWithProxy(suitename, {withAppHomeInternalUrl: true})); }); + after(() => tearDown(proxy, [home, docs])); it('should succeed to compare docs', async function () { @@ -243,13 +269,14 @@ describe('DocApi', function () { }); }); - describe('without APP_HOME_INTERNAL_URL', function () { + describe('specific tests without APP_HOME_INTERNAL_URL', function () { setup('behind-proxy-without-apphomeinternalurl', async () => { - ({proxy, home, docs} = await setupServersWithProxy(suitename, {APP_HOME_INTERNAL_URL: ''})); + ({proxy, home, docs} = await setupServersWithProxy(suitename, {withAppHomeInternalUrl: false})); }); + after(() => tearDown(proxy, [home, docs])); - it('should succeed to compare docs', async function () { + it('should fail to compare docs', async function () { const promise = testCompareDocs(proxy, home); await assert.isRejected(promise, /TestServerReverseProxy: called public URL/); }); @@ -263,8 +290,8 @@ describe('DocApi', function () { GRIST_DATA_DIR: dataDir }; home = await TestServer.startServer('home', tmpDir, suitename, additionalEnvConfiguration); - docs = await TestServer.startServer('docs', tmpDir, suitename, additionalEnvConfiguration, home.serverUrl); homeUrl = home.serverUrl; + docs = await TestServer.startServer('docs', tmpDir, suitename, additionalEnvConfiguration, homeUrl); serverUrl = docs.serverUrl; hasHomeApi = false; }); @@ -358,7 +385,7 @@ function testDocApi(settings: { const ws1 = (await userApi.getOrgWorkspaces('current'))[0].id; // Make sure kiwi isn't allowed here. await userApi.updateOrgPermissions(ORG_NAME, {users: {[kiwiEmail]: null}}); - const kiwiApi = home.makeUserApi(ORG_NAME, 'kiwi'); + const kiwiApi = makeUserApi(ORG_NAME, 'kiwi'); await assert.isRejected(kiwiApi.getWorkspaceAccess(ws1), /Forbidden/); // Add kiwi as an editor for the org. await assert.isRejected(kiwiApi.getOrgAccess(ORG_NAME), /Forbidden/); @@ -378,7 +405,7 @@ function testDocApi(settings: { const ws1 = (await userApi.getOrgWorkspaces('current'))[0].id; await userApi.updateOrgPermissions(ORG_NAME, {users: {[kiwiEmail]: null}}); // Make sure kiwi isn't allowed here. - const kiwiApi = home.makeUserApi(ORG_NAME, 'kiwi'); + const kiwiApi = makeUserApi(ORG_NAME, 'kiwi'); await assert.isRejected(kiwiApi.getWorkspaceAccess(ws1), /Forbidden/); // Add kiwi as an editor of this workspace. await userApi.updateWorkspacePermissions(ws1, {users: {[kiwiEmail]: 'editors'}}); @@ -397,7 +424,7 @@ function testDocApi(settings: { it("should allow only owners to remove a document", async () => { const ws1 = (await userApi.getOrgWorkspaces('current'))[0].id; const doc1 = await userApi.newDoc({name: 'testdeleteme1'}, ws1); - const kiwiApi = home.makeUserApi(ORG_NAME, 'kiwi'); + const kiwiApi = makeUserApi(ORG_NAME, 'kiwi'); // Kiwi is editor of the document, so he can't delete it. await userApi.updateDocPermissions(doc1, {users: {'kiwi@getgrist.com': 'editors'}}); @@ -413,7 +440,7 @@ function testDocApi(settings: { it("should allow only owners to rename a document", async () => { const ws1 = (await userApi.getOrgWorkspaces('current'))[0].id; const doc1 = await userApi.newDoc({name: 'testrenameme1'}, ws1); - const kiwiApi = home.makeUserApi(ORG_NAME, 'kiwi'); + const kiwiApi = makeUserApi(ORG_NAME, 'kiwi'); // Kiwi is editor of the document, so he can't rename it. await userApi.updateDocPermissions(doc1, {users: {'kiwi@getgrist.com': 'editors'}}); @@ -2931,7 +2958,7 @@ function testDocApi(settings: { }); it('POST /workspaces/{wid}/import handles empty filenames', async function () { - if (!process.env.TEST_REDIS_URL || docs.proxiedServer) { + if (!process.env.TEST_REDIS_URL) { this.skip(); } const worker1 = await userApi.getWorkerAPI('import'); @@ -2985,15 +3012,11 @@ function testDocApi(settings: { }); it("document is protected during upload-and-import sequence", async function () { - if (!process.env.TEST_REDIS_URL || home.proxiedServer) { + if (!process.env.TEST_REDIS_URL) { this.skip(); } // Prepare an API for a different user. - const kiwiApi = new UserAPIImpl(`${homeUrl}/o/Fish`, { - headers: {Authorization: 'Bearer api_key_for_kiwi'}, - fetch: fetch as any, - newFormData: () => new FormData() as any, - }); + const kiwiApi = makeUserApi('Fish', 'kiwi'); // upload something for Chimpy and something else for Kiwi. const worker1 = await userApi.getWorkerAPI('import'); const fakeData1 = await testUtils.readFixtureDoc('Hello.grist'); @@ -3093,15 +3116,10 @@ function testDocApi(settings: { }); it('filters urlIds by org', async function () { - if (home.proxiedServer) { this.skip(); } // Make two documents with same urlId const ws1 = (await userApi.getOrgWorkspaces('current'))[0].id; const doc1 = await userApi.newDoc({name: 'testdoc1', urlId: 'urlid'}, ws1); - const nasaApi = new UserAPIImpl(`${homeUrl}/o/nasa`, { - headers: {Authorization: 'Bearer api_key_for_chimpy'}, - fetch: fetch as any, - newFormData: () => new FormData() as any, - }); + const nasaApi = makeUserApi('nasa', 'chimpy'); const ws2 = (await nasaApi.getOrgWorkspaces('current'))[0].id; const doc2 = await nasaApi.newDoc({name: 'testdoc2', urlId: 'urlid'}, ws2); try { @@ -3126,14 +3144,9 @@ function testDocApi(settings: { it('allows docId access to any document from merged org', async function () { // Make two documents - if (home.proxiedServer) { this.skip(); } const ws1 = (await userApi.getOrgWorkspaces('current'))[0].id; const doc1 = await userApi.newDoc({name: 'testdoc1'}, ws1); - const nasaApi = new UserAPIImpl(`${homeUrl}/o/nasa`, { - headers: {Authorization: 'Bearer api_key_for_chimpy'}, - fetch: fetch as any, - newFormData: () => new FormData() as any, - }); + const nasaApi = makeUserApi('nasa', 'chimpy'); const ws2 = (await nasaApi.getOrgWorkspaces('current'))[0].id; const doc2 = await nasaApi.newDoc({name: 'testdoc2'}, ws2); try { @@ -3260,9 +3273,7 @@ function testDocApi(settings: { // Pass kiwi's headers as it contains both Authorization and Origin headers // if run behind a proxy, so we can ensure that the Origin header check is not made. const userApiServerUrl = docs.proxiedServer ? serverUrl : undefined; - const chimpyApi = home.makeUserApi( - ORG_NAME, 'chimpy', { serverUrl: userApiServerUrl, headers: chimpy.headers as Record } - ); + const chimpyApi = makeUserApi(ORG_NAME, 'chimpy', { baseUrl: userApiServerUrl }); const ws1 = (await chimpyApi.getOrgWorkspaces('current'))[0].id; const docId1 = await chimpyApi.newDoc({name: 'testdoc1'}, ws1); const docId2 = await chimpyApi.newDoc({name: 'testdoc2'}, ws1); @@ -3780,7 +3791,7 @@ function testDocApi(settings: { it("limits daily API usage", async function () { // Make a new document in a test product with a low daily limit - const api = home.makeUserApi('testdailyapilimit'); + const api = makeUserApi('testdailyapilimit', 'chimpy'); const workspaceId = await getWorkspaceId(api, 'TestDailyApiLimitWs'); const docId = await api.newDoc({name: 'TestDoc1'}, workspaceId); const max = testDailyApiLimitFeatures.baseMaxApiUnitsPerDocumentPerDay; @@ -3808,7 +3819,7 @@ function testDocApi(settings: { it("limits daily API usage and sets the correct keys in redis", async function () { this.retries(3); // Make a new document in a free team site, currently the only real product which limits daily API usage. - const freeTeamApi = home.makeUserApi('freeteam'); + const freeTeamApi = makeUserApi('freeteam', 'chimpy'); const workspaceId = await getWorkspaceId(freeTeamApi, 'FreeTeamWs'); const docId = await freeTeamApi.newDoc({name: 'TestDoc2'}, workspaceId); // Rather than making 5000 requests, set high counts directly for the current and next daily and hourly keys @@ -5381,7 +5392,7 @@ function setup(name: string, cb: () => Promise) { await cb(); // create TestDoc as an empty doc into Private workspace - userApi = api = home.makeUserApi(ORG_NAME); + userApi = api = makeUserApi(ORG_NAME, 'chimpy'); const wid = await getWorkspaceId(api, 'Private'); docIds.TestDoc = await api.newDoc({name: 'TestDoc'}, wid); }); diff --git a/test/server/lib/helpers/TestServer.ts b/test/server/lib/helpers/TestServer.ts index 53cba45a96..c35d7cbf5a 100644 --- a/test/server/lib/helpers/TestServer.ts +++ b/test/server/lib/helpers/TestServer.ts @@ -12,7 +12,6 @@ import {delay} from "bluebird"; import fetch from "node-fetch"; import {Writable} from "stream"; import express from "express"; -import { AddressInfo } from "net"; import { isAffirmative } from "app/common/gutil"; import httpProxy from 'http-proxy'; @@ -28,8 +27,8 @@ export class TestServer { _homeUrl?: string, options: {output?: Writable} = {}, // Pipe server output to the given stream ): Promise { - - const server = new this(serverTypes, tempDirectory, suitename); + const port = await getAvailablePort(parseInt(process.env.GET_AVAILABLE_PORT_START || '8080', 10)); + const server = new this(serverTypes, port, tempDirectory, suitename); await server.start(_homeUrl, customEnv, options); return server; } @@ -41,18 +40,23 @@ export class TestServer { if (this._proxiedServer) { throw new Error('Direct access to this test server is disallowed'); } - return this._serverUrl; + + return `http://localhost:${this.port}`; } public get proxiedServer() { return this._proxiedServer; } - private _serverUrl: string; private _server: ChildProcess; private _exitPromise: Promise; private _proxiedServer: boolean = false; private readonly _defaultEnv; - constructor(private _serverTypes: string, public readonly rootDir: string, private _suiteName: string) { + constructor( + private _serverTypes: string, + public readonly port: number, + public readonly rootDir: string, + private _suiteName: string + ) { this._defaultEnv = { GRIST_INST_DIR: this.rootDir, GRIST_DATA_DIR: path.join(this.rootDir, "data"), @@ -66,7 +70,8 @@ export class TestServer { ...process.env }; } - public async start(_homeUrl?: string, customEnv?: NodeJS.ProcessEnv, options: {output?: Writable} = {}) { + + public async start(homeUrl?: string, customEnv?: NodeJS.ProcessEnv, options: {output?: Writable} = {}) { // put node logs into files with meaningful name that relate to the suite name and server type const fixedName = this._serverTypes.replace(/,/, '_'); const nodeLogPath = path.join(this.rootDir, `${this._suiteName}-${fixedName}-node.log`); @@ -79,15 +84,10 @@ export class TestServer { throw new Error(`Path of testingSocket too long: ${this.testingSocket.length} (${this.testingSocket})`); } - const port = await getAvailablePort(Number(process.env.GET_AVAILABLE_PORT_START || '8000')); - this._serverUrl = `http://localhost:${port}`; - const homeUrl = _homeUrl ?? (this._serverTypes.includes('home') ? this._serverUrl : undefined); - const env: NodeJS.ProcessEnv = { APP_HOME_URL: homeUrl, - APP_HOME_INTERNAL_URL: homeUrl, GRIST_TESTING_SOCKET: this.testingSocket, - GRIST_PORT: String(port), + GRIST_PORT: String(this.port), ...this._defaultEnv, ...customEnv }; @@ -115,7 +115,7 @@ export class TestServer { .catch(() => undefined); await this._waitServerReady(); - log.info(`server ${this._serverTypes} up and listening on ${this._serverUrl}`); + log.info(`server ${this._serverTypes} up and listening on ${this.serverUrl}`); } public async stop() { @@ -144,7 +144,7 @@ export class TestServer { this.testingHooks = await connectTestingHooks(this.testingSocket); // wait for check - return (await fetch(`${this._serverUrl}/status/hooks`, {timeout: 1000})).ok; + return (await fetch(`${this.serverUrl}/status/hooks`, {timeout: 1000})).ok; } catch (err) { log.warn("Failed to initialize server", err); return false; @@ -157,19 +157,9 @@ export class TestServer { // Returns the promise for the ChildProcess's signal or exit code. public getExitPromise(): Promise { return this._exitPromise; } - public makeUserApi( - org: string, - user: string = 'chimpy', - { - headers = {Authorization: `Bearer api_key_for_${user}`}, - serverUrl = this._serverUrl, - }: { - headers?: Record - serverUrl?: string, - } = { headers: undefined, serverUrl: undefined }, - ): UserAPIImpl { - return new UserAPIImpl(`${serverUrl}/o/${org}`, { - headers, + public makeUserApi(org: string, user: string = 'chimpy'): UserAPIImpl { + return new UserAPIImpl(`${this.serverUrl}/o/${org}`, { + headers: {Authorization: `Bearer api_key_for_${user}`}, fetch: fetch as unknown as typeof globalThis.fetch, newFormData: () => new FormData() as any, }); @@ -204,6 +194,8 @@ export class TestServer { } } +const FROM_OUTSIDE_HEADER_KEY = "X-FROM-OUTSIDE"; + /** * Creates a reverse-proxy for a home and a doc worker. * @@ -212,7 +204,7 @@ export class TestServer { * * You may use it like follow: * ```ts - * const proxy = new TestServerReverseProxy(); + * const proxy = await TestServerReverseProxy.build(); * // Create here a doc and a home workers with their env variables * proxy.requireFromOutsideHeader(); // Optional * await proxy.start(home, docs); @@ -226,23 +218,24 @@ export class TestServerReverseProxy { // https://github.com/gristlabs/grist-core/blob/24b39c651b9590cc360cc91b587d3e1b301a9c63/app/server/lib/requestUtils.ts#L85-L98 public static readonly HOSTNAME: string = 'grist-test-proxy.127.0.0.1.nip.io'; - public static FROM_OUTSIDE_HEADER = {"X-FROM-OUTSIDE": true}; + public static FROM_OUTSIDE_HEADER = {[FROM_OUTSIDE_HEADER_KEY]: true}; + + public static async build() { + const port = await getAvailablePort(parseInt(process.env.GET_AVAILABLE_PORT_START || '8080', 10)); + return new this(port); + } + + public get serverUrl() { return `http://${TestServerReverseProxy.HOSTNAME}:${this.port}`; } private _app = express(); private _proxyServer: http.Server; private _proxy: httpProxy = httpProxy.createProxy(); - private _address: Promise; private _requireFromOutsideHeader = false; public get stopped() { return !this._proxyServer.listening; } - public constructor() { - this._proxyServer = this._app.listen(0); - this._address = new Promise((resolve) => { - this._proxyServer.once('listening', () => { - resolve(this._proxyServer.address() as AddressInfo); - }); - }); + public constructor(public readonly port: number) { + this._proxyServer = this._app.listen(port); } /** @@ -257,8 +250,8 @@ export class TestServerReverseProxy { this._requireFromOutsideHeader = true; } - public async start(homeServer: TestServer, docServer: TestServer) { - this._app.all(['/dw/dw1', '/dw/dw1/*'], (oreq, ores) => this._getRequestHandlerFor(docServer)); + public start(homeServer: TestServer, docServer: TestServer) { + this._app.all(['/dw/dw1', '/dw/dw1/*'], this._getRequestHandlerFor(docServer)); this._app.all('/*', this._getRequestHandlerFor(homeServer)); // Forbid now the use of serverUrl property, so we don't allow the tests to @@ -266,16 +259,7 @@ export class TestServerReverseProxy { homeServer.disallowDirectAccess(); docServer.disallowDirectAccess(); - log.info('proxy server running on ', await this.getServerUrl()); - } - - public async getAddress() { - return this._address; - } - - public async getServerUrl() { - const address = await this.getAddress(); - return `http://${TestServerReverseProxy.HOSTNAME}:${address.port}`; + log.info('proxy server running on ', this.serverUrl); } public stop() { @@ -294,7 +278,7 @@ export class TestServerReverseProxy { log.debug(`[proxy] Requesting (method=${oreq.method}): ${new URL(oreq.url, serverUrl).href}`); // See the requireFromOutsideHeader() method for the explanation - if (this._requireFromOutsideHeader && !isAffirmative(oreq.get("X-FROM-OUTSIDE"))) { + if (this._requireFromOutsideHeader && !isAffirmative(oreq.get(FROM_OUTSIDE_HEADER_KEY))) { log.error('TestServerReverseProxy: called public URL from internal'); return ores.status(403).json({error: "TestServerReverseProxy: called public URL from internal "}); }