From d65711df0ede9d51a97a5d1fe91519b1bf315bed Mon Sep 17 00:00:00 2001 From: mateuszc-kainos Date: Thu, 4 Jun 2020 16:09:33 +0200 Subject: [PATCH 01/13] Implement authentication middleware --- docker-compose.yml | 4 ++++ package-lock.json | 8 ++++++++ package.json | 3 ++- services/infrastructure/mongo.initdb.js | 3 ++- src/controllers/form.controller.ts | 3 ++- src/middleware/auth.middleware.ts | 25 +++++++++++++++++++++++++ src/paths.ts | 1 + src/utils/uri.factory.ts | 13 +++++++++++++ 8 files changed, 57 insertions(+), 3 deletions(-) create mode 100644 src/middleware/auth.middleware.ts create mode 100644 src/utils/uri.factory.ts diff --git a/docker-compose.yml b/docker-compose.yml index f86b3786..f45a6d38 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -23,6 +23,7 @@ services: - chs-dev working_dir: /app environment: + - ACCOUNT_WEB_URL=http://account.chs-dev - CDN_HOST=cdn.chs-dev - CACHE_SERVER=redis - CACHE_DB=0 @@ -32,6 +33,9 @@ services: - COOKIE_SECURE_ONLY=0 - COOKIE_SECRET=ChGovUk-XQrbf3sLj2abFxIY2TlapsJ - DEFAULT_SESSION_EXPIRATION=3600 + - OAUTH2_TOKEN_URI=account.chs-dev/oauth2/token + - OAUTH2_CLIENT_ID= + - OAUTH2_CLIENT_SECRET= cdn-ch-gov-uk: container_name: chs-cdn-ch-gov-uk image: 169942020521.dkr.ecr.eu-west-1.amazonaws.com/local/cdn.ch.gov.uk:latest diff --git a/package-lock.json b/package-lock.json index c8709e33..b98159d3 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12703,6 +12703,14 @@ "integrity": "sha512-obtSWTlbJ+a+TFRYGaUumtVwb+InIUVI0Lu0VBUAPmj2cU5JutEXg3xUE0c2J5Tcy7h2DEKVJBFi+Y9ZSFzzPQ==", "dev": true }, + "web-security-node": { + "version": "git+ssh://git@github.com/companieshouse/web-security-node.git#e35f2199b35f729eec2445d38d2b40a588e85ece", + "from": "git+ssh://git@github.com/companieshouse/web-security-node.git", + "requires": { + "ch-logging": "git+ssh://git@github.com/companieshouse/ch-structured-logging-node.git#a3e0dcc57f91f63803e02ceef2cc5e4038de9a19", + "ch-node-session-handler": "git+ssh://git@github.com/companieshouse/node-session-handler.git#2.1.1" + } + }, "which": { "version": "2.0.2", "resolved": "https://registry.npmjs.org/which/-/which-2.0.2.tgz", diff --git a/package.json b/package.json index 563a6c3c..2ee086b9 100644 --- a/package.json +++ b/package.json @@ -36,7 +36,8 @@ "nunjucks": "^3.2.1", "reflect-metadata": "^0.1.13", "tsconfig-paths": "^3.9.0", - "tslib": "^2.0.0" + "tslib": "^2.0.0", + "web-security-node": "git@github.com:companieshouse/web-security-node.git" }, "devDependencies": { "@types/body-parser": "^1.19.0", diff --git a/services/infrastructure/mongo.initdb.js b/services/infrastructure/mongo.initdb.js index 90633237..2283e092 100644 --- a/services/infrastructure/mongo.initdb.js +++ b/services/infrastructure/mongo.initdb.js @@ -4,7 +4,8 @@ db.api_clients.insert({ "client_secret" : "M2UwYzRkNzIwOGQ1OGQ0OWIzMzViYjJjOTEyYTc1", "user_id" : "Y2VkZWVlMzhlZWFjY2M4MzQ3MT", "redirect_uris" : [ - "http://chs-dev/oauth2/user/callback" + "http://chs-dev/oauth2/user/callback", + "http://account.chs-dev/oauth2/user/callback" ], "type" : "web", "is_internal_app": true diff --git a/src/controllers/form.controller.ts b/src/controllers/form.controller.ts index a528756f..6bf8a8a8 100644 --- a/src/controllers/form.controller.ts +++ b/src/controllers/form.controller.ts @@ -3,6 +3,7 @@ import { inject } from 'inversify' import { controller, httpGet, httpPost, requestBody } from 'inversify-express-utils' import BaseController from './base.controller' +import { AuthMiddleware } from 'app/middleware/auth.middleware' import { FormFormModel } from 'app/models/form.model' import Optional from 'app/models/optional' import ValidationErrors from 'app/models/validationErrors' @@ -15,7 +16,7 @@ interface FormViewModel { errors?: ValidationErrors } -@controller(FORM_PAGE_URI) +@controller(FORM_PAGE_URI, AuthMiddleware) export class FormController extends BaseController { public constructor(@inject(FormValidator) private validator: FormValidator) { diff --git a/src/middleware/auth.middleware.ts b/src/middleware/auth.middleware.ts new file mode 100644 index 00000000..f3302e66 --- /dev/null +++ b/src/middleware/auth.middleware.ts @@ -0,0 +1,25 @@ +import { NextFunction, Request, RequestHandler, Response } from 'express' +import { provide } from 'inversify-binding-decorators' +import { BaseMiddleware } from 'inversify-express-utils' +import { authMiddleware, AuthOptions } from 'web-security-node' + +import {SEARCH_COMPANY_URI} from 'app/paths' +import {getEnvOrDefault} from 'app/utils/env.util' +import {newUriFactory} from 'app/utils/uri.factory' + +@provide(AuthMiddleware) +export class AuthMiddleware extends BaseMiddleware { +​ + + public getReturnToPage(req: Request): string { + return newUriFactory(req).createAbsoluteUri(SEARCH_COMPANY_URI) + } + + public handler: RequestHandler = (req: Request, res: Response, next: NextFunction): void => { + const authOptions = { + returnUrl: this.getReturnToPage(req), + accountWebUrl: getEnvOrDefault('ACCOUNT_WEB_URL', ''), + } as AuthOptions + authMiddleware(authOptions)(req, res, next) + } +} \ No newline at end of file diff --git a/src/paths.ts b/src/paths.ts index 309980d7..1c5ae0ad 100644 --- a/src/paths.ts +++ b/src/paths.ts @@ -4,3 +4,4 @@ export const HEALTHCHECK_URI = `${ROOT_URI}/healthcheck` // Pages export const FORM_PAGE_URI = `${ROOT_URI}/form` +export const SEARCH_COMPANY_URI = `${ROOT_URI}/search-company` \ No newline at end of file diff --git a/src/utils/uri.factory.ts b/src/utils/uri.factory.ts new file mode 100644 index 00000000..1de80667 --- /dev/null +++ b/src/utils/uri.factory.ts @@ -0,0 +1,13 @@ +import { Request } from 'express' + +export interface UriFactory { + createAbsoluteUri(path: string): string +} + +export const newUriFactory = (req: Request): UriFactory => { + return { + createAbsoluteUri(path: string): string { + return new URL(path, `${req.protocol}://${req.headers.host}`).href + } + } +} From 3cb50d9f13193fea1c11d919229cabf9c0801302 Mon Sep 17 00:00:00 2001 From: mateuszc-kainos Date: Fri, 5 Jun 2020 11:25:47 +0200 Subject: [PATCH 02/13] Redirect to search company page --- docker-compose.yml | 2 +- services/infrastructure/mongo.initdb.js | 3 +-- src/constants/app.const.ts | 2 +- src/controllers/form.controller.ts | 2 +- src/controllers/index.ts | 3 ++- src/controllers/searchCompany.controller.ts | 14 +++++++++++ src/middleware/auth.middleware.ts | 7 +++--- src/paths.ts | 1 + src/views/search-company.njk | 28 +++++++++++++++++++++ 9 files changed, 52 insertions(+), 10 deletions(-) create mode 100644 src/controllers/searchCompany.controller.ts create mode 100644 src/views/search-company.njk diff --git a/docker-compose.yml b/docker-compose.yml index f45a6d38..14c65f03 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -23,7 +23,7 @@ services: - chs-dev working_dir: /app environment: - - ACCOUNT_WEB_URL=http://account.chs-dev + - ACCOUNT_WEB_URL=http://chs-dev - CDN_HOST=cdn.chs-dev - CACHE_SERVER=redis - CACHE_DB=0 diff --git a/services/infrastructure/mongo.initdb.js b/services/infrastructure/mongo.initdb.js index 2283e092..90633237 100644 --- a/services/infrastructure/mongo.initdb.js +++ b/services/infrastructure/mongo.initdb.js @@ -4,8 +4,7 @@ db.api_clients.insert({ "client_secret" : "M2UwYzRkNzIwOGQ1OGQ0OWIzMzViYjJjOTEyYTc1", "user_id" : "Y2VkZWVlMzhlZWFjY2M4MzQ3MT", "redirect_uris" : [ - "http://chs-dev/oauth2/user/callback", - "http://account.chs-dev/oauth2/user/callback" + "http://chs-dev/oauth2/user/callback" ], "type" : "web", "is_internal_app": true diff --git a/src/constants/app.const.ts b/src/constants/app.const.ts index 7458098a..cab424f3 100644 --- a/src/constants/app.const.ts +++ b/src/constants/app.const.ts @@ -1 +1 @@ -export const APP_NAME = 'example-ch-service-name' +export const APP_NAME = 'dissolution-web' diff --git a/src/controllers/form.controller.ts b/src/controllers/form.controller.ts index 6bf8a8a8..4d65401b 100644 --- a/src/controllers/form.controller.ts +++ b/src/controllers/form.controller.ts @@ -3,7 +3,7 @@ import { inject } from 'inversify' import { controller, httpGet, httpPost, requestBody } from 'inversify-express-utils' import BaseController from './base.controller' -import { AuthMiddleware } from 'app/middleware/auth.middleware' +import { AuthMiddleware } from 'app/middleware/auth.middleware'; import { FormFormModel } from 'app/models/form.model' import Optional from 'app/models/optional' import ValidationErrors from 'app/models/validationErrors' diff --git a/src/controllers/index.ts b/src/controllers/index.ts index 05b7a89f..dbd57499 100644 --- a/src/controllers/index.ts +++ b/src/controllers/index.ts @@ -1,3 +1,4 @@ import 'app/controllers/form.controller' import 'app/controllers/healthcheck.controller' -import 'app/controllers/landing.controller' \ No newline at end of file +import 'app/controllers/landing.controller' +import 'app/controllers/searchCompany.controller' \ No newline at end of file diff --git a/src/controllers/searchCompany.controller.ts b/src/controllers/searchCompany.controller.ts new file mode 100644 index 00000000..edfc9d6e --- /dev/null +++ b/src/controllers/searchCompany.controller.ts @@ -0,0 +1,14 @@ +import { controller, httpGet } from 'inversify-express-utils' + +import BaseController from 'app/controllers/base.controller' +import { AuthMiddleware } from 'app/middleware/auth.middleware' +import { SEARCH_COMPANY_URI } from 'app/paths' + +@controller(SEARCH_COMPANY_URI, AuthMiddleware) +export class SearchCompanyController extends BaseController { + + @httpGet('') + public async get(): Promise { + return super.render('search-company') + } +} diff --git a/src/middleware/auth.middleware.ts b/src/middleware/auth.middleware.ts index f3302e66..bcebbd8e 100644 --- a/src/middleware/auth.middleware.ts +++ b/src/middleware/auth.middleware.ts @@ -3,14 +3,13 @@ import { provide } from 'inversify-binding-decorators' import { BaseMiddleware } from 'inversify-express-utils' import { authMiddleware, AuthOptions } from 'web-security-node' -import {SEARCH_COMPANY_URI} from 'app/paths' -import {getEnvOrDefault} from 'app/utils/env.util' -import {newUriFactory} from 'app/utils/uri.factory' +import { SEARCH_COMPANY_URI } from 'app/paths' +import { getEnvOrDefault } from 'app/utils/env.util' +import { newUriFactory } from 'app/utils/uri.factory' @provide(AuthMiddleware) export class AuthMiddleware extends BaseMiddleware { ​ - public getReturnToPage(req: Request): string { return newUriFactory(req).createAbsoluteUri(SEARCH_COMPANY_URI) } diff --git a/src/paths.ts b/src/paths.ts index 1c5ae0ad..a8185410 100644 --- a/src/paths.ts +++ b/src/paths.ts @@ -4,4 +4,5 @@ export const HEALTHCHECK_URI = `${ROOT_URI}/healthcheck` // Pages export const FORM_PAGE_URI = `${ROOT_URI}/form` +export const WHO_TO_TELL_URI = `${ROOT_URI}/who-to-tell` export const SEARCH_COMPANY_URI = `${ROOT_URI}/search-company` \ No newline at end of file diff --git a/src/views/search-company.njk b/src/views/search-company.njk new file mode 100644 index 00000000..c387909a --- /dev/null +++ b/src/views/search-company.njk @@ -0,0 +1,28 @@ +{% extends 'layout.njk' %} + +{% from 'govuk/components/breadcrumbs/macro.njk' import govukBreadcrumbs %} +{% from 'govuk/components/button/macro.njk' import govukButton %} + +{% block pageTitle %} + {{ serviceName }} +{% endblock %} + +{% block beforeContent %} + {{ + govukBreadcrumbs({ + items: [ + { href: 'https://www.gov.uk/', text: 'Home' }, + { href: 'https://www.gov.uk/browse/business', text: 'Business and self-employed' }, + { href: 'https://www.gov.uk/browse/business/limited-company', text: 'Running a limited company' } + ] + }) + }} +{% endblock %} + +{% block content %} +
+
+

