From 7c3d6cc376464ec3323d5bbed8d43e44ecb32bc6 Mon Sep 17 00:00:00 2001 From: Clement Michaud Date: Mon, 18 Mar 2019 09:27:41 +0100 Subject: [PATCH 1/4] Log what is retrieved from headers to help debugging. --- scripts/authelia-scripts-suites-test | 2 +- scripts/authelia-scripts-unittest | 16 +- server/src/lib/ErrorReplies.ts | 39 ++-- .../src/lib/IdentityCheckMiddleware.spec.ts | 126 +++++-------- server/src/lib/Server.ts | 15 +- .../src/lib/routes/firstfactor/post.spec.ts | 27 +-- server/src/lib/routes/firstfactor/post.ts | 6 +- .../routes/password-reset/form/post.spec.ts | 23 +-- .../identity/PasswordResetHandler.spec.ts | 28 +-- .../lib/routes/secondfactor/redirect.spec.ts | 39 ++-- .../src/lib/routes/secondfactor/redirect.ts | 42 ++--- .../totp/identity/RegistrationHandler.spec.ts | 12 +- .../totp/identity/RegistrationHandler.ts | 12 +- .../secondfactor/totp/sign/post.spec.ts | 21 +-- .../u2f/identity/RegistrationHandler.spec.ts | 13 +- .../secondfactor/u2f/register/post.spec.ts | 11 +- .../u2f/register_request/get.spec.ts | 7 +- .../routes/secondfactor/u2f/sign/post.spec.ts | 6 +- .../secondfactor/u2f/sign_request/get.spec.ts | 8 +- server/src/lib/routes/verify/get.spec.ts | 60 +++--- server/src/lib/routes/verify/get.ts | 35 ++-- .../src/lib/routes/verify/get_basic_auth.ts | 7 +- .../lib/routes/verify/get_session_cookie.ts | 8 +- server/src/lib/stubs/express.spec.ts | 172 ++++++++++-------- server/src/lib/utils/GetHeader.spec.ts | 20 ++ server/src/lib/utils/GetHeader.ts | 19 ++ server/src/lib/utils/IsRedirectionSafe.ts | 3 - shared/DomainExtractor.spec.ts | 2 +- shared/constants.ts | 12 +- spec-helper.js | 3 + 30 files changed, 374 insertions(+), 420 deletions(-) create mode 100644 server/src/lib/utils/GetHeader.spec.ts create mode 100644 server/src/lib/utils/GetHeader.ts create mode 100644 spec-helper.js diff --git a/scripts/authelia-scripts-suites-test b/scripts/authelia-scripts-suites-test index 6e35e2fe..09ce874d 100755 --- a/scripts/authelia-scripts-suites-test +++ b/scripts/authelia-scripts-suites-test @@ -19,7 +19,7 @@ async function runTests(suite, withEnv) { return new Promise((resolve, reject) => { let mochaArgs = ['--exit', '--colors', '--require', 'ts-node/register', 'test/suites/' + suite + '/test.ts'] if (program.forbidOnly) { - mochaArgs = ['--forbid-only', '--forbid-pending'] + mochaArgs; + mochaArgs = ['--forbid-only', '--forbid-pending'].concat(mochaArgs); } const mochaCommand = './node_modules/.bin/mocha ' + mochaArgs.join(' '); diff --git a/scripts/authelia-scripts-unittest b/scripts/authelia-scripts-unittest index 8e7c113c..63d57df6 100755 --- a/scripts/authelia-scripts-unittest +++ b/scripts/authelia-scripts-unittest @@ -4,11 +4,17 @@ var program = require('commander'); var spawn = require('child_process').spawn; program + .option('--forbid-only', 'Forbid only and pending filters.') .parse(process.argv); -async function runTests(pattern) { +async function runTests(patterns) { return new Promise((resolve, reject) => { - mocha = spawn('./node_modules/.bin/mocha', ['--colors', '--require', 'ts-node/register', pattern], { + let mochaArgs = ['--require', 'ts-node/register', '--require', './spec-helper.js', '--colors']; + if (program.forbidOnly) { + mochaArgs = ['--forbid-only', '--forbid-pending'].concat(mochaArgs); + } + + const mocha = spawn('./node_modules/.bin/mocha', mochaArgs.concat(patterns), { env: { ...process.env, 'TS_NODE_PROJECT': './server/tsconfig.json' @@ -27,8 +33,10 @@ async function runTests(pattern) { } async function test() { - await runTests('server/src/**/*.spec.ts'); - await runTests('shared/**/*.spec.ts'); + await runTests([ + 'server/src/**/*.spec.ts', + 'shared/**/*.spec.ts' + ]); } test() diff --git a/server/src/lib/ErrorReplies.ts b/server/src/lib/ErrorReplies.ts index 93bc8984..85a8d817 100644 --- a/server/src/lib/ErrorReplies.ts +++ b/server/src/lib/ErrorReplies.ts @@ -1,25 +1,18 @@ -import express = require("express"); +import * as Express from "express"; import { IRequestLogger } from "./logging/IRequestLogger"; -function replyWithError(req: express.Request, res: express.Response, +function replyWithError(req: Express.Request, res: Express.Response, code: number, logger: IRequestLogger, body?: Object): (err: Error) => void { return function (err: Error): void { - if (req.originalUrl.startsWith("/api/") || code == 200) { - logger.error(req, "Reply with error %d: %s", code, err.message); - logger.debug(req, "%s", err.stack); - res.status(code); - res.send(body); - } - else { - logger.error(req, "Redirect to error %d: %s", code, err.message); - logger.debug(req, "%s", err.stack); - res.redirect("/error/" + code); - } + logger.error(req, "Reply with error %d: %s", code, err.message); + logger.debug(req, "%s", err.stack); + res.status(code); + res.send(body); }; } -export function redirectTo(redirectUrl: string, req: express.Request, - res: express.Response, logger: IRequestLogger) { +export function redirectTo(redirectUrl: string, req: Express.Request, + res: Express.Response, logger: IRequestLogger) { return function(err: Error) { logger.error(req, "Error: %s", err.message); logger.debug(req, "Redirecting to %s", redirectUrl); @@ -27,22 +20,22 @@ export function redirectTo(redirectUrl: string, req: express.Request, }; } -export function replyWithError400(req: express.Request, - res: express.Response, logger: IRequestLogger) { +export function replyWithError400(req: Express.Request, + res: Express.Response, logger: IRequestLogger) { return replyWithError(req, res, 400, logger); } -export function replyWithError401(req: express.Request, - res: express.Response, logger: IRequestLogger) { +export function replyWithError401(req: Express.Request, + res: Express.Response, logger: IRequestLogger) { return replyWithError(req, res, 401, logger); } -export function replyWithError403(req: express.Request, - res: express.Response, logger: IRequestLogger) { +export function replyWithError403(req: Express.Request, + res: Express.Response, logger: IRequestLogger) { return replyWithError(req, res, 403, logger); } -export function replyWithError200(req: express.Request, - res: express.Response, logger: IRequestLogger, message: string) { +export function replyWithError200(req: Express.Request, + res: Express.Response, logger: IRequestLogger, message: string) { return replyWithError(req, res, 200, logger, { error: message }); } \ No newline at end of file diff --git a/server/src/lib/IdentityCheckMiddleware.spec.ts b/server/src/lib/IdentityCheckMiddleware.spec.ts index db32e4e0..b6f38f76 100644 --- a/server/src/lib/IdentityCheckMiddleware.spec.ts +++ b/server/src/lib/IdentityCheckMiddleware.spec.ts @@ -1,10 +1,9 @@ -import sinon = require("sinon"); +import * as Sinon from "sinon"; import * as IdentityCheckMiddleware from "./IdentityCheckMiddleware"; -import exceptions = require("./Exceptions"); import { ServerVariables } from "./ServerVariables"; -import Assert = require("assert"); -import express = require("express"); +import * as Assert from "assert"; +import * as Express from "express"; import BluebirdPromise = require("bluebird"); import ExpressMock = require("./stubs/express.spec"); import { IdentityValidableStub } from "./IdentityValidableStub.spec"; @@ -13,11 +12,11 @@ import { ServerVariablesMock, ServerVariablesMockBuilder } import { OPERATION_FAILED } from "../../../shared/UserMessages"; describe("IdentityCheckMiddleware", function () { - let req: ExpressMock.RequestMock; + let req: Express.Request; let res: ExpressMock.ResponseMock; - let app: express.Application; - let app_get: sinon.SinonStub; - let app_post: sinon.SinonStub; + let app: Express.Application; + let app_get: Sinon.SinonStub; + let app_post: Sinon.SinonStub; let identityValidable: IdentityValidableStub; let mocks: ServerVariablesMock; let vars: ServerVariables; @@ -29,14 +28,6 @@ describe("IdentityCheckMiddleware", function () { req = ExpressMock.RequestMock(); res = ExpressMock.ResponseMock(); - - req.headers = {}; - req.originalUrl = "/non-api/xxx"; - req.session = {}; - - req.query = {}; - req.app = {}; - identityValidable = new IdentityValidableStub(); mocks.notifier.notifyStub.returns(BluebirdPromise.resolve()); @@ -45,9 +36,9 @@ describe("IdentityCheckMiddleware", function () { mocks.userDataStore.consumeIdentityValidationTokenStub .returns(BluebirdPromise.resolve({ userId: "user" })); - app = express(); - app_get = sinon.stub(app, "get"); - app_post = sinon.stub(app, "post"); + app = Express(); + app_get = Sinon.stub(app, "get"); + app_post = Sinon.stub(app, "post"); }); afterEach(function () { @@ -56,22 +47,8 @@ describe("IdentityCheckMiddleware", function () { }); describe("test start GET", function () { - it("should redirect to error 401 if pre validation initialization \ -throws a first factor error", function () { - identityValidable.preValidationInitStub.returns(BluebirdPromise.reject( - new exceptions.FirstFactorValidationError( - "Error during prevalidation"))); - const callback = IdentityCheckMiddleware.post_start_validation( - identityValidable, vars); - - return callback(req as any, res as any, undefined) - .then(() => { - Assert(res.redirect.calledWith("/error/401")); - }); - }); - // In that case we answer with 200 to avoid user enumeration. - it("should send 200 if email is missing in provided identity", function () { + it("should send 200 if email is missing in provided identity", async function () { const identity = { userid: "abc" }; identityValidable.preValidationInitStub @@ -79,31 +56,26 @@ throws a first factor error", function () { const callback = IdentityCheckMiddleware .post_start_validation(identityValidable, vars); - return callback(req as any, res as any, undefined) - .then(function () { - Assert(identityValidable.preValidationResponseStub.called); - }); + await callback(req as any, res as any, undefined) + Assert(identityValidable.preValidationResponseStub.called); }); // In that case we answer with 200 to avoid user enumeration. - it("should send 200 if userid is missing in provided identity", - function () { - const identity = { email: "abc@example.com" }; + it("should send 200 if userid is missing in provided identity", async function () { + const identity = { email: "abc@example.com" }; - identityValidable.preValidationInitStub - .returns(BluebirdPromise.resolve(identity)); - const callback = IdentityCheckMiddleware - .post_start_validation(identityValidable, vars); + identityValidable.preValidationInitStub + .returns(BluebirdPromise.resolve(identity)); + const callback = IdentityCheckMiddleware + .post_start_validation(identityValidable, vars); - return callback(req as any, res as any, undefined) - .then(function () { - Assert(identityValidable.preValidationResponseStub.called); - }); - }); + await callback(req as any, res as any, undefined) + Assert(identityValidable.preValidationResponseStub.called); + }); it("should issue a token, send an email and return 204", async function () { const identity = { userid: "user", email: "abc@example.com" }; - req.get = sinon.stub().withArgs("Host").returns("localhost"); + req.get = Sinon.stub().withArgs("Host").returns("localhost"); identityValidable.preValidationInitStub .returns(BluebirdPromise.resolve(identity)); @@ -124,42 +96,36 @@ throws a first factor error", function () { describe("test finish GET", function () { - it("should return an error if no identity_token is provided", () => { + it("should return an error if no identity_token is provided", async () => { const callback = IdentityCheckMiddleware .post_finish_validation(identityValidable, vars); - return callback(req as any, res as any, undefined) - .then(function () { - Assert(res.status.calledWith(200)); - Assert(res.send.calledWith({'error': OPERATION_FAILED})); - }); + await callback(req as any, res as any, undefined) + Assert(res.status.calledWith(200)); + Assert(res.send.calledWith({'error': OPERATION_FAILED})); }); - it("should call postValidation if identity_token is provided and still \ -valid", function () { - req.query.identity_token = "token"; - const callback = IdentityCheckMiddleware - .post_finish_validation(identityValidable, vars); - return callback(req as any, res as any, undefined); - }); + it("should call postValidation if identity_token is provided and still valid", async function () { + req.query.identity_token = "token"; + const callback = IdentityCheckMiddleware + .post_finish_validation(identityValidable, vars); + await callback(req as any, res as any, undefined); + }); - it("should return an error if identity_token is provided but invalid", - function () { - req.query.identity_token = "token"; + it("should return an error if identity_token is provided but invalid", async function () { + req.query.identity_token = "token"; - identityValidable.postValidationInitStub - .returns(BluebirdPromise.resolve()); - mocks.userDataStore.consumeIdentityValidationTokenStub.reset(); - mocks.userDataStore.consumeIdentityValidationTokenStub - .returns(BluebirdPromise.reject(new Error("Invalid token"))); + identityValidable.postValidationInitStub + .returns(BluebirdPromise.resolve()); + mocks.userDataStore.consumeIdentityValidationTokenStub.reset(); + mocks.userDataStore.consumeIdentityValidationTokenStub + .returns(() => Promise.reject(new Error("Invalid token"))); - const callback = IdentityCheckMiddleware - .post_finish_validation(identityValidable, vars); - return callback(req as any, res as any, undefined) - .then(() => { - Assert(res.status.calledWith(200)); - Assert(res.send.calledWith({'error': OPERATION_FAILED})); - }); - }); + const callback = IdentityCheckMiddleware + .post_finish_validation(identityValidable, vars); + await callback(req as any, res as any, undefined) + Assert(res.status.calledWith(200)); + Assert(res.send.calledWith({'error': OPERATION_FAILED})); + }); }); }); diff --git a/server/src/lib/Server.ts b/server/src/lib/Server.ts index c3d54a11..c36854d1 100644 --- a/server/src/lib/Server.ts +++ b/server/src/lib/Server.ts @@ -1,4 +1,6 @@ -import BluebirdPromise = require("bluebird"); +import * as Bluebird from "bluebird"; +import * as Express from "express"; +import * as http from "http"; import { Configuration } from "./configuration/schema/Configuration"; import { GlobalDependencies } from "../../types/Dependencies"; @@ -9,8 +11,7 @@ import { ServerVariables } from "./ServerVariables"; import { ServerVariablesInitializer } from "./ServerVariablesInitializer"; import { Configurator } from "./web_server/Configurator"; -import * as Express from "express"; -import * as http from "http"; +import { GET_VARIABLE_KEY } from "../../../shared/constants"; function clone(obj: any) { return JSON.parse(JSON.stringify(obj)); @@ -44,11 +45,11 @@ export default class Server { JSON.stringify(displayableConfiguration, undefined, 2)); } - private setup(config: Configuration, app: Express.Application, deps: GlobalDependencies): BluebirdPromise { - const that = this; + private setup(config: Configuration, app: Express.Application, deps: GlobalDependencies): Bluebird { return ServerVariablesInitializer.initialize( config, this.globalLogger, this.requestLogger, deps) .then(function (vars: ServerVariables) { + app.set(GET_VARIABLE_KEY, vars); return Configurator.configure(config, app, vars, deps); }); } @@ -56,7 +57,7 @@ export default class Server { private startServer(app: Express.Application, port: number) { const that = this; that.globalLogger.info("Starting Authelia..."); - return new BluebirdPromise((resolve, reject) => { + return new Bluebird((resolve, reject) => { this.httpServer = app.listen(port, function (err: string) { that.globalLogger.info("Listening on port %d...", port); resolve(); @@ -65,7 +66,7 @@ export default class Server { } start(configuration: Configuration, deps: GlobalDependencies) - : BluebirdPromise { + : Bluebird { const that = this; const app = Express(); diff --git a/server/src/lib/routes/firstfactor/post.spec.ts b/server/src/lib/routes/firstfactor/post.spec.ts index fb2a7f3e..eb5fc6a3 100644 --- a/server/src/lib/routes/firstfactor/post.spec.ts +++ b/server/src/lib/routes/firstfactor/post.spec.ts @@ -1,16 +1,17 @@ +import * as Express from 'express'; import BluebirdPromise = require("bluebird"); import Assert = require("assert"); import FirstFactorPost = require("./post"); import exceptions = require("../../Exceptions"); import { AuthenticationSessionHandler } from "../../AuthenticationSessionHandler"; import { AuthenticationSession } from "../../../../types/AuthenticationSession"; -import ExpressMock = require("../../stubs/express.spec"); +import * as ExpressMock from "../../stubs/express.spec"; import { ServerVariablesMock, ServerVariablesMockBuilder } from "../../ServerVariablesMockBuilder.spec"; import { ServerVariables } from "../../ServerVariables"; import AuthenticationError from "../../authentication/AuthenticationError"; describe("routes/firstfactor/post", function () { - let req: ExpressMock.RequestMock; + let req: Express.Request; let res: ExpressMock.ResponseMock; let emails: string[]; let groups: string[]; @@ -29,23 +30,11 @@ describe("routes/firstfactor/post", function () { mocks.regulator.regulateStub.returns(BluebirdPromise.resolve()); mocks.regulator.markStub.returns(BluebirdPromise.resolve()); - req = { - originalUrl: "/api/firstfactor", - body: { - username: "username", - password: "password" - }, - query: { - redirect: "http://redirect.url" - }, - session: { - cookie: {} - }, - headers: { - host: "home.example.com" - } - }; - + req = ExpressMock.RequestMock(); + req.body = { + username: "username", + password: "password" + } res = ExpressMock.ResponseMock(); authSession = AuthenticationSessionHandler.get(req as any, vars.logger); }); diff --git a/server/src/lib/routes/firstfactor/post.ts b/server/src/lib/routes/firstfactor/post.ts index 3dcb4dd2..f5aec487 100644 --- a/server/src/lib/routes/firstfactor/post.ts +++ b/server/src/lib/routes/firstfactor/post.ts @@ -1,4 +1,3 @@ -import * as ObjectPath from "object-path"; import BluebirdPromise = require("bluebird"); import express = require("express"); import ErrorReplies = require("../../ErrorReplies"); @@ -16,6 +15,7 @@ import { Subject } from "../../../lib/authorization/Subject"; import AuthenticationError from "../../../lib/authentication/AuthenticationError"; import IsRedirectionSafe from "../../../lib/utils/IsRedirectionSafe"; import * as URLParse from "url-parse"; +import GetHeader from "../../utils/GetHeader"; export default function (vars: ServerVariables) { return function (req: express.Request, res: express.Response) @@ -63,7 +63,7 @@ export default function (vars: ServerVariables) { vars.regulator.mark(username, true); }) .then(function() { - const targetUrl = ObjectPath.get(req, "headers.x-target-url", undefined); + const targetUrl = GetHeader(req, "x-target-url"); if (!targetUrl) { res.status(204); @@ -85,7 +85,7 @@ export default function (vars: ServerVariables) { const authorizationLevel = vars.authorizer.authorization(resObject, subject); if (authorizationLevel <= AuthorizationLevel.ONE_FACTOR) { - if (IsRedirectionSafe(vars, authSession, new URLParse(targetUrl))) { + if (IsRedirectionSafe(vars, new URLParse(targetUrl))) { res.json({redirect: targetUrl}); return BluebirdPromise.resolve(); } else { diff --git a/server/src/lib/routes/password-reset/form/post.spec.ts b/server/src/lib/routes/password-reset/form/post.spec.ts index ed029c90..87b03390 100644 --- a/server/src/lib/routes/password-reset/form/post.spec.ts +++ b/server/src/lib/routes/password-reset/form/post.spec.ts @@ -1,9 +1,7 @@ - +import * as Express from "express"; import PasswordResetFormPost = require("./post"); import { AuthenticationSessionHandler } from "../../../AuthenticationSessionHandler"; import { AuthenticationSession } from "../../../../../types/AuthenticationSession"; -import { UserDataStore } from "../../../storage/UserDataStore"; -import Sinon = require("sinon"); import Assert = require("assert"); import BluebirdPromise = require("bluebird"); import ExpressMock = require("../../../stubs/express.spec"); @@ -12,32 +10,19 @@ import { ServerVariables } from "../../../ServerVariables"; import { Level } from "../../../authentication/Level"; describe("routes/password-reset/form/post", function () { - let req: ExpressMock.RequestMock; + let req: Express.Request; let res: ExpressMock.ResponseMock; let vars: ServerVariables; let mocks: ServerVariablesMock; let authSession: AuthenticationSession; beforeEach(function () { - req = { - originalUrl: "/api/password-reset", - body: { - userid: "user" - }, - session: {}, - headers: { - host: "localhost" - } - }; + req = ExpressMock.RequestMock(); const s = ServerVariablesMockBuilder.build(); mocks = s.mocks; vars = s.variables; - const options = { - inMemoryOnly: true - }; - mocks.userDataStore.saveU2FRegistrationStub.returns(BluebirdPromise.resolve({})); mocks.userDataStore.retrieveU2FRegistrationStub.returns(BluebirdPromise.resolve({})); mocks.userDataStore.produceIdentityValidationTokenStub.returns(BluebirdPromise.resolve({})); @@ -65,7 +50,6 @@ describe("routes/password-reset/form/post", function () { describe("test reset password post", () => { it("should update the password and reset auth_session for reauthentication", function () { - req.body = {}; req.body.password = "new-password"; mocks.usersDatabase.updatePasswordStub.returns(BluebirdPromise.resolve()); @@ -99,7 +83,6 @@ describe("routes/password-reset/form/post", function () { }); it("should fail when ldap fails", function () { - req.body = {}; req.body.password = "new-password"; mocks.usersDatabase.updatePasswordStub diff --git a/server/src/lib/routes/password-reset/identity/PasswordResetHandler.spec.ts b/server/src/lib/routes/password-reset/identity/PasswordResetHandler.spec.ts index cd3a5775..b3f3980b 100644 --- a/server/src/lib/routes/password-reset/identity/PasswordResetHandler.spec.ts +++ b/server/src/lib/routes/password-reset/identity/PasswordResetHandler.spec.ts @@ -1,4 +1,4 @@ - +import * as Express from "express"; import PasswordResetHandler from "./PasswordResetHandler"; import BluebirdPromise = require("bluebird"); @@ -8,28 +8,20 @@ import { ServerVariablesMock, ServerVariablesMockBuilder } import { ServerVariables } from "../../../ServerVariables"; describe("routes/password-reset/identity/PasswordResetHandler", function () { - let req: ExpressMock.RequestMock; + let req: Express.Request; let mocks: ServerVariablesMock; let vars: ServerVariables; beforeEach(function () { - req = { - originalUrl: "/non-api/xxx", - body: { - username: "user" - }, - session: { - auth: { - userid: "user", - email: "user@example.com", - first_factor: true, - second_factor: false - } - }, - headers: { - host: "localhost" - } + req = ExpressMock.RequestMock(); + req.body.username = "user"; + req.session.auth = { + userid: "user", + email: "user@example.com", + first_factor: true, + second_factor: false }; + req.headers.host = "localhost"; const s = ServerVariablesMockBuilder.build(); mocks = s.mocks; diff --git a/server/src/lib/routes/secondfactor/redirect.spec.ts b/server/src/lib/routes/secondfactor/redirect.spec.ts index ea66e6dc..af11fb53 100644 --- a/server/src/lib/routes/secondfactor/redirect.spec.ts +++ b/server/src/lib/routes/secondfactor/redirect.spec.ts @@ -1,41 +1,44 @@ +import * as Express from "express"; import Redirect from "./redirect"; import ExpressMock = require("../../stubs/express.spec"); -import { ServerVariablesMockBuilder, ServerVariablesMock } +import { ServerVariablesMockBuilder } from "../../ServerVariablesMockBuilder.spec"; import { ServerVariables } from "../../ServerVariables"; import Assert = require("assert"); +import { HEADER_X_TARGET_URL } from "../../../../../shared/constants"; describe("routes/secondfactor/redirect", function() { - let req: ExpressMock.RequestMock; + let req: Express.Request; let res: ExpressMock.ResponseMock; - let mocks: ServerVariablesMock; let vars: ServerVariables; beforeEach(function () { const s = ServerVariablesMockBuilder.build(); - mocks = s.mocks; vars = s.variables; req = ExpressMock.RequestMock(); res = ExpressMock.ResponseMock(); + vars.config.session.domain = 'example.com'; }); - it("should redirect to default_redirection_url", function() { - vars.config.default_redirection_url = "http://default_redirection_url"; - Redirect(vars)(req as any, res as any) - .then(function() { - Assert(res.json.calledWith({ - redirect: "http://default_redirection_url" - })); + describe('redirect to default url if no target provided', () => { + it("should redirect to default url", async () => { + vars.config.default_redirection_url = "https://home.example.com"; + await Redirect(vars)(req, res as any) + Assert(res.json.calledWith({redirect: "https://home.example.com"})); }); }); - it("should redirect to /", function() { - Redirect(vars)(req as any, res as any) - .then(function() { - Assert(res.json.calledWith({ - redirect: "/" - })); - }); + it("should redirect to safe url https://test.example.com/", async () => { + req.headers[HEADER_X_TARGET_URL] = "https://test.example.com/"; + await Redirect(vars)(req, res as any); + Assert(res.json.calledWith({redirect: "https://test.example.com/"})); }); + + it('should not redirect to unsafe target url', async () => { + vars.config.default_redirection_url = "https://home.example.com"; + req.headers[HEADER_X_TARGET_URL] = "http://test.example.com/"; + await Redirect(vars)(req, res as any); + Assert(res.status.calledWith(204)); + }) }); \ No newline at end of file diff --git a/server/src/lib/routes/secondfactor/redirect.ts b/server/src/lib/routes/secondfactor/redirect.ts index e0af16cc..87d55c43 100644 --- a/server/src/lib/routes/secondfactor/redirect.ts +++ b/server/src/lib/routes/secondfactor/redirect.ts @@ -1,39 +1,27 @@ -import express = require("express"); +import * as Express from "express"; import * as URLParse from "url-parse"; -import * as ObjectPath from "object-path"; import { ServerVariables } from "../../ServerVariables"; -import BluebirdPromise = require("bluebird"); -import ErrorReplies = require("../../ErrorReplies"); -import UserMessages = require("../../../../../shared/UserMessages"); import IsRedirectionSafe from "../../../lib/utils/IsRedirectionSafe"; -import { AuthenticationSessionHandler } from "../../../lib/AuthenticationSessionHandler"; +import GetHeader from "../../utils/GetHeader"; +import { HEADER_X_TARGET_URL } from "../../../../../shared/constants"; export default function (vars: ServerVariables) { - return function (req: express.Request, res: express.Response) - : BluebirdPromise { + return async function (req: Express.Request, res: Express.Response): Promise { + let redirectUrl = GetHeader(req, HEADER_X_TARGET_URL); - return new BluebirdPromise(function (resolve, reject) { - let redirectUrl: string = ObjectPath.get( - req, "headers.x-target-url", undefined); + if (!redirectUrl && vars.config.default_redirection_url) { + redirectUrl = vars.config.default_redirection_url; + } - if (!redirectUrl && vars.config.default_redirection_url) { - redirectUrl = vars.config.default_redirection_url; - } + if ((redirectUrl && !IsRedirectionSafe(vars, new URLParse(redirectUrl))) + || !redirectUrl) { + res.status(204); + res.send(); + return; + } - const authSession = AuthenticationSessionHandler.get(req, vars.logger); - if ((redirectUrl && !IsRedirectionSafe(vars, authSession, new URLParse(redirectUrl))) - || !redirectUrl) { - res.status(204); - res.send(); - return resolve(); - } - - res.json({redirect: redirectUrl}); - return resolve(); - }) - .catch(ErrorReplies.replyWithError200(req, res, vars.logger, - UserMessages.OPERATION_FAILED)); + res.json({redirect: redirectUrl}); }; } diff --git a/server/src/lib/routes/secondfactor/totp/identity/RegistrationHandler.spec.ts b/server/src/lib/routes/secondfactor/totp/identity/RegistrationHandler.spec.ts index 78b8ea3e..394ee141 100644 --- a/server/src/lib/routes/secondfactor/totp/identity/RegistrationHandler.spec.ts +++ b/server/src/lib/routes/secondfactor/totp/identity/RegistrationHandler.spec.ts @@ -1,7 +1,4 @@ -import Sinon = require("sinon"); import RegistrationHandler from "./RegistrationHandler"; -import { Identity } from "../../../../../../types/Identity"; -import { UserDataStore } from "../../../../storage/UserDataStore"; import BluebirdPromise = require("bluebird"); import ExpressMock = require("../../../../stubs/express.spec"); import { ServerVariablesMock, ServerVariablesMockBuilder } @@ -10,7 +7,7 @@ import { ServerVariables } from "../../../../ServerVariables"; import Assert = require("assert"); describe("routes/secondfactor/totp/identity/RegistrationHandler", function () { - let req: ExpressMock.RequestMock; + let req: Express.Request; let res: ExpressMock.ResponseMock; let mocks: ServerVariablesMock; let vars: ServerVariables; @@ -22,6 +19,7 @@ describe("routes/secondfactor/totp/identity/RegistrationHandler", function () { req = ExpressMock.RequestMock(); req.session = { + ...req.session, auth: { userid: "user", email: "user@example.com", @@ -33,12 +31,6 @@ describe("routes/secondfactor/totp/identity/RegistrationHandler", function () { } } }; - req.headers = {}; - req.headers.host = "localhost"; - - const options = { - inMemoryOnly: true - }; mocks.userDataStore.saveU2FRegistrationStub .returns(BluebirdPromise.resolve({})); diff --git a/server/src/lib/routes/secondfactor/totp/identity/RegistrationHandler.ts b/server/src/lib/routes/secondfactor/totp/identity/RegistrationHandler.ts index e7d67146..62d6581a 100644 --- a/server/src/lib/routes/secondfactor/totp/identity/RegistrationHandler.ts +++ b/server/src/lib/routes/secondfactor/totp/identity/RegistrationHandler.ts @@ -1,5 +1,5 @@ -import express = require("express"); +import * as Express from "express"; import BluebirdPromise = require("bluebird"); import { Identity } from "../../../../../../types/Identity"; @@ -35,7 +35,7 @@ export default class RegistrationHandler implements IdentityValidable { return Constants.CHALLENGE; } - private retrieveIdentity(req: express.Request): BluebirdPromise { + private retrieveIdentity(req: Express.Request): BluebirdPromise { const that = this; return new BluebirdPromise(function (resolve, reject) { const authSession = AuthenticationSessionHandler.get(req, that.logger); @@ -54,7 +54,7 @@ export default class RegistrationHandler implements IdentityValidable { }); } - preValidationInit(req: express.Request): BluebirdPromise { + preValidationInit(req: Express.Request): BluebirdPromise { const that = this; return FirstFactorValidator.validate(req, this.logger) .then(function () { @@ -62,16 +62,16 @@ export default class RegistrationHandler implements IdentityValidable { }); } - preValidationResponse(req: express.Request, res: express.Response) { + preValidationResponse(req: Express.Request, res: Express.Response) { res.status(204); res.send(); } - postValidationInit(req: express.Request) { + postValidationInit(req: Express.Request) { return FirstFactorValidator.validate(req, this.logger); } - postValidationResponse(req: express.Request, res: express.Response) + postValidationResponse(req: Express.Request, res: Express.Response) : BluebirdPromise { const that = this; let secret: TOTPSecret; diff --git a/server/src/lib/routes/secondfactor/totp/sign/post.spec.ts b/server/src/lib/routes/secondfactor/totp/sign/post.spec.ts index d0c29b70..58c367c4 100644 --- a/server/src/lib/routes/secondfactor/totp/sign/post.spec.ts +++ b/server/src/lib/routes/secondfactor/totp/sign/post.spec.ts @@ -1,20 +1,17 @@ import BluebirdPromise = require("bluebird"); -import Sinon = require("sinon"); import Assert = require("assert"); -import Exceptions = require("../../../../Exceptions"); +import * as Express from "express"; import { AuthenticationSessionHandler } from "../../../../AuthenticationSessionHandler"; import { AuthenticationSession } from "../../../../../../types/AuthenticationSession"; import SignPost = require("./post"); import { ServerVariables } from "../../../../ServerVariables"; - import ExpressMock = require("../../../../stubs/express.spec"); -import { UserDataStoreStub } from "../../../../storage/UserDataStoreStub.spec"; import { ServerVariablesMock, ServerVariablesMockBuilder } from "../../../../ServerVariablesMockBuilder.spec"; import { Level } from "../../../../authentication/Level"; describe("routes/secondfactor/totp/sign/post", function () { - let req: ExpressMock.RequestMock; + let req: Express.Request; let res: ExpressMock.ResponseMock; let authSession: AuthenticationSession; let vars: ServerVariables; @@ -24,17 +21,9 @@ describe("routes/secondfactor/totp/sign/post", function () { const s = ServerVariablesMockBuilder.build(); vars = s.variables; mocks = s.mocks; - const app_get = Sinon.stub(); - req = { - originalUrl: "/api/totp-register", - app: {}, - body: { - token: "abc" - }, - session: {}, - query: { - redirect: "http://redirect" - } + req = ExpressMock.RequestMock(); + req.body = { + token: "abc", }; res = ExpressMock.ResponseMock(); diff --git a/server/src/lib/routes/secondfactor/u2f/identity/RegistrationHandler.spec.ts b/server/src/lib/routes/secondfactor/u2f/identity/RegistrationHandler.spec.ts index a54bfbfe..ad65a1e8 100644 --- a/server/src/lib/routes/secondfactor/u2f/identity/RegistrationHandler.spec.ts +++ b/server/src/lib/routes/secondfactor/u2f/identity/RegistrationHandler.spec.ts @@ -1,16 +1,13 @@ +import * as Express from "express"; import Sinon = require("sinon"); -import Assert = require("assert"); import BluebirdPromise = require("bluebird"); - -import { Identity } from "../../../../../../types/Identity"; import RegistrationHandler from "./RegistrationHandler"; import ExpressMock = require("../../../../stubs/express.spec"); -import { UserDataStoreStub } from "../../../../storage/UserDataStoreStub.spec"; import { ServerVariablesMock, ServerVariablesMockBuilder } from "../../../../ServerVariablesMockBuilder.spec"; import { ServerVariables } from "../../../../ServerVariables"; describe("routes/secondfactor/u2f/identity/RegistrationHandler", function () { - let req: ExpressMock.RequestMock; + let req: Express.Request; let res: ExpressMock.ResponseMock; let mocks: ServerVariablesMock; let vars: ServerVariables; @@ -21,8 +18,8 @@ describe("routes/secondfactor/u2f/identity/RegistrationHandler", function () { vars = s.variables; req = ExpressMock.RequestMock(); - req.app = {}; req.session = { + ...req.session, auth: { userid: "user", email: "user@example.com", @@ -33,10 +30,6 @@ describe("routes/secondfactor/u2f/identity/RegistrationHandler", function () { req.headers = {}; req.headers.host = "localhost"; - const options = { - inMemoryOnly: true - }; - mocks.userDataStore.saveU2FRegistrationStub.returns(BluebirdPromise.resolve({})); mocks.userDataStore.retrieveU2FRegistrationStub.returns(BluebirdPromise.resolve({})); mocks.userDataStore.produceIdentityValidationTokenStub.returns(BluebirdPromise.resolve({})); diff --git a/server/src/lib/routes/secondfactor/u2f/register/post.spec.ts b/server/src/lib/routes/secondfactor/u2f/register/post.spec.ts index de3347a2..1bf7e103 100644 --- a/server/src/lib/routes/secondfactor/u2f/register/post.spec.ts +++ b/server/src/lib/routes/secondfactor/u2f/register/post.spec.ts @@ -1,4 +1,4 @@ - +import * as Express from "express"; import sinon = require("sinon"); import BluebirdPromise = require("bluebird"); import assert = require("assert"); @@ -6,13 +6,12 @@ import U2FRegisterPost = require("./post"); import { AuthenticationSessionHandler } from "../../../../AuthenticationSessionHandler"; import { AuthenticationSession } from "../../../../../../types/AuthenticationSession"; import ExpressMock = require("../../../../stubs/express.spec"); -import { UserDataStoreStub } from "../../../../storage/UserDataStoreStub.spec"; import { ServerVariablesMockBuilder, ServerVariablesMock } from "../../../../ServerVariablesMockBuilder.spec"; import { ServerVariables } from "../../../../ServerVariables"; describe("routes/secondfactor/u2f/register/post", function () { - let req: ExpressMock.RequestMock; + let req: Express.Request; let res: ExpressMock.ResponseMock; let mocks: ServerVariablesMock; let vars: ServerVariables; @@ -21,8 +20,8 @@ describe("routes/secondfactor/u2f/register/post", function () { beforeEach(function () { req = ExpressMock.RequestMock(); req.originalUrl = "/api/xxxx"; - req.app = {}; req.session = { + ...req.session, auth: { userid: "user", first_factor: true, @@ -40,10 +39,6 @@ describe("routes/secondfactor/u2f/register/post", function () { mocks = s.mocks; vars = s.variables; - const options = { - inMemoryOnly: true - }; - mocks.userDataStore.saveU2FRegistrationStub.returns(BluebirdPromise.resolve({})); mocks.userDataStore.retrieveU2FRegistrationStub.returns(BluebirdPromise.resolve({})); diff --git a/server/src/lib/routes/secondfactor/u2f/register_request/get.spec.ts b/server/src/lib/routes/secondfactor/u2f/register_request/get.spec.ts index 00d5ae5e..7dd19716 100644 --- a/server/src/lib/routes/secondfactor/u2f/register_request/get.spec.ts +++ b/server/src/lib/routes/secondfactor/u2f/register_request/get.spec.ts @@ -1,15 +1,14 @@ - +import * as Express from "express"; import sinon = require("sinon"); import BluebirdPromise = require("bluebird"); import Assert = require("assert"); import U2FRegisterRequestGet = require("./get"); import ExpressMock = require("../../../../stubs/express.spec"); -import { UserDataStoreStub } from "../../../../storage/UserDataStoreStub.spec"; import { ServerVariablesMockBuilder, ServerVariablesMock } from "../../../../ServerVariablesMockBuilder.spec"; import { ServerVariables } from "../../../../ServerVariables"; describe("routes/secondfactor/u2f/register_request/get", function () { - let req: ExpressMock.RequestMock; + let req: Express.Request; let res: ExpressMock.ResponseMock; let mocks: ServerVariablesMock; let vars: ServerVariables; @@ -17,8 +16,8 @@ describe("routes/secondfactor/u2f/register_request/get", function () { beforeEach(function () { req = ExpressMock.RequestMock(); req.originalUrl = "/api/xxxx"; - req.app = {}; req.session = { + ...req.session, auth: { userid: "user", first_factor: true, diff --git a/server/src/lib/routes/secondfactor/u2f/sign/post.spec.ts b/server/src/lib/routes/secondfactor/u2f/sign/post.spec.ts index 3473bf3e..9f95a0bb 100644 --- a/server/src/lib/routes/secondfactor/u2f/sign/post.spec.ts +++ b/server/src/lib/routes/secondfactor/u2f/sign/post.spec.ts @@ -1,4 +1,4 @@ - +import * as Express from "express"; import sinon = require("sinon"); import BluebirdPromise = require("bluebird"); import Assert = require("assert"); @@ -10,14 +10,13 @@ import ExpressMock = require("../../../../stubs/express.spec"); import { Level } from "../../../../authentication/Level"; describe("routes/secondfactor/u2f/sign/post", function () { - let req: ExpressMock.RequestMock; + let req: Express.Request; let res: ExpressMock.ResponseMock; let mocks: ServerVariablesMock; let vars: ServerVariables; beforeEach(function () { req = ExpressMock.RequestMock(); - req.app = {}; req.originalUrl = "/api/xxxx"; const s = ServerVariablesMockBuilder.build(); @@ -25,6 +24,7 @@ describe("routes/secondfactor/u2f/sign/post", function () { vars = s.variables; req.session = { + ...req.session, auth: { userid: "user", authentication_level: Level.ONE_FACTOR, diff --git a/server/src/lib/routes/secondfactor/u2f/sign_request/get.spec.ts b/server/src/lib/routes/secondfactor/u2f/sign_request/get.spec.ts index dd52b27e..415feaf5 100644 --- a/server/src/lib/routes/secondfactor/u2f/sign_request/get.spec.ts +++ b/server/src/lib/routes/secondfactor/u2f/sign_request/get.spec.ts @@ -1,4 +1,4 @@ - +import * as Express from "express"; import sinon = require("sinon"); import BluebirdPromise = require("bluebird"); import assert = require("assert"); @@ -8,10 +8,8 @@ import { Request } from "u2f"; import { ServerVariablesMock, ServerVariablesMockBuilder } from "../../../../ServerVariablesMockBuilder.spec"; import { ServerVariables } from "../../../../ServerVariables"; -import { SignMessage } from "../../../../../../../shared/SignMessage"; - describe("routes/secondfactor/u2f/sign_request/get", function () { - let req: ExpressMock.RequestMock; + let req: Express.Request; let res: ExpressMock.ResponseMock; let mocks: ServerVariablesMock; let vars: ServerVariables; @@ -19,8 +17,8 @@ describe("routes/secondfactor/u2f/sign_request/get", function () { beforeEach(function () { req = ExpressMock.RequestMock(); req.originalUrl = "/api/xxxx"; - req.app = {}; req.session = { + ...req.session, auth: { userid: "user", first_factor: true, diff --git a/server/src/lib/routes/verify/get.spec.ts b/server/src/lib/routes/verify/get.spec.ts index 67cf19fb..ec6751d3 100644 --- a/server/src/lib/routes/verify/get.spec.ts +++ b/server/src/lib/routes/verify/get.spec.ts @@ -1,9 +1,7 @@ -import Assert = require("assert"); -import BluebirdPromise = require("bluebird"); -import Express = require("express"); -import Sinon = require("sinon"); -import winston = require("winston"); +import * as Assert from "assert"; +import * as Express from "express"; +import * as Sinon from "sinon"; import VerifyGet = require("./get"); import { AuthenticationSessionHandler } from "../../AuthenticationSessionHandler"; @@ -13,9 +11,10 @@ import { ServerVariables } from "../../ServerVariables"; import { ServerVariablesMockBuilder, ServerVariablesMock } from "../../ServerVariablesMockBuilder.spec"; import { Level } from "../../authentication/Level"; import { Level as AuthorizationLevel } from "../../authorization/Level"; +import { HEADER_X_ORIGINAL_URL } from "../../../../../shared/constants"; describe("routes/verify/get", function () { - let req: ExpressMock.RequestMock; + let req: Express.Request; let res: ExpressMock.ResponseMock; let mocks: ServerVariablesMock; let vars: ServerVariables; @@ -29,7 +28,7 @@ describe("routes/verify/get", function () { redirect: "undefined" }; AuthenticationSessionHandler.reset(req as any); - req.headers["x-original-url"] = "https://secret.example.com/"; + req.headers[HEADER_X_ORIGINAL_URL] = "https://secret.example.com/"; const s = ServerVariablesMockBuilder.build(false); mocks = s.mocks; vars = s.variables; @@ -37,40 +36,37 @@ describe("routes/verify/get", function () { }); describe("with session cookie", function () { - it("should be already authenticated", function () { + it("should be already authenticated", async function () { mocks.authorizer.authorizationMock.returns(AuthorizationLevel.TWO_FACTOR); authSession.authentication_level = Level.TWO_FACTOR; authSession.userid = "myuser"; authSession.groups = ["mygroup", "othergroup"]; - return VerifyGet.default(vars)(req as Express.Request, res as any) - .then(function () { - Sinon.assert.calledWithExactly(res.setHeader, "Remote-User", "myuser"); - Sinon.assert.calledWithExactly(res.setHeader, "Remote-Groups", "mygroup,othergroup"); - Assert.equal(204, res.status.getCall(0).args[0]); - }); + await VerifyGet.default(vars)(req as Express.Request, res as any); + res.setHeader.calledWith("Remote-User", "myuser"); + res.setHeader.calledWith("Remote-Groups", "mygroup,othergroup"); + Assert.equal(204, res.status.getCall(0).args[0]); }); function test_session(_authSession: AuthenticationSession, status_code: number) { + const GetMock = Sinon.stub(AuthenticationSessionHandler, 'get'); + GetMock.returns(_authSession); return VerifyGet.default(vars)(req as Express.Request, res as any) .then(function () { Assert.equal(status_code, res.status.getCall(0).args[0]); - }); + GetMock.restore(); + }) } - function test_non_authenticated_401(authSession: AuthenticationSession) { - return test_session(authSession, 401); + function test_non_authenticated_401(_authSession: AuthenticationSession) { + return test_session(_authSession, 401); } - function test_unauthorized_403(authSession: AuthenticationSession) { - return test_session(authSession, 403); - } - - function test_authorized(authSession: AuthenticationSession) { - return test_session(authSession, 204); + function test_unauthorized_403(_authSession: AuthenticationSession) { + return test_session(_authSession, 403); } describe("given user tries to access a 2-factor endpoint", function () { - before(function () { + beforeEach(function () { mocks.authorizer.authorizationMock.returns(AuthorizationLevel.TWO_FACTOR); }); @@ -115,7 +111,7 @@ describe("routes/verify/get", function () { it("should not be authenticated when domain is not allowed for user", function () { authSession.authentication_level = Level.TWO_FACTOR; authSession.userid = "myuser"; - req.headers["x-original-url"] = "https://test.example.com/"; + req.headers[HEADER_X_ORIGINAL_URL] = "https://test.example.com/"; mocks.authorizer.authorizationMock.returns(AuthorizationLevel.DENY); return test_unauthorized_403({ @@ -132,7 +128,7 @@ describe("routes/verify/get", function () { describe("given user tries to access a single factor endpoint", function () { beforeEach(function () { - req.headers["x-original-url"] = "https://redirect.url/"; + req.headers[HEADER_X_ORIGINAL_URL] = "https://redirect.url/"; }); it("should be authenticated when first factor is validated", function () { @@ -227,7 +223,7 @@ describe("routes/verify/get", function () { }); describe("with basic auth", function () { - it("should authenticate correctly", function () { + it("should authenticate correctly", async function () { mocks.authorizer.authorizationMock.returns(AuthorizationLevel.ONE_FACTOR); mocks.config.access_control.default_policy = "one_factor"; mocks.usersDatabase.checkUserPasswordStub.returns({ @@ -235,12 +231,10 @@ describe("routes/verify/get", function () { }); req.headers["proxy-authorization"] = "Basic am9objpwYXNzd29yZA=="; - return VerifyGet.default(vars)(req as Express.Request, res as any) - .then(function () { - Sinon.assert.calledWithExactly(res.setHeader, "Remote-User", "john"); - Sinon.assert.calledWithExactly(res.setHeader, "Remote-Groups", "mygroup,othergroup"); - Assert.equal(204, res.status.getCall(0).args[0]); - }); + await VerifyGet.default(vars)(req as Express.Request, res as any) + res.setHeader.calledWith("Remote-User", "john"); + res.setHeader.calledWith("Remote-Groups", "mygroup,othergroup"); + Assert.equal(204, res.status.getCall(0).args[0]); }); it("should fail when endpoint is protected by two factors", function () { diff --git a/server/src/lib/routes/verify/get.ts b/server/src/lib/routes/verify/get.ts index e5fe87fc..40e1b940 100644 --- a/server/src/lib/routes/verify/get.ts +++ b/server/src/lib/routes/verify/get.ts @@ -6,34 +6,44 @@ import { ServerVariables } from "../../ServerVariables"; import GetWithSessionCookieMethod from "./get_session_cookie"; import GetWithBasicAuthMethod from "./get_basic_auth"; import Constants = require("../../../../../shared/constants"); -import ObjectPath = require("object-path"); - import { AuthenticationSessionHandler } from "../../AuthenticationSessionHandler"; import { AuthenticationSession } from "../../../../types/AuthenticationSession"; +import GetHeader from "../../utils/GetHeader"; const REMOTE_USER = "Remote-User"; const REMOTE_GROUPS = "Remote-Groups"; function verifyWithSelectedMethod(req: Express.Request, res: Express.Response, - vars: ServerVariables, authSession: AuthenticationSession) + vars: ServerVariables, authSession: AuthenticationSession | undefined) : () => BluebirdPromise<{ username: string, groups: string[] }> { return function () { - const authorization: string = "" + req.headers["proxy-authorization"]; - if (authorization && authorization.startsWith("Basic ")) - return GetWithBasicAuthMethod(req, res, vars, authorization); - - return GetWithSessionCookieMethod(req, res, vars, authSession); + const authorization = GetHeader(req, Constants.HEADER_PROXY_AUTHORIZATION); + if (authorization) { + if (authorization.startsWith("Basic ")) { + return GetWithBasicAuthMethod(req, res, vars, authorization); + } + else { + throw new Error("The authorization header should be of the form 'Basic XXXXXX'"); + } + } + else { + if (authSession) { + return GetWithSessionCookieMethod(req, res, vars, authSession); + } + else { + throw new Error("No cookie detected."); + } + } }; } function setRedirectHeader(req: Express.Request, res: Express.Response) { return function () { - const originalUrl = ObjectPath.get( - req, "headers.x-original-url"); - res.set("Redirect", originalUrl); + const originalUrl = GetHeader(req, Constants.HEADER_X_ORIGINAL_URL); + res.set(Constants.HEADER_REDIRECT, originalUrl); return BluebirdPromise.resolve(); }; } @@ -62,7 +72,7 @@ function getRedirectParam(req: Express.Request) { export default function (vars: ServerVariables) { return function (req: Express.Request, res: Express.Response) : BluebirdPromise { - let authSession: AuthenticationSession; + let authSession: AuthenticationSession | undefined; return new BluebirdPromise(function (resolve, reject) { authSession = AuthenticationSessionHandler.get(req, vars.logger); resolve(); @@ -78,6 +88,7 @@ export default function (vars: ServerVariables) { ErrorReplies.replyWithError401(req, res, vars.logger)) // The user is not yet authenticated -> 401 .catch((err) => { + console.error(err); // This redirect parameter is used in Kubernetes to annotate the ingress with // the url to the authentication portal. const redirectUrl = getRedirectParam(req); diff --git a/server/src/lib/routes/verify/get_basic_auth.ts b/server/src/lib/routes/verify/get_basic_auth.ts index af23c76c..73e81063 100644 --- a/server/src/lib/routes/verify/get_basic_auth.ts +++ b/server/src/lib/routes/verify/get_basic_auth.ts @@ -1,18 +1,17 @@ import Express = require("express"); import BluebirdPromise = require("bluebird"); -import ObjectPath = require("object-path"); import { ServerVariables } from "../../ServerVariables"; -import { AuthenticationSession } - from "../../../../types/AuthenticationSession"; import AccessControl from "./access_control"; import { URLDecomposer } from "../../utils/URLDecomposer"; import { Level } from "../../authentication/Level"; +import GetHeader from "../../utils/GetHeader"; +import { HEADER_X_ORIGINAL_URL } from "../../../../../shared/constants"; export default function (req: Express.Request, res: Express.Response, vars: ServerVariables, authorizationHeader: string) : BluebirdPromise<{ username: string, groups: string[] }> { let username: string; - const uri = ObjectPath.get(req, "headers.x-original-url"); + const uri = GetHeader(req, HEADER_X_ORIGINAL_URL); const urlDecomposition = URLDecomposer.fromUrl(uri); return BluebirdPromise.resolve() diff --git a/server/src/lib/routes/verify/get_session_cookie.ts b/server/src/lib/routes/verify/get_session_cookie.ts index f703b944..d7bfc04d 100644 --- a/server/src/lib/routes/verify/get_session_cookie.ts +++ b/server/src/lib/routes/verify/get_session_cookie.ts @@ -1,8 +1,5 @@ import Express = require("express"); import BluebirdPromise = require("bluebird"); -import Util = require("util"); -import ObjectPath = require("object-path"); - import Exceptions = require("../../Exceptions"); import { Configuration } from "../../configuration/schema/Configuration"; import { ServerVariables } from "../../ServerVariables"; @@ -13,6 +10,8 @@ import { AuthenticationSessionHandler } from "../../AuthenticationSessionHandler"; import AccessControl from "./access_control"; import { URLDecomposer } from "../../utils/URLDecomposer"; +import GetHeader from "../../utils/GetHeader"; +import { HEADER_X_ORIGINAL_URL } from "../../../../../shared/constants"; function verify_inactivity(req: Express.Request, authSession: AuthenticationSession, @@ -54,8 +53,7 @@ export default function (req: Express.Request, res: Express.Response, "userid is missing")); } - const originalUrl = ObjectPath.get( - req, "headers.x-original-url"); + const originalUrl = GetHeader(req, HEADER_X_ORIGINAL_URL); const d = URLDecomposer.fromUrl(originalUrl); vars.logger.debug(req, "domain=%s, path=%s, user=%s, groups=%s", d.domain, diff --git a/server/src/lib/stubs/express.spec.ts b/server/src/lib/stubs/express.spec.ts index 48f15d7e..8b995ac9 100644 --- a/server/src/lib/stubs/express.spec.ts +++ b/server/src/lib/stubs/express.spec.ts @@ -1,103 +1,117 @@ -import sinon = require("sinon"); -import express = require("express"); - -export interface RequestMock { - app?: any; - body?: any; - session?: any; - headers?: any; - get?: any; - query?: any; - originalUrl: string; -} +import * as Sinon from "sinon"; +import * as Express from "express"; +import { GET_VARIABLE_KEY } from "../../../../shared/constants"; +import { RequestLoggerStub } from "../logging/RequestLoggerStub.spec"; export interface ResponseMock { - send: sinon.SinonStub | sinon.SinonSpy; - sendStatus: sinon.SinonStub; - sendFile: sinon.SinonStub; - sendfile: sinon.SinonStub; - status: sinon.SinonStub | sinon.SinonSpy; - json: sinon.SinonStub | sinon.SinonSpy; - links: sinon.SinonStub; - jsonp: sinon.SinonStub; - download: sinon.SinonStub; - contentType: sinon.SinonStub; - type: sinon.SinonStub; - format: sinon.SinonStub; - attachment: sinon.SinonStub; - set: sinon.SinonStub; - header: sinon.SinonStub; + send: Sinon.SinonStub | Sinon.SinonSpy; + sendStatus: Sinon.SinonStub; + sendFile: Sinon.SinonStub; + sendfile: Sinon.SinonStub; + status: Sinon.SinonStub | Sinon.SinonSpy; + json: Sinon.SinonStub | Sinon.SinonSpy; + links: Sinon.SinonStub; + jsonp: Sinon.SinonStub; + download: Sinon.SinonStub; + contentType: Sinon.SinonStub; + type: Sinon.SinonStub; + format: Sinon.SinonStub; + attachment: Sinon.SinonStub; + set: Sinon.SinonStub; + header: Sinon.SinonStub; headersSent: boolean; - get: sinon.SinonStub; - clearCookie: sinon.SinonStub; - cookie: sinon.SinonStub; - location: sinon.SinonStub; - redirect: sinon.SinonStub | sinon.SinonSpy; - render: sinon.SinonStub | sinon.SinonSpy; - locals: sinon.SinonStub; + get: Sinon.SinonStub; + clearCookie: Sinon.SinonStub; + cookie: Sinon.SinonStub; + location: Sinon.SinonStub; + redirect: Sinon.SinonStub | Sinon.SinonSpy; + render: Sinon.SinonStub | Sinon.SinonSpy; + locals: Sinon.SinonStub; charset: string; - vary: sinon.SinonStub; + vary: Sinon.SinonStub; app: any; - write: sinon.SinonStub; - writeContinue: sinon.SinonStub; - writeHead: sinon.SinonStub; + write: Sinon.SinonStub; + writeContinue: Sinon.SinonStub; + writeHead: Sinon.SinonStub; statusCode: number; statusMessage: string; - setHeader: sinon.SinonStub; - setTimeout: sinon.SinonStub; + setHeader: Sinon.SinonStub; + setTimeout: Sinon.SinonStub; sendDate: boolean; - getHeader: sinon.SinonStub; + getHeader: Sinon.SinonStub; } -export function RequestMock(): RequestMock { +export function RequestMock(): Express.Request { + const getMock = Sinon.mock() + .withArgs(GET_VARIABLE_KEY).atLeast(0).returns({ + logger: new RequestLoggerStub() + }); return { - originalUrl: "/non-api/xxx", + id: '1234', + headers: {}, app: { - get: sinon.stub() + get: getMock, + set: Sinon.mock(), }, - headers: { - "x-forwarded-for": "127.0.0.1" - }, - session: {} - }; + body: {}, + query: {}, + session: { + id: '1234', + regenerate: function() {}, + reload: function() {}, + destroy: function() {}, + save: function() {}, + touch: function() {}, + cookie: { + domain: 'example.com', + expires: true, + httpOnly: true, + maxAge: 36000, + originalMaxAge: 36000, + path: '/', + secure: true, + serialize: () => '', + } + } + } as any; } export function ResponseMock(): ResponseMock { return { - send: sinon.stub(), - status: sinon.stub(), - json: sinon.stub(), - sendStatus: sinon.stub(), - links: sinon.stub(), - jsonp: sinon.stub(), - sendFile: sinon.stub(), - sendfile: sinon.stub(), - download: sinon.stub(), - contentType: sinon.stub(), - type: sinon.stub(), - format: sinon.stub(), - attachment: sinon.stub(), - set: sinon.stub(), - header: sinon.stub(), + send: Sinon.stub(), + status: Sinon.stub(), + json: Sinon.stub(), + sendStatus: Sinon.stub(), + links: Sinon.stub(), + jsonp: Sinon.stub(), + sendFile: Sinon.stub(), + sendfile: Sinon.stub(), + download: Sinon.stub(), + contentType: Sinon.stub(), + type: Sinon.stub(), + format: Sinon.stub(), + attachment: Sinon.stub(), + set: Sinon.stub(), + header: Sinon.stub(), headersSent: true, - get: sinon.stub(), - clearCookie: sinon.stub(), - cookie: sinon.stub(), - location: sinon.stub(), - redirect: sinon.stub(), - render: sinon.stub(), - locals: sinon.stub(), + get: Sinon.stub(), + clearCookie: Sinon.stub(), + cookie: Sinon.stub(), + location: Sinon.stub(), + redirect: Sinon.stub(), + render: Sinon.stub(), + locals: Sinon.stub(), charset: "utf-8", - vary: sinon.stub(), - app: sinon.stub(), - write: sinon.stub(), - writeContinue: sinon.stub(), - writeHead: sinon.stub(), + vary: Sinon.stub(), + app: Sinon.stub(), + write: Sinon.stub(), + writeContinue: Sinon.stub(), + writeHead: Sinon.stub(), statusCode: 200, statusMessage: "message", - setHeader: sinon.stub(), - setTimeout: sinon.stub(), + setHeader: Sinon.stub(), + setTimeout: Sinon.stub(), sendDate: true, - getHeader: sinon.stub() + getHeader: Sinon.stub() }; } diff --git a/server/src/lib/utils/GetHeader.spec.ts b/server/src/lib/utils/GetHeader.spec.ts new file mode 100644 index 00000000..d2543719 --- /dev/null +++ b/server/src/lib/utils/GetHeader.spec.ts @@ -0,0 +1,20 @@ +import * as Express from "express"; +import GetHeader from "./GetHeader"; +import { RequestMock } from "../stubs/express.spec"; +import * as Assert from "assert"; + +describe('GetHeader', function() { + let req: Express.Request; + beforeEach(() => { + req = RequestMock(); + }); + + it('should return the header if it exists', function() { + req.headers["x-target-url"] = 'www.example.com'; + Assert.equal(GetHeader(req, 'x-target-url'), 'www.example.com'); + }); + + it('should return undefined if header does not exist', function() { + Assert.equal(GetHeader(req, 'x-target-url'), undefined); + }); +}); \ No newline at end of file diff --git a/server/src/lib/utils/GetHeader.ts b/server/src/lib/utils/GetHeader.ts new file mode 100644 index 00000000..a1666bf4 --- /dev/null +++ b/server/src/lib/utils/GetHeader.ts @@ -0,0 +1,19 @@ +import * as Express from "express"; +import * as ObjectPath from "object-path"; +import { ServerVariables } from "../ServerVariables"; +import { GET_VARIABLE_KEY } from "../../../../shared/constants"; + +/** + * + * @param req The express request to extract headers from + * @param header The name of the header to extract in lowercase. + * @returns The header if found, otherwise undefined. + */ +export default function(req: Express.Request, header: string): string | undefined { + const variables: ServerVariables = req.app.get(GET_VARIABLE_KEY); + if (!variables) return undefined; + + const value = ObjectPath.get(req, "headers." + header, undefined); + variables.logger.debug(req, "Header %s is set to %s", header, value); + return value; +} \ No newline at end of file diff --git a/server/src/lib/utils/IsRedirectionSafe.ts b/server/src/lib/utils/IsRedirectionSafe.ts index 8500b1bd..ad5f1aaa 100644 --- a/server/src/lib/utils/IsRedirectionSafe.ts +++ b/server/src/lib/utils/IsRedirectionSafe.ts @@ -1,11 +1,8 @@ import { ServerVariables } from "../ServerVariables"; -import { Level } from "../authentication/Level"; import * as URLParse from "url-parse"; -import { AuthenticationSession } from "AuthenticationSession"; export default function IsRedirectionSafe( vars: ServerVariables, - authSession: AuthenticationSession, url: URLParse): boolean { const urlInDomain = url.hostname.endsWith(vars.config.session.domain); diff --git a/shared/DomainExtractor.spec.ts b/shared/DomainExtractor.spec.ts index c1ba0415..8e198666 100644 --- a/shared/DomainExtractor.spec.ts +++ b/shared/DomainExtractor.spec.ts @@ -1,7 +1,7 @@ import { DomainExtractor } from "./DomainExtractor"; import Assert = require("assert"); -describe.only("shared/DomainExtractor", function () { +describe("shared/DomainExtractor", function () { describe("test fromUrl", function () { it("should return domain from https url", function () { const domain = DomainExtractor.fromUrl("https://www.example.com/test/abc"); diff --git a/shared/constants.ts b/shared/constants.ts index 47d3852a..8a1ca26d 100644 --- a/shared/constants.ts +++ b/shared/constants.ts @@ -1 +1,11 @@ -export const REDIRECT_QUERY_PARAM = "rd"; \ No newline at end of file +export const REDIRECT_QUERY_PARAM = "rd"; + +// Used as a first factor script parameter for knowing the authorizations +// related to the domain and resource. +export const HEADER_X_TARGET_URL = "x-target-url"; + +export const HEADER_X_ORIGINAL_URL = "x-original-url"; +export const HEADER_PROXY_AUTHORIZATION = "proxy-authorization"; +export const HEADER_REDIRECT = "redirect"; + +export const GET_VARIABLE_KEY = "variables"; \ No newline at end of file diff --git a/spec-helper.js b/spec-helper.js new file mode 100644 index 00000000..d43365b9 --- /dev/null +++ b/spec-helper.js @@ -0,0 +1,3 @@ +var colors = require('mocha/lib/reporters/base').colors; + colors['pass'] = 32; + colors['error stack'] = 34; \ No newline at end of file From 6a19f7eb919e56ad3f2ad07c70e93e51244e47ea Mon Sep 17 00:00:00 2001 From: Clement Michaud Date: Fri, 22 Mar 2019 15:01:05 +0100 Subject: [PATCH 2/4] Add documentation for nginx proxy. --- docs/features.md | 7 ++ docs/proxies/nginx.md | 75 +++++++++++++++++++++ example/compose/nginx/portal/nginx.conf.ejs | 5 ++ 3 files changed, 87 insertions(+) create mode 100644 docs/proxies/nginx.md diff --git a/docs/features.md b/docs/features.md index 83bbc6f8..b601cda9 100644 --- a/docs/features.md +++ b/docs/features.md @@ -81,6 +81,13 @@ Please check [config.template.yml] to see an example of configuration. It is also possible to use [basic authentication] to access a resource protected by a single factor. +Please note that Authelia uses the *Proxy-Authorization* header and not +*Authorization* since one might be willing to authenticate against both +Authelia and the proxy. For instance you can use the following command to +access your service: + + curl -H "Proxy-Authorization: Basic am9objpwYXNzd29yZA==" https://myservice.example.com" + ## Session management with Redis When your users authenticate against Authelia, sessions are stored in a diff --git a/docs/proxies/nginx.md b/docs/proxies/nginx.md new file mode 100644 index 00000000..538c1bb1 --- /dev/null +++ b/docs/proxies/nginx.md @@ -0,0 +1,75 @@ +# Nginx + +[nginx] is the only official reverse proxy supported by **Authelia** for now. + +## Configuration + +Here is a commented example of configuration + + server { + listen 443 ssl; + server_name myapp.example.com; + + resolver 127.0.0.11 ipv6=off; + set $upstream_verify https://authelia.example.com/api/verify; + set $upstream_endpoint http://nginx-backend; + + ssl_certificate /etc/ssl/server.crt; + ssl_certificate_key /etc/ssl/server.key; + + # Use HSTS, please beware of what you're doing if you set it. + add_header Strict-Transport-Security "max-age=31536000; includeSubDomains" always; + add_header X-Frame-Options "SAMEORIGIN"; + + # Virtual endpoint created by nginx to forward auth requests. + location /auth_verify { + internal; + proxy_set_header Host $http_host; + + # [REQUIRED] Needed by Authelia to check authorizations of the resource. + proxy_set_header X-Original-URL $scheme://$http_host$request_uri; + + # [OPTIONAL] The IP of the client shown in Authelia logs. + proxy_set_header X-Real-IP $remote_addr; + + # [REQUIRED] Needed by Authelia to ensure that the query was served over SSL + # Check this out: https://expressjs.com/en/guide/behind-proxies.html + proxy_set_header X-Forwarded-Proto $scheme; + + # [OPTIONAL] The list of IPs of client and proxies in the chain. + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + + proxy_pass_request_body off; + proxy_set_header Content-Length ""; + + proxy_pass $upstream_verify; + } + + location / { + # Send a subsequent request to Authelia to verify if the user is authenticated + # and has the right permissions to access the resource. + auth_request /auth_verify; + + auth_request_set $redirect $upstream_http_redirect; + + # Set the X-Forwarded-User and X-Forwarded-Groups with the headers + # returned by Authelia for the backends which can consume them. + # This is not safe, as the backend must make sure that they come from the + # proxy. In the future, it's gonna be safe to just use OAuth. + auth_request_set $user $upstream_http_remote_user; + proxy_set_header X-Forwarded-User $user; + + auth_request_set $groups $upstream_http_remote_groups; + proxy_set_header Remote-Groups $groups; + + # If Authelia returns 401, then nginx redirects the user to the login portal. + # If it returns 200, then the request pass through to the backend. + # For other type of errors, nginx will handle them as usual. + error_page 401 =302 https://login.example.com:8080/#/?rd=$redirect; + + proxy_pass $upstream_endpoint; + } + } + + +[nginx]: https://www.nginx.com/ \ No newline at end of file diff --git a/example/compose/nginx/portal/nginx.conf.ejs b/example/compose/nginx/portal/nginx.conf.ejs index 957d7b38..3306e45a 100644 --- a/example/compose/nginx/portal/nginx.conf.ejs +++ b/example/compose/nginx/portal/nginx.conf.ejs @@ -1,3 +1,8 @@ +# +# You can find a documented example of configuration in ./docs/proxies/nginx.md. +# + + worker_processes 1; events { From 9366741980ed5466a1536195b5ceb7db2b25242b Mon Sep 17 00:00:00 2001 From: Clement Michaud Date: Fri, 22 Mar 2019 15:02:15 +0100 Subject: [PATCH 3/4] Forbid test selected with only in CI. --- scripts/authelia-scripts-travis | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/authelia-scripts-travis b/scripts/authelia-scripts-travis index c5d592a8..0650bc0b 100755 --- a/scripts/authelia-scripts-travis +++ b/scripts/authelia-scripts-travis @@ -25,13 +25,13 @@ docker-compose -f docker-compose.yml \ authelia-scripts build # Run unit tests -authelia-scripts unittest +authelia-scripts unittest --forbid-only # Build the docker image authelia-scripts docker build # Run integration tests -authelia-scripts suites test --headless +authelia-scripts suites test --headless --forbid-only # Test npm deployment before actual deployment # ./scripts/npm-deployment-test.sh From bd5bb497e3cbcb6c54f1b150db70a97564e6b6ee Mon Sep 17 00:00:00 2001 From: Clement Michaud Date: Fri, 22 Mar 2019 15:17:38 +0100 Subject: [PATCH 4/4] Log stage names as they are running in travis script. --- scripts/authelia-scripts-travis | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/scripts/authelia-scripts-travis b/scripts/authelia-scripts-travis index 0650bc0b..fdd1d16a 100755 --- a/scripts/authelia-scripts-travis +++ b/scripts/authelia-scripts-travis @@ -22,15 +22,19 @@ docker-compose -f docker-compose.yml \ pull # Build +echo "===> Build stage" authelia-scripts build # Run unit tests +echo "===> Unit tests stage" authelia-scripts unittest --forbid-only # Build the docker image +echo "===> Docker image build stage" authelia-scripts docker build # Run integration tests +echo "===> e2e stage" authelia-scripts suites test --headless --forbid-only # Test npm deployment before actual deployment