Skip to content

Commit

Permalink
Allow URLs with only a docID #768 (#771)
Browse files Browse the repository at this point in the history
Co-authored-by: Florent FAYOLLE <[email protected]>
  • Loading branch information
fflorent and Florent FAYOLLE authored Nov 29, 2023
1 parent de13a2f commit cf0cbb4
Show file tree
Hide file tree
Showing 12 changed files with 55 additions and 49 deletions.
4 changes: 2 additions & 2 deletions app/client/models/AppModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export type PageType =
| "billing"
| "welcome"
| "account"
| "support-grist"
| "support"
| "activation";

const G = getBrowserGlobals('document', 'window');
Expand Down Expand Up @@ -316,7 +316,7 @@ export class AppModelImpl extends Disposable implements AppModel {
} else if (state.account) {
return 'account';
} else if (state.supportGrist) {
return 'support-grist';
return 'support';
} else if (state.activation) {
return 'activation';
} else {
Expand Down
2 changes: 1 addition & 1 deletion app/client/ui/AccountWidget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ export class AccountWidget extends Disposable {
return menuItemLink(
t('Support Grist'),
cssHeartIcon('💛'),
urlState().setLinkUrl({supportGrist: 'support-grist'}),
urlState().setLinkUrl({supportGrist: 'support'}),
testId('usermenu-support-grist'),
);
}
Expand Down
2 changes: 1 addition & 1 deletion app/client/ui/AppUI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ function createMainPage(appModel: AppModel, appObj: App) {
return dom.create(WelcomePage, appModel);
} else if (pageType === 'account') {
return domAsync(loadAccountPage().then(ap => dom.create(ap.AccountPage, appModel)));
} else if (pageType === 'support-grist') {
} else if (pageType === 'support') {
return domAsync(loadSupportGristPage().then(sgp => dom.create(sgp.SupportGristPage, appModel)));
} else if (pageType === 'activation') {
return domAsync(loadActivationPage().then(ap => dom.create(ap.ActivationPage, appModel)));
Expand Down
8 changes: 4 additions & 4 deletions app/client/ui/SupportGristNudge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ type ButtonState =
| 'expanded';

type CardPage =
| 'support-grist'
| 'support'
| 'opted-in';

/**
Expand All @@ -45,7 +45,7 @@ export class SupportGristNudge extends Disposable {
this._buttonState = localStorageObs(
`u=${this._appModel.currentValidUser?.id ?? 0};supportGristNudge`, 'expanded'
) as Observable<ButtonState>;
this._currentPage = Observable.create(null, 'support-grist');
this._currentPage = Observable.create(null, 'support');
this._isClosed = Observable.create(this, false);
}

Expand Down Expand Up @@ -122,7 +122,7 @@ export class SupportGristNudge extends Disposable {
private _buildCard() {
return cssCard(
dom.domComputed(this._currentPage, page => {
if (page === 'support-grist') {
if (page === 'support') {
return this._buildSupportGristCardContent();
} else {
return this._buildOptedInCardContent();
Expand Down Expand Up @@ -205,7 +205,7 @@ function helpCenterLink() {
function supportGristLink() {
return cssLink(
t('Support Grist page'),
{href: urlState().makeUrl({supportGrist: 'support-grist'}), target: '_blank'},
{href: urlState().makeUrl({supportGrist: 'support'}), target: '_blank'},
);
}

Expand Down
2 changes: 1 addition & 1 deletion app/client/ui/SupportGristPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ export class SupportGristPage extends Disposable {
const suffix = getPageTitleSuffix(getGristConfig());
switch (page) {
case undefined:
case 'support-grist': {
case 'support': {
return document.title = `Support Grist${suffix}`;
}
}
Expand Down
6 changes: 3 additions & 3 deletions app/common/gristUrls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export type ActivationPage = typeof ActivationPage.type;
export const LoginPage = StringUnion('signup', 'login', 'verified', 'forgot-password');
export type LoginPage = typeof LoginPage.type;

export const SupportGristPage = StringUnion('support-grist');
export const SupportGristPage = StringUnion('support');
export type SupportGristPage = typeof SupportGristPage.type;

// Overall UI style. "full" is normal, "singlePage" is a single page focused, panels hidden experience.
Expand Down Expand Up @@ -408,8 +408,8 @@ export function decodeUrl(gristConfig: Partial<GristLoadConfig>, location: Locat
state.activation = ActivationPage.parse(map.get('activation')) || 'activation';
}
if (map.has('welcome')) { state.welcome = WelcomePage.parse(map.get('welcome')); }
if (map.has('support-grist')) {
state.supportGrist = SupportGristPage.parse(map.get('support-grist')) || 'support-grist';
if (map.has('support')) {
state.supportGrist = SupportGristPage.parse(map.get('support')) || 'support';
}
if (sp.has('planType')) { state.params!.planType = sp.get('planType')!; }
if (sp.has('billingPlan')) { state.params!.billingPlan = sp.get('billingPlan')!; }
Expand Down
2 changes: 1 addition & 1 deletion app/server/lib/AppEndpoint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,6 @@ export function attachAppEndpoint(options: AttachOptions): void {
// The * is a wildcard in express 4, rather than a regex symbol.
// See https://expressjs.com/en/guide/routing.html
app.get('/doc/:urlId([^/]+):remainder(*)', ...docMiddleware, docHandler);
app.get('/:urlId([^/]{12,})/:slug([^/]+):remainder(*)',
app.get('/:urlId([^/]{12,})(/:slug([^/]+):remainder(*))?',
...docMiddleware, docHandler);
}
2 changes: 1 addition & 1 deletion app/server/lib/Telemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ export class Telemetry implements ITelemetry {

public addPages(app: express.Application, middleware: express.RequestHandler[]) {
if (this._deploymentType === 'core') {
app.get('/support-grist', ...middleware, expressWrap(async (req, resp) => {
app.get('/support', ...middleware, expressWrap(async (req, resp) => {
return this._gristServer.sendAppPage(req, resp,
{path: 'app.html', status: 200, config: {}});
}));
Expand Down
2 changes: 1 addition & 1 deletion test/gen-server/seed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ class Seed {
const d = new Document();
d.name = doc;
d.workspace = w;
d.id = `sample_${docId}`;
d.id = `sampledocid_${docId}`;
docId++;
await d.save();
const dgrps = await this.createGroups(w);
Expand Down
58 changes: 32 additions & 26 deletions test/server/lib/Authorizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,48 +101,54 @@ describe('Authorizer', function() {
it.skip("viewer gets redirect by title", async function() {
const resp = await axios.get(`${serverUrl}/o/pr/doc/Bananas`, chimpy);
assert.equal(resp.status, 200);
assert.equal(getGristConfig(resp.data).assignmentId, 'sample_6');
assert.match(resp.request.res.responseUrl, /\/doc\/sample_6$/);
assert.equal(getGristConfig(resp.data).assignmentId, 'sampledocid_6');
assert.match(resp.request.res.responseUrl, /\/doc\/sampledocid_6$/);
const resp2 = await axios.get(`${serverUrl}/o/nasa/doc/Pluto`, chimpy);
assert.equal(resp2.status, 200);
assert.equal(getGristConfig(resp2.data).assignmentId, 'sample_2');
assert.match(resp2.request.res.responseUrl, /\/doc\/sample_2$/);
assert.equal(getGristConfig(resp2.data).assignmentId, 'sampledocid_2');
assert.match(resp2.request.res.responseUrl, /\/doc\/sampledocid_2$/);
});

it('viewer loads document without slug in the URL', async function () {
const docId = docs.Bananas.id;
const resp = await axios.get(`${serverUrl}/o/pr/${docId}`, chimpy);
assert.equal(resp.status, 200);
});

it("stranger gets consistent refusal regardless of title", async function() {
const resp = await axios.get(`${serverUrl}/o/pr/doc/Bananas`, charon);
assert.equal(resp.status, 404);
assert.notMatch(resp.data, /sample_6/);
assert.notMatch(resp.data, /sampledocid_6/);
const resp2 = await axios.get(`${serverUrl}/o/pr/doc/Bananas2`, charon);
assert.equal(resp2.status, 404);
assert.notMatch(resp.data, /sample_6/);
assert.notMatch(resp.data, /sampledocid_6/);
assert.deepEqual(withoutTimestamp(resp.data),
withoutTimestamp(resp2.data));
});

it("viewer can access title", async function() {
const resp = await axios.get(`${serverUrl}/o/pr/doc/sample_6`, chimpy);
const resp = await axios.get(`${serverUrl}/o/pr/doc/sampledocid_6`, chimpy);
assert.equal(resp.status, 200);
const config = getGristConfig(resp.data);
assert.equal(config.getDoc![config.assignmentId!].name, 'Bananas');
});

it("stranger cannot access title", async function() {
const resp = await axios.get(`${serverUrl}/o/pr/doc/sample_6`, charon);
const resp = await axios.get(`${serverUrl}/o/pr/doc/sampledocid_6`, charon);
assert.equal(resp.status, 403);
assert.notMatch(resp.data, /Bananas/);
});

it("viewer cannot access document from wrong org", async function() {
const resp = await axios.get(`${serverUrl}/o/nasa/doc/sample_6`, chimpy);
const resp = await axios.get(`${serverUrl}/o/nasa/doc/sampledocid_6`, chimpy);
assert.equal(resp.status, 404);
});

it("websocket allows openDoc for viewer", async function() {
const cli = await openClient(server, '[email protected]', 'pr');
cli.ignoreTrivialActions();
assert.equal((await cli.readMessage()).type, 'clientConnect');
const openDoc = await cli.send("openDoc", "sample_6");
const openDoc = await cli.send("openDoc", "sampledocid_6");
assert.equal(openDoc.error, undefined);
assert.match(JSON.stringify(openDoc.data), /Table1/);
await cli.close();
Expand All @@ -152,7 +158,7 @@ describe('Authorizer', function() {
const cli = await openClient(server, '[email protected]', 'pr');
cli.ignoreTrivialActions();
assert.equal((await cli.readMessage()).type, 'clientConnect');
const openDoc = await cli.send("openDoc", "sample_6");
const openDoc = await cli.send("openDoc", "sampledocid_6");
assert.match(openDoc.error!, /No view access/);
assert.equal(openDoc.data, undefined);
assert.match(openDoc.errorCode!, /AUTH_NO_VIEW/);
Expand All @@ -163,7 +169,7 @@ describe('Authorizer', function() {
const cli = await openClient(server, '[email protected]', 'nasa');
cli.ignoreTrivialActions();
assert.equal((await cli.readMessage()).type, 'clientConnect');
const openDoc = await cli.openDocOnConnect("sample_2");
const openDoc = await cli.openDocOnConnect("sampledocid_2");
assert.equal(openDoc.error, undefined);
const nonce = uuidv4();
const applyUserActions = await cli.send("applyUserActions",
Expand All @@ -182,7 +188,7 @@ describe('Authorizer', function() {
const cli = await openClient(server, '[email protected]', 'nasa');
cli.ignoreTrivialActions();
assert.equal((await cli.readMessage()).type, 'clientConnect');
const openDoc = await cli.openDocOnConnect("sample_2");
const openDoc = await cli.openDocOnConnect("sampledocid_2");
assert.equal(openDoc.error, undefined);
const nonce = uuidv4();
const applyUserActions = await cli.send("applyUserActions",
Expand All @@ -209,9 +215,9 @@ describe('Authorizer', function() {
editor.ignoreTrivialActions();
viewer.ignoreTrivialActions();
stranger.ignoreTrivialActions();
assert.equal((await editor.send("openDoc", "sample_2")).error, undefined);
assert.equal((await viewer.send("openDoc", "sample_2")).error, undefined);
assert.match((await stranger.send("openDoc", "sample_2")).error!, /No view access/);
assert.equal((await editor.send("openDoc", "sampledocid_2")).error, undefined);
assert.equal((await viewer.send("openDoc", "sampledocid_2")).error, undefined);
assert.match((await stranger.send("openDoc", "sampledocid_2")).error!, /No view access/);

const action = [0, [["UpdateRecord", "Table1", 1, {A: "foo"}]]];
assert.equal((await editor.send("applyUserActions", ...action)).error, undefined);
Expand All @@ -224,7 +230,7 @@ describe('Authorizer', function() {
const cli = await openClient(server, '[email protected]', 'nasa');
cli.ignoreTrivialActions();
assert.equal((await cli.readMessage()).type, 'clientConnect');
const openDoc = await cli.send("openDoc", "sample_2");
const openDoc = await cli.send("openDoc", "sampledocid_2");
assert.equal(openDoc.error, undefined);
const nonce = uuidv4();
const applyUserActions = await cli.send("applyUserActions",
Expand All @@ -243,12 +249,12 @@ describe('Authorizer', function() {
const cli = await openClient(server, '[email protected]', 'nasa');
cli.ignoreTrivialActions();
assert.equal((await cli.readMessage()).type, 'clientConnect');
const openDoc = await cli.send("openDoc", "sample_2");
const openDoc = await cli.send("openDoc", "sampledocid_2");
assert.equal(openDoc.error, undefined);
const result = await cli.send("fork", 0);
assert.equal(result.data.docId, result.data.urlId);
const parts = parseUrlId(result.data.docId);
assert.equal(parts.trunkId, "sample_2");
assert.equal(parts.trunkId, "sampledocid_2");
assert.isAbove(parts.forkId!.length, 4);
assert.equal(parts.forkUserId, await dbManager.testGetId('Charon') as number);
});
Expand All @@ -258,31 +264,31 @@ describe('Authorizer', function() {
const cli = await openClient(server, '[email protected]', 'nasa');
cli.ignoreTrivialActions();
assert.equal((await cli.readMessage()).type, 'clientConnect');
let openDoc = await cli.send("openDoc", "sample_2");
let openDoc = await cli.send("openDoc", "sampledocid_2");
assert.match(openDoc.error!, /No view access/);

// grant anon access to doc and retry
await dbManager.updateDocPermissions({
userId: await dbManager.testGetId('Chimpy') as number,
urlId: 'sample_2',
urlId: 'sampledocid_2',
org: 'nasa'
}, {users: {"[email protected]": "viewers"}});
dbManager.flushDocAuthCache();
openDoc = await cli.send("openDoc", "sample_2");
openDoc = await cli.send("openDoc", "sampledocid_2");
assert.equal(openDoc.error, undefined);

// make a fork
const result = await cli.send("fork", 0);
assert.equal(result.data.docId, result.data.urlId);
const parts = parseUrlId(result.data.docId);
assert.equal(parts.trunkId, "sample_2");
assert.equal(parts.trunkId, "sampledocid_2");
assert.isAbove(parts.forkId!.length, 4);
assert.equal(parts.forkUserId, undefined);
});

it("can set user via GRIST_PROXY_AUTH_HEADER", async function() {
// User can access a doc by setting header.
const docUrl = `${serverUrl}/o/pr/api/docs/sample_6`;
const docUrl = `${serverUrl}/o/pr/api/docs/sampledocid_6`;
const resp = await axios.get(docUrl, {
headers: {'X-email': '[email protected]'}
});
Expand All @@ -297,7 +303,7 @@ describe('Authorizer', function() {
let cli = await openClient(server, '[email protected]', 'pr', 'X-email');
cli.ignoreTrivialActions();
assert.equal((await cli.readMessage()).type, 'clientConnect');
let openDoc = await cli.send("openDoc", "sample_6");
let openDoc = await cli.send("openDoc", "sampledocid_6");
assert.equal(openDoc.error, undefined);
assert.match(JSON.stringify(openDoc.data), /Table1/);
await cli.close();
Expand All @@ -306,7 +312,7 @@ describe('Authorizer', function() {
cli = await openClient(server, '[email protected]', 'pr', 'X-email');
cli.ignoreTrivialActions();
assert.equal((await cli.readMessage()).type, 'clientConnect');
openDoc = await cli.send("openDoc", "sample_6");
openDoc = await cli.send("openDoc", "sampledocid_6");
assert.match(openDoc.error!, /No view access/);
assert.equal(openDoc.data, undefined);
assert.match(openDoc.errorCode!, /AUTH_NO_VIEW/);
Expand Down
8 changes: 4 additions & 4 deletions test/server/lib/DocApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@ const support = configForUser('support');

// some doc ids
const docIds: { [name: string]: string } = {
ApiDataRecordsTest: 'sample_7',
Timesheets: 'sample_13',
Bananas: 'sample_6',
Antartic: 'sample_11'
ApiDataRecordsTest: 'sampledocid_7',
Timesheets: 'sampledocid_13',
Bananas: 'sampledocid_6',
Antartic: 'sampledocid_11'
};

// A testDir of the form grist_test_{USER}_{SERVER_NAME}
Expand Down
8 changes: 4 additions & 4 deletions test/server/lib/Webhooks-Proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ const chimpy = configForUser('Chimpy');

// some doc ids
const docIds: { [name: string]: string } = {
ApiDataRecordsTest: 'sample_7',
Timesheets: 'sample_13',
Bananas: 'sample_6',
Antartic: 'sample_11'
ApiDataRecordsTest: 'sampledocid_7',
Timesheets: 'sampledocid_13',
Bananas: 'sampledocid_6',
Antartic: 'sampledocid_11',
};

let dataDir: string;
Expand Down

0 comments on commit cf0cbb4

Please sign in to comment.