Skip to content

Commit

Permalink
Merge pull request #1136 from jembi/OHM-1078-fix-request-matching-alpha
Browse files Browse the repository at this point in the history 
Ohm 1078 fix request matching alpha
  • Loading branch information
@MattyJ007
MattyJ007 authored Jul 8, 2021
2 parents 32a0fe7 + a833f3f commit fcd69d1
Show file tree
Hide file tree
Showing 10 changed files with 64 additions and 7 deletions.
3 changes: 3 additions & 0 deletions src/middleware/authorisation.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ export async function authorise (ctx, done) {
// authorisation succeeded
ctx.authorisedChannel = channel
logger.info(`The request, '${ctx.request.path}' is authorised to access ${ctx.authorisedChannel.name}`)
} else if (!channel) {
// No channel found
ctx.response.status = 404
} else {
// authorisation failed
ctx.response.status = 401
Expand Down
5 changes: 5 additions & 0 deletions src/middleware/requestMatching.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ function matchContent (channel, ctx) {
}
}

function matchMethod (channel, ctx) {
return !!channel.methods.find(method => ctx.request.method.toUpperCase() === method)
}

export function matchRegex (regexPat, body) {
const regex = new RegExp(regexPat)
return regex.test(body.toString())
Expand Down Expand Up @@ -98,6 +102,7 @@ function matchContentTypes (channel, ctx) {
// TODO: OHM-695 uncomment line below when working on ticket
const matchFunctions = [
matchUrlPattern,
matchMethod,
matchContentTypes
]

Expand Down
4 changes: 4 additions & 0 deletions test/integration/authenticationAPITests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,7 @@ describe('API Integration Tests', () => {
await new ChannelModelAPI({
name: 'TEST DATA - Mock endpoint',
urlPattern: 'test/mock',
methods: ['GET'],
allow: ['PoC'],
routes: [
{
Expand Down Expand Up @@ -591,6 +592,7 @@ describe('API Integration Tests', () => {
name: 'TEST DATA - Mock endpoint',
urlPattern: 'test/mock',
allow: ['PoC'],
methods: ['GET'],
routes: [
{
name: 'test route',
Expand Down Expand Up @@ -671,6 +673,7 @@ describe('API Integration Tests', () => {
name: 'TEST DATA - Mock endpoint',
urlPattern: 'test/mock',
allow: ['PoC'],
methods: ['GET'],
routes: [
{
name: 'test route',
Expand Down Expand Up @@ -749,6 +752,7 @@ describe('API Integration Tests', () => {
name: 'TEST DATA - Mock endpoint',
urlPattern: 'test/mock',
allow: ['PoC'],
methods: ['GET'],
routes: [
{
name: 'test route',
Expand Down
5 changes: 5 additions & 0 deletions test/integration/autoRetryIntegrationTests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ describe('Auto Retry Integration Tests', () => {
name: 'TEST DATA - Will break channel',
urlPattern: '^/test/nowhere$',
allow: ['PoC'],
methods: ['GET'],
routes: [{
name: 'unavailable route',
host: 'localhost',
Expand All @@ -110,6 +111,7 @@ describe('Auto Retry Integration Tests', () => {
name: 'TEST DATA - Will break channel - attempt once',
urlPattern: '^/test/nowhere/2$',
allow: ['PoC'],
methods: ['GET'],
routes: [{
name: 'unavailable route',
host: 'localhost',
Expand Down Expand Up @@ -232,6 +234,7 @@ describe('Auto Retry Integration Tests', () => {
name: 'TEST DATA - Secondary route will break channel',
urlPattern: '^/test/nowhere$',
allow: ['PoC'],
methods: ['GET'],
responseBody: true,
autoRetryEnabled: true,
routes: [{
Expand Down Expand Up @@ -291,6 +294,7 @@ describe('Auto Retry Integration Tests', () => {
name: 'TEST DATA - Mediator has error channel',
urlPattern: '^/test/nowhere$',
allow: ['PoC'],
methods: ['GET'],
responseBody: true,
autoRetryEnabled: true,
routes: [{
Expand Down Expand Up @@ -359,6 +363,7 @@ describe('Auto Retry Integration Tests', () => {
name: 'TEST DATA - Both will break channel',
urlPattern: '^/test/nowhere$',
allow: ['PoC'],
methods: ['GET'],
responseBody: true,
autoRetryEnabled: true,
routes: [{
Expand Down
4 changes: 4 additions & 0 deletions test/integration/channelsAPITests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1744,6 +1744,7 @@ describe('API Integration Tests', () => {
name: 'TEST DATA - Mock endpoint 1',
urlPattern: '^/test/undefined/priority$',
allow: ['PoC'],
methods: ['GET'],
routes: [{
name: 'test route',
host: 'localhost',
Expand All @@ -1760,6 +1761,7 @@ describe('API Integration Tests', () => {
name: 'TEST DATA - Mock endpoint 2',
urlPattern: '^/.*$',
priority: 3,
methods: ['GET'],
allow: ['PoC'],
routes: [{
name: 'test route',
Expand All @@ -1777,6 +1779,7 @@ describe('API Integration Tests', () => {
name: 'TEST DATA - Mock endpoint 3',
urlPattern: '^/test/mock$',
priority: 2,
methods: ['GET'],
allow: ['PoC'],
routes: [{
name: 'test route',
Expand Down Expand Up @@ -1860,6 +1863,7 @@ describe('API Integration Tests', () => {
name: 'TEST DATA - Mock endpoint 4',
urlPattern: '^/test/mock$',
priority: 1,
methods: ['GET'],
allow: ['something else'],
routes: [{
name: 'test route',
Expand Down
7 changes: 7 additions & 0 deletions test/integration/httpTests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ describe('HTTP tests', () => {
name: 'TEST DATA - Mock endpoint',
urlPattern: 'test/mock',
allow: ['PoC'],
methods: ['GET'],
routes: [{
name: 'test route',
host: 'localhost',
Expand Down Expand Up @@ -101,6 +102,7 @@ describe('HTTP tests', () => {
name: 'TEST DATA - Mock endpoint',
urlPattern: '/test/mock',
allow: ['PoC'],
methods: ['POST', 'PUT'],
routes: [{
name: 'test route',
host: 'localhost',
Expand All @@ -117,6 +119,7 @@ describe('HTTP tests', () => {
name: 'TEST DATA - Mock With Return endpoint',
urlPattern: '/gmo',
allow: ['PoC'],
methods: ['POST', 'PUT'],
routes: [{
name: 'test route return',
host: 'localhost',
Expand All @@ -133,6 +136,7 @@ describe('HTTP tests', () => {
name: 'TEST DATA - Mock With Return endpoint public',
urlPattern: '/public',
allow: [],
methods: ['POST', 'PUT'],
authType: 'public',
routes: [{
name: 'test route',
Expand All @@ -150,6 +154,7 @@ describe('HTTP tests', () => {
name: 'TEST DATA - Mock With Return endpoint private - whitelist',
urlPattern: '/private',
allow: [],
methods: ['POST', 'PUT'],
whitelist: ['::ffff:127.0.0.1', '127.0.0.1'], // localhost in IPV6
authType: 'public',
routes: [{
Expand All @@ -168,6 +173,7 @@ describe('HTTP tests', () => {
name: 'TEST DATA - whitelist but un-authorised',
urlPattern: '/un-auth',
allow: ['private'],
methods: ['POST', 'PUT'],
whitelist: ['::ffff:127.0.0.1', '127.0.0.1'], // localhost in IPV6
authType: 'private',
routes: [{
Expand All @@ -186,6 +192,7 @@ describe('HTTP tests', () => {
name: 'TEST DATA - whitelist but authorised',
urlPattern: '/auth',
allow: ['PoC'],
methods: ['POST', 'PUT'],
whitelist: ['::ffff:127.0.0.1', '127.0.0.1'], // localhost in IPV6
authType: 'private',
routes: [{
Expand Down
1 change: 1 addition & 0 deletions test/integration/mediatorAPITests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1225,6 +1225,7 @@ describe('API Integration Tests', () => {
name: 'TEST DATA - Mock mediator endpoint',
urlPattern: 'test/mediator',
allow: ['PoC'],
methods: ['GET'],
routes: [{
name: 'mediator route',
host: 'localhost',
Expand Down
1 change: 1 addition & 0 deletions test/integration/multipartFormDataTests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ describe('Multipart form data tests', () => {
name: 'TEST DATA - Mock endpoint - multipart',
urlPattern: '/test/multipart',
allow: ['PoC'],
methods: ['POST'],
routes: [{
name: 'test route',
host: 'localhost',
Expand Down
17 changes: 10 additions & 7 deletions test/integration/routesTests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ describe('Routes enabled/disabled tests', () => {
name: 'TEST DATA - Mock endpoint 1',
urlPattern: '^/test/channel1$',
allow: ['PoC'],
methods: ['GET'],
responseBody: true,
routes: [
{
Expand All @@ -66,6 +67,7 @@ describe('Routes enabled/disabled tests', () => {
name: 'TEST DATA - Mock endpoint 2',
urlPattern: '^/test/channel2$',
allow: ['PoC'],
methods: ['GET'],
routes: [
{
name: 'test route',
Expand All @@ -90,6 +92,7 @@ describe('Routes enabled/disabled tests', () => {
name: 'TEST DATA - Mock endpoint 3',
urlPattern: '^/test/channel3$',
allow: ['PoC'],
methods: ['GET'],
routes: [
{
name: 'test route',
Expand All @@ -115,6 +118,7 @@ describe('Routes enabled/disabled tests', () => {
name: 'TEST DATA - Mock endpoint 4',
urlPattern: '^/test/channel4$',
allow: ['PoC'],
methods: ['GET'],
routes: [
{
name: 'test transaction orchestration',
Expand All @@ -134,6 +138,7 @@ describe('Routes enabled/disabled tests', () => {
name: 'TEST DATA - Mock endpoint 5',
urlPattern: '^/test/channel5$',
allow: ['PoC'],
methods: ['GET'],
routes: [
{
name: 'test transaction fail orchestration',
Expand All @@ -153,6 +158,7 @@ describe('Routes enabled/disabled tests', () => {
name: 'TEST DATA - Mock endpoint 6',
urlPattern: '^/test/channel6$',
allow: ['PoC'],
methods: ['GET'],
routes: [
{
name: 'test route',
Expand All @@ -177,6 +183,7 @@ describe('Routes enabled/disabled tests', () => {
name: 'TEST DATA - timeoutChannel',
urlPattern: '^/test/timeoutChannel$',
allow: ['PoC'],
methods: ['GET'],
timeout: 20,
routes: [
{
Expand Down Expand Up @@ -325,15 +332,11 @@ describe('Routes enabled/disabled tests', () => {
req.method.should.eql('GET')
})

it('should deny a request if the method is not in the "methods"', async () => {
const res = await request(constants.HTTP_BASE_URL)
it('should deny a request if the method is not in the "methods" (404 returned)', async () => {
await request(constants.HTTP_BASE_URL)
.post('/test/restricted')
.auth('testApp', 'password')
.expect(405)

res.text.should.eql('Request with method POST is not allowed. Only GET methods are allowed')
// routes are async
restrictedSpy.callCount.should.eql(0)
.expect(404)
})

it('should allow a request and produce an orchestration recording the openhim\'s request and received response', async () => {
Expand Down
24 changes: 24 additions & 0 deletions test/unit/requestMatchingTest.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,22 @@ describe('Request Matching middleware', () => {
})
})

describe('.matchMethod', () => {
const matchMethod = requestMatching.__get__('matchMethod')
const channel = { methods: ['GET', 'POST', 'DELETE'] }

it('should match a request http method', () => {
const actual = matchMethod(channel, { request: { method: 'GET' } })
return actual.should.be.true()
})

it('should reject request with excluded method', () => {
// PUT is not included in the channel definition
const actual = matchMethod(channel, { request: { method: 'PUT' } })
return actual.should.be.false()
})
})

describe('.matchContentTypes', () => {
it('should match correct content types', () => {
const matchContentTypes = requestMatching.__get__('matchContentTypes')
Expand Down Expand Up @@ -326,6 +342,7 @@ describe('Request Matching middleware', () => {
name: 'Authorisation mock channel 4',
urlPattern: 'test/authorisation',
allow: ['Test1', 'Musha_OpenMRS', 'Test2'],
methods: ['GET'],
routes: [{
name: 'test route',
host: 'localhost',
Expand Down Expand Up @@ -360,6 +377,7 @@ describe('Request Matching middleware', () => {
cert: ''
}
ctx.request = {}
ctx.request.method = 'GET'
ctx.request.url = 'test/authorisation'
ctx.request.path = 'test/authorisation'
ctx.request.header = {}
Expand All @@ -379,6 +397,7 @@ describe('Request Matching middleware', () => {
name: 'Authorisation mock channel 4',
urlPattern: 'test/authorisation',
allow: ['Test1', 'Musha_OpenMRS', 'Test2'],
methods: ['GET'],
routes: [{
name: 'test route',
host: 'localhost',
Expand Down Expand Up @@ -414,6 +433,7 @@ describe('Request Matching middleware', () => {
}
ctx.request = {}
ctx.request.url = 'test/authorisation'
ctx.request.method = 'GET'
ctx.request.path = 'test/authorisation'
ctx.request.header = {}
ctx.request.header['content-type'] = 'text/dodgy-xml; charset=utf-8'
Expand All @@ -433,6 +453,7 @@ describe('Request Matching middleware', () => {
name: 'Mock for Channel Status Test (enabled)',
urlPattern: 'test/status/enabled',
allow: ['PoC', 'Test1', 'Test2'],
methods: ['GET'],
routes: [{
name: 'test route',
host: 'localhost',
Expand Down Expand Up @@ -464,6 +485,7 @@ describe('Request Matching middleware', () => {
cert: ''
}
ctx.request = {}
ctx.request.method = 'GET'
ctx.request.url = 'test/status/enabled'
ctx.request.path = 'test/status/enabled'
ctx.response = {}
Expand Down Expand Up @@ -512,6 +534,7 @@ describe('Request Matching middleware', () => {
cert: ''
}
ctx.request = {}
ctx.request.method = 'GET'
ctx.request.url = 'test/status/disabled'
ctx.request.path = 'test/status/disabled'
ctx.response = {}
Expand Down Expand Up @@ -561,6 +584,7 @@ describe('Request Matching middleware', () => {
cert: ''
}
ctx.request = {}
ctx.request.method = 'GET'
ctx.request.url = 'test/status/deleted'
ctx.request.path = 'test/status/deleted'
ctx.response = {}
Expand Down

0 comments on commit fcd69d1

Please sign in to comment.