timekpr-gui fails to start if login.defs is corrupt/empty

[Steve Dockar]

Running Ubuntu 9.10 Gnome 2.28.1 timekpr 0.3.0

Empty login.defs file causes timekpr-gui.py to fail. Debug information is shown if run from CLI, but no indication of the cause is displayed and the app simply appears not to start if called from the menu.

It's taken me a little while to track the cause of my timekpr-gui problem down, and, when I did track it down to an empty /etc/login.defs file, I'm now struggling to work out how that happened... But..

/usr/share/python-support/timekpr/timekpr-gui.py:97: GtkWarning: GtkSpinButton: setting an adjustment with non-zero page size is deprecated
  self.wTree = gtk.glade.XML(gladefile, 'mainwindow', APP_NAME)
Traceback (most recent call last):
  File "/usr/share/python-support/timekpr/timekpr-gui.py", line 587, in <module>
    timekprGUI()
  File "/usr/share/python-support/timekpr/timekpr-gui.py", line 151, in __init__
    if isnormal(userinfo[0]):
  File "/usr/share/python-support/timekpr/timekpr-gui.py", line 76, in isnormal
    if uidminmax[0] < uidminmax[1]:
IndexError: list index out of range

If "uidminmax = re.compile('^UID_(?:MIN|MAX)\s+(\d+)', re.M).findall(logindefs.read())" in "isnormal()" can't find UID_MIN and MAX, uidminmax[n] aren't populated and the Index out of range error causes the app to fail. This is understandable, and not too difficult to figure out "why", but if the app is being used by a Linux/Python novice, the app just failing to start is all they will "see" and they will just assume it is "buggy".

login.defs shouldn't be empty, but if it is, the app should either work around the missing values or, fail gracefully with an error message or dialog.

[Savvas Radevic (medigeek)]

(@Even, your opinion please :) )

Steve, thanks for the bug report! are you using some other way to manage users, like libuser / libuser.conf ?

We will keep the current login.defs check, but as a default fallback option there are two solutions:
a) list all the users, real and system ones (and hence isnormal() will just return True on each call)
b) add default UID_MIN / MAX variables inside isnormal() -- but which values as default?
Do you happen to know the default value of UID_MIN / UID_MAX if they are not set in login.defs ?

I'm not really sure which one to go with though... :) I like (a), dirty, but less to tamper with

[Even Nedberg (nedberg)]

I like a) + a pop up a warning that something might be wrong, and suggesting that the admin uses caution not to lock out an unwanted user.

UID_MIN/MAX might be different on other distros so I don't like that solution.

[Steve Dockar (steve-dockar)]

Hi Savvas,

Thanks for the prompt reply :-) No, I'm not using anything else to manage users, all standard Karmic I'm afraid. I don't know if something else I've tinkered with had opened the file RW and then written an empty file in its place. I'm keeping an eye on that side of the problem. I also discovered when I tried to check the current UID values that the Ubuntu user-admin GUI seems to rely on the MIN/MAX values to populate its list, as mine showed only the root user/account until I fixed the login.defs file. There were one or two threads on ubuntuforums with users seeing the same behaviour, so maybe they too had corrupt loing.defs?

I did some checking, and it seems that RedHat derived systems start their normal users at 500 whereas Ubuntu, so I'm guessing Debian starts at 1000. I agree with Even, if you build in a list of distro defaults, with warnings when you can't determine for sure what the real values should be, it becomes very messy. If the user-base is likely to include non-expert users, it needs to be as simple and consistent as possible.

I would have settled for a "Sorry, there's a problem with your login.defs file and we can't continue" message, but I think a "Sorry there's a problem working out your users accounts so I'm going to list all accounts. Please take care not to restrict the wrong one (click for more details)" would be nicer. Then, if the user clicks the link you could tell them what you were trying to read, and from where so they could look into fixing it.

[Savvas Radevic (medigeek)]

in trunk, after a couple of adjustments (common.popup and split up reading login.defs to a separate function).
It's not tested per se, due to heavy changes, but I am 90% sure it works.

def read_uid_minmax(f=dirs.LOGIN_DEFS):
    # NOTE: If problem with login.defs or variables, show all (system and normal) users -- bug #529770
    try:
        logindefs = open(f)
    except IOError:
        common.popup(_("timekpr administration warning"), _("Could not open file %(file)s -- cannot distinguish normal users from system users.\nAll users will be shown.") % {'file': f})
        return ("ERROR", "ERROR")

    uidminmax = re.compile('^UID_(?:MIN|MAX)\s+(\d+)', re.M).findall(logindefs.read())
    # Check if uidminmax array has less than 2 items
    # If less, return "ERROR". Show all users, system and normal users. Show a warning popup
    if len(uidminmax) < 2:
        # Two cases included here, either missing UID_MIN or UID_MAX, or both missing.
        common.popup(_("timekpr administration warning"), _("Missing UID_MIN / UID_MAX variables -- Cannot distinguish normal users from system users.\nAll users will be shown."))
        return ("ERROR", "ERROR")

    else:
        if uidminmax[0] < uidminmax[1]:
            uidmin = int(uidminmax[0])
            uidmax = int(uidminmax[1])
        else:
            uidmin = int(uidminmax[1])
            uidmax = int(uidminmax[0])
        return (uidmin, uidmax)