Menu should list users if more than 1, fewer than 7 are on the system

Bug #422052 reported by Ted Gould
68
This bug affects 11 people
Affects Status Importance Assigned to Milestone
Session Menu
Fix Released
High
Cody Russell
Ubuntu
Invalid
Undecided
Unassigned
Nominated for Karmic by Robert Pollak

Bug Description

It should in a submenu.

Blocked on GDM support of listing users over DBus.

Relevant portion of the spec:
https://wiki.ubuntu.com/DesktopTeam/Specs/KarmicFusa#(Fast)%20User%20Switching

Related branches

Ted Gould (ted)
Changed in indicator-session:
importance: Undecided → Medium
milestone: none → ubuntu-9.10-ui-freeze
status: New → Confirmed
Changed in indicator-session:
importance: Medium → Critical
assignee: nobody → Robert Ancell (robert-ancell)
importance: Critical → High
importance: High → Medium
Revision history for this message
Martin Pitt (pitti) wrote :

Does this really need to come from gdm? We can certainly add a d-bus call there, but why not just iterate over getpwent() and filter out UIDs < 500 (the system users) and >= 65534 (nobody)? That's more or less what gdm does as well.

In shell, you can do that with

  getent passwd | FS=: awk -F: '($3 >= 500 && $3 < 65534) { print }'

but in C, using getpwent() is more efficient.

Revision history for this message
Martin Pitt (pitti) wrote :

To be precise, gdm uses "ck-history --frequent", but that's not exactly more efficient, and not even what you need here.

Revision history for this message
Ted Gould (ted) wrote : Re: [Bug 422052] Re: Menu should list users if fewer than 6 are on the system

On Wed, 2009-09-02 at 14:46 +0000, Martin Pitt wrote:
> Does this really need to come from gdm? We can certainly add a d-bus
> call there, but why not just iterate over getpwent() and filter out UIDs
> < 500 (the system users) and >= 65534 (nobody)? That's more or less what
> gdm does as well.

GDM does other things like watch for changes, etc. So we wanted to have
one person doing that on the whole system instead of GDM doing it, the
greeter doing it, and every instance of indicator-session doing it.
There are also things like the user names that are blocked, and that
logic should really be in one place so that if you hide a user it gets
hidden everywhere instead of needing to copy the logic between the
different code bases.

Revision history for this message
Ted Gould (ted) wrote :

On Wed, 2009-09-02 at 15:04 +0000, Ted Gould wrote:
> On Wed, 2009-09-02 at 14:46 +0000, Martin Pitt wrote:
> > Does this really need to come from gdm? We can certainly add a d-bus
> > call there, but why not just iterate over getpwent() and filter out UIDs
> > < 500 (the system users) and >= 65534 (nobody)? That's more or less what
> > gdm does as well.
>
> GDM does other things like watch for changes, etc. So we wanted to have
> one person doing that on the whole system instead of GDM doing it, the
> greeter doing it, and every instance of indicator-session doing it.
> There are also things like the user names that are blocked, and that
> logic should really be in one place so that if you hide a user it gets
> hidden everywhere instead of needing to copy the logic between the
> different code bases.

Note also, I did discuss this with Ray Strode (upstream) at GCDS and he
agreed that it seemed like a good idea. That of course doesn't
guarantee that'd accept a patch, but it does mean that it is probably
possible.

Revision history for this message
Martin Pitt (pitti) wrote :

Ted Gould [2009-09-02 15:16 -0000]:
> Note also, I did discuss this with Ray Strode (upstream) at GCDS and he
> agreed that it seemed like a good idea. That of course doesn't
> guarantee that'd accept a patch, but it does mean that it is probably
> possible.

OK. However, I'm still not convinced to do it in gdm; gdm would need
to grow that functionality just for this external service, not for
itself. Also, this functionality is not related at all to GNOME.

So if something like this gets implemented (with notifications about
changes, etc.), then I believe ConsoleKit is a much better place for
it than gdm.

Revision history for this message
Ted Gould (ted) wrote :

