From 460d932aa8e8fbdc0c72057e2e43f6e98d1d590c Mon Sep 17 00:00:00 2001 From: Pilou Date: Sun, 11 Jun 2017 23:48:39 +0200 Subject: [PATCH] postgresql_user: fix bugs related to 'expires' option (#23862) * Factorize tests related to no_password_change using an include task * Refactor: deduplicate tasks * postgresql_user: test 'expires' parameter * Change 'valid until' even it's the only updated field * value is changed when another value is provided * value isn't returned when unset * Remove unused variable * psycopg2.extras.DictRow is able to handle comparison * postgresql_user: simplify helper method * postgresql_user: define variable just before using it * Fix comparison between user input and applied configuration * new test: adding an invalid attribute * Refactor, add cleaning task * Check that using same attribute a 2nd time does nothing * Always try to remove created user * postgresql_user: fix pep8 --- .../database/postgresql/postgresql_user.py | 111 +++++++----- .../targets/postgresql/tasks/main.yml | 159 ++--------------- .../tasks/postgresql_user_9.5_or_greater.yml | 90 ---------- .../tasks/postgresql_user_less_than_9.5.yml | 87 --------- .../tasks/test_no_password_change.yml | 167 ++++++++++++++++++ .../{test_user.yml => test_password.yml} | 57 +++++- test/sanity/pep8/legacy-files.txt | 1 - 7 files changed, 304 insertions(+), 368 deletions(-) delete mode 100644 test/integration/targets/postgresql/tasks/postgresql_user_9.5_or_greater.yml delete mode 100644 test/integration/targets/postgresql/tasks/postgresql_user_less_than_9.5.yml create mode 100644 test/integration/targets/postgresql/tasks/test_no_password_change.yml rename test/integration/targets/postgresql/tasks/{test_user.yml => test_password.yml} (79%) diff --git a/lib/ansible/modules/database/postgresql/postgresql_user.py b/lib/ansible/modules/database/postgresql/postgresql_user.py index 976998c4f43..d78d42632b6 100644 --- a/lib/ansible/modules/database/postgresql/postgresql_user.py +++ b/lib/ansible/modules/database/postgresql/postgresql_user.py @@ -207,11 +207,10 @@ EXAMPLES = ''' password: NULL ''' -from hashlib import md5 import itertools import re - from distutils.version import StrictVersion +from hashlib import md5 try: import psycopg2 @@ -220,15 +219,15 @@ except ImportError: postgresqldb_found = False else: postgresqldb_found = True + from ansible.module_utils._text import to_bytes from ansible.module_utils.six import iteritems -_flags = ('SUPERUSER', 'CREATEROLE', 'CREATEUSER', 'CREATEDB', 'INHERIT', 'LOGIN', 'REPLICATION') -_flags_by_version = {'BYPASSRLS': '9.5.0'} +FLAGS = ('SUPERUSER', 'CREATEROLE', 'CREATEUSER', 'CREATEDB', 'INHERIT', 'LOGIN', 'REPLICATION') +FLAGS_BY_VERSION = {'BYPASSRLS': '9.5.0'} VALID_PRIVS = dict(table=frozenset(('SELECT', 'INSERT', 'UPDATE', 'DELETE', 'TRUNCATE', 'REFERENCES', 'TRIGGER', 'ALL')), - database=frozenset(('CREATE', 'CONNECT', 'TEMPORARY', 'TEMP', 'ALL')), - ) + database=frozenset(('CREATE', 'CONNECT', 'TEMPORARY', 'TEMP', 'ALL')),) # map to cope with idiosyncracies of SUPERUSER and LOGIN PRIV_TO_AUTHID_COLUMN = dict(SUPERUSER='rolsuper', CREATEROLE='rolcreaterole', @@ -236,9 +235,11 @@ PRIV_TO_AUTHID_COLUMN = dict(SUPERUSER='rolsuper', CREATEROLE='rolcreaterole', INHERIT='rolinherit', LOGIN='rolcanlogin', REPLICATION='rolreplication', BYPASSRLS='rolbypassrls') + class InvalidFlagsError(Exception): pass + class InvalidPrivsError(Exception): pass @@ -260,9 +261,9 @@ def user_add(cursor, user, password, role_attr_flags, encrypted, expires): """Create a new database user (role).""" # Note: role_attr_flags escaped by parse_role_attrs and encrypted is a literal query_password_data = dict(password=password, expires=expires) - query = ['CREATE USER %(user)s' % { "user": pg_quote_identifier(user, 'role')}] + query = ['CREATE USER %(user)s' % {"user": pg_quote_identifier(user, 'role')}] if password is not None: - query.append("WITH %(crypt)s" % { "crypt": encrypted }) + query.append("WITH %(crypt)s" % {"crypt": encrypted}) query.append("PASSWORD %(password)s") if expires is not None: query.append("VALID UNTIL %(expires)s") @@ -271,6 +272,7 @@ def user_add(cursor, user, password, role_attr_flags, encrypted, expires): cursor.execute(query, query_password_data) return True + def user_alter(cursor, module, user, password, role_attr_flags, encrypted, expires, no_password_changes): """Change user password and/or attributes. Return True if changed, False otherwise.""" changed = False @@ -285,9 +287,8 @@ def user_alter(cursor, module, user, password, role_attr_flags, encrypted, expir return False # Handle passwords. - if not no_password_changes and (password is not None or role_attr_flags != ''): + if not no_password_changes and (password is not None or role_attr_flags != '' or expires is not None): # Select password and all flag-like columns in order to verify changes. - query_password_data = dict(password=password, expires=expires) select = "SELECT * FROM pg_authid where rolname=%(user)s" cursor.execute(select, {"user": user}) # Grab current role attributes. @@ -300,7 +301,7 @@ def user_alter(cursor, module, user, password, role_attr_flags, encrypted, expir # 3: The size of the 'md5' prefix # When the provided password looks like a MD5-hash, value of # 'encrypted' is ignored. - if ((password.startswith('md5') and len(password) == 32+3) or encrypted == 'UNENCRYPTED'): + if ((password.startswith('md5') and len(password) == 32 + 3) or encrypted == 'UNENCRYPTED'): if password != current_role_attrs['rolpassword']: pwchanging = True elif encrypted == 'ENCRYPTED': @@ -321,7 +322,12 @@ def user_alter(cursor, module, user, password, role_attr_flags, encrypted, expir if current_role_attrs[PRIV_TO_AUTHID_COLUMN[role_attr_name]] != role_attr_value: role_attr_flags_changing = True - expires_changing = (expires is not None and expires == current_role_attrs['rolvaliduntil']) + if expires is not None: + cursor.execute("SELECT %s::timestamptz;", (expires,)) + expires_with_tz = cursor.fetchone()[0] + expires_changing = expires_with_tz != current_role_attrs.get('rolvaliduntil') + else: + expires_changing = False if not pwchanging and not role_attr_flags_changing and not expires_changing: return False @@ -336,6 +342,7 @@ def user_alter(cursor, module, user, password, role_attr_flags, encrypted, expir if expires is not None: alter.append("VALID UNTIL %(expires)s") + query_password_data = dict(password=password, expires=expires) try: cursor.execute(' '.join(alter), query_password_data) changed = True @@ -396,12 +403,11 @@ def user_alter(cursor, module, user, password, role_attr_flags, encrypted, expir new_role_attrs = cursor.fetchone() # Detect any differences between current_ and new_role_attrs. - for i in range(len(current_role_attrs)): - if current_role_attrs[i] != new_role_attrs[i]: - changed = True + changed = current_role_attrs != new_role_attrs return changed + def user_delete(cursor, user): """Try to remove a user. Returns True if successful otherwise False""" cursor.execute("SAVEPOINT ansible_pgsql_user_delete") @@ -415,6 +421,7 @@ def user_delete(cursor, user): cursor.execute("RELEASE SAVEPOINT ansible_pgsql_user_delete") return True + def has_table_privileges(cursor, user, table, privs): """ Return the difference between the privileges that a user already has and @@ -431,6 +438,7 @@ def has_table_privileges(cursor, user, table, privs): desired = privs.difference(cur_privs) return (have_currently, other_current, desired) + def get_table_privileges(cursor, user, table): if '.' in table: schema, table = table.split('.', 1) @@ -441,25 +449,28 @@ def get_table_privileges(cursor, user, table): cursor.execute(query, (user, table, schema)) return frozenset([x[0] for x in cursor.fetchall()]) + def grant_table_privileges(cursor, user, table, privs): # Note: priv escaped by parse_privs privs = ', '.join(privs) query = 'GRANT %s ON TABLE %s TO %s' % ( - privs, pg_quote_identifier(table, 'table'), pg_quote_identifier(user, 'role') ) + privs, pg_quote_identifier(table, 'table'), pg_quote_identifier(user, 'role')) cursor.execute(query) + def revoke_table_privileges(cursor, user, table, privs): # Note: priv escaped by parse_privs privs = ', '.join(privs) query = 'REVOKE %s ON TABLE %s FROM %s' % ( - privs, pg_quote_identifier(table, 'table'), pg_quote_identifier(user, 'role') ) + privs, pg_quote_identifier(table, 'table'), pg_quote_identifier(user, 'role')) cursor.execute(query) + def get_database_privileges(cursor, user, db): priv_map = { - 'C':'CREATE', - 'T':'TEMPORARY', - 'c':'CONNECT', + 'C': 'CREATE', + 'T': 'TEMPORARY', + 'c': 'CONNECT', } query = 'SELECT datacl FROM pg_database WHERE datname = %s' cursor.execute(query, (db,)) @@ -474,6 +485,7 @@ def get_database_privileges(cursor, user, db): o.add(priv_map[v]) return normalize_privileges(o, 'database') + def has_database_privileges(cursor, user, db, privs): """ Return the difference between the privileges that a user already has and @@ -490,9 +502,10 @@ def has_database_privileges(cursor, user, db, privs): desired = privs.difference(cur_privs) return (have_currently, other_current, desired) + def grant_database_privileges(cursor, user, db, privs): # Note: priv escaped by parse_privs - privs =', '.join(privs) + privs = ', '.join(privs) if user == "PUBLIC": query = 'GRANT %s ON DATABASE %s TO PUBLIC' % ( privs, pg_quote_identifier(db, 'database')) @@ -502,6 +515,7 @@ def grant_database_privileges(cursor, user, db, privs): pg_quote_identifier(user, 'role')) cursor.execute(query) + def revoke_database_privileges(cursor, user, db, privs): # Note: priv escaped by parse_privs privs = ', '.join(privs) @@ -514,6 +528,7 @@ def revoke_database_privileges(cursor, user, db, privs): pg_quote_identifier(user, 'role')) cursor.execute(query) + def revoke_privileges(cursor, user, privs): if privs is None: return False @@ -532,6 +547,7 @@ def revoke_privileges(cursor, user, privs): changed = True return changed + def grant_privileges(cursor, user, privs): if privs is None: return False @@ -550,6 +566,7 @@ def grant_privileges(cursor, user, privs): changed = True return changed + def parse_role_attrs(cursor, role_attr_flags): """ Parse role attributes string for user creation. @@ -567,20 +584,17 @@ def parse_role_attrs(cursor, role_attr_flags): Note: "[NO]BYPASSRLS" role attribute introduced in 9.5 """ - flags = frozenset(itertools.chain(_flags, get_valid_flags_by_version(cursor))) - valid_flags = frozenset(itertools.chain(flags, ('NO%s' % f for f in flags))) + flags = frozenset(role.upper() for role in role_attr_flags.split(',') if role) - if ',' in role_attr_flags: - flag_set = frozenset(r.upper() for r in role_attr_flags.split(",")) - elif role_attr_flags: - flag_set = frozenset((role_attr_flags.upper(),)) - else: - flag_set = frozenset() - if not flag_set.issubset(valid_flags): + valid_flags = frozenset(itertools.chain(FLAGS, get_valid_flags_by_version(cursor))) + valid_flags = frozenset(itertools.chain(valid_flags, ('NO%s' % flag for flag in valid_flags))) + + if not flags.issubset(valid_flags): raise InvalidFlagsError('Invalid role_attr_flags specified: %s' % - ' '.join(flag_set.difference(valid_flags))) - o_flags = ' '.join(flag_set) - return o_flags + ' '.join(flags.difference(valid_flags))) + + return ' '.join(flags) + def normalize_privileges(privs, type_): new_privs = set(privs) @@ -593,6 +607,7 @@ def normalize_privileges(privs, type_): return new_privs + def parse_privs(privs, db): """ Parse privilege string to determine permissions for database db. @@ -609,8 +624,8 @@ def parse_privs(privs, db): return privs o_privs = { - 'database':{}, - 'table':{} + 'database': {}, + 'table': {} } for token in privs.split('/'): if ':' not in token: @@ -624,13 +639,14 @@ def parse_privs(privs, db): if not priv_set.issubset(VALID_PRIVS[type_]): raise InvalidPrivsError('Invalid privs specified for %s: %s' % - (type_, ' '.join(priv_set.difference(VALID_PRIVS[type_])))) + (type_, ' '.join(priv_set.difference(VALID_PRIVS[type_])))) priv_set = normalize_privileges(priv_set, type_) o_privs[type_][name] = priv_set return o_privs + def get_pg_server_version(cursor): """ Queries Postgres for its server version. @@ -646,6 +662,7 @@ def get_pg_server_version(cursor): cursor.execute("SHOW SERVER_VERSION") return cursor.fetchone()['server_version'] + def get_valid_flags_by_version(cursor): """ Some role attributes were introduced after certain versions. We want to @@ -655,7 +672,7 @@ def get_valid_flags_by_version(cursor): return [ flag - for flag, version_introduced in _flags_by_version.items() + for flag, version_introduced in FLAGS_BY_VERSION.items() if current_version >= StrictVersion(version_introduced) ] @@ -685,7 +702,7 @@ def main(): ssl_mode=dict(default='prefer', choices=['disable', 'allow', 'prefer', 'require', 'verify-ca', 'verify-full']), ssl_rootcert=dict(default=None) ), - supports_check_mode = True + supports_check_mode=True ) user = module.params["user"] @@ -696,7 +713,6 @@ def main(): if db == '' and module.params["priv"] is not None: module.fail_json(msg="privileges require a database to be specified") privs = parse_privs(module.params["priv"], db) - port = module.params["port"] no_password_changes = module.params["no_password_changes"] if module.params["encrypted"]: encrypted = "ENCRYPTED" @@ -712,15 +728,15 @@ def main(): # check which values are empty and don't include in the **kw # dictionary params_map = { - "login_host":"host", - "login_user":"user", - "login_password":"password", - "port":"port", - "db":"database", - "ssl_mode":"sslmode", - "ssl_rootcert":"sslrootcert" + "login_host": "host", + "login_user": "user", + "login_password": "password", + "port": "port", + "db": "database", + "ssl_mode": "sslmode", + "ssl_rootcert": "sslrootcert" } - kw = dict( (params_map[k], v) for (k, v) in iteritems(module.params) + kw = dict((params_map[k], v) for (k, v) in iteritems(module.params) if k in params_map and v != "" and v is not None) # If a login_unix_socket is specified, incorporate it here. @@ -800,6 +816,7 @@ def main(): kw['changed'] = changed module.exit_json(**kw) + # import module snippets from ansible.module_utils.basic import * from ansible.module_utils.database import * diff --git a/test/integration/targets/postgresql/tasks/main.yml b/test/integration/targets/postgresql/tasks/main.yml index dde8947b6e5..cbc4e275e30 100644 --- a/test/integration/targets/postgresql/tasks/main.yml +++ b/test/integration/targets/postgresql/tasks/main.yml @@ -184,9 +184,9 @@ - "result.stdout_lines[-1] == '(0 rows)'" # -# Create and destroy user +# Create and destroy user, test 'password' and 'encrypted' parameters # -- include: test_user.yml +- include: test_password.yml vars: encrypted: '{{ item.user_creation_encrypted_value }}' db_password1: 'secretù' # use UTF-8 @@ -194,154 +194,31 @@ - user_creation_encrypted_value: 'yes' - user_creation_encrypted_value: 'no' -# BYPASSRLS role attribute was introduced in Postgres 9.5, so +# BYPASSRLS role attribute was introduced in PostgreSQL 9.5, so # we want to test atrribute management differently depending -# on the version. See https://github.com/ansible/ansible/pull/24625 -# for more details. -- name: Get Postgres version +# on the version. +- name: Get PostgreSQL version become_user: "{{ pg_user }}" become: True - shell: echo 'SHOW SERVER_VERSION' | psql -d postgres + shell: "echo 'SHOW SERVER_VERSION' | psql --tuples-only --no-align --dbname postgres" register: postgres_version_resp -- name: Print Postgres server version +- name: Print PostgreSQL server version debug: - msg: "{{ postgres_version_resp.stdout_lines[-2] | trim }}" + msg: "{{ postgres_version_resp.stdout }}" -- name: Role attribute testing for Postgres 9.5+ - include: postgresql_user_9.5_or_greater.yml - when: (postgres_version_resp.stdout_lines[-2] | trim) | version_compare('9.5.0', '>=') +- set_fact: + bypassrls_supported: "{{ postgres_version_resp.stdout | version_compare('9.5.0', '>=') }}" -- name: Role attribute testing for Postgres versions below 9.5 - include: postgresql_user_less_than_9.5.yml - when: (postgres_version_resp.stdout_lines[-2] | trim) | version_compare('9.5.0', '<') +# test 'no_password_change' and 'role_attr_flags' parameters +- include: test_no_password_change.yml + vars: + no_password_changes: '{{ item }}' + with_items: + - 'yes' + - 'no' -- name: Cleanup the user - become_user: "{{ pg_user }}" - become: True - postgresql_user: - name: "{{ db_user1 }}" - state: 'absent' - login_user: "{{ pg_user }}" - db: postgres - -- name: Check that they were removed - become_user: "{{ pg_user }}" - become: True - shell: echo "select * from pg_user where usename='{{ db_user1 }}';" | psql -d postgres - register: result - -- assert: - that: - - "result.stdout_lines[-1] == '(0 rows)'" - -# Test cases to replicate issue 19835 -- name: Create a user "{{ db_user3 }}" to test issue 19835 - become_user: "{{ pg_user }}" - become: True - postgresql_user: - name: "{{ db_user3 }}" - encrypted: 'yes' - password: "md55c8ccfd9d6711fc69a7eae647fc54f51" - login_user: "{{ pg_user }}" - #role_attr_flags: "NOSUPERUSER,NOCREATEROLE,NOCREATEDB,noinherit,NOLOGIN" - db: postgres - register: result - -- name: Check that ansible reports that "{{ db_user3 }}" was created for testing issue 19835 - assert: - that: - - "result.changed == True" - -- name: debug result - debug: - var: result - -- name: Check that "{{ db_user3 }}" was created for testing issue 19835 - become_user: "{{ pg_user }}" - become: True - shell: echo "select * from pg_user where usename='{{ db_user3 }}';" | psql -d postgres - register: result - -- assert: - that: - - "result.stdout_lines[-1] == '(1 row)'" - -- name: Modify user "{{ db_user3 }}" to have only login role attributes for testing issue 19835 - become_user: "{{ pg_user }}" - become: True - postgresql_user: - name: "{{ db_user3 }}" - state: "present" - role_attr_flags: "NOSUPERUSER,NOCREATEROLE,NOCREATEDB,noinherit" - login_user: "{{ pg_user }}" - db: postgres - register: result - -- name: Check that ansible reports it modified the roles for testing issue 19835 - assert: - that: - - "result.changed == True" - -- name: Check that the user "{{ db_user3 }}" has the requested role attributes for testing issue 19835 - become_user: "{{ pg_user }}" - become: True - shell: echo "select 'super:'||rolsuper, 'createrole:'||rolcreaterole, 'create:'||rolcreatedb, 'inherit:'||rolinherit, 'login:'||rolcanlogin from pg_roles where rolname='{{ db_user3 }}';" | psql -d postgres - register: result - -- name: Modify a single role attribute on the user "{{ db_user3 }}" with no_password_changes set to yes. issue 19835 - become_user: "{{ pg_user }}" - become: True - postgresql_user: - name: "{{ db_user3 }}" - state: "present" - role_attr_flags: "CREATEDB" - no_password_changes: yes - login_user: "{{ pg_user }}" - db: postgres - register: result - -- name: Check that ansible reports it modified the role with no_password_changes set to yes. issue 19835 - assert: - that: - - "result.changed == True" - -- name: Check that the user "{{ db_user3 }}" has the requested role attributes with no_password_changes set to yes. issue 19835 - become_user: "{{ pg_user }}" - become: True - shell: echo "select 'super:'||rolsuper, 'createrole:'||rolcreaterole, 'create:'||rolcreatedb, 'inherit:'||rolinherit, 'login:'||rolcanlogin from pg_roles where rolname='{{ db_user3 }}';" | psql -d postgres - register: result - -- name: Assert that the request role attributes check for user "{{ db_user3 }}" was correct with no_password_changes set to yes. issue 19835 - assert: - that: - - "result.stdout_lines[-1] == '(1 row)'" - - "'super:f' in result.stdout_lines[-2]" - - "'createrole:f' in result.stdout_lines[-2]" - - "'create:t' in result.stdout_lines[-2]" - - "'inherit:f' in result.stdout_lines[-2]" - - "'login:t' in result.stdout_lines[-2]" - -- name: Cleanup the "{{ db_user3 }}" user - become_user: "{{ pg_user }}" - become: True - postgresql_user: - name: "{{ db_user3 }}" - state: 'absent' - login_user: "{{ pg_user }}" - db: postgres - -- name: Check that "{{ db_user3 }}" was removed - become_user: "{{ pg_user }}" - become: True - shell: echo "select * from pg_user where usename='{{ db_user3 }}';" | psql -d postgres - register: result - -- assert: - that: - - "result.stdout_lines[-1] == '(0 rows)'" - -### TODO: test expires, fail_on_user +### TODO: fail_on_user # # Test db ownership diff --git a/test/integration/targets/postgresql/tasks/postgresql_user_9.5_or_greater.yml b/test/integration/targets/postgresql/tasks/postgresql_user_9.5_or_greater.yml deleted file mode 100644 index e028cc80964..00000000000 --- a/test/integration/targets/postgresql/tasks/postgresql_user_9.5_or_greater.yml +++ /dev/null @@ -1,90 +0,0 @@ ---- -- name: Create a user with all role attributes - become_user: "{{ pg_user }}" - become: True - postgresql_user: - name: "{{ db_user1 }}" - state: "present" - role_attr_flags: "SUPERUSER,CREATEROLE,CREATEDB,INHERIT,login,BYPASSRLS" - login_user: "{{ pg_user }}" - db: postgres - -- name: Check that the user has the requested role attributes - become_user: "{{ pg_user }}" - become: True - shell: echo "select 'super:'||rolsuper, 'createrole:'||rolcreaterole, 'create:'||rolcreatedb, 'inherit:'||rolinherit, 'login:'||rolcanlogin, 'bypassrls:'||rolbypassrls from pg_roles where rolname='{{ db_user1 }}';" | psql -d postgres - register: result - -- assert: - that: - - "result.stdout_lines[-1] == '(1 row)'" - - "'super:t' in result.stdout_lines[-2]" - - "'createrole:t' in result.stdout_lines[-2]" - - "'create:t' in result.stdout_lines[-2]" - - "'inherit:t' in result.stdout_lines[-2]" - - "'login:t' in result.stdout_lines[-2]" - - "'bypassrls:t' in result.stdout_lines[-2]" - -- name: Modify a user to have no role attributes - become_user: "{{ pg_user }}" - become: True - postgresql_user: - name: "{{ db_user1 }}" - state: "present" - role_attr_flags: "NOSUPERUSER,NOCREATEROLE,NOCREATEDB,noinherit,NOLOGIN,NOBYPASSRLS" - login_user: "{{ pg_user }}" - db: postgres - register: result - -- name: Check that ansible reports it modified the role - assert: - that: - - "result.changed == True" - -- name: Check that the user has the requested role attributes - become_user: "{{ pg_user }}" - become: True - shell: echo "select 'super:'||rolsuper, 'createrole:'||rolcreaterole, 'create:'||rolcreatedb, 'inherit:'||rolinherit, 'login:'||rolcanlogin, 'bypassrls:'||rolbypassrls from pg_roles where rolname='{{ db_user1 }}';" | psql -d postgres - register: result - -- assert: - that: - - "result.stdout_lines[-1] == '(1 row)'" - - "'super:f' in result.stdout_lines[-2]" - - "'createrole:f' in result.stdout_lines[-2]" - - "'create:f' in result.stdout_lines[-2]" - - "'inherit:f' in result.stdout_lines[-2]" - - "'login:f' in result.stdout_lines[-2]" - - "'bypassrls:f' in result.stdout_lines[-2]" - -- name: Modify a single role attribute on a user - become_user: "{{ pg_user }}" - become: True - postgresql_user: - name: "{{ db_user1 }}" - state: "present" - role_attr_flags: "LOGIN" - login_user: "{{ pg_user }}" - db: postgres - register: result - -- name: Check that ansible reports it modified the role - assert: - that: - - "result.changed == True" - -- name: Check that the user has the requested role attributes - become_user: "{{ pg_user }}" - become: True - shell: echo "select 'super:'||rolsuper, 'createrole:'||rolcreaterole, 'create:'||rolcreatedb, 'inherit:'||rolinherit, 'login:'||rolcanlogin, 'bypassrls:'||rolbypassrls from pg_roles where rolname='{{ db_user1 }}';" | psql -d postgres - register: result - -- assert: - that: - - "result.stdout_lines[-1] == '(1 row)'" - - "'super:f' in result.stdout_lines[-2]" - - "'createrole:f' in result.stdout_lines[-2]" - - "'create:f' in result.stdout_lines[-2]" - - "'inherit:f' in result.stdout_lines[-2]" - - "'login:t' in result.stdout_lines[-2]" - - "'bypassrls:f' in result.stdout_lines[-2]" diff --git a/test/integration/targets/postgresql/tasks/postgresql_user_less_than_9.5.yml b/test/integration/targets/postgresql/tasks/postgresql_user_less_than_9.5.yml deleted file mode 100644 index 4f335391ecf..00000000000 --- a/test/integration/targets/postgresql/tasks/postgresql_user_less_than_9.5.yml +++ /dev/null @@ -1,87 +0,0 @@ ---- -- name: Create a user with all role attributes - become_user: "{{ pg_user }}" - become: True - postgresql_user: - name: "{{ db_user1 }}" - state: "present" - role_attr_flags: "SUPERUSER,CREATEROLE,CREATEDB,INHERIT,login" - login_user: "{{ pg_user }}" - db: postgres - -- name: Check that the user has the requested role attributes - become_user: "{{ pg_user }}" - become: True - shell: echo "select 'super:'||rolsuper, 'createrole:'||rolcreaterole, 'create:'||rolcreatedb, 'inherit:'||rolinherit, 'login:'||rolcanlogin from pg_roles where rolname='{{ db_user1 }}';" | psql -d postgres - register: result - -- assert: - that: - - "result.stdout_lines[-1] == '(1 row)'" - - "'super:t' in result.stdout_lines[-2]" - - "'createrole:t' in result.stdout_lines[-2]" - - "'create:t' in result.stdout_lines[-2]" - - "'inherit:t' in result.stdout_lines[-2]" - - "'login:t' in result.stdout_lines[-2]" - -- name: Modify a user to have no role attributes - become_user: "{{ pg_user }}" - become: True - postgresql_user: - name: "{{ db_user1 }}" - state: "present" - role_attr_flags: "NOSUPERUSER,NOCREATEROLE,NOCREATEDB,noinherit,NOLOGIN" - login_user: "{{ pg_user }}" - db: postgres - register: result - -- name: Check that ansible reports it modified the role - assert: - that: - - "result.changed == True" - -- name: Check that the user has the requested role attributes - become_user: "{{ pg_user }}" - become: True - shell: echo "select 'super:'||rolsuper, 'createrole:'||rolcreaterole, 'create:'||rolcreatedb, 'inherit:'||rolinherit, 'login:'||rolcanlogin from pg_roles where rolname='{{ db_user1 }}';" | psql -d postgres - register: result - -- assert: - that: - - "result.stdout_lines[-1] == '(1 row)'" - - "'super:f' in result.stdout_lines[-2]" - - "'createrole:f' in result.stdout_lines[-2]" - - "'create:f' in result.stdout_lines[-2]" - - "'inherit:f' in result.stdout_lines[-2]" - - "'login:f' in result.stdout_lines[-2]" - -- name: Modify a single role attribute on a user - become_user: "{{ pg_user }}" - become: True - postgresql_user: - name: "{{ db_user1 }}" - state: "present" - role_attr_flags: "LOGIN" - login_user: "{{ pg_user }}" - db: postgres - register: result - -- name: Check that ansible reports it modified the role - assert: - that: - - "result.changed == True" - -- name: Check that the user has the requested role attributes - become_user: "{{ pg_user }}" - become: True - shell: echo "select 'super:'||rolsuper, 'createrole:'||rolcreaterole, 'create:'||rolcreatedb, 'inherit:'||rolinherit, 'login:'||rolcanlogin from pg_roles where rolname='{{ db_user1 }}';" | psql -d postgres - register: result - -- assert: - that: - - "result.stdout_lines[-1] == '(1 row)'" - - "'super:f' in result.stdout_lines[-2]" - - "'createrole:f' in result.stdout_lines[-2]" - - "'create:f' in result.stdout_lines[-2]" - - "'inherit:f' in result.stdout_lines[-2]" - - "'login:t' in result.stdout_lines[-2]" diff --git a/test/integration/targets/postgresql/tasks/test_no_password_change.yml b/test/integration/targets/postgresql/tasks/test_no_password_change.yml new file mode 100644 index 00000000000..16ac35b1fa5 --- /dev/null +++ b/test/integration/targets/postgresql/tasks/test_no_password_change.yml @@ -0,0 +1,167 @@ +- vars: + task_parameters: &task_parameters + become_user: "{{ pg_user }}" + become: True + register: result + postgresql_parameters: ¶meters + db: postgres + name: "{{ db_user1 }}" + login_user: "{{ pg_user }}" + + block: + + - name: Create a user with all role attributes + <<: *task_parameters + postgresql_user: + <<: *parameters + state: "present" + role_attr_flags: "SUPERUSER,CREATEROLE,CREATEDB,INHERIT,login{{ bypassrls_supported | ternary(',BYPASSRLS', '') }}" + no_password_changes: '{{ no_password_changes }}' # no_password_changes is ignored when user doesn't already exist + + - name: Check that the user has the requested role attributes + <<: *task_parameters + shell: "echo \"select 'super:'||rolsuper, 'createrole:'||rolcreaterole, 'create:'||rolcreatedb, 'inherit:'||rolinherit, 'login:'||rolcanlogin {{ bypassrls_supported | ternary(\", 'bypassrls:'||rolbypassrls\", '') }} from pg_roles where rolname='{{ db_user1 }}';\" | psql -d postgres" + + - assert: + that: + - "result.stdout_lines[-1] == '(1 row)'" + - "'super:t' in result.stdout_lines[-2]" + - "'createrole:t' in result.stdout_lines[-2]" + - "'create:t' in result.stdout_lines[-2]" + - "'inherit:t' in result.stdout_lines[-2]" + - "'login:t' in result.stdout_lines[-2]" + + - block: + - name: Check that the user has the requested role attribute BYPASSRLS + <<: *task_parameters + shell: "echo \"select 'bypassrls:'||rolbypassrls from pg_roles where rolname='{{ db_user1 }}';\" | psql -d postgres" + + - assert: + that: + - "not bypassrls_supported or 'bypassrls:t' in result.stdout_lines[-2]" + when: bypassrls_supported + + - name: Modify a user to have no role attributes + <<: *task_parameters + postgresql_user: + <<: *parameters + state: "present" + role_attr_flags: "NOSUPERUSER,NOCREATEROLE,NOCREATEDB,noinherit,NOLOGIN{{ bypassrls_supported | ternary(',NOBYPASSRLS', '') }}" + no_password_changes: '{{ no_password_changes }}' + + - name: Check that ansible reports it modified the role + assert: + that: + - "result.changed" + + - name: "Check that the user doesn't have any attribute" + <<: *task_parameters + shell: "echo \"select 'super:'||rolsuper, 'createrole:'||rolcreaterole, 'create:'||rolcreatedb, 'inherit:'||rolinherit, 'login:'||rolcanlogin from pg_roles where rolname='{{ db_user1 }}';\" | psql -d postgres" + + - assert: + that: + - "result.stdout_lines[-1] == '(1 row)'" + - "'super:f' in result.stdout_lines[-2]" + - "'createrole:f' in result.stdout_lines[-2]" + - "'create:f' in result.stdout_lines[-2]" + - "'inherit:f' in result.stdout_lines[-2]" + - "'login:f' in result.stdout_lines[-2]" + + - block: + - name: Check that the user has the requested role attribute BYPASSRLS + <<: *task_parameters + shell: "echo \"select 'bypassrls:'||rolbypassrls from pg_roles where rolname='{{ db_user1 }}';\" | psql -d postgres" + + - assert: + that: + - "not bypassrls_supported or 'bypassrls:f' in result.stdout_lines[-2]" + when: bypassrls_supported + + - name: Try to add an invalid attribute + <<: *task_parameters + postgresql_user: + <<: *parameters + state: "present" + role_attr_flags: "NOSUPERUSER,NOCREATEROLE,NOCREATEDB,noinherit,NOLOGIN{{ bypassrls_supported | ternary(',NOBYPASSRLS', '') }},INVALID" + no_password_changes: '{{ no_password_changes }}' + ignore_errors: True + + - name: Check that ansible reports failure + assert: + that: + - "not result.changed" + - "result.failed" + - "result.msg == 'Invalid role_attr_flags specified: INVALID'" + + - name: Modify a single role attribute on a user + <<: *task_parameters + postgresql_user: + <<: *parameters + state: "present" + role_attr_flags: "LOGIN" + no_password_changes: '{{ no_password_changes }}' + + - name: Check that ansible reports it modified the role + assert: + that: + - "result.changed" + + - name: Check the role attributes + <<: *task_parameters + shell: echo "select 'super:'||rolsuper, 'createrole:'||rolcreaterole, 'create:'||rolcreatedb, 'inherit:'||rolinherit, 'login:'||rolcanlogin from pg_roles where rolname='{{ db_user1 }}';" | psql -d postgres + + - assert: + that: + - "result.stdout_lines[-1] == '(1 row)'" + - "'super:f' in result.stdout_lines[-2]" + - "'createrole:f' in result.stdout_lines[-2]" + - "'create:f' in result.stdout_lines[-2]" + - "'inherit:f' in result.stdout_lines[-2]" + - "'login:t' in result.stdout_lines[-2]" + + - block: + - name: Check the role attribute BYPASSRLS + <<: *task_parameters + shell: echo "select 'bypassrls:'||rolbypassrls from pg_roles where rolname='{{ db_user1 }}';" | psql -d postgres + + - assert: + that: + - "(postgres_version_resp.stdout | version_compare('9.5.0', '<')) or 'bypassrls:f' in result.stdout_lines[-2]" + when: bypassrls_supported + + - name: Check that using same attribute a second time does nothing + <<: *task_parameters + postgresql_user: + <<: *parameters + state: "present" + role_attr_flags: "LOGIN" + no_password_changes: '{{ no_password_changes }}' + environment: + PGOPTIONS: '-c default_transaction_read_only=on' # ensure 'alter user' query isn't executed + + - name: Check there isn't any update reported + assert: + that: + - "not result.changed" + + - name: Cleanup the user + <<: *task_parameters + postgresql_user: + <<: *parameters + state: 'absent' + no_password_changes: '{{ no_password_changes }}' # user deletion: no_password_changes is ignored + + - name: Check that user was removed + <<: *task_parameters + shell: echo "select * from pg_user where usename='{{ db_user1 }}';" | psql -d postgres + + - assert: + that: + - "result.stdout_lines[-1] == '(0 rows)'" + + always: + - name: Cleanup the user + <<: *task_parameters + postgresql_user: + <<: *parameters + state: 'absent' diff --git a/test/integration/targets/postgresql/tasks/test_user.yml b/test/integration/targets/postgresql/tasks/test_password.yml similarity index 79% rename from test/integration/targets/postgresql/tasks/test_user.yml rename to test/integration/targets/postgresql/tasks/test_password.yml index 96fabeb258c..2f02e90c7cf 100644 --- a/test/integration/targets/postgresql/tasks/test_user.yml +++ b/test/integration/targets/postgresql/tasks/test_password.yml @@ -8,7 +8,7 @@ name: "{{ db_user1 }}" login_user: "{{ pg_user }}" - block: # block is only used here in order to be able to define YAML anchors at the beginning in 'vars' section + block: - name: 'Check that PGOPTIONS environment variable is effective (1/2)' <<: *task_parameters postgresql_user: @@ -63,6 +63,27 @@ that: - "{{ not result|changed }}" + - name: 'Define an expiration time' + <<: *task_parameters + postgresql_user: + <<: *parameters + expires: '2025-01-01' + environment: + PGCLIENTENCODING: 'UTF8' + + - <<: *changed + + - name: 'Redefine the same expiration time' + <<: *task_parameters + postgresql_user: + expires: '2025-01-01' + <<: *parameters + environment: + PGCLIENTENCODING: 'UTF8' + PGOPTIONS: '-c default_transaction_read_only=on' # ensure 'alter user' query isn't executed + + - <<: *not_changed + - block: - name: 'Using MD5-hashed password: check that password not changed when using cleartext password' @@ -72,7 +93,7 @@ password: '{{ db_password1 }}' encrypted: 'yes' environment: -# PGCLIENTENCODING: 'UTF8' + PGCLIENTENCODING: 'UTF8' PGOPTIONS: '-c default_transaction_read_only=on' # ensure 'alter user' query isn't executed - <<: *not_changed @@ -99,6 +120,18 @@ - <<: *not_changed + - name: 'Redefine the same expiration time and password (encrypted)' + <<: *task_parameters + postgresql_user: + <<: *parameters + encrypted: 'yes' + password: "md5{{ (db_password1 ~ db_user1) | hash('md5')}}" + expires: '2025-01-01' + environment: + PGOPTIONS: '-c default_transaction_read_only=on' # ensure 'alter user' query isn't executed + + - <<: *not_changed + - name: 'Using MD5-hashed password: check that password changed when using another cleartext password' <<: *task_parameters postgresql_user: @@ -144,6 +177,19 @@ - <<: *not_changed + - name: 'Redefine the same expiration time and password (not encrypted)' + <<: *task_parameters + postgresql_user: + <<: *parameters + password: "{{ db_password1 }}" + encrypted: 'no' + expires: '2025-01-01' + environment: + PGCLIENTENCODING: 'UTF8' + PGOPTIONS: '-c default_transaction_read_only=on' # ensure 'alter user' query isn't executed + + - <<: *not_changed + - name: 'Using cleartext password: check that password changed when using another cleartext password' <<: *task_parameters postgresql_user: @@ -184,3 +230,10 @@ PGOPTIONS: '-c default_transaction_read_only=on' # ensure 'alter user' query isn't executed - <<: *not_changed + + always: + - name: Remove user + <<: *task_parameters + postgresql_user: + state: 'absent' + <<: *parameters diff --git a/test/sanity/pep8/legacy-files.txt b/test/sanity/pep8/legacy-files.txt index 12a159fd3b5..ea95ed7160e 100644 --- a/test/sanity/pep8/legacy-files.txt +++ b/test/sanity/pep8/legacy-files.txt @@ -233,7 +233,6 @@ lib/ansible/modules/database/postgresql/postgresql_ext.py lib/ansible/modules/database/postgresql/postgresql_lang.py lib/ansible/modules/database/postgresql/postgresql_privs.py lib/ansible/modules/database/postgresql/postgresql_schema.py -lib/ansible/modules/database/postgresql/postgresql_user.py lib/ansible/modules/database/vertica/vertica_configuration.py lib/ansible/modules/database/vertica/vertica_facts.py lib/ansible/modules/database/vertica/vertica_role.py