Skip to content

Commit

Permalink
CN Ensure certain POST routes still work in readOnlyMode (#4645)
Browse files Browse the repository at this point in the history
  • Loading branch information
SidSethi committed Jan 20, 2023
1 parent 311b88b commit 49baab5
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 18 deletions.
88 changes: 76 additions & 12 deletions creator-node/src/middlewares/readOnly/readOnlyMiddleware.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { Request, Response } from 'express'
import {
readOnlyMiddleware,
readOnlyMiddlewareHelper
canProceedInReadOnlyMode
} from './readOnlyMiddleware'
import assert from 'assert'
import config from '../../config'
Expand All @@ -13,36 +13,48 @@ describe('Test read-only middleware', function () {
config.reset('spID')
})

it('Should pass if read-only enabled and is GET request', function () {
it('Should allow request if read-only enabled and is GET request', function () {
const method = 'GET'
const isReadOnlyMode = true
const spIDNotDefined = false
config.set('isReadOnlyMode', isReadOnlyMode)
config.set('spID', 1)
const originalUrl = '/random_url'
let nextCalled = false

readOnlyMiddleware({ method } as Request, {} as Response, function () {
nextCalled = true
})
readOnlyMiddleware(
{ method, originalUrl } as Request,
{} as Response,
function () {
nextCalled = true
}
)

assert.deepStrictEqual(
readOnlyMiddlewareHelper(isReadOnlyMode, spIDNotDefined, method),
canProceedInReadOnlyMode(
isReadOnlyMode,
spIDNotDefined,
method,
originalUrl
),
true
)
assert.deepStrictEqual(nextCalled, true)
})

it('Should fail if read-only enabled and is not GET request', function () {
it('Should not allow request if read-only enabled and is not GET request', function () {
const isReadOnlyMode = true
const spIDNotDefined = false
const method = 'POST'
config.set('isReadOnlyMode', isReadOnlyMode)
config.set('spID', 1)
const originalUrl = '/random_url'
let nextCalled = false

const logger = loggerFactory()
const req = {
method,
originalUrl,
logger
} as unknown as Request
const res = resFactory() as unknown as Response
Expand All @@ -53,23 +65,30 @@ describe('Test read-only middleware', function () {

assert.deepStrictEqual(res.statusCode, 500)
assert.deepStrictEqual(
readOnlyMiddlewareHelper(isReadOnlyMode, spIDNotDefined, method),
canProceedInReadOnlyMode(
isReadOnlyMode,
spIDNotDefined,
method,
originalUrl
),
false
)
assert.deepStrictEqual(nextCalled, false)
})

it('Should pass if read-only not enabled and is POST request', function () {
it('Should allow request if read-only not enabled and is POST request', function () {
const method = 'POST'
const isReadOnlyMode = false
const spIDNotDefined = false
config.set('isReadOnlyMode', isReadOnlyMode)
config.set('spID', 1)
const originalUrl = '/random_url'
let nextCalled = false

const logger = loggerFactory()
const req = {
method,
originalUrl,
logger
} as unknown as Request
const res = resFactory() as unknown as Response
Expand All @@ -79,23 +98,30 @@ describe('Test read-only middleware', function () {
})

assert.deepStrictEqual(
readOnlyMiddlewareHelper(isReadOnlyMode, spIDNotDefined, method),
canProceedInReadOnlyMode(
isReadOnlyMode,
spIDNotDefined,
method,
originalUrl
),
true
)
assert.deepStrictEqual(nextCalled, true)
})

it('Should fail if spID not defined and is POST request', function () {
it('Should not allow request if spID not defined and is POST request', function () {
const method = 'POST'
const isReadOnlyMode = false
const spIDNotDefined = true
config.set('isReadOnlyMode', isReadOnlyMode)
config.set('spID', 0)
const originalUrl = '/random_url'
let nextCalled = false

const logger = loggerFactory()
const req = {
method,
originalUrl,
logger
} as unknown as Request
const res = resFactory() as unknown as Response
Expand All @@ -105,9 +131,47 @@ describe('Test read-only middleware', function () {
})

assert.deepStrictEqual(
readOnlyMiddlewareHelper(isReadOnlyMode, spIDNotDefined, method),
canProceedInReadOnlyMode(
isReadOnlyMode,
spIDNotDefined,
method,
originalUrl
),
false
)
assert.deepStrictEqual(nextCalled, false)
})

it('Should allow request on excluded route even if read-only is enabled', function () {
const method = 'POST'
const isReadOnlyMode = true
const spIDNotDefined = false
config.set('isReadOnlyMode', isReadOnlyMode)
config.set('spID', 1)
const originalUrl = '/users/batch_clock_status'
let nextCalled = false

const logger = loggerFactory()
const req = {
method,
originalUrl,
logger
} as unknown as Request
const res = resFactory() as unknown as Response

readOnlyMiddleware(req, res, function () {
nextCalled = true
})

assert.deepStrictEqual(
canProceedInReadOnlyMode(
isReadOnlyMode,
spIDNotDefined,
method,
originalUrl
),
true
)
assert.deepStrictEqual(nextCalled, true)
})
})
27 changes: 21 additions & 6 deletions creator-node/src/middlewares/readOnly/readOnlyMiddleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@ import type { Request, Response, NextFunction } from 'express'
import { sendResponse, errorResponseServerError } from '../../apiHelpers'
import config from '../../config'

const EXCLUDED_ROUTES: string[] = [
'/batch_cids_exist',
'/batch_image_cids_exist',
'/batch_id_to_cid',
'/users/batch_clock_status'
]

/**
* Middleware to block all non-GET api calls if the server should be in "read-only" mode
*/
Expand All @@ -13,10 +20,12 @@ export function readOnlyMiddleware(
const isReadOnlyMode = config.get('isReadOnlyMode')
const spIDNotDefined = !config.get('spID') || config.get('spID') <= 0 // true if not a valid spID
const method = req.method
const canProceed = readOnlyMiddlewareHelper(
const originalUrl = req.originalUrl
const canProceed = canProceedInReadOnlyMode(
isReadOnlyMode,
spIDNotDefined,
method
method,
originalUrl
)

if (!canProceed)
Expand All @@ -32,16 +41,22 @@ export function readOnlyMiddleware(
* @param {Boolean} isReadOnlyMode From config.get('isReadOnlyMode')
* @param {Boolean} spIDNotDefined set in serviceRegistry after recovering this node's identity. true if not a valid spID
* @param {String} method REST method for this request eg. POST, GET
* @param {String} originalUrl request url string e.g. /health_check/sync
* @returns {Boolean} returns true if the request can proceed. eg GET in read only or any request in non read-only mode
*/
export function readOnlyMiddlewareHelper(
export function canProceedInReadOnlyMode(
isReadOnlyMode: boolean,
spIDNotDefined: boolean,
method: string
method: string,
originalUrl: string
) {
if ((isReadOnlyMode || spIDNotDefined) && method !== 'GET') {
if (spIDNotDefined) return false
if (
isReadOnlyMode &&
method !== 'GET' &&
!EXCLUDED_ROUTES.includes(originalUrl)
) {
return false
}

return true
}

0 comments on commit 49baab5

Please sign in to comment.