Block switching to same user as an user occupying current seat 59/41259/7
authorYoumin Ha <youmin.ha@samsung.com>
Wed, 13 May 2015 06:54:51 +0000 (15:54 +0900)
committerYoumin Ha <youmin.ha@samsung.com>
Mon, 12 Oct 2015 03:02:31 +0000 (12:02 +0900)
Switching to same user as the current user raises a abnormal behavior,
failing on re-login process. This is due to the inconsistance between
the tlm-sessiond daemon lifecycle and the systemd-logind's logout
procedure.
The standard solution is to receive a sessionTerminated signal from the
systemd-logind and sync it to the re-login procedure of TLM, but it
will take many structure changes of tlm-sessiond.
This patch just blocks switching to same user, preventing the abnormal
case with just a small effort.

 * src/daemon/tlm-seat.c|h: tlm_seat_get_occupying_username() is added.
 * src/daemon/tlm-dbus-observer.c: _is_valid_switch_user_dbus_request()
   is added and used in _process_request().

In addition, several comments and debug logs are given.

Change-Id: I9238098771bcce2b40aa89da47e9391ff7556922

configure.ac
src/daemon/dbus/tlm-dbus-login-adapter.c
src/daemon/tlm-dbus-observer.c
src/daemon/tlm-main.c
src/daemon/tlm-manager.c
src/daemon/tlm-seat.c
src/daemon/tlm-seat.h
src/daemon/tlm-session-remote.c
src/utils/tlm-client.c

