Skip to content

Commit

Permalink
Merge pull request #1135 from jembi/OHM-1078-fix-request-matching
Browse files Browse the repository at this point in the history 
Add logic for matching the request http method
  • Loading branch information
@MattyJ007
MattyJ007 authored Jul 8, 2021
2 parents 20ad2aa + 88cd508 commit e2acacd
Show file tree
Hide file tree
Showing 13 changed files with 72 additions and 7 deletions.
2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "openhim-core",
"description": "The OpenHIM core application that provides logging and routing of http requests",
"version": "7.0.0",
"version": "7.0.1",
"main": "./lib/server.js",
"bin": {
"openhim-core": "./bin/openhim-core.js"
Expand Down
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) {
// Channel not 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 @@ -25,6 +25,10 @@ function matchContent (channel, ctx) {
}
}

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

function matchRegex (regexPat, body) {
const regex = new RegExp(regexPat)
return regex.test(body.toString())
Expand Down Expand Up @@ -97,6 +101,7 @@ function matchContentTypes (channel, ctx) {
let matchFunctions = [
matchUrlPattern,
matchContent,
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 @@ -347,6 +347,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 @@ -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'],
routes: [{
name: 'available route',
host: 'localhost',
Expand Down Expand Up @@ -289,6 +292,7 @@ describe(`Auto Retry Integration Tests`, () => {
name: 'TEST DATA - Mediator has error channel',
urlPattern: '^/test/nowhere$',
allow: ['PoC'],
methods: ['GET'],
routes: [{
name: 'mediator route',
host: 'localhost',
Expand Down Expand Up @@ -355,6 +359,7 @@ describe(`Auto Retry Integration Tests`, () => {
name: 'TEST DATA - Both will break channel',
urlPattern: '^/test/nowhere$',
allow: ['PoC'],
methods: ['GET'],
routes: [{
name: 'unavailable route 1',
host: 'localhost',
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
2 changes: 2 additions & 0 deletions test/integration/eventsAPITests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ describe('Events API Integration Tests', () => {
name: channelName,
urlPattern: 'test/mock',
allow: ['PoC'],
methods: ['GET'],
routes: [
{
name: primaryRouteName,
Expand All @@ -78,6 +79,7 @@ describe('Events API Integration Tests', () => {
name: `${channelName}-slow`,
urlPattern: 'test/slow',
allow: ['PoC'],
methods: ['GET'],
routes: [
{
name: primaryRouteName,
Expand Down
10 changes: 10 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 @@ -152,6 +156,7 @@ describe('HTTP tests', () => {
allow: [],
whitelist: ['::ffff:127.0.0.1', '127.0.0.1'], // localhost in IPV6
authType: 'public',
methods: ['POST', 'PUT'],
routes: [{
name: 'test route',
host: 'localhost',
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 Expand Up @@ -344,6 +351,7 @@ describe('HTTP tests', () => {
name: 'TEST DATA - Mock endpoint',
urlPattern: 'test/mock',
allow: ['PoC'],
methods: ['POST', 'PUT'],
routes: [{
name: 'test route',
host: 'localhost',
Expand Down Expand Up @@ -434,6 +442,7 @@ describe('HTTP tests', () => {
name: 'TEST DATA - Mock endpoint',
urlPattern: 'test/mock',
allow: ['PoC'],
methods: ['POST', 'PUT'],
routes: [{
name: 'test route',
host: 'localhost',
Expand Down Expand Up @@ -518,6 +527,7 @@ describe('HTTP tests', () => {
name: 'TEST DATA - Mock endpoint',
urlPattern: 'test/mock',
allow: ['PoC'],
methods: ['POST', 'PUT'],
routes: [{
name: 'test route',
host: 'localhost',
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', 'PUT'],
routes: [{
name: 'test route',
host: 'localhost',
Expand Down
15 changes: 9 additions & 6 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'],
routes: [
{
name: 'test route',
Expand All @@ -65,6 +66,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 @@ -89,6 +91,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 @@ -114,6 +117,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 @@ -133,6 +137,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 @@ -152,6 +157,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 @@ -176,6 +182,7 @@ describe('Routes enabled/disabled tests', () => {
name: 'TEST DATA - timeoutChannel',
urlPattern: '^/test/timeoutChannel$',
allow: ['PoC'],
methods: ['GET'],
timeout: 20,
routes: [
{
Expand Down Expand Up @@ -323,15 +330,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 () => {
it('should deny a request if the method is not in the "methods" (returns 404)', async () => {
const res = await request(constants.HTTP_BASE_URL)
.post('/test/restricted')
.auth('testApp', 'password')
.expect(405)

res.body.toString().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
1 change: 1 addition & 0 deletions test/integration/urlRewriting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ describe('URL rewriting test', () => {
name: 'TEST DATA - Mock endpoint',
urlPattern: 'test/mock',
allow: ['PoC'],
methods: ['GET'],
routes: [{
name: 'test route',
host: 'localhost',
Expand Down
Loading

0 comments on commit e2acacd

Please sign in to comment.