[WIP] Switch the Attica KDE plugin to use KAccounts
AbandonedPublic

Authored by leinir on Dec 13 2019, 5:44 PM.

Details

Reviewers
None
Summary

Don't require KWallet, and require KAccounts and libaccounts-Qt5

Link and use includes from new requirements

Remove KWallet bits, and use KAccounts/libaccounts-qt instead

This removes the custom KWallet integration in favour of an approach
based on the more generic KAccounts framework. It is a work in progress
which currently does not allow for creating new accounts (which would
be done through saveCredentials), but which does load and use the data
from any accounts which already exist.

Diff Detail

Repository
R119 Plasma Desktop
Branch
switch-attica-plugin-to-kaccounts (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 22864
Build 22882: arc lint + arc unit
leinir created this revision.Dec 13 2019, 5:44 PM
Restricted Application added a project: Plasma. · View Herald TranscriptDec 13 2019, 5:44 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
leinir requested review of this revision.Dec 13 2019, 5:44 PM
bshah awarded a token.Dec 14 2019, 7:05 AM
leinir updated this revision to Diff 71495.Dec 14 2019, 9:56 AM
  • Add the provider and service files (these use temp client data for now)
  • Actually install the service and provider files
leinir updated this revision to Diff 71607.Dec 15 2019, 3:52 PM
  • Add functionality to open the accounts kcm on saveCredentials calls
zachus added a subscriber: zachus.Dec 16 2019, 10:50 AM

is there a server out there to run it against?

is there a server out there to run it against?

The provider file points it at the opendesktop.org server (that is to say, the KDE Store), so testing can be done against the live servers that already provide all our content :)

Note also that the current client id and secret means you will be told that Bhushan Shah is asking for access to your things during sign-in - this is because that's who asked for test access in order to build this, so don't worry, that's currently expected. Before merging, it'll want to be swapped out for a more proper id, though we'll probably want to give this some thought - do we want one default for all of KDE's software, do we want one that can be replaced by distributions, or do we want distributions to have to do this? i'm currently unsure, but leaning towards option two (meaning, ship with a default id and secret set, but offer the ability for distributions and app developers to swap this out with one of their own).

leinir updated this revision to Diff 73756.Jan 17 2020, 11:25 AM

Rebase on master

  • Add the provider and service files (these use temp client data for now)
  • Actually install the service and provider files
  • Add functionality to open the accounts kcm on saveCredentials calls
leinir updated this revision to Diff 74208.Jan 23 2020, 10:55 AM
  • Actually make the service locator work (need to query for the right thing)
leinir updated this revision to Diff 76049.Feb 20 2020, 1:48 PM

Fair bit of work gone on here, but in short, given a patch which handles the
OpenID ID token in the OAuth2 requests in signon-plugin-oauth2, we are now
able to construct a bearer-authenticated request and whatnot. There is more
work to be done, but it seems like things are happening now! Progress.

The merge request for the signon-plugin-oauth2 patch can be found at:
https://gitlab.com/accounts-sso/signon-plugin-oauth2/-/merge_requests/25

  • Add the provider and service files (these use temp client data for now)
  • Actually install the service and provider files
  • Add functionality to open the accounts kcm on saveCredentials calls
  • Actually make the service locator work (need to query for the right thing)
  • Add the SignOn OAuth plugin as a runtime dependency
  • Minor cleanup for attica_kde cmakelists
  • The opendesktop provider file is... not google
  • Fetch the access token (actually ID token) from AccountsManager via KAccounts
leinir updated this revision to Diff 76309.Feb 24 2020, 3:54 PM
  • Switch to using the kaccounts cmake commands
apol added a subscriber: apol.Feb 24 2020, 4:32 PM

+1 cool stuff, much better than using kwallet for it.

Have you checked how well it works regarding startup? In Discover we do a bunch of queries at start for every KNSEngine and we need to make sure we don't block the whole thing (more than QNAM does, at least)

In D25961#617057, @apol wrote:

+1 cool stuff, much better than using kwallet for it.

Thanks! :)

Have you checked how well it works regarding startup? In Discover we do a bunch of queries at start for every KNSEngine and we need to make sure we don't block the whole thing (more than QNAM does, at least)

Not yet, no; the intention is to postpone the qnam creation in there (which is currently a touch on the ugly side, and also not acceptable given the whole redirection situation), and then also a mix of caching and delayed creation for the accounts stuff, so... yeah, bit of work to be done in that bit, for thought i'd delay that until it actually works :)

nicolasfella added inline comments.
attica-kde/kdeplugin/kdeplatformdependent.cpp
165

This sounds like a use case for KCMultiDialog, like e.g. in https://invent.kde.org/kde/kdeconnect-kde/-/blob/master/settings/main.cpp

leinir added inline comments.Feb 26 2020, 8:28 AM
attica-kde/kdeplugin/kdeplatformdependent.cpp
165

That certainly could be! i'll take a look, thanks for the hint :)

@leinir it seems like this was merged so I guess this should be closed?

leinir abandoned this revision.Dec 14 2020, 5:42 PM

@leinir it seems like this was merged so I guess this should be closed?

Probably make sense, yeah - it basically got moved to invent and worked on there, so it would really want closing anyway :)