index 0192719..a1a416b 100644 (file)
@@ -79,7 +79,7 @@ fi
 
 # Define socket path
 AC_ARG_ENABLE(sockets-path,
-          [  --enable-sockets-path=path  enable dbus sockets path at location'
+          [  --enable-sockets-path=path  enable dbus sockets path at location
            "path" instead of default "/var/run/tlm"],
           [enable_sockets_path=$enableval],
           [enable_sockets_path="/var/run/tlm"])
index b74e022..3047aec 100644 (file)
@@ -265,7 +265,7 @@ _handle_login_user (
         g_error_free (error);
         return TRUE;
     }
-    DBG ("seat_id %s username %s", seat_id, username);
+    DBG ("Emit login-user signal: seat_id=%s, username=%s", seat_id, username);
 
     g_signal_emit (self, signals[SIG_LOGIN_USER], 0, seat_id, username,
             password, environment, invocation);
@@ -292,7 +292,7 @@ _handle_logout_user (
         g_error_free (error);
         return TRUE;
     }
-    DBG ("seat_id %s", seat_id);
+    DBG ("Emit logout-user signal: seat_id=%s", seat_id);
 
     g_signal_emit (self, signals[SIG_LOGOUT_USER], 0, seat_id, invocation);
 
@@ -321,7 +321,7 @@ _handle_switch_user (
         g_error_free (error);
         return TRUE;
     }
-    DBG ("seat_id %s username %s", seat_id, username);
+    DBG ("Emit switch-user signal: seat_id=%s, username=%s", seat_id, username);
 
     g_signal_emit (self, signals[SIG_SWITCH_USER], 0, seat_id, username,
             password, environment, invocation);
index 13519bf..c749fa6 100644 (file)
@@ -287,9 +287,14 @@ _connect_dbus_adapter (
         TlmDbusObserver *self,
         TlmDbusLoginAdapter *adapter)
 {
-    if (self->priv->enable_flags & DBUS_OBSERVER_ENABLE_LOGIN_USER)
-        g_signal_connect_swapped (G_OBJECT (adapter),
+    DBG("Connecting signals to signal handlers");
+    if (self->priv->enable_flags & DBUS_OBSERVER_ENABLE_LOGIN_USER) {
+        int r = g_signal_connect_swapped (G_OBJECT (adapter),
                 "login-user", G_CALLBACK(_handle_dbus_login_user), self);
+        DBG("_handle_dbus_login_user() is connected to 'login-user' signal. return value=%d", r);
+    } else {
+        DBG("'login-user' signal callback is not connected");
+    }
     if (self->priv->enable_flags & DBUS_OBSERVER_ENABLE_LOGOUT_USER)
         g_signal_connect_swapped (G_OBJECT (adapter),
                 "logout-user", G_CALLBACK(_handle_dbus_logout_user), self);
@@ -303,6 +308,7 @@ _disconnect_dbus_adapter (
         TlmDbusObserver *self,
         TlmDbusLoginAdapter *adapter)
 {
+    DBG("Disconnecting signals to signal handlers");
     if (self->priv->enable_flags & DBUS_OBSERVER_ENABLE_LOGIN_USER)
         g_signal_handlers_disconnect_by_func (G_OBJECT(adapter),
                 _handle_dbus_login_user, self);
@@ -403,6 +409,19 @@ _is_request_supported (
 }
 
 static gboolean
+_is_valid_switch_user_dbus_request(
+        TlmDbusRequest *dbus_request,
+        TlmSeat *seat)
+{
+    if (0 == g_strcmp0(dbus_request->username,
+            tlm_seat_get_occupying_username(seat))) {
+        WARN("Cannot switch to same username");
+        return FALSE;
+    }
+    return TRUE;
+}
+
+static gboolean
 _process_request (
         TlmDbusObserver *self)
 {
@@ -431,7 +450,15 @@ _process_request (
             goto _finished;
         }
 
-        seat = self->priv->seat;
+        // NOTE:
+        // The below code intends that the current seat of the TlmDbusObserver
+        // is always selected, even the given request has specifed ananother
+        // seat.  The TlmDbusRequest's seat is applied only when the current
+        // TlmDbusObserver is a default dbus socket(/var/run/dbus-sock), which
+        // has priv->seat as NULL.
+        // This means that only the root user(who can access the default dbus
+        // socket) can specify the seat in his TlmDbusRequest.
+        seat = self->priv->seat;  // observer's seat has high priority
         if (!seat && self->priv->manager) {
             seat = tlm_manager_get_seat (self->priv->manager,
                     dbus_req->seat_id);
@@ -463,8 +490,12 @@ _process_request (
             ret = tlm_seat_terminate_session (seat);
             break;
         case TLM_DBUS_REQUEST_TYPE_SWITCH_USER:
-            ret = tlm_seat_switch_user (seat, NULL, dbus_req->username,
+            // Refuse request if the request's username is same to
+            // the current user who occupies the seat.
+            if (_is_valid_switch_user_dbus_request(dbus_req, seat))
+                ret = tlm_seat_switch_user (seat, NULL, dbus_req->username,
                     dbus_req->password, dbus_req->environment);
+            else ret = FALSE;
             break;
         }
         if (!ret) {
@@ -478,7 +509,8 @@ _finished:
         _complete_request (self, req, err);
     }
 
-    return self->priv->active_request != NULL ? FALSE : TRUE;
+    return self->priv->active_request != NULL ?
+            G_SOURCE_REMOVE : G_SOURCE_CONTINUE;
 }
 
 static void
@@ -722,7 +754,7 @@ static void
 tlm_dbus_observer_init (TlmDbusObserver *dbus_observer)
 {
     TlmDbusObserverPrivate *priv = TLM_DBUS_OBSERVER_PRIV (dbus_observer);
-    
+
     priv->manager = NULL;
     priv->seat = NULL;
     priv->enable_flags = DBUS_OBSERVER_ENABLE_ALL;
@@ -753,7 +785,7 @@ tlm_dbus_observer_new (
      * seat is connected on per dbus request basis and then
      * disconnected when the dbus request is completed or aborted */
     if (seat) {
-        dbus_observer->priv->seat = seat;
+        dbus_observer->priv->seat = seat;  // Remember specified seat
         g_object_weak_ref (G_OBJECT (seat), (GWeakNotify)_on_seat_dispose,
                 dbus_observer);
     }
index 8a62e37..05c4d2f 100644 (file)
@@ -141,14 +141,19 @@ int main(int argc, char *argv[])
     main_loop = g_main_loop_new (NULL, FALSE);
 
     manager = tlm_manager_new (username);
-    _setup_unix_signal_handlers (manager);
-    tlm_manager_start (manager);
+    if (manager) {
+        _setup_unix_signal_handlers (manager);
 
-    g_main_loop_run (main_loop);
+        if (TRUE == tlm_manager_start (manager)) {
+            g_main_loop_run (main_loop);
+        }
 
-    g_object_unref (G_OBJECT(manager));
+        g_object_unref (G_OBJECT(manager));
 
-    DBG ("clean shutdown");
+        DBG ("clean shutdown");
+    } else {
+        ERR ("Cannot create tlm_manager object");
+    }
 
     tlm_log_close (NULL);
 
index d1f73bd..8ba4ec8 100644 (file)
@@ -554,6 +554,7 @@ _add_seat (TlmManager *manager, const gchar *seat_id, const gchar *seat_path)
 
     TlmManagerPrivate *priv = TLM_MANAGER_PRIV (manager);
 
+    // Do nothing if the seat is not active
     if (!tlm_config_get_boolean (priv->config,
                                  seat_id,
                                  TLM_CONFIG_SEAT_ACTIVE,
@@ -575,7 +576,7 @@ _add_seat (TlmManager *manager, const gchar *seat_id, const gchar *seat_path)
           g_free (watchx);
         }
         watch_items[nwatch] = NULL;
-        TlmSeatWatchClosure *watch_closure = 
+        TlmSeatWatchClosure *watch_closure =
             g_new0 (TlmSeatWatchClosure, 1);
         watch_closure->manager = g_object_ref (manager);
         watch_closure->seat_id = g_strdup (seat_id);
@@ -587,7 +588,8 @@ _add_seat (TlmManager *manager, const gchar *seat_id, const gchar *seat_path)
         if (watch_id <= 0) {
             WARN ("Failed to add watch on seat %s", seat_id);
         } else {
-            return;
+            // NOTE: Do nothing here, to run _create_seat()
+            // return;
         }
     }
 
index d756eb0..d3236ff 100644 (file)
@@ -127,8 +127,10 @@ _close_active_session (TlmSeat *self)
 {
     TlmSeatPrivate *priv = TLM_SEAT_PRIV (self);
     _disconnect_session_signals (self);
-    if (priv->session)
+    if (priv->session) {
+        DBG("Clear session_remote object");
         g_clear_object (&priv->session);
+    }
 }
 
 static void
@@ -143,8 +145,11 @@ _handle_session_terminated (
     gboolean stop = FALSE;
 
     DBG ("seat %p session %p", self, priv->session);
+
     _close_active_session (seat);
 
+    // NOTE: This "session-terminated" signal to seat object is caught by
+    // _session_terminated_cb() which is conencted in tlm_manager_stop().
     g_signal_emit (seat,
             signals[SIG_SESSION_TERMINATED],
             0,
@@ -154,8 +159,10 @@ _handle_session_terminated (
         DBG ("no relogin or switch user");
         return;
     }
+
     g_clear_object (&priv->dbus_observer);
 
+    // If X11 session is used, kill self
     if (tlm_config_get_boolean (priv->config,
                                 TLM_CONFIG_GENERAL,
                                 TLM_CONFIG_GENERAL_X11_SESSION,
@@ -166,12 +173,14 @@ _handle_session_terminated (
         return;
     }
 
+    // Auto re-login, if the auto-login option is set
     if (tlm_config_get_boolean (priv->config,
                                 TLM_CONFIG_GENERAL,
                                 TLM_CONFIG_GENERAL_AUTO_LOGIN,
                                 TRUE) ||
                                 seat->priv->next_user) {
         DBG ("auto re-login with '%s'", seat->priv->next_user);
+
         tlm_seat_create_session (seat,
                 seat->priv->next_service,
                 seat->priv->next_user,
@@ -305,7 +314,7 @@ _seat_set_property (GObject *obj,
         case PROP_CONFIG:
             priv->config = g_value_dup_object (value);
             break;
-        case PROP_ID: 
+        case PROP_ID:
             priv->id = g_value_dup_string (value);
             break;
         case PROP_PATH:
@@ -329,7 +338,7 @@ _seat_get_property (GObject *obj,
         case PROP_CONFIG:
             g_value_set_object (value, priv->config);
             break;
-        case PROP_ID: 
+        case PROP_ID:
             g_value_set_string (value, priv->id);
             break;
         case PROP_PATH:
@@ -431,7 +440,7 @@ static void
 tlm_seat_init (TlmSeat *seat)
 {
     TlmSeatPrivate *priv = TLM_SEAT_PRIV (seat);
-    
+
     priv->id = priv->path = priv->default_user = NULL;
     priv->dbus_observer = priv->prev_dbus_observer = NULL;
     priv->default_active = FALSE;
@@ -446,6 +455,19 @@ tlm_seat_get_id (TlmSeat *seat)
     return (const gchar*) seat->priv->id;
 }
 
+const gchar *
+tlm_seat_get_occupying_username (TlmSeat *seat) {
+    TlmSeatPrivate *priv = TLM_SEAT_PRIV (seat);
+    if (!priv->session) return NULL;
+
+    TlmSessionRemote *sr = priv->session;
+    gchar *username = NULL;
+
+    g_object_get(G_OBJECT(sr), "username", &username, NULL);
+
+    return username;
+}
+
 gboolean
 tlm_seat_switch_user (TlmSeat *seat,
                       const gchar *service,
@@ -453,6 +475,7 @@ tlm_seat_switch_user (TlmSeat *seat,
                       const gchar *password,
                       GHashTable *environment)
 {
+    DBG("run tlm_seat_switch_user()");
     g_return_val_if_fail (seat && TLM_IS_SEAT(seat), FALSE);
 
     TlmSeatPrivate *priv = TLM_SEAT_PRIV (seat);
@@ -465,10 +488,12 @@ tlm_seat_switch_user (TlmSeat *seat,
     }
 
     if (!priv->session) {
+        DBG("No live session, so just create session.");
         return tlm_seat_create_session (seat, service, username, password,
                 environment);
     }
 
+    DBG("Store service/user/password/env as seat->priv->next_*, and terminate current session");
     _reset_next (priv);
     priv->next_service = g_strdup (service);
     priv->next_user = g_strdup (username);
@@ -551,12 +576,16 @@ tlm_seat_create_session (TlmSeat *seat,
     g_return_val_if_fail (seat && TLM_IS_SEAT(seat), FALSE);
     TlmSeatPrivate *priv = TLM_SEAT_PRIV (seat);
 
+    // Ignore creating session if there is an existing session already
     if (priv->session != NULL) {
+        ERR("Session already exists on this seat(%s)", priv->id);
         g_signal_emit (seat, signals[SIG_SESSION_ERROR],  0,
                 TLM_ERROR_SESSION_ALREADY_EXISTS);
         return FALSE;
     }
 
+    // Prevent short-delay relogin.
+    // If the login delay is too fast (<1s), run _delayed_session()
     if (g_get_monotonic_time () - priv->prev_time < 1000000) {
         DBG ("short time relogin");
         priv->prev_time = g_get_monotonic_time ();
@@ -578,6 +607,8 @@ tlm_seat_create_session (TlmSeat *seat,
         priv->prev_count = 1;
     }
 
+    // Check for function arguments
+    // service: if NULL, get default service
     if (!service) {
         DBG ("PAM service not defined, looking up configuration");
         service = tlm_config_get_string (priv->config,
@@ -592,6 +623,7 @@ tlm_seat_create_session (TlmSeat *seat,
     }
     DBG ("using PAM service %s for seat %s", service, priv->id);
 
+    // username: if NULL, get default user
     if (!username) {
         if (!priv->default_user) {
             const gchar *name_tmpl =
@@ -616,11 +648,13 @@ tlm_seat_create_session (TlmSeat *seat,
         }
     }
 
+    // Create a session_remote object (It creates actual remote session process)
     priv->session = tlm_session_remote_new (priv->config,
             priv->id,
             service,
             priv->default_active ? priv->default_user : username);
     if (!priv->session) {
+
         g_signal_emit (seat, signals[SIG_SESSION_ERROR], 0,
                 TLM_ERROR_SESSION_CREATION_FAILURE);
         return FALSE;
@@ -628,11 +662,15 @@ tlm_seat_create_session (TlmSeat *seat,
 
     /*It is needed to handle switch user case which completes after new session
      *is created */
+    // TODO: process this prev_dbus_observer somewhere!
     seat->priv->prev_dbus_observer = seat->priv->dbus_observer;
     seat->priv->dbus_observer = NULL;
     if (!_create_dbus_observer (seat,
             priv->default_active ? priv->default_user : username)) {
-        g_clear_object (&priv->session);
+        if (priv->session) {
+            DBG("Clear session_remote object");
+            g_clear_object (&priv->session);
+        }
         g_signal_emit (seat, signals[SIG_SESSION_ERROR],  0,
                 TLM_ERROR_DBUS_SERVER_START_FAILURE);
         return FALSE;
@@ -664,7 +702,6 @@ tlm_seat_terminate_session (TlmSeat *seat)
                 TLM_ERROR_SESSION_NOT_VALID);
         return FALSE;
     }
-
     return TRUE;
 }
 
index 8e832ed..b0e0567 100644 (file)
@@ -67,6 +67,13 @@ tlm_seat_new (TlmConfig *config,
 const gchar *
 tlm_seat_get_id (TlmSeat *seat);
 
+/** Get the username who occupies the seat
+ * @return  The name of the user who holds the seat
+ * @return  NULL if nobody occupies the seat
+ */
+const gchar *
+tlm_seat_get_occupying_username (TlmSeat* seat);
+
 gboolean
 tlm_seat_switch_user (TlmSeat *seat,
                       const gchar *service,
index 0eeb84a..1d343be 100644 (file)
@@ -55,7 +55,7 @@ static GParamSpec *properties[N_PROPERTIES];
 
 struct _TlmSessionRemotePrivate
 {
-       TlmConfig *config;
+    TlmConfig *config;
     GDBusConnection *connection;
     TlmDbusSession *dbus_session_proxy;
     GPid cpid;
@@ -97,7 +97,7 @@ _on_child_down_cb (
 
     TlmSessionRemote *session = TLM_SESSION_REMOTE (data);
 
-    DBG ("Sessiond(%p) with pid (%d) closed with status %d", session, pid,
+    DBG ("Sessiond(%p) with pid (%d) died with status %d", session, pid,
             status);
 
     session->priv->is_sessiond_up = FALSE;
@@ -416,6 +416,7 @@ _on_session_terminated_cb (
         gpointer user_data)
 {
     g_return_if_fail (self && TLM_IS_SESSION_REMOTE (self));
+    DBG ("dbus-session-proxy got a session-terminated signal");
     if (self->priv->can_emit_signal)
         g_signal_emit (self, signals[SIG_SESSION_TERMINATED], 0);
 }
index 8c3a883..7551f61 100644 (file)
@@ -118,7 +118,7 @@ _setup_daemon ()
     }
     sleep (5); /* 5 seconds */
 
-    DBG ("Daemon PID = %d\n", daemon_pid);
+    ERR ("Daemon PID = %d\n", daemon_pid);
     return TRUE;
 }
 
@@ -153,6 +153,7 @@ _get_bus_connection (
     gchar address[128];
     g_snprintf (address, 127, "unix:path=%s/%s-%d", TLM_DBUS_SOCKET_PATH,
             seat_id, user_id);
+    DBG ("trying to connect socket:%s", address);
     return g_dbus_connection_new_for_address_sync (address,
             G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT, NULL, NULL, error);
 }