From c05733e56eb3f35fccd3a9214d8164d055e941b1 Mon Sep 17 00:00:00 2001 From: Martin Ortbauer Date: Fri, 7 Jan 2022 11:46:36 +0100 Subject: [PATCH 1/2] fix redirect_uri for token_endpoint in oauth2 * the serverPort was accessed after the server close --- src/libsync/creds/oauth.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/libsync/creds/oauth.cpp b/src/libsync/creds/oauth.cpp index 68ca30689ac..1d3359c4a9e 100644 --- a/src/libsync/creds/oauth.cpp +++ b/src/libsync/creds/oauth.cpp @@ -227,9 +227,6 @@ void OAuth::startAuthentication() httpReplyAndClose(socket, QByteArrayLiteral("400 Bad Request"), QByteArrayLiteral("400 Bad Request

400 Bad Request

")); return; } - // we only allow one response - qCDebug(lcOauth) << "Recieved the first valid request, stoping to listen"; - _server.close(); auto job = postTokenRequest({ { QStringLiteral("grant_type"), QStringLiteral("authorization_code") }, @@ -237,6 +234,11 @@ void OAuth::startAuthentication() { QStringLiteral("redirect_uri"), QStringLiteral("%1:%2").arg(_redirectUrl, QString::number(_server.serverPort())) }, { QStringLiteral("code_verifier"), QString::fromUtf8(_pkceCodeVerifier) }, }); + + // we only allow one response + qCDebug(lcOauth) << "Recieved the first valid request, stoping to listen"; + _server.close(); + QObject::connect(job, &SimpleNetworkJob::finishedSignal, this, [this, socket](QNetworkReply *reply) { const auto jsonData = reply->readAll(); QJsonParseError jsonParseError; From fbe4b0c15e3b67bf44b8238ce0591b2539ae1b1b Mon Sep 17 00:00:00 2001 From: Martin Ortbauer Date: Fri, 7 Jan 2022 11:50:30 +0100 Subject: [PATCH 2/2] add some configuration to oauth2 auth * add option oauthPrompt with default value true to disable it the prompt parameter for the authorization_endpoint optionally, ory hydra doesn't support that option and the oauth2 flow would fail * add option oauthBasicAuth with default value true to disable sending of the Authorization header for the token_endpoint, ory hydra only allows either client_secret_basic or client_secret_post not a mix of both --- src/libsync/configfile.cpp | 27 +++++++++++++++++++++++++++ src/libsync/configfile.h | 8 ++++++++ src/libsync/creds/oauth.cpp | 16 ++++++++++++---- 3 files changed, 47 insertions(+), 4 deletions(-) diff --git a/src/libsync/configfile.cpp b/src/libsync/configfile.cpp index 6d52dc9d9a1..6f50796e29f 100644 --- a/src/libsync/configfile.cpp +++ b/src/libsync/configfile.cpp @@ -91,6 +91,8 @@ const QString newBigFolderSizeLimitC() { return QStringLiteral("newBigFolderSize const QString useNewBigFolderSizeLimitC() { return QStringLiteral("useNewBigFolderSizeLimit"); } const QString confirmExternalStorageC() { return QStringLiteral("confirmExternalStorage"); } const QString moveToTrashC() { return QStringLiteral("moveToTrash"); } +const QString oauthPromptC() { return QStringLiteral("oauthPrompt"); } +const QString oauthBasicAuthC() { return QStringLiteral("oauthBasicAuth"); } } QString ConfigFile::_confDir = QString(); @@ -752,6 +754,31 @@ void ConfigFile::setPromptDeleteFiles(bool promptDeleteFiles) settings.setValue(promptDeleteC(), promptDeleteFiles); } +bool ConfigFile::oauthPrompt() const +{ + QSettings settings(configFile(), QSettings::IniFormat); + return settings.value(oauthPromptC(), true).toBool(); +} + +void ConfigFile::setOauthPrompt(bool oauthPrompt) +{ + QSettings settings(configFile(), QSettings::IniFormat); + settings.setValue(oauthPromptC(), oauthPrompt); +} + +bool ConfigFile::oauthBasicAuth() const +{ + QSettings settings(configFile(), QSettings::IniFormat); + return settings.value(oauthBasicAuthC(), true).toBool(); +} + +void ConfigFile::setOauthBasicAuth(bool oauthBasicAuth) +{ + QSettings settings(configFile(), QSettings::IniFormat); + settings.setValue(oauthBasicAuthC(), oauthBasicAuth); +} + + bool ConfigFile::monoIcons() const { QSettings settings(configFile(), QSettings::IniFormat); diff --git a/src/libsync/configfile.h b/src/libsync/configfile.h index 1f64a90680b..542978ae811 100644 --- a/src/libsync/configfile.h +++ b/src/libsync/configfile.h @@ -138,6 +138,14 @@ class OWNCLOUDSYNC_EXPORT ConfigFile bool confirmExternalStorage() const; void setConfirmExternalStorage(bool); + //disable sending of the prompt parameter for the oauth2 authorization_endpoint + bool oauthPrompt() const; + void setOauthPrompt(bool); + + //disable sending of basic auth header for the oauth2 token endpoint + bool oauthBasicAuth() const; + void setOauthBasicAuth(bool); + /** If we should move the files deleted on the server in the trash */ bool moveToTrash() const; void setMoveToTrash(bool); diff --git a/src/libsync/creds/oauth.cpp b/src/libsync/creds/oauth.cpp index 1d3359c4a9e..098f0a68816 100644 --- a/src/libsync/creds/oauth.cpp +++ b/src/libsync/creds/oauth.cpp @@ -14,6 +14,7 @@ #include "creds/oauth.h" +#include "configfile.h" #include "account.h" #include "common/version.h" #include "credentialmanager.h" @@ -405,11 +406,14 @@ void OAuth::finalize(const QPointer &socket, const QString &accessTo SimpleNetworkJob *OAuth::postTokenRequest(const QList> &queryItems) { + ConfigFile cfg; const QUrl requestTokenUrl = _tokenEndpoint.isEmpty() ? Utility::concatUrlPath(_account->url(), QStringLiteral("/index.php/apps/oauth2/api/v1/token")) : _tokenEndpoint; QNetworkRequest req; - const QByteArray basicAuth = QStringLiteral("%1:%2").arg(_clientId, _clientSecret).toUtf8().toBase64(); - req.setRawHeader("Authorization", "Basic " + basicAuth); - req.setAttribute(HttpCredentials::DontAddCredentialsAttribute, true); + if (cfg.oauthBasicAuth()){ + const QByteArray basicAuth = QStringLiteral("%1:%2").arg(_clientId, _clientSecret).toUtf8().toBase64(); + req.setRawHeader("Authorization", "Basic " + basicAuth); + req.setAttribute(HttpCredentials::DontAddCredentialsAttribute, true); + } QUrlQuery arguments; arguments.setQueryItems(QList> { { QStringLiteral("client_id"), _clientId }, @@ -433,6 +437,7 @@ QByteArray OAuth::generateRandomString(size_t size) const QUrl OAuth::authorisationLink() const { + ConfigFile cfg; Q_ASSERT(_server.isListening()); QUrlQuery query; const QByteArray code_challenge = QCryptographicHash::hash(_pkceCodeVerifier, QCryptographicHash::Sha256) @@ -443,9 +448,12 @@ QUrl OAuth::authorisationLink() const { QStringLiteral("code_challenge"), QString::fromLatin1(code_challenge) }, { QStringLiteral("code_challenge_method"), QStringLiteral("S256") }, { QStringLiteral("scope"), Theme::instance()->openIdConnectScopes() }, - { QStringLiteral("prompt"), Theme::instance()->openIdConnectPrompt() }, { QStringLiteral("state"), QString::fromUtf8(_state) } }); + if (cfg.oauthPrompt()){ + query.addQueryItem(QStringLiteral("prompt"), Theme::instance()->openIdConnectPrompt()); + } + if (!_account->davUser().isNull()) { const QString davUser = _account->davUser().replace(QLatin1Char('+'), QStringLiteral("%2B")); // Issue #7762; // open id connect