Add ws id and doc name params to POST /docs (#655)

This commit is contained in:
George Gevoian
2023-09-05 14:27:35 -04:00
committed by GitHub
parent b9b0632be8
commit 90fb4434cc
16 changed files with 310 additions and 83 deletions

View File

@@ -492,7 +492,7 @@ export class ApiServer {
// Sets active user for active org
this._app.post('/api/session/access/active', expressWrap(async (req, res) => {
const mreq = req as RequestWithLogin;
let domain = optStringParam(req.body.org);
let domain = optStringParam(req.body.org, 'org');
if (!domain || domain === 'current') {
domain = getOrgFromRequest(mreq) || '';
}

View File

@@ -1807,6 +1807,9 @@ export class HomeDBManager extends EventEmitter {
return queryResult;
}
const workspace: Workspace = queryResult.data;
if (workspace.removedAt) {
throw new ApiError('Cannot add document to a deleted workspace', 400);
}
await this._checkRoomForAnotherDoc(workspace, manager);
// Create a new document.
const doc = new Document();

View File

@@ -260,7 +260,7 @@ export class Housekeeper {
// which worker group the document is assigned a worker from.
app.post('/api/housekeeping/docs/:docId/assign', this._withSupport(async (req, docId, headers) => {
const url = new URL(await this._server.getHomeUrlByDocId(docId, `/api/docs/${docId}/assign`));
const group = optStringParam(req.query.group);
const group = optStringParam(req.query.group, 'group');
if (group !== undefined) { url.searchParams.set('group', group); }
return fetch(url.toString(), {
method: 'POST',

View File

@@ -165,7 +165,7 @@ export async function addRequestUser(dbManager: HomeDBManager, permitStore: IPer
// Support providing an access token via an `auth` query parameter.
// This is useful for letting the browser load assets like image
// attachments.
const auth = optStringParam(mreq.query.auth);
const auth = optStringParam(mreq.query.auth, 'auth');
if (auth) {
const tokens = options.gristServer.getAccessTokens();
const token = await tokens.verify(auth);
@@ -310,7 +310,8 @@ export async function addRequestUser(dbManager: HomeDBManager, permitStore: IPer
// See if we have a profile linked with the active organization already.
// TODO: implement userSelector for rest API, to allow "sticky" user selection on pages.
let sessionUser: SessionUserObj|null = getSessionUser(session, mreq.org, optStringParam(mreq.query.user) || '');
let sessionUser: SessionUserObj|null = getSessionUser(session, mreq.org,
optStringParam(mreq.query.user, 'user') || '');
if (!sessionUser) {
// No profile linked yet, so let's elect one.

View File

@@ -440,9 +440,9 @@ export class DocWorkerApi {
// Responds with attachment contents, with suitable Content-Type and Content-Disposition.
this._app.get('/api/docs/:docId/attachments/:attId/download', canView, withDoc(async (activeDoc, req, res) => {
const attId = integerParam(req.params.attId, 'attId');
const tableId = optStringParam(req.params.tableId);
const colId = optStringParam(req.params.colId);
const rowId = optIntegerParam(req.params.rowId);
const tableId = optStringParam(req.params.tableId, 'tableId');
const colId = optStringParam(req.params.colId, 'colId');
const rowId = optIntegerParam(req.params.rowId, 'rowId');
if ((tableId || colId || rowId) && !(tableId && colId && rowId)) {
throw new ApiError('define all of tableId, colId and rowId, or none.', 400);
}
@@ -720,8 +720,9 @@ export class DocWorkerApi {
const options = {
add: !isAffirmative(req.query.noadd),
update: !isAffirmative(req.query.noupdate),
onMany: stringParam(req.query.onmany || "first", "onmany",
["first", "none", "all"]) as 'first'|'none'|'all'|undefined,
onMany: stringParam(req.query.onmany || "first", "onmany", {
allowed: ["first", "none", "all"],
}) as 'first'|'none'|'all'|undefined,
allowEmptyRequire: isAffirmative(req.query.allow_empty_require),
};
await ops.upsert(body.records, options);
@@ -982,7 +983,7 @@ export class DocWorkerApi {
// (Requires a special permit.)
this._app.post('/api/docs/:docId/assign', canEdit, throttled(async (req, res) => {
const docId = getDocId(req);
const group = optStringParam(req.query.group);
const group = optStringParam(req.query.group, 'group');
if (group !== undefined && req.specialPermit?.action === 'assign-doc') {
if (group.trim() === '') {
await this._docWorkerMap.removeDocGroup(docId);
@@ -1148,7 +1149,12 @@ export class DocWorkerApi {
const userId = getUserId(req);
const wsId = integerParam(req.params.wid, 'wid');
const uploadId = integerParam(req.body.uploadId, 'uploadId');
const result = await this._docManager.importDocToWorkspace(userId, uploadId, wsId, req.body.browserSettings);
const result = await this._docManager.importDocToWorkspace({
userId,
uploadId,
workspaceId: wsId,
browserSettings: req.body.browserSettings,
});
res.json(result);
}));
@@ -1215,16 +1221,22 @@ export class DocWorkerApi {
})
);
// Create a document. When an upload is included, it is imported as the initial
// state of the document. Otherwise a fresh empty document is created.
// A "timezone" option can be supplied.
// Documents are created "unsaved".
// TODO: support workspaceId option for creating regular documents, at which point
// existing import endpoint and doc creation endpoint can share implementation
// with this.
// Returns the id of the created document.
/**
* Create a document.
*
* When an upload is included, it is imported as the initial state of the document.
* Otherwise, the document is left empty.
*
* If a workspace id is included, the document will be saved there instead of
* being left "unsaved".
*
* Returns the id of the created document.
*
* TODO: unify this with the other document creation and import endpoints.
*/
this._app.post('/api/docs', expressWrap(async (req, res) => {
const userId = getUserId(req);
let uploadId: number|undefined;
let parameters: {[key: string]: any};
if (req.is('multipart/form-data')) {
@@ -1236,22 +1248,52 @@ export class DocWorkerApi {
} else {
parameters = req.body;
}
if (parameters.workspaceId) { throw new Error('workspaceId not supported'); }
const documentName = optStringParam(parameters.documentName, 'documentName', {
allowEmpty: false,
});
const workspaceId = optIntegerParam(parameters.workspaceId, 'workspaceId');
const browserSettings: BrowserSettings = {};
if (parameters.timezone) { browserSettings.timezone = parameters.timezone; }
browserSettings.locale = localeFromRequest(req);
let docId: string;
if (uploadId !== undefined) {
const result = await this._docManager.importDocToWorkspace(userId, uploadId, null,
browserSettings);
return res.json(result.id);
const result = await this._docManager.importDocToWorkspace({
userId,
uploadId,
documentName,
workspaceId,
browserSettings,
});
docId = result.id;
} else if (workspaceId !== undefined) {
const {status, data, errMessage} = await this._dbManager.addDocument(getScope(req), workspaceId, {
name: documentName ?? 'Untitled document',
});
if (status !== 200) {
throw new ApiError(errMessage || 'unable to create document', status);
}
docId = data!;
} else {
const isAnonymous = isAnonymousUser(req);
const result = makeForkIds({
userId,
isAnonymous,
trunkDocId: NEW_DOCUMENT_CODE,
trunkUrlId: NEW_DOCUMENT_CODE,
});
docId = result.docId;
await this._docManager.createNamedDoc(
makeExceptionalDocSession('nascent', {
req: req as RequestWithLogin,
browserSettings,
}),
docId
);
}
const isAnonymous = isAnonymousUser(req);
const {docId} = makeForkIds({userId, isAnonymous, trunkDocId: NEW_DOCUMENT_CODE,
trunkUrlId: NEW_DOCUMENT_CODE});
await this._docManager.createNamedDoc(makeExceptionalDocSession('nascent', {
req: req as RequestWithLogin,
browserSettings
}), docId);
return res.status(200).json(docId);
}));
}
@@ -1556,7 +1598,7 @@ export class DocWorkerApi {
}
const timeout =
Math.max(0, Math.min(MAX_CUSTOM_SQL_MSEC,
optIntegerParam(options.timeout) || MAX_CUSTOM_SQL_MSEC));
optIntegerParam(options.timeout, 'timeout') || MAX_CUSTOM_SQL_MSEC));
// Wrap in a select to commit to the SELECT branch of SQLite
// grammar. Note ; isn't a problem.
//
@@ -1639,7 +1681,7 @@ export interface QueryParameters {
* as a header.
*/
function getSortParameter(req: Request): string[]|undefined {
const sortString: string|undefined = optStringParam(req.query.sort) || req.get('X-Sort');
const sortString: string|undefined = optStringParam(req.query.sort, 'sort') || req.get('X-Sort');
if (!sortString) { return undefined; }
return sortString.split(',');
}
@@ -1650,7 +1692,7 @@ function getSortParameter(req: Request): string[]|undefined {
* parameter, or as a header.
*/
function getLimitParameter(req: Request): number|undefined {
const limitString: string|undefined = optStringParam(req.query.limit) || req.get('X-Limit');
const limitString: string|undefined = optStringParam(req.query.limit, 'limit') || req.get('X-Limit');
if (!limitString) { return undefined; }
const limit = parseInt(limitString, 10);
if (isNaN(limit)) { throw new Error('limit is not a number'); }

View File

@@ -178,21 +178,36 @@ export class DocManager extends EventEmitter {
{naming: 'saved'});
}
// Do an import targeted at a specific workspace. Cleans up uploadId.
// UserId should correspond to the user making the request.
// A workspaceId of null results in an import to an unsaved doc, not
// associated with a specific workspace.
public async importDocToWorkspace(
userId: number, uploadId: number, workspaceId: number|null, browserSettings?: BrowserSettings,
): Promise<DocCreationInfo> {
/**
* Do an import targeted at a specific workspace.
*
* `userId` should correspond to the user making the request.
*
* If workspaceId is omitted, an unsaved doc unassociated with a specific workspace
* will be created.
*
* Cleans up `uploadId` and returns creation info about the imported doc.
*/
public async importDocToWorkspace(options: {
userId: number,
uploadId: number,
documentName?: string,
workspaceId?: number,
browserSettings?: BrowserSettings,
}): Promise<DocCreationInfo> {
if (!this._homeDbManager) { throw new Error("HomeDbManager not available"); }
const {userId, uploadId, documentName, workspaceId, browserSettings} = options;
const accessId = this.makeAccessId(userId);
const docSession = makeExceptionalDocSession('nascent', {browserSettings});
const register = async (docId: string, docTitle: string) => {
if (!workspaceId || !this._homeDbManager) { return; }
const queryResult = await this._homeDbManager.addDocument({userId}, workspaceId,
{name: docTitle}, docId);
const register = async (docId: string, uploadBaseFilename: string) => {
if (workspaceId === undefined || !this._homeDbManager) { return; }
const queryResult = await this._homeDbManager.addDocument(
{userId},
workspaceId,
{name: documentName ?? uploadBaseFilename},
docId
);
if (queryResult.status !== 200) {
// TODO The ready-to-add document is not yet in storageManager, but is in the filesystem. It
// should get cleaned up in case of error here.
@@ -555,7 +570,7 @@ export class DocManager extends EventEmitter {
private async _doImportDoc(docSession: OptDocSession, uploadInfo: UploadInfo,
options: {
naming: 'classic'|'saved'|'unsaved',
register?: (docId: string, docTitle: string) => Promise<void>,
register?: (docId: string, uploadBaseFilename: string) => Promise<void>,
userId?: number,
}): Promise<DocCreationInfo> {
try {

View File

@@ -88,16 +88,20 @@ export class DocWorker {
await filterDocumentInPlace(docSessionFromRequest(mreq), tmpPath);
// NOTE: We may want to reconsider the mimeType used for Grist files.
return res.type('application/x-sqlite3')
.download(tmpPath, (optStringParam(req.query.title) || docTitle || 'document') + ".grist", async (err: any) => {
if (err) {
if (err.message && /Request aborted/.test(err.message)) {
log.warn(`Download request aborted for doc ${docId}`, err);
} else {
log.error(`Download failure for doc ${docId}`, err);
.download(
tmpPath,
(optStringParam(req.query.title, 'title') || docTitle || 'document') + ".grist",
async (err: any) => {
if (err) {
if (err.message && /Request aborted/.test(err.message)) {
log.warn(`Download request aborted for doc ${docId}`, err);
} else {
log.error(`Download failure for doc ${docId}`, err);
}
}
await fse.unlink(tmpPath);
}
await fse.unlink(tmpPath);
});
);
}
// Register main methods related to documents.
@@ -156,7 +160,7 @@ export class DocWorker {
const mreq = req as RequestWithLogin;
let urlId: string|undefined;
try {
if (optStringParam(req.query.clientId)) {
if (optStringParam(req.query.clientId, 'clientId')) {
const activeDoc = this._getDocSession(stringParam(req.query.clientId, 'clientId'),
integerParam(req.query.docFD, 'docFD')).activeDoc;
// TODO: The docId should be stored in the ActiveDoc class. Currently docName is

View File

@@ -113,7 +113,7 @@ export interface DownloadOptions extends ExportParameters {
*/
export function parseExportParameters(req: express.Request): ExportParameters {
const tableId = stringParam(req.query.tableId, 'tableId');
const viewSectionId = optIntegerParam(req.query.viewSection);
const viewSectionId = optIntegerParam(req.query.viewSection, 'viewSection');
const sortOrder = optJsonParam(req.query.activeSortSpec, []) as number[];
const filters: Filter[] = optJsonParam(req.query.filters, []);
const linkingFilter: FilterColValues = optJsonParam(req.query.linkingFilter, null);

View File

@@ -436,7 +436,7 @@ export class FlexServer implements GristServer {
public testAddRouter() {
if (this._check('router')) { return; }
this.app.get('/test/router', (req, res) => {
const act = optStringParam(req.query.act) || 'none';
const act = optStringParam(req.query.act, 'act') || 'none';
const port = stringParam(req.query.port, 'port'); // port is trusted in mock; in prod it is not.
if (act === 'add' || act === 'remove') {
const host = `localhost:${port}`;
@@ -1036,12 +1036,12 @@ export class FlexServer implements GristServer {
log.warn("Serving unauthenticated /test/login endpoint, made available because GRIST_TEST_LOGIN is set.");
// Query parameter is called "username" for compatibility with Cognito.
const email = optStringParam(req.query.username);
const email = optStringParam(req.query.username, 'username');
if (email) {
const redirect = optStringParam(req.query.next);
const redirect = optStringParam(req.query.next, 'next');
const profile: UserProfile = {
email,
name: optStringParam(req.query.name) || email,
name: optStringParam(req.query.name, 'name') || email,
};
const url = new URL(redirect || getOrgUrl(req));
// Make sure we update session for org we'll be redirecting to.
@@ -1884,7 +1884,7 @@ export class FlexServer implements GristServer {
forceSessionChange(mreq.session);
// Redirect to the requested URL after successful login.
if (!nextUrl) {
const nextPath = optStringParam(req.query.next);
const nextPath = optStringParam(req.query.next, 'next');
nextUrl = new URL(getOrgUrl(req, nextPath));
}
if (signUp === undefined) {

View File

@@ -71,7 +71,7 @@ export async function getForwardAuthLoginSystem(): Promise<GristLoginSystem|unde
}
await setUserInSession(req, gristServer, profile);
const target = new URL(gristServer.getHomeUrl(req));
const next = optStringParam(req.query.next);
const next = optStringParam(req.query.next, 'next');
if (next) {
target.pathname = next;
}

View File

@@ -91,7 +91,7 @@ export async function googleAuthTokenMiddleware(
res: express.Response,
next: express.NextFunction) {
// If access token is in place, proceed
if (!optStringParam(req.query.code)) {
if (!optStringParam(req.query.code, 'code')) {
throw new ApiError("Google Auth endpoint requires a code parameter in the query string", 400);
} else {
try {
@@ -134,11 +134,11 @@ export function addGoogleAuthEndpoint(
// our request. It is encrypted (with CLIENT_SECRET) and signed with redirect url.
// In state query parameter we will receive an url that was send as part of the request to Google.
if (optStringParam(req.query.code)) {
if (optStringParam(req.query.code, 'code')) {
log.debug("GoogleAuth - response from Google with valid code");
messagePage(req, res, { code: stringParam(req.query.code, 'code'),
origin: stringParam(req.query.state, 'state') });
} else if (optStringParam(req.query.error)) {
} else if (optStringParam(req.query.error, 'error')) {
log.debug("GoogleAuth - response from Google with error code", stringParam(req.query.error, 'error'));
if (stringParam(req.query.error, 'error') === "access_denied") {
messagePage(req, res, { error: stringParam(req.query.error, 'error'),

View File

@@ -17,7 +17,7 @@ export async function exportToDrive(
res: Response
) {
// Token should come from auth middleware
const access_token = optStringParam(req.query.access_token);
const access_token = optStringParam(req.query.access_token, 'access_token');
if (!access_token) {
throw new Error("No access token - Can't send file to Google Drive");
}
@@ -30,7 +30,7 @@ export async function exportToDrive(
};
// Prepare file for exporting.
log.debug(`Export to drive - Preparing file for export`, meta);
const name = (optStringParam(req.query.title) || activeDoc.docName);
const name = (optStringParam(req.query.title, 'title') || activeDoc.docName);
const stream = new PassThrough();
try {

View File

@@ -150,7 +150,7 @@ export class Telemetry implements ITelemetry {
*/
app.post('/api/telemetry', expressWrap(async (req, resp) => {
const mreq = req as RequestWithLogin;
const event = stringParam(req.body.event, 'event', TelemetryEvents.values) as TelemetryEvent;
const event = stringParam(req.body.event, 'event', {allowed: TelemetryEvents.values}) as TelemetryEvent;
if ('eventSource' in req.body.metadata) {
this._telemetryLogger.rawLog('info', getEventType(event), event, {
...(removeNullishKeys(req.body.metadata)),

View File

@@ -256,23 +256,40 @@ export function getDocId(req: Request) {
return mreq.docAuth.docId;
}
export function optStringParam(p: any): string|undefined {
if (typeof p === 'string') { return p; }
return undefined;
export interface StringParamOptions {
allowed?: readonly string[];
/* Defaults to true. */
allowEmpty?: boolean;
}
export function stringParam(p: any, name: string, allowed?: readonly string[]): string {
export function optStringParam(p: any, name: string, options: StringParamOptions = {}): string|undefined {
if (p === undefined) { return p; }
return stringParam(p, name, options);
}
export function stringParam(p: any, name: string, options: StringParamOptions = {}): string {
const {allowed, allowEmpty = true} = options;
if (typeof p !== 'string') {
throw new ApiError(`${name} parameter should be a string: ${p}`, 400);
}
if (!allowEmpty && p === '') {
throw new ApiError(`${name} parameter cannot be empty`, 400);
}
if (allowed && !allowed.includes(p)) {
throw new ApiError(`${name} parameter ${p} should be one of ${allowed}`, 400);
}
return p;
}
export function optIntegerParam(p: any, name: string): number|undefined {
if (p === undefined) { return p; }
return integerParam(p, name);
}
export function integerParam(p: any, name: string): number {
if (typeof p === 'number') { return Math.floor(p); }
if (typeof p === 'number' && !Number.isNaN(p)) { return Math.floor(p); }
if (typeof p === 'string') {
const result = parseInt(p, 10);
if (isNaN(result)) {
@@ -283,12 +300,6 @@ export function integerParam(p: any, name: string): number {
throw new ApiError(`${name} parameter should be an integer: ${p}`, 400);
}
export function optIntegerParam(p: any): number|undefined {
if (typeof p === 'number') { return Math.floor(p); }
if (typeof p === 'string') { return parseInt(p, 10); }
return undefined;
}
export function optJsonParam(p: any, defaultValue: any): any {
if (typeof p !== 'string') { return defaultValue; }
return gutil.safeJsonParse(p, defaultValue);

View File

@@ -67,8 +67,8 @@ export function addUploadRoute(server: GristServer, expressApp: Application, ...
// Like upload, but copy data from a document already known to us.
expressApp.post(`/copy`, ...handlers, expressWrap(async (req: Request, res: Response) => {
const docId = optStringParam(req.query.doc);
const name = optStringParam(req.query.name);
const docId = optStringParam(req.query.doc, 'doc');
const name = optStringParam(req.query.name, 'name');
if (!docId) { throw new Error('doc must be specified'); }
const accessId = makeAccessId(req, getAuthorizedUserId(req));
try {