From bc72f5c50849bc0f1e93c37d98dce597498a3501 Mon Sep 17 00:00:00 2001 From: Clement Michaud Date: Wed, 25 Apr 2018 00:41:41 +0200 Subject: [PATCH] Use x-original-url instead of host to deduce domain to check permissions for --- example/compose/nginx/portal/nginx.conf | 71 ++++++++++++++--------- server/src/lib/routes/logout/get.ts | 20 +++++-- server/src/lib/web_server/RestApi.ts | 2 +- server/test/routes/verify/get.test.ts | 8 +-- server/test/utils/DomainExtractor.test.ts | 12 ---- 5 files changed, 65 insertions(+), 48 deletions(-) diff --git a/example/compose/nginx/portal/nginx.conf b/example/compose/nginx/portal/nginx.conf index a3bf5295c..84dbd5b0a 100644 --- a/example/compose/nginx/portal/nginx.conf +++ b/example/compose/nginx/portal/nginx.conf @@ -74,11 +74,15 @@ http { location /auth_verify { internal; - proxy_set_header Host $http_host; - proxy_set_header X-Original-URI $request_uri; - proxy_set_header X-Real-IP $remote_addr; - proxy_set_header X-Forwarded-Proto $scheme; - proxy_set_header Content-Length ""; + proxy_set_header Host $http_host; + + proxy_set_header X-Original-URI $request_uri; + proxy_set_header X-Original-URL $scheme://$http_host$request_uri; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-Proto $scheme; + + proxy_pass_request_body off; + proxy_set_header Content-Length ""; proxy_pass $upstream_verify; } @@ -137,11 +141,15 @@ http { location /auth_verify { internal; - proxy_set_header Host $http_host; - proxy_set_header X-Original-URI $request_uri; - proxy_set_header X-Real-IP $remote_addr; - proxy_set_header X-Forwarded-Proto $scheme; - proxy_set_header Content-Length ""; + proxy_set_header Host $http_host; + + proxy_set_header X-Original-URI $request_uri; + proxy_set_header X-Original-URL $scheme://$http_host$request_uri; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-Proto $scheme; + + proxy_pass_request_body off; + proxy_set_header Content-Length ""; proxy_pass $upstream_verify; } @@ -183,11 +191,15 @@ http { location /auth_verify { internal; - proxy_set_header Host $http_host; - proxy_set_header X-Original-URI $request_uri; - proxy_set_header X-Real-IP $remote_addr; - proxy_set_header X-Forwarded-Proto $scheme; - proxy_set_header Content-Length ""; + proxy_set_header Host $http_host; + + proxy_set_header X-Original-URI $request_uri; + proxy_set_header X-Original-URL $scheme://$http_host$request_uri; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-Proto $scheme; + + proxy_pass_request_body off; + proxy_set_header Content-Length ""; proxy_pass $upstream_verify; } @@ -229,11 +241,15 @@ http { location /auth_verify { internal; - proxy_set_header Host $http_host; - proxy_set_header X-Original-URI $request_uri; - proxy_set_header X-Real-IP $remote_addr; - proxy_set_header X-Forwarded-Proto $scheme; - proxy_set_header Content-Length ""; + proxy_set_header Host $http_host; + + proxy_set_header X-Original-URI $request_uri; + proxy_set_header X-Original-URL $scheme://$http_host$request_uri; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-Proto $scheme; + + proxy_pass_request_body off; + proxy_set_header Content-Length ""; proxy_pass $upstream_verify; } @@ -276,12 +292,15 @@ http { location /auth_verify { internal; - proxy_set_header Host $http_host; - proxy_set_header X-Original-URI $request_uri; - proxy_set_header X-Real-IP $remote_addr; - proxy_set_header X-Forwarded-Proto $scheme; - proxy_set_header Content-Length ""; - proxy_set_header Proxy-Authorization $http_authorization; + proxy_set_header Host $http_host; + + proxy_set_header X-Original-URI $request_uri; + proxy_set_header X-Original-URL $scheme://$http_host$request_uri; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-Proto $scheme; + + proxy_pass_request_body off; + proxy_set_header Content-Length ""; proxy_pass $upstream_verify; } diff --git a/server/src/lib/routes/logout/get.ts b/server/src/lib/routes/logout/get.ts index eb8040959..4d5112146 100644 --- a/server/src/lib/routes/logout/get.ts +++ b/server/src/lib/routes/logout/get.ts @@ -1,10 +1,20 @@ import express = require("express"); import { AuthenticationSessionHandler } from "../../AuthenticationSessionHandler"; +import Constants = require("../../../../../shared/constants"); +import { ServerVariables } from "../../ServerVariables"; -export default function(req: express.Request, res: express.Response) { - const redirect_param = req.query.redirect; - const redirect_url = redirect_param || "/"; - AuthenticationSessionHandler.reset(req); - res.redirect(redirect_url); +function getRedirectParam(req: express.Request) { + return req.query[Constants.REDIRECT_QUERY_PARAM] != "undefined" + ? req.query[Constants.REDIRECT_QUERY_PARAM] + : undefined; +} + +export default function (vars: ServerVariables) { + return function(req: express.Request, res: express.Response) { + const redirect_param = getRedirectParam(req); + const redirect_url = redirect_param || "/"; + AuthenticationSessionHandler.reset(req); + res.redirect(redirect_url); + }; } \ No newline at end of file diff --git a/server/src/lib/web_server/RestApi.ts b/server/src/lib/web_server/RestApi.ts index ed2319971..204003c88 100644 --- a/server/src/lib/web_server/RestApi.ts +++ b/server/src/lib/web_server/RestApi.ts @@ -129,7 +129,7 @@ export class RestApi { RequireValidatedFirstFactor.middleware(vars.logger), SecondFactorGet.default(vars)); - app.get(Endpoints.LOGOUT_GET, LogoutGet.default); + app.get(Endpoints.LOGOUT_GET, LogoutGet.default(vars)); app.get(Endpoints.VERIFY_GET, VerifyGet.default(vars)); app.post(Endpoints.FIRST_FACTOR_POST, FirstFactorPost.default(vars)); diff --git a/server/test/routes/verify/get.test.ts b/server/test/routes/verify/get.test.ts index d46a0af6a..2baa49c3b 100644 --- a/server/test/routes/verify/get.test.ts +++ b/server/test/routes/verify/get.test.ts @@ -27,7 +27,7 @@ describe("test /api/verify endpoint", function () { redirect: "undefined" }; AuthenticationSessionHandler.reset(req as any); - req.headers.host = "secret.example.com"; + req.headers["x-original-url"] = "https://secret.example.com/"; const s = ServerVariablesMockBuilder.build(false); mocks = s.mocks; vars = s.variables; @@ -130,7 +130,7 @@ describe("test /api/verify endpoint", function () { authSession.first_factor = true; authSession.second_factor = true; authSession.userid = "myuser"; - req.headers.host = "test.example.com"; + req.headers["x-original-url"] = "https://test.example.com/"; mocks.accessController.isAccessAllowedMock.returns(false); return test_unauthorized_403({ @@ -147,7 +147,7 @@ describe("test /api/verify endpoint", function () { describe("given user tries to access a single factor endpoint", function () { beforeEach(function () { - req.headers["host"] = "redirect.url"; + req.headers["x-original-url"] = "https://redirect.url/"; mocks.config.authentication_methods.per_subdomain_methods = { "redirect.url": "single_factor" }; @@ -238,7 +238,7 @@ describe("test /api/verify endpoint", function () { mocks.ldapAuthenticator.authenticateStub.rejects(new Error( "Invalid credentials")); req.headers["proxy-authorization"] = "Basic am9objpwYXNzd29yZA=="; - req.query["redirect"] = REDIRECT_URL; + req.query["rd"] = REDIRECT_URL; return VerifyGet.default(vars)(req as express.Request, res as any) .then(function () { diff --git a/server/test/utils/DomainExtractor.test.ts b/server/test/utils/DomainExtractor.test.ts index f179ba9e7..80c971d2b 100644 --- a/server/test/utils/DomainExtractor.test.ts +++ b/server/test/utils/DomainExtractor.test.ts @@ -18,16 +18,4 @@ describe("test DomainExtractor", function () { Assert.equal(domain, "www.example.com"); }); }); - - describe("test fromHostHeader", function () { - it("should return domain when default port is used", function () { - const domain = DomainExtractor.fromHostHeader("www.example.com"); - Assert.equal(domain, "www.example.com"); - }); - - it("should return domain when non default port is used", function () { - const domain = DomainExtractor.fromHostHeader("www.example.com:8080"); - Assert.equal(domain, "www.example.com"); - }); - }); }); \ No newline at end of file