Bug 217934 - /etc/pw.conf is being ignored by pw
Summary: /etc/pw.conf is being ignored by pw
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 10.3-RELEASE
Hardware: Any Any
: --- Affects Some People
Assignee: Eugene Grosbein
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2017-03-20 02:48 UTC by Victor Sudakov
Modified: 2017-05-11 13:33 UTC (History)
3 users (show)

See Also:
eugen: mfc-stable11?
eugen: mfc-stable10?


Attachments
respect defaultgroup specified in pw.conf (449 bytes, patch)
2017-03-20 10:51 UTC, Eugene Grosbein
no flags Details | Diff
Proposed fix (1.08 KB, patch)
2017-03-20 13:19 UTC, Eugene Grosbein
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Victor Sudakov 2017-03-20 02:48:51 UTC
root@km:~ # grep ^defaultgroup /etc/pw.conf
defaultgroup = "user"
root@km:~ # pw groupshow user
user:*:1000:
root@km:~ # pw useradd kabanov
root@km:~ # id kabanov
uid=1141(kabanov) gid=1141(kabanov) groups=1141(kabanov)
root@km:~ # freebsd-version
10.3-RELEASE-p17
root@km:~ #

Expected behavior: kabanov's primary group should be "user".

Other settings (e.g. defaultshell) are being ignored too.
Comment 1 Mikhail Timofeev 2017-03-20 03:59:06 UTC
reproducible on 11.0-STABLE r312369
Comment 2 Eugene Grosbein freebsd_committer freebsd_triage 2017-03-20 10:51:02 UTC
Created attachment 180987 [details]
respect defaultgroup specified in pw.conf

