From edbad801231668f67e1078c45643c1654ac31e16 Mon Sep 17 00:00:00 2001 From: Guillaume Dufour Date: Thu, 25 Feb 2016 11:01:34 +0100 Subject: [PATCH 1/5] fix #1731 : mongodb_user always says changed --- database/misc/mongodb_user.py | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/database/misc/mongodb_user.py b/database/misc/mongodb_user.py index 12d348e9a92..44659221bc4 100644 --- a/database/misc/mongodb_user.py +++ b/database/misc/mongodb_user.py @@ -148,9 +148,9 @@ else: # MongoDB module specific support methods. # -def user_find(client, user): +def user_find(client, user, db_name): for mongo_user in client["admin"].system.users.find(): - if mongo_user['user'] == user: + if mongo_user['user'] == user and mongo_user['db'] == db_name: return mongo_user return False @@ -158,6 +158,7 @@ def user_add(module, client, db_name, user, password, roles): #pymono's user_add is a _create_or_update_user so we won't know if it was changed or updated #without reproducing a lot of the logic in database.py of pymongo db = client[db_name] + if roles is None: db.add_user(user, password, False) else: @@ -170,7 +171,7 @@ def user_add(module, client, db_name, user, password, roles): module.fail_json(msg=err_msg) def user_remove(module, client, db_name, user): - exists = user_find(client, user) + exists = user_find(client, user, db_name) if exists: db = client[db_name] db.remove_user(user) @@ -223,7 +224,7 @@ def main(): login_host = module.params['login_host'] login_port = module.params['login_port'] login_database = module.params['login_database'] - + replica_set = module.params['replica_set'] db_name = module.params['database'] user = module.params['name'] @@ -261,14 +262,22 @@ def main(): if password is None and update_password == 'always': module.fail_json(msg='password parameter required when adding a user unless update_password is set to on_create') - if update_password != 'always' and user_find(client, user): + uinfo = user_find(client, user, db_name) + if update_password != 'always' and uinfo: password = None + if list(map((lambda x: x['role']), uinfo['roles'])) == roles: + module.exit_json(changed=False, user=user) try: user_add(module, client, db_name, user, password, roles) except OperationFailure, e: module.fail_json(msg='Unable to add or update user: %s' % str(e)) + # Here we can check password change if mongo provide a query for that : https://jira.mongodb.org/browse/SERVER-22848 + #newuinfo = user_find(client, user, db_name) + #if uinfo['role'] == newuinfo['role'] and CheckPasswordHere: + # module.exit_json(changed=False, user=user) + elif state == 'absent': try: user_remove(module, client, db_name, user) From 29daa6ffe08c138f09fac3702f8fbe71863eb583 Mon Sep 17 00:00:00 2001 From: Guillaume Dufour Date: Fri, 26 Feb 2016 15:08:04 +0100 Subject: [PATCH 2/5] avoid problem with old mongo version without roles --- database/misc/mongodb_user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/database/misc/mongodb_user.py b/database/misc/mongodb_user.py index 44659221bc4..848371884d6 100644 --- a/database/misc/mongodb_user.py +++ b/database/misc/mongodb_user.py @@ -265,7 +265,7 @@ def main(): uinfo = user_find(client, user, db_name) if update_password != 'always' and uinfo: password = None - if list(map((lambda x: x['role']), uinfo['roles'])) == roles: + if 'roles' in uinfo and list(map((lambda x: x['role']), uinfo['roles'])) == roles: module.exit_json(changed=False, user=user) try: From c6ebfa548019a48095140c3f95b68ba8cede212e Mon Sep 17 00:00:00 2001 From: Guillaume Dufour Date: Sun, 28 Feb 2016 08:05:20 +0100 Subject: [PATCH 3/5] use python fallback to avoid error on old mongo version without roles --- database/misc/mongodb_user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/database/misc/mongodb_user.py b/database/misc/mongodb_user.py index 848371884d6..e58857586eb 100644 --- a/database/misc/mongodb_user.py +++ b/database/misc/mongodb_user.py @@ -265,7 +265,7 @@ def main(): uinfo = user_find(client, user, db_name) if update_password != 'always' and uinfo: password = None - if 'roles' in uinfo and list(map((lambda x: x['role']), uinfo['roles'])) == roles: + if list(map((lambda x: x['role']), uinfo.get('roles', []))) == roles: module.exit_json(changed=False, user=user) try: From 5ff957322a66dcae902aace216d9eb06d9f91d8a Mon Sep 17 00:00:00 2001 From: Marcos Diez Date: Wed, 16 Mar 2016 21:59:24 +0200 Subject: [PATCH 4/5] mongodb_user: fix checking if the roles of an oplog reader user changed --- database/misc/mongodb_user.py | 39 ++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/database/misc/mongodb_user.py b/database/misc/mongodb_user.py index e58857586eb..bc636f52397 100644 --- a/database/misc/mongodb_user.py +++ b/database/misc/mongodb_user.py @@ -193,6 +193,43 @@ def load_mongocnf(): return creds + + +def check_if_roles_changed(uinfo, roles, db_name): +# The reason for such complicated method is a user which can read the oplog on a replicaset +# This user must have access to the local DB, but since this DB does not have users +# and is not synchronized among replica sets, the user must be stored on the admin db +# { +# "_id" : "admin.oplog_reader", +# "user" : "oplog_reader", +# "db" : "admin", +# "roles" : [ +# { +# "role" : "read", +# "db" : "local" +# } +# ] +# } + + def make_sure_roles_are_a_list_of_dict(roles, db_name): + output = list() + for role in roles: + if isinstance(role, basestring): + new_role = { "role": role, "db": db_name } + output.append(new_role) + else: + output.append(role) + return output + + roles_as_list_of_dict = make_sure_roles_are_a_list_of_dict(roles, db_name) + uinfo_roles = uinfo.get('roles', []) + + if sorted(roles_as_list_of_dict) == sorted(uinfo_roles): + return False + return True + + + # ========================================= # Module execution. # @@ -265,7 +302,7 @@ def main(): uinfo = user_find(client, user, db_name) if update_password != 'always' and uinfo: password = None - if list(map((lambda x: x['role']), uinfo.get('roles', []))) == roles: + if not check_if_roles_changed(uinfo, roles, db_name): module.exit_json(changed=False, user=user) try: From 7ec5209e18439fd984e1b4705e83dc3a2509185c Mon Sep 17 00:00:00 2001 From: Marcos Diez Date: Wed, 16 Mar 2016 22:07:58 +0200 Subject: [PATCH 5/5] mongodb_user.py: changes on comments --- database/misc/mongodb_user.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/database/misc/mongodb_user.py b/database/misc/mongodb_user.py index bc636f52397..829dcb7e6b5 100644 --- a/database/misc/mongodb_user.py +++ b/database/misc/mongodb_user.py @@ -196,17 +196,18 @@ def load_mongocnf(): def check_if_roles_changed(uinfo, roles, db_name): -# The reason for such complicated method is a user which can read the oplog on a replicaset -# This user must have access to the local DB, but since this DB does not have users +# We must be aware of users which can read the oplog on a replicaset +# Such users must have access to the local DB, but since this DB does not store users credentials # and is not synchronized among replica sets, the user must be stored on the admin db +# Therefore their structure is the following : # { # "_id" : "admin.oplog_reader", # "user" : "oplog_reader", -# "db" : "admin", +# "db" : "admin", # <-- admin DB # "roles" : [ # { # "role" : "read", -# "db" : "local" +# "db" : "local" # <-- local DB # } # ] # }