From 193aa6faf4c9f881d58df7baa4447958bf61c4ac Mon Sep 17 00:00:00 2001 From: Jonathan Lorimer Date: Fri, 11 Nov 2022 11:41:57 -0500 Subject: [PATCH 1/3] Add declarative role config to postgres.service remove trailing whitespace switch docs to markdown use mdDoc remove trailing whitespace get rid of double space add tests and update options to use submodule remove whitespace remove whitespace use mdDoc remove whitespace make default a no-op make ALTER ROLE a single sql statement document null case --- .../modules/services/databases/postgresql.nix | 178 +++++++++++++++++- nixos/tests/postgresql.nix | 91 ++++++++- 2 files changed, 261 insertions(+), 8 deletions(-) diff --git a/nixos/modules/services/databases/postgresql.nix b/nixos/modules/services/databases/postgresql.nix index 2919022496a3..910c149b8f59 100644 --- a/nixos/modules/services/databases/postgresql.nix +++ b/nixos/modules/services/databases/postgresql.nix @@ -147,6 +147,7 @@ in Name of the user to ensure. ''; }; + ensurePermissions = mkOption { type = types.attrsOf types.str; default = {}; @@ -168,6 +169,154 @@ in } ''; }; + + ensureClauses = mkOption { + description = lib.mdDoc '' + An attrset of clauses to grant to the user. Under the hood this uses the + [ALTER USER syntax](https://www.postgresql.org/docs/current/sql-alteruser.html) for each attrName where + the attrValue is true in the attrSet: + `ALTER USER user.name WITH attrName` + ''; + example = literalExpression '' + { + superuser = true; + createrole = true; + createdb = true; + } + ''; + default = {}; + defaultText = lib.literalMD '' + If not set then the user created will have the default permissions assigned by postgres + ''; + type = types.submodule { + options = let + defaultText = lib.literalMD '' + `null`: do not set. For newly created roles, use PostgreSQL's default. For existing roles, do not touch this clause. + ''; + in { + superuser = mkOption { + type = types.nullOr types.bool; + description = lib.mdDoc '' + Grants the user, created by the ensureUser attr, superuser permissions. From the postgres docs: + + A database superuser bypasses all permission checks, + except the right to log in. This is a dangerous privilege + and should not be used carelessly; it is best to do most + of your work as a role that is not a superuser. To create + a new database superuser, use CREATE ROLE name SUPERUSER. + You must do this as a role that is already a superuser. + + More information on postgres roles can be found [here](https://www.postgresql.org/docs/current/role-attributes.html) + ''; + default = null; + inherit defaultText; + }; + createrole = mkOption { + type = types.nullOr types.bool; + description = lib.mdDoc '' + Grants the user, created by the ensureUser attr, createrole permissions. From the postgres docs: + + A role must be explicitly given permission to create more + roles (except for superusers, since those bypass all + permission checks). To create such a role, use CREATE + ROLE name CREATEROLE. A role with CREATEROLE privilege + can alter and drop other roles, too, as well as grant or + revoke membership in them. However, to create, alter, + drop, or change membership of a superuser role, superuser + status is required; CREATEROLE is insufficient for that. + + More information on postgres roles can be found [here](https://www.postgresql.org/docs/current/role-attributes.html) + ''; + default = null; + inherit defaultText; + }; + createdb = mkOption { + type = types.nullOr types.bool; + description = lib.mdDoc '' + Grants the user, created by the ensureUser attr, createdb permissions. From the postgres docs: + + A role must be explicitly given permission to create + databases (except for superusers, since those bypass all + permission checks). To create such a role, use CREATE + ROLE name CREATEDB. + + More information on postgres roles can be found [here](https://www.postgresql.org/docs/current/role-attributes.html) + ''; + default = null; + inherit defaultText; + }; + "inherit" = mkOption { + type = types.nullOr types.bool; + description = lib.mdDoc '' + Grants the user created inherit permissions. From the postgres docs: + + A role is given permission to inherit the privileges of + roles it is a member of, by default. However, to create a + role without the permission, use CREATE ROLE name + NOINHERIT. + + More information on postgres roles can be found [here](https://www.postgresql.org/docs/current/role-attributes.html) + ''; + default = null; + inherit defaultText; + }; + login = mkOption { + type = types.nullOr types.bool; + description = lib.mdDoc '' + Grants the user, created by the ensureUser attr, login permissions. From the postgres docs: + + Only roles that have the LOGIN attribute can be used as + the initial role name for a database connection. A role + with the LOGIN attribute can be considered the same as a + “database user”. To create a role with login privilege, + use either: + + CREATE ROLE name LOGIN; CREATE USER name; + + (CREATE USER is equivalent to CREATE ROLE except that + CREATE USER includes LOGIN by default, while CREATE ROLE + does not.) + + More information on postgres roles can be found [here](https://www.postgresql.org/docs/current/role-attributes.html) + ''; + default = null; + inherit defaultText; + }; + replication = mkOption { + type = types.nullOr types.bool; + description = lib.mdDoc '' + Grants the user, created by the ensureUser attr, replication permissions. From the postgres docs: + + A role must explicitly be given permission to initiate + streaming replication (except for superusers, since those + bypass all permission checks). A role used for streaming + replication must have LOGIN permission as well. To create + such a role, use CREATE ROLE name REPLICATION LOGIN. + + More information on postgres roles can be found [here](https://www.postgresql.org/docs/current/role-attributes.html) + ''; + default = null; + inherit defaultText; + }; + bypassrls = mkOption { + type = types.nullOr types.bool; + description = lib.mdDoc '' + Grants the user, created by the ensureUser attr, replication permissions. From the postgres docs: + + A role must be explicitly given permission to bypass + every row-level security (RLS) policy (except for + superusers, since those bypass all permission checks). To + create such a role, use CREATE ROLE name BYPASSRLS as a + superuser. + + More information on postgres roles can be found [here](https://www.postgresql.org/docs/current/role-attributes.html) + ''; + default = null; + inherit defaultText; + }; + }; + }; + }; }; }); default = []; @@ -380,12 +529,29 @@ in $PSQL -tAc "SELECT 1 FROM pg_database WHERE datname = '${database}'" | grep -q 1 || $PSQL -tAc 'CREATE DATABASE "${database}"' '') cfg.ensureDatabases} '' + '' - ${concatMapStrings (user: '' - $PSQL -tAc "SELECT 1 FROM pg_roles WHERE rolname='${user.name}'" | grep -q 1 || $PSQL -tAc 'CREATE USER "${user.name}"' - ${concatStringsSep "\n" (mapAttrsToList (database: permission: '' - $PSQL -tAc 'GRANT ${permission} ON ${database} TO "${user.name}"' - '') user.ensurePermissions)} - '') cfg.ensureUsers} + ${ + concatMapStrings + (user: + let + userPermissions = concatStringsSep "\n" + (mapAttrsToList + (database: permission: ''$PSQL -tAc 'GRANT ${permission} ON ${database} TO "${user.name}"' '') + user.ensurePermissions + ); + + filteredClauses = filterAttrs (name: value: value != null) user.ensureClauses; + + clauseSqlStatements = attrValues (mapAttrs (n: v: if v then n else "no${n}") filteredClauses); + + userClauses = ''$PSQL -tAc 'ALTER ROLE "${user.name}" ${concatStringsSep " " clauseSqlStatements}' ''; + in '' + $PSQL -tAc "SELECT 1 FROM pg_roles WHERE rolname='${user.name}'" | grep -q 1 || $PSQL -tAc 'CREATE USER "${user.name}"' + ${userPermissions} + ${userClauses} + '' + ) + cfg.ensureUsers + } ''; serviceConfig = mkMerge [ diff --git a/nixos/tests/postgresql.nix b/nixos/tests/postgresql.nix index 2b487c20a625..6a58c88b53cf 100644 --- a/nixos/tests/postgresql.nix +++ b/nixos/tests/postgresql.nix @@ -130,8 +130,95 @@ let ''; }; + + mk-ensure-clauses-test = postgresql-name: postgresql-package: makeTest { + name = postgresql-name; + meta = with pkgs.lib.maintainers; { + maintainers = [ zagy ]; + }; + + machine = {...}: + { + services.postgresql = { + enable = true; + package = postgresql-package; + ensureUsers = [ + { + name = "all-clauses"; + ensureClauses = { + superuser = true; + createdb = true; + createrole = true; + "inherit" = true; + login = true; + replication = true; + bypassrls = true; + }; + } + { + name = "default-clauses"; + } + ]; + }; + }; + + testScript = let + getClausesQuery = user: pkgs.lib.concatStringsSep " " + [ + "SELECT row_to_json(row)" + "FROM (" + "SELECT" + "rolsuper," + "rolinherit," + "rolcreaterole," + "rolcreatedb," + "rolcanlogin," + "rolreplication," + "rolbypassrls" + "FROM pg_roles" + "WHERE rolname = '${user}'" + ") row;" + ]; + in '' + import json + machine.start() + machine.wait_for_unit("postgresql") + + with subtest("All user permissions are set according to the ensureClauses attr"): + clauses = json.loads( + machine.succeed( + "sudo -u postgres psql -tc \"${getClausesQuery "all-clauses"}\"" + ) + ) + print(clauses) + assert clauses['rolsuper'], 'expected user with clauses to have superuser clause' + assert clauses['rolinherit'], 'expected user with clauses to have inherit clause' + assert clauses['rolcreaterole'], 'expected user with clauses to have create role clause' + assert clauses['rolcreatedb'], 'expected user with clauses to have create db clause' + assert clauses['rolcanlogin'], 'expected user with clauses to have login clause' + assert clauses['rolreplication'], 'expected user with clauses to have replication clause' + assert clauses['rolbypassrls'], 'expected user with clauses to have bypassrls clause' + + with subtest("All user permissions default when ensureClauses is not provided"): + clauses = json.loads( + machine.succeed( + "sudo -u postgres psql -tc \"${getClausesQuery "default-clauses"}\"" + ) + ) + assert not clauses['rolsuper'], 'expected user with no clauses set to have default superuser clause' + assert clauses['rolinherit'], 'expected user with no clauses set to have default inherit clause' + assert not clauses['rolcreaterole'], 'expected user with no clauses set to have default create role clause' + assert not clauses['rolcreatedb'], 'expected user with no clauses set to have default create db clause' + assert clauses['rolcanlogin'], 'expected user with no clauses set to have default login clause' + assert not clauses['rolreplication'], 'expected user with no clauses set to have default replication clause' + assert not clauses['rolbypassrls'], 'expected user with no clauses set to have default bypassrls clause' + + machine.shutdown() + ''; + }; in - (mapAttrs' (name: package: { inherit name; value=make-postgresql-test name package false;}) postgresql-versions) // { + (mapAttrs' (name: package: { inherit name; value=make-postgresql-test name package false;}) postgresql-versions) // + (mapAttrs' (name: package: { inherit name; value=mk-ensure-clauses-test name package;}) postgresql-versions) // + { postgresql_11-backup-all = make-postgresql-test "postgresql_11-backup-all" postgresql-versions.postgresql_11 true; } - From 4e502a1c4d207610b0964b92bb41e574ed62e9c9 Mon Sep 17 00:00:00 2001 From: Jonathan Lorimer <32466011+JonathanLorimer@users.noreply.github.com> Date: Mon, 28 Nov 2022 10:21:18 -0500 Subject: [PATCH 2/3] Update nixos/modules/services/databases/postgresql.nix Co-authored-by: Robert Hensing --- nixos/modules/services/databases/postgresql.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nixos/modules/services/databases/postgresql.nix b/nixos/modules/services/databases/postgresql.nix index 910c149b8f59..154967840533 100644 --- a/nixos/modules/services/databases/postgresql.nix +++ b/nixos/modules/services/databases/postgresql.nix @@ -186,7 +186,7 @@ in ''; default = {}; defaultText = lib.literalMD '' - If not set then the user created will have the default permissions assigned by postgres + The default, `null`, means that the user created will have the default permissions assigned by PostgreSQL. Subsequent server starts will not set or unset the clause, so imperative changes are preserved. ''; type = types.submodule { options = let From 146620355f7861545722b1e6b112f4d1acc6b249 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Fri, 2 Dec 2022 18:15:01 +0000 Subject: [PATCH 3/3] nixosTests.postgresql: Fix attribute name shadowing --- nixos/tests/postgresql.nix | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/nixos/tests/postgresql.nix b/nixos/tests/postgresql.nix index 6b57e69acde9..7e0a82c38828 100644 --- a/nixos/tests/postgresql.nix +++ b/nixos/tests/postgresql.nix @@ -217,8 +217,10 @@ let ''; }; in - (mapAttrs' (name: package: { inherit name; value=make-postgresql-test name package false;}) postgresql-versions) // - (mapAttrs' (name: package: { inherit name; value=mk-ensure-clauses-test name package;}) postgresql-versions) // - { + concatMapAttrs (name: package: { + ${name} = make-postgresql-test name package false; + ${name + "-clauses"} = mk-ensure-clauses-test name package; + }) postgresql-versions + // { postgresql_11-backup-all = make-postgresql-test "postgresql_11-backup-all" postgresql-versions.postgresql_11 true; }