Merge pull request #185 from clems4ever/totp-issuer-and-label

Use issuer and label when generating otpauthURL for TOTP

server/src/lib/authentication/totp/ITotpHandler.ts

@@ -1,14 +1,6 @@
 import { TOTPSecret } from "../../../../types/TOTPSecret";
 
-export interface GenerateSecretOptions {
-  length?: number;
-  symbols?: boolean;
-  otpauth_url?: boolean;
-  name?: string;
-  issuer?: string;
-}
-
 export interface ITotpHandler {
-  generate(options?: GenerateSecretOptions): TOTPSecret;
+  generate(label: string, issuer: string): TOTPSecret;
   validate(token: string, secret: string): boolean;
 }

server/src/lib/authentication/totp/TotpHandler.ts

@@ -1,4 +1,4 @@
-import { ITotpHandler, GenerateSecretOptions } from "./ITotpHandler";
+import { ITotpHandler } from "./ITotpHandler";
 import { TOTPSecret } from "../../../../types/TOTPSecret";
 import Speakeasy = require("speakeasy");
 
@@ -12,8 +12,17 @@ export class TotpHandler implements ITotpHandler {
     this.speakeasy = speakeasy;
   }
 
-  generate(options?: GenerateSecretOptions): TOTPSecret {
+  generate(label: string, issuer: string): TOTPSecret {
-    return this.speakeasy.generateSecret(options);
+    const secret = this.speakeasy.generateSecret({
+      otpauth_url: false
+    });
+
+    secret.otpauth_url = this.speakeasy.otpauthURL({
+      secret: secret.ascii,
+      label: label,
+      issuer: issuer
+    });
+    return secret;
   }
 
   validate(token: string, secret: string): boolean {
@@ -22,6 +31,6 @@ export class TotpHandler implements ITotpHandler {
       encoding: TOTP_ENCODING,
       token: token,
       window: WINDOW
-    } as any);
+    });
   }
 }

server/src/lib/configuration/Configuration.d.ts

@@ -35,7 +35,7 @@ type UserName = string;
 type GroupName = string;
 type DomainPattern = string;
 
