Bug 72596 - [PATCH] Remove user cache files if user account no longer exists
Summary: [PATCH] Remove user cache files if user account no longer exists
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: 2013-12-11 11:56 UTC by Gunnar Hjalmarsson
Modified: 2017-12-19 02:02 UTC (History)
5 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Clean up cache dir (1.43 KB, patch)
2013-12-11 12:01 UTC, Gunnar Hjalmarsson
Details | Splinter Review
Clean up cache dir 2 (1.60 KB, patch)
2013-12-12 21:51 UTC, Gunnar Hjalmarsson
Details | Splinter Review
Clean up cache dir 3 (3.09 KB, patch)
2013-12-13 18:40 UTC, Gunnar Hjalmarsson
Details | Splinter Review
Move cache cleanup out into a common function and clean up icon too (2.58 KB, patch)
2014-02-05 15:27 UTC, Iain Lane
Details | Splinter Review
On startup, clean out the data of removed users (1.37 KB, patch)
2014-02-05 15:27 UTC, Iain Lane
Details | Splinter Review

Description Gunnar Hjalmarsson 2013-12-11 11:56:14 UTC
This is a forward of the Ubuntu bug https://launchpad.net/bugs/1259562

The guest session feature provided by lightdm has resulted in files for deleted temporary guest users being kept in the cache dir. To fix this, we are about to patch src/daemon.c so it deletes such files, and not just reports their existence in the log.

Since users may be deleted in a number of various ways without accountsservice being notified, we think it would be a good idea to make this change upstream.
Comment 1 Gunnar Hjalmarsson 2013-12-11 12:01:05 UTC
Created attachment 90603 [details] [review]
Clean up cache dir
Comment 2 Ray Strode [halfline] 2013-12-11 18:47:12 UTC
don't you need to clean up the icon too?

Aside from that, I wonder if we'll run into bug reports after this where users lose their settings after a domain controller blip.

Maybe it should avoid cleaning up remote users?
Comment 3 Gunnar Hjalmarsson 2013-12-11 21:49:24 UTC
Right, it's logical to clean up the icon as well. (Guest users typically don't bother with icons, which explains why I missed it.)

Maybe checking errno would provide sufficient safety?

   errno = 0;
   pwent = getpwnam (name);
   if (pwent == NULL && errno == 0) {
Comment 4 Gunnar Hjalmarsson 2013-12-11 22:18:16 UTC
From "man getpwnam":

RETURN VALUE
The  getpwnam() and getpwuid() functions return a pointer to a passwd structure, or NULL if the matching entry is not found or an error occurs.  If an error occurs, errno is set appropriately.
Comment 5 Gunnar Hjalmarsson 2013-12-12 21:51:21 UTC
Created attachment 90679 [details] [review]
Clean up cache dir 2

Attaching a new patch in accordance with comment #3.
Comment 6 Ray Strode [halfline] 2013-12-13 14:44:34 UTC
I like this patch.  Two requests, if you don't mind:

1) Can you make it in git-format-patch format so it has a commit message, etc ?
2) Can you do one preliminary clean up patch before this one that moves the existing cases where the cache files are removed into a standalone function, then change this patch to call that function?  This sort of clean up is repeated in two places already, it'd be good to drop the duplication rather than add a third place.
Comment 7 Gunnar Hjalmarsson 2013-12-13 18:40:11 UTC
Created attachment 90741 [details] [review]
Clean up cache dir 3

I chose to modify the patch so it makes use of a standalone function.

Sorry, but I'm not enough familiar with git, so I leave it to you to convert the patch into the desired format.
Comment 8 Iain Lane 2014-02-05 15:27:25 UTC
Created attachment 93452 [details] [review]
Move cache cleanup out into a common function and clean up icon too

I did as Ray suggested.

Also I passed --author to pretend that Gunnar wrote the git patches. If you find that objectionable, set it to me.
Comment 9 Iain Lane 2014-02-05 15:27:59 UTC
Created attachment 93453 [details] [review]
On startup, clean out the data of removed users
Comment 10 Michael Catanzaro 2015-12-18 20:23:48 UTC
Seems these patches have been forgotten....
Comment 11 Robert Ancell 2017-12-19 02:02:11 UTC
I've cleaned up and applied these patches to master.


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.