Commit 8a37b420 authored by Nate Graham's avatar Nate Graham
Browse files

Make initial setup much clearer

There are a lot of ways that initial Samba sharing setup can go wrong if
the distro hasn't set up everything perfectly out of the box. Right now
the wizard shows some error messages when problems are encountered, but
the logic fails to account for the full set of things that can go wrong
during initial setup, and as a result are sometimes displays misleading
error messages that can send the user down the wrong path.

This commit expands the set of conditions that are checked for, and
offers more verbose and actionable error messages specific to each of
them.

To accomplish this, the code is refactored to move nearly all of the
logic to the backend, with the QML side simply presenting the
information for whatever error was encountered.

The existing code already had logic for showing a friendly button to fix
the "you're not a member of the right group" error condition, which is
preserved here. TODOs are added in the place where other similar actions
to fix the other errors could be defined, which can be done later.

BUG: 425202
FIXED-IN: 22.12
parent c6ddc1de
Pipeline #213576 passed with stage
in 1 minute and 32 seconds
......@@ -28,77 +28,100 @@ GroupManager::GroupManager(QObject *parent)
connect(proc, QOverload<int, QProcess::ExitStatus>::of(&QProcess::finished), this, [this, proc] {
proc->deleteLater();
const QString path = QString::fromUtf8(proc->readAllStandardOutput().simplified());
m_ready = true;
Q_EMIT isReadyChanged();
QFileInfo info(path);
m_targetGroup = info.group();
m_user = KUser().loginName();
const QStringList groups = KUser(m_user).groupNames();
// First check to see if the path where user shares will be exported exists
if (path.isEmpty() || !info.exists()) {
return; // usershares may be disabled or path is invalid :|
m_errorText = xi18nc("@info:status", "This folder can't be shared because <filename>%1</filename> does not exist.", path);
Q_EMIT errorTextChanged();
m_errorExplanation = xi18nc("@info:status", "This error is caused by your distro not setting up Samba sharing properly. You can fix it yourself by creating that folder manually. Then close and re-open this window.");
Q_EMIT errorExplanationChanged();
// TODO: define a helpfulAction that creates the folder
}
if (info.isWritable()) {
m_isMember = true;
Q_EMIT isMemberChanged();
// Now see if the group is set to something valid
else if (m_targetGroup == QLatin1String("root")) {
m_errorText = xi18nc("@info:status", "This folder can't be shared because <filename>%1</filename> has its group owner inappropriately set to <resource>%2</resource>.", path, m_targetGroup);
Q_EMIT errorTextChanged();
m_errorExplanation = xi18nc("@info:status", "This error is caused by your distro not setting up Samba sharing properly. You can fix it yourself by changing that folder's group owner to <resource>usershares</resource> and making yourself a member of that group. Then restart the system.");
Q_EMIT errorExplanationChanged();
// TODO: define a helpfulAction that creates the group and applies it to the folder
}
m_targetGroup = info.group();
Q_EMIT targetGroupChanged();
// Now see if the user is a member of the group
else if (!groups.contains(m_targetGroup)) {
m_errorText = xi18nc("@info:status", "This folder can't be shared because your user account isn't a member of the <resource>%1</resource> group.", m_targetGroup);
Q_EMIT errorTextChanged();
m_errorExplanation = xi18nc("@info:status", "You can fix this by making your user a member of that group. Then restart the system.");
Q_EMIT errorExplanationChanged();
m_helpfulActionIcon = QStringLiteral("resource-group-new");
Q_EMIT helpfulActionIconChanged();
m_helpfulActionText = i18nc("action@button makes user a member of the samba share group", "Make me a Group Member");
Q_EMIT helpfulActionTextChanged();
m_helpfulAction = HelpfulAction::AddUserToGroup;
m_hasHelpfulAction = true;
Q_EMIT hasHelpfulActionChanged();
}
if (m_targetGroup != QLatin1String("root")) {
m_canMakeMember = true;
Q_EMIT canMakeMemberChanged();
// Now see if it's writable by the user
else if (!info.isWritable()) {
m_errorText = xi18nc("@info:status", "This folder can't be shared because your user account doesn't have permission to write into <filename>%1</filename>.", path);
Q_EMIT errorTextChanged();
m_errorExplanation = xi18nc("@info:status", "You can fix this by ensuring that the <resource>%1</resource> group has write permission for <filename>%2</filename>. Then close and re-open this window.", m_targetGroup, path);
Q_EMIT errorExplanationChanged();
// TODO: define a helpfulAction that adds group write permission to the folder
}
m_ready = true;
Q_EMIT isReadyChanged();
// If we got here without hitting any errors, everything should work
});
proc->start();
});
}
bool GroupManager::canMakeMember() const
{
return m_canMakeMember;
}
bool GroupManager::isReady() const
void GroupManager::performHelpfulAction()
{
return m_ready;
}
QString GroupManager::targetGroup() const
{
return m_targetGroup;
}
bool GroupManager::isMember() const
{
return m_isMember;
}
void GroupManager::makeMember()
{
Q_ASSERT(m_canMakeMember);
const QString user = KUser().loginName();
const QString group = m_targetGroup;
Q_ASSERT(!user.isEmpty());
Q_ASSERT(!group.isEmpty());
auto action = KAuth::Action(QStringLiteral("org.kde.filesharing.samba.addtogroup"));
action.setHelperId(QStringLiteral("org.kde.filesharing.samba"));
action.addArgument(QStringLiteral("user"), user);
action.addArgument(QStringLiteral("group"), group);
action.setDetailsV2({{KAuth::Action::AuthDetail::DetailMessage,
i18nc("@label kauth action description %1 is a username %2 a group name",
"Adding user '%1' to group '%2' so they may configure Samba user shares",
user,
group) }
});
KAuth::ExecuteJob *job = action.execute();
connect(job, &KAuth::ExecuteJob::result, this, [this, job] {
job->deleteLater();
if (job->error() != KAuth::ExecuteJob::NoError) {
Q_EMIT makeMemberError(job->errorString());
return;
switch (m_helpfulAction) {
case HelpfulAction::AddUserToGroup: {
const QString user = m_user;
const QString group = m_targetGroup;
Q_ASSERT(!user.isEmpty());
Q_ASSERT(!group.isEmpty());
auto action = KAuth::Action(QStringLiteral("org.kde.filesharing.samba.addtogroup"));
action.setHelperId(QStringLiteral("org.kde.filesharing.samba"));
action.addArgument(QStringLiteral("user"), user);
action.addArgument(QStringLiteral("group"), group);
action.setDetailsV2({{KAuth::Action::AuthDetail::DetailMessage,
i18nc("@label kauth action description %1 is a username %2 a group name",
"Adding user '%1' to group '%2' so they may configure Samba user shares",
user,
group) }
});
KAuth::ExecuteJob *job = action.execute();
connect(job, &KAuth::ExecuteJob::result, this, [this, job, user, group] {
job->deleteLater();
if (job->error() != KAuth::ExecuteJob::NoError) {
QString helpfulActionErrorText = job->errorString();
if (helpfulActionErrorText.isEmpty()) {
// unknown error :(
helpfulActionErrorText = i18nc("@info", "Failed to make user <resource>%1<resource> a member of group <resource>%2<resource>", user, group);
}
Q_EMIT helpfulActionError(helpfulActionErrorText);
return;
}
Q_EMIT needsReboot();
});
job->start();
break;
}
Q_EMIT madeMember();
});
job->start();
case HelpfulAction::None:
// Do nothing
break;
// no Default so we have to explicitly handle new enums in the future
}
}
......@@ -7,35 +7,44 @@
#include <QObject>
enum class HelpfulAction {
None,
AddUserToGroup
};
class GroupManager : public QObject
{
Q_OBJECT
Q_PROPERTY(bool ready READ isReady NOTIFY isReadyChanged)
Q_PROPERTY(bool member READ isMember NOTIFY isMemberChanged)
Q_PROPERTY(bool canMakeMember READ canMakeMember NOTIFY canMakeMemberChanged)
Q_PROPERTY(QString targetGroup READ targetGroup NOTIFY targetGroupChanged)
Q_PROPERTY(QString errorText MEMBER m_errorText NOTIFY errorTextChanged)
Q_PROPERTY(QString errorExplanation MEMBER m_errorExplanation NOTIFY errorExplanationChanged)
Q_PROPERTY(bool hasHelpfulAction MEMBER m_hasHelpfulAction NOTIFY hasHelpfulActionChanged)
Q_PROPERTY(QString helpfulActionIcon MEMBER m_helpfulActionIcon NOTIFY helpfulActionIconChanged)
Q_PROPERTY(QString helpfulActionText MEMBER m_helpfulActionText NOTIFY helpfulActionTextChanged)
Q_PROPERTY(bool ready MEMBER m_ready NOTIFY isReadyChanged)
public:
explicit GroupManager(QObject *parent = nullptr);
bool canMakeMember() const;
bool isReady() const;
QString targetGroup() const;
bool isMember() const;
public Q_SLOTS:
void makeMember();
void performHelpfulAction();
Q_SIGNALS:
void isReadyChanged();
void isMemberChanged();
void canMakeMemberChanged();
void madeMember();
void targetGroupChanged();
void makeMemberError(const QString &error);
void errorTextChanged();
void errorExplanationChanged();
void hasHelpfulActionChanged();
void helpfulActionIconChanged();
void helpfulActionTextChanged();
void helpfulActionError(const QString &error);
void needsReboot();
private:
bool m_canMakeMember = false;
bool m_isMember = false;
bool m_ready = false;
QString m_targetGroup;
QString m_user;
QString m_errorText;
QString m_errorExplanation;
HelpfulAction m_helpfulAction;
bool m_hasHelpfulAction = false;
QString m_helpfulActionIcon;
QString m_helpfulActionText;
};
......@@ -16,20 +16,15 @@ Item {
Samba.GroupManager {
id: manager
onReadyChanged: {
if (ready && member) { // already member nothing to do for us
if (ready && manager.errorText.length === 0) { // no error; nothing for us to do
stackReplace(pendingStack.pop())
}
}
onMadeMember: {
onNeedsReboot: () => {
stackReplace("RebootPage.qml")
}
onMakeMemberError: {
var text = error
if (text == "") { // unknown error :(
text = i18nc("@label failed to change user groups so they can manage shares",
"Group changes failed.")
}
errorMessage.text = text
onHelpfulActionError: (error) => {
actionErrorMessage.text = error
}
}
......@@ -44,7 +39,7 @@ Item {
visible: manager.ready
Kirigami.InlineMessage {
id: errorMessage
id: actionErrorMessage
Layout.alignment: Qt.AlignTop
Layout.fillWidth: true
type: Kirigami.MessageType.Error
......@@ -52,16 +47,14 @@ Item {
}
Kirigami.PlaceholderMessage {
text: manager.targetGroup ?
xi18nc("@label",
"To manage Samba user shares you need to be a member of the <resource>%1</resource> group.",
manager.targetGroup) :
i18nc("@label", "You appear to not have sufficient permissions to manage Samba user shares.")
icon.name: "emblem-error"
text: manager.errorText
explanation: manager.errorExplanation
helpfulAction: Kirigami.Action {
enabled: manager.canMakeMember
iconName: "resource-group-new"
text: i18nc("@button makes user a member of the samba share group", "Make me a Group Member")
onTriggered: manager.makeMember()
enabled: manager.hasHelpfulAction
iconName: manager.helpfulActionIcon
text: manager.helpfulActionText
onTriggered: manager.performHelpfulAction()
}
}
}
......
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