Bug 185097 - system-config-users doesn't display shadow information from an ldap directory
Summary: system-config-users doesn't display shadow information from an ldap directory
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Fedora
Classification: Fedora
Component: system-config-users
Version: 7
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nils Philippsen
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2006-03-10 16:42 UTC by Ed van Gasteren
Modified: 2008-06-17 01:11 UTC (History)
3 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2008-06-17 01:11:33 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
combined patch as mentioned in comment #14 (2.15 KB, patch)
2006-03-19 15:37 UTC, Ed van Gasteren
no flags Details | Diff

Description Ed van Gasteren 2006-03-10 16:42:07 UTC
Description of problem:

I have configured /etc/libuser.conf such that new users are created in my ldap
directory (as apposed to in /etc/passwd /etc/shadow ...). If I then create a new
user then indeed all the information, including the shadowAccount objectclass
and all its attributes, end up in the ldap directory.

However when using system-config-users to display all the information about such
a user (from the ldap directory), the shadow information is NOT presented. When
you display all the information about a user which is in the /etc files, the
shadow information is presented.

Version-Release number of selected component (if applicable):

system-config-users-1.2.41-0.fc4.1

How reproducible:

Use system-config-users to have look at users. Pick one that is in the /etc
files. Inspect its properties and note that it does have the "Password Info"
tab. Pick a user that is not in /etc files but in the ldap directory. Inspect
its properties and note that it does NOT have the "Password Info" tab.

Steps to Reproduce:

See above.
  
Actual results:

"Password Info" tab missing.

Expected results:

"Password Info" tab shown.

Additional info:

Obviously in order to test this you need to have an ldap directory setup and
have libuser configured for it.

I took a peek at userProperties.py and discovered that indeed the code doesn't
look for shadow information in the the ldap directory when it should. There are
just 2 places where this is wrong and the fix is easy:

diff userProperties.py userProperties.py.org
208c208
<         if "shadow" in self.userEnt.modules() or "ldap" in self.userEnt.modules():
---
>         if "shadow" in self.userEnt.modules():
251c251
<         if "shadow" not in self.userEnt.modules() and "ldap" not in
self.userEnt.modules():
---
>         if "shadow" not in self.userEnt.modules():

Comment 1 Nils Philippsen 2006-03-10 16:50:56 UTC
Mirek, if someone uses the ldap backend, can I rely on shadow information being
available from libuser?

Comment 2 Ed van Gasteren 2006-03-10 18:00:24 UTC
My experience is that if the objectclass and the attributes are there then:
yes!, the ldap module of libuser will pick it up and pass it on up to the python
level.

I have experimented with ldap users with and without shadow stuff. Obviously for
users without, libuser can't make it available.


Comment 3 Nils Philippsen 2006-03-10 20:06:37 UTC
May question was rather: Can (or how can) I easily determine whether shadow info
is available from ldap, I wanted to avoid messing up the code with try ...
except constructs. That's why I Cc'ed Miloslav Trmac who maintains libuser.

Comment 4 Miloslav Trmač 2006-03-11 00:19:16 UTC
You can't determine it globally, each LDAP entry can have a different set of
objectclasses, and all attributes in the shadow objectclass are optional, so
this should really be done per-entity, per-attribute.  AFAICS the original
patch would thus cause exceptions when editing an LDAP entry without some
or all of the shadow attributes.

As for try...except, instead of
  entity[attrname]
which raises KeyError on nonexistent attributes, you can use
  entity.get(attrname)
which returns [], or even
  entity.get(attrname, defaultValue)

Adding LDAP attributes that didn't exist should work, but it is not possible
to change the structural class of the LDAP entry.  This should only be
a problem if you try to modify fields that don't exist in /etc/passwd
or /etc/shadow, such as given name/surname, room number, phone numbers.

Comment 5 Ed van Gasteren 2006-03-11 14:48:59 UTC
Indeed I have seen such exceptions while experimenting. With defaultValue
argument to entity.get they can be avoided. We should also set the same default
values in the initial dialoog. This patch does both:

diff userProperties.py userProperties.py.org
158,160c158,160
<         self.pwRequireEntry.set_text('99999')
<         self.pwWarnEntry.set_text('7')
<         self.pwInactiveEntry.set_text('-1')
---
>         self.pwRequireEntry.set_text('0')
>         self.pwWarnEntry.set_text('0')
>         self.pwInactiveEntry.set_text('0')
208c208
<         if "shadow" in self.userEnt.modules() or "ldap" in self.userEnt.modules():
---
>         if "shadow" in self.userEnt.modules():
209a210,213
>             min = self.userEnt.get(libuser.SHADOWMIN)
>             max = self.userEnt.get(libuser.SHADOWMAX)
>             warning = self.userEnt.get(libuser.SHADOWWARNING)
>             inactive = self.userEnt.get(libuser.SHADOWINACTIVE)
227,231d230
<             min = self.userEnt.get(libuser.SHADOWMIN, [0])
<             max = self.userEnt.get(libuser.SHADOWMAX, [99999])
<             warning = self.userEnt.get(libuser.SHADOWWARNING, [7])
<             inactive = self.userEnt.get(libuser.SHADOWINACTIVE, [-1])
<
252c251
<         if "shadow" not in self.userEnt.modules() and "ldap" not in
self.userEnt.modules():
---
>         if "shadow" not in self.userEnt.modules():

