Differential D26813

Have user-manager write to SDDM's new config file
ClosedPublic
Actions

Authored by filipf on Jan 21 2020, 5:58 PM.

Details

Reviewers
davidedmundson
ngraham
Group Reviewers
Plasma
Commits
R128:b2d0e463a490: Have user-manager write to SDDM's new config file
Summary

user-manager still wants to write to sddm.conf and it's using sddm_kcm's function to do so, but this function no longer supports writing to sddm.conf

That means auto-login settings are no longer getting saved anywhere.

This patch tweaks user-manager to write to /etc/sddm.conf.d/kde_setings.conf, making the feature work again.

NOTE: The autologin checkbox will keep getting reset until we fix bug 414396 because the kde_settings.conf file is not world-readable at this moment.

BUG: 342722
FIXED-IN: 5.18.1

Test Plan
  • checked the autologin checkbox for filip.
  • filip got written into kde_settings.conf under autologin users

Diff Detail

Repository
R128 User Manager
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
filipf created this revision.Jan 21 2020, 5:58 PM
Restricted Application added a project: Plasma. · View Herald TranscriptJan 21 2020, 5:58 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
filipf requested review of this revision.Jan 21 2020, 5:58 PM
filipf edited the summary of this revision. (Show Details)Jan 21 2020, 6:00 PM
filipf edited the test plan for this revision. (Show Details)
filipf added reviewers: Plasma, davidedmundson, ngraham.
davidedmundson accepted this revision.Jan 21 2020, 6:36 PM

We could move the full file path to the helper rather than having the calling user send it.
I'll be more secure that way.

This revision is now accepted and ready to land.Jan 21 2020, 6:36 PM
ngraham accepted this revision.Jan 21 2020, 7:03 PM

Sounds good, should we do that refactor in master and land this on the stable branch?

filipf added a comment.Jan 22 2020, 11:09 AM
In D26813#598261, @ngraham wrote:

Sounds good, should we do that refactor in master and land this on the stable branch?

I'll leave that up to @davidedmundson's judgment.

filipf updated this revision to Diff 74954.Feb 3 2020, 8:19 PM

don't define any sort of sddm config file path in user manager (needs an additional sddm patch I'm going to send in now)

filipf added a dependency: D27142: Have authhelper define config file paths on its own.Feb 3 2020, 8:28 PM
In D26813#598252, @davidedmundson wrote:

We could move the full file path to the helper rather than having the calling user send it.
I'll be more secure that way.

This should work now with the dependent revision.

filipf planned changes to this revision.Feb 3 2020, 8:45 PM
filipf added inline comments.
src/lib/accountmodel.cpp
45

actually this is wrong.... I don't know what to specify without typing out the path here again

filipf updated this revision to Diff 75007.Feb 4 2020, 6:17 PM

go back to defining a path for reading sddm's config file (I don't know how else to obtain it, but saving is still done completely on sddm's auth helper side)

This revision is now accepted and ready to land.Feb 4 2020, 6:17 PM
davidedmundson added inline comments.Feb 4 2020, 6:43 PM
src/lib/accountmodel.cpp
45

ideally we want the exact same code as the other place

load /etc/sddm.conf
loop through /etc/sddm.conf.d
add all those files as config sources

filipf added inline comments.Feb 5 2020, 10:25 PM
src/lib/accountmodel.cpp
45

Seems doable, just not sure what to have this config line here open. I copied the old code but it won't let me open the 'mSddmConfig' config.

ngraham edited the summary of this revision. (Show Details)Feb 16 2020, 9:00 PM
filipf added a comment.Feb 16 2020, 10:35 PM

For 5.18.1 though maybe we could just roll this version out?

ngraham added a comment.Feb 16 2020, 11:47 PM

+1, we can to the refactoring on master.

Closed by commit R128:b2d0e463a490: Have user-manager write to SDDM's new config file (authored by filipf). · Explain WhyFeb 17 2020, 9:09 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Msrc/lib/accountmodel.cpp (7 lines)

Diff 75861

View Options

src/lib/accountmodel.cpp

Loading...