Commit 7c742583 authored by Krzysztof Nowicki's avatar Krzysztof Nowicki

EWS: Refactor server connection and password retrieval

In the old behaviour the resource would attempt to connect to the server
at resource construction time, which is not the way it should work. This
was a corner cut done in early development, when I was still a novice
and I wanted it to just work. As we know cutting corners tends to
backfire eventually...

The updated implementation connects to the server when the resource is
brought online.

The password request code also received refactoring. Instead of
synchronously waiting for KWallet it now performs the opening
asynchronously together with a timeout for safety.

A new unit test ensures that the code behaves as designed.
parent 39d5dba1
......@@ -51,6 +51,7 @@ static const QVector<StringPair> userAgents = {
EwsConfigDialog::EwsConfigDialog(EwsResource *parentResource, EwsClient &client, WId wId)
: QDialog()
, mParentResource(parentResource)
, mSettings(new EwsSettings(wId))
{
if (wId) {
KWindowSystem::setMainWindow(this, wId);
......@@ -74,12 +75,12 @@ EwsConfigDialog::EwsConfigDialog(EwsResource *parentResource, EwsClient &client,
mUi->setupUi(mainWidget);
mUi->accountName->setText(parentResource->name());
mSubWidget = new EwsSubscriptionWidget(client, mParentResource->settings(), this);
mSubWidget = new EwsSubscriptionWidget(client, mSettings.data(), this);
mUi->subscriptionTabLayout->addWidget(mSubWidget);
mConfigManager = new KConfigDialogManager(this, mParentResource->settings());
mConfigManager = new KConfigDialogManager(this, mSettings.data());
mConfigManager->updateWidgets();
switch (mParentResource->settings()->retrievalMethod()) {
switch (mSettings->retrievalMethod()) {
case 0:
mUi->pollRadioButton->setChecked(true);
break;
......@@ -102,23 +103,24 @@ EwsConfigDialog::EwsConfigDialog(EwsResource *parentResource, EwsClient &client,
mTryConnectNeeded = baseUrlEmpty;
QString password;
mParentResource->settings()->requestPassword(password, false);
mUi->passwordEdit->setPassword(password);
connect(mSettings.data(), &EwsSettings::passwordRequestFinished, mUi->passwordEdit,
&KPasswordLineEdit::setPassword);
mSettings->requestPassword(false);
int selectedIndex = -1;
int i = 0;
Q_FOREACH (const StringPair &item, userAgents) {
mUi->userAgentCombo->addItem(item.first, item.second);
if (mParentResource->settings()->userAgent() == item.second) {
if (mSettings->userAgent() == item.second) {
selectedIndex = i;
}
i++;
}
mUi->userAgentCombo->addItem(i18nc("User Agent", "Custom"));
if (!mParentResource->settings()->userAgent().isEmpty()) {
if (!mSettings->userAgent().isEmpty()) {
mUi->userAgentGroupBox->setChecked(true);
mUi->userAgentCombo->setCurrentIndex(selectedIndex >= 0 ? selectedIndex : mUi->userAgentCombo->count() - 1);
mUi->userAgentEdit->setText(mParentResource->settings()->userAgent());
mUi->userAgentEdit->setText(mSettings->userAgent());
} else {
mUi->userAgentCombo->setCurrentIndex(mUi->userAgentCombo->count());
}
......@@ -157,32 +159,32 @@ void EwsConfigDialog::save()
mParentResource->setName(mUi->accountName->text());
mConfigManager->updateSettings();
if (mUi->pollRadioButton->isChecked()) {
mParentResource->settings()->setRetrievalMethod(0);
mSettings->setRetrievalMethod(0);
} else {
mParentResource->settings()->setRetrievalMethod(1);
mSettings->setRetrievalMethod(1);
}
/* Erase the subscription id in case subscription is disabled or its parameters changed. This
* fill force creation of a new subscription. */
if (!mSubWidget->subscriptionEnabled() ||
(mSubWidget->subscribedList() != mParentResource->settings()->serverSubscriptionList())) {
mParentResource->settings()->setEventSubscriptionId(QString());
mParentResource->settings()->setEventSubscriptionWatermark(QString());
(mSubWidget->subscribedList() != mSettings->serverSubscriptionList())) {
mSettings->setEventSubscriptionId(QString());
mSettings->setEventSubscriptionWatermark(QString());
}
mParentResource->settings()->setServerSubscription(mSubWidget->subscriptionEnabled());
mSettings->setServerSubscription(mSubWidget->subscriptionEnabled());
if (mSubWidget->subscribedListValid()) {
mParentResource->settings()->setServerSubscriptionList(mSubWidget->subscribedList());
mSettings->setServerSubscriptionList(mSubWidget->subscribedList());
}
if (mUi->userAgentGroupBox->isChecked()) {
mParentResource->settings()->setUserAgent(mUi->userAgentEdit->text());
mSettings->setUserAgent(mUi->userAgentEdit->text());
} else {
mParentResource->settings()->setUserAgent(QString());
mSettings->setUserAgent(QString());
}
mParentResource->settings()->setPassword(mUi->passwordEdit->password());
mParentResource->settings()->save();
mSettings->setPassword(mUi->passwordEdit->password());
mSettings->save();
}
void EwsConfigDialog::performAutoDiscovery()
......
......@@ -36,6 +36,7 @@ class EwsAutodiscoveryJob;
class EwsGetFolderRequest;
class EwsProgressDialog;
class EwsSubscriptionWidget;
class EwsSettings;
class EwsConfigDialog : public QDialog
{
......@@ -69,6 +70,7 @@ private:
bool mTryConnectNeeded = false;
EwsProgressDialog *mProgressDialog = nullptr;
EwsSubscriptionWidget *mSubWidget = nullptr;
QScopedPointer<EwsSettings> mSettings;
};
#endif
......@@ -99,14 +99,6 @@ EwsResource::EwsResource(const QString &id)
: Akonadi::ResourceBase(id), mTagsRetrieved(false), mReconnectTimeout(InitialReconnectTimeout),
mSettings(new EwsSettings(winIdForDialogs()))
{
//setName(i18n("Microsoft Exchange"));
mEwsClient.setUrl(mSettings->baseUrl());
mSettings->requestPassword(mPassword, true);
if (!mSettings->hasDomain()) {
mEwsClient.setCredentials(mSettings->username(), mPassword);
} else {
mEwsClient.setCredentials(mSettings->domain() + QChar::fromLatin1('\\') + mSettings->username(), mPassword);
}
mEwsClient.setUserAgent(mSettings->userAgent());
mEwsClient.setEnableNTLMv2(mSettings->enableNTLMv2());
......@@ -124,13 +116,6 @@ EwsResource::EwsResource(const QString &id)
setScheduleAttributeSyncBeforeItemSync(true);
if (mSettings->baseUrl().isEmpty()) {
setOnline(false);
Q_EMIT status(NotConfigured, i18nc("@info:status", "No server configured yet."));
} else {
resetUrl();
}
// Load the sync state
QByteArray data = QByteArray::fromBase64(mSettings->syncState().toAscii());
if (!data.isEmpty()) {
......@@ -155,6 +140,9 @@ EwsResource::EwsResource(const QString &id)
QMetaObject::invokeMethod(this, "delayedInit", Qt::QueuedConnection);
connect(this, &AgentBase::reloadConfiguration, this, &EwsResource::reloadConfig);
connect(mSettings.data(), &EwsSettings::passwordRequestFinished, this,
&EwsResource::passwordRequestFinished);
}
EwsResource::~EwsResource()
......@@ -461,14 +449,29 @@ void EwsResource::reloadConfig()
{
mSubManager.reset(nullptr);
mEwsClient.setUrl(mSettings->baseUrl());
mSettings->requestPassword(mPassword, false);
if (mSettings->domain().isEmpty()) {
mEwsClient.setCredentials(mSettings->username(), mPassword);
mSettings->requestPassword(true);
}
void EwsResource::passwordRequestFinished(const QString &password)
{
mPassword = password;
if (mPassword.isNull()) {
setOnline(false);
Q_EMIT status(NotConfigured, i18nc("@info:status", "No password configured."));
} else {
mEwsClient.setCredentials(mSettings->domain() + QChar::fromLatin1('\\') + mSettings->username(), mPassword);
if (mSettings->domain().isEmpty()) {
mEwsClient.setCredentials(mSettings->username(), mPassword);
} else {
mEwsClient.setCredentials(mSettings->domain() + QChar::fromLatin1('\\') + mSettings->username(), mPassword);
}
mSettings->save();
if (mSettings->baseUrl().isEmpty()) {
setOnline(false);
Q_EMIT status(NotConfigured, i18nc("@info:status", "No server configured yet."));
} else {
resetUrl();
}
}
mSettings->save();
resetUrl();
}
void EwsResource::configure(WId windowId)
......@@ -1248,7 +1251,7 @@ void EwsResource::saveState()
void EwsResource::doSetOnline(bool online)
{
if (online) {
resetUrl();
reloadConfig();
} else {
mSubManager.reset(nullptr);
}
......
......@@ -144,6 +144,7 @@ private:
void doRetrieveCollections();
int reconnectTimeout();
void passwordRequestFinished(const QString &password);
EwsClient mEwsClient;
Akonadi::Collection mRootCollection;
......
......@@ -17,83 +17,171 @@
Boston, MA 02110-1301, USA.
*/
#include <ewssettings.h>
#include "ewssettings.h"
#include <QPointer>
#include <KPasswordDialog>
#include <KWallet/KWallet>
#include <KLocalizedString>
#include "ewsresource_debug.h"
static const QString ewsWalletFolder = QStringLiteral("akonadi-ews");
// Allow unittest to override this to shorten test execution
#ifndef WALLET_TIMEOUT
#define WALLET_TIMEOUT 30000
#endif
EwsSettings::EwsSettings(WId windowId)
: EwsSettingsBase(), mWindowId(windowId)
: EwsSettingsBase(), mWindowId(windowId), mWalletReadTimer(this), mWalletWriteTimer(this)
{
mWalletReadTimer.setInterval(WALLET_TIMEOUT);
mWalletReadTimer.setSingleShot(true);
connect(&mWalletReadTimer, &QTimer::timeout, this, [this]() {
qCWarning(EWSRES_LOG) << "Timeout waiting for wallet open for read";
onWalletOpenedForRead(false);
});
mWalletWriteTimer.setInterval(WALLET_TIMEOUT);
mWalletWriteTimer.setSingleShot(true);
connect(&mWalletWriteTimer, &QTimer::timeout, this, [this]() {
qCWarning(EWSRES_LOG) << "Timeout waiting for wallet open for write";
onWalletOpenedForWrite(false);
});
}
EwsSettings::~EwsSettings()
{
}
bool EwsSettings::requestPassword(QString &password, bool ask)
void EwsSettings::requestPassword(bool ask)
{
bool status = true;
bool status = false;
if (!mPassword.isEmpty()) {
password = mPassword;
return true;
qCDebug(EWSRES_LOG) << "requestPassword: start";
if (!mPassword.isNull()) {
qCDebug(EWSRES_LOG) << "requestPassword: password already set";
Q_EMIT passwordRequestFinished(mPassword);
return;
}
QScopedPointer<KWallet::Wallet> wallet(KWallet::Wallet::openWallet(KWallet::Wallet::NetworkWallet(),
mWindowId));
if (wallet && wallet->isOpen()) {
if (wallet->hasFolder(QStringLiteral("akonadi-ews"))) {
wallet->setFolder(QStringLiteral("akonadi-ews"));
wallet->readPassword(config()->name(), password);
if (!mWallet) {
mWallet = KWallet::Wallet::openWallet(KWallet::Wallet::NetworkWallet(),
mWindowId, KWallet::Wallet::Asynchronous);
if (mWallet) {
connect(mWallet.data(), &KWallet::Wallet::walletOpened, this,
&EwsSettings::onWalletOpenedForRead);
mWalletReadTimer.start();
return;
} else {
wallet->createFolder(QStringLiteral("akonadi-ews"));
qCWarning(EWSRES_LOG) << "Failed to open wallet";
}
} else {
status = false;
}
if (mWallet && mWallet->isOpen()) {
mPassword = readPassword();
mWallet.clear();
status = true;
}
if (!status) {
if (!ask) {
return false;
}
QPointer<KPasswordDialog> dlg = new KPasswordDialog(nullptr);
dlg->setModal(true);
dlg->setPrompt(i18n("Please enter password for user '%1' and Exchange account '%2'.",
username(), email()));
dlg->setAttribute(Qt::WA_DeleteOnClose);
if (dlg->exec() == QDialog::Accepted) {
password = dlg->password();
setPassword(password);
qCDebug(EWSRES_LOG) << "requestPassword: Not allowed to ask";
} else {
qCDebug(EWSRES_LOG) << "requestPassword: Requesting interactively";
QPointer<KPasswordDialog> dlg = new KPasswordDialog(nullptr);
dlg->setModal(true);
dlg->setPrompt(i18n("Please enter password for user '%1' and Exchange account '%2'.",
username(), email()));
if (dlg->exec() == QDialog::Accepted) {
mPassword = dlg->password();
setPassword(mPassword);
}
delete dlg;
return false;
}
delete dlg;
}
return true;
Q_EMIT passwordRequestFinished(mPassword);
return;
}
QString EwsSettings::readPassword() const
{
QString password;
if (mWallet->hasFolder(ewsWalletFolder)) {
mWallet->setFolder(ewsWalletFolder);
mWallet->readPassword(config()->name(), password);
} else {
mWallet->createFolder(ewsWalletFolder);
}
return password;
}
void EwsSettings::onWalletOpenedForRead(bool success)
{
qCDebug(EWSRES_LOG) << "onWalletOpenedForRead: start" << success;
mWalletReadTimer.stop();
if (mWallet) {
if (success) {
if (mPassword.isNull()) {
mPassword = readPassword();
}
qCDebug(EWSRES_LOG) << "onWalletOpenedForRead: got password";
Q_EMIT passwordRequestFinished(mPassword);
} else {
qCDebug(EWSRES_LOG) << "onWalletOpenedForRead: failed to retrieve password";
Q_EMIT passwordRequestFinished(QString());
}
mWallet.clear();
}
}
void EwsSettings::setPassword(const QString &password)
{
if (password.isNull()) {
qCWarning(EWSRES_LOG) << "Trying to set a null password";
return;
}
mPassword = password;
QScopedPointer<KWallet::Wallet> wallet(KWallet::Wallet::openWallet(KWallet::Wallet::NetworkWallet(),
mWindowId));
if (wallet && wallet->isOpen()) {
if (!wallet->hasFolder(QStringLiteral("akonadi-ews"))) {
wallet->createFolder(QStringLiteral("akonadi-ews"));
/* If a pending password request is running, satisfy it. */
if (mWallet) {
onWalletOpenedForRead(true);
}
mWallet = KWallet::Wallet::openWallet(KWallet::Wallet::NetworkWallet(),
mWindowId, KWallet::Wallet::Asynchronous);
if (mWallet) {
connect(mWallet.data(), &KWallet::Wallet::walletOpened, this, &EwsSettings::onWalletOpenedForWrite);
mWalletWriteTimer.start();
} else {
qCWarning(EWSRES_LOG) << "Failed to open wallet";
}
}
void EwsSettings::onWalletOpenedForWrite(bool success)
{
mWalletWriteTimer.stop();
if (success) {
if (!mWallet->hasFolder(ewsWalletFolder)) {
mWallet->createFolder(ewsWalletFolder);
}
wallet->setFolder(QStringLiteral("akonadi-ews"));
wallet->writePassword(config()->name(), password);
mWallet->setFolder(ewsWalletFolder);
mWallet->writePassword(config()->name(), mPassword);
}
mWallet.clear();
}
void EwsSettings::setTestPassword(const QString &password)
{
if (password.isNull()) {
qCWarning(EWSRES_LOG) << "Trying to set a null password";
return;
}
mPassword = password;
if (mWallet) {
onWalletOpenedForRead(true);
}
}
......@@ -22,6 +22,13 @@
#include "ewssettingsbase.h"
#include <QTimer>
#include <QPointer>
namespace KWallet {
class Wallet;
}
class EwsSettings : public EwsSettingsBase
{
Q_OBJECT
......@@ -30,14 +37,22 @@ public:
explicit EwsSettings(WId windowId);
~EwsSettings() override;
bool requestPassword(QString &password, bool ask);
void requestPassword(bool ask);
public Q_SLOTS:
Q_SCRIPTABLE void setPassword(const QString &password);
Q_SCRIPTABLE void setTestPassword(const QString &password);
Q_SIGNALS:
void passwordRequestFinished(const QString &password);
private Q_SLOTS:
void onWalletOpenedForRead(bool success);
void onWalletOpenedForWrite(bool success);
private:
QString readPassword() const;
WId mWindowId;
QString mPassword;
QPointer<KWallet::Wallet> mWallet;
QTimer mWalletReadTimer;
QTimer mWalletWriteTimer;
};
#endif
......
......@@ -33,4 +33,13 @@ akonadi_ews_add_ut(ewsgetitemrequest_ut)
akonadi_ews_add_ut(ewsunsubscriberequest_ut)
akonadi_ews_add_ut(ewsattachment_ut)
add_executable(ewssettings_ut
ewssettings_ut.cpp
${CMAKE_CURRENT_SOURCE_DIR}/../../ewssettings.cpp
${CMAKE_CURRENT_BINARY_DIR}/../../ewssettingsbase.cpp
${CMAKE_CURRENT_BINARY_DIR}/../../ewsres_debug.cpp)
target_link_libraries(ewssettings_ut Qt5::Core Qt5::Test KF5::WidgetsAddons KF5::I18n KF5::ConfigCore KF5::ConfigGui KF5::Wallet)
target_compile_definitions(ewssettings_ut PUBLIC -DWALLET_TIMEOUT=2000)
add_test(NAME ewssettings_ut COMMAND ewssettings_ut)
This diff is collapsed.
Markdown is supported
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