Prevent user.present to change uid and gid of existing user

[mitar]

Description of Issue/Question

I would prefer if user.present would throw an error instead of trying to change uid and gid of an existing user. Currently, if an user already exists, it changes uid and gid values to new values. This might leave some files on the system previously belonging to the user with old permissions.

Versions Report

Salt Version:
           Salt: 2016.11.1
 
Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.9.4
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.8
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
         pygit2: Not Installed
         Python: 2.7.10 (default, Jul 14 2015, 19:46:27)
   python-gnupg: 0.3.8
         PyYAML: 3.12
          PyZMQ: 16.0.2
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.4.2
            ZMQ: 4.1.5
 
System Versions:
           dist:   
        machine: x86_64
        release: 14.5.0
         system: Darwin
        version: 10.10.5 x86_64

[Ch3LL]

@mitar can you clarify your use case a little more. Can you share the state your using? Just need to clarify if you have set a UID/GID in your state or not? in other words Is it changing the UID of a user without even declaring a UID in the state?

[mitar]

No, I have an explicit state with uid:

user-mitar:
  user.present:
    - name: mitar
    - uid: 2000
    - gid: users
    - empty_password: True
    - remove_groups: True
    - require:
      - cmd: user-uid-match-mitar

I am currently using user-uid-match-mitar workaround to manually check that uid matches.

user-uid-match-mitar:
  cmd.run:
    - name: |
        ! id -u mitar || [ "$(id -u mitar)" = "2000" ]

[noelmcloughlin]

I too have troubles with user.present and group.present. The objective is to ensure user/group is present (create locally if necessary), but salt throws ERRORS.

[WARNING ] Group "oracle" specified in both groups and optional_groups for user oracle
[ERROR   ] Command '['usermod', '-g', 'oracle', 'oracle']' failed with return code: 6
[ERROR   ] output: usermod: user 'oracle' does not exist in /etc/passwd
[ERROR   ] Command '['usermod', '-u', '501', 'oracle']' failed with return code: 6
[ERROR   ] output: usermod: user 'oracle' does not exist in /etc/passwd
[ERROR   ] These values could not be changed: {'gid': 'oracle', 'uid': 501}

User and/or group may already exist.
Note: The authentication database can be aggregation of local/remote databases ... NSS (name switch service) and PAM (pluggable authentication modules) provide the abstractions.

$ net ads testjoin
Join is OK
$ getent passwd oracle
oracle:*:16779755:16777216:oracle:/home/oracle:/bin/bash
$ getent group oracle
oracle:x:501: 

$ head /etc/nsswitch.conf
# /etc/nsswitch.conf
#
# Example configuration of GNU Name Service Switch functionality.
# If you have the `glibc-doc-reference' and `info' packages installed, try:
# `info libc "Name Service Switch"' for information about this file.

passwd:         compat winbind 
group:          compat winbind 

I am asking for any workaround at users formula issue.

[noelmcloughlin]

@gtmanfred are you aware of some general issues with user/group present/absent ?

[Ch3LL]

@noelmcloughlin your issue seems to be unrelated to this issue. Would you mind opening a new one?

Back to the original issue:

I would say that this would be considered expected behavior since as an admin you can change the uid/gid of user as well, but i want to ping @terminalmage here as well to see if he agrees. Or maybe we can add an option possibly that ensures we don't change the gid/uid if user changes it in the state.

[terminalmage]

@Ch3LL I think it's a good idea to gate a change like this behind an argument.

[noelmcloughlin]

@Ch3LL Thanks. Opened new issue: #46361

[terminalmage]

#46502 fixes this, PR message includes a docker container to confirm the new behavior, and new unit tests have been added.

[terminalmage]

@mitar #46502 has been merged, so I will mark this as closed. 2017.7.5 and 2018.3.0 will have these changes included.