By the way, nasty tester as I am, I also entered non numeric values for e.g.
"warning". That isn't handled "gracefully". But that is not the issue at hand.


Comment 6 Nils Philippsen 2006-03-14 09:50:58 UTC
Mirek, do I read you correctly that libuser provides no means to convert
non-shadow to shadow accounts and vice versa? Fair enough -- shall I file an RFE
;-)? Honestly, I think converting to and from shadow accounts should be made
accessible in s-c-users. Tomas, could authconfig/system-config-authentication
provide controls for that? Besides, can we convert between non-shadow/shadow in
LDAP with s-c-auth?

Ed, please in future produce unified diffs ("diff -u", they're much more
readable) and attach them to the bug instead of pasting them wholesale into the
comments. And please open a separate bug for the non-numerical stuff not being
handled gracefully, thanks.

Comment 7 Miloslav Trmač 2006-03-14 21:16:04 UTC
No, adding/removing the shadow attributes should work fine.  The trouble is with
adding the extra fields mentioned in comment #4, that under some conditions can't
be done in LDAP except by creating a new entry and deleting the original one

Comment 8 Nils Philippsen 2006-03-14 21:56:47 UTC
You mean that converting between posixAccount and shadowAccount(?) objectclasses
isn't possible, right?

Comment 9 Miloslav Trmač 2006-03-14 22:05:07 UTC
No.  The trouble is that neither "posixAccount" nor "shadowAccount" is a
structural objectclass, so the LDAP entry must have one of the "inetOrgPerson" and
"account" objectlasses, and converting between them (e.g. to add the "givenName"
attribute) isn't possible.

All attributes in "shadowAccount" but "uid" are optional, so an entry may have
the "shadowAccount" class without having any shadow data.

Comment 10 Nils Philippsen 2006-03-15 13:56:17 UTC
Do the differences between inetOrgPerson and account objectclasses have any
relevance in the realm of OS account management (i.e. should I bother that one
can't convert between the two)?

Comment 11 Miloslav Trmač 2006-03-15 13:58:03 UTC
AFAIK they don't if you are limited to the fields stored in traditional
/etc/passwd and /etc/shadow files.

Comment 12 Nils Philippsen 2006-03-15 16:02:14 UTC
I guess I am (unless I want to extend s-c-users in that direction, which I
don't). Is there anything else I need to know, for instance can I just write the
shadow attributes on a non-shadow account and they will "magically" appear/that
object will magically become a "shadowAccount" one if it has only been a
"posixAccount" before?

Comment 13 Miloslav Trmač 2006-03-15 16:05:14 UTC
Yes, that should just work.

Comment 14 Ed van Gasteren 2006-03-19 15:35:12 UTC
In summary I think all this means:

1) the orginal problem gets solved by the first proposed patch.

2) in case other tools have been used to create "accounts" in the ldap directory
and those tools did not put the shadow stuff in properly, then (depending on
what was missing) we could get exceptions. We can get around those with the
second proposed patch (which includes the first). s-c-users sanitizes the shadow
mess of those other tools.

Although strictly speeking this bug is about 1) only, I propose to apply the
combined patch. That will make s-c-users more robust to handle the "incorrect"
shadow stuff of other tools.



Comment 15 Ed van Gasteren 2006-03-19 15:37:23 UTC
Created attachment 126324 [details]
combined patch as mentioned in comment #14

Comment 16 Nils Philippsen 2006-04-25 07:55:24 UTC
I plan to apply this in the FC6 version.

Comment 17 Ed van Gasteren 2006-11-28 20:23:01 UTC
system-config-users-1.2.47-1.fc6 still has this problem.

Comment 18 Christian Iseli 2007-01-20 00:49:40 UTC
This report targets the FC3 or FC4 products, which have now been EOL'd.

Could you please check that it still applies to a current Fedora release, and
either update the target product or close it ?

Thanks.

Comment 19 Ed van Gasteren 2007-01-20 12:09:19 UTC
Comment 18 requests to check if this bug is still present in the current Fedora
release. I beleave the current release to be FC6. If that is indeed the case
then this question was already answered before it was asked, in comment 17.

Comment 18 further requests to either update the target product or close it.
Well since this bug is still not fixed in the current realease, I think it is
inappropriate to close it.

The request to update the target is in a sense a "procedure" issue. When
updating a component, do we check for bugs on previous versions? If so then it
is better to  keep the bug registered on the original version of the component.
Otherwise we end up updating that kind of stuff with every new version of EOL of
an old version.

Comment 20 Nils Philippsen 2007-11-07 13:47:27 UTC
I've applied your changes, but modified them a bit to define defaults at one
place instead of scattered all over the file.

