(core) Fix bug forcing login on some form URLs

Summary:
Login (and other) middleware was included in the public form URL by mistake,
forcing logins on forms hosted on non-personal sites.

Test Plan: Browser test.

Reviewers: paulfitz

Reviewed By: paulfitz

Differential Revision: https://phab.getgrist.com/D4181
pull/841/head
George Gevoian 5 months ago
parent 716144ed46
commit cb298e63d4

@ -26,8 +26,9 @@ import {ISendAppPageOptions} from 'app/server/lib/sendAppPage';
export interface AttachOptions { export interface AttachOptions {
app: express.Application; // Express app to which to add endpoints app: express.Application; // Express app to which to add endpoints
middleware: express.RequestHandler[]; // Middleware to apply for all endpoints except docs middleware: express.RequestHandler[]; // Middleware to apply for all endpoints except docs and forms
docMiddleware: express.RequestHandler[]; // Middleware to apply for doc landing pages docMiddleware: express.RequestHandler[]; // Middleware to apply for doc landing pages
formMiddleware: express.RequestHandler[]; // Middleware to apply for form landing pages
forceLogin: express.RequestHandler|null; // Method to force user to login (if logins are possible) forceLogin: express.RequestHandler|null; // Method to force user to login (if logins are possible)
docWorkerMap: IDocWorkerMap|null; docWorkerMap: IDocWorkerMap|null;
sendAppPage: (req: express.Request, resp: express.Response, options: ISendAppPageOptions) => Promise<void>; sendAppPage: (req: express.Request, resp: express.Response, options: ISendAppPageOptions) => Promise<void>;
@ -37,8 +38,8 @@ export interface AttachOptions {
} }
export function attachAppEndpoint(options: AttachOptions): void { export function attachAppEndpoint(options: AttachOptions): void {
const {app, middleware, docMiddleware, docWorkerMap, forceLogin, const {app, middleware, docMiddleware, formMiddleware, docWorkerMap,
sendAppPage, dbManager, plugins, gristServer} = options; forceLogin, sendAppPage, dbManager, plugins, gristServer} = options;
// Per-workspace URLs open the same old Home page, and it's up to the client to notice and // Per-workspace URLs open the same old Home page, and it's up to the client to notice and
// render the right workspace. // render the right workspace.
app.get(['/', '/ws/:wsId', '/p/:page'], ...middleware, expressWrap(async (req, res) => app.get(['/', '/ws/:wsId', '/p/:page'], ...middleware, expressWrap(async (req, res) =>
@ -226,7 +227,7 @@ export function attachAppEndpoint(options: AttachOptions): void {
...docMiddleware, docHandler); ...docMiddleware, docHandler);
app.get('/:urlId([^-/]{12,})(/:slug([^/]+):remainder(*))?', app.get('/:urlId([^-/]{12,})(/:slug([^/]+):remainder(*))?',
...docMiddleware, docHandler); ...docMiddleware, docHandler);
app.get('/forms/:urlId([^/]+)/:sectionId', ...middleware, expressWrap(async (req, res) => { app.get('/forms/:urlId([^/]+)/:sectionId', ...formMiddleware, expressWrap(async (req, res) => {
const formUrl = gristServer.getHomeUrl(req, const formUrl = gristServer.getHomeUrl(req,
`/api/s/${req.params.urlId}/forms/${req.params.sectionId}`); `/api/s/${req.params.urlId}/forms/${req.params.sectionId}`);
const response = await fetch(formUrl, { const response = await fetch(formUrl, {

@ -1036,6 +1036,9 @@ export class FlexServer implements GristServer {
this._redirectToOrgMiddleware, this._redirectToOrgMiddleware,
welcomeNewUser welcomeNewUser
], ],
formMiddleware: [
forcedLoginMiddleware,
],
forceLogin: this._redirectToLoginUnconditionally, forceLogin: this._redirectToLoginUnconditionally,
docWorkerMap: isSingleUserMode() ? null : this._docWorkerMap, docWorkerMap: isSingleUserMode() ? null : this._docWorkerMap,
sendAppPage: this._sendAppPage, sendAppPage: this._sendAppPage,

@ -21,17 +21,6 @@ describe('FormView', function() {
afterEach(() => gu.checkForErrors()); afterEach(() => gu.checkForErrors());
before(async function() {
const session = await gu.session().login();
docId = await session.tempNewDoc(cleanup);
api = session.createHomeApi();
await driver.executeScript(createClipboardTextArea);
});
after(async function() {
await driver.executeScript(removeClipboardTextArea);
});
/** /**
* Adds a temporary textarea to the document for pasting the contents of * Adds a temporary textarea to the document for pasting the contents of
* the clipboard. * the clipboard.
@ -123,6 +112,18 @@ describe('FormView', function() {
assert.deepEqual(await api.getTable(docId, 'Table1').then(t => t.D), values); assert.deepEqual(await api.getTable(docId, 'Table1').then(t => t.D), values);
} }
describe('on personal site', async function() {
before(async function() {
const session = await gu.session().login();
docId = await session.tempNewDoc(cleanup);
api = session.createHomeApi();
await driver.executeScript(createClipboardTextArea);
});
after(async function() {
await driver.executeScript(removeClipboardTextArea);
});
it('updates creator panel when navigated away', async function() { it('updates creator panel when navigated away', async function() {
// Add 2 new pages. // Add 2 new pages.
await gu.addNewPage('Form', 'New Table', {tableName: 'TabA'}); await gu.addNewPage('Form', 'New Table', {tableName: 'TabA'});
@ -1189,6 +1190,39 @@ describe('FormView', function() {
assert.deepEqual(await readLabels(), ['A', 'B', 'C']); assert.deepEqual(await readLabels(), ['A', 'B', 'C']);
}); });
}); });
});
describe('on team site', async function() {
before(async function() {
const session = await gu.session().teamSite.login();
docId = await session.tempNewDoc(cleanup);
api = session.createHomeApi();
await driver.executeScript(createClipboardTextArea);
});
after(async function() {
await driver.executeScript(removeClipboardTextArea);
});
it('can submit a form', async function() {
// A bug was preventing this by forcing a login redirect from the public form URL.
const formUrl = await createFormWith('Text');
await gu.removeLogin();
// We are in a new window.
await gu.onNewTab(async () => {
await driver.get(formUrl);
await driver.findWait('input[name="D"]', 1000).click();
await gu.sendKeys('Hello World');
await driver.find('input[type="submit"]').click();
await waitForConfirm();
});
// Make sure we see the new record.
const session = await gu.session().teamSite.login();
await session.loadDoc(`/doc/${docId}`);
await expectSingle('Hello World');
await removeForm();
});
});
}); });
function element(type: string, parent?: WebElement): ExtraElement; function element(type: string, parent?: WebElement): ExtraElement;

Loading…
Cancel
Save