From 97bfafb6eb8539cb0f4ef000db26696a03d2b15a Mon Sep 17 00:00:00 2001 From: Clement Michaud Date: Wed, 24 Oct 2018 00:29:09 +0200 Subject: [PATCH] [BREAKING] Flatten the ACL rules to enable some use cases. With previous configuration format rules were not ordered between groups and thus not predictable. Also in some cases `any` must have been a higher precedence than `groups`. Flattening the rules let the user apply whatever policy he can think of. When several rules match the (subject, domain, resource), the first one is applied. NOTE: This commit changed the format for declaring ACLs. Be sure to update your configuration file before upgrading. --- config.minimal.yml | 56 ++--- config.template.yml | 172 +++++++--------- server/src/lib/IdentityCheckMiddleware.ts | 8 +- .../src/lib/authorization/Authorizer.spec.ts | 194 +++++++++--------- server/src/lib/authorization/Authorizer.ts | 65 +++--- .../configuration/ConfigurationParser.spec.ts | 34 ++- .../SessionConfigurationBuilder.spec.ts | 4 +- .../schema/AclConfiguration.spec.ts | 28 ++- .../configuration/schema/AclConfiguration.ts | 35 ++-- .../lib/configuration/schema/Configuration.ts | 10 +- .../schema/NotifierConfiguration.spec.ts | 4 +- server/src/lib/routes/verify/get.spec.ts | 2 +- .../lib/storage/mongo/MongoCollection.spec.ts | 8 +- .../src/lib/storage/mongo/MongoCollection.ts | 2 +- test/features/access-control.feature | 4 +- test/features/restrictions.feature | 21 -- test/features/single-factor-domain.feature | 1 + .../single-factor-only-server.feature | 16 -- .../features/step_definitions/restrictions.ts | 1 - 19 files changed, 317 insertions(+), 348 deletions(-) delete mode 100644 test/features/single-factor-only-server.feature diff --git a/config.minimal.yml b/config.minimal.yml index 9170fcb92..8da7bc5e1 100644 --- a/config.minimal.yml +++ b/config.minimal.yml @@ -29,37 +29,37 @@ totp: access_control: # Default policy can either be `bypass`, `one_factor`, `two_factor` or `deny`. default_policy: deny - any: + + rules: - domain: single_factor.example.com policy: one_factor - groups: - admins: - # All resources in all domains - - domain: '*.example.com' - policy: two_factor - # Except mx2.mail.example.com (it restricts the first rule) - #- domain: 'mx2.mail.example.com' - # policy: deny - # User-based rules. - users: - john: - - domain: dev.example.com - policy: two_factor - resources: - - '^/users/john/.*$' - harry: - - domain: dev.example.com - policy: two_factor - resources: - - '^/users/harry/.*$' - bob: - - domain: '*.mail.example.com' - policy: two_factor - - domain: 'dev.example.com' - policy: two_factor - resources: - - '^/users/bob/.*$' + - domain: '*.example.com' + subject: "group:admins" + policy: two_factor + + - domain: dev.example.com + resources: + - '^/users/john/.*$' + subject: "user:john" + policy: two_factor + + - domain: dev.example.com + resources: + - '^/users/harry/.*$' + subject: "user:harry" + policy: two_factor + + - domain: '*.mail.example.com' + subject: "user:bob" + policy: two_factor + + - domain: dev.example.com + resources: + - '^/users/bob/.*$' + subject: "user:bob" + policy: two_factor + # Configuration of the authentication regulation mechanism. regulation: diff --git a/config.template.yml b/config.template.yml index 5b74c247a..2bfcb28e6 100644 --- a/config.template.yml +++ b/config.template.yml @@ -86,112 +86,96 @@ authentication_backend: ## path: ./users_database.yml -# Authentication methods -# -# Authentication methods can be defined per subdomain. -# There are currently two available methods: "single_factor" and "two_factor" -# -# Note: by default a domain uses "two_factor" method. -# -# Note: 'per_subdomain_methods' is a dictionary where keys must be subdomains and -# values must be one of the two possible methods. -# -# Note: 'per_subdomain_methods' is optional. -# -# Note: authentication_methods is optional. If it is not set all sub-domains -# are protected by two factors. -authentication_methods: - default_method: two_factor - per_subdomain_methods: - - - # Access Control # -# Access control is a set of rules you can use to restrict user access to certain -# resources. -# Any (apply to anyone), per-user or per-group rules can be defined. +# Access control is a list of rules defining the authorizations applied for one +# resource to users or group of users. # -# If 'access_control' is not defined, ACL rules are disabled and the `allow` default -# policy is applied, i.e., access is allowed to anyone. Otherwise restrictions follow +# If 'access_control' is not defined, ACL rules are disabled and the `bypass` +# rule is applied, i.e., access is allowed to anyone. Otherwise restrictions follow # the rules defined. -# -# Note: One can use the wildcard * to match any subdomain. +# +# Note: One can use the wildcard * to match any subdomain. # It must stand at the beginning of the pattern. (example: *.mydomain.com) -# -# Note: You must put the pattern in simple quotes when using the wildcard for the YAML +# +# Note: You must put patterns containing wildcards between simple quotes for the YAML # to be syntaxically correct. # -# Definition: A `rule` is an object with the following keys: `domain`, `policy` -# and `resources`. +# Definition: A `rule` is an object with the following keys: `domain`, `subject`, +# `policy` and `resources`. +# # - `domain` defines which domain or set of domains the rule applies to. -# - `policy` is the policy to apply to resources. It must be either `allow` or `deny`. -# - `resources` is a list of regular expressions that matches a set of resources to -# apply the policy to. # -# Note: Rules follow an order of priority defined as follows: -# In each category (`any`, `groups`, `users`), the latest rules have the highest -# priority. In other words, it means that if a given resource matches two rules in the -# same category, the latest one overrides the first one. -# Each category has also its own priority. That is, `users` has the highest priority, then -# `groups` and `any` has the lowest priority. It means if two rules in different categories -# match a given resource, the one in the category with the highest priority overrides the -# other one. +# - `subject` defines the subject to apply authorizations to. This parameter is +# optional and matching any user if not provided. If provided, the parameter +# represents either a user or a group. It should be of the form 'user:' +# or 'group:'. # +# - `policy` is the policy to apply to resources. It must be either `bypass`, +# `one_factor`, `two_factor` or `deny`. +# +# - `resources` is a list of regular expressions that matches a set of resources to +#  apply the policy to. This parameter is optional and matches any resource if not +# provided. +# +# Note: the order of the rules is important. The first policy matching +# (domain, resource, subject) applies. access_control: - # Default policy can either be `allow` or `deny`. - # It is the policy applied to any resource if it has not been overriden - # in the `any`, `groups` or `users` category. + # Default policy can either be `bypass`, `one_factor`, `two_factor` or `deny`. + # It is the policy applied to any resource if there is no policy to be applied + # to the user. default_policy: deny - # The rules that apply to anyone. - # The value is a list of rules. - any: + rules: + # Rules applied to everyone - domain: public.example.com policy: two_factor - domain: single_factor.example.com policy: one_factor - - # Group-based rules. The key is a group name and the value - # is a list of rules. - groups: - admin: - # All resources in all domains - - domain: '*.example.com' - policy: two_factor - # Except mx2.mail.example.com (it restricts the first rule) - - domain: 'mx2.mail.example.com' - policy: deny - dev: - - domain: dev.example.com - policy: two_factor - resources: - - '^/groups/dev/.*$' - - # User-based rules. The key is a user name and the value - # is a list of rules. - users: - john: - - domain: dev.example.com - policy: two_factor - resources: - - '^/users/john/.*$' - harry: - - domain: dev.example.com - policy: two_factor - resources: - - '^/users/harry/.*$' - bob: - - domain: '*.mail.example.com' - policy: two_factor - - domain: 'dev.example.com' - policy: two_factor - resources: - - '^/users/bob/.*$' + + # Rules applied to 'admin' group + - domain: 'mx2.mail.example.com' + subject: 'group:admin' + policy: deny + - domain: '*.example.com' + subject: 'group:admin' + policy: two_factor + + # Rules applied to 'dev' group + - domain: dev.example.com + resources: + - '^/groups/dev/.*$' + subject: 'group:dev' + policy: two_factor + + # Rules applied to user 'john' + - domain: dev.example.com + resources: + - '^/users/john/.*$' + subject: 'user:john' + policy: two_factor + + + # Rules applied to user 'harry' + - domain: dev.example.com + resources: + - '^/users/harry/.*$' + subject: 'user:harry' + policy: two_factor + + # Rules applied to user 'bob' + - domain: '*.mail.example.com' + subject: 'user:bob' + policy: two_factor + - domain: 'dev.example.com' + resources: + - '^/users/bob/.*$' + subject: 'user:bob' + policy: two_factor # Configuration of session cookies -# +# # The session cookies identify the user once logged in. session: # The name of the session cookie. (default: authelia_session). @@ -199,7 +183,7 @@ session: # The secret to encrypt the session cookie. secret: unsecure_session_secret - + # The time in ms before the cookie expires and session is reset. expiration: 3600000 # 1 hour @@ -208,9 +192,9 @@ session: # The domain to protect. # Note: the authenticator must also be in that domain. If empty, the cookie - # is restricted to the subdomain of the issuer. + # is restricted to the subdomain of the issuer. domain: example.com - + # The redis connection details redis: host: redis @@ -223,12 +207,12 @@ session: # It bans the user if too many attempts are done in a short period of # time. regulation: - # The number of failed login attempts before user is banned. + # The number of failed login attempts before user is banned. # Set it to 0 to disable regulation. max_retries: 3 # The time range during which the user can attempt login before being banned. - # The user is banned if the authenticaction failed `max_retries` times in a `find_time` seconds window. + # The user is banned if the authenticaction failed `max_retries` times in a `find_time` seconds window. find_time: 120 # The length of time before a banned user can login again. @@ -241,7 +225,7 @@ storage: # The directory where the DB files will be saved ## local: ## path: /var/lib/authelia/store - + # Settings to connect to mongo server mongo: url: mongodb://mongo @@ -261,13 +245,13 @@ notifier: ## filename: /tmp/authelia/notification.txt # Use your email account to send the notifications. You can use an app password. - # List of valid services can be found here: https://nodemailer.com/smtp/well-known/ + # List of valid services can be found here: https://nodemailer.com/smtp/well-known/ ## email: ## username: user@example.com ## password: yourpassword ## sender: admin@example.com ## service: gmail - + # Use a SMTP server for sending notifications smtp: username: test diff --git a/server/src/lib/IdentityCheckMiddleware.ts b/server/src/lib/IdentityCheckMiddleware.ts index fa525b4b2..e72ea4db1 100644 --- a/server/src/lib/IdentityCheckMiddleware.ts +++ b/server/src/lib/IdentityCheckMiddleware.ts @@ -102,7 +102,7 @@ export function get_start_validation(handler: IdentityValidable, let identity: Identity.Identity; return handler.preValidationInit(req) - .then(function (id: Identity.Identity) { + .then((id: Identity.Identity) => { identity = id; const email = identity.email; const userid = identity.userid; @@ -116,7 +116,7 @@ export function get_start_validation(handler: IdentityValidable, return createAndSaveToken(userid, handler.challenge(), vars.userDataStore); }) - .then(function (token: string) { + .then((token) => { const host = req.get("Host"); const link_url = util.format("https://%s%s?identity_token=%s", host, postValidationEndpoint, token); @@ -125,11 +125,11 @@ export function get_start_validation(handler: IdentityValidable, return vars.notifier.notify(identity.email, handler.mailSubject(), link_url); }) - .then(function () { + .then(() => { handler.preValidationResponse(req, res); return BluebirdPromise.resolve(); }) - .catch(Exceptions.IdentityError, function (err: Error) { + .catch(Exceptions.IdentityError, (err: Error) => { handler.preValidationResponse(req, res); return BluebirdPromise.resolve(); }) diff --git a/server/src/lib/authorization/Authorizer.spec.ts b/server/src/lib/authorization/Authorizer.spec.ts index 814773049..1027fb4b0 100644 --- a/server/src/lib/authorization/Authorizer.spec.ts +++ b/server/src/lib/authorization/Authorizer.spec.ts @@ -25,9 +25,7 @@ describe("authorization/Authorizer", function () { beforeEach(function () { configuration = { default_policy: "deny", - any: [], - users: {}, - groups: {} + rules: [] }; authorizer = new Authorizer(configuration, winston); }); @@ -42,9 +40,10 @@ describe("authorization/Authorizer", function () { }); it("should control access when multiple domain matcher is provided", function () { - configuration.users["user1"] = [{ + configuration.rules = [{ domain: "*.mail.example.com", policy: "two_factor", + subject: "user:user1", resources: [".*"] }]; Assert.equal(authorizer.authorization("home.example.com", "/", "user1", ["group1"]), Level.DENY); @@ -54,9 +53,10 @@ describe("authorization/Authorizer", function () { }); it("should allow access to all resources when resources is not provided", function () { - configuration.users["user1"] = [{ + configuration.rules = [{ domain: "*.mail.example.com", - policy: "two_factor" + policy: "two_factor", + subject: "user:user1" }]; Assert.equal(authorizer.authorization("home.example.com", "/", "user1", ["group1"]), Level.DENY); Assert.equal(authorizer.authorization("mx1.mail.example.com", "/", "user1", ["group1"]), Level.TWO_FACTOR); @@ -66,10 +66,11 @@ describe("authorization/Authorizer", function () { describe("check user rules", function () { it("should allow access when user has a matching allowing rule", function () { - configuration.users["user1"] = [{ + configuration.rules = [{ domain: "home.example.com", policy: "two_factor", - resources: [".*"] + resources: [".*"], + subject: "user:user1" }]; Assert.equal(authorizer.authorization("home.example.com", "/", "user1", ["group1"]), Level.TWO_FACTOR); Assert.equal(authorizer.authorization("home.example.com", "/another/resource", "user1", ["group1"]), Level.TWO_FACTOR); @@ -77,10 +78,11 @@ describe("authorization/Authorizer", function () { }); it("should deny to other users", function () { - configuration.users["user1"] = [{ + configuration.rules = [{ domain: "home.example.com", policy: "two_factor", - resources: [".*"] + resources: [".*"], + subject: "user:user1" }]; Assert.equal(authorizer.authorization("home.example.com", "/", "user2", ["group1"]), Level.DENY); Assert.equal(authorizer.authorization("home.example.com", "/another/resource", "user2", ["group1"]), Level.DENY); @@ -88,10 +90,11 @@ describe("authorization/Authorizer", function () { }); it("should allow user access only to specific resources", function () { - configuration.users["user1"] = [{ + configuration.rules = [{ domain: "home.example.com", policy: "two_factor", - resources: ["/private/.*", "^/begin", "/end$"] + resources: ["/private/.*", "^/begin", "/end$"], + subject: "user:user1" }]; Assert.equal(authorizer.authorization("home.example.com", "/", "user1", ["group1"]), Level.DENY); Assert.equal(authorizer.authorization("home.example.com", "/private", "user1", ["group1"]), Level.DENY); @@ -106,18 +109,21 @@ describe("authorization/Authorizer", function () { }); it("should allow access to multiple domains", function () { - configuration.users["user1"] = [{ + configuration.rules = [{ domain: "home.example.com", policy: "two_factor", - resources: [".*"] + resources: [".*"], + subject: "user:user1" }, { domain: "home1.example.com", policy: "one_factor", - resources: [".*"] + resources: [".*"], + subject: "user:user1" }, { domain: "home2.example.com", policy: "deny", - resources: [".*"] + resources: [".*"], + subject: "user:user1" }]; Assert.equal(authorizer.authorization("home.example.com", "/", "user1", ["group1"]), Level.TWO_FACTOR); Assert.equal(authorizer.authorization("home1.example.com", "/", "user1", ["group1"]), Level.ONE_FACTOR); @@ -125,19 +131,22 @@ describe("authorization/Authorizer", function () { Assert.equal(authorizer.authorization("home3.example.com", "/", "user1", ["group1"]), Level.DENY); }); - it("should always apply latest rule", function () { - configuration.users["user1"] = [{ + it("should apply rules in order", function () { + configuration.rules = [{ domain: "home.example.com", - policy: "two_factor", - resources: ["^/my/.*"] + policy: "one_factor", + resources: ["/my/private/resource"], + subject: "user:user1" }, { domain: "home.example.com", policy: "deny", - resources: ["^/my/private/.*"] + resources: ["^/my/private/.*"], + subject: "user:user1" }, { domain: "home.example.com", - policy: "one_factor", - resources: ["/my/private/resource"] + policy: "two_factor", + resources: ["^/my/.*"], + subject: "user:user1" }]; Assert.equal(authorizer.authorization("home.example.com", "/my/poney", "user1", ["group1"]), Level.TWO_FACTOR); @@ -148,19 +157,21 @@ describe("authorization/Authorizer", function () { describe("check group rules", function () { it("should allow access when user is in group having a matching allowing rule", function () { - configuration.groups["group1"] = [{ + configuration.rules = [{ domain: "home.example.com", policy: "two_factor", - resources: ["^/$"] - }]; - configuration.groups["group2"] = [{ + resources: ["^/$"], + subject: "group:group1" + }, { domain: "home.example.com", policy: "one_factor", - resources: ["^/test$"] + resources: ["^/test$"], + subject: "group:group2" }, { domain: "home.example.com", policy: "deny", - resources: ["^/private$"] + resources: ["^/private$"], + subject: "group:group2" }]; Assert.equal(authorizer.authorization("home.example.com", "/", "user1", ["group1", "group2", "group3"]), Level.TWO_FACTOR); @@ -176,9 +187,9 @@ describe("authorization/Authorizer", function () { describe("check any rules", function () { it("should control access when any rules are defined", function () { - configuration.any = [{ + configuration.rules = [{ domain: "home.example.com", - policy: "two_factor", + policy: "bypass", resources: ["^/public$"] }, { domain: "home.example.com", @@ -186,11 +197,11 @@ describe("authorization/Authorizer", function () { resources: ["^/private$"] }]; Assert.equal(authorizer.authorization("home.example.com", "/public", "user1", - ["group1", "group2", "group3"]), Level.TWO_FACTOR); + ["group1", "group2", "group3"]), Level.BYPASS); Assert.equal(authorizer.authorization("home.example.com", "/private", "user1", ["group1", "group2", "group3"]), Level.DENY); Assert.equal(authorizer.authorization("home.example.com", "/public", "user4", - ["group5"]), Level.TWO_FACTOR); + ["group5"]), Level.BYPASS); Assert.equal(authorizer.authorization("home.example.com", "/private", "user4", ["group5"]), Level.DENY); }); @@ -208,10 +219,11 @@ describe("authorization/Authorizer", function () { }); it("should deny access to one resource when defined", function () { - configuration.users["user1"] = [{ + configuration.rules = [{ domain: "home.example.com", policy: "deny", - resources: ["/test"] + resources: ["/test"], + subject: "user:user1" }]; Assert.equal(authorizer.authorization("home.example.com", "/", "user1", ["group1"]), Level.BYPASS); Assert.equal(authorizer.authorization("home.example.com", "/test", "user1", ["group1"]), Level.DENY); @@ -229,39 +241,30 @@ describe("authorization/Authorizer", function () { // admin is in groups ["admins"] // john is in groups ["dev", "admin-private"] // harry is in groups ["dev"] - configuration.any = [{ + configuration.rules = [{ domain: "home.example.com", policy: "two_factor", resources: ["^/public$", "^/$"] - }]; - configuration.groups["dev"] = [{ - domain: "home.example.com", - policy: "two_factor", - resources: ["^/dev/?.*$"] - }]; - configuration.groups["admins"] = [{ - domain: "home.example.com", - policy: "two_factor", - resources: [".*"] - }]; - configuration.groups["admin-private"] = [{ - domain: "home.example.com", - policy: "two_factor", - resources: ["^/private/?.*"] - }]; - configuration.users["john"] = [{ - domain: "home.example.com", - policy: "two_factor", - resources: ["^/private/john$"] - }]; - configuration.users["harry"] = [{ - domain: "home.example.com", - policy: "two_factor", - resources: ["^/private/harry"] }, { domain: "home.example.com", - policy: "deny", - resources: ["^/dev/b.*$"] + policy: "two_factor", + resources: [".*"], + subject: "group:admins" + }, { + domain: "home.example.com", + policy: "two_factor", + resources: ["^/private/?.*"], + subject: "group:admin-private" + }, { + domain: "home.example.com", + policy: "two_factor", + resources: ["^/private/john$"], + subject: "user:john" + }, { + domain: "home.example.com", + policy: "two_factor", + resources: ["^/private/harry"], + subject: "user:harry" }]; Assert.equal(authorizer.authorization("home.example.com", "/", "admin", ["admins"]), Level.TWO_FACTOR); @@ -275,8 +278,8 @@ describe("authorization/Authorizer", function () { Assert.equal(authorizer.authorization("home.example.com", "/", "john", ["dev", "admin-private"]), Level.TWO_FACTOR); Assert.equal(authorizer.authorization("home.example.com", "/public", "john", ["dev", "admin-private"]), Level.TWO_FACTOR); - Assert.equal(authorizer.authorization("home.example.com", "/dev", "john", ["dev", "admin-private"]), Level.TWO_FACTOR); - Assert.equal(authorizer.authorization("home.example.com", "/dev/bob", "john", ["dev", "admin-private"]), Level.TWO_FACTOR); + Assert.equal(authorizer.authorization("home.example.com", "/dev", "john", ["dev", "admin-private"]), Level.DENY); + Assert.equal(authorizer.authorization("home.example.com", "/dev/bob", "john", ["dev", "admin-private"]), Level.DENY); Assert.equal(authorizer.authorization("home.example.com", "/admin", "john", ["dev", "admin-private"]), Level.DENY); Assert.equal(authorizer.authorization("home.example.com", "/private/josh", "john", ["dev", "admin-private"]), Level.TWO_FACTOR); Assert.equal(authorizer.authorization("home.example.com", "/private/john", "john", ["dev", "admin-private"]), Level.TWO_FACTOR); @@ -284,7 +287,7 @@ describe("authorization/Authorizer", function () { Assert.equal(authorizer.authorization("home.example.com", "/", "harry", ["dev"]), Level.TWO_FACTOR); Assert.equal(authorizer.authorization("home.example.com", "/public", "harry", ["dev"]), Level.TWO_FACTOR); - Assert.equal(authorizer.authorization("home.example.com", "/dev", "harry", ["dev"]), Level.TWO_FACTOR); + Assert.equal(authorizer.authorization("home.example.com", "/dev", "harry", ["dev"]), Level.DENY); Assert.equal(authorizer.authorization("home.example.com", "/dev/bob", "harry", ["dev"]), Level.DENY); Assert.equal(authorizer.authorization("home.example.com", "/admin", "harry", ["dev"]), Level.DENY); Assert.equal(authorizer.authorization("home.example.com", "/private/josh", "harry", ["dev"]), Level.DENY); @@ -292,49 +295,50 @@ describe("authorization/Authorizer", function () { Assert.equal(authorizer.authorization("home.example.com", "/private/harry", "harry", ["dev"]), Level.TWO_FACTOR); }); - it("should control access when allowed at group level and denied at user level", function () { - configuration.groups["dev"] = [{ - domain: "home.example.com", - policy: "two_factor", - resources: ["^/dev/?.*$"] - }]; - configuration.users["john"] = [{ + it("should allow when allowed at group level and denied at user level", function () { + configuration.rules = [{ domain: "home.example.com", policy: "deny", - resources: ["^/dev/bob$"] + resources: ["^/dev/bob$"], + subject: "user:john" + }, { + domain: "home.example.com", + policy: "two_factor", + resources: ["^/dev/?.*$"], + subject: "group:dev" }]; Assert.equal(authorizer.authorization("home.example.com", "/dev/john", "john", ["dev"]), Level.TWO_FACTOR); Assert.equal(authorizer.authorization("home.example.com", "/dev/bob", "john", ["dev"]), Level.DENY); }); - it("should control access when allowed at 'any' level and denied at user level", function () { - configuration.any = [{ + it("should allow access when allowed at 'any' level and denied at user level", function () { + configuration.rules = [{ + domain: "home.example.com", + policy: "deny", + resources: ["^/dev/bob$"], + subject: "user:john" + }, { domain: "home.example.com", policy: "two_factor", resources: ["^/dev/?.*$"] }]; - configuration.users["john"] = [{ - domain: "home.example.com", - policy: "deny", - resources: ["^/dev/bob$"] - }]; Assert.equal(authorizer.authorization("home.example.com", "/dev/john", "john", ["dev"]), Level.TWO_FACTOR); Assert.equal(authorizer.authorization("home.example.com", "/dev/bob", "john", ["dev"]), Level.DENY); }); - it("should control access when allowed at 'any' level and denied at group level", function () { - configuration.any = [{ + it("should allow access when allowed at 'any' level and denied at group level", function () { + configuration.rules = [{ + domain: "home.example.com", + policy: "deny", + resources: ["^/dev/bob$"], + subject: "group:dev" + }, { domain: "home.example.com", policy: "two_factor", resources: ["^/dev/?.*$"] }]; - configuration.groups["dev"] = [{ - domain: "home.example.com", - policy: "deny", - resources: ["^/dev/bob$"] - }]; Assert.equal(authorizer.authorization("home.example.com", "/dev/john", "john", ["dev"]), Level.TWO_FACTOR); Assert.equal(authorizer.authorization("home.example.com", "/dev/bob", "john", ["dev"]), Level.DENY); @@ -344,17 +348,17 @@ describe("authorization/Authorizer", function () { // the priority from least to most is 'default_policy', 'all', 'group', 'user' // and the first rules in each category as a lower priority than the latest. // You can think of it that way: they override themselves inside each category. - configuration.any = [{ + configuration.rules = [{ domain: "home.example.com", policy: "two_factor", - resources: ["^/dev/?.*$"] - }]; - configuration.groups["dev"] = [{ + resources: ["^/dev/?.*$"], + subject: "user:john" + }, { domain: "home.example.com", policy: "deny", - resources: ["^/dev/bob$"] - }]; - configuration.users["john"] = [{ + resources: ["^/dev/bob$"], + subject: "group:dev" + }, { domain: "home.example.com", policy: "two_factor", resources: ["^/dev/?.*$"] diff --git a/server/src/lib/authorization/Authorizer.ts b/server/src/lib/authorization/Authorizer.ts index e235a3914..3cb640d3e 100644 --- a/server/src/lib/authorization/Authorizer.ts +++ b/server/src/lib/authorization/Authorizer.ts @@ -24,6 +24,24 @@ function MatchResource(actualResource: string) { }; } +function MatchSubject(user: string, groups: string[]) { + return (rule: ACLRule) => { + // If no subject, matches anybody + if (!rule.subject) return true; + + if (rule.subject.startsWith("user:")) { + const ruleUser = rule.subject.split(":")[1]; + if (user == ruleUser) return true; + } + + if (rule.subject.startsWith("group:")) { + const ruleGroup = rule.subject.split(":")[1]; + if (groups.indexOf(ruleGroup) > -1) return true; + } + return false; + }; +} + export class Authorizer implements IAuthorizer { private logger: Winston; private readonly configuration: ACLConfiguration; @@ -33,39 +51,16 @@ export class Authorizer implements IAuthorizer { this.configuration = configuration; } - private getMatchingUserRules(user: string, domain: string, resource: string): ACLRule[] { - const userRules = this.configuration.users[user]; - if (!userRules) return []; - return userRules.filter(MatchDomain(domain)).filter(MatchResource(resource)); - } - - private getMatchingGroupRules(groups: string[], domain: string, resource: string): ACLRule[] { - const that = this; - // There is no ordering between group rules. That is, when a user belongs to 2 groups, there is no - // guarantee one set of rules has precedence on the other one. - const groupRules = groups.reduce(function (rules: ACLRule[], group: string) { - const groupRules = that.configuration.groups[group]; - if (groupRules) rules = rules.concat(groupRules); - return rules; - }, []); - return groupRules.filter(MatchDomain(domain)).filter(MatchResource(resource)); - } - - private getMatchingAllRules(domain: string, resource: string): ACLRule[] { - const rules = this.configuration.any; + private getMatchingRules(domain: string, resource: string, user: string, groups: string[]): ACLRule[] { + const rules = this.configuration.rules; if (!rules) return []; - return rules.filter(MatchDomain(domain)).filter(MatchResource(resource)); + return rules + .filter(MatchDomain(domain)) + .filter(MatchResource(resource)) + .filter(MatchSubject(user, groups)); } - authorization(domain: string, resource: string, user: string, groups: string[]): Level { - if (!this.configuration) return Level.BYPASS; - - const allRules = this.getMatchingAllRules(domain, resource); - const groupRules = this.getMatchingGroupRules(groups, domain, resource); - const userRules = this.getMatchingUserRules(user, domain, resource); - const rules = allRules.concat(groupRules).concat(userRules).reverse(); - const policy = rules.map(r => r.policy).concat([this.configuration.default_policy])[0]; - + private ruleToLevel(policy: string): Level { if (policy == "bypass") { return Level.BYPASS; } else if (policy == "one_factor") { @@ -75,4 +70,14 @@ export class Authorizer implements IAuthorizer { } return Level.DENY; } + + authorization(domain: string, resource: string, user: string, groups: string[]): Level { + if (!this.configuration) return Level.BYPASS; + + const rules = this.getMatchingRules(domain, resource, user, groups); + + return (rules.length > 0) + ? this.ruleToLevel(rules[0].policy) // extract the policy of the first matching rule + : this.ruleToLevel(this.configuration.default_policy); // otherwise use the default policy + } } \ No newline at end of file diff --git a/server/src/lib/configuration/ConfigurationParser.spec.ts b/server/src/lib/configuration/ConfigurationParser.spec.ts index 2baefc8a9..60c0f6182 100644 --- a/server/src/lib/configuration/ConfigurationParser.spec.ts +++ b/server/src/lib/configuration/ConfigurationParser.spec.ts @@ -125,32 +125,26 @@ describe("configuration/ConfigurationParser", function () { const userConfig = buildYamlConfig(); userConfig.access_control = { default_policy: "deny", - any: [{ + rules: [{ + domain: "www.example.com", + policy: "two_factor", + subject: "user:user" + }, { domain: "public.example.com", policy: "two_factor" - }], - users: { - "user": [{ - domain: "www.example.com", - policy: "two_factor" - }] - }, - groups: {} + }] }; const config = ConfigurationParser.parse(userConfig); Assert.deepEqual(config.access_control, { default_policy: "deny", - any: [{ + rules: [{ + domain: "www.example.com", + policy: "two_factor", + subject: "user:user" + }, { domain: "public.example.com", policy: "two_factor" - }], - users: { - "user": [{ - domain: "www.example.com", - policy: "two_factor" - }] - }, - groups: {} + }] } as ACLConfiguration); }); @@ -161,9 +155,7 @@ describe("configuration/ConfigurationParser", function () { const config = ConfigurationParser.parse(userConfig); Assert.deepEqual(config.access_control, { default_policy: "bypass", - any: [], - users: {}, - groups: {} + rules: [] }); }); }); diff --git a/server/src/lib/configuration/SessionConfigurationBuilder.spec.ts b/server/src/lib/configuration/SessionConfigurationBuilder.spec.ts index 0a4c02c7c..d4a3093ee 100644 --- a/server/src/lib/configuration/SessionConfigurationBuilder.spec.ts +++ b/server/src/lib/configuration/SessionConfigurationBuilder.spec.ts @@ -11,9 +11,7 @@ describe("configuration/SessionConfigurationBuilder", function () { const configuration: Configuration = { access_control: { default_policy: "deny", - any: [], - users: {}, - groups: {} + rules: [] }, totp: { issuer: "authelia.com" diff --git a/server/src/lib/configuration/schema/AclConfiguration.spec.ts b/server/src/lib/configuration/schema/AclConfiguration.spec.ts index 6b2f47f9e..d1e2a03a2 100644 --- a/server/src/lib/configuration/schema/AclConfiguration.spec.ts +++ b/server/src/lib/configuration/schema/AclConfiguration.spec.ts @@ -4,11 +4,31 @@ import Assert = require("assert"); describe("configuration/schema/AclConfiguration", function() { it("should complete ACLConfiguration", function() { const configuration: ACLConfiguration = {}; - const newConfiguration = complete(configuration); + const [newConfiguration, errors] = complete(configuration); Assert.deepEqual(newConfiguration.default_policy, "bypass"); - Assert.deepEqual(newConfiguration.any, []); - Assert.deepEqual(newConfiguration.groups, {}); - Assert.deepEqual(newConfiguration.users, {}); + Assert.deepEqual(newConfiguration.rules, []); + }); + + it("should return errors when subject is not good", function() { + const configuration: ACLConfiguration = { + default_policy: "deny", + rules: [{ + domain: "dev.example.com", + subject: "user:abc", + policy: "bypass" + }, { + domain: "dev.example.com", + subject: "user:def", + policy: "bypass" + }, { + domain: "dev.example.com", + subject: "badkey:abc", + policy: "bypass" + }] + }; + const [newConfiguration, errors] = complete(configuration); + + Assert.deepEqual(errors, ["Rule 2 has wrong subject. It should be starting with user: or group:."]); }); }); \ No newline at end of file diff --git a/server/src/lib/configuration/schema/AclConfiguration.ts b/server/src/lib/configuration/schema/AclConfiguration.ts index e29dceb23..40401dd64 100644 --- a/server/src/lib/configuration/schema/AclConfiguration.ts +++ b/server/src/lib/configuration/schema/AclConfiguration.ts @@ -3,22 +3,17 @@ export type ACLPolicy = "deny" | "bypass" | "one_factor" | "two_factor"; export type ACLRule = { domain: string; - policy: ACLPolicy; resources?: string[]; + subject?: string; + policy: ACLPolicy; }; -export type ACLDefaultRules = ACLRule[]; -export type ACLGroupsRules = { [group: string]: ACLRule[]; }; -export type ACLUsersRules = { [user: string]: ACLRule[]; }; - export interface ACLConfiguration { default_policy?: ACLPolicy; - any?: ACLDefaultRules; - groups?: ACLGroupsRules; - users?: ACLUsersRules; + rules?: ACLRule[]; } -export function complete(configuration: ACLConfiguration): ACLConfiguration { +export function complete(configuration: ACLConfiguration): [ACLConfiguration, string[]] { const newConfiguration: ACLConfiguration = (configuration) ? JSON.parse(JSON.stringify(configuration)) : {}; @@ -26,17 +21,21 @@ export function complete(configuration: ACLConfiguration): ACLConfiguration { newConfiguration.default_policy = "bypass"; } - if (!newConfiguration.any) { - newConfiguration.any = []; + if (!newConfiguration.rules) { + newConfiguration.rules = []; } - if (!newConfiguration.groups) { - newConfiguration.groups = {}; + if (newConfiguration.rules.length > 0) { + const errors: string[] = []; + newConfiguration.rules.forEach((r, idx) => { + if (r.subject && !r.subject.match(/^(user|group):[a-zA-Z0-9]+$/)) { + errors.push(`Rule ${idx} has wrong subject. It should be starting with user: or group:.`); + } + }); + if (errors.length > 0) { + return [newConfiguration, errors]; + } } - if (!newConfiguration.users) { - newConfiguration.users = {}; - } - - return newConfiguration; + return [newConfiguration, []]; } \ No newline at end of file diff --git a/server/src/lib/configuration/schema/Configuration.ts b/server/src/lib/configuration/schema/Configuration.ts index 9798bc83e..8d16a5fb2 100644 --- a/server/src/lib/configuration/schema/Configuration.ts +++ b/server/src/lib/configuration/schema/Configuration.ts @@ -27,9 +27,13 @@ export function complete( JSON.stringify(configuration)); const errors: string[] = []; - newConfiguration.access_control = - AclConfigurationComplete( - newConfiguration.access_control); + const [acls, aclsErrors] = AclConfigurationComplete( + newConfiguration.access_control); + + newConfiguration.access_control = acls; + if (aclsErrors.length > 0) { + errors.concat(aclsErrors); + } const [backend, error] = AuthenticationBackendComplete( diff --git a/server/src/lib/configuration/schema/NotifierConfiguration.spec.ts b/server/src/lib/configuration/schema/NotifierConfiguration.spec.ts index 689c42330..6c576e8e0 100644 --- a/server/src/lib/configuration/schema/NotifierConfiguration.spec.ts +++ b/server/src/lib/configuration/schema/NotifierConfiguration.spec.ts @@ -6,12 +6,12 @@ describe("configuration/schema/NotifierConfiguration", function() { const configuration: NotifierConfiguration = {}; const [newConfiguration, error] = complete(configuration); - Assert.deepEqual(newConfiguration.filesystem, {filename: "/tmp/authelia/notification.txt"}) + Assert.deepEqual(newConfiguration.filesystem, {filename: "/tmp/authelia/notification.txt"}); }); it("should ensure correct key is provided", function() { const configuration = { - abc: 'badvalue' + abc: "badvalue" }; const [newConfiguration, error] = complete(configuration as any); diff --git a/server/src/lib/routes/verify/get.spec.ts b/server/src/lib/routes/verify/get.spec.ts index 376fa622c..67cf19fb6 100644 --- a/server/src/lib/routes/verify/get.spec.ts +++ b/server/src/lib/routes/verify/get.spec.ts @@ -246,7 +246,7 @@ describe("routes/verify/get", function () { it("should fail when endpoint is protected by two factors", function () { mocks.authorizer.authorizationMock.returns(AuthorizationLevel.TWO_FACTOR); mocks.config.access_control.default_policy = "one_factor"; - mocks.config.access_control.any = [{ + mocks.config.access_control.rules = [{ domain: "secret.example.com", policy: "two_factor" }]; diff --git a/server/src/lib/storage/mongo/MongoCollection.spec.ts b/server/src/lib/storage/mongo/MongoCollection.spec.ts index 9838c21c7..74a773a1e 100644 --- a/server/src/lib/storage/mongo/MongoCollection.spec.ts +++ b/server/src/lib/storage/mongo/MongoCollection.spec.ts @@ -10,7 +10,7 @@ describe("storage/mongo/MongoCollection", function () { let mongoClientStub: MongoClientStub; let findStub: Sinon.SinonStub; let findOneStub: Sinon.SinonStub; - let insertStub: Sinon.SinonStub; + let insertOneStub: Sinon.SinonStub; let updateStub: Sinon.SinonStub; let removeStub: Sinon.SinonStub; let countStub: Sinon.SinonStub; @@ -21,7 +21,7 @@ describe("storage/mongo/MongoCollection", function () { mongoCollectionStub = Sinon.createStubInstance(require("mongodb").Collection as any); findStub = mongoCollectionStub.find as Sinon.SinonStub; findOneStub = mongoCollectionStub.findOne as Sinon.SinonStub; - insertStub = mongoCollectionStub.insert as Sinon.SinonStub; + insertOneStub = mongoCollectionStub.insertOne as Sinon.SinonStub; updateStub = mongoCollectionStub.update as Sinon.SinonStub; removeStub = mongoCollectionStub.remove as Sinon.SinonStub; countStub = mongoCollectionStub.count as Sinon.SinonStub; @@ -63,11 +63,11 @@ describe("storage/mongo/MongoCollection", function () { describe("insert", function () { it("should insert a document in the collection", function () { const collection = new MongoCollection(COLLECTION_NAME, mongoClientStub); - insertStub.returns(BluebirdPromise.resolve({})); + insertOneStub.returns(BluebirdPromise.resolve({})); return collection.insert({ key: "KEY" }) .then(function () { - Assert(insertStub.calledWith({ key: "KEY" })); + Assert(insertOneStub.calledWith({ key: "KEY" })); }); }); }); diff --git a/server/src/lib/storage/mongo/MongoCollection.ts b/server/src/lib/storage/mongo/MongoCollection.ts index f23f75ba2..9771389f1 100644 --- a/server/src/lib/storage/mongo/MongoCollection.ts +++ b/server/src/lib/storage/mongo/MongoCollection.ts @@ -40,7 +40,7 @@ export class MongoCollection implements ICollection { insert(document: any): Bluebird { return this.collection() - .then((collection) => collection.insert(document)); + .then((collection) => collection.insertOne(document)); } count(query: any): Bluebird { diff --git a/test/features/access-control.feature b/test/features/access-control.feature index 5539b5596..0e513ea1f 100644 --- a/test/features/access-control.feature +++ b/test/features/access-control.feature @@ -33,7 +33,7 @@ Feature: User has access restricted access to domains And I have access to "https://dev.example.com:8080/users/bob/secret.html" And I have no access to "https://admin.example.com:8080/secret.html" And I have access to "https://mx1.mail.example.com:8080/secret.html" - And I have no access to "https://single_factor.example.com:8080/secret.html" + And I have access to "https://single_factor.example.com:8080/secret.html" And I have access to "https://mx2.mail.example.com:8080/secret.html" @need-registered-user-harry @@ -51,5 +51,5 @@ Feature: User has access restricted access to domains And I have no access to "https://dev.example.com:8080/users/bob/secret.html" And I have no access to "https://admin.example.com:8080/secret.html" And I have no access to "https://mx1.mail.example.com:8080/secret.html" - And I have no access to "https://single_factor.example.com:8080/secret.html" + And I have access to "https://single_factor.example.com:8080/secret.html" And I have no access to "https://mx2.mail.example.com:8080/secret.html" diff --git a/test/features/restrictions.feature b/test/features/restrictions.feature index 2e33371d1..97c85a342 100644 --- a/test/features/restrictions.feature +++ b/test/features/restrictions.feature @@ -14,24 +14,3 @@ Feature: Non authenticated users have no access to certain pages | https://login.example.com:8080/api/u2f/sign | 401 | POST | | https://login.example.com:8080/api/u2f/register_request | 401 | GET | | https://login.example.com:8080/api/u2f/register | 401 | POST | - - - @needs-single_factor-config - @need-registered-user-john - Scenario: User does not have acces to second factor related endpoints when in single factor mode - Given I post "https://login.example.com:8080/api/firstfactor" with body: - | key | value | - | username | john | - | password | password | - Then I get the following status code when requesting: - | url | code | method | - | https://login.example.com:8080/secondfactor | 401 | GET | - | https://login.example.com:8080/secondfactor/u2f/identity/start | 401 | GET | - | https://login.example.com:8080/secondfactor/u2f/identity/finish | 401 | GET | - | https://login.example.com:8080/secondfactor/totp/identity/start | 401 | GET | - | https://login.example.com:8080/secondfactor/totp/identity/finish | 401 | GET | - | https://login.example.com:8080/api/totp | 401 | POST | - | https://login.example.com:8080/api/u2f/sign_request | 401 | GET | - | https://login.example.com:8080/api/u2f/sign | 401 | POST | - | https://login.example.com:8080/api/u2f/register_request | 401 | GET | - | https://login.example.com:8080/api/u2f/register | 401 | POST | \ No newline at end of file diff --git a/test/features/single-factor-domain.feature b/test/features/single-factor-domain.feature index db13bb94d..9fee7be9a 100644 --- a/test/features/single-factor-domain.feature +++ b/test/features/single-factor-domain.feature @@ -13,3 +13,4 @@ Feature: User can access certain subdomains with single factor Scenario: User can login using basic authentication When I request "https://single_factor.example.com:8080/secret.html" with username "john" and password "password" using basic authentication Then I receive the secret page + diff --git a/test/features/single-factor-only-server.feature b/test/features/single-factor-only-server.feature deleted file mode 100644 index 4d3fc42f9..000000000 --- a/test/features/single-factor-only-server.feature +++ /dev/null @@ -1,16 +0,0 @@ -@needs-single_factor-config -Feature: Server is configured as a single factor only server - - @need-registered-user-john - Scenario: User is redirected to service after first factor if allowed - When I visit "https://login.example.com:8080/?rd=https://public.example.com:8080/secret.html" - And I login with user "john" and password "password" - Then I'm redirected to "https://public.example.com:8080/secret.html" - - @need-registered-user-john - Scenario: User is correctly redirected according to default redirection URL - When I visit "https://login.example.com:8080" - And I login with user "john" and password "password" - Then I'm redirected to "https://login.example.com:8080/loggedin" - And I sleep for 5 seconds - Then I'm redirected to "https://home.example.com:8080/" diff --git a/test/features/step_definitions/restrictions.ts b/test/features/step_definitions/restrictions.ts index cf7eb0c18..3ab37390b 100644 --- a/test/features/step_definitions/restrictions.ts +++ b/test/features/step_definitions/restrictions.ts @@ -33,7 +33,6 @@ function requestAndExpectStatusCode(ctx: any, url: string, method: string, Assert.equal(statusCode, expectedStatusCode); } catch (e) { - console.log(url); console.log("%s (actual) != %s (expected)", statusCode, expectedStatusCode); throw e;