Bug 55915 - accountsservice: Add user login time(s) properties
Summary: accountsservice: Add user login time(s) properties
Status: RESOLVED FIXED
Alias: None
Product: accountsservice
Classification: Unclassified
Component: general (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Matthias Clasen
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-12 12:37 UTC by Ondrej Holy
Modified: 2012-11-12 21:11 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
accountsservice: Add User.LoginTimes property (11.11 KB, patch)
2012-10-12 12:38 UTC, Ondrej Holy
Details | Splinter Review
accountsservice: Add User.LoginTime property (9.23 KB, patch)
2012-10-12 12:39 UTC, Ondrej Holy
Details | Splinter Review
accountsservice: Add User.LogoutTimes property (15.66 KB, patch)
2012-11-01 13:01 UTC, Ondrej Holy
Details | Splinter Review
accountsservice: User login/logout times for x-sessions only (968 bytes, patch)
2012-11-01 13:03 UTC, Ondrej Holy
Details | Splinter Review
accountsservice: Add User.LoginTime property (9.54 KB, patch)
2012-11-08 13:59 UTC, Ondrej Holy
Details | Splinter Review
accountsservice: Add User.LoginHistory property (15.63 KB, patch)
2012-11-08 14:00 UTC, Ondrej Holy
Details | Splinter Review

Description Ondrej Holy 2012-10-12 12:37:49 UTC
Add User.LoginTime and User.LoginTimes property which is necessary for a last login field in user accounts panel.

See https://bugzilla.gnome.org/show_bug.cgi?id=681772
Comment 1 Ondrej Holy 2012-10-12 12:38:33 UTC
Created attachment 68487 [details] [review]
accountsservice: Add User.LoginTimes property
Comment 2 Ondrej Holy 2012-10-12 12:39:04 UTC
Created attachment 68488 [details] [review]
accountsservice: Add User.LoginTime property
Comment 3 Ondrej Holy 2012-11-01 13:01:03 UTC
Created attachment 69392 [details] [review]
accountsservice: Add User.LogoutTimes property
Comment 4 Ondrej Holy 2012-11-01 13:03:39 UTC
Created attachment 69393 [details] [review]
accountsservice: User login/logout times for x-sessions only
Comment 5 Matthias Clasen 2012-11-02 00:34:50 UTC
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>
Comment 6 Ondrej Holy 2012-11-02 09:29:39 UTC
(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).
Comment 7 Matthias Clasen 2012-11-02 09:43:36 UTC
> 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.
Comment 8 Ondrej Holy 2012-11-08 13:59:58 UTC
Created attachment 69700 [details] [review]
accountsservice: Add User.LoginTime property
Comment 9 Ondrej Holy 2012-11-08 14:00:31 UTC
Created attachment 69701 [details] [review]
accountsservice: Add User.LoginHistory property
Comment 10 Ray Strode [halfline] 2012-11-12 20:02:01 UTC
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 11 Ray Strode [halfline] 2012-11-12 21:11:26 UTC
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.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.