Fix a problem introduced with #1101 and optimize privilege handling
* If a db user belonged to a role which had a privilege, the user would not have the privilege added as the role gave the appearance that the user already had it. Fixed to always check the privileges specific to the user. * Make fewer db queries to determine if privileges need to be changed and change them (was four for each privilege. Now two for each object that has a set of privileges changed).
This commit is contained in:
Toshio Kuratomi
parent
7dd9f57e16
commit
c9b17136e4
1 changed files with 81 additions and 55 deletions
|
|
@ -324,12 +324,21 @@ def user_delete(cursor, user):
|
|||
cursor.execute("RELEASE SAVEPOINT ansible_pgsql_user_delete")
|
||||
return True
|
||||
|
||||
def has_table_privilege(cursor, user, table, priv):
|
||||
if priv == 'ALL':
|
||||
priv = ','.join([ p for p in VALID_PRIVS['table'] if p != 'ALL' ])
|
||||
query = 'SELECT has_table_privilege(%s, %s, %s)'
|
||||
cursor.execute(query, (user, table, priv))
|
||||
return cursor.fetchone()[0]
|
||||
def has_table_privileges(cursor, user, table, privs):
|
||||
"""
|
||||
Return the difference between the privileges that a user already has and
|
||||
the privileges that they desire to have.
|
||||
|
||||
:returns: tuple of:
|
||||
* privileges that they have and were requested
|
||||
* privileges they currently hold but were not requested
|
||||
* privileges requested that they do not hold
|
||||
"""
|
||||
cur_privs = get_table_privileges(cursor, user, table)
|
||||
have_currently = cur_privs.intersection(privs)
|
||||
other_current = cur_privs.difference(privs)
|
||||
desired = privs.difference(cur_privs)
|
||||
return (have_currently, other_current, desired)
|
||||
|
||||
def get_table_privileges(cursor, user, table):
|
||||
if '.' in table:
|
||||
|
|
@ -339,26 +348,21 @@ def get_table_privileges(cursor, user, table):
|
|||
query = '''SELECT privilege_type FROM information_schema.role_table_grants
|
||||
WHERE grantee=%s AND table_name=%s AND table_schema=%s'''
|
||||
cursor.execute(query, (user, table, schema))
|
||||
return set([x[0] for x in cursor.fetchall()])
|
||||
return frozenset([x[0] for x in cursor.fetchall()])
|
||||
|
||||
def grant_table_privilege(cursor, user, table, priv):
|
||||
def grant_table_privileges(cursor, user, table, privs):
|
||||
# Note: priv escaped by parse_privs
|
||||
prev_priv = get_table_privileges(cursor, user, table)
|
||||
privs = ', '.join(privs)
|
||||
query = 'GRANT %s ON TABLE %s TO %s' % (
|
||||
priv, pg_quote_identifier(table, 'table'), pg_quote_identifier(user, 'role') )
|
||||
privs, pg_quote_identifier(table, 'table'), pg_quote_identifier(user, 'role') )
|
||||
cursor.execute(query)
|
||||
curr_priv = get_table_privileges(cursor, user, table)
|
||||
return len(curr_priv) > len(prev_priv)
|
||||
|
||||
def revoke_table_privilege(cursor, user, table, priv):
|
||||
def revoke_table_privileges(cursor, user, table, privs):
|
||||
# Note: priv escaped by parse_privs
|
||||
prev_priv = get_table_privileges(cursor, user, table)
|
||||
privs = ', '.join(privs)
|
||||
query = 'REVOKE %s ON TABLE %s FROM %s' % (
|
||||
priv, pg_quote_identifier(table, 'table'), pg_quote_identifier(user, 'role') )
|
||||
privs, pg_quote_identifier(table, 'table'), pg_quote_identifier(user, 'role') )
|
||||
cursor.execute(query)
|
||||
curr_priv = get_table_privileges(cursor, user, table)
|
||||
return len(curr_priv) < len(prev_priv)
|
||||
|
||||
|
||||
def get_database_privileges(cursor, user, db):
|
||||
priv_map = {
|
||||
|
|
@ -370,80 +374,89 @@ def get_database_privileges(cursor, user, db):
|
|||
cursor.execute(query, (db,))
|
||||
datacl = cursor.fetchone()[0]
|
||||
if datacl is None:
|
||||
return []
|
||||
return set()
|
||||
r = re.search('%s=(C?T?c?)/[a-z]+\,?' % user, datacl)
|
||||
if r is None:
|
||||
return []
|
||||
o = []
|
||||
return set()
|
||||
o = set()
|
||||
for v in r.group(1):
|
||||
o.append(priv_map[v])
|
||||
return o
|
||||
o.add(priv_map[v])
|
||||
return normalize_privileges(o, 'database')
|
||||
|
||||
def has_database_privilege(cursor, user, db, priv):
|
||||
if priv == 'ALL':
|
||||
priv = ','.join([ p for p in VALID_PRIVS['database'] if p != 'ALL' ])
|
||||
query = 'SELECT has_database_privilege(%s, %s, %s)'
|
||||
cursor.execute(query, (user, db, priv))
|
||||
return cursor.fetchone()[0]
|
||||
def has_database_privileges(cursor, user, db, privs):
|
||||
"""
|
||||
Return the difference between the privileges that a user already has and
|
||||
the privileges that they desire to have.
|
||||
|
||||
def grant_database_privilege(cursor, user, db, priv):
|
||||
:returns: tuple of:
|
||||
* privileges that they have and were requested
|
||||
* privileges they currently hold but were not requested
|
||||
* privileges requested that they do not hold
|
||||
"""
|
||||
cur_privs = get_database_privileges(cursor, user, db)
|
||||
have_currently = cur_privs.intersection(privs)
|
||||
other_current = cur_privs.difference(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
|
||||
prev_priv = get_database_privileges(cursor, user, db)
|
||||
privs =', '.join(privs)
|
||||
if user == "PUBLIC":
|
||||
query = 'GRANT %s ON DATABASE %s TO PUBLIC' % (
|
||||
priv, pg_quote_identifier(db, 'database'))
|
||||
privs, pg_quote_identifier(db, 'database'))
|
||||
else:
|
||||
query = 'GRANT %s ON DATABASE %s TO %s' % (
|
||||
priv, pg_quote_identifier(db, 'database'),
|
||||
privs, pg_quote_identifier(db, 'database'),
|
||||
pg_quote_identifier(user, 'role'))
|
||||
cursor.execute(query)
|
||||
curr_priv = get_database_privileges(cursor, user, db)
|
||||
return len(curr_priv) > len(prev_priv)
|
||||
|
||||
def revoke_database_privilege(cursor, user, db, priv):
|
||||
def revoke_database_privileges(cursor, user, db, privs):
|
||||
# Note: priv escaped by parse_privs
|
||||
prev_priv = get_database_privileges(cursor, user, db)
|
||||
privs = ', '.join(privs)
|
||||
if user == "PUBLIC":
|
||||
query = 'REVOKE %s ON DATABASE %s FROM PUBLIC' % (
|
||||
priv, pg_quote_identifier(db, 'database'))
|
||||
privs, pg_quote_identifier(db, 'database'))
|
||||
else:
|
||||
query = 'REVOKE %s ON DATABASE %s FROM %s' % (
|
||||
priv, pg_quote_identifier(db, 'database'),
|
||||
privs, pg_quote_identifier(db, 'database'),
|
||||
pg_quote_identifier(user, 'role'))
|
||||
cursor.execute(query)
|
||||
curr_priv = get_database_privileges(cursor, user, db)
|
||||
return len(curr_priv) < len(prev_priv)
|
||||
|
||||
def revoke_privileges(cursor, user, privs):
|
||||
if privs is None:
|
||||
return False
|
||||
|
||||
revoke_funcs = dict(table=revoke_table_privileges, database=revoke_database_privileges)
|
||||
check_funcs = dict(table=has_table_privileges, database=has_database_privileges)
|
||||
|
||||
changed = False
|
||||
revoke_funcs = dict(table=revoke_table_privilege, database=revoke_database_privilege)
|
||||
check_funcs = dict(table=has_table_privilege, database=has_database_privilege)
|
||||
for type_ in privs:
|
||||
for name, privileges in privs[type_].iteritems():
|
||||
for privilege in privileges:
|
||||
if check_funcs[type_](cursor, user, name, privilege):
|
||||
changed = revoke_funcs[type_](cursor, user, name, privilege)\
|
||||
or changed
|
||||
|
||||
# Check that any of the privileges requested to be removed are
|
||||
# currently granted to the user
|
||||
differences = check_funcs[type_](cursor, user, name, privileges)
|
||||
if differences[0]:
|
||||
revoke_funcs[type_](cursor, user, name, privileges)
|
||||
changed = True
|
||||
return changed
|
||||
|
||||
def grant_privileges(cursor, user, privs):
|
||||
if privs is None:
|
||||
return False
|
||||
grant_funcs = dict(table=grant_table_privilege, database=grant_database_privilege)
|
||||
check_funcs = dict(table=has_table_privilege, database=has_database_privilege)
|
||||
|
||||
grant_funcs = dict(table=grant_table_privileges, database=grant_database_privileges)
|
||||
check_funcs = dict(table=has_table_privileges, database=has_database_privileges)
|
||||
|
||||
changed = False
|
||||
for type_ in privs:
|
||||
for name, privileges in privs[type_].iteritems():
|
||||
for privilege in privileges:
|
||||
if not check_funcs[type_](cursor, user, name, privilege):
|
||||
changed = grant_funcs[type_](cursor, user, name, privilege)\
|
||||
or changed
|
||||
|
||||
# Check that any of the privileges requested for the user are
|
||||
# currently missing
|
||||
differences = check_funcs[type_](cursor, user, name, privileges)
|
||||
if differences[2]:
|
||||
grant_funcs[type_](cursor, user, name, privileges)
|
||||
changed = True
|
||||
return changed
|
||||
|
||||
def parse_role_attrs(role_attr_flags):
|
||||
|
|
@ -472,6 +485,17 @@ def parse_role_attrs(role_attr_flags):
|
|||
o_flags = ' '.join(flag_set)
|
||||
return o_flags
|
||||
|
||||
def normalize_privileges(privs, type_):
|
||||
new_privs = set(privs)
|
||||
if 'ALL' in privs:
|
||||
new_privs.update(VALID_PRIVS[type_])
|
||||
new_privs.remove('ALL')
|
||||
if 'TEMP' in privs:
|
||||
new_privs.add('TEMPORARY')
|
||||
new_privs.remove('TEMP')
|
||||
|
||||
return new_privs
|
||||
|
||||
def parse_privs(privs, db):
|
||||
"""
|
||||
Parse privilege string to determine permissions for database db.
|
||||
|
|
@ -504,6 +528,8 @@ 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_]))))
|
||||
|
||||
priv_set = normalize_privileges(priv_set, type_)
|
||||
o_privs[type_][name] = priv_set
|
||||
|
||||
return o_privs
|
||||
|
|
|
|||
Loading…
Reference in a new issue