GNOME Bugzilla – Bug 671156
Need to support network login/ kerberos better
Last modified: 2012-10-22 14:16:01 UTC
Right now the accounts dialog is very local-login, user-as-admin, centric. There's a proposal here: https://live.gnome.org/Design/Proposals/UserIdentities to make it work better in a managed login setup.
Created attachment 208803 [details] [review] user-accounts: port from GtkTable to GtkGrid GtkTable is deprecated and no longer encouraged. This commit changes the user panel over to gtkgrid instead.
Created attachment 208804 [details] [review] user-accounts: add "Other Passwords" button Since the user can change the login password from the user panel, it stands to reason the user may want to be able to adjust other passwords from there as well. This commit adds an "Other Passwords" button which runs seahorse when clicked. See: https://live.gnome.org/Design/Proposals/UserIdentities for more information.
Created attachment 208805 [details] [review] user-accounts: hide admin tasks from non-administrators If the user isn't an administrator (either by account type, or through polkit escalation), then don't show administrative tasks in the panel like editing other users, setting up autologin, etc.
Created attachment 208806 [details] [review] user-accounts: Add identity manager interface The page here: https://live.gnome.org/Design/Proposals/UserIdentities proposes adding a list of "accessible realms" to the user panel. To implement that we'll need to get a list of realms to show. This commit adds the basics of the required interface. A subsequent commit will had a concrete implementation based on kerberos. At some point, we may add an SSSD based implementation, too, which is why there's the abstraction.
Created attachment 208807 [details] [review] user-accounts: add alarm class When we support showing kerberos realms in the UI, we're going to need some way to get notified when the users credentials are no longer valid for those realms. This commit adds an alarm class that abstracts timerfd so we can get proper notification when kerberos credentials expire.
Created attachment 208808 [details] [review] user-accounts: add kerberos implemention of identity manager interface This commits adds a concrete implementation of the identity manager inteface using the krb5 libraries. In the future we may add a different interface that leverages SSSD.
Created attachment 208809 [details] [review] user-accounts: Use identity manager in user panel This commit hooks up the identity manager to the actual UI. It creates a list of currently "accessible realms" and provides buttons for signing out of those realms. This is outlined here: https://live.gnome.org/Design/Proposals/UserIdentities
This first round of patches makes some of the changes mentioned in that wiki page.
Review of attachment 208803 [details] [review]: Looks good (if it works obviously). Could you run it through glade once to verify that the delta isn't too big. Fine to commit after that. ::: panels/user-accounts/data/user-accounts-dialog.ui @@ +134,2 @@ <property name="column_spacing">10</property> + <property name="column_homogeneous">TRUE</property> Are those hand-made changes? s/TRUE/true/
Review of attachment 208804 [details] [review]: Looks good.
Review of attachment 208805 [details] [review]: Looks fine.
Review of attachment 208806 [details] [review]: Fine to commit after the changes. ::: panels/user-accounts/um-identity-manager.c @@ +94,3 @@ + gpointer user_data) +{ + UM_IDENTITY_MANAGER_GET_IFACE (self)->list_identities (self, NULL checks for all the iface calls. ::: panels/user-accounts/um-identity.c @@ +48,3 @@ +um_identity_get_identifier (UmIdentity *self) +{ + return UM_IDENTITY_GET_IFACE (self)->get_identifier (self); I would g_return_val_if_fail() if self->get_identifier was NULL. Programmer error. @@ +54,3 @@ +um_identity_is_signed_in (UmIdentity *self) +{ + return UM_IDENTITY_GET_IFACE (self)->is_signed_in (self); Ditto.
Review of attachment 208807 [details] [review]: Unix Goblins. I'll assume you didn't copy paste any mistakes ;) Please add a bug reference for de-duping this and the libgnome-desktop code from Colin.
Review of attachment 208809 [details] [review]: Fine after those few changes. ::: panels/user-accounts/um-user-panel.c @@ +1599,3 @@ +on_identity_added (UmIdentityManager *manager, + UmIdentity *identity, +{ s/gpointer user_data/UmUserPanelPrivate/ less code, and just as safe. @@ +1611,3 @@ + gpointer user_data) +{ + (GAsyncReadyCallback) ditto, etc. @@ +1695,3 @@ + + node = identities; +realm_free (Realm *realm) I'd rather have a for loop here. @@ +1703,3 @@ + + node = node->next; + g_slice_free (Realm, realm); Doesn't the list need destruction here? @@ +1713,3 @@ + g_str_equal, + NULL, + UmIdentity *identity) As below, we have wide screens. @@ +1721,3 @@ + NULL, + (GAsyncReadyCallback) + int left_position, top_position; same line as the cast please. @@ +1811,3 @@ + g_hash_table_unref (priv->accessible_realms); + priv->accessible_realms = NULL; + } Does the ->identity_manager need clearing as well?
Review of attachment 208807 [details] [review]: configure.ac is missing a mention of the state of the compile-time option. For example: configure: Users panel webcam support disabled
Review of attachment 208809 [details] [review]: um-user-panel.c:1647:1: warning: ‘on_identities_listed’ defined but not used [-Wunused-function] without krb5 support.
Review of attachment 208808 [details] [review]: Looks fine ::: configure.ac @@ +148,3 @@ +AC_SUBST(KRB5_LIBS) + +else I shouldn't need to explicitely ask for kerberos support if the devel libraries are already installed. (me looking for a while why kerberos support wasn't enabled)
On startup I get: Gtk-CRITICAL **: gtk_widget_hide: assertion `GTK_IS_WIDGET (widget)' failed when not signed in any kerberos realms.
Created attachment 209169 [details] [review] user-accounts: port from GtkTable to GtkGrid GtkTable is deprecated and no longer encouraged. This commit changes the user panel over to gtkgrid instead.
Created attachment 209170 [details] [review] user-accounts: add "Other Passwords" button Since the user can change the login password from the user panel, it stands to reason the user may want to be able to adjust other passwords from there as well. This commit adds an "Other Passwords" button which runs seahorse when clicked. See: https://live.gnome.org/Design/Proposals/UserIdentities for more information.
Created attachment 209171 [details] [review] user-accounts: hide admin tasks from non-administrators If the user isn't an administrator (either by account type, or through polkit escalation), then don't show administrative tasks in the panel like editing other users, setting up autologin, etc.
Created attachment 209172 [details] [review] user-accounts: Add identity manager interface The page here: https://live.gnome.org/Design/Proposals/UserIdentities proposes adding a list of "accessible realms" to the user panel. To implement that we'll need to get a list of realms to show. This commit adds the basics of the required interface. A subsequent commit will had a concrete implementation based on kerberos. At some point, we may add an SSSD based implementation, too, which is why there's the abstraction.
Created attachment 209173 [details] [review] user-accounts: add alarm class When we support showing kerberos realms in the UI, we're going to need some way to get notified when the users credentials are no longer valid for those realms. This commit adds an alarm class that abstracts timerfd so we can get proper notification when kerberos credentials expire.
Created attachment 209174 [details] [review] user-accounts: add kerberos implemention of identity manager interface This commits adds a concrete implementation of the identity manager inteface using the krb5 libraries. In the future we may add a different interface that leverages SSSD.
Created attachment 209175 [details] [review] user-accounts: Use identity manager in user panel This commit hooks up the identity manager to the actual UI. It creates a list of currently "accessible realms" and provides buttons for signing out of those realms. This is outlined here: https://live.gnome.org/Design/Proposals/UserIdentities
Created attachment 209176 [details] [review] build: Print out status of kerberos support
Created attachment 209177 [details] [review] build: Enable Kerberos by default if present
All the bugs mentioned above fixed about from the widget movement when logging out of a Kerberos realm, which makes me cringe. String changes, UI changes, so you'll need to request a freeze break for 3.4.
Just copying this in from an RFE I opened in Fedora which is relevant to this upstream ticket (specifically the Kerberos realm integration portion): Description of problem: Starting with Kerberos 1.10 (available in Fedora 17 and later), Kerberos now has the ability to store TGTs for multiple KDCs in a single cache (called a DIR cache). The benefit of this is that a user can now have single-sign-on between multiple Kerberos realms simultaneously. GNOME should implement a user interface for users to configure one or more such Kerberos realms. The online accounts tool seems like the appropriate place to me. Version-Release number of selected component (if applicable): gnome-online-accounts-3.4.2-1.fc17.x86_64 How reproducible: N/A (RFE) Additional info: https://fedoraproject.org/wiki/Features/KRB5DirCache Starting with Fedora 18, Fedora's default Kerberos credential cache location will be moved to a known, secure, user-private location (KRB5CCNAME=DIR:/run/user/<UID>/ccdir). See https://fedoraproject.org/wiki/Features/KRB5CacheMove for details. GNOME should do the following: 1. If the KRB5CCNAME variable is present in the environment and starts with DIR:, use that location for the cache 2. If the KRB5CCNAME variable is not present in the environment or does NOT start with DIR:, disable this feature from Online Accounts (multiple TGTs cannot work with non-DIR caches), and if the environment variable isn't set, other applications in the environment won't be able to pick up changes made by the Online Accounts dialog. The KRB5CCNAME variable is set in the user's session by SSSD or pam_krb5 if they are in use. If they are not, it may be prudent for GNOME/Freedesktop or perhaps pam_systemd to provide a session module to set this value to DIR:/run/user/<UID>/ccdir by default (if it has not been set in the environment already; pam_krb5 and SSSD set it during the auth phase, by necessity), if we want local users to be able to have this capability. Another option here would be to change the libkrb5 defaults on Fedora, but since GNOME runs on other distros it might be best to solve the problem generally.
I've posted patches for gnome-online-accounts integration to bug 679253
Also wanted to mention (just for purposes of paper trail), Stef went ahead and split this bug up. See bug 681762 and bug 681753.
Ray, what's the status of all this?
this bug has been completely subsumed by other bugs from me, stefw, and oholy afaik so we can close it out.