On Wed, 2009-09-02 at 15:32 +0000, Martin Pitt wrote:
> Ted Gould [2009-09-02 15:16 -0000]:
> > Note also, I did discuss this with Ray Strode (upstream) at GCDS and he
> > agreed that it seemed like a good idea. That of course doesn't
> > guarantee that'd accept a patch, but it does mean that it is probably
> > possible.
>
> OK. However, I'm still not convinced to do it in gdm; gdm would need
> to grow that functionality just for this external service, not for
> itself. Also, this functionality is not related at all to GNOME.
>
> So if something like this gets implemented (with notifications about
> changes, etc.), then I believe ConsoleKit is a much better place for
> it than gdm.

ConsoleKit doing it would be fine with me, but the code to do it already
exists inside GDM. And GDM already does this for the greeter. So in
the GDM case it's more an issue of exporting the information over DBus
while in the CK case it would be adding the code/functionality.

Revision history for this message
Martin Pitt (pitti) wrote :

Ted Gould [2009-09-02 15:59 -0000]:
> ConsoleKit doing it would be fine with me, but the code to do it already
> exists inside GDM. And GDM already does this for the greeter.

Well, it's doing it with Robert's gdmsetup patch, it's not upstream
yet. upstream gdm uses ck-history --frequent which does something
slightly different.

> So in the GDM case it's more an issue of exporting the information
> over DBus while in the CK case it would be adding the
> code/functionality.

I don't want to dwell on it too much, but from an architectural point
of view it seems just wrong to me. You'd end up implementing
everything twice again, for gdm, for kdm, etc...

Martin

--
Martin Pitt | http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)

Revision history for this message
Rick Spencer (rick-rickspencer3) wrote : Re: Menu should list users if fewer than 6 are on the system

Please use pitti's suggested few lines of code to get the users list within the indicator-session code. If a different architectural model arises, such as an api call, we can switch to that later.

Changed in indicator-session:
assignee: Robert Ancell (robert-ancell) → Ted Gould (ted)
Revision history for this message
Ted Gould (ted) wrote : Re: [Bug 422052] Re: Menu should list users if fewer than 6 are on the system

On Wed, 2009-09-02 at 17:26 +0000, Rick Spencer wrote:
> Please use pitti's suggested few lines of code to get the users list
> within the indicator-session code. If a different architectural model
> arises, such as an api call, we can switch to that later.

Pitti's code won't do everything that's required. It's a small part of
the problem. Here's the code that's required for figuring out, the list
is users, where their icons are, whether they are currently logged in
and switching to them if they are selected:

http://git.gnome.org/cgit/gdm/tree/gui/simple-greeter/gdm-user-manager.c
  http://git.gnome.org/cgit/gdm/tree/gui/simple-greeter/gdm-user.c

There's simply a lot of cases to deal with. And I don't believe that
GDM code has any protections in place for large networks, which also
would need to be added.

I'm not in any way saying that this is impossible, but we shouldn't
minimize the problem.

Revision history for this message
Robert Ancell (robert-ancell) wrote : Re: Menu should list users if fewer than 6 are on the system

Request for changes to gdm are tracked in bug 423450

Revision history for this message
Robert Ancell (robert-ancell) wrote :

I see the work on serving the user information from one place as an optimisation. The code to monitor users is currently copied once for the greeter and once for the configuration applet. If there is a risk to the delivery of the session indicator can it copy the code again? Note that the d-bus interface will be very similar to the GdmUserManager GObject interface and a future replacement with a d-bus interface should be relatively simple.

Revision history for this message
Martin Pitt (pitti) wrote : Re: [Bug 422052] Re: Menu should list users if fewer than 6 are on the system

Hello Ted,

just some questions to understand the problem further.

Ted Gould [2009-09-02 18:04 -0000]:
>
> the list is users

OK, we talked about that.

> where their icons are

Shouldn't that be a file in the user's home directory? Or where are
they saved?

Anyway, we certainly don't need the icons in the session applet, for
they would be much too small anyway?

If we want to add icons, can we do that at a later step and not block
on that?

> whether they are currently logged in and switching to them if they
> are selected:

All the seat information (who/where/new display/display removed/etc.)
and opening a new login screen are already provided over D-BUS by gdm,
switching seat to an existing user is provided by ConsoleKit's D-BUS
API.

> And I don't believe that GDM code has any protections in place for
> large networks, which also would need to be added.

What do you mean, if there are too many users? (Isn't that why it says
"less than 6"?)

> I'm not in any way saying that this is impossible, but we shouldn't
> minimize the problem.

Well, we should minimize it in the sense of not providing duplicate
APIs which already exist in a better place IMHO, and even less so
block on getting those new APIs.

Since we begin to get under time pressure, I would like to understand
what the blockers are right now, since we I'd like us to concentrate
on resolving them as fast as possible (with Robert, or me).

Thanks!

Martin
--
Martin Pitt | http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)

Revision history for this message
Ted Gould (ted) wrote :

On Thu, 2009-09-03 at 07:17 +0000, Martin Pitt wrote:
> Ted Gould [2009-09-02 18:04 -0000]:
> > the list is users
>
> OK, we talked about that.

Yes. The list of non-users is also an issue. :) That's currently a
structure inside of GDM User Manager.

In the future, it'd probably be nice if packages could add to that
list... so the apache package could blacklist an apache user if it was
to create one. Off-topic, sorry :)

> > where their icons are
>
> Shouldn't that be a file in the user's home directory? Or where are
> they saved?

There is two places. The home directory and a global directory. I'm
not sure if anyone is using the /usr/share/faces anymore, but GDM does
support it. Also, we need to set up file watches on these to notice if
the icons change and/or get added.

> Anyway, we certainly don't need the icons in the session applet, for
> they would be much too small anyway?

Yes and no. I'm not sure if it's a requirement today, it seems to
appear and disappear from the specs :) We could probably remove it if
it was the only issue.

> > whether they are currently logged in and switching to them if they
> > are selected:
>
> All the seat information (who/where/new display/display removed/etc.)
> and opening a new login screen are already provided over D-BUS by gdm,
> switching seat to an existing user is provided by ConsoleKit's D-BUS
> API.

Yes, it is using CK. It's not a trivial task as you have to go through
all the seats and sessions, but that is possible, and already done in
the GDM User Manager.

> > And I don't believe that GDM code has any protections in place for
> > large networks, which also would need to be added.
>
> What do you mean, if there are too many users? (Isn't that why it says
> "less than 6"?)

Yes, but one of the cases here is dealing with that intelligently.
Things like getent don't really have ways of saying anything other than
going through the whole list, and they're not naturally asynchronous.
All solvable, but something that should be done once and handled by one
person in the system.

> > I'm not in any way saying that this is impossible, but we shouldn't
> > minimize the problem.
>
> Well, we should minimize it in the sense of not providing duplicate
> APIs which already exist in a better place IMHO, and even less so
> block on getting those new APIs.
>
> Since we begin to get under time pressure, I would like to understand
> what the blockers are right now, since we I'd like us to concentrate
> on resolving them as fast as possible (with Robert, or me).

Yes, of course. It's more a goal of removing code duplication. I don't
really care whether the interface/watchers/updaters are in ConsoleKit or
GDM (or whatever). I just know that today the code is already
implemented in GDM and I don't want to just copy it so that we have more
than one person going through all of this.

  --Ted

Revision history for this message
Martin Pitt (pitti) wrote :

Ted Gould [2009-09-03 14:02 -0000]:
> In the future, it'd probably be nice if packages could add to that
> list... so the apache package could blacklist an apache user if it was
> to create one. Off-topic, sorry :)

But if the apache package creates a non-system user, that's a serious
bug. System users like www-data should be in the system UID range
(100..499).

> There is two places. The home directory and a global directory. I'm
> not sure if anyone is using the /usr/share/faces anymore, but GDM does
> support it.

Ah, thanks for the heads-up.

> > What do you mean, if there are too many users? (Isn't that why it says
> > "less than 6"?)
>
> Yes, but one of the cases here is dealing with that intelligently.
> Things like getent don't really have ways of saying anything other than
> going through the whole list, and they're not naturally asynchronous.

You could just stop after reading 6 users?