Search Company Placeholder

+
+
+{% endblock %} From 968ead329d6fb2d68d71b2425819b4379946e174 Mon Sep 17 00:00:00 2001 From: mateuszc-kainos Date: Fri, 5 Jun 2020 13:46:55 +0200 Subject: [PATCH 03/13] Add logger, correct cookie name --- docker-compose.yml | 7 +++---- src/controllers/form.controller.ts | 3 +-- src/controllers/searchCompany.controller.ts | 3 +-- src/middleware/auth.middleware.ts | 13 ++++++++++--- src/middleware/logger.ts | 13 +++++++++++++ 5 files changed, 28 insertions(+), 11 deletions(-) create mode 100644 src/middleware/logger.ts diff --git a/docker-compose.yml b/docker-compose.yml index 14c65f03..b7aeb591 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -28,14 +28,13 @@ services: - CACHE_SERVER=redis - CACHE_DB=0 - CACHE_PASSWORD='' - - COOKIE_NAME=SID + - COOKIE_NAME=__SID - COOKIE_DOMAIN=chs-dev - COOKIE_SECURE_ONLY=0 - COOKIE_SECRET=ChGovUk-XQrbf3sLj2abFxIY2TlapsJ - DEFAULT_SESSION_EXPIRATION=3600 - - OAUTH2_TOKEN_URI=account.chs-dev/oauth2/token - - OAUTH2_CLIENT_ID= - - OAUTH2_CLIENT_SECRET= + - LOG_LEVEL=info + - HUMAN_LOG=1 cdn-ch-gov-uk: container_name: chs-cdn-ch-gov-uk image: 169942020521.dkr.ecr.eu-west-1.amazonaws.com/local/cdn.ch.gov.uk:latest diff --git a/src/controllers/form.controller.ts b/src/controllers/form.controller.ts index 4d65401b..a528756f 100644 --- a/src/controllers/form.controller.ts +++ b/src/controllers/form.controller.ts @@ -3,7 +3,6 @@ import { inject } from 'inversify' import { controller, httpGet, httpPost, requestBody } from 'inversify-express-utils' import BaseController from './base.controller' -import { AuthMiddleware } from 'app/middleware/auth.middleware'; import { FormFormModel } from 'app/models/form.model' import Optional from 'app/models/optional' import ValidationErrors from 'app/models/validationErrors' @@ -16,7 +15,7 @@ interface FormViewModel { errors?: ValidationErrors } -@controller(FORM_PAGE_URI, AuthMiddleware) +@controller(FORM_PAGE_URI) export class FormController extends BaseController { public constructor(@inject(FormValidator) private validator: FormValidator) { diff --git a/src/controllers/searchCompany.controller.ts b/src/controllers/searchCompany.controller.ts index edfc9d6e..55ee84a6 100644 --- a/src/controllers/searchCompany.controller.ts +++ b/src/controllers/searchCompany.controller.ts @@ -1,10 +1,9 @@ import { controller, httpGet } from 'inversify-express-utils' import BaseController from 'app/controllers/base.controller' -import { AuthMiddleware } from 'app/middleware/auth.middleware' import { SEARCH_COMPANY_URI } from 'app/paths' -@controller(SEARCH_COMPANY_URI, AuthMiddleware) +@controller(SEARCH_COMPANY_URI) export class SearchCompanyController extends BaseController { @httpGet('') diff --git a/src/middleware/auth.middleware.ts b/src/middleware/auth.middleware.ts index bcebbd8e..09c52713 100644 --- a/src/middleware/auth.middleware.ts +++ b/src/middleware/auth.middleware.ts @@ -1,23 +1,30 @@ +import ApplicationLogger from 'ch-logging/lib/ApplicationLogger'; import { NextFunction, Request, RequestHandler, Response } from 'express' +import { inject } from 'inversify' import { provide } from 'inversify-binding-decorators' import { BaseMiddleware } from 'inversify-express-utils' import { authMiddleware, AuthOptions } from 'web-security-node' -import { SEARCH_COMPANY_URI } from 'app/paths' +import { FORM_PAGE_URI } from 'app/paths' import { getEnvOrDefault } from 'app/utils/env.util' import { newUriFactory } from 'app/utils/uri.factory' @provide(AuthMiddleware) export class AuthMiddleware extends BaseMiddleware { -​ + + public constructor(@inject(ApplicationLogger) private logger: ApplicationLogger) { + super() + } + public getReturnToPage(req: Request): string { - return newUriFactory(req).createAbsoluteUri(SEARCH_COMPANY_URI) + return newUriFactory(req).createAbsoluteUri(FORM_PAGE_URI) } public handler: RequestHandler = (req: Request, res: Response, next: NextFunction): void => { const authOptions = { returnUrl: this.getReturnToPage(req), accountWebUrl: getEnvOrDefault('ACCOUNT_WEB_URL', ''), + logger: this.logger } as AuthOptions authMiddleware(authOptions)(req, res, next) } diff --git a/src/middleware/logger.ts b/src/middleware/logger.ts new file mode 100644 index 00000000..788ec5ce --- /dev/null +++ b/src/middleware/logger.ts @@ -0,0 +1,13 @@ +import { createLogger } from 'ch-logging' +import ApplicationLogger from 'ch-logging/lib/ApplicationLogger' + +import { APP_NAME } from 'app/constants/app.const' + +let logger: ApplicationLogger + +export function loggerInstance(): ApplicationLogger { + if (!logger) { + logger = createLogger(APP_NAME) + } + return logger +} From 60c52f28abd042e168a1542962fa4f23f7ce4811 Mon Sep 17 00:00:00 2001 From: mateuszc-kainos Date: Fri, 5 Jun 2020 18:12:02 +0200 Subject: [PATCH 04/13] Add middleware to SearchCompanyController --- src/controllers/searchCompany.controller.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/controllers/searchCompany.controller.ts b/src/controllers/searchCompany.controller.ts index 55ee84a6..7c7de269 100644 --- a/src/controllers/searchCompany.controller.ts +++ b/src/controllers/searchCompany.controller.ts @@ -1,9 +1,11 @@ +import { SessionMiddleware } from 'ch-node-session-handler' import { controller, httpGet } from 'inversify-express-utils' import BaseController from 'app/controllers/base.controller' +import { AuthMiddleware } from 'app/middleware/auth.middleware' import { SEARCH_COMPANY_URI } from 'app/paths' -@controller(SEARCH_COMPANY_URI) +@controller(SEARCH_COMPANY_URI, SessionMiddleware, AuthMiddleware) export class SearchCompanyController extends BaseController { @httpGet('') From 1d92d8b610d31bd1ee92f70aa13f9bf1ada0c354 Mon Sep 17 00:00:00 2001 From: mateuszc-kainos Date: Mon, 8 Jun 2020 12:54:14 +0200 Subject: [PATCH 05/13] Add test and mocking session util --- src/application.factory.ts | 92 +++++++++++++------ .../searchCompany.controller.test.ts | 18 ++++ test/utils/session/SessionFactory.ts | 42 +++++++++ 3 files changed, 125 insertions(+), 27 deletions(-) create mode 100644 test/controllers/searchCompany.controller.test.ts create mode 100644 test/utils/session/SessionFactory.ts diff --git a/src/application.factory.ts b/src/application.factory.ts index 62e815b1..af9da7d5 100644 --- a/src/application.factory.ts +++ b/src/application.factory.ts @@ -1,40 +1,78 @@ import * as bodyParser from 'body-parser' -import { Application } from 'express' +import ApplicationLogger from 'ch-logging/lib/ApplicationLogger' +import { Session, SessionMiddleware, SessionStore } from 'ch-node-session-handler' +import { SessionKey } from 'ch-node-session-handler/lib/session/keys/SessionKey' +import { Cookie } from 'ch-node-session-handler/lib/session/model/Cookie' +import { Application, NextFunction, Request, Response } from 'express' import { Container } from 'inversify' import { buildProviderModule } from 'inversify-binding-decorators' import { InversifyExpressServer } from 'inversify-express-utils' import * as nunjucks from 'nunjucks' +import { deepEqual, instance, mock, when } from 'ts-mockito' import { addFilters } from 'app/utils/nunjucks.util' +import { createSession } from 'test/utils/session/SessionFactory' + +// import { getEnvOrThrow } from 'app/utils/env.util' + // tslint:disable-next-line: no-empty export const createApp = (configureBindings: (container: Container) => void = () => { }): Application => { - const container: Container = new Container() - container.load(buildProviderModule()) - configureBindings(container) - - return new InversifyExpressServer(container) - .setConfig(server => { - - server.use(bodyParser.json()) - server.use(bodyParser.urlencoded({extended: false})) - - server.set('view engine', 'njk') - - const env: nunjucks.Environment = nunjucks.configure( - [ - 'src/views', - 'node_modules/govuk-frontend', - 'node_modules/govuk-frontend/components', - ], - { - autoescape: true, - express: server - } - ) - - addFilters(env) + const container: Container = new Container() + container.load(buildProviderModule()) + configureBindings(container) + + return new InversifyExpressServer(container) + .setConfig(server => { + + server.use(bodyParser.json()) + server.use(bodyParser.urlencoded({extended: false})) + + server.set('view engine', 'njk') + + const env: nunjucks.Environment = nunjucks.configure( + [ + 'src/views', + 'node_modules/govuk-frontend', + 'node_modules/govuk-frontend/components', + ], + { + autoescape: true, + express: server + } + ) + + addFilters(env) + }) + .build() +} + +// tslint:disable-next-line: no-empty +export const createAppWithFakeSession = (configureBindings: (container: Container) => void = () => { +}): Application => { + return createApp(container => { + const cookieName = '__SID' + const cookieSecret = 'ChGovUk-XQrbf3sLj2abFxIY2TlapsJ ' + + const session: Session = createSession(cookieSecret) + + const sessionId = session.data[SessionKey.Id] + const signature = session.data[SessionKey.ClientSig] + + const cookie = Cookie.createFrom(sessionId! + signature) + + const sessionStore = mock(SessionStore) + when(sessionStore.load(deepEqual(cookie))).thenResolve(session.data) + const mockSessionStore = instance(sessionStore) + + container.bind(ApplicationLogger).toConstantValue(mock(instance(ApplicationLogger))) + container.bind(SessionMiddleware).toConstantValue((req: Request, res: Response, next: NextFunction) => { + req.cookies = {} + req.cookies[cookieName] = cookie.value + SessionMiddleware({cookieName, cookieSecret}, mockSessionStore)(req, res, next) + }) + + configureBindings(container) }) - .build() } \ No newline at end of file diff --git a/test/controllers/searchCompany.controller.test.ts b/test/controllers/searchCompany.controller.test.ts new file mode 100644 index 00000000..d901d981 --- /dev/null +++ b/test/controllers/searchCompany.controller.test.ts @@ -0,0 +1,18 @@ +import 'reflect-metadata' + +import { assert } from 'chai' +import { Application } from 'express' +import request from 'supertest' + +import { createAppWithFakeSession } from 'app/application.factory' +import 'app/controllers/searchCompany.controller' +import { SEARCH_COMPANY_URI } from 'app/paths' + +describe('SearchCompanyController', () => { + it('should return 200 when trying to access page with a session', async () => { + const app: Application = createAppWithFakeSession() + const res = await request(app).get(SEARCH_COMPANY_URI).expect(200) + + assert.include(res.text, 'Search Company Placeholder') + }) +}) diff --git a/test/utils/session/SessionFactory.ts b/test/utils/session/SessionFactory.ts new file mode 100644 index 00000000..32b919bf --- /dev/null +++ b/test/utils/session/SessionFactory.ts @@ -0,0 +1,42 @@ +import { Session } from 'ch-node-session-handler/lib/session/model/Session' +import { generateSessionId, generateSignature } from 'ch-node-session-handler/lib/utils/CookieUtils' + + +export function createSession( + secret: string, + signedIn: boolean = true, + isAdmin: boolean = false, + permissions: { [permission: string]: number } = {}): Session { + const id = generateSessionId() + const sig = generateSignature(id, secret) + + return new Session({ + '.id': id, + '.client.signature': sig, + '.hijacked': undefined, + '.oauth2_nonce': 'LBvC2UC8EJ4FbpNfUlrOchBgXk//9WZYezudvWpd5txyx3ziELR7AcajZvam2XoMNBTGTgIddrdMs1ccE9seUw==', + '.zxs_key': 'CxKb2u0GILQPQalUuIYy1ZjL3QquDuYgnedwIafZC7V3mqJ0wH988/VZUMZMvlCs7rYLVHRvEagnYT8TBb9E3w==', + expires: Date.now() + 3600 * 1000, + last_access: Date.now(), + pst: 'all', + signin_info: { + access_token: { + // tslint:disable-next-line: max-line-length + access_token: '/T+R3ABq5SPPbZWSeePnrDE1122FEZSAGRuhmn21aZSqm5UQt/wqixlSViQPOrWe2iFb8PeYjZzmNehMA3JCJg==', + expires_in: 3600, + token_type: 'Bearer' + }, + admin_permissions: isAdmin ? '1' : '0', + signed_in: signedIn ? 1 : 0, + user_profile: { + id: 'sA==', + email: 'test', + forename: 'tester', + surname: 'test', + locale: 'GB_en', + permissions, + scope: undefined + } + } + }) +} From 0f9e4b152f02d9c6b827b64fe6f9928c5db0ccc6 Mon Sep 17 00:00:00 2001 From: mateuszc-kainos Date: Mon, 8 Jun 2020 12:56:47 +0200 Subject: [PATCH 06/13] Reformat code --- src/application.factory.ts | 90 ++++++++++++++-------------- test/utils/session/SessionFactory.ts | 70 +++++++++++----------- 2 files changed, 80 insertions(+), 80 deletions(-) diff --git a/src/application.factory.ts b/src/application.factory.ts index af9da7d5..eb674024 100644 --- a/src/application.factory.ts +++ b/src/application.factory.ts @@ -19,60 +19,60 @@ import { createSession } from 'test/utils/session/SessionFactory' // tslint:disable-next-line: no-empty export const createApp = (configureBindings: (container: Container) => void = () => { }): Application => { - const container: Container = new Container() - container.load(buildProviderModule()) - configureBindings(container) - - return new InversifyExpressServer(container) - .setConfig(server => { - - server.use(bodyParser.json()) - server.use(bodyParser.urlencoded({extended: false})) - - server.set('view engine', 'njk') - - const env: nunjucks.Environment = nunjucks.configure( - [ - 'src/views', - 'node_modules/govuk-frontend', - 'node_modules/govuk-frontend/components', - ], - { - autoescape: true, - express: server - } - ) - - addFilters(env) - }) - .build() + const container: Container = new Container() + container.load(buildProviderModule()) + configureBindings(container) + + return new InversifyExpressServer(container) + .setConfig(server => { + + server.use(bodyParser.json()) + server.use(bodyParser.urlencoded({extended: false})) + + server.set('view engine', 'njk') + + const env: nunjucks.Environment = nunjucks.configure( + [ + 'src/views', + 'node_modules/govuk-frontend', + 'node_modules/govuk-frontend/components', + ], + { + autoescape: true, + express: server + } + ) + + addFilters(env) + }) + .build() } // tslint:disable-next-line: no-empty export const createAppWithFakeSession = (configureBindings: (container: Container) => void = () => { }): Application => { - return createApp(container => { - const cookieName = '__SID' - const cookieSecret = 'ChGovUk-XQrbf3sLj2abFxIY2TlapsJ ' + return createApp(container => { + const cookieName = '__SID' + const cookieSecret = 'ChGovUk-XQrbf3sLj2abFxIY2TlapsJ ' - const session: Session = createSession(cookieSecret) + const session: Session = createSession(cookieSecret) - const sessionId = session.data[SessionKey.Id] - const signature = session.data[SessionKey.ClientSig] + const sessionId = session.data[SessionKey.Id] + const signature = session.data[SessionKey.ClientSig] - const cookie = Cookie.createFrom(sessionId! + signature) + const cookie = Cookie.createFrom(sessionId! + signature) - const sessionStore = mock(SessionStore) - when(sessionStore.load(deepEqual(cookie))).thenResolve(session.data) - const mockSessionStore = instance(sessionStore) + const sessionStore = mock(SessionStore) + when(sessionStore.load(deepEqual(cookie))).thenResolve(session.data) + const mockSessionStore = instance(sessionStore) - container.bind(ApplicationLogger).toConstantValue(mock(instance(ApplicationLogger))) - container.bind(SessionMiddleware).toConstantValue((req: Request, res: Response, next: NextFunction) => { - req.cookies = {} - req.cookies[cookieName] = cookie.value - SessionMiddleware({cookieName, cookieSecret}, mockSessionStore)(req, res, next) - }) - - configureBindings(container) + container.bind(ApplicationLogger).toConstantValue(mock(instance(ApplicationLogger))) + container.bind(SessionMiddleware).toConstantValue((req: Request, res: Response, next: NextFunction) => { + req.cookies = {} + req.cookies[cookieName] = cookie.value + SessionMiddleware({cookieName, cookieSecret}, mockSessionStore)(req, res, next) }) + + configureBindings(container) + }) } \ No newline at end of file diff --git a/test/utils/session/SessionFactory.ts b/test/utils/session/SessionFactory.ts index 32b919bf..24d79386 100644 --- a/test/utils/session/SessionFactory.ts +++ b/test/utils/session/SessionFactory.ts @@ -3,40 +3,40 @@ import { generateSessionId, generateSignature } from 'ch-node-session-handler/li export function createSession( - secret: string, - signedIn: boolean = true, - isAdmin: boolean = false, - permissions: { [permission: string]: number } = {}): Session { - const id = generateSessionId() - const sig = generateSignature(id, secret) + secret: string, + signedIn: boolean = true, + isAdmin: boolean = false, + permissions: { [permission: string]: number } = {}): Session { + const id = generateSessionId() + const sig = generateSignature(id, secret) - return new Session({ - '.id': id, - '.client.signature': sig, - '.hijacked': undefined, - '.oauth2_nonce': 'LBvC2UC8EJ4FbpNfUlrOchBgXk//9WZYezudvWpd5txyx3ziELR7AcajZvam2XoMNBTGTgIddrdMs1ccE9seUw==', - '.zxs_key': 'CxKb2u0GILQPQalUuIYy1ZjL3QquDuYgnedwIafZC7V3mqJ0wH988/VZUMZMvlCs7rYLVHRvEagnYT8TBb9E3w==', - expires: Date.now() + 3600 * 1000, - last_access: Date.now(), - pst: 'all', - signin_info: { - access_token: { - // tslint:disable-next-line: max-line-length - access_token: '/T+R3ABq5SPPbZWSeePnrDE1122FEZSAGRuhmn21aZSqm5UQt/wqixlSViQPOrWe2iFb8PeYjZzmNehMA3JCJg==', - expires_in: 3600, - token_type: 'Bearer' - }, - admin_permissions: isAdmin ? '1' : '0', - signed_in: signedIn ? 1 : 0, - user_profile: { - id: 'sA==', - email: 'test', - forename: 'tester', - surname: 'test', - locale: 'GB_en', - permissions, - scope: undefined - } - } - }) + return new Session({ + '.id': id, + '.client.signature': sig, + '.hijacked': undefined, + '.oauth2_nonce': 'LBvC2UC8EJ4FbpNfUlrOchBgXk//9WZYezudvWpd5txyx3ziELR7AcajZvam2XoMNBTGTgIddrdMs1ccE9seUw==', + '.zxs_key': 'CxKb2u0GILQPQalUuIYy1ZjL3QquDuYgnedwIafZC7V3mqJ0wH988/VZUMZMvlCs7rYLVHRvEagnYT8TBb9E3w==', + expires: Date.now() + 3600 * 1000, + last_access: Date.now(), + pst: 'all', + signin_info: { + access_token: { + // tslint:disable-next-line: max-line-length + access_token: '/T+R3ABq5SPPbZWSeePnrDE1122FEZSAGRuhmn21aZSqm5UQt/wqixlSViQPOrWe2iFb8PeYjZzmNehMA3JCJg==', + expires_in: 3600, + token_type: 'Bearer' + }, + admin_permissions: isAdmin ? '1' : '0', + signed_in: signedIn ? 1 : 0, + user_profile: { + id: 'sA==', + email: 'test', + forename: 'tester', + surname: 'test', + locale: 'GB_en', + permissions, + scope: undefined + } + } + }) } From 7c366d118792fdef0d26a0c844e68300a0a919fe Mon Sep 17 00:00:00 2001 From: mateuszc-kainos Date: Mon, 8 Jun 2020 13:06:53 +0200 Subject: [PATCH 07/13] Fix URL --- src/middleware/auth.middleware.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/middleware/auth.middleware.ts b/src/middleware/auth.middleware.ts index 09c52713..6bc40bb1 100644 --- a/src/middleware/auth.middleware.ts +++ b/src/middleware/auth.middleware.ts @@ -5,7 +5,7 @@ import { provide } from 'inversify-binding-decorators' import { BaseMiddleware } from 'inversify-express-utils' import { authMiddleware, AuthOptions } from 'web-security-node' -import { FORM_PAGE_URI } from 'app/paths' +import { SEARCH_COMPANY_URI } from 'app/paths' import { getEnvOrDefault } from 'app/utils/env.util' import { newUriFactory } from 'app/utils/uri.factory' @@ -17,7 +17,7 @@ export class AuthMiddleware extends BaseMiddleware { } public getReturnToPage(req: Request): string { - return newUriFactory(req).createAbsoluteUri(FORM_PAGE_URI) + return newUriFactory(req).createAbsoluteUri(SEARCH_COMPANY_URI) } public handler: RequestHandler = (req: Request, res: Response, next: NextFunction): void => { From cd3d064c27a069eea1bebe47bd0fa729aaab001a Mon Sep 17 00:00:00 2001 From: mateuszc-kainos Date: Mon, 8 Jun 2020 15:58:07 +0200 Subject: [PATCH 08/13] Remove comment --- src/application.factory.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/application.factory.ts b/src/application.factory.ts index eb674024..642f5dc4 100644 --- a/src/application.factory.ts +++ b/src/application.factory.ts @@ -14,8 +14,6 @@ import { addFilters } from 'app/utils/nunjucks.util' import { createSession } from 'test/utils/session/SessionFactory' -// import { getEnvOrThrow } from 'app/utils/env.util' - // tslint:disable-next-line: no-empty export const createApp = (configureBindings: (container: Container) => void = () => { }): Application => { From f65a0051c3bec8aba010c0ae538baf469c62a206 Mon Sep 17 00:00:00 2001 From: mateuszc-kainos Date: Tue, 9 Jun 2020 17:02:45 +0200 Subject: [PATCH 09/13] Code review refactoring --- package-lock.json | 2 +- package.json | 2 +- src/application.factory.ts | 45 +++++++------------ src/inversify.config.ts | 3 ++ src/middleware/auth.middleware.ts | 37 ++++++++------- src/middleware/logger.ts | 13 ------ src/types.ts | 3 +- src/utils/uri.factory.ts | 14 ++---- test/auth.middleware.test.ts | 30 +++++++++++++ .../searchCompany.controller.test.ts | 8 +++- test/utils/session/SessionFactory.ts | 42 ----------------- tsconfig.json | 1 + 12 files changed, 83 insertions(+), 117 deletions(-) delete mode 100644 src/middleware/logger.ts create mode 100644 test/auth.middleware.test.ts delete mode 100644 test/utils/session/SessionFactory.ts diff --git a/package-lock.json b/package-lock.json index b98159d3..2fed6ee8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12705,7 +12705,7 @@ }, "web-security-node": { "version": "git+ssh://git@github.com/companieshouse/web-security-node.git#e35f2199b35f729eec2445d38d2b40a588e85ece", - "from": "git+ssh://git@github.com/companieshouse/web-security-node.git", + "from": "git+ssh://git@github.com/companieshouse/web-security-node.git#e35f2199b35f729eec2445d38d2b40a588e85ece", "requires": { "ch-logging": "git+ssh://git@github.com/companieshouse/ch-structured-logging-node.git#a3e0dcc57f91f63803e02ceef2cc5e4038de9a19", "ch-node-session-handler": "git+ssh://git@github.com/companieshouse/node-session-handler.git#2.1.1" diff --git a/package.json b/package.json index 2ee086b9..83fe3de3 100644 --- a/package.json +++ b/package.json @@ -37,7 +37,7 @@ "reflect-metadata": "^0.1.13", "tsconfig-paths": "^3.9.0", "tslib": "^2.0.0", - "web-security-node": "git@github.com:companieshouse/web-security-node.git" + "web-security-node": "git@github.com:companieshouse/web-security-node.git#e35f2199b35f729eec2445d38d2b40a588e85ece" }, "devDependencies": { "@types/body-parser": "^1.19.0", diff --git a/src/application.factory.ts b/src/application.factory.ts index 642f5dc4..1bc0b67c 100644 --- a/src/application.factory.ts +++ b/src/application.factory.ts @@ -1,25 +1,22 @@ +import { AuthMiddleware } from 'app/middleware/auth.middleware' import * as bodyParser from 'body-parser' import ApplicationLogger from 'ch-logging/lib/ApplicationLogger' -import { Session, SessionMiddleware, SessionStore } from 'ch-node-session-handler' -import { SessionKey } from 'ch-node-session-handler/lib/session/keys/SessionKey' -import { Cookie } from 'ch-node-session-handler/lib/session/model/Cookie' +import { SessionMiddleware } from 'ch-node-session-handler' import { Application, NextFunction, Request, Response } from 'express' import { Container } from 'inversify' import { buildProviderModule } from 'inversify-binding-decorators' import { InversifyExpressServer } from 'inversify-express-utils' import * as nunjucks from 'nunjucks' -import { deepEqual, instance, mock, when } from 'ts-mockito' +import { anything, instance, mock, when, } from 'ts-mockito' import { addFilters } from 'app/utils/nunjucks.util' -import { createSession } from 'test/utils/session/SessionFactory' // tslint:disable-next-line: no-empty -export const createApp = (configureBindings: (container: Container) => void = () => { -}): Application => { +export const createApp = (configureBindings?: (container: Container) => void): Application => { const container: Container = new Container() container.load(buildProviderModule()) - configureBindings(container) + configureBindings?.(container) return new InversifyExpressServer(container) .setConfig(server => { @@ -46,31 +43,19 @@ export const createApp = (configureBindings: (container: Container) => void = () .build() } -// tslint:disable-next-line: no-empty -export const createAppWithFakeSession = (configureBindings: (container: Container) => void = () => { -}): Application => { +export const createAppWithFakeSession = (configureBindings?: (container: Container) => void): Application => { return createApp(container => { - const cookieName = '__SID' - const cookieSecret = 'ChGovUk-XQrbf3sLj2abFxIY2TlapsJ ' - - const session: Session = createSession(cookieSecret) - - const sessionId = session.data[SessionKey.Id] - const signature = session.data[SessionKey.ClientSig] - - const cookie = Cookie.createFrom(sessionId! + signature) - - const sessionStore = mock(SessionStore) - when(sessionStore.load(deepEqual(cookie))).thenResolve(session.data) - const mockSessionStore = instance(sessionStore) + const authMiddleware: AuthMiddleware = mock(AuthMiddleware) + when(authMiddleware.handler(anything(), anything(), anything)).thenCall((_1, _2, next) => { + console.log('auth middleware hit!') + next() + }) + container.rebind(AuthMiddleware).toConstantValue(instance(authMiddleware)) container.bind(ApplicationLogger).toConstantValue(mock(instance(ApplicationLogger))) - container.bind(SessionMiddleware).toConstantValue((req: Request, res: Response, next: NextFunction) => { - req.cookies = {} - req.cookies[cookieName] = cookie.value - SessionMiddleware({cookieName, cookieSecret}, mockSessionStore)(req, res, next) - }) + container.bind(SessionMiddleware).toConstantValue((_1: Request, _2: Response, next: NextFunction) => next()) + - configureBindings(container) + configureBindings?.(container) }) } \ No newline at end of file diff --git a/src/inversify.config.ts b/src/inversify.config.ts index 350d8b4c..5794f0d8 100644 --- a/src/inversify.config.ts +++ b/src/inversify.config.ts @@ -1,4 +1,5 @@ import 'reflect-metadata' +import { UriFactory } from 'app/utils/uri.factory' import { createLogger } from 'ch-logging' import ApplicationLogger from 'ch-logging/lib/ApplicationLogger' @@ -21,9 +22,11 @@ export function initContainer(): Container { container.bind(TYPES.CDN_HOST).toConstantValue(getEnvOrThrow('CDN_HOST')) container.bind>(TYPES.PIWIK_SITE_ID).toConstantValue(getEnv('PIWIK_SITE_ID')) container.bind>(TYPES.PIWIK_URL).toConstantValue(getEnv('PIWIK_URL')) + container.bind>(TYPES.ACCOUNT_WEB_URL).toConstantValue(getEnv('ACCOUNT_WEB_URL')) // Utils container.bind(ApplicationLogger).toConstantValue(createLogger(APP_NAME)) + container.bind(UriFactory).toConstantValue(new UriFactory()) // Session const config: CookieConfig = { diff --git a/src/middleware/auth.middleware.ts b/src/middleware/auth.middleware.ts index 6bc40bb1..23ed396d 100644 --- a/src/middleware/auth.middleware.ts +++ b/src/middleware/auth.middleware.ts @@ -1,4 +1,4 @@ -import ApplicationLogger from 'ch-logging/lib/ApplicationLogger'; +import ApplicationLogger from 'ch-logging/lib/ApplicationLogger' import { NextFunction, Request, RequestHandler, Response } from 'express' import { inject } from 'inversify' import { provide } from 'inversify-binding-decorators' @@ -6,26 +6,29 @@ import { BaseMiddleware } from 'inversify-express-utils' import { authMiddleware, AuthOptions } from 'web-security-node' import { SEARCH_COMPANY_URI } from 'app/paths' -import { getEnvOrDefault } from 'app/utils/env.util' -import { newUriFactory } from 'app/utils/uri.factory' +import TYPES from 'app/types' +import { UriFactory } from 'app/utils/uri.factory' @provide(AuthMiddleware) export class AuthMiddleware extends BaseMiddleware { - public constructor(@inject(ApplicationLogger) private logger: ApplicationLogger) { - super() - } + public constructor( + @inject(ApplicationLogger) private logger: ApplicationLogger, + @inject(TYPES.ACCOUNT_WEB_URL) private ACCOUNT_WEB_URL: string, + @inject(UriFactory) private uriFactory: UriFactory) { + super() + } - public getReturnToPage(req: Request): string { - return newUriFactory(req).createAbsoluteUri(SEARCH_COMPANY_URI) - } + private getReturnToPage(req: Request): string { + return this.uriFactory.createAbsoluteUri(req, SEARCH_COMPANY_URI) + } - public handler: RequestHandler = (req: Request, res: Response, next: NextFunction): void => { - const authOptions = { - returnUrl: this.getReturnToPage(req), - accountWebUrl: getEnvOrDefault('ACCOUNT_WEB_URL', ''), - logger: this.logger - } as AuthOptions - authMiddleware(authOptions)(req, res, next) - } + public handler: RequestHandler = (req: Request, res: Response, next: NextFunction): void => { + const authOptions = { + returnUrl: this.getReturnToPage(req), + accountWebUrl: this.ACCOUNT_WEB_URL, + logger: this.logger + } as AuthOptions + authMiddleware(authOptions)(req, res, next) + } } \ No newline at end of file diff --git a/src/middleware/logger.ts b/src/middleware/logger.ts deleted file mode 100644 index 788ec5ce..00000000 --- a/src/middleware/logger.ts +++ /dev/null @@ -1,13 +0,0 @@ -import { createLogger } from 'ch-logging' -import ApplicationLogger from 'ch-logging/lib/ApplicationLogger' - -import { APP_NAME } from 'app/constants/app.const' - -let logger: ApplicationLogger - -export function loggerInstance(): ApplicationLogger { - if (!logger) { - logger = createLogger(APP_NAME) - } - return logger -} diff --git a/src/types.ts b/src/types.ts index 8340879a..114a3b78 100644 --- a/src/types.ts +++ b/src/types.ts @@ -3,7 +3,8 @@ const TYPES = { NODE_ENV: 'NODE_ENV', CDN_HOST: 'CDN_HOST', PIWIK_URL: 'PIWIK_URL', - PIWIK_SITE_ID: 'PIWIK_SITE_ID' + PIWIK_SITE_ID: 'PIWIK_SITE_ID', + ACCOUNT_WEB_URL: 'ACCOUNT_WEB_URL' } export default TYPES diff --git a/src/utils/uri.factory.ts b/src/utils/uri.factory.ts index 1de80667..63be0988 100644 --- a/src/utils/uri.factory.ts +++ b/src/utils/uri.factory.ts @@ -1,13 +1,7 @@ import { Request } from 'express' -export interface UriFactory { - createAbsoluteUri(path: string): string -} - -export const newUriFactory = (req: Request): UriFactory => { - return { - createAbsoluteUri(path: string): string { - return new URL(path, `${req.protocol}://${req.headers.host}`).href - } - } +export class UriFactory { + public createAbsoluteUri(req: Request, path: string): string { + return new URL(path, `${req.protocol}://${req.headers.host}`).href + } } diff --git a/test/auth.middleware.test.ts b/test/auth.middleware.test.ts new file mode 100644 index 00000000..63831fac --- /dev/null +++ b/test/auth.middleware.test.ts @@ -0,0 +1,30 @@ +import 'reflect-metadata' + +import ApplicationLogger from 'ch-logging/lib/ApplicationLogger' +import { instance, mock, verify, } from 'ts-mockito' +import { authMiddleware as commonMiddleware } from 'web-security-node' + +import { AuthMiddleware } from 'app/middleware/auth.middleware' +import { UriFactory } from 'app/utils/uri.factory' + +describe('AuthMiddleware', () => { + it('when handling request should call common auth library', () => { + const middleware = new AuthMiddleware( + instance(mock(ApplicationLogger)), + 'EXAMPLE_URL', + instance(mock(UriFactory))) + + const req = {} + const res = { + redirect: () => undefined + } + const func = () => void + + middleware.handler(req as any, res as any, func) + verify(commonMiddleware({ + req, + res, + next: func() + } as any)).called() + }) +}) \ No newline at end of file diff --git a/test/controllers/searchCompany.controller.test.ts b/test/controllers/searchCompany.controller.test.ts index d901d981..9e0fa9ec 100644 --- a/test/controllers/searchCompany.controller.test.ts +++ b/test/controllers/searchCompany.controller.test.ts @@ -2,16 +2,20 @@ import 'reflect-metadata' import { assert } from 'chai' import { Application } from 'express' +import { OK } from 'http-status-codes' import request from 'supertest' import { createAppWithFakeSession } from 'app/application.factory' import 'app/controllers/searchCompany.controller' import { SEARCH_COMPANY_URI } from 'app/paths' + import TYPES from 'app/types' describe('SearchCompanyController', () => { it('should return 200 when trying to access page with a session', async () => { - const app: Application = createAppWithFakeSession() - const res = await request(app).get(SEARCH_COMPANY_URI).expect(200) + const app: Application = createAppWithFakeSession(container => { + container.bind(TYPES.ACCOUNT_WEB_URL).toConstantValue('') + }) + const res = await request(app).get(SEARCH_COMPANY_URI).expect(OK) assert.include(res.text, 'Search Company Placeholder') }) diff --git a/test/utils/session/SessionFactory.ts b/test/utils/session/SessionFactory.ts deleted file mode 100644 index 24d79386..00000000 --- a/test/utils/session/SessionFactory.ts +++ /dev/null @@ -1,42 +0,0 @@ -import { Session } from 'ch-node-session-handler/lib/session/model/Session' -import { generateSessionId, generateSignature } from 'ch-node-session-handler/lib/utils/CookieUtils' - - -export function createSession( - secret: string, - signedIn: boolean = true, - isAdmin: boolean = false, - permissions: { [permission: string]: number } = {}): Session { - const id = generateSessionId() - const sig = generateSignature(id, secret) - - return new Session({ - '.id': id, - '.client.signature': sig, - '.hijacked': undefined, - '.oauth2_nonce': 'LBvC2UC8EJ4FbpNfUlrOchBgXk//9WZYezudvWpd5txyx3ziELR7AcajZvam2XoMNBTGTgIddrdMs1ccE9seUw==', - '.zxs_key': 'CxKb2u0GILQPQalUuIYy1ZjL3QquDuYgnedwIafZC7V3mqJ0wH988/VZUMZMvlCs7rYLVHRvEagnYT8TBb9E3w==', - expires: Date.now() + 3600 * 1000, - last_access: Date.now(), - pst: 'all', - signin_info: { - access_token: { - // tslint:disable-next-line: max-line-length - access_token: '/T+R3ABq5SPPbZWSeePnrDE1122FEZSAGRuhmn21aZSqm5UQt/wqixlSViQPOrWe2iFb8PeYjZzmNehMA3JCJg==', - expires_in: 3600, - token_type: 'Bearer' - }, - admin_permissions: isAdmin ? '1' : '0', - signed_in: signedIn ? 1 : 0, - user_profile: { - id: 'sA==', - email: 'test', - forename: 'tester', - surname: 'test', - locale: 'GB_en', - permissions, - scope: undefined - } - } - }) -} diff --git a/tsconfig.json b/tsconfig.json index 5aace5e0..4b2b922d 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -7,6 +7,7 @@ "noImplicitReturns": true, "noFallthroughCasesInSwitch": true, "pretty": true, + "sourceMap": true, "baseUrl": "./src", "removeComments": true, "experimentalDecorators": true, From 3963a3b473c98829f9f5a520c827d6b5ef1a9d28 Mon Sep 17 00:00:00 2001 From: Peadar Kelly Date: Tue, 9 Jun 2020 18:03:55 +0100 Subject: [PATCH 10/13] Updated middlewares to have a TYPES definition to decouple the implementation from the inverisfy binding. Updated search company tests to use a mocked version of auth middleware. Updated auth middleware definition to enable mocking in unit tests. Moved app factory into test/controllers as it is only used by controllers --- package-lock.json | 134 ++++++++++++++++++ package.json | 2 + src/controllers/searchCompany.controller.ts | 5 +- src/inversify.config.ts | 14 +- src/middleware/auth.middleware.ts | 36 ++--- src/types.ts | 4 +- src/utils/uri.factory.ts | 4 +- test/auth.middleware.test.ts | 30 ---- .../controllers}/application.factory.ts | 30 ++-- test/controllers/form.controller.test.ts | 2 +- test/controllers/landing.controller.test.ts | 2 +- .../searchCompany.controller.test.ts | 8 +- test/middleware/auth.middleware.test.ts | 56 ++++++++ 13 files changed, 232 insertions(+), 95 deletions(-) delete mode 100644 test/auth.middleware.test.ts rename {src => test/controllers}/application.factory.ts (53%) create mode 100644 test/middleware/auth.middleware.test.ts diff --git a/package-lock.json b/package-lock.json index 2fed6ee8..abcdfcb6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -443,6 +443,51 @@ "integrity": "sha512-9NET910DNaIPngYnLLPeg+Ogzqsi9uM4mSboU5y6p8S5DzMTVEsJZrawi+BoDNUVBa2DhJqQYUFvMDfgU062LQ==", "dev": true }, + "@sinonjs/commons": { + "version": "1.8.0", + "resolved": "https://registry.npmjs.org/@sinonjs/commons/-/commons-1.8.0.tgz", + "integrity": "sha512-wEj54PfsZ5jGSwMX68G8ZXFawcSglQSXqCftWX3ec8MDUzQdHgcKvw97awHbY0efQEL5iKUOAmmVtoYgmrSG4Q==", + "dev": true, + "requires": { + "type-detect": "4.0.8" + } + }, + "@sinonjs/fake-timers": { + "version": "6.0.1", + "resolved": "https://registry.npmjs.org/@sinonjs/fake-timers/-/fake-timers-6.0.1.tgz", + "integrity": "sha512-MZPUxrmFubI36XS1DI3qmI0YdN1gks62JtFZvxR67ljjSNCeK6U08Zx4msEWOXuofgqUt6zPHSi1H9fbjR/NRA==", + "dev": true, + "requires": { + "@sinonjs/commons": "^1.7.0" + } + }, + "@sinonjs/formatio": { + "version": "5.0.1", + "resolved": "https://registry.npmjs.org/@sinonjs/formatio/-/formatio-5.0.1.tgz", + "integrity": "sha512-KaiQ5pBf1MpS09MuA0kp6KBQt2JUOQycqVG1NZXvzeaXe5LGFqAKueIS0bw4w0P9r7KuBSVdUk5QjXsUdu2CxQ==", + "dev": true, + "requires": { + "@sinonjs/commons": "^1", + "@sinonjs/samsam": "^5.0.2" + } + }, + "@sinonjs/samsam": { + "version": "5.0.3", + "resolved": "https://registry.npmjs.org/@sinonjs/samsam/-/samsam-5.0.3.tgz", + "integrity": "sha512-QucHkc2uMJ0pFGjJUDP3F9dq5dx8QIaqISl9QgwLOh6P9yv877uONPGXh/OH/0zmM3tW1JjuJltAZV2l7zU+uQ==", + "dev": true, + "requires": { + "@sinonjs/commons": "^1.6.0", + "lodash.get": "^4.4.2", + "type-detect": "^4.0.8" + } + }, + "@sinonjs/text-encoding": { + "version": "0.7.1", + "resolved": "https://registry.npmjs.org/@sinonjs/text-encoding/-/text-encoding-0.7.1.tgz", + "integrity": "sha512-+iTbntw2IZPb/anVDbypzfQa+ay64MW0Zo8aJ8gZPWMMK6/OubMVb6lUPMagqjOPnmtauXnFCACVl3O7ogjeqQ==", + "dev": true + }, "@szmarczak/http-timer": { "version": "1.1.2", "resolved": "https://registry.npmjs.org/@szmarczak/http-timer/-/http-timer-1.1.2.tgz", @@ -640,6 +685,21 @@ "@types/mime": "*" } }, + "@types/sinon": { + "version": "9.0.4", + "resolved": "https://registry.npmjs.org/@types/sinon/-/sinon-9.0.4.tgz", + "integrity": "sha512-sJmb32asJZY6Z2u09bl0G2wglSxDlROlAejCjsnor+LzBMz17gu8IU7vKC/vWDnv9zEq2wqADHVXFjf4eE8Gdw==", + "dev": true, + "requires": { + "@types/sinonjs__fake-timers": "*" + } + }, + "@types/sinonjs__fake-timers": { + "version": "6.0.1", + "resolved": "https://registry.npmjs.org/@types/sinonjs__fake-timers/-/sinonjs__fake-timers-6.0.1.tgz", + "integrity": "sha512-yYezQwGWty8ziyYLdZjwxyMb0CZR49h8JALHGrxjQHWlqGgc8kLdHEgWrgL0uZ29DMvEVBDnHU2Wg36zKSIUtA==", + "dev": true + }, "@types/superagent": { "version": "4.1.7", "resolved": "https://registry.npmjs.org/@types/superagent/-/superagent-4.1.7.tgz", @@ -7443,6 +7503,12 @@ "integrity": "sha1-h/zPrv/AtozRnVX2cilD+SnqNeo=", "dev": true }, + "just-extend": { + "version": "4.1.0", + "resolved": "https://registry.npmjs.org/just-extend/-/just-extend-4.1.0.tgz", + "integrity": "sha512-ApcjaOdVTJ7y4r08xI5wIqpvwS48Q0PBG4DJROcEkH1f8MdAiNFyFxz3xoL0LWAVwjrwPYZdVHHxhRHcx/uGLA==", + "dev": true + }, "keyv": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/keyv/-/keyv-3.1.0.tgz", @@ -8436,6 +8502,36 @@ "integrity": "sha1-yobR/ogoFpsBICCOPchCS524NCw=", "dev": true }, + "nise": { + "version": "4.0.3", + "resolved": "https://registry.npmjs.org/nise/-/nise-4.0.3.tgz", + "integrity": "sha512-EGlhjm7/4KvmmE6B/UFsKh7eHykRl9VH+au8dduHLCyWUO/hr7+N+WtTvDUwc9zHuM1IaIJs/0lQ6Ag1jDkQSg==", + "dev": true, + "requires": { + "@sinonjs/commons": "^1.7.0", + "@sinonjs/fake-timers": "^6.0.0", + "@sinonjs/text-encoding": "^0.7.1", + "just-extend": "^4.0.2", + "path-to-regexp": "^1.7.0" + }, + "dependencies": { + "isarray": { + "version": "0.0.1", + "resolved": "https://registry.npmjs.org/isarray/-/isarray-0.0.1.tgz", + "integrity": "sha1-ihis/Kmo9Bd+Cav8YDiTmwXR7t8=", + "dev": true + }, + "path-to-regexp": { + "version": "1.8.0", + "resolved": "https://registry.npmjs.org/path-to-regexp/-/path-to-regexp-1.8.0.tgz", + "integrity": "sha512-n43JRhlUKUAlibEJhPeir1ncUID16QnEjNpwzNdO3Lm4ywrBpBZ5oLD0I6br9evr1Y9JTqwRtAh7JLoOzAQdVA==", + "dev": true, + "requires": { + "isarray": "0.0.1" + } + } + } + }, "nock": { "version": "12.0.3", "resolved": "https://registry.npmjs.org/nock/-/nock-12.0.3.tgz", @@ -10982,6 +11078,44 @@ "is-arrayish": "^0.3.1" } }, + "sinon": { + "version": "9.0.2", + "resolved": "https://registry.npmjs.org/sinon/-/sinon-9.0.2.tgz", + "integrity": "sha512-0uF8Q/QHkizNUmbK3LRFqx5cpTttEVXudywY9Uwzy8bTfZUhljZ7ARzSxnRHWYWtVTeh4Cw+tTb3iU21FQVO9A==", + "dev": true, + "requires": { + "@sinonjs/commons": "^1.7.2", + "@sinonjs/fake-timers": "^6.0.1", + "@sinonjs/formatio": "^5.0.1", + "@sinonjs/samsam": "^5.0.3", + "diff": "^4.0.2", + "nise": "^4.0.1", + "supports-color": "^7.1.0" + }, + "dependencies": { + "diff": { + "version": "4.0.2", + "resolved": "https://registry.npmjs.org/diff/-/diff-4.0.2.tgz", + "integrity": "sha512-58lmxKSA4BNyLz+HHMUzlOEpg09FV+ev6ZMe3vJihgdxzgcwZ8VoEEPmALCZG9LmqfVoNMMKpttIYTVG6uDY7A==", + "dev": true + }, + "has-flag": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/has-flag/-/has-flag-4.0.0.tgz", + "integrity": "sha512-EykJT/Q1KjTWctppgIAgfSO0tKVuZUjhgMr17kqTumMl6Afv3EISleU7qZUzoXDFTAHTDC4NOoG/ZxU3EvlMPQ==", + "dev": true + }, + "supports-color": { + "version": "7.1.0", + "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-7.1.0.tgz", + "integrity": "sha512-oRSIpR8pxT1Wr2FquTNnGet79b3BWljqOuoW/h4oBhxJ/HUbX5nX6JSruTkvXDCFMwDPvsaTTbvMLKZWSy0R5g==", + "dev": true, + "requires": { + "has-flag": "^4.0.0" + } + } + } + }, "slash": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/slash/-/slash-3.0.0.tgz", diff --git a/package.json b/package.json index 83fe3de3..ddfff944 100644 --- a/package.json +++ b/package.json @@ -51,6 +51,7 @@ "@types/mocha": "^7.0.2", "@types/multer": "^1.4.3", "@types/nunjucks": "^3.1.3", + "@types/sinon": "^9.0.4", "@types/supertest": "^2.0.9", "chai": "^4.2.0", "del": "^5.1.0", @@ -67,6 +68,7 @@ "node-sass": "^4.14.1", "nyc": "^15.0.1", "sass-lint": "^1.13.1", + "sinon": "^9.0.2", "sonarqube-scanner": "^2.6.0", "supertest": "^4.0.2", "ts-mockito": "^2.5.0", diff --git a/src/controllers/searchCompany.controller.ts b/src/controllers/searchCompany.controller.ts index 7c7de269..365e5473 100644 --- a/src/controllers/searchCompany.controller.ts +++ b/src/controllers/searchCompany.controller.ts @@ -1,11 +1,10 @@ -import { SessionMiddleware } from 'ch-node-session-handler' import { controller, httpGet } from 'inversify-express-utils' import BaseController from 'app/controllers/base.controller' -import { AuthMiddleware } from 'app/middleware/auth.middleware' import { SEARCH_COMPANY_URI } from 'app/paths' +import TYPES from 'app/types' -@controller(SEARCH_COMPANY_URI, SessionMiddleware, AuthMiddleware) +@controller(SEARCH_COMPANY_URI, TYPES.SessionMiddleware, TYPES.AuthMiddleware) export class SearchCompanyController extends BaseController { @httpGet('') diff --git a/src/inversify.config.ts b/src/inversify.config.ts index 5794f0d8..02e49d06 100644 --- a/src/inversify.config.ts +++ b/src/inversify.config.ts @@ -1,5 +1,4 @@ import 'reflect-metadata' -import { UriFactory } from 'app/utils/uri.factory' import { createLogger } from 'ch-logging' import ApplicationLogger from 'ch-logging/lib/ApplicationLogger' @@ -7,11 +6,14 @@ import { CookieConfig, SessionMiddleware, SessionStore } from 'ch-node-session-h import { Container } from 'inversify' import { buildProviderModule } from 'inversify-binding-decorators' import IORedis from 'ioredis' -import Optional from './models/optional' +import { authMiddleware } from 'web-security-node' import { APP_NAME } from 'app/constants/app.const' +import Optional from 'app/models/optional' import TYPES from 'app/types' import { getEnv, getEnvOrDefault, getEnvOrThrow } from 'app/utils/env.util' +import UriFactory from 'app/utils/uri.factory' +import AuthMiddleware from 'app/middleware/auth.middleware' export function initContainer(): Container { const container: Container = new Container() @@ -22,7 +24,6 @@ export function initContainer(): Container { container.bind(TYPES.CDN_HOST).toConstantValue(getEnvOrThrow('CDN_HOST')) container.bind>(TYPES.PIWIK_SITE_ID).toConstantValue(getEnv('PIWIK_SITE_ID')) container.bind>(TYPES.PIWIK_URL).toConstantValue(getEnv('PIWIK_URL')) - container.bind>(TYPES.ACCOUNT_WEB_URL).toConstantValue(getEnv('ACCOUNT_WEB_URL')) // Utils container.bind(ApplicationLogger).toConstantValue(createLogger(APP_NAME)) @@ -35,7 +36,12 @@ export function initContainer(): Container { } const sessionStore = new SessionStore(new IORedis(`${getEnvOrThrow('CACHE_SERVER')}`)) container.bind(SessionStore).toConstantValue(sessionStore) - container.bind(SessionMiddleware).toConstantValue(SessionMiddleware(config, sessionStore)) + container.bind(TYPES.SessionMiddleware).toConstantValue(SessionMiddleware(config, sessionStore)) + container.bind(TYPES.AuthMiddleware).toConstantValue(AuthMiddleware( + getEnvOrThrow('ACCOUNT_WEB_URL'), + new UriFactory(), + authMiddleware + )) container.load(buildProviderModule()) diff --git a/src/middleware/auth.middleware.ts b/src/middleware/auth.middleware.ts index 23ed396d..edfbe196 100644 --- a/src/middleware/auth.middleware.ts +++ b/src/middleware/auth.middleware.ts @@ -1,34 +1,16 @@ -import ApplicationLogger from 'ch-logging/lib/ApplicationLogger' import { NextFunction, Request, RequestHandler, Response } from 'express' -import { inject } from 'inversify' -import { provide } from 'inversify-binding-decorators' -import { BaseMiddleware } from 'inversify-express-utils' -import { authMiddleware, AuthOptions } from 'web-security-node' +import { AuthOptions } from 'web-security-node' import { SEARCH_COMPANY_URI } from 'app/paths' -import TYPES from 'app/types' -import { UriFactory } from 'app/utils/uri.factory' +import UriFactory from 'app/utils/uri.factory' -@provide(AuthMiddleware) -export class AuthMiddleware extends BaseMiddleware { +export default function AuthMiddleware(accountWebUrl: string, uriFactory: UriFactory, commonAuthMiddleware: (opts: AuthOptions) => RequestHandler): RequestHandler { + return (req: Request, res: Response, next: NextFunction) => { + const authOptions: AuthOptions = { + returnUrl: uriFactory.createAbsoluteUri(req, SEARCH_COMPANY_URI), + accountWebUrl: accountWebUrl + } - public constructor( - @inject(ApplicationLogger) private logger: ApplicationLogger, - @inject(TYPES.ACCOUNT_WEB_URL) private ACCOUNT_WEB_URL: string, - @inject(UriFactory) private uriFactory: UriFactory) { - super() - } - - private getReturnToPage(req: Request): string { - return this.uriFactory.createAbsoluteUri(req, SEARCH_COMPANY_URI) - } - - public handler: RequestHandler = (req: Request, res: Response, next: NextFunction): void => { - const authOptions = { - returnUrl: this.getReturnToPage(req), - accountWebUrl: this.ACCOUNT_WEB_URL, - logger: this.logger - } as AuthOptions - authMiddleware(authOptions)(req, res, next) + return commonAuthMiddleware(authOptions)(req, res, next) } } \ No newline at end of file diff --git a/src/types.ts b/src/types.ts index 114a3b78..35f53869 100644 --- a/src/types.ts +++ b/src/types.ts @@ -4,7 +4,9 @@ const TYPES = { CDN_HOST: 'CDN_HOST', PIWIK_URL: 'PIWIK_URL', PIWIK_SITE_ID: 'PIWIK_SITE_ID', - ACCOUNT_WEB_URL: 'ACCOUNT_WEB_URL' + ACCOUNT_WEB_URL: 'ACCOUNT_WEB_URL', + SessionMiddleware: 'SessionMiddleware', + AuthMiddleware: 'AuthMiddleware' } export default TYPES diff --git a/src/utils/uri.factory.ts b/src/utils/uri.factory.ts index 63be0988..db7e94d7 100644 --- a/src/utils/uri.factory.ts +++ b/src/utils/uri.factory.ts @@ -1,6 +1,8 @@ import { Request } from 'express' +import { provide } from 'inversify-binding-decorators' -export class UriFactory { +@provide(UriFactory) +export default class UriFactory { public createAbsoluteUri(req: Request, path: string): string { return new URL(path, `${req.protocol}://${req.headers.host}`).href } diff --git a/test/auth.middleware.test.ts b/test/auth.middleware.test.ts deleted file mode 100644 index 63831fac..00000000 --- a/test/auth.middleware.test.ts +++ /dev/null @@ -1,30 +0,0 @@ -import 'reflect-metadata' - -import ApplicationLogger from 'ch-logging/lib/ApplicationLogger' -import { instance, mock, verify, } from 'ts-mockito' -import { authMiddleware as commonMiddleware } from 'web-security-node' - -import { AuthMiddleware } from 'app/middleware/auth.middleware' -import { UriFactory } from 'app/utils/uri.factory' - -describe('AuthMiddleware', () => { - it('when handling request should call common auth library', () => { - const middleware = new AuthMiddleware( - instance(mock(ApplicationLogger)), - 'EXAMPLE_URL', - instance(mock(UriFactory))) - - const req = {} - const res = { - redirect: () => undefined - } - const func = () => void - - middleware.handler(req as any, res as any, func) - verify(commonMiddleware({ - req, - res, - next: func() - } as any)).called() - }) -}) \ No newline at end of file diff --git a/src/application.factory.ts b/test/controllers/application.factory.ts similarity index 53% rename from src/application.factory.ts rename to test/controllers/application.factory.ts index 1bc0b67c..98b0017a 100644 --- a/src/application.factory.ts +++ b/test/controllers/application.factory.ts @@ -1,20 +1,18 @@ -import { AuthMiddleware } from 'app/middleware/auth.middleware' import * as bodyParser from 'body-parser' -import ApplicationLogger from 'ch-logging/lib/ApplicationLogger' -import { SessionMiddleware } from 'ch-node-session-handler' import { Application, NextFunction, Request, Response } from 'express' import { Container } from 'inversify' import { buildProviderModule } from 'inversify-binding-decorators' import { InversifyExpressServer } from 'inversify-express-utils' import * as nunjucks from 'nunjucks' -import { anything, instance, mock, when, } from 'ts-mockito' import { addFilters } from 'app/utils/nunjucks.util' +import TYPES from '../../src/types' - -// tslint:disable-next-line: no-empty export const createApp = (configureBindings?: (container: Container) => void): Application => { const container: Container = new Container() + + mockMiddlewares(container) + container.load(buildProviderModule()) configureBindings?.(container) @@ -43,19 +41,7 @@ export const createApp = (configureBindings?: (container: Container) => void): A .build() } -export const createAppWithFakeSession = (configureBindings?: (container: Container) => void): Application => { - return createApp(container => { - const authMiddleware: AuthMiddleware = mock(AuthMiddleware) - when(authMiddleware.handler(anything(), anything(), anything)).thenCall((_1, _2, next) => { - console.log('auth middleware hit!') - next() - }) - container.rebind(AuthMiddleware).toConstantValue(instance(authMiddleware)) - - container.bind(ApplicationLogger).toConstantValue(mock(instance(ApplicationLogger))) - container.bind(SessionMiddleware).toConstantValue((_1: Request, _2: Response, next: NextFunction) => next()) - - - configureBindings?.(container) - }) -} \ No newline at end of file +const mockMiddlewares = (container: Container): void => { + container.bind(TYPES.SessionMiddleware).toConstantValue((_1: Request, _2: Response, next: NextFunction) => next()) + container.bind(TYPES.AuthMiddleware).toConstantValue((_1: Request, _2: Response, next: NextFunction) => next()) +} diff --git a/test/controllers/form.controller.test.ts b/test/controllers/form.controller.test.ts index 58f5a970..e85cdfc9 100644 --- a/test/controllers/form.controller.test.ts +++ b/test/controllers/form.controller.test.ts @@ -5,7 +5,7 @@ import { BAD_REQUEST, MOVED_TEMPORARILY, OK } from 'http-status-codes' import request from 'supertest' import { deepEqual, instance, mock, when } from 'ts-mockito' -import { createApp } from 'app/application.factory' +import { createApp } from './application.factory' import 'app/controllers/form.controller' import ValidationErrors from 'app/models/validationErrors' import { FORM_PAGE_URI } from 'app/paths' diff --git a/test/controllers/landing.controller.test.ts b/test/controllers/landing.controller.test.ts index 711988a7..c56241fa 100644 --- a/test/controllers/landing.controller.test.ts +++ b/test/controllers/landing.controller.test.ts @@ -5,7 +5,7 @@ import { Application } from 'express' import { OK } from 'http-status-codes' import request from 'supertest' -import { createApp } from 'app/application.factory' +import { createApp } from './application.factory' import 'app/controllers/landing.controller' const app: Application = createApp() diff --git a/test/controllers/searchCompany.controller.test.ts b/test/controllers/searchCompany.controller.test.ts index 9e0fa9ec..c82ebee6 100644 --- a/test/controllers/searchCompany.controller.test.ts +++ b/test/controllers/searchCompany.controller.test.ts @@ -5,16 +5,14 @@ import { Application } from 'express' import { OK } from 'http-status-codes' import request from 'supertest' -import { createAppWithFakeSession } from 'app/application.factory' +import { createApp } from './application.factory' import 'app/controllers/searchCompany.controller' import { SEARCH_COMPANY_URI } from 'app/paths' - import TYPES from 'app/types' describe('SearchCompanyController', () => { it('should return 200 when trying to access page with a session', async () => { - const app: Application = createAppWithFakeSession(container => { - container.bind(TYPES.ACCOUNT_WEB_URL).toConstantValue('') - }) + const app: Application = createApp() + const res = await request(app).get(SEARCH_COMPANY_URI).expect(OK) assert.include(res.text, 'Search Company Placeholder') diff --git a/test/middleware/auth.middleware.test.ts b/test/middleware/auth.middleware.test.ts new file mode 100644 index 00000000..263f0723 --- /dev/null +++ b/test/middleware/auth.middleware.test.ts @@ -0,0 +1,56 @@ +import 'reflect-metadata' + +import { assert } from 'chai' +import * as sinon from 'sinon' +import { instance, mock, when } from 'ts-mockito' + +import AuthMiddleware from 'app/middleware/auth.middleware' +import UriFactory from 'app/utils/uri.factory' +import { RequestHandler, Request, Response, NextFunction } from 'express' +import { AuthOptions } from 'web-security-node' +import { SEARCH_COMPANY_URI } from 'app/paths' + +describe('AuthMiddleware', () => { + + let middleware: RequestHandler + + let uriFactory: UriFactory + let commonAuthStub: sinon.SinonStub + let authCallbackStub: sinon.SinonStub + + const accountWebUrl = 'some-account-url' + + beforeEach(() => { + uriFactory = mock(UriFactory) + authCallbackStub = sinon.stub() + commonAuthStub = sinon.stub().returns(authCallbackStub) + + middleware = AuthMiddleware( + accountWebUrl, + instance(uriFactory), + commonAuthStub + ) + }) + + it('should invoke common auth library with correct values when handling incoming request', () => { + const req = {} as Request + const res = {} as Response + const next = {} as NextFunction + + const expectedAuthOptions: AuthOptions = { + accountWebUrl, + returnUrl: 'some-uri' + } + + when(uriFactory.createAbsoluteUri(req, SEARCH_COMPANY_URI)).thenReturn('some-uri') + + middleware(req, res, next) + + assert.isTrue(commonAuthStub.calledOnce) + + const actualAuthOptions: AuthOptions = commonAuthStub.args[0][0] + assert.deepEqual(actualAuthOptions, expectedAuthOptions) + + assert.isTrue(authCallbackStub.calledOnceWith(req, res, next)) + }) +}) \ No newline at end of file From 09703b2f199eb89653fc621e59e859c895a55a11 Mon Sep 17 00:00:00 2001 From: mateuszc-kainos Date: Wed, 10 Jun 2020 10:52:36 +0200 Subject: [PATCH 11/13] Rename common auth middleware for clarity --- src/inversify.config.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/inversify.config.ts b/src/inversify.config.ts index 02e49d06..f57e77b3 100644 --- a/src/inversify.config.ts +++ b/src/inversify.config.ts @@ -6,14 +6,14 @@ import { CookieConfig, SessionMiddleware, SessionStore } from 'ch-node-session-h import { Container } from 'inversify' import { buildProviderModule } from 'inversify-binding-decorators' import IORedis from 'ioredis' -import { authMiddleware } from 'web-security-node' +import { authMiddleware as commonAuthMiddleware } from 'web-security-node' import { APP_NAME } from 'app/constants/app.const' +import AuthMiddleware from 'app/middleware/auth.middleware' import Optional from 'app/models/optional' import TYPES from 'app/types' import { getEnv, getEnvOrDefault, getEnvOrThrow } from 'app/utils/env.util' import UriFactory from 'app/utils/uri.factory' -import AuthMiddleware from 'app/middleware/auth.middleware' export function initContainer(): Container { const container: Container = new Container() @@ -40,7 +40,7 @@ export function initContainer(): Container { container.bind(TYPES.AuthMiddleware).toConstantValue(AuthMiddleware( getEnvOrThrow('ACCOUNT_WEB_URL'), new UriFactory(), - authMiddleware + commonAuthMiddleware )) container.load(buildProviderModule()) From 6fb5d6cb92085dbc8884c3dd5a385b3b1c48ed79 Mon Sep 17 00:00:00 2001 From: mateuszc-kainos Date: Wed, 10 Jun 2020 11:08:48 +0200 Subject: [PATCH 12/13] Fix import --- test/controllers/application.factory.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/controllers/application.factory.ts b/test/controllers/application.factory.ts index 98b0017a..2f46c410 100644 --- a/test/controllers/application.factory.ts +++ b/test/controllers/application.factory.ts @@ -5,8 +5,8 @@ import { buildProviderModule } from 'inversify-binding-decorators' import { InversifyExpressServer } from 'inversify-express-utils' import * as nunjucks from 'nunjucks' +import TYPES from 'app/types' import { addFilters } from 'app/utils/nunjucks.util' -import TYPES from '../../src/types' export const createApp = (configureBindings?: (container: Container) => void): Application => { const container: Container = new Container() From 6e8ebcfd606e57a2e35d520fb8dd73eb57a31639 Mon Sep 17 00:00:00 2001 From: mateuszc-kainos Date: Wed, 10 Jun 2020 12:18:41 +0200 Subject: [PATCH 13/13] Fix TSLint issues, add test --- src/controllers/searchCompany.controller.ts | 2 +- src/middleware/auth.middleware.ts | 5 +++-- test/controllers/form.controller.test.ts | 2 +- test/controllers/landing.controller.test.ts | 2 +- .../searchCompany.controller.test.ts | 5 +++-- test/middleware/auth.middleware.test.ts | 6 ++--- test/utils/uri.factory.test.ts | 22 +++++++++++++++++++ 7 files changed, 34 insertions(+), 10 deletions(-) create mode 100644 test/utils/uri.factory.test.ts diff --git a/src/controllers/searchCompany.controller.ts b/src/controllers/searchCompany.controller.ts index 365e5473..dffd895f 100644 --- a/src/controllers/searchCompany.controller.ts +++ b/src/controllers/searchCompany.controller.ts @@ -11,4 +11,4 @@ export class SearchCompanyController extends BaseController { public async get(): Promise { return super.render('search-company') } -} +} \ No newline at end of file diff --git a/src/middleware/auth.middleware.ts b/src/middleware/auth.middleware.ts index edfbe196..97d102b3 100644 --- a/src/middleware/auth.middleware.ts +++ b/src/middleware/auth.middleware.ts @@ -4,11 +4,12 @@ import { AuthOptions } from 'web-security-node' import { SEARCH_COMPANY_URI } from 'app/paths' import UriFactory from 'app/utils/uri.factory' -export default function AuthMiddleware(accountWebUrl: string, uriFactory: UriFactory, commonAuthMiddleware: (opts: AuthOptions) => RequestHandler): RequestHandler { +export default function AuthMiddleware(accountWebUrl: string, uriFactory: UriFactory, + commonAuthMiddleware: (opts: AuthOptions) => RequestHandler): RequestHandler { return (req: Request, res: Response, next: NextFunction) => { const authOptions: AuthOptions = { returnUrl: uriFactory.createAbsoluteUri(req, SEARCH_COMPANY_URI), - accountWebUrl: accountWebUrl + accountWebUrl } return commonAuthMiddleware(authOptions)(req, res, next) diff --git a/test/controllers/form.controller.test.ts b/test/controllers/form.controller.test.ts index e85cdfc9..a451050e 100644 --- a/test/controllers/form.controller.test.ts +++ b/test/controllers/form.controller.test.ts @@ -4,8 +4,8 @@ import { assert } from 'chai' import { BAD_REQUEST, MOVED_TEMPORARILY, OK } from 'http-status-codes' import request from 'supertest' import { deepEqual, instance, mock, when } from 'ts-mockito' - import { createApp } from './application.factory' + import 'app/controllers/form.controller' import ValidationErrors from 'app/models/validationErrors' import { FORM_PAGE_URI } from 'app/paths' diff --git a/test/controllers/landing.controller.test.ts b/test/controllers/landing.controller.test.ts index c56241fa..fc214fa6 100644 --- a/test/controllers/landing.controller.test.ts +++ b/test/controllers/landing.controller.test.ts @@ -4,8 +4,8 @@ import { assert } from 'chai' import { Application } from 'express' import { OK } from 'http-status-codes' import request from 'supertest' - import { createApp } from './application.factory' + import 'app/controllers/landing.controller' const app: Application = createApp() diff --git a/test/controllers/searchCompany.controller.test.ts b/test/controllers/searchCompany.controller.test.ts index c82ebee6..d43b375a 100644 --- a/test/controllers/searchCompany.controller.test.ts +++ b/test/controllers/searchCompany.controller.test.ts @@ -4,17 +4,18 @@ import { assert } from 'chai' import { Application } from 'express' import { OK } from 'http-status-codes' import request from 'supertest' - import { createApp } from './application.factory' + import 'app/controllers/searchCompany.controller' import { SEARCH_COMPANY_URI } from 'app/paths' describe('SearchCompanyController', () => { - it('should return 200 when trying to access page with a session', async () => { + it('should render the search company page', async () => { const app: Application = createApp() const res = await request(app).get(SEARCH_COMPANY_URI).expect(OK) + // TODO Replace with actual page components assert.include(res.text, 'Search Company Placeholder') }) }) diff --git a/test/middleware/auth.middleware.test.ts b/test/middleware/auth.middleware.test.ts index 263f0723..08a166d4 100644 --- a/test/middleware/auth.middleware.test.ts +++ b/test/middleware/auth.middleware.test.ts @@ -1,14 +1,14 @@ import 'reflect-metadata' import { assert } from 'chai' +import { NextFunction, Request, RequestHandler, Response } from 'express' import * as sinon from 'sinon' import { instance, mock, when } from 'ts-mockito' +import { AuthOptions } from 'web-security-node' import AuthMiddleware from 'app/middleware/auth.middleware' -import UriFactory from 'app/utils/uri.factory' -import { RequestHandler, Request, Response, NextFunction } from 'express' -import { AuthOptions } from 'web-security-node' import { SEARCH_COMPANY_URI } from 'app/paths' +import UriFactory from 'app/utils/uri.factory' describe('AuthMiddleware', () => { diff --git a/test/utils/uri.factory.test.ts b/test/utils/uri.factory.test.ts new file mode 100644 index 00000000..8ef52dd2 --- /dev/null +++ b/test/utils/uri.factory.test.ts @@ -0,0 +1,22 @@ +import 'reflect-metadata' + +import { assert } from 'chai' + +import UriFactory from 'app/utils/uri.factory' + +describe('UriFactory', () => { + it('should return absolute uri', () => { + const req = { + protocol: 'http', + headers: { + host: 'www.example.com' + } + } + const uriFactory = new UriFactory() + const uri = uriFactory.createAbsoluteUri(req as any, 'example-page') + + + assert.equal(uri, 'http://www.example.com/example-page') + }) + +})