Bug 55915

Created attachment 68487 [details] [review]
accountsservice: Add User.LoginTimes property

Created attachment 68488 [details] [review]
accountsservice: Add User.LoginTime property

Created attachment 69392 [details] [review]
accountsservice: Add User.LogoutTimes property

Created attachment 69393 [details] [review]
accountsservice: User login/logout times for x-sessions only

I don't think it is a great idea to split login and logout times into separate arrays. That is only going to force callers to manually match up, and is actually unclear - what if the current session has started but not ended yet ? is it included in the login array, but not the logout one ?

And only including X sessions seems too arbitrary to me. Another caller might well be interested in the console sessions.

Here is what I would suggest:

<property name="LoginHistory" type="a(xxa{sv})" access="read">
<doc:doc><doc:description><doc:para>
  The Login history for this user.
  Each entry in the array represents a login session. The first
  two members are the login time and logout time, as timestamps (seconds
  since the epoch). If the session is still running, the logout
  time is 0.
</doc:para><doc:para>
  The a{sv} member is a dictionary containing additional information
  about the session. Possible members include 'seat' (with values like 'seat0',
  'seat1', etc), 'vc' (with values like 'vc0', 'vc1',...), and 'type' (possible
  values include 'x11' and 'tty').
</doc:para></doc:description></doc:doc>
</property>

(In reply to comment #5)

Thank you for review.

> I don't think it is a great idea to split login and logout times into
> separate arrays. That is only going to force callers to manually match up,
> and is actually unclear - what if the current session has started but not
> ended yet ? is it included in the login array, but not the logout one ?
 
Yes, actually you are right. Splitting login and logout times wasn't the best idea. I'll fix it.

> And only including X sessions seems too arbitrary to me. Another caller
> might well be interested in the console sessions.

Yes, it makes sense, I'm forgetting that account service isn't service only for user accounts. 

> Here is what I would suggest:
> 
> <property name="LoginHistory" type="a(xxa{sv})" access="read">
> <doc:doc><doc:description><doc:para>
>   The Login history for this user.
>   Each entry in the array represents a login session. The first
>   two members are the login time and logout time, as timestamps (seconds
>   since the epoch). If the session is still running, the logout
>   time is 0.
> </doc:para><doc:para>
>   The a{sv} member is a dictionary containing additional information
>   about the session. Possible members include 'seat' (with values like
> 'seat0',
>   'seat1', etc), 'vc' (with values like 'vc0', 'vc1',...), and 'type'
> (possible
>   values include 'x11' and 'tty').
> </doc:para></doc:description></doc:doc>
> </property>

What do you mean with members like seat and vc? I'm not sure if we can get these information from wtmp. I think only additional info we can get is type with members like pts, tty, x11 (as in utility last based on wtmp).

> What do you mean with members like seat and vc? I'm not sure if we can get 
> these information from wtmp. I think only additional info we can get is type 
> with members like pts, tty, x11 (as in utility last based on wtmp).

Yeah, this would be information that is available if we ever switched to talking to systemd for this information - it is what shows up in loginctl session-status.
For now, we only need the type information to filter out non-X sessions. Specifying an a{sv} in the return value lets us extend this in the future without breaking api.

Created attachment 69700 [details] [review]
accountsservice: Add User.LoginTime property

Created attachment 69701 [details] [review]
accountsservice: Add User.LoginHistory property

Comment on attachment 69700 [details] [review]
accountsservice: Add User.LoginTime property

Review of attachment 69700 [details] [review]:
-----------------------------------------------------------------

pushed with the small changes including a fix for:

::: src/daemon.c
@@ +282,4 @@
>                          continue;
>                  }
>  
> +                g_object_set (user, "login-frequency", data->frequency, NULL);

data isn't reinitialized here to point to the currently iterated entry. It's just using the last value set from above.

Comment on attachment 69701 [details] [review]
accountsservice: Add User.LoginHistory property

Review of attachment 69701 [details] [review]:
-----------------------------------------------------------------

I pushed this with minor name changes and such.