Proposed fix: respect defaultgroup specified in pw.conf
Comment 3 Victor Sudakov 2017-03-20 12:31:01 UTC
(In reply to Eugene Grosbein from comment #2)

Eugene, in 10.3 pw.conf seems to be totally ignored, not only the defaultgroup setting. ktrace/kdump does not show pw.conf being opened at all.
Comment 4 Eugene Grosbein freebsd_committer freebsd_triage 2017-03-20 13:19:31 UTC
Created attachment 180993 [details]
Proposed fix

Properly initialize cfg variable. Respect defaultgroup if specified in pw.conf.
Comment 5 Eugene Grosbein freebsd_committer freebsd_triage 2017-03-20 13:21:20 UTC
(In reply to vas from comment #3)

Please try new patch. It adds some needed initialisation to variables. Tested with 10.3-STABLE and 11.0-STABLE.
Comment 6 Baptiste Daroussin freebsd_committer freebsd_triage 2017-03-24 09:50:16 UTC
Can you also make a regression test when the refactoring of pw happened we added plenty of regression tests, would be nice to have a regression test for this case.
Comment 7 Eugene Grosbein freebsd_committer freebsd_triage 2017-03-24 11:04:00 UTC
(In reply to Baptiste Daroussin from comment #6)

Where can I read about how to do that?
Comment 8 Baptiste Daroussin freebsd_committer freebsd_triage 2017-03-24 12:31:36 UTC
usr.sbin/pw/tests you will have all the tests.

make install will install them in /usr/tests/usr.sbin/pw

you would need to install kyua from ports to execute them and access to the documentation: kyua test in that directory would run the tests
man 3 atf-sh will give you a bit of documentation.

I'm sorry there are not much more documentation, if that is too painful to make a test I will make a test myself afterward.

Btw you patch looks good to me so you can anyway consider it approved :)
Comment 9 commit-hook freebsd_committer freebsd_triage 2017-03-24 16:19:20 UTC
A commit references this bug:

Author: eugen
Date: Fri Mar 24 16:18:58 UTC 2017
New revision: 315912
URL: https://svnweb.freebsd.org/changeset/base/315912

Log:
  Properly initialise with content of pw.conf(5) that was mistakenly ignored.
  Also, respect "defaultgroup" if specified there.

  PR:		217934
  Reported by:	Victor Sudakov <vas@mpeks.tomsk.su>
  Reviewed by:	bapt
  Approved by:	bapt, vsevolod (mentor)
  MFC after:	1 week

Changes:
  head/usr.sbin/pw/pw_user.c
Comment 10 commit-hook freebsd_committer freebsd_triage 2017-03-25 10:48:46 UTC
A commit references this bug:

Author: bapt
Date: Sat Mar 25 10:47:58 UTC 2017
New revision: 315935
URL: https://svnweb.freebsd.org/changeset/base/315935

Log:
  Add a regression test for r31512 fix

  PR:		217934
  MFC after:	1 week

Changes:
  head/usr.sbin/pw/tests/pw_useradd_test.sh
Comment 11 Victor Sudakov 2017-03-27 08:49:35 UTC
Yes, the proposed fix works on 10.3, thanks!
Comment 12 Eugene Grosbein freebsd_committer freebsd_triage 2017-03-27 09:34:47 UTC
Take it as I've commited a fix.
Comment 13 Eugene Grosbein freebsd_committer freebsd_triage 2017-03-27 12:41:20 UTC
(In reply to Baptiste Daroussin from comment #8)

Another bug was reported to me in local IRC channel: pw usedmod username -G ''
is broken too. It it supposed to remove username from all supplementary groups but does nothing.

Please take a look on another patch fixing this: http://www.grosbein.net/freebsd/patches/pw-G.diff
Comment 14 Victor Sudakov 2017-03-27 13:11:54 UTC
Gentlemen,

While we are at it :-) Could someone please add "-Y" to "pw lock"?
Comment 15 Baptiste Daroussin freebsd_committer freebsd_triage 2017-03-27 13:21:17 UTC
(In reply to Eugene Grosbein from comment #13)
Looks good to me, maybe this time you can add a regression test?

I have added one for the previous change https://svnweb.freebsd.org/changeset/base/315935 if you want an example :)
Comment 16 Baptiste Daroussin freebsd_committer freebsd_triage 2017-03-27 13:22:23 UTC
(In reply to vas from comment #14)

forcing us to use/test NIS in 2017 you are being hard with us :)
Comment 17 Victor Sudakov 2017-03-27 15:18:49 UTC
(In reply to Baptiste Daroussin from comment #16)
There is still no viable alternative to NIS in FreeBSD, even in 2017.

"-Y" is supported anyway in all other modes of pw :-)
Comment 18 commit-hook freebsd_committer freebsd_triage 2017-04-01 09:27:32 UTC
A commit references this bug:

Author: bapt
Date: Sat Apr  1 09:27:00 UTC 2017
New revision: 316347
URL: https://svnweb.freebsd.org/changeset/base/316347

Log:
  MFC: r315912 (by eugen@) and r315935

  Properly initialise with content of pw.conf(5) that was mistakenly ignored.
  Also, respect "defaultgroup" if specified there.

  Add a regression test

  PR:		217934
  Reported by:	Victor Sudakov <vas@mpeks.tomsk.su>

Changes:
_U  stable/11/
  stable/11/usr.sbin/pw/pw_user.c
  stable/11/usr.sbin/pw/tests/pw_useradd_test.sh
Comment 19 commit-hook freebsd_committer freebsd_triage 2017-04-01 09:30:36 UTC
A commit references this bug:

Author: bapt
Date: Sat Apr  1 09:29:47 UTC 2017
New revision: 316348
URL: https://svnweb.freebsd.org/changeset/base/316348

Log:
  MFC: r315912 (by eugen@) and r315935

  Properly initialise with content of pw.conf(5) that was mistakenly ignored.
  Also, respect "defaultgroup" if specified there.

  Add a regression test

  PR:		217934
  Submitted by:	Victor Sudakov <vas@mpeks.tomsk.su>

Changes:
_U  stable/10/
  stable/10/usr.sbin/pw/pw_user.c
  stable/10/usr.sbin/pw/tests/pw_useradd_test.sh
Comment 20 Eugene Grosbein freebsd_committer freebsd_triage 2017-05-10 17:04:10 UTC
Fix committed and MFC'd.

To submitter: please fill another PR for "pw lock -Y" if you still need it.
If possible, try to prepare a fix and test it because NIS environment a bit specific.
Comment 21 Victor Sudakov 2017-05-11 05:45:57 UTC
(In reply to Eugene Grosbein from comment #20)

> To submitter: please fill another PR for "pw lock -Y"

There is one already: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=87529
Yes, I still need it.