-export type ACLPolicy = 'deny' | 'allow';
+export type ACLPolicy = 'deny' | 'allow';
 
 export type ACLRule = {
   domain: string;
@@ -121,6 +121,10 @@ export interface AuthenticationMethodsConfiguration {
   per_subdomain_methods?: AuthenticationMethodPerSubdomain;
 }
 
+export interface TOTPConfiguration {
+  issuer: string;
+}
+
 export interface UserConfiguration {
   port?: number;
   logs_level?: string;
@@ -132,6 +136,7 @@ export interface UserConfiguration {
   access_control?: ACLConfiguration;
   regulation: RegulationConfiguration;
   default_redirection_url?: string;
+  totp?: TOTPConfiguration;
 }
 
 export interface AppConfiguration {
@@ -145,4 +150,5 @@ export interface AppConfiguration {
   access_control?: ACLConfiguration;
   regulation: RegulationConfiguration;
   default_redirection_url?: string;
+  totp: TOTPConfiguration;
 }

server/src/lib/configuration/ConfigurationParser.ts

@@ -8,6 +8,7 @@ import {
 } from "./Configuration";
 import Util = require("util");
 import { ACLAdapter } from "./adapters/ACLAdapter";
+import { TOTPAdapter } from "./adapters/TOTPAdapter";
 import { AuthenticationMethodsAdapter } from "./adapters/AuthenticationMethodsAdapter";
 import { Validator } from "./Validator";
 
@@ -63,6 +64,7 @@ function adaptFromUserConfiguration(userConfiguration: UserConfiguration)
   const ldapConfiguration = adaptLdapConfiguration(userConfiguration.ldap);
   const authenticationMethods = AuthenticationMethodsAdapter
     .adapt(userConfiguration.authentication_methods);
+  const totpConfiguration = TOTPAdapter.adapt(userConfiguration.totp);
 
   return {
     port: port,
@@ -83,7 +85,8 @@ function adaptFromUserConfiguration(userConfiguration: UserConfiguration)
     access_control: ACLAdapter.adapt(userConfiguration.access_control),
     regulation: userConfiguration.regulation,
     authentication_methods: authenticationMethods,
-    default_redirection_url: userConfiguration.default_redirection_url
+    default_redirection_url: userConfiguration.default_redirection_url,
+    totp: totpConfiguration
   };
 }
 

server/src/lib/configuration/adapters/TOTPAdapter.ts

@@ -0,0 +1,20 @@
+import { TOTPConfiguration } from "../Configuration";
+import { ObjectCloner } from "../../utils/ObjectCloner";
+
+const DEFAULT_ISSUER = "authelia.com";
+
+export class TOTPAdapter {
+  static adapt(configuration: TOTPConfiguration): TOTPConfiguration {
+    const newConfiguration = {
+      issuer: DEFAULT_ISSUER
+    };
+
+    if (!configuration)
+      return newConfiguration;
+
+    if (configuration && configuration.issuer)
+      newConfiguration.issuer = configuration.issuer;
+
+    return newConfiguration;
+  }
+}

server/src/lib/routes/secondfactor/totp/identity/RegistrationHandler.ts

@@ -16,19 +16,22 @@ import { IRequestLogger } from "../../../../logging/IRequestLogger";
 import { IUserDataStore } from "../../../../storage/IUserDataStore";
 import { ITotpHandler } from "../../../../authentication/totp/ITotpHandler";
 import { TOTPSecret } from "../../../../../../types/TOTPSecret";
+import { TOTPConfiguration } from "../../../../configuration/Configuration";
 
 
 export default class RegistrationHandler implements IdentityValidable {
   private logger: IRequestLogger;
   private userDataStore: IUserDataStore;
   private totp: ITotpHandler;
+  private configuration: TOTPConfiguration;
 
   constructor(logger: IRequestLogger,
     userDataStore: IUserDataStore,
-    totp: ITotpHandler) {
+    totp: ITotpHandler, configuration: TOTPConfiguration) {
     this.logger = logger;
     this.userDataStore = userDataStore;
     this.totp = totp;
+    this.configuration = configuration;
   }
 
   challenge(): string {
@@ -70,22 +73,24 @@ export default class RegistrationHandler implements IdentityValidable {
     return FirstFactorValidator.validate(req, this.logger);
   }
 
-  postValidationResponse(req: express.Request, res: express.Response): BluebirdPromise<void> {
+  postValidationResponse(req: express.Request, res: express.Response)
+    : BluebirdPromise<void> {
     const that = this;
     let secret: TOTPSecret;
     let userId: string;
     return new BluebirdPromise(function (resolve, reject) {
       const authSession = AuthenticationSessionHandler.get(req, that.logger);
-      const challenge = authSession.identity_check.challenge;
+      userId = authSession.userid;
-      userId = authSession.identity_check.userid;
 
-      if (challenge != Constants.CHALLENGE || !userId) {
+      if (authSession.identity_check.challenge != Constants.CHALLENGE
+        || !userId)
         return reject(new Error("Bad challenge."));
-      }
+
       resolve();
     })
       .then(function () {
-        secret = that.totp.generate();
+        secret = that.totp.generate(userId,
+          that.configuration.issuer);
         that.logger.debug(req, "Save the TOTP secret in DB");
         return that.userDataStore.saveTOTPSecret(userId, secret);
       })

server/src/lib/web_server/RestApi.ts

@@ -55,7 +55,7 @@ function setupTotp(app: Express.Application, vars: ServerVariables) {
     Endpoints.SECOND_FACTOR_TOTP_IDENTITY_START_GET,
     Endpoints.SECOND_FACTOR_TOTP_IDENTITY_FINISH_GET,
     new TOTPRegistrationIdentityHandler(vars.logger,
-      vars.userDataStore, vars.totpHandler),
+      vars.userDataStore, vars.totpHandler, vars.config.totp),
     vars);
 }
 

server/test/SessionConfigurationBuilder.test.ts

@@ -16,6 +16,9 @@ describe("test session configuration builder", function () {
         users: {},
         groups: {}
       },
+      totp: {
+        issuer: "authelia.com"
+      },
       ldap: {
         url: "ldap://ldap",
         user: "user",
@@ -90,6 +93,9 @@ describe("test session configuration builder", function () {
         users: {},
         groups: {}
       },
+      totp: {
+        issuer: "authelia.com"
+      },
       ldap: {
         url: "ldap://ldap",
         user: "user",

server/test/mocks/ServerVariablesMockBuilder.ts

@@ -35,6 +35,9 @@ export class ServerVariablesMockBuilder {
         authentication_methods: {
           default_method: "two_factor"
         },
+        totp: {
+          issuer: "authelia.com"
+        },
         ldap: {
           url: "ldap://ldap",
           user: "user",

server/test/mocks/TotpHandlerStub.ts

@@ -1,6 +1,6 @@
 import Sinon = require("sinon");
 import BluebirdPromise = require("bluebird");
-import { ITotpHandler, GenerateSecretOptions } from "../../src/lib/authentication/totp/ITotpHandler";
+import { ITotpHandler } from "../../src/lib/authentication/totp/ITotpHandler";
 import { TOTPSecret } from "../../types/TOTPSecret";
 
 export class TotpHandlerStub implements ITotpHandler {
@@ -12,8 +12,8 @@ export class TotpHandlerStub implements ITotpHandler {
     this.validateStub = Sinon.stub();
   }
 
-  generate(options?: GenerateSecretOptions): TOTPSecret {
+  generate(label: string, issuer: string): TOTPSecret {
-    return this.generateStub(options);
+    return this.generateStub(label, issuer);
   }
 
   validate(token: string, secret: string): boolean {

server/test/routes/secondfactor/totp/register/RegistrationHandler.test.ts

@@ -1,13 +1,13 @@
 import Sinon = require("sinon");
-import winston = require("winston");
 import RegistrationHandler from "../../../../../src/lib/routes/secondfactor/totp/identity/RegistrationHandler";
 import { Identity } from "../../../../../types/Identity";
 import { UserDataStore } from "../../../../../src/lib/storage/UserDataStore";
-import assert = require("assert");
 import BluebirdPromise = require("bluebird");
 import ExpressMock = require("../../../../mocks/express");
-import { ServerVariablesMock, ServerVariablesMockBuilder } from "../../../../mocks/ServerVariablesMockBuilder";
+import { ServerVariablesMock, ServerVariablesMockBuilder }
+  from "../../../../mocks/ServerVariablesMockBuilder";
 import { ServerVariables } from "../../../../../src/lib/ServerVariables";
+import Assert = require("assert");
 
 describe("test totp register", function () {
   let req: ExpressMock.RequestMock;
@@ -26,7 +26,11 @@ describe("test totp register", function () {
         userid: "user",
         email: "user@example.com",
         first_factor: true,
-        second_factor: false
+        second_factor: false,
+        identity_check: {
+          userid: "user",
+          challenge: "totp-register"
+        }
       }
     };
     req.headers = {};
@@ -36,23 +40,29 @@ describe("test totp register", function () {
       inMemoryOnly: true
     };
 
-    mocks.userDataStore.saveU2FRegistrationStub.returns(BluebirdPromise.resolve({}));
+    mocks.userDataStore.saveU2FRegistrationStub
-    mocks.userDataStore.retrieveU2FRegistrationStub.returns(BluebirdPromise.resolve({}));
+      .returns(BluebirdPromise.resolve({}));
-    mocks.userDataStore.produceIdentityValidationTokenStub.returns(BluebirdPromise.resolve({}));
+    mocks.userDataStore.retrieveU2FRegistrationStub
-    mocks.userDataStore.consumeIdentityValidationTokenStub.returns(BluebirdPromise.resolve({}));
+      .returns(BluebirdPromise.resolve({}));
-    mocks.userDataStore.saveTOTPSecretStub.returns(BluebirdPromise.resolve({}));
+    mocks.userDataStore.produceIdentityValidationTokenStub
+      .returns(BluebirdPromise.resolve({}));
+    mocks.userDataStore.consumeIdentityValidationTokenStub
+      .returns(BluebirdPromise.resolve({}));
+    mocks.userDataStore.saveTOTPSecretStub
+      .returns(BluebirdPromise.resolve({}));
 
     res = ExpressMock.ResponseMock();
   });
 
-  describe("test totp registration check", test_registration_check);
+  describe("test totp registration pre validation", function () {
-
-  function test_registration_check() {
     it("should fail if first_factor has not been passed", function () {
       req.session.auth.first_factor = false;
-      return new RegistrationHandler(vars.logger, vars.userDataStore, vars.totpHandler)
+      return new RegistrationHandler(vars.logger, vars.userDataStore,
+        vars.totpHandler, vars.config.totp)
         .preValidationInit(req as any)
-        .then(function () { return BluebirdPromise.reject(new Error("It should fail")); })
+        .then(function () {
+          return BluebirdPromise.reject(new Error("It should fail"));
+        })
         .catch(function (err: Error) {
           return BluebirdPromise.resolve();
         });
@@ -62,7 +72,8 @@ describe("test totp register", function () {
       req.session.auth.first_factor = false;
       req.session.auth.userid = undefined;
 
-      new RegistrationHandler(vars.logger, vars.userDataStore, vars.totpHandler)
+      new RegistrationHandler(vars.logger, vars.userDataStore, vars.totpHandler,
+        vars.config.totp)
         .preValidationInit(req as any)
         .catch(function (err: Error) {
           done();
@@ -73,19 +84,33 @@ describe("test totp register", function () {
       req.session.auth.first_factor = false;
       req.session.auth.email = undefined;
 
-      new RegistrationHandler(vars.logger, vars.userDataStore, vars.totpHandler)
+      new RegistrationHandler(vars.logger, vars.userDataStore, vars.totpHandler,
+        vars.config.totp)
         .preValidationInit(req as any)
         .catch(function (err: Error) {
           done();
         });
     });
 
-    it("should succeed if first factor passed, userid and email are provided", function (done) {
+    it("should succeed if first factor passed, userid and email are provided",
-      new RegistrationHandler(vars.logger, vars.userDataStore, vars.totpHandler)
+      function () {
-        .preValidationInit(req as any)
+        return new RegistrationHandler(vars.logger, vars.userDataStore,
-        .then(function (identity: Identity) {
+          vars.totpHandler, vars.config.totp)
-          done();
+          .preValidationInit(req as any);
+      });
+  });
+
+  describe("test totp registration post validation", function () {
+    it("should generate a secret using userId as label and issuer defined in config", function () {
+      vars.config.totp = {
+        issuer: "issuer"
+      };
+      return new RegistrationHandler(vars.logger, vars.userDataStore,
+        vars.totpHandler, vars.config.totp)
+        .postValidationResponse(req as any, res as any)
+        .then(function() {
+          Assert(mocks.totpHandler.generateStub.calledWithExactly("user", "issuer"));
+        });
     });
   });
-  }
 });

test/features/registration.feature

@@ -0,0 +1,14 @@
+Feature: Register secret for second factor
+
+  Scenario: Register a TOTP secret with correct label and issuer
+    Given I visit "https://auth.test.local:8080/"
+    And I login with user "john" and password "password"
+    When I register a TOTP secret called "Sec0"
+    Then the otpauth url has label "john" and issuer "authelia.com"
+
+  @needs-totp_issuer-config
+  Scenario: Register a TOTP secret with correct label and custom issuer
+    Given I visit "https://auth.test.local:8080/"
+    And I login with user "john" and password "password"
+    When I register a TOTP secret called "Sec0"
+    Then the otpauth url has label "john" and issuer "custom.com"

test/features/step_definitions/hooks.ts

@@ -43,6 +43,14 @@ Cucumber.defineSupportCode(function ({ After, Before }) {
     ");
   }
 
+  function createCustomTotpIssuerConfiguration(): BluebirdPromise<void> {
+    return exec("\
+    cat config.template.yml > config.test.yml && \
+    echo 'totp:' >> config.test.yml && \
+    echo '  issuer: custom.com' >> config.test.yml \
+    ");
+  }
+
   function declareNeedsConfiguration(tag: string, cb: () => BluebirdPromise<void>) {
     Before({ tags: "@needs-" + tag + "-config", timeout: 20 * 1000 }, function () {
       return cb()
@@ -62,6 +70,7 @@ Cucumber.defineSupportCode(function ({ After, Before }) {
   declareNeedsConfiguration("regulation", createRegulationConfiguration);
   declareNeedsConfiguration("inactivity", createInactivityConfiguration);
   declareNeedsConfiguration("single_factor", createSingleFactorConfiguration);
+  declareNeedsConfiguration("totp_issuer", createCustomTotpIssuerConfiguration);
 
   function registerUser(context: any, username: string) {
     let secret: Speakeasy.Key;
@@ -72,7 +81,7 @@ Cucumber.defineSupportCode(function ({ After, Before }) {
         const userDataStore = new UserDataStore(collectionFactory);
 
         const generator = new TotpHandler(Speakeasy);
-        secret = generator.generate();
+        secret = generator.generate("user", "authelia.com");
         return userDataStore.saveTOTPSecret(username, secret);
       })
       .then(function () {

test/features/step_definitions/registration.ts

@@ -0,0 +1,15 @@
+import Cucumber = require("cucumber");
+import seleniumWebdriver = require("selenium-webdriver");
+import Assert = require("assert");
+
+Cucumber.defineSupportCode(function ({ Given, When, Then }) {
+  When("the otpauth url has label {stringInDoubleQuotes} and issuer \
+{stringInDoubleQuotes}", function (label: string, issuer: string) {
+      return this.driver.findElement(seleniumWebdriver.By.id("qrcode"))
+        .getAttribute("title")
+        .then(function (title: string) {
+          const re = `^otpauth://totp/${label}\\?secret=[A-Z0-9]+&issuer=${issuer}$`;
+          Assert(new RegExp(re).test(title));
+        })
+    });
+});