It would be great if you could check out the Mercurial repo from
http://hg.fedoraproject.org/hg/hosted/system-config-users and see whether this
fixes the issue for you as I don't have an LDAP directory handy/configured with
which to test this.

As these changes require translations to be updated, I don't plan to issue fixed
packages for FC6 -- translations won't be updated in time before it is EOLed.
Therefore I'll move this bug to F7.

Comment 21 Nils Philippsen 2007-11-07 13:51:16 UTC
NB: The command to check out the source code is:

hg clone http://hg.fedoraproject.org/hg/hosted/system-config-users

This will check out things into a (new) directory system-config-users in the
current directory.

Comment 22 Ed van Gasteren 2007-12-02 11:01:01 UTC
Checked it out. Reviewed the code and compared it with the F8 version. Looks good.

On my F8 test system I then moved your version of userProperties.py in place of
the installed one. I checked a few accounts that are only to be found in my ldap
directory. Looks good, no problems found.

So this solves my original problem.


The way you have dealt with the defaults, by defining pwAllowDefault = 0 and the
 consistently using self.pwAllowDefault, is indeed a very good way of handling
it. But I do have one remaining question about defining the values in the source
code: shouldn't we be getting them from /etc/libuser.conf (somehow by means of
the libuser API) instead?

Comment 23 Miloslav Trmač 2007-12-02 12:38:24 UTC
(In reply to comment #22)
> But I do have one remaining question about defining the values in the source
> code: shouldn't we be getting them from /etc/libuser.conf (somehow by means of
> the libuser API) instead?
  user = admin.initUser('this_can_be_anything')
and then access the attributes of the user.

Comment 24 Nils Philippsen 2007-12-03 09:28:58 UTC
(In reply to comment #23)
>   user = admin.initUser('this_can_be_anything')
> and then access the attributes of the user.

Mirek, could I "admin.InitUser (None)" to get an "anonymous" user? If I just use
an improbable user name someone will complain that this won't work because they
created such a user ;-).



Comment 25 Miloslav Trmač 2007-12-04 18:34:17 UTC
(In reply to comment #24)
> Mirek, could I "admin.InitUser (None)" to get an "anonymous" user? If I just use
> an improbable user name someone will complain that this won't work because they
> created such a user ;-).
That's not a problem, initUser doesn't fail if the user name is already used. 
The check is performed only before actually creating the user.

Comment 26 Nils Philippsen 2007-12-05 14:33:43 UTC
I've implemented this upstream and a package for Rawhide is building at the
moment. I'll postpone building this for F7/F8 until the translations are in
order again.

It'd be great if you would try this out, either from the Mercurial repo or the
Rawhide package.

Comment 27 Ed van Gasteren 2007-12-11 19:28:48 UTC
Checked it out. Reviewed the code and compared it with the previous version.
Again, looks good.

On my F8 test system I then moved your version of userProperties.py in place of
the previous one. I changed some values in /etc/userlib.conf. Those clearly show
up in the user properties dialoog.

So far so good! 

I haven't checked any of "logic" (code flow, results) because I expect that to
have remained the same.


Comment 28 Bug Zapper 2008-05-14 11:59:24 UTC
This message is a reminder that Fedora 7 is nearing the end of life. Approximately 30 (thirty) days from now Fedora will stop maintaining and issuing updates for Fedora 7. It is Fedora's policy to close all bug reports from releases that are no longer maintained. At that time this bug will be closed as WONTFIX if it remains open with a Fedora 'version' of '7'.

Package Maintainer: If you wish for this bug to remain open because you plan to fix it in a currently maintained version, simply change the 'version' to a later Fedora version prior to Fedora 7's end of life.

Bug Reporter: Thank you for reporting this issue and we are sorry that we may not be able to fix it before Fedora 7 is end of life. If you would still like to see this bug fixed and are able to reproduce it against a later version of Fedora please change the 'version' of this bug. If you are unable to change the version, please add a comment here and someone will do it for you.

Although we aim to fix as many bugs as possible during every release's lifetime, sometimes those efforts are overtaken by events. Often a more recent Fedora release includes newer upstream software that fixes bugs or makes them obsolete. If possible, it is recommended that you try the newest available Fedora distribution to see if your bug still exists.

Please read the Release Notes for the newest Fedora distribution to make sure it will meet your needs:
http://docs.fedoraproject.org/release-notes/

The process we are following is described here: http://fedoraproject.org/wiki/BugZappers/HouseKeeping

Comment 29 Bug Zapper 2008-06-17 01:11:32 UTC
Fedora 7 changed to end-of-life (EOL) status on June 13, 2008. 
Fedora 7 is no longer maintained, which means that it will not 
receive any further security or bug fix updates. As a result we 
are closing this bug. 

If you can reproduce this bug against a currently maintained version 
of Fedora please feel free to reopen this bug against that version.

Thank you for reporting this bug and we are sorry it could not be fixed.


Note You need to log in before you can comment on or make changes to this bug.