Bug 437286 - Changing the user picture shouldn't require admin rights
Summary: Changing the user picture shouldn't require admin rights
Status: RESOLVED FIXED
Alias: None
Product: systemsettings
Classification: Applications
Component: kcm_users (show other bugs)
Version: unspecified
Platform: Other Linux
: NOR wishlist
Target Milestone: ---
Assignee: Janet Blackquill
URL:
Keywords:
: 430508 (view as bug list)
Depends on:
Blocks:
 
Reported: 2021-05-18 04:09 UTC by Lua
Modified: 2022-07-24 21:41 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.26
Sentry Crash Report:


Attachments
Password prompt (123.45 KB, image/png)
2021-05-18 04:09 UTC, Lua
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Lua 2021-05-18 04:09:03 UTC
Created attachment 138527 [details]
Password prompt

SUMMARY


STEPS TO REPRODUCE
1. Go to System Settings > Users;
2. Change the picture.

OBSERVED RESULT


EXPECTED RESULT
Changing the user picture should not require admin rights.

SOFTWARE/OS VERSIONS

ADDITIONAL INFORMATION
Comment 1 Nate Graham 2021-05-19 02:22:55 UTC
Jan, is this something that AccountService can do for us?
Comment 2 Janet Blackquill 2021-05-19 02:36:38 UTC
This is PolKit's job; distros should probably ship polkit rules permitting the "org.freedesktop.accounts.change-own-user-data" action to be performed without password
Comment 3 Nate Graham 2021-05-19 15:30:56 UTC
If they did, would this bug be automatically fixed, or would we need to make any code changes to support it?
Comment 4 Nate Graham 2021-07-30 17:22:18 UTC
Indeed it would. Please bug your distro about this. I have done so for Fedora: https://bugzilla.redhat.com/show_bug.cgi?id=1988509
Comment 5 Nate Graham 2021-07-30 17:27:45 UTC
*** Bug 430508 has been marked as a duplicate of this bug. ***
Comment 6 Dan Robinson 2021-07-31 13:11:03 UTC
Reported for OpenSUSE here: https://bugzilla.opensuse.org/show_bug.cgi?id=1188948
Comment 7 Nate Graham 2021-08-16 13:04:36 UTC
SUSE security auditors have found that it is not accurate that we only require org.freedesktop.accounts.change-own-user-data; apparently it is org.freedesktop.accounts.user-administration, which does require a password upstream. See https://bugzilla.suse.com/show_bug.cgi?id=1188948

Perhaps the bug is that we should only be requiring org.freedesktop.accounts.change-own-user-data when changing your own data.
Comment 8 Dan Robinson 2021-12-05 02:22:47 UTC
So I think this is actually even more complicated than what we original thought.

AccountsService actually does use org.freedesktop.accounts.change-own-user-data to change face icon and logically should work. 

What's making this appear to happen is that we're writing every field from the UI to the DBus interface every time we apply changes, and blocking on getting authorization to do so. See https://invent.kde.org/plasma/plasma-workspace/-/blob/master/kcms/users/src/user.cpp#L280 for details.

So basically changing the icon doesn't require an admin password, but changing the account type does, and we don't even try to write the face icon until we can successfully write the account type by getting admin authorization.

So probably the real answer here is to track whether or not a field in the KCM UI actually changed before trying to write it to the DBus interface. Then we can only commit the actual changes to the transaction, and therefore only get auth_admin when we actually need it.
Comment 9 Bug Janitor Service 2021-12-05 02:47:45 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/plasma-workspace/-/merge_requests/1248
Comment 10 Janet Blackquill 2022-07-24 21:40:41 UTC
Git commit 0cf0de17bde4f08899a90ac891317b30bc171eb7 by Janet Blackquill, on behalf of Jan Blackquill.
Committed on 24/07/2022 at 03:27.
Pushed by ngraham into branch 'master'.

kcms/users: only set changed values via dbus api

Setting unchanged values may falsely trigger an authentication
prompt in the case that the changed value doesn't need authentication
to change, but the unchanged ones do, leading to an authentication prompt
to show up.

Adding a little bit of original vs new comparison lets us only send over
DBus what got changed.

M  +43   -14   kcms/users/src/user.cpp
M  +21   -7    kcms/users/src/user.h

https://invent.kde.org/plasma/plasma-workspace/commit/0cf0de17bde4f08899a90ac891317b30bc171eb7