From ab8aaeda258e77b3b80370194382c93db8156435 Mon Sep 17 00:00:00 2001 From: Clement Michaud Date: Tue, 10 Oct 2017 21:59:20 +0200 Subject: [PATCH] Add configuration schema validation before starting Authelia --- Gruntfile.js | 10 +- server/src/lib/Server.ts | 4 +- .../src/lib/configuration/Configuration.d.ts | 8 +- .../configuration/Configuration.schema.json | 6 -- ...ationAdapter.ts => ConfigurationParser.ts} | 9 +- server/src/lib/configuration/Validator.ts | 93 ++++++++++++++++--- ...er.test.ts => ConfigurationParser.test.ts} | 40 ++++---- .../LdapConfigurationAdaptation.test.ts | 14 +-- server/test/configuration/Validator.test.ts | 91 +++++++++++++++++- 9 files changed, 213 insertions(+), 62 deletions(-) rename server/src/lib/configuration/{ConfigurationAdapter.ts => ConfigurationParser.ts} (91%) rename server/test/configuration/{ConfigurationAdapter.test.ts => ConfigurationParser.test.ts} (74%) diff --git a/Gruntfile.js b/Gruntfile.js index 413b9684c..92bd0d4bd 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -1,5 +1,6 @@ module.exports = function (grunt) { const buildDir = "dist"; + const schemaDir = "server/src/lib/configuration/Configuration.schema.json" grunt.initConfig({ env: { @@ -17,7 +18,7 @@ module.exports = function (grunt) { }, "generate-config-schema": { cmd: "./node_modules/.bin/typescript-json-schema", - args: ["-o", `${buildDir}/server/src/lib/configuration/Configuration.schema.json`, "--strictNullChecks", + args: ["-o", schemaDir, "--strictNullChecks", "--required", "server/tsconfig.json", "UserConfiguration"] }, "compile-client": { @@ -86,6 +87,10 @@ module.exports = function (grunt) { src: '**', dest: `${buildDir}/server/src/public_html/js/` }, + schema: { + src: schemaDir, + dest: `${buildDir}/${schemaDir}` + } }, browserify: { dist: { @@ -181,9 +186,10 @@ module.exports = function (grunt) { grunt.registerTask('test-int', ['run:test-int']); grunt.registerTask('copy-resources', ['copy:resources', 'copy:views', 'copy:images', 'copy:thirdparties', 'concat:css']); + grunt.registerTask('generate-config-schema', ['run:generate-config-schema', 'copy:schema']); grunt.registerTask('build-client', ['compile-client', 'browserify']); - grunt.registerTask('build-server', ['compile-server', 'copy-resources', 'run:generate-config-schema']); + grunt.registerTask('build-server', ['compile-server', 'copy-resources', 'generate-config-schema']); grunt.registerTask('build', ['build-client', 'build-server']); grunt.registerTask('build-dist', ['build', 'run:minify', 'cssmin', 'run:include-minified-script']); diff --git a/server/src/lib/Server.ts b/server/src/lib/Server.ts index 8f9f092af..436571fc8 100644 --- a/server/src/lib/Server.ts +++ b/server/src/lib/Server.ts @@ -6,7 +6,7 @@ import { AppConfiguration, UserConfiguration } from "./configuration/Configurati import { GlobalDependencies } from "../../types/Dependencies"; import { AuthenticationRegulator } from "./AuthenticationRegulator"; import { UserDataStore } from "./storage/UserDataStore"; -import { ConfigurationAdapter } from "./configuration/ConfigurationAdapter"; +import { ConfigurationParser } from "./configuration/ConfigurationParser"; import { TOTPValidator } from "./TOTPValidator"; import { TOTPGenerator } from "./TOTPGenerator"; import { RestApi } from "./RestApi"; @@ -110,7 +110,7 @@ export default class Server { const that = this; const app = Express(); - const appConfiguration = ConfigurationAdapter.adapt(userConfiguration); + const appConfiguration = ConfigurationParser.parse(userConfiguration); // by default the level of logs is info deps.winston.level = userConfiguration.logs_level; diff --git a/server/src/lib/configuration/Configuration.d.ts b/server/src/lib/configuration/Configuration.d.ts index b06aecd1f..802572b2f 100644 --- a/server/src/lib/configuration/Configuration.d.ts +++ b/server/src/lib/configuration/Configuration.d.ts @@ -48,10 +48,10 @@ export type ACLGroupsRules = { [group: string]: ACLRule[]; }; export type ACLUsersRules = { [user: string]: ACLRule[]; }; export interface ACLConfiguration { - default_policy: ACLPolicy; - any: ACLDefaultRules; - groups: ACLGroupsRules; - users: ACLUsersRules; + default_policy?: ACLPolicy; + any?: ACLDefaultRules; + groups?: ACLGroupsRules; + users?: ACLUsersRules; } export interface SessionRedisOptions { diff --git a/server/src/lib/configuration/Configuration.schema.json b/server/src/lib/configuration/Configuration.schema.json index ccb76813b..9fd4e9238 100644 --- a/server/src/lib/configuration/Configuration.schema.json +++ b/server/src/lib/configuration/Configuration.schema.json @@ -85,12 +85,6 @@ "type": "object" } }, - "required": [ - "any", - "default_policy", - "groups", - "users" - ], "type": "object" }, "ACLPolicy": { diff --git a/server/src/lib/configuration/ConfigurationAdapter.ts b/server/src/lib/configuration/ConfigurationParser.ts similarity index 91% rename from server/src/lib/configuration/ConfigurationAdapter.ts rename to server/src/lib/configuration/ConfigurationParser.ts index bf7365fd3..32f5acf53 100644 --- a/server/src/lib/configuration/ConfigurationAdapter.ts +++ b/server/src/lib/configuration/ConfigurationParser.ts @@ -88,8 +88,13 @@ function adaptFromUserConfiguration(userConfiguration: UserConfiguration) }; } -export class ConfigurationAdapter { - static adapt(userConfiguration: UserConfiguration): AppConfiguration { +export class ConfigurationParser { + static parse(userConfiguration: UserConfiguration): AppConfiguration { + const errors = Validator.isValid(userConfiguration); + if (errors.length > 0) { + errors.forEach((e: string) => { console.log(e); }); + throw new Error("Malformed configuration. Please double-check your configuration file."); + } const appConfiguration = adaptFromUserConfiguration(userConfiguration); const ldapUrl = process.env[LDAP_URL_ENV_VARIABLE]; diff --git a/server/src/lib/configuration/Validator.ts b/server/src/lib/configuration/Validator.ts index ce88f29ab..5058d08d7 100644 --- a/server/src/lib/configuration/Validator.ts +++ b/server/src/lib/configuration/Validator.ts @@ -1,20 +1,85 @@ import Ajv = require("ajv"); import Path = require("path"); +import Util = require("util"); +import { + UserConfiguration, StorageConfiguration, + NotifierConfiguration +} from "./Configuration"; + +function validateSchema(configuration: UserConfiguration): string[] { + const schema = require(Path.resolve(__dirname, "./Configuration.schema.json")); + const ajv = new Ajv({ + allErrors: true, + missingRefs: "fail" + }); + ajv.addMetaSchema(require("ajv/lib/refs/json-schema-draft-04.json")); + const valid = ajv.validate(schema, configuration); + if (!valid) + return ajv.errors.map( + (e: Ajv.ErrorObject) => { return ajv.errorsText([e]); }); + return []; +} + +function validateUnknownKeys(path: string, obj: any, knownKeys: string[]) { + const keysSet = new Set(Object.keys(obj)); + const knownKeysSet = new Set(knownKeys); + + const unknownKeysSet = new Set( + [...keysSet].filter(x => !knownKeysSet.has(x))); + + if (unknownKeysSet.size > 0) { + const unknownKeys = Array.from(unknownKeysSet); + return unknownKeys.map((k: string) => { return Util.format("data.%s has unknown key '%s'", path, k); }); + } + return []; +} + +function validateStorage(storage: any) { + const ERROR = "Storage must be either 'local' or 'mongo'"; + + if (!storage) + return []; + + const errors = validateUnknownKeys("storage", storage, ["local", "mongo"]); + if (errors.length > 0) + return errors; + + if (storage.local && storage.mongo) + return [ERROR]; + + if (!storage.local && !storage.mongo) + return [ERROR]; + + return []; +} + +function validateNotifier(notifier: NotifierConfiguration) { + const ERROR = "Notifier must be either 'filesystem', 'gmail' or 'smtp'"; + + if (!notifier) + return []; + + const errors = validateUnknownKeys("notifier", notifier, ["filesystem", "gmail", "smtp"]); + if (errors.length > 0) + return errors; + + if (notifier && notifier.filesystem && notifier.gmail && notifier.smtp) + return [ERROR]; + + if (notifier && !notifier.filesystem && !notifier.gmail && !notifier.smtp) + return [ERROR]; + + return []; +} export class Validator { - static isValid(configuration: any) { - const schema = require(Path.resolve(__dirname, "./Configuration.schema.json")); - const ajv = new Ajv({ - allErrors: true, - missingRefs: "fail" - }); - ajv.addMetaSchema(require("ajv/lib/refs/json-schema-draft-04.json")); - const valid = ajv.validate(schema, configuration); - if (!valid) { - for (const i in ajv.errors) { - console.log(ajv.errorsText([ajv.errors[i]])); - } - } - return valid; + static isValid(configuration: any): string[] { + const schemaErrors = validateSchema(configuration); + const storageErrors = validateStorage(configuration.storage); + const notifierErrors = validateNotifier(configuration.notifier); + + return schemaErrors + .concat(storageErrors) + .concat(notifierErrors); } } \ No newline at end of file diff --git a/server/test/configuration/ConfigurationAdapter.test.ts b/server/test/configuration/ConfigurationParser.test.ts similarity index 74% rename from server/test/configuration/ConfigurationAdapter.test.ts rename to server/test/configuration/ConfigurationParser.test.ts index 5a2f5299a..f851dde02 100644 --- a/server/test/configuration/ConfigurationAdapter.test.ts +++ b/server/test/configuration/ConfigurationParser.test.ts @@ -3,10 +3,10 @@ import { UserConfiguration, LdapConfiguration, ACLConfiguration } from "../../src/lib/configuration/Configuration"; -import { ConfigurationAdapter } from "../../src/lib/configuration/ConfigurationAdapter"; +import { ConfigurationParser } from "../../src/lib/configuration/ConfigurationParser"; -describe("test config adapter", function () { - function build_yaml_config(): UserConfiguration { +describe("test config parser", function () { + function buildYamlConfig(): UserConfiguration { const yaml_config: UserConfiguration = { port: 8080, ldap: { @@ -46,50 +46,50 @@ describe("test config adapter", function () { describe("port", function () { it("should read the port from the yaml file", function () { - const yaml_config = build_yaml_config(); + const yaml_config = buildYamlConfig(); yaml_config.port = 7070; - const config = ConfigurationAdapter.adapt(yaml_config); + const config = ConfigurationParser.parse(yaml_config); Assert.equal(config.port, 7070); }); it("should default the port to 8080 if not provided", function () { - const yaml_config = build_yaml_config(); + const yaml_config = buildYamlConfig(); delete yaml_config.port; - const config = ConfigurationAdapter.adapt(yaml_config); + const config = ConfigurationParser.parse(yaml_config); Assert.equal(config.port, 8080); }); }); it("should get the session attributes", function () { - const yaml_config = build_yaml_config(); + const yaml_config = buildYamlConfig(); yaml_config.session = { domain: "example.com", secret: "secret", expiration: 3600 }; - const config = ConfigurationAdapter.adapt(yaml_config); + const config = ConfigurationParser.parse(yaml_config); Assert.equal(config.session.domain, "example.com"); Assert.equal(config.session.secret, "secret"); Assert.equal(config.session.expiration, 3600); }); it("should get the log level", function () { - const yaml_config = build_yaml_config(); + const yaml_config = buildYamlConfig(); yaml_config.logs_level = "debug"; - const config = ConfigurationAdapter.adapt(yaml_config); + const config = ConfigurationParser.parse(yaml_config); Assert.equal(config.logs_level, "debug"); }); it("should get the notifier config", function () { - const yaml_config = build_yaml_config(); - yaml_config.notifier = { + const userConfig = buildYamlConfig(); + userConfig.notifier = { gmail: { username: "user", password: "pass", sender: "admin@example.com" } }; - const config = ConfigurationAdapter.adapt(yaml_config); + const config = ConfigurationParser.parse(userConfig); Assert.deepEqual(config.notifier, { gmail: { username: "user", @@ -101,8 +101,8 @@ describe("test config adapter", function () { describe("access_control", function() { it("should adapt access_control when it is already ok", function () { - const yaml_config = build_yaml_config(); - yaml_config.access_control = { + const userConfig = buildYamlConfig(); + userConfig.access_control = { default_policy: "deny", any: [{ domain: "public.example.com", @@ -116,7 +116,7 @@ describe("test config adapter", function () { }, groups: {} }; - const config = ConfigurationAdapter.adapt(yaml_config); + const config = ConfigurationParser.parse(userConfig); Assert.deepEqual(config.access_control, { default_policy: "deny", any: [{ @@ -135,9 +135,9 @@ describe("test config adapter", function () { it("should adapt access_control when it is empty", function () { - const yaml_config = build_yaml_config(); - yaml_config.access_control = {} as any; - const config = ConfigurationAdapter.adapt(yaml_config); + const userConfig = buildYamlConfig(); + userConfig.access_control = {} as any; + const config = ConfigurationParser.parse(userConfig); Assert.deepEqual(config.access_control, { default_policy: "deny", any: [], diff --git a/server/test/configuration/LdapConfigurationAdaptation.test.ts b/server/test/configuration/LdapConfigurationAdaptation.test.ts index 79d9ae24e..59aab2537 100644 --- a/server/test/configuration/LdapConfigurationAdaptation.test.ts +++ b/server/test/configuration/LdapConfigurationAdaptation.test.ts @@ -1,6 +1,6 @@ import * as Assert from "assert"; import { UserConfiguration, LdapConfiguration } from "../../src/lib/configuration/Configuration"; -import { ConfigurationAdapter } from "../../src/lib/configuration/ConfigurationAdapter"; +import { ConfigurationParser } from "../../src/lib/configuration/ConfigurationParser"; describe("test ldap configuration adaptation", function () { function build_yaml_config(): UserConfiguration { @@ -42,15 +42,15 @@ describe("test ldap configuration adaptation", function () { } it("should adapt correctly while user only specify mandatory fields", function () { - const yaml_config = build_yaml_config(); - yaml_config.ldap = { + const userConfig = build_yaml_config(); + userConfig.ldap = { url: "http://ldap", base_dn: "dc=example,dc=com", user: "admin", password: "password" }; - const config = ConfigurationAdapter.adapt(yaml_config); + const config = ConfigurationParser.parse(userConfig); const expectedConfig: LdapConfiguration = { url: "http://ldap", users_dn: "dc=example,dc=com", @@ -67,8 +67,8 @@ describe("test ldap configuration adaptation", function () { }); it("should adapt correctly while user specify every fields", function () { - const yaml_config = build_yaml_config(); - yaml_config.ldap = { + const userConfig = build_yaml_config(); + userConfig.ldap = { url: "http://ldap-server", base_dn: "dc=example,dc=com", additional_users_dn: "ou=users", @@ -81,7 +81,7 @@ describe("test ldap configuration adaptation", function () { password: "password2" }; - const config = ConfigurationAdapter.adapt(yaml_config); + const config = ConfigurationParser.parse(userConfig); const expectedConfig: LdapConfiguration = { url: "http://ldap-server", users_dn: "ou=users,dc=example,dc=com", diff --git a/server/test/configuration/Validator.test.ts b/server/test/configuration/Validator.test.ts index db433f3d8..69d3afca5 100644 --- a/server/test/configuration/Validator.test.ts +++ b/server/test/configuration/Validator.test.ts @@ -1,10 +1,91 @@ import { Validator } from "../../src/lib/configuration/Validator"; import Assert = require("assert"); -describe.only("test validator", function() { - it("should validate a correct user configuration", function() { - Assert(Validator.validate({ - ldap: {} - })); +describe("test validator", function () { + it("should validate wrong user configurations", function () { + // Some examples + Assert.deepStrictEqual(Validator.isValid({}), [ + "data should have required property 'ldap'", + "data should have required property 'notifier'", + "data should have required property 'regulation'", + "data should have required property 'session'", + "data should have required property 'storage'" + ]); + + Assert.deepStrictEqual(Validator.isValid({ + ldap: {}, + notifier: {}, + regulation: {}, + session: {}, + storage: {} + }), [ + "data.ldap should have required property 'base_dn'", + "data.ldap should have required property 'password'", + "data.ldap should have required property 'url'", + "data.ldap should have required property 'user'", + "data.regulation should have required property 'ban_time'", + "data.regulation should have required property 'find_time'", + "data.regulation should have required property 'max_retries'", + "data.session should have required property 'secret'", + "Storage must be either 'local' or 'mongo'", + "Notifier must be either 'filesystem', 'gmail' or 'smtp'" + ]); + + Assert.deepStrictEqual(Validator.isValid({ + ldap: { + base_dn: "dc=example,dc=com", + password: "password", + url: "ldap://ldap", + user: "user" + }, + notifier: { + abcd: [] + }, + regulation: { + ban_time: 120, + find_time: 30, + max_retries: 3 + }, + session: { + secret: "unsecure_secret" + }, + storage: { + abc: {} + } + }), [ + "data.storage has unknown key 'abc'", + "data.notifier has unknown key 'abcd'" + ]); + }); + + it("should validate correct user configurations", function () { + Assert.deepStrictEqual(Validator.isValid({ + ldap: { + base_dn: "dc=example,dc=com", + password: "password", + url: "ldap://ldap", + user: "user" + }, + notifier: { + gmail: { + username: "user@gmail.com", + password: "pass", + sender: "admin@example.com" + } + }, + regulation: { + ban_time: 120, + find_time: 30, + max_retries: 3 + }, + session: { + secret: "unsecure_secret" + }, + storage: { + local: { + path: "/var/lib/authelia" + } + } + }), []); }); }); \ No newline at end of file