From ee5de37805e1c548430923d763e02d2ca263235b Mon Sep 17 00:00:00 2001 From: Martin Rotter Date: Thu, 25 Feb 2021 14:36:23 +0100 Subject: [PATCH] massive work, enhanced oauth2 post-login lambdas, simplified interfaces for acc editing --- src/librssguard/network-web/oauth2service.cpp | 11 +++++-- src/librssguard/network-web/oauth2service.h | 11 +++---- .../services/abstract/serviceroot.cpp | 1 + .../gmail/gui/formeditgmailaccount.cpp | 15 ++++------ .../gmail/gui/gmailaccountdetails.cpp | 13 -------- .../services/greader/greaderserviceroot.cpp | 10 ++++--- .../greader/gui/formeditgreaderaccount.cpp | 2 +- .../services/inoreader/definitions.h | 3 -- .../gui/formeditinoreaderaccount.cpp | 24 +++++++-------- .../inoreader/gui/inoreaderaccountdetails.cpp | 30 +++++++++++-------- .../inoreader/inoreaderserviceroot.cpp | 17 +++++------ .../network/inoreadernetworkfactory.cpp | 9 ++++++ .../owncloud/gui/formeditowncloudaccount.cpp | 2 +- .../services/owncloud/owncloudserviceroot.cpp | 9 +++--- .../tt-rss/gui/formeditttrssaccount.cpp | 4 +-- .../services/tt-rss/ttrssserviceroot.cpp | 9 +++--- 16 files changed, 86 insertions(+), 84 deletions(-) diff --git a/src/librssguard/network-web/oauth2service.cpp b/src/librssguard/network-web/oauth2service.cpp index cd69de977..a20401e0b 100644 --- a/src/librssguard/network-web/oauth2service.cpp +++ b/src/librssguard/network-web/oauth2service.cpp @@ -47,7 +47,8 @@ OAuth2Service::OAuth2Service(const QString& auth_url, const QString& token_url, const QString& client_secret, const QString& scope, QObject* parent) : QObject(parent), m_id(QString::number(QRandomGenerator::global()->generate())), m_timerId(-1), - m_redirectionHandler(new OAuthHttpHandler(tr("You can close this window now. Go back to %1.").arg(APP_NAME), this)) { + m_redirectionHandler(new OAuthHttpHandler(tr("You can close this window now. Go back to %1.").arg(APP_NAME), this)), + m_functorOnLogin({}) { m_tokenGrantType = QSL("authorization_code"); m_tokenUrl = QUrl(token_url); m_authUrl = auth_url; @@ -229,6 +230,7 @@ void OAuth2Service::tokenRequestFinished(QNetworkReply* network_reply) { << "Obtained refresh token" << QUOTE_W_SPACE(refreshToken()) << "- expires on date/time" << QUOTE_W_SPACE_DOT(tokensExpireIn()); + m_functorOnLogin(); emit tokensRetrieved(accessToken(), refreshToken(), expires); } @@ -297,7 +299,9 @@ void OAuth2Service::setRefreshToken(const QString& refresh_token) { startRefreshTimer(); } -bool OAuth2Service::login() { +bool OAuth2Service::login(const std::function& functor_when_logged_in) { + m_functorOnLogin = {}; + if (!m_redirectionHandler->isListening()) { qCriticalNN << LOGSEC_OAUTH << "Cannot log-in because OAuth redirection handler is not listening."; @@ -312,6 +316,8 @@ bool OAuth2Service::login() { bool did_token_expire = tokensExpireIn().isNull() || tokensExpireIn() < QDateTime::currentDateTime().addSecs(-120); bool does_token_exist = !refreshToken().isEmpty(); + m_functorOnLogin = functor_when_logged_in; + // We refresh current tokens only if: // 1. We have some existing refresh token. // AND @@ -325,6 +331,7 @@ bool OAuth2Service::login() { return false; } else { + functor_when_logged_in(); return true; } } diff --git a/src/librssguard/network-web/oauth2service.h b/src/librssguard/network-web/oauth2service.h index a0e7ef47f..093c6c32d 100644 --- a/src/librssguard/network-web/oauth2service.h +++ b/src/librssguard/network-web/oauth2service.h @@ -26,11 +26,14 @@ #define OAUTH2SERVICE_H #include -#include #include "network-web/oauthhttphandler.h" #include "network-web/silentnetworkaccessmanager.h" +#include + +#include + class OAuth2Service : public QObject { Q_OBJECT @@ -96,10 +99,7 @@ class OAuth2Service : public QObject { // // Returns true, if user is already logged in (final state). // Returns false, if user is NOT logged in (asynchronous flow). - // - // NOTE: This can be called ONLY on main GUI thread, - // because widgets may be displayed. - bool login(); + bool login(const std::function& functor_when_logged_in = {}); // Removes all state data and stops redirection handler. void logout(bool stop_redirection_handler = true); @@ -131,6 +131,7 @@ class OAuth2Service : public QObject { QString m_scope; SilentNetworkAccessManager m_networkManager; OAuthHttpHandler* m_redirectionHandler; + std::function m_functorOnLogin; }; #endif // OAUTH2SERVICE_H diff --git a/src/librssguard/services/abstract/serviceroot.cpp b/src/librssguard/services/abstract/serviceroot.cpp index e1c07c454..6ebd51d9f 100644 --- a/src/librssguard/services/abstract/serviceroot.cpp +++ b/src/librssguard/services/abstract/serviceroot.cpp @@ -390,6 +390,7 @@ void ServiceRoot::syncIn() { setIcon(original_icon); itemChanged(getSubTree()); + requestItemExpand({ this }, true); } void ServiceRoot::performInitialAssembly(const Assignment& categories, const Assignment& feeds, const QList& labels) { diff --git a/src/librssguard/services/gmail/gui/formeditgmailaccount.cpp b/src/librssguard/services/gmail/gui/formeditgmailaccount.cpp index 77f77bfa6..1eb377754 100644 --- a/src/librssguard/services/gmail/gui/formeditgmailaccount.cpp +++ b/src/librssguard/services/gmail/gui/formeditgmailaccount.cpp @@ -22,12 +22,6 @@ FormEditGmailAccount::FormEditGmailAccount(QWidget* parent) void FormEditGmailAccount::apply() { FormAccountDetails::apply(); - if (!m_creatingNew) { - // Disable "Cancel" button because all changes made to - // existing account are always saved anyway. - m_ui.m_buttonBox->button(QDialogButtonBox::StandardButton::Cancel)->setVisible(false); - } - // Make sure that the data copied from GUI are used for brand new login. account()->network()->oauth()->logout(false); account()->network()->oauth()->setClientId(m_details->m_ui.m_txtAppId->lineEdit()->text()); @@ -42,9 +36,6 @@ void FormEditGmailAccount::apply() { if (!m_creatingNew) { account()->completelyRemoveAllData(); - - // Account data are erased, it is similar to situation - // where we start the account after it was freshly added. account()->start(true); } } @@ -52,6 +43,12 @@ void FormEditGmailAccount::apply() { void FormEditGmailAccount::loadAccountData() { FormAccountDetails::loadAccountData(); + if (!m_creatingNew) { + // Disable "Cancel" button because all changes made to + // existing account are always saved anyway. + m_ui.m_buttonBox->button(QDialogButtonBox::StandardButton::Cancel)->setVisible(false); + } + m_details->m_oauth = account()->network()->oauth(); m_details->hookNetwork(); diff --git a/src/librssguard/services/gmail/gui/gmailaccountdetails.cpp b/src/librssguard/services/gmail/gui/gmailaccountdetails.cpp index 8cdea4d3f..8f50397cd 100644 --- a/src/librssguard/services/gmail/gui/gmailaccountdetails.cpp +++ b/src/librssguard/services/gmail/gui/gmailaccountdetails.cpp @@ -58,21 +58,8 @@ GmailAccountDetails::GmailAccountDetails(QWidget* parent) void GmailAccountDetails::testSetup() { m_oauth->logout(); - -#if defined(GMAIL_OFFICIAL_SUPPORT) - if (m_ui.m_txtAppId->lineEdit()->text().isEmpty() || m_ui.m_txtAppKey->lineEdit()->text().isEmpty()) { - m_oauth->setClientId(TextFactory::decrypt(GMAIL_CLIENT_ID, OAUTH_DECRYPTION_KEY)); - m_oauth->setClientSecret(TextFactory::decrypt(GMAIL_CLIENT_SECRET, OAUTH_DECRYPTION_KEY)); - } - else { -#endif m_oauth->setClientId(m_ui.m_txtAppId->lineEdit()->text()); m_oauth->setClientSecret(m_ui.m_txtAppKey->lineEdit()->text()); - -#if defined(GMAIL_OFFICIAL_SUPPORT) -} -#endif - m_oauth->setRedirectUrl(m_ui.m_txtRedirectUrl->lineEdit()->text()); if (m_oauth->login()) { diff --git a/src/librssguard/services/greader/greaderserviceroot.cpp b/src/librssguard/services/greader/greaderserviceroot.cpp index d7bb72608..57785b431 100755 --- a/src/librssguard/services/greader/greaderserviceroot.cpp +++ b/src/librssguard/services/greader/greaderserviceroot.cpp @@ -51,11 +51,13 @@ bool GreaderServiceRoot::deleteViaGui() { } void GreaderServiceRoot::start(bool freshly_activated) { - Q_UNUSED(freshly_activated) - loadFromDatabase(); - loadCacheFromFile(); + if (!freshly_activated) { + loadFromDatabase(); + loadCacheFromFile(); - if (childCount() <= 3) { + } + + if (getSubTreeFeeds().isEmpty()) { syncIn(); } } diff --git a/src/librssguard/services/greader/gui/formeditgreaderaccount.cpp b/src/librssguard/services/greader/gui/formeditgreaderaccount.cpp index bdcd93543..e5a43353f 100755 --- a/src/librssguard/services/greader/gui/formeditgreaderaccount.cpp +++ b/src/librssguard/services/greader/gui/formeditgreaderaccount.cpp @@ -34,7 +34,7 @@ void FormEditGreaderAccount::apply() { if (!m_creatingNew) { account()->completelyRemoveAllData(); - account()->syncIn(); + account()->start(true); } } diff --git a/src/librssguard/services/inoreader/definitions.h b/src/librssguard/services/inoreader/definitions.h index 8d117bc95..487cf9f14 100644 --- a/src/librssguard/services/inoreader/definitions.h +++ b/src/librssguard/services/inoreader/definitions.h @@ -8,9 +8,6 @@ #define INOREADER_OAUTH_AUTH_URL "https://www.inoreader.com/oauth2/auth" #define INOREADER_REG_API_URL "https://www.inoreader.com/developers/register-app" -#define INOREADER_OAUTH_CLI_ID "999999019" -#define INOREADER_OAUTH_CLI_KEY "k4bkOJ5Jj1erc52tmniluKtU6lZdNoZc" - #define INOREADER_REFRESH_TOKEN_KEY "refresh_token" #define INOREADER_ACCESS_TOKEN_KEY "access_token" diff --git a/src/librssguard/services/inoreader/gui/formeditinoreaderaccount.cpp b/src/librssguard/services/inoreader/gui/formeditinoreaderaccount.cpp index 613b640ac..ad20ce603 100644 --- a/src/librssguard/services/inoreader/gui/formeditinoreaderaccount.cpp +++ b/src/librssguard/services/inoreader/gui/formeditinoreaderaccount.cpp @@ -25,16 +25,13 @@ FormEditInoreaderAccount::FormEditInoreaderAccount(QWidget* parent) void FormEditInoreaderAccount::apply() { FormAccountDetails::apply(); - if (m_creatingNew) { - // We transfer refresh token to avoid the need to login once more, - // then we delete testing OAuth service. - account()->network()->oauth()->setRefreshToken(m_details->m_oauth->refreshToken()); - account()->network()->oauth()->setAccessToken(m_details->m_oauth->accessToken()); - account()->network()->oauth()->setTokensExpireIn(m_details->m_oauth->tokensExpireIn()); - m_details->m_oauth->logout(true); - m_details->m_oauth->deleteLater(); + if (!m_creatingNew) { + // Disable "Cancel" button because all changes made to + // existing account are always saved anyway. + m_ui.m_buttonBox->button(QDialogButtonBox::StandardButton::Cancel)->setVisible(false); } + account()->network()->oauth()->logout(false); account()->network()->oauth()->setClientId(m_details->m_ui.m_txtAppId->lineEdit()->text()); account()->network()->oauth()->setClientSecret(m_details->m_ui.m_txtAppKey->lineEdit()->text()); account()->network()->oauth()->setRedirectUrl(m_details->m_ui.m_txtRedirectUrl->lineEdit()->text()); @@ -47,18 +44,17 @@ void FormEditInoreaderAccount::apply() { if (!m_creatingNew) { account()->completelyRemoveAllData(); - account()->syncIn(); + account()->start(true); } } void FormEditInoreaderAccount::loadAccountData() { FormAccountDetails::loadAccountData(); - if (m_details->m_oauth != nullptr) { - // We will use live OAuth service for testing. - m_details->m_oauth->logout(true); - delete m_details->m_oauth; - m_details->m_oauth = nullptr; + if (!m_creatingNew) { + // Disable "Cancel" button because all changes made to + // existing account are always saved anyway. + m_ui.m_buttonBox->button(QDialogButtonBox::StandardButton::Cancel)->setVisible(false); } m_details->m_oauth = account()->network()->oauth(); diff --git a/src/librssguard/services/inoreader/gui/inoreaderaccountdetails.cpp b/src/librssguard/services/inoreader/gui/inoreaderaccountdetails.cpp index 6c8665ea5..978290f8a 100755 --- a/src/librssguard/services/inoreader/gui/inoreaderaccountdetails.cpp +++ b/src/librssguard/services/inoreader/gui/inoreaderaccountdetails.cpp @@ -10,17 +10,20 @@ #include "services/inoreader/network/inoreadernetworkfactory.h" InoreaderAccountDetails::InoreaderAccountDetails(QWidget* parent) - : QWidget(parent), m_oauth(new OAuth2Service(INOREADER_OAUTH_AUTH_URL, INOREADER_OAUTH_TOKEN_URL, - {}, {}, INOREADER_OAUTH_SCOPE, this)) { + : QWidget(parent), m_oauth(nullptr) { m_ui.setupUi(this); GuiUtilities::setLabelAsNotice(*m_ui.m_lblInfo, true); - m_ui.m_lblInfo->setText(tr("Specified redirect URL must start with \"http://localhost\" and " - "must be configured in your OAuth \"application\".\n\n" - "It is highly recommended to create your own \"App ID\". " - "Because predefined one may be limited due to usage quotas if used by " - "too many users simultaneously.")); +#if defined(INOREADER_OFFICIAL_SUPPORT) + m_ui.m_lblInfo->setText(tr("There are some preconfigured OAuth tokens so you do not have to fill in your " + "client ID/secret, but it is strongly recommended to obtain your " + "own as it preconfigured tokens have limited global usage quota. If you wash " + "to use preconfigured tokens, simply leave those fields empty and make sure " + "to leave default value of redirect URL.")); +#else + m_ui.m_lblInfo->setText(tr("You have to fill in your client ID/secret and also fill in correct redirect URL.")); +#endif m_ui.m_lblTestResult->setStatus(WidgetWithStatus::StatusType::Information, tr("Not tested yet."), @@ -46,12 +49,9 @@ InoreaderAccountDetails::InoreaderAccountDetails(QWidget* parent) m_ui.m_spinLimitMessages->setMaximum(INOREADER_MAX_BATCH_SIZE); emit m_ui.m_txtUsername->lineEdit()->textChanged(m_ui.m_txtUsername->lineEdit()->text()); - - m_ui.m_txtAppId->lineEdit()->setText(INOREADER_OAUTH_CLI_ID); - m_ui.m_txtAppKey->lineEdit()->setText(INOREADER_OAUTH_CLI_KEY); - m_ui.m_txtRedirectUrl->lineEdit()->setText(QString(OAUTH_REDIRECT_URI) + - QL1C(':') + - QString::number(OAUTH_REDIRECT_URI_PORT)); + emit m_ui.m_txtAppId->lineEdit()->textChanged(m_ui.m_txtAppId->lineEdit()->text()); + emit m_ui.m_txtAppKey->lineEdit()->textChanged(m_ui.m_txtAppKey->lineEdit()->text()); + emit m_ui.m_txtRedirectUrl->lineEdit()->textChanged(m_ui.m_txtAppKey->lineEdit()->text()); hookNetwork(); } @@ -113,7 +113,11 @@ void InoreaderAccountDetails::checkOAuthValue(const QString& value) { if (line_edit != nullptr) { if (value.isEmpty()) { +#if defined(INOREADER_OFFICIAL_SUPPORT) + line_edit->setStatus(WidgetWithStatus::StatusType::Ok, tr("Preconfigured client ID/secret will be used.")); +#else line_edit->setStatus(WidgetWithStatus::StatusType::Error, tr("Empty value is entered.")); +#endif } else { line_edit->setStatus(WidgetWithStatus::StatusType::Ok, tr("Some value is entered.")); diff --git a/src/librssguard/services/inoreader/inoreaderserviceroot.cpp b/src/librssguard/services/inoreader/inoreaderserviceroot.cpp index 01c38b7e5..b5e109de3 100644 --- a/src/librssguard/services/inoreader/inoreaderserviceroot.cpp +++ b/src/librssguard/services/inoreader/inoreaderserviceroot.cpp @@ -94,16 +94,15 @@ bool InoreaderServiceRoot::supportsCategoryAdding() const { } void InoreaderServiceRoot::start(bool freshly_activated) { - Q_UNUSED(freshly_activated) - - loadFromDatabase(); - loadCacheFromFile(); - - if (childCount() <= 3) { - syncIn(); + if (!freshly_activated) { + loadFromDatabase(); + loadCacheFromFile(); } - else { - m_network->oauth()->login(); + + if (getSubTreeFeeds().isEmpty()) { + m_network->oauth()->login([this]() { + syncIn(); + }); } } diff --git a/src/librssguard/services/inoreader/network/inoreadernetworkfactory.cpp b/src/librssguard/services/inoreader/network/inoreadernetworkfactory.cpp index f210ec84f..bc3c3c781 100644 --- a/src/librssguard/services/inoreader/network/inoreadernetworkfactory.cpp +++ b/src/librssguard/services/inoreader/network/inoreadernetworkfactory.cpp @@ -53,6 +53,15 @@ void InoreaderNetworkFactory::setBatchSize(int batch_size) { } void InoreaderNetworkFactory::initializeOauth() { +#if defined(INOREADER_OFFICIAL_SUPPORT) + m_oauth2->setClientSecretId(TextFactory::decrypt(INOREADER_CLIENT_ID, OAUTH_DECRYPTION_KEY)); + m_oauth2->setClientSecretSecret(TextFactory::decrypt(INOREADER_CLIENT_SECRET, OAUTH_DECRYPTION_KEY)); +#endif + + m_oauth2->setRedirectUrl(QString(OAUTH_REDIRECT_URI) + + QL1C(':') + + QString::number(OAUTH_REDIRECT_URI_PORT)); + connect(m_oauth2, &OAuth2Service::tokensRetrieveError, this, &InoreaderNetworkFactory::onTokensError); connect(m_oauth2, &OAuth2Service::authFailed, this, &InoreaderNetworkFactory::onAuthFailed); connect(m_oauth2, &OAuth2Service::tokensRetrieved, this, [this](QString access_token, QString refresh_token, int expires_in) { diff --git a/src/librssguard/services/owncloud/gui/formeditowncloudaccount.cpp b/src/librssguard/services/owncloud/gui/formeditowncloudaccount.cpp index 632bdf52a..54dac1e7e 100644 --- a/src/librssguard/services/owncloud/gui/formeditowncloudaccount.cpp +++ b/src/librssguard/services/owncloud/gui/formeditowncloudaccount.cpp @@ -35,7 +35,7 @@ void FormEditOwnCloudAccount::apply() { if (!m_creatingNew) { account()->completelyRemoveAllData(); - account()->syncIn(); + account()->start(true); } } diff --git a/src/librssguard/services/owncloud/owncloudserviceroot.cpp b/src/librssguard/services/owncloud/owncloudserviceroot.cpp index d4c8d5378..32ae88cf7 100644 --- a/src/librssguard/services/owncloud/owncloudserviceroot.cpp +++ b/src/librssguard/services/owncloud/owncloudserviceroot.cpp @@ -63,11 +63,12 @@ bool OwnCloudServiceRoot::supportsCategoryAdding() const { } void OwnCloudServiceRoot::start(bool freshly_activated) { - Q_UNUSED(freshly_activated) - loadFromDatabase(); - loadCacheFromFile(); + if (!freshly_activated) { + loadFromDatabase(); + loadCacheFromFile(); + } - if (childCount() <= 3) { + if (getSubTreeFeeds().isEmpty()) { syncIn(); } } diff --git a/src/librssguard/services/tt-rss/gui/formeditttrssaccount.cpp b/src/librssguard/services/tt-rss/gui/formeditttrssaccount.cpp index 7d250aee7..225f3aaea 100644 --- a/src/librssguard/services/tt-rss/gui/formeditttrssaccount.cpp +++ b/src/librssguard/services/tt-rss/gui/formeditttrssaccount.cpp @@ -20,6 +20,7 @@ FormEditTtRssAccount::FormEditTtRssAccount(QWidget* parent) void FormEditTtRssAccount::apply() { FormAccountDetails::apply(); + account()->network()->logout(m_account->networkProxy()); account()->network()->setUrl(m_details->m_ui.m_txtUrl->lineEdit()->text()); account()->network()->setUsername(m_details->m_ui.m_txtUsername->lineEdit()->text()); account()->network()->setPassword(m_details->m_ui.m_txtPassword->lineEdit()->text()); @@ -33,9 +34,8 @@ void FormEditTtRssAccount::apply() { accept(); if (!m_creatingNew) { - account()->network()->logout(m_account->networkProxy()); account()->completelyRemoveAllData(); - account()->syncIn(); + account()->start(true); } } diff --git a/src/librssguard/services/tt-rss/ttrssserviceroot.cpp b/src/librssguard/services/tt-rss/ttrssserviceroot.cpp index a17a6daa8..591af88cd 100644 --- a/src/librssguard/services/tt-rss/ttrssserviceroot.cpp +++ b/src/librssguard/services/tt-rss/ttrssserviceroot.cpp @@ -37,11 +37,12 @@ ServiceRoot::LabelOperation TtRssServiceRoot::supportedLabelOperations() const { } void TtRssServiceRoot::start(bool freshly_activated) { - Q_UNUSED(freshly_activated) - loadFromDatabase(); - loadCacheFromFile(); + if (!freshly_activated) { + loadFromDatabase(); + loadCacheFromFile(); + } - if (childCount() <= 3) { + if (getSubTreeFeeds().isEmpty()) { syncIn(); } }