From 8cf58d7b311b79056e68d247546fcb39d3a6c5a7 Mon Sep 17 00:00:00 2001 From: Clement Michaud Date: Sat, 14 Oct 2017 15:04:43 +0200 Subject: [PATCH 1/3] Add tests on headers forwarded to backend Ensure Remote-User and Remote-Groups can be forwarded to the backend app. --- example/httpbin/docker-compose.yml | 6 +++ example/nginx/nginx.conf | 37 +++++++++++++------ scripts/dc-dev.sh | 1 + scripts/example-commit/dc-example.sh | 1 + scripts/example-commit/deploy-example.sh | 2 +- scripts/example-dockerhub/dc-example.sh | 1 + scripts/example-dockerhub/deploy-example.sh | 2 +- scripts/integration-tests.sh | 4 +- test/features/forward-headers.feature | 6 +++ .../step_definitions/forward-headers.ts | 20 ++++++++++ .../step_definitions/notifications.ts | 1 - 11 files changed, 65 insertions(+), 16 deletions(-) create mode 100644 example/httpbin/docker-compose.yml create mode 100644 test/features/forward-headers.feature create mode 100644 test/features/step_definitions/forward-headers.ts diff --git a/example/httpbin/docker-compose.yml b/example/httpbin/docker-compose.yml new file mode 100644 index 000000000..b21cdc54e --- /dev/null +++ b/example/httpbin/docker-compose.yml @@ -0,0 +1,6 @@ +version: '2' +services: + httpbin: + image: citizenstig/httpbin + networks: + - example-network diff --git a/example/nginx/nginx.conf b/example/nginx/nginx.conf index 542d6221d..a1ec3dbc2 100644 --- a/example/nginx/nginx.conf +++ b/example/nginx/nginx.conf @@ -74,16 +74,16 @@ http { proxy_set_header X-Original-URI $request_uri; proxy_set_header X-Real-IP $remote_addr; proxy_set_header Host $http_host; + proxy_set_header Content-Length ""; proxy_pass http://authelia/verify; } location / { auth_request /auth_verify; - - auth_request_set $redirect $upstream_http_redirect; - proxy_set_header Redirect $redirect; + auth_request_set $redirect $upstream_http_redirect; + auth_request_set $user $upstream_http_remote_user; proxy_set_header X-Forwarded-User $user; @@ -93,6 +93,23 @@ http { error_page 401 =302 https://auth.test.local:8080?redirect=$redirect; error_page 403 = https://auth.test.local:8080/error/403; } + + location /headers { + auth_request /auth_verify; + + auth_request_set $redirect $upstream_http_redirect; + + auth_request_set $user $upstream_http_remote_user; + proxy_set_header Custom-Forwarded-User $user; + + auth_request_set $groups $upstream_http_remote_groups; + proxy_set_header Custom-Forwarded-Groups $groups; + + proxy_pass http://httpbin:8000/headers; + + error_page 401 =302 https://auth.test.local:8080?redirect=$redirect; + error_page 403 = https://auth.test.local:8080/error/403; + } } server { @@ -110,15 +127,15 @@ http { proxy_set_header X-Original-URI $request_uri; proxy_set_header X-Real-IP $remote_addr; proxy_set_header Host $http_host; + proxy_set_header Content-Length ""; proxy_pass http://authelia/verify; } location / { auth_request /auth_verify; - + auth_request_set $redirect $upstream_http_redirect; - proxy_set_header Redirect $redirect; auth_request_set $user $upstream_http_remote_user; proxy_set_header X-Forwarded-User $user; @@ -146,15 +163,15 @@ http { proxy_set_header X-Original-URI $request_uri; proxy_set_header X-Real-IP $remote_addr; proxy_set_header Host $http_host; + proxy_set_header Content-Length ""; proxy_pass http://authelia/verify; } location / { auth_request /auth_verify; - + auth_request_set $redirect $upstream_http_redirect; - proxy_set_header Redirect $redirect; auth_request_set $user $upstream_http_remote_user; proxy_set_header X-Forwarded-User $user; @@ -189,9 +206,8 @@ http { location / { auth_request /auth_verify; - + auth_request_set $redirect $upstream_http_redirect; - proxy_set_header Redirect $redirect; auth_request_set $user $upstream_http_remote_user; proxy_set_header X-Forwarded-User $user; @@ -226,9 +242,8 @@ http { location / { auth_request /auth_verify; - + auth_request_set $redirect $upstream_http_redirect; - proxy_set_header Redirect $redirect; auth_request_set $user $upstream_http_remote_user; proxy_set_header X-Forwarded-User $user; diff --git a/scripts/dc-dev.sh b/scripts/dc-dev.sh index 3d83b914b..75b6ba6a2 100755 --- a/scripts/dc-dev.sh +++ b/scripts/dc-dev.sh @@ -10,5 +10,6 @@ docker-compose \ -f example/redis/docker-compose.yml \ -f example/nginx/docker-compose.yml \ -f example/smtp/docker-compose.yml \ + -f example/httpbin/docker-compose.yml \ -f example/ldap/docker-compose.admin.yml \ -f example/ldap/docker-compose.yml $* diff --git a/scripts/example-commit/dc-example.sh b/scripts/example-commit/dc-example.sh index 9ede68d99..59c344c44 100755 --- a/scripts/example-commit/dc-example.sh +++ b/scripts/example-commit/dc-example.sh @@ -9,4 +9,5 @@ docker-compose \ -f example/redis/docker-compose.yml \ -f example/nginx/docker-compose.yml \ -f example/smtp/docker-compose.yml \ + -f example/httpbin/docker-compose.yml \ -f example/ldap/docker-compose.yml $* diff --git a/scripts/example-commit/deploy-example.sh b/scripts/example-commit/deploy-example.sh index e5855f1f2..3ea60e07e 100755 --- a/scripts/example-commit/deploy-example.sh +++ b/scripts/example-commit/deploy-example.sh @@ -3,4 +3,4 @@ DC_SCRIPT=./scripts/example-commit/dc-example.sh $DC_SCRIPT build -$DC_SCRIPT up -d mongo redis openldap authelia nginx smtp +$DC_SCRIPT up -d httpbin mongo redis openldap authelia nginx smtp diff --git a/scripts/example-dockerhub/dc-example.sh b/scripts/example-dockerhub/dc-example.sh index e73486744..94d242ba1 100755 --- a/scripts/example-dockerhub/dc-example.sh +++ b/scripts/example-dockerhub/dc-example.sh @@ -9,4 +9,5 @@ docker-compose \ -f example/redis/docker-compose.yml \ -f example/nginx/docker-compose.yml \ -f example/smtp/docker-compose.yml \ + -f example/httpbin/docker-compose.yml \ -f example/ldap/docker-compose.yml $* diff --git a/scripts/example-dockerhub/deploy-example.sh b/scripts/example-dockerhub/deploy-example.sh index 81eddfdbc..ec042ec78 100755 --- a/scripts/example-dockerhub/deploy-example.sh +++ b/scripts/example-dockerhub/deploy-example.sh @@ -3,4 +3,4 @@ DC_SCRIPT=./scripts/example-dockerhub/dc-example.sh #$DC_SCRIPT build -$DC_SCRIPT up -d mongo redis openldap authelia nginx smtp +$DC_SCRIPT up -d httpbin mongo redis openldap authelia nginx smtp diff --git a/scripts/integration-tests.sh b/scripts/integration-tests.sh index 1cabe9bca..09f16ad9d 100755 --- a/scripts/integration-tests.sh +++ b/scripts/integration-tests.sh @@ -1,14 +1,14 @@ #!/bin/bash DC_SCRIPT=./scripts/example-commit/dc-example.sh -EXPECTED_SERVICES_COUNT=6 +EXPECTED_SERVICES_COUNT=7 build_services() { $DC_SCRIPT build authelia } start_services() { - $DC_SCRIPT up -d mongo redis openldap authelia nginx smtp + $DC_SCRIPT up -d httpbin mongo redis openldap authelia nginx smtp sleep 3 } diff --git a/test/features/forward-headers.feature b/test/features/forward-headers.feature new file mode 100644 index 000000000..3b3635269 --- /dev/null +++ b/test/features/forward-headers.feature @@ -0,0 +1,6 @@ +Feature: User and groups headers are correctly forwarded to backend + @need-authenticated-user-john + Scenario: Custom-Forwarded-User and Custom-Forwarded-Groups are correctly forwarded to protected backend + When I visit "https://public.test.local:8080/headers" + Then I see header "Custom-Forwarded-User" set to "john" + Then I see header "Custom-Forwarded-Groups" set to "dev,admin" diff --git a/test/features/step_definitions/forward-headers.ts b/test/features/step_definitions/forward-headers.ts new file mode 100644 index 000000000..2e61771cc --- /dev/null +++ b/test/features/step_definitions/forward-headers.ts @@ -0,0 +1,20 @@ +import Cucumber = require("cucumber"); +import seleniumWebdriver = require("selenium-webdriver"); +import CustomWorld = require("../support/world"); +import Util = require("util"); +import BluebirdPromise = require("bluebird"); + +Cucumber.defineSupportCode(function ({ Given, When, Then }) { + Then("I see header {stringInDoubleQuotes} set to {stringInDoubleQuotes}", + { timeout: 5000 }, + function (expectedHeaderName: string, expectedValue: string) { + return this.driver.findElement(seleniumWebdriver.By.tagName("body")).getText() + .then(function (txt: string) { + const expectedLine = Util.format("\"%s\": \"%s\"", expectedHeaderName, expectedValue); + if (txt.indexOf(expectedLine) > 0) + return BluebirdPromise.resolve(); + else + return BluebirdPromise.reject(new Error(Util.format("No such header or with unexpected value."))); + }); + }) +}); \ No newline at end of file diff --git a/test/features/step_definitions/notifications.ts b/test/features/step_definitions/notifications.ts index 8916ba7e9..5da9d06ab 100644 --- a/test/features/step_definitions/notifications.ts +++ b/test/features/step_definitions/notifications.ts @@ -23,5 +23,4 @@ Cucumber.defineSupportCode(function ({ Given, When, Then }) { return that.driver.sleep(500); }); }); - }); \ No newline at end of file From c02d9b4a6e1d73c44e9304f7f934798134dc942c Mon Sep 17 00:00:00 2001 From: Clement Michaud Date: Sat, 14 Oct 2017 15:16:14 +0200 Subject: [PATCH 2/3] Display current URL when redirection step fails in integration tests --- test/features/support/world.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test/features/support/world.ts b/test/features/support/world.ts index e3302fda5..2bed95561 100644 --- a/test/features/support/world.ts +++ b/test/features/support/world.ts @@ -56,7 +56,15 @@ function CustomWorld() { }; this.waitUntilUrlContains = function(url: string) { - return this.driver.wait(seleniumWebdriver.until.urlIs(url), 15000); + const that = this; + return this.driver.wait(seleniumWebdriver.until.urlIs(url), 15000) + .then(function() {}, function(err: Error) { + that.driver.getCurrentUrl() + .then(function(current: string) { + console.error("====> Error due to: %s (current) != %s (expected)", current, url); + }); + return BluebirdPromise.reject(err); + }); }; this.loginWithUserPassword = function (username: string, password: string) { From 3a88ca95b8fa364fc62564bf29386f455f56cd8c Mon Sep 17 00:00:00 2001 From: Clement Michaud Date: Sat, 14 Oct 2017 15:23:00 +0200 Subject: [PATCH 3/3] Check TOTP token with window of 1 A window of 1 means the token is checked against current time slot T as well as at time slot T-1 and T+1. A time slot is 30 seconds by default in Authelia. --- package.json | 1 - server/src/lib/TOTPGenerator.ts | 18 +++-- server/src/lib/TOTPValidator.ts | 22 +++--- server/test/TOTPValidator.test.ts | 21 +++-- server/test/storage/UserDataStore.test.ts | 7 +- server/types/TOTPSecret.ts | 11 ++- server/types/speakeasy.d.ts | 96 +++++++++++++++++++++++ test/features/support/world.ts | 18 ++--- 8 files changed, 160 insertions(+), 34 deletions(-) create mode 100644 server/types/speakeasy.d.ts diff --git a/package.json b/package.json index c2fef38ec..d0ebe5e9c 100644 --- a/package.json +++ b/package.json @@ -71,7 +71,6 @@ "@types/request-promise": "^4.1.38", "@types/selenium-webdriver": "^3.0.4", "@types/sinon": "^2.2.1", - "@types/speakeasy": "^2.0.1", "@types/tmp": "0.0.33", "@types/winston": "^2.3.2", "@types/yamljs": "^0.2.30", diff --git a/server/src/lib/TOTPGenerator.ts b/server/src/lib/TOTPGenerator.ts index 1dece0361..c0c7133bf 100644 --- a/server/src/lib/TOTPGenerator.ts +++ b/server/src/lib/TOTPGenerator.ts @@ -1,16 +1,24 @@ -import * as speakeasy from "speakeasy"; -import { Speakeasy } from "../../types/Dependencies"; +import Speakeasy = require("speakeasy"); import BluebirdPromise = require("bluebird"); +import { TOTPSecret } from "../../types/TOTPSecret"; + +interface GenerateSecretOptions { + length?: number; + symbols?: boolean; + otpauth_url?: boolean; + name?: string; + issuer?: string; +} export class TOTPGenerator { - private speakeasy: Speakeasy; + private speakeasy: typeof Speakeasy; - constructor(speakeasy: Speakeasy) { + constructor(speakeasy: typeof Speakeasy) { this.speakeasy = speakeasy; } - generate(options?: speakeasy.GenerateOptions): speakeasy.Key { + generate(options?: GenerateSecretOptions): TOTPSecret { return this.speakeasy.generateSecret(options); } } \ No newline at end of file diff --git a/server/src/lib/TOTPValidator.ts b/server/src/lib/TOTPValidator.ts index cd6cba33d..d99be78b8 100644 --- a/server/src/lib/TOTPValidator.ts +++ b/server/src/lib/TOTPValidator.ts @@ -1,23 +1,27 @@ - -import { Speakeasy } from "../../types/Dependencies"; +import Speakeasy = require("speakeasy"); import BluebirdPromise = require("bluebird"); const TOTP_ENCODING = "base32"; +const WINDOW: number = 1; export class TOTPValidator { - private speakeasy: Speakeasy; + private speakeasy: typeof Speakeasy; - constructor(speakeasy: Speakeasy) { + constructor(speakeasy: typeof Speakeasy) { this.speakeasy = speakeasy; } validate(token: string, secret: string): BluebirdPromise { - const real_token = this.speakeasy.totp({ + const isValid = this.speakeasy.totp.verify({ secret: secret, - encoding: TOTP_ENCODING - }); + encoding: TOTP_ENCODING, + token: token, + window: WINDOW + } as any); - if (token == real_token) return BluebirdPromise.resolve(); - return BluebirdPromise.reject(new Error("Wrong challenge")); + if (isValid) + return BluebirdPromise.resolve(); + else + return BluebirdPromise.reject(new Error("Wrong TOTP token.")); } } \ No newline at end of file diff --git a/server/test/TOTPValidator.test.ts b/server/test/TOTPValidator.test.ts index 6fd3981f0..2856583f1 100644 --- a/server/test/TOTPValidator.test.ts +++ b/server/test/TOTPValidator.test.ts @@ -1,26 +1,37 @@ import { TOTPValidator } from "../src/lib/TOTPValidator"; -import sinon = require("sinon"); -import Promise = require("bluebird"); -import SpeakeasyMock = require("./mocks/speakeasy"); +import Sinon = require("sinon"); +import Speakeasy = require("speakeasy"); describe("test TOTP validation", function() { let totpValidator: TOTPValidator; + let totpValidateStub: Sinon.SinonStub; beforeEach(() => { - SpeakeasyMock.totp.returns("token"); - totpValidator = new TOTPValidator(SpeakeasyMock as any); + totpValidateStub = Sinon.stub(Speakeasy.totp, "verify"); + totpValidator = new TOTPValidator(Speakeasy); + }); + + afterEach(function() { + totpValidateStub.restore(); }); it("should validate the TOTP token", function() { const totp_secret = "NBD2ZV64R9UV1O7K"; const token = "token"; + totpValidateStub.withArgs({ + secret: totp_secret, + token: token, + encoding: "base32", + window: 1 + }).returns(true); return totpValidator.validate(token, totp_secret); }); it("should not validate a wrong TOTP token", function(done) { const totp_secret = "NBD2ZV64R9UV1O7K"; const token = "wrong token"; + totpValidateStub.returns(false); totpValidator.validate(token, totp_secret) .catch(function() { done(); diff --git a/server/test/storage/UserDataStore.test.ts b/server/test/storage/UserDataStore.test.ts index fcf2abbb5..c43d52a1d 100644 --- a/server/test/storage/UserDataStore.test.ts +++ b/server/test/storage/UserDataStore.test.ts @@ -29,7 +29,12 @@ describe("test user data store", function () { totpSecret = { ascii: "abc", base32: "ABCDKZLEFZGREJK", - otpauth_url: "totp://test" + otpauth_url: "totp://test", + google_auth_qr: "dummy", + hex: "dummy", + qr_code_ascii: "dummy", + qr_code_base32: "dummy", + qr_code_hex: "dummy" }; u2fRegistration = { diff --git a/server/types/TOTPSecret.ts b/server/types/TOTPSecret.ts index 33ce602c0..d6775f2f0 100644 --- a/server/types/TOTPSecret.ts +++ b/server/types/TOTPSecret.ts @@ -1,6 +1,11 @@ export interface TOTPSecret { - base32: string; ascii: string; - otpauth_url?: string; -} \ No newline at end of file + hex: string; + base32: string; + qr_code_ascii: string; + qr_code_hex: string; + qr_code_base32: string; + google_auth_qr: string; + otpauth_url: string; + } \ No newline at end of file diff --git a/server/types/speakeasy.d.ts b/server/types/speakeasy.d.ts new file mode 100644 index 000000000..6ea06948b --- /dev/null +++ b/server/types/speakeasy.d.ts @@ -0,0 +1,96 @@ +declare module "speakeasy" { + export = speakeasy + + interface SharedOptions { + encoding?: string + algorithm?: string + } + + interface DigestOptions extends SharedOptions { + secret: string + counter: number + } + + interface HOTPOptions extends SharedOptions { + secret: string + counter: number + digest?: Buffer + digits?: number + } + + interface HOTPVerifyOptions extends SharedOptions { + secret: string + token: string + counter: number + digits?: number + window?: number + } + + interface TOTPOptions extends SharedOptions { + secret: string + time?: number + step?: number + epoch?: number + counter?: number + digits?: number + } + + interface TOTPVerifyOptions extends SharedOptions { + secret: string + token: string + time?: number + step?: number + epoch?: number + counter?: number + digits?: number + window?: number + } + + interface GenerateSecretOptions { + length?: number + symbols?: boolean + otpauth_url?: boolean + name?: string + issuer?: string + } + + interface GeneratedSecret { + ascii: string + hex: string + base32: string + qr_code_ascii: string + qr_code_hex: string + qr_code_base32: string + google_auth_qr: string + otpauth_url: string + } + + interface OTPAuthURLOptions extends SharedOptions { + secret: string + label: string + type?: string + counter?: number + issuer?: string + digits?: number + period?: number + } + + interface Speakeasy { + digest: (options: DigestOptions) => Buffer + hotp: { + (options: HOTPOptions): string, + verifyDelta: (options: HOTPVerifyOptions) => boolean, + verify: (options: HOTPVerifyOptions) => boolean, + } + totp: { + (options: TOTPOptions): string + verifyDelta: (options: TOTPVerifyOptions) => boolean, + verify: (options: TOTPVerifyOptions) => boolean, + } + generateSecret: (options?: GenerateSecretOptions) => GeneratedSecret + generateSecretASCII: (length?: number, symbols?: boolean) => string + otpauthURL: (options: OTPAuthURLOptions) => string + } + + const speakeasy: Speakeasy +} \ No newline at end of file diff --git a/test/features/support/world.ts b/test/features/support/world.ts index 2bed95561..275135b5f 100644 --- a/test/features/support/world.ts +++ b/test/features/support/world.ts @@ -21,6 +21,7 @@ function CustomWorld() { }; this.setFieldTo = function (fieldName: string, content: string) { + const that = this; return this.driver.findElement(seleniumWebdriver.By.id(fieldName)) .sendKeys(content); }; @@ -49,22 +50,19 @@ function CustomWorld() { .findElement(seleniumWebdriver.By.tagName("button")) .findElement(seleniumWebdriver.By.xpath("//button[contains(.,'" + buttonText + "')]")) .click(); - }) - .then(function () { - return that.driver.sleep(1000); }); }; - this.waitUntilUrlContains = function(url: string) { + this.waitUntilUrlContains = function (url: string) { const that = this; return this.driver.wait(seleniumWebdriver.until.urlIs(url), 15000) - .then(function() {}, function(err: Error) { - that.driver.getCurrentUrl() - .then(function(current: string) { - console.error("====> Error due to: %s (current) != %s (expected)", current, url); + .then(function () { }, function (err: Error) { + that.driver.getCurrentUrl() + .then(function (current: string) { + console.error("====> Error due to: %s (current) != %s (expected)", current, url); + }); + return BluebirdPromise.reject(err); }); - return BluebirdPromise.reject(err); - }); }; this.loginWithUserPassword = function (username: string, password: string) {