mirror of
https://github.com/yu-i-i/overleaf-cep.git
synced 2026-11-12 13:13:45 +01:00
Add request validation to HistoryController endpoints (#35730)
* Add schemas for file restoration and label management in HistoryController * Add request schemas for proxying to history API and resyncing project history * prevent transforming version:null into version:0 with coerce * move jsDoc comments above the handler * check for undefined resyncProjectStructureOnly * Add tests for restoreFileFromV2 and revertFile * Set default value for resyncProjectStructureOnly in resyncProjectHistorySchema GitOrigin-RevId: 7b8b8a4c3834b8c8a83f93052ab1f59e86c529a6
This commit is contained in:
committed by
Copybot
parent
c821cc695c
commit
d8de261370
@@ -139,12 +139,22 @@ function setBlobCacheHeaders(res, etag) {
|
||||
res.set('ETag', etag)
|
||||
}
|
||||
|
||||
const proxyToHistoryApiSchema = z.object({
|
||||
// both project_id and Project_id are accepted for backwards compatibility
|
||||
params: z.object({
|
||||
Project_id: zz.objectId().optional(),
|
||||
project_id: zz.objectId().optional(),
|
||||
doc_id: zz.objectId().optional(),
|
||||
}),
|
||||
})
|
||||
|
||||
/**
|
||||
* @param {any} req
|
||||
* @param {any} res
|
||||
* @param {any} next
|
||||
*/
|
||||
async function proxyToHistoryApi(req, res, next) {
|
||||
parseReq(req, proxyToHistoryApiSchema)
|
||||
const userId = SessionManager.getLoggedInUserId(req.session)
|
||||
const url = settings.apis.project_history.url + req.url
|
||||
|
||||
@@ -173,12 +183,19 @@ async function proxyToHistoryApi(req, res, next) {
|
||||
}
|
||||
}
|
||||
|
||||
const proxyToHistoryApiAndInjectUserDetailsSchema = z.object({
|
||||
params: z.object({
|
||||
Project_id: zz.objectId(),
|
||||
}),
|
||||
})
|
||||
|
||||
/**
|
||||
* @param {any} req
|
||||
* @param {any} res
|
||||
* @param {any} next
|
||||
*/
|
||||
async function proxyToHistoryApiAndInjectUserDetails(req, res, next) {
|
||||
parseReq(req, proxyToHistoryApiAndInjectUserDetailsSchema)
|
||||
const userId = SessionManager.getLoggedInUserId(req.session)
|
||||
const url = settings.apis.project_history.url + req.url
|
||||
const body = await fetchJson(url, {
|
||||
@@ -189,6 +206,16 @@ async function proxyToHistoryApiAndInjectUserDetails(req, res, next) {
|
||||
res.json(data)
|
||||
}
|
||||
|
||||
const resyncProjectHistorySchema = z.object({
|
||||
params: z.object({
|
||||
Project_id: zz.objectId(),
|
||||
}),
|
||||
body: z.object({
|
||||
historyRangesMigration: z.enum(['forwards', 'backwards']).optional(),
|
||||
resyncProjectStructureOnly: z.boolean().default(false),
|
||||
}),
|
||||
})
|
||||
|
||||
/**
|
||||
* @param {any} req
|
||||
* @param {any} res
|
||||
@@ -197,14 +224,11 @@ async function proxyToHistoryApiAndInjectUserDetails(req, res, next) {
|
||||
async function resyncProjectHistory(req, res, next) {
|
||||
// increase timeout to 6 minutes
|
||||
res.setTimeout(6 * 60 * 1000)
|
||||
const projectId = req.params.Project_id
|
||||
const opts = {}
|
||||
const historyRangesMigration = req.body.historyRangesMigration
|
||||
if (historyRangesMigration) {
|
||||
opts.historyRangesMigration = historyRangesMigration
|
||||
}
|
||||
if (req.body.resyncProjectStructureOnly) {
|
||||
opts.resyncProjectStructureOnly = req.body.resyncProjectStructureOnly
|
||||
const { params, body } = parseReq(req, resyncProjectHistorySchema)
|
||||
const projectId = params.Project_id
|
||||
const opts = {
|
||||
historyRangesMigration: body.historyRangesMigration,
|
||||
resyncProjectStructureOnly: body.resyncProjectStructureOnly,
|
||||
}
|
||||
|
||||
try {
|
||||
@@ -223,14 +247,25 @@ async function resyncProjectHistory(req, res, next) {
|
||||
res.sendStatus(204)
|
||||
}
|
||||
|
||||
const restoreFileFromV2Schema = z.object({
|
||||
params: z.object({
|
||||
project_id: zz.objectId(),
|
||||
}),
|
||||
body: z.object({
|
||||
version: z.number().int().min(0),
|
||||
pathname: zz.filepath(),
|
||||
}),
|
||||
})
|
||||
|
||||
/**
|
||||
* @param {any} req
|
||||
* @param {any} res
|
||||
* @param {any} next
|
||||
*/
|
||||
async function restoreFileFromV2(req, res, next) {
|
||||
const { project_id: projectId } = req.params
|
||||
const { version, pathname } = req.body
|
||||
const { params, body } = parseReq(req, restoreFileFromV2Schema)
|
||||
const { project_id: projectId } = params
|
||||
const { version, pathname } = body
|
||||
const userId = SessionManager.getLoggedInUserId(req.session)
|
||||
|
||||
const entity = await RestoreManager.promises.restoreFileFromV2(
|
||||
@@ -259,14 +294,25 @@ async function restoreFileFromV2(req, res, next) {
|
||||
})
|
||||
}
|
||||
|
||||
const revertFileSchema = z.object({
|
||||
params: z.object({
|
||||
project_id: zz.objectId(),
|
||||
}),
|
||||
body: z.object({
|
||||
version: z.number().int().min(0),
|
||||
pathname: zz.filepath(),
|
||||
}),
|
||||
})
|
||||
|
||||
/**
|
||||
* @param {any} req
|
||||
* @param {any} res
|
||||
* @param {any} next
|
||||
*/
|
||||
async function revertFile(req, res, next) {
|
||||
const { project_id: projectId } = req.params
|
||||
const { version, pathname } = req.body
|
||||
const { params, body } = parseReq(req, revertFileSchema)
|
||||
const { project_id: projectId } = params
|
||||
const { version, pathname } = body
|
||||
const userId = SessionManager.getLoggedInUserId(req.session)
|
||||
|
||||
const entity = await RestoreManager.promises.revertFile(
|
||||
@@ -296,14 +342,24 @@ async function revertFile(req, res, next) {
|
||||
})
|
||||
}
|
||||
|
||||
const revertProjectSchema = z.object({
|
||||
params: z.object({
|
||||
project_id: zz.objectId(),
|
||||
}),
|
||||
body: z.object({
|
||||
version: z.number().int().min(0),
|
||||
}),
|
||||
})
|
||||
|
||||
/**
|
||||
* @param {any} req
|
||||
* @param {any} res
|
||||
* @param {any} next
|
||||
*/
|
||||
async function revertProject(req, res, next) {
|
||||
const { project_id: projectId } = req.params
|
||||
const { version } = req.body
|
||||
const { params, body } = parseReq(req, revertProjectSchema)
|
||||
const { project_id: projectId } = params
|
||||
const { version } = body
|
||||
const userId = SessionManager.getLoggedInUserId(req.session)
|
||||
|
||||
const reverted = await RestoreManager.promises.revertProject(
|
||||
@@ -327,13 +383,20 @@ async function revertProject(req, res, next) {
|
||||
res.json(reverted)
|
||||
}
|
||||
|
||||
const getLabelsSchema = z.object({
|
||||
params: z.object({
|
||||
Project_id: zz.objectId(),
|
||||
}),
|
||||
})
|
||||
|
||||
/**
|
||||
* @param {any} req
|
||||
* @param {any} res
|
||||
* @param {any} next
|
||||
*/
|
||||
async function getLabels(req, res, next) {
|
||||
const projectId = req.params.Project_id
|
||||
const { params } = parseReq(req, getLabelsSchema)
|
||||
const projectId = params.Project_id
|
||||
|
||||
let labels = await fetchJson(
|
||||
`${settings.apis.project_history.url}/project/${projectId}/labels`
|
||||
@@ -343,14 +406,25 @@ async function getLabels(req, res, next) {
|
||||
res.json(labels)
|
||||
}
|
||||
|
||||
const createLabelSchema = z.object({
|
||||
params: z.object({
|
||||
Project_id: zz.objectId(),
|
||||
}),
|
||||
body: z.object({
|
||||
comment: z.string(),
|
||||
version: z.number().int().min(0),
|
||||
}),
|
||||
})
|
||||
|
||||
/**
|
||||
* @param {any} req
|
||||
* @param {any} res
|
||||
* @param {any} next
|
||||
*/
|
||||
async function createLabel(req, res, next) {
|
||||
const projectId = req.params.Project_id
|
||||
const { comment, version } = req.body
|
||||
const { params, body } = parseReq(req, createLabelSchema)
|
||||
const projectId = params.Project_id
|
||||
const { comment, version } = body
|
||||
const userId = SessionManager.getLoggedInUserId(req.session)
|
||||
|
||||
let label = await fetchJson(
|
||||
@@ -444,13 +518,21 @@ function _displayNameForUser(user) {
|
||||
return name
|
||||
}
|
||||
|
||||
const deleteLabelSchema = z.object({
|
||||
params: z.object({
|
||||
Project_id: zz.objectId(),
|
||||
label_id: zz.objectId(),
|
||||
}),
|
||||
})
|
||||
|
||||
/**
|
||||
* @param {any} req
|
||||
* @param {any} res
|
||||
* @param {any} next
|
||||
*/
|
||||
async function deleteLabel(req, res, next) {
|
||||
const { Project_id: projectId, label_id: labelId } = req.params
|
||||
const { params } = parseReq(req, deleteLabelSchema)
|
||||
const { Project_id: projectId, label_id: labelId } = params
|
||||
const userId = SessionManager.getLoggedInUserId(req.session)
|
||||
|
||||
const project = await ProjectGetter.promises.getProject(projectId, {
|
||||
|
||||
@@ -119,7 +119,17 @@ describe('HistoryController', function () {
|
||||
vi.doMock(
|
||||
'../../../../app/src/Features/History/RestoreManager.mjs',
|
||||
() => ({
|
||||
default: (ctx.RestoreManager = {}),
|
||||
default: (ctx.RestoreManager = {
|
||||
promises: {
|
||||
restoreFileFromV2: sinon
|
||||
.stub()
|
||||
.resolves({ _id: 'restored-id', type: 'doc' }),
|
||||
revertFile: sinon
|
||||
.stub()
|
||||
.resolves({ _id: 'reverted-id', type: 'doc' }),
|
||||
revertProject: sinon.stub().resolves([]),
|
||||
},
|
||||
}),
|
||||
})
|
||||
)
|
||||
|
||||
@@ -137,7 +147,12 @@ describe('HistoryController', function () {
|
||||
|
||||
describe('proxyToHistoryApi', function () {
|
||||
beforeEach(async function (ctx) {
|
||||
ctx.req = { url: '/mock/url', method: 'POST', session: sinon.stub() }
|
||||
ctx.req = {
|
||||
url: '/mock/url',
|
||||
method: 'POST',
|
||||
session: sinon.stub(),
|
||||
params: {},
|
||||
}
|
||||
ctx.res = {
|
||||
set: sinon.stub(),
|
||||
}
|
||||
@@ -188,7 +203,11 @@ describe('HistoryController', function () {
|
||||
|
||||
describe('proxyToHistoryApiAndInjectUserDetails', function () {
|
||||
beforeEach(async function (ctx) {
|
||||
ctx.req = { url: '/mock/url', method: 'POST' }
|
||||
ctx.req = {
|
||||
url: '/mock/url',
|
||||
method: 'POST',
|
||||
params: { Project_id: ctx.project_id },
|
||||
}
|
||||
ctx.res = { json: sinon.stub() }
|
||||
ctx.data = 'mock-data'
|
||||
ctx.dataWithUsers = 'mock-injected-data'
|
||||
@@ -233,7 +252,11 @@ describe('HistoryController', function () {
|
||||
describe('proxyToHistoryApiAndInjectUserDetails (with the history API failing)', function () {
|
||||
beforeEach(async function (ctx) {
|
||||
ctx.url = '/mock/url'
|
||||
ctx.req = { url: ctx.url, method: 'POST' }
|
||||
ctx.req = {
|
||||
url: ctx.url,
|
||||
method: 'POST',
|
||||
params: { Project_id: ctx.project_id },
|
||||
}
|
||||
ctx.res = { json: sinon.stub() }
|
||||
ctx.err = new RequestFailedError(ctx.url, {}, { status: 500 })
|
||||
ctx.fetchJson.rejects(ctx.err)
|
||||
@@ -308,6 +331,165 @@ describe('HistoryController', function () {
|
||||
})
|
||||
})
|
||||
|
||||
describe('restoreFileFromV2', function () {
|
||||
beforeEach(function (ctx) {
|
||||
ctx.res = { json: sinon.stub() }
|
||||
ctx.ProjectAuditLogHandler.addEntryIfManagedInBackground = sinon.stub()
|
||||
})
|
||||
|
||||
it('restores the file for a valid request', async function (ctx) {
|
||||
ctx.req = {
|
||||
params: { project_id: ctx.project_id },
|
||||
body: { version: 42, pathname: 'foo/bar.tex' },
|
||||
session: {},
|
||||
ip: '1.2.3.4',
|
||||
}
|
||||
await ctx.HistoryController.restoreFileFromV2(ctx.req, ctx.res, ctx.next)
|
||||
ctx.RestoreManager.promises.restoreFileFromV2.should.have.been.calledWith(
|
||||
ctx.user_id,
|
||||
ctx.project_id,
|
||||
42,
|
||||
'foo/bar.tex'
|
||||
)
|
||||
})
|
||||
|
||||
it('rejects a non-numeric version without invoking RestoreManager', async function (ctx) {
|
||||
ctx.req = {
|
||||
params: { project_id: ctx.project_id },
|
||||
body: { version: 'not-a-number', pathname: 'foo/bar.tex' },
|
||||
session: {},
|
||||
ip: '1.2.3.4',
|
||||
}
|
||||
await ctx.HistoryController.restoreFileFromV2(ctx.req, ctx.res, ctx.next)
|
||||
ctx.next.should.have.been.calledOnce
|
||||
ctx.next.firstCall.args[0].should.be.an.instanceof(Error)
|
||||
ctx.RestoreManager.promises.restoreFileFromV2.should.not.have.been.called
|
||||
})
|
||||
|
||||
it('rejects a negative version without invoking RestoreManager', async function (ctx) {
|
||||
ctx.req = {
|
||||
params: { project_id: ctx.project_id },
|
||||
body: { version: -1, pathname: 'foo/bar.tex' },
|
||||
session: {},
|
||||
ip: '1.2.3.4',
|
||||
}
|
||||
await ctx.HistoryController.restoreFileFromV2(ctx.req, ctx.res, ctx.next)
|
||||
ctx.next.should.have.been.calledOnce
|
||||
ctx.next.firstCall.args[0].should.be.an.instanceof(Error)
|
||||
ctx.RestoreManager.promises.restoreFileFromV2.should.not.have.been.called
|
||||
})
|
||||
|
||||
it('rejects a pathname containing .. without invoking RestoreManager', async function (ctx) {
|
||||
ctx.req = {
|
||||
params: { project_id: ctx.project_id },
|
||||
body: { version: 42, pathname: '../../etc/passwd' },
|
||||
session: {},
|
||||
ip: '1.2.3.4',
|
||||
}
|
||||
await ctx.HistoryController.restoreFileFromV2(ctx.req, ctx.res, ctx.next)
|
||||
ctx.next.should.have.been.calledOnce
|
||||
ctx.next.firstCall.args[0].should.be.an.instanceof(Error)
|
||||
ctx.RestoreManager.promises.restoreFileFromV2.should.not.have.been.called
|
||||
})
|
||||
|
||||
it('rejects an absolute pathname without invoking RestoreManager', async function (ctx) {
|
||||
ctx.req = {
|
||||
params: { project_id: ctx.project_id },
|
||||
body: { version: 42, pathname: '/etc/passwd' },
|
||||
session: {},
|
||||
ip: '1.2.3.4',
|
||||
}
|
||||
await ctx.HistoryController.restoreFileFromV2(ctx.req, ctx.res, ctx.next)
|
||||
ctx.next.should.have.been.calledOnce
|
||||
ctx.next.firstCall.args[0].should.be.an.instanceof(Error)
|
||||
ctx.RestoreManager.promises.restoreFileFromV2.should.not.have.been.called
|
||||
})
|
||||
|
||||
it('rejects a path-traversal string as version without invoking RestoreManager', async function (ctx) {
|
||||
ctx.req = {
|
||||
params: { project_id: ctx.project_id },
|
||||
body: { version: '0/../../../ObjectId', pathname: 'foo/bar.tex' },
|
||||
session: {},
|
||||
ip: '1.2.3.4',
|
||||
}
|
||||
await ctx.HistoryController.restoreFileFromV2(ctx.req, ctx.res, ctx.next)
|
||||
ctx.next.should.have.been.calledOnce
|
||||
ctx.next.firstCall.args[0].should.be.an.instanceof(Error)
|
||||
ctx.RestoreManager.promises.restoreFileFromV2.should.not.have.been.called
|
||||
})
|
||||
})
|
||||
|
||||
describe('revertFile', function () {
|
||||
beforeEach(function (ctx) {
|
||||
ctx.res = { json: sinon.stub() }
|
||||
ctx.ProjectAuditLogHandler.addEntryIfManagedInBackground = sinon.stub()
|
||||
})
|
||||
|
||||
it('reverts the file for a valid request', async function (ctx) {
|
||||
ctx.req = {
|
||||
params: { project_id: ctx.project_id },
|
||||
body: { version: 42, pathname: 'foo/bar.tex' },
|
||||
session: {},
|
||||
ip: '1.2.3.4',
|
||||
}
|
||||
await ctx.HistoryController.revertFile(ctx.req, ctx.res, ctx.next)
|
||||
ctx.RestoreManager.promises.revertFile.should.have.been.calledWith(
|
||||
ctx.user_id,
|
||||
ctx.project_id,
|
||||
42,
|
||||
'foo/bar.tex'
|
||||
)
|
||||
})
|
||||
|
||||
it('rejects a malformed pathname without invoking RestoreManager', async function (ctx) {
|
||||
ctx.req = {
|
||||
params: { project_id: ctx.project_id },
|
||||
body: { version: 42, pathname: '../secret.tex' },
|
||||
session: {},
|
||||
ip: '1.2.3.4',
|
||||
}
|
||||
await ctx.HistoryController.revertFile(ctx.req, ctx.res, ctx.next)
|
||||
ctx.next.should.have.been.calledOnce
|
||||
ctx.next.firstCall.args[0].should.be.an.instanceof(Error)
|
||||
ctx.RestoreManager.promises.revertFile.should.not.have.been.called
|
||||
})
|
||||
})
|
||||
|
||||
describe('revertProject', function () {
|
||||
beforeEach(function (ctx) {
|
||||
ctx.res = { json: sinon.stub() }
|
||||
ctx.ProjectAuditLogHandler.addEntryIfManagedInBackground = sinon.stub()
|
||||
})
|
||||
|
||||
it('reverts the project for a valid request', async function (ctx) {
|
||||
ctx.req = {
|
||||
params: { project_id: ctx.project_id },
|
||||
body: { version: 42 },
|
||||
session: {},
|
||||
ip: '1.2.3.4',
|
||||
}
|
||||
await ctx.HistoryController.revertProject(ctx.req, ctx.res, ctx.next)
|
||||
ctx.RestoreManager.promises.revertProject.should.have.been.calledWith(
|
||||
ctx.user_id,
|
||||
ctx.project_id,
|
||||
42
|
||||
)
|
||||
})
|
||||
|
||||
it('rejects a non-integer version without invoking RestoreManager', async function (ctx) {
|
||||
ctx.req = {
|
||||
params: { project_id: ctx.project_id },
|
||||
body: { version: 1.5 },
|
||||
session: {},
|
||||
ip: '1.2.3.4',
|
||||
}
|
||||
await ctx.HistoryController.revertProject(ctx.req, ctx.res, ctx.next)
|
||||
ctx.next.should.have.been.calledOnce
|
||||
ctx.next.firstCall.args[0].should.be.an.instanceof(Error)
|
||||
ctx.RestoreManager.promises.revertProject.should.not.have.been.called
|
||||
})
|
||||
})
|
||||
|
||||
describe('requestBlob', function () {
|
||||
describe('With Range header', function () {
|
||||
beforeEach(async function (ctx) {
|
||||
|
||||
Reference in New Issue
Block a user