Fix redirection after authentication and error page when accessing restricted pages

example/nginx/nginx.conf

@@ -61,6 +61,7 @@ http {
         ssl_certificate_key /etc/ssl/server.key;
 
         error_page 401 = @error401;
+        error_page 403 = https://auth.test.local:8080/error/403;
         location @error401 {
             return 302 https://auth.test.local:8080;
         }

src/server/lib/routes/verify/get.ts

@@ -39,13 +39,15 @@ function verify_filter(req: express.Request, res: express.Response): BluebirdPro
     });
 }
 
-export default function (req: express.Request, res: express.Response) {
+export default function (req: express.Request, res: express.Response): BluebirdPromise<void> {
   const logger = ServerVariablesHandler.getLogger(req.app);
-  verify_filter(req, res)
+  return verify_filter(req, res)
     .then(function () {
       res.status(204);
       res.send();
+      return BluebirdPromise.resolve();
     })
+    .catch(exceptions.DomainAccessDenied, ErrorReplies.replyWithError403(res, logger))
     .catch(ErrorReplies.replyWithError401(res, logger));
 }
 

src/server/views/errors/401.pug

@@ -8,4 +8,4 @@ block form-header
   <img class="header-img" src="/img/warning.png" alt="">
 
 block content
-    <p>You are either not authorized or not <a href="/">logged in</a>.</p>
+    <p>Please <a href="/">log in</a> to access this resource.</p>

src/server/views/errors/403.pug

@@ -8,4 +8,4 @@ block form-header
   <img class="header-img" src="/img/warning.png" alt="">
 
 block content
-    <p>You are either not authorized or not <a href="/">logged in</a>.</p>
+    <p>You are not authorized to access this resource.</p>

test/features/redirection.feature

@@ -1,7 +1,23 @@
-Feature: User is redirected to authelia when he is not authenticated
+Feature: User is correctly redirected correctly 
 
-  Scenario: User is redirected to authelia
+  Scenario: User is redirected to authelia when he is not authenticated
     Given I'm on https://home.test.local:8080
     When I click on the link to secret.test.local
     Then I'm redirected to "https://auth.test.local:8080/"
 
+  Scenario: User is redirected to home page after several authentication tries
+    Given I'm on https://auth.test.local:8080/
+    And I login with user "john" and password "password"
+    And I register a TOTP secret called "Sec0"
+    And I visit "https://public.test.local:8080/secret.html"
+    When I login with user "john" and password "badpassword"
+    And I clear field "username"
+    And I login with user "john" and password "password" 
+    And I use "Sec0" as TOTP token handle
+    And I click on "TOTP"
+    Then I'm redirected to "https://public.test.local:8080/secret.html"
+
+  Scenario: User Harry does not have access to https://secret.test.local:8080/secret.html and thus he must get an error 401
+    When I register TOTP and login with user "harry" and password "password"
+    And I visit "https://secret.test.local:8080/secret.html"
+    Then I get an error 403

test/features/step_definitions/authentication.ts

@@ -14,6 +14,10 @@ Cucumber.defineSupportCode(function ({ Given, When, Then }) {
     return this.setFieldTo(fieldName, content);
   });
 
+  When("I clear field {stringInDoubleQuotes}", function (fieldName: string) {
+    return this.clearField(fieldName);
+  });
+
   When("I click on {stringInDoubleQuotes}", function (text: string) {
     return this.clickOnButton(text);
   });
@@ -55,20 +59,20 @@ Cucumber.defineSupportCode(function ({ Given, When, Then }) {
     return this.registerTotpAndSignin(username, password);
   });
 
-  function hasAccessToSecret(link: string, driver: any) {
-    return driver.get(link)
+  function hasAccessToSecret(link: string, that: any) {
+    return that.driver.get(link)
       .then(function () {
-        return driver.findElement(seleniumWebdriver.By.tagName("body")).getText()
+        return that.driver.findElement(seleniumWebdriver.By.tagName("body")).getText()
           .then(function (body: string) {
-            Assert(body.indexOf("This is a very important secret!") > -1);
+            Assert(body.indexOf("This is a very important secret!") > -1, body);
           });
       });
   }
 
-  function hasNoAccessToSecret(link: string, driver: any) {
-    return driver.get(link)
+  function hasNoAccessToSecret(link: string, that: any) {
+    return that.driver.get(link)
       .then(function () {
-        return driver.wait(seleniumWebdriver.until.urlIs("https://auth.test.local:8080/"));
+        return that.getErrorPage(403);
       });
   }
 
@@ -76,7 +80,7 @@ Cucumber.defineSupportCode(function ({ Given, When, Then }) {
     const promises = [];
     for (let i = 0; i < dataTable.rows().length; i++) {
       const url = (dataTable.hashes() as any)[i].url;
-      promises.push(hasAccessToSecret(url, this.driver));
+      promises.push(hasAccessToSecret(url, this));
     }
     return Promise.all(promises);
   });
@@ -85,7 +89,7 @@ Cucumber.defineSupportCode(function ({ Given, When, Then }) {
     const promises = [];
     for (let i = 0; i < dataTable.rows().length; i++) {
       const url = (dataTable.hashes() as any)[i].url;
-      promises.push(hasNoAccessToSecret(url, this.driver));
+      promises.push(hasNoAccessToSecret(url, this));
     }
     return Promise.all(promises);
   });

test/features/step_definitions/restrictions.ts

@@ -4,8 +4,6 @@ import Assert = require("assert");
 
 Cucumber.defineSupportCode(function ({ Given, When, Then }) {
   Then("I get an error {number}", function (code: number) {
-    return this.driver
-      .findElement(seleniumWebdriver.By.tagName("h1"))
-      .findElement(seleniumWebdriver.By.xpath("//h1[contains(.,'Error " + code + "')]"));
+    return this.getErrorPage(code);
   });
 });

test/features/support/world.ts

@@ -21,6 +21,16 @@ function CustomWorld() {
       .sendKeys(content);
   };
 
+  this.clearField = function (fieldName: string) {
+    return this.driver.findElement(seleniumWebdriver.By.id(fieldName)).clear();
+  };
+
+  this.getErrorPage = function (code: number) {
+    return this.driver
+      .findElement(seleniumWebdriver.By.tagName("h1"))
+      .findElement(seleniumWebdriver.By.xpath("//h1[contains(.,'Error " + code + "')]"));
+  };
+
   this.clickOnButton = function (buttonText: string) {
     return this.driver
       .findElement(seleniumWebdriver.By.tagName("button"))
@@ -29,9 +39,11 @@ function CustomWorld() {
   };
 
   this.loginWithUserPassword = function (username: string, password: string) {
-    return this.driver
-      .findElement(seleniumWebdriver.By.id("username"))
-      .sendKeys(username)
+    return that.driver.wait(seleniumWebdriver.until.elementLocated(seleniumWebdriver.By.id("username")), 4000)
+      .then(function () {
+        return that.driver.findElement(seleniumWebdriver.By.id("username"))
+          .sendKeys(username);
+      })
       .then(function () {
         return that.driver.findElement(seleniumWebdriver.By.id("password"))
           .sendKeys(password);
@@ -39,14 +51,14 @@ function CustomWorld() {
       .then(function () {
         return that.driver.findElement(seleniumWebdriver.By.tagName("button"))
           .click();
-      })
-      .then(function () {
-        return that.driver.wait(seleniumWebdriver.until.elementLocated(seleniumWebdriver.By.className("register-totp")), 4000);
       });
   };
 
   this.registerTotpSecret = function (totpSecretHandle: string) {
-    return this.driver.findElement(seleniumWebdriver.By.className("register-totp")).click()
+    return that.driver.wait(seleniumWebdriver.until.elementLocated(seleniumWebdriver.By.className("register-totp")), 4000)
+      .then(function () {
+        return that.driver.findElement(seleniumWebdriver.By.className("register-totp")).click();
+      })
       .then(function () {
         const notif = Fs.readFileSync("./notifications/notification.txt").toString();
         const regexp = new RegExp(/Link: (.+)/);
@@ -75,8 +87,11 @@ function CustomWorld() {
   };
 
   this.useTotpToken = function (totpSecret: string) {
-    return this.driver.findElement(seleniumWebdriver.By.id("token"))
+    return that.driver.wait(seleniumWebdriver.until.elementLocated(seleniumWebdriver.By.className("register-totp")), 4000)
+      .then(function () {
+        return that.driver.findElement(seleniumWebdriver.By.id("token"))
           .sendKeys(totpSecret);
+      });
   };
 
   this.registerTotpAndSignin = function (username: string, password: string) {

test/unit/server/routes/verify/get.test.ts

@@ -24,6 +24,8 @@ describe("test authentication token verification", function () {
 
     req = ExpressMock.RequestMock();
     res = ExpressMock.ResponseMock();
+    req.session = {};
+    AuthenticationSession.reset(req as any);
     req.headers = {};
     req.headers.host = "secret.example.com";
     const mocks = ServerVariablesMock.mock(req.app);
@@ -32,7 +34,7 @@ describe("test authentication token verification", function () {
     mocks.accessController = accessController as any;
   });
 
-  it("should be already authenticated", function (done) {
+  it("should be already authenticated", function () {
     req.session = {};
     AuthenticationSession.reset(req as any);
     const authSession = AuthenticationSession.get(req as any);
@@ -40,39 +42,34 @@ describe("test authentication token verification", function () {
     authSession.second_factor = true;
     authSession.userid = "myuser";
 
-    res.send = sinon.spy(function () {
+    return VerifyGet.default(req as express.Request, res as any)
+      .then(function () {
         assert.equal(204, res.status.getCall(0).args[0]);
-      done();
       });
-
-    VerifyGet.default(req as express.Request, res as any);
   });
 
   describe("given different cases of session", function () {
     function test_session(auth_session: AuthenticationSession.AuthenticationSession, status_code: number) {
-      return new BluebirdPromise(function (resolve, reject) {
-        req.session = {};
-        req.session.auth_session = auth_session;
-
-        res.send = sinon.spy(function () {
+      return VerifyGet.default(req as express.Request, res as any)
+        .then(function () {
           assert.equal(status_code, res.status.getCall(0).args[0]);
-          resolve();
-        });
-
-        VerifyGet.default(req as express.Request, res as any);
         });
     }
 
-    function test_unauthorized(auth_session: AuthenticationSession.AuthenticationSession) {
+    function test_non_authenticated_401(auth_session: AuthenticationSession.AuthenticationSession) {
       return test_session(auth_session, 401);
     }
 
+    function test_unauthorized_403(auth_session: AuthenticationSession.AuthenticationSession) {
+      return test_session(auth_session, 403);
+    }
+
     function test_authorized(auth_session: AuthenticationSession.AuthenticationSession) {
       return test_session(auth_session, 204);
     }
 
     it("should not be authenticated when second factor is missing", function () {
-      return test_unauthorized({
+      return test_non_authenticated_401({
         userid: "user",
         first_factor: true,
         second_factor: false,
@@ -82,7 +79,7 @@ describe("test authentication token verification", function () {
     });
 
     it("should not be authenticated when first factor is missing", function () {
-      return test_unauthorized({
+      return test_non_authenticated_401({
         userid: "user",
         first_factor: false,
         second_factor: true,
@@ -92,7 +89,7 @@ describe("test authentication token verification", function () {
     });
 
     it("should not be authenticated when userid is missing", function () {
-      return test_unauthorized({
+      return test_non_authenticated_401({
         userid: undefined,
         first_factor: true,
         second_factor: false,
@@ -102,7 +99,7 @@ describe("test authentication token verification", function () {
     });
 
     it("should not be authenticated when first and second factor are missing", function () {
-      return test_unauthorized({
+      return test_non_authenticated_401({
         userid: "user",
         first_factor: false,
         second_factor: false,
@@ -112,16 +109,21 @@ describe("test authentication token verification", function () {
     });
 
     it("should not be authenticated when session has not be initiated", function () {
-      return test_unauthorized(undefined);
+      return test_non_authenticated_401(undefined);
     });
 
     it("should not be authenticated when domain is not allowed for user", function () {
+      const authSession = AuthenticationSession.get(req as any);
+      authSession.first_factor = true;
+      authSession.second_factor = true;
+      authSession.userid = "myuser";
+
       req.headers.host = "test.example.com";
 
       accessController.isDomainAllowedForUser.returns(false);
       accessController.isDomainAllowedForUser.withArgs("test.example.com", "user", ["group1", "group2"]).returns(true);
 
-      return test_unauthorized({
+      return test_unauthorized_403({
         first_factor: true,
         second_factor: true,
         userid: "user",