> All solvable, but something that should be done once and handled by one
> person in the system.

Right, I wasn't saying that there shouldn't be such an interface, just
that creating one shouldn't be a blocker for this, since it won't work to
create such an API in a hurry. If it should make any sense, it needs
to be discussed with and accepted by upstream first, etc.

OK, thanks for the heads-up!

Revision history for this message
Ted Gould (ted) wrote :

On Thu, 2009-09-03 at 07:08 +0000, Robert Ancell wrote:
> I see the work on serving the user information from one place as an
> optimisation. The code to monitor users is currently copied once for
> the greeter and once for the configuration applet. If there is a risk
> to the delivery of the session indicator can it copy the code again?
> Note that the d-bus interface will be very similar to the GdmUserManager
> GObject interface and a future replacement with a d-bus interface should
> be relatively simple.

Yes, the question is timing. If you'll have it done in a couple of
days, I've got plenty to do so it's not worth doing twice. If the DBus
interface won't happen at all, I'd probably look towards copying the
code.

description: updated
summary: - Menu should list users if fewer than 6 are on the system
+ Menu should list users if more than 1, fewer than 7 are on the system
David Barth (dbarth)
Changed in indicator-session:
assignee: Ted Gould (ted) → Cody Russell (bratsche)
status: Confirmed → In Progress
David Barth (dbarth)
Changed in indicator-session:
milestone: ubuntu-9.10-ui-freeze → ubuntu-9.10-beta-freeze
Revision history for this message
Rick Spencer (rick-rickspencer3) wrote :

Let's get this done by beta freeze so we don't deliver with a functional regression

Changed in indicator-session:
importance: Medium → High
Cody Russell (bratsche)
Changed in indicator-session:
status: In Progress → Fix Committed
Ted Gould (ted)
Changed in indicator-session:
milestone: ubuntu-9.10-beta-freeze → 0.1.5
Ted Gould (ted)
Changed in indicator-session:
status: Fix Committed → Fix Released
Revision history for this message
André (andred-ubuntu-deactivatedaccount-deactivatedaccount) wrote :

Problem persists in Ubuntu 9.10 Release Candidate. I have two users on my system, and the other user is not listed in the indicator applet.

I propose that this bug should be reopened.

Revision history for this message
Vorik (launchpad-gerapeldoorn) wrote :

I have 5 users on my system (Karmic) and I cannot see any users on the menu.

The kids are complaining they cannot go to their session, because two of them can only read their name and to going through "Switch User" is too complicated if you cannot read.

Please fix this.. :)

Revision history for this message
Matt Goodall (matt-goodall) wrote :

No sign of the fix for me either, running official release with all updates from karmic-security, karmic-updates and karmic-proposed installed.

Revision history for this message
Ted Gould (ted) wrote : Re: [Bug 422052] Re: Menu should list users if more than 1, fewer than 7 are on the system

This was fixed but the patch got dropped for Karmic because it wasn't
stable enough and the fixes were too risky. We're sorry about the
regression and it will be fixed for Lucid.

Revision history for this message
Brad V (brad-visscher) wrote :

Is there a way to switch back to the old user switcher? I had no problems with the old one, and I can't wait six months to have this feature back. What can I do in the interim?

Revision history for this message
Ted Gould (ted) wrote :

On Wed, 2009-12-23 at 02:51 +0000, Brad Visscher wrote:
> Is there a way to switch back to the old user switcher?

No, GDM changed which changed all the interfaces to talk to it. The old
one will not work unless you use the old GDM, which would be tricky at
best. It would definitely be an untested setup.

Revision history for this message
Brad V (brad-visscher) wrote :

So you're telling me that there's no way to get this feature back? I've been using this feature since feisty, and my family relies heavily on it.

Switching back to the gdm screen and typing in the password again is not something I can expect my four-year-old to do, but selecting his name from a simple list is easy enough for him.

How can a regression like this be marked "Fix released", and no further attention paid to it?

Revision history for this message
David Tombs (dgtombs) wrote :

Invalidating in Ubuntu task as it already has an assigned package.

Changed in ubuntu:
status: New → Invalid
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.