Commit 0cf0de17 authored by Jan Blackquill's avatar Jan Blackquill 🌈 Committed by Jan Blackquill
Browse files

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.

BUG: 437286
parent 3de294d8
Pipeline #208537 passed with stage
in 6 minutes and 33 seconds
......@@ -135,40 +135,48 @@ void User::loadData()
bool userDataChanged = false;
if (mUid != m_dbusIface->uid()) {
mUid = m_dbusIface->uid();
mOriginalUid = mUid;
userDataChanged = true;
Q_EMIT uidChanged();
}
if (mName != m_dbusIface->userName()) {
mName = m_dbusIface->userName();
mOriginalName = mName;
userDataChanged = true;
Q_EMIT nameChanged();
}
if (mFace != QUrl(m_dbusIface->iconFile())) {
mFace = QUrl(m_dbusIface->iconFile());
mOriginalFace = mFace;
mFaceValid = QFileInfo::exists(mFace.toString());
mOriginalFaceValid = mFaceValid;
userDataChanged = true;
Q_EMIT faceChanged();
Q_EMIT faceValidChanged();
}
if (mRealName != m_dbusIface->realName()) {
mRealName = m_dbusIface->realName();
mOriginalRealName = mRealName;
userDataChanged = true;
Q_EMIT realNameChanged();
}
if (mEmail != m_dbusIface->email()) {
mEmail = m_dbusIface->email();
mOriginalEmail = mEmail;
userDataChanged = true;
Q_EMIT emailChanged();
}
const auto administrator = (m_dbusIface->accountType() == 1);
if (mAdministrator != administrator) {
mAdministrator = administrator;
mOriginalAdministrator = mAdministrator;
userDataChanged = true;
Q_EMIT administratorChanged();
}
const auto loggedIn = (mUid == getuid());
if (mLoggedIn != loggedIn) {
mLoggedIn = loggedIn;
mOriginalLoggedIn = mLoggedIn;
userDataChanged = true;
}
if (userDataChanged) {
......@@ -231,7 +239,19 @@ QDBusObjectPath User::path() const
void User::apply()
{
auto job = new UserApplyJob(m_dbusIface, mName, mEmail, mRealName, mFace.toString().replace("file://", ""), mAdministrator ? 1 : 0);
const auto opt = [](bool cond, auto v) {
if (cond) {
return std::optional<decltype(v)>(v);
}
return std::optional<decltype(v)>();
};
auto job =
new UserApplyJob(m_dbusIface,
opt(mOriginalName != mName, mName),
opt(mOriginalEmail != mEmail, mEmail),
opt(mOriginalRealName != mRealName, mRealName),
opt(mOriginalFace != mFace, mFace.toString().replace("file://", "")),
opt(mOriginalAdministrator != mAdministrator, mAdministrator ? 1 : 0));
connect(job, &UserApplyJob::result, this, [this, job] {
switch (static_cast<UserApplyJob::Error>(job->error())) {
case UserApplyJob::Error::PermissionDenied:
......@@ -264,7 +284,12 @@ bool User::loggedIn() const
return mLoggedIn;
}
UserApplyJob::UserApplyJob(QPointer<OrgFreedesktopAccountsUserInterface> dbusIface, QString name, QString email, QString realname, QString icon, int type)
UserApplyJob::UserApplyJob(QPointer<OrgFreedesktopAccountsUserInterface> dbusIface,
std::optional<QString> name,
std::optional<QString> email,
std::optional<QString> realname,
std::optional<QString> icon,
std::optional<int> type)
: KJob()
, m_name(name)
, m_email(email)
......@@ -293,23 +318,27 @@ void UserApplyJob::start()
// This avoids settings any data when the user thinks they aborted the transaction, see https://bugs.kde.org/show_bug.cgi?id=425036
// Subsequent calls do not trigger the auth dialog again
auto setAccount = asyncCall(m_dbusIface, "SetAccountType", {m_type});
setAccount.waitForFinished();
if (setAccount.isError()) {
setError(setAccount.error());
qCWarning(KCMUSERS) << setAccount.error().name() << setAccount.error().message();
emitResult();
return;
if (m_type.has_value()) {
auto setAccount = asyncCall(m_dbusIface, "SetAccountType", {*m_type});
setAccount.waitForFinished();
if (setAccount.isError()) {
setError(setAccount.error());
qCWarning(KCMUSERS) << setAccount.error().name() << setAccount.error().message();
emitResult();
return;
}
}
const std::multimap<QString, QString> set = {
const std::multimap<std::optional<QString>, QString> set = {
{m_name, "SetUserName"},
{m_email, "SetEmail"},
{m_realname, "SetRealName"},
};
for (auto const &x : set) {
auto resp = asyncCall(m_dbusIface, x.second, {x.first});
if (!x.first.has_value())
continue;
auto resp = asyncCall(m_dbusIface, x.second, {*x.first});
resp.waitForFinished();
if (resp.isError()) {
setError(resp.error());
......@@ -320,8 +349,8 @@ void UserApplyJob::start()
}
// Icon is special, since we want to resize it.
{
QImage icon(m_icon);
if (m_icon.has_value()) {
QImage icon(*m_icon);
// 256dp square is plenty big for an avatar and will definitely be smaller than 1MB
QImage scaled = icon.scaled(QSize(256, 256), Qt::KeepAspectRatio, Qt::SmoothTransformation);
......
......@@ -13,6 +13,8 @@
#include <QPointer>
#include <QUrl>
#include <optional>
class OrgFreedesktopAccountsUserInterface;
class QDBusError;
......@@ -21,7 +23,12 @@ class UserApplyJob : public KJob
Q_OBJECT
public:
UserApplyJob(QPointer<OrgFreedesktopAccountsUserInterface> dbusIface, QString name, QString email, QString realname, QString icon, int type);
UserApplyJob(QPointer<OrgFreedesktopAccountsUserInterface> dbusIface,
std::optional<QString> name,
std::optional<QString> email,
std::optional<QString> realname,
std::optional<QString> icon,
std::optional<int> type);
void start() override;
enum class Error {
......@@ -35,12 +42,11 @@ public:
private:
void setError(const QDBusError &error);
QString m_name;
QString m_email;
QString m_realname;
QString m_icon;
int m_type;
int m_jobs;
std::optional<QString> m_name;
std::optional<QString> m_email;
std::optional<QString> m_realname;
std::optional<QString> m_icon;
std::optional<int> m_type;
QPointer<OrgFreedesktopAccountsUserInterface> m_dbusIface;
};
......@@ -115,13 +121,21 @@ Q_SIGNALS:
private:
int mUid = 0;
int mOriginalUid = 0;
QString mName = QString();
QString mOriginalName = QString();
QString mRealName = QString();
QString mOriginalRealName = QString();
QString mEmail = QString();
QString mOriginalEmail = QString();
QUrl mFace = QUrl();
QUrl mOriginalFace = QUrl();
bool mAdministrator = false;
bool mOriginalAdministrator = false;
bool mFaceValid = false;
bool mOriginalFaceValid = false;
bool mLoggedIn = false;
bool mOriginalLoggedIn = false;
QDBusObjectPath mPath;
QPointer<OrgFreedesktopAccountsUserInterface> m_dbusIface;
};
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment