user groups option always triggers changed

[raffomania]

My ansible version is 1.8.2 and I'm managing an Arch Linux box from an Arch Linux box.

Summary:

For the user module, if the groups option is specified, it always triggers "changed" even if the group list doesn't change.

Steps To Reproduce:

- user: name={{user}} home=/srv/samba createhome=yes system=yes shell=/bin/nologin groups={{group}} group={{group}} append=yes

Expected Results:

TASK: [samba | Create samba user] ********************************************* 
ok: [prokyon]

Actual Results:

TASK: [samba | Create samba user] ********************************************* 
changed: [prokyon]

[juliedavila]

Hitting this bug RHEL6.6 as well

[blackreed9]

I believe the issue is in User.user_group_membership(). If the user is only a member of its own group then user_group_membership() method returns a empty list so the code thinks it needs update the group file.

Current Method

def user_group_membership(self):
        groups = []
        info = self.get_pwd_info()
        for group in grp.getgrall():
            if self.name in group.gr_mem and not info[3] == group.gr_gid:
                groups.append(group[0])
        return groups

The current method:

1. Gets the user's info
2. Gets all the group info
3. Loops through the groups looking for the groups that the user is a member of, but doesn't have the same GID as that group

The check for the GID I think is the problem. If the purpose of the method is to get a list of all the groups that a user belongs to ( and I think it is ), then all we should care about is if self.name in group.gr_mem.

Proposed Method

def user_group_membership(self):
        groups = []
        info = self.get_pwd_info()
        for group in grp.getgrall():
            if self.name in group.gr_mem:
                groups.append(group[0])
        return groups

I would just have dropped a pull request, but someone put the GID check in there and I want to make sure I'm not missing some subtle nuance. If I'm not I'm happy to submit my pull request.

[bcoca]

I'm not sure but it sounds like an optimization for group appending and/or when the user utility is setup to automatically create a group with the same name and gid as the user and its uid.

[blackreed9]

If the point of the method is to get the list of groups the user is a member of then that should include all of the groups including the one that matches your GID ( since you are automatically associated with that group even if you're not explicitly listed in the members list )

Since the next thing the code does is remove duplicates between what was asked for with the groups argument and what exists in the /etc/group file. It isn't really much of an optimization.

group_diff = set(current_groups).symmetric_difference(groups)

User creation follows a different path and never calls these methods.

Tests without the GID check ( I added some prints to make it easier to see what is happening )

If I have three existing users ( monkeys, chicken, turkey ), each with a corresponding group.

# tail -3 /etc/group
monkeys:x:502:
chicken:x:503:
turkey:x:504:

If I run a play with MODULE_ARGS = 'name=monkeys groups=monkeys,turkey append=yes' it does what I'd expect and append monkeys to the two groups and changed: true

# python user
Current Groups: []
Self.Groups: monkeys,turkey
Groups Set: set(['turkey', 'monkeys'])
Group Diff: set(['turkey', 'monkeys'])
{"comment": "", "shell": "/bin/nologin", "group": 502, "name": "monkeys", "changed": true, "state": "present", "groups": "monkeys,turkey", "home": "/home/monkeys", "move_home": false, "append": true, "uid": 498}
@core-a1 12:33:46 ~/.ansible/tmp/ansible-tmp-1430834351.8-88768220443752# tail -3 /etc/group
monkeys:x:502:monkeys
chicken:x:503:
turkey:x:504:monkeys

if I run the same play again, changing nothing, you can see that the current groups matched my requested groups, my group diff list is empty and changed: false

This is where @raffomania's error comes in. With the GID check in place monkeys would have been missing from the Current Groups list and when the set() is run would have shown up in the group diff and required processing.

# python user
Current Groups: ['monkeys', 'turkey']
Self.Groups: monkeys,turkey
Groups Set: set(['turkey', 'monkeys'])
Group Diff: set([])
{"comment": "", "shell": "/bin/nologin", "group": 502, "name": "monkeys", "changed": false, "state": "present", "groups": "monkeys,turkey", "home": "/home/monkeys", "move_home": false, "append": true, "uid": 498}
@core-a1 12:35:29 ~/.ansible/tmp/ansible-tmp-1430834351.8-88768220443752# tail -3 /etc/group
monkeys:x:502:monkeys
chicken:x:503:
turkey:x:504:monkeys

[msabramo]

I also ran into this and @blackreed9's change worked for me (on an Ubuntu 14.04 host), so 👍 from me.

[msabramo]

and here's the results of adding my own print statements for reference:

This is a case where I am trying to add the profilesvcuser user to the vagrant group and it already belongs to that group -- i.e.:

- name: "create application user"
  user: name=profilesvcuser
        state=present
        group=vagrant
        groups=vagrant

*** ansible.modules.core.system.user.User.user_group_membership:
    group = grp.struct_group(gr_name='vagrant', gr_passwd='x', gr_gid=1101, gr_mem=['vagrant', 'profilesvcuser'])
    info = ['profilesvcuser', 'x', 1002, 1101, '', '/home/profilesvcuser', '']
    info[3] = 1101
    group.gr_gid = 1101
    (not info[3] == group.gr_id) = False

[gundalow]

+1 for getting this fixed

[asmartin]

I'm also observing this when I use the password option (I am not using the group option at all) - the task is always shown as changed, even when the password remains the same. I am using it as follows:

 - name: Create users
   user: name={{ item.username }} password="{{ item.password }}"
   with_items:
    - { username: user1, password: "{{ user1_password|password_hash('sha512') }}" }
    - { username: user2, password: "{{ user2_password|password_hash('sha512') }}" }

[mkollaro]

This also happens for me when the password option is specified, without the group. Task is always shown as changed.

- name: Add user
  user: name=abc
        password={{ abc_password | password_hash('sha512') }}

[angystardust]

@mkollaro @asmartin i think this happen 'cause the generated hash is different every time (even if the string to hash remain the same). That's how the 'sha' algorithm works!
https://en.wikipedia.org/wiki/Secure_Hash_Algorithm

[mkollaro]

@angystardust I don't remember much from my crypto class, but I'm pretty sure that a basic property of any hash function is to generate the same result given the same input. Different inputs can result in the same hash, but not the other way around. https://en.wikipedia.org/wiki/Hash_function

[angystardust]

Sorry to contraddict you, @mkollaro but what you said is wrong.
You can run this simple test from a Linux host:

for n in seq 1 3 ; do echo password | mkpasswd -m sha-512 --stdin ; done

You'll have 3 different hashes from the same (not so much creative) password.

[bcoca]

mkpasswd adds a random salt which is what is modifying the hash, the hash should be reproducible when the salt used is the same.

[angystardust]

Gosh! Totally forget about 'salt', @bcoca. (in fact, i'm just using ansible instead of salt(stack) 😄 )
Sorry for the noise and I ask for forgiveness to @mkollaro 🙏

[yeroc]

+1, still seeing this with v2.0.1.0.

[tsaridas]

+1, same here.
its hard to believe that such basic module is buggy. blackreed9 change works for me too.
@blackreed9 maybe you can do a pr for this ? I could do it too but I'm not sure what breaks.

[jctanner]

Hi!

A change has been applied for this ticket, which should resolve this item for you.

If you believe this ticket is not resolved, or have further questions, please let us know by stopping by one of the two mailing lists, as appropriate:

• https://groups.google.com/forum/#!forum/ansible-project - for user questions, tips, and tricks
• https://groups.google.com/forum/#!forum/ansible-devel - for strategy, future planning, and questions about writing code

Because this project is very active, we're unlikely to see comments on closed tickets, though the mailing list is a great way to get involved or discuss this one further.

Thanks!