Skip to content

Commit cf3d734

Browse files
committed
Properly encode scope and prompt items in OAuth URL
The scope and the prompt items of an OAuth query can be branded and can contain characters that are not valid without encoding them. This change makes sure that those get encoded properly. Fixes: #11472
1 parent 523c720 commit cf3d734

File tree

6 files changed

+16
-20
lines changed

6 files changed

+16
-20
lines changed

changelog/unreleased/11472

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
Bugfix: Properly encode scope and prompt items in OAuth URL
2+
3+
Fixed a bug where the scope and prompt items of an OAuth query would not
4+
be encoded, resulting in an invalid request.
5+
6+
https://github.com/owncloud/client/issues/11472
7+
https://github.com/owncloud/client/pull/11479

src/gui/accountsettings.cpp

-5
Original file line numberDiff line numberDiff line change
@@ -736,11 +736,6 @@ void AccountSettings::slotAccountStateChanged()
736736
connect(cred, &HttpCredentialsGui::authorisationLinkChanged, contentWidget,
737737
[cred, contentWidget] { contentWidget->setUrl(cred->authorisationLink()); });
738738

739-
connect(contentWidget, &OAuthLoginWidget::copyUrlToClipboardButtonClicked, _askForOAuthLoginDialog, [](const QUrl &url) {
740-
// TODO: use authorisationLinkAsync
741-
qApp->clipboard()->setText(url.toString());
742-
});
743-
744739
connect(contentWidget, &OAuthLoginWidget::openBrowserButtonClicked, cred, &HttpCredentialsGui::openBrowser);
745740

746741
connect(_askForOAuthLoginDialog, &LoginRequiredDialog::rejected, this, [this]() {

src/gui/loginrequireddialog/oauthloginwidget.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ OAuthLoginWidget::OAuthLoginWidget(QWidget *parent)
4040
});
4141
connect(_ui->copyUrlToClipboardButton, &QPushButton::clicked, this, [this] {
4242
Q_ASSERT(_url.isValid());
43-
Q_EMIT copyUrlToClipboardButtonClicked(_url);
43+
qApp->clipboard()->setText(_url.toString(QUrl::FullyEncoded));
4444
});
4545

4646
// depending on the theme we have to use a light or dark icon

src/gui/loginrequireddialog/oauthloginwidget.h

-1
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ class OAuthLoginWidget : public AbstractLoginWidget
5252

5353
Q_SIGNALS:
5454
void openBrowserButtonClicked(const QUrl &url);
55-
void copyUrlToClipboardButtonClicked(const QUrl &url);
5655

5756
private:
5857
::Ui::OAuthLoginWidget *_ui;

src/gui/newwizard/pages/oauthcredentialssetupwizardpage.cpp

-2
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,6 @@ OAuthCredentialsSetupWizardPage::OAuthCredentialsSetupWizardPage(const QUrl &ser
4242
// TODO: move to OAuthLoginWidget
4343
_ui->oauthLoginWidget->setOpenBrowserButtonText(tr("Reopen Browser"));
4444
});
45-
connect(
46-
_ui->oauthLoginWidget, &OAuthLoginWidget::copyUrlToClipboardButtonClicked, this, [](const QUrl &url) { qApp->clipboard()->setText(url.toString()); });
4745
connect(this, &AbstractSetupWizardPage::pageDisplayed, _ui->oauthLoginWidget, qOverload<>(&OAuthLoginWidget::setFocus));
4846

4947
_ui->topLabel->setText(tr("Please use your browser to log in to %1.").arg(Theme::instance()->appNameGUI()));

src/libsync/creds/oauth.cpp

+8-11
Original file line numberDiff line numberDiff line change
@@ -443,9 +443,8 @@ QNetworkReply *OAuth::postTokenRequest(const QList<QPair<QString, QString>> &que
443443
req.setAttribute(HttpCredentials::DontAddCredentialsAttribute, true);
444444

445445
QUrlQuery arguments;
446-
arguments.setQueryItems(QList<QPair<QString, QString>> { { QStringLiteral("client_id"), _clientId },
447-
{ QStringLiteral("client_secret"), _clientSecret },
448-
{ QStringLiteral("scope"), Theme::instance()->openIdConnectScopes() } }
446+
arguments.setQueryItems(QList<QPair<QString, QString>>{{QStringLiteral("client_id"), _clientId}, {QStringLiteral("client_secret"), _clientSecret},
447+
{QStringLiteral("scope"), QString::fromUtf8(QUrl::toPercentEncoding(Theme::instance()->openIdConnectScopes()))}}
449448
<< queryItems);
450449
req.setUrl(requestTokenUrl);
451450
return _networkAccessManager->post(req, arguments.toString(QUrl::FullyEncoded).toUtf8());
@@ -466,14 +465,12 @@ QUrl OAuth::authorisationLink() const
466465

467466
const QByteArray code_challenge = QCryptographicHash::hash(_pkceCodeVerifier, QCryptographicHash::Sha256)
468467
.toBase64(QByteArray::Base64UrlEncoding | QByteArray::OmitTrailingEquals);
469-
QUrlQuery query { { QStringLiteral("response_type"), QStringLiteral("code") },
470-
{ QStringLiteral("client_id"), _clientId },
471-
{ QStringLiteral("redirect_uri"), QStringLiteral("%1:%2").arg(_redirectUrl, QString::number(_server.serverPort())) },
472-
{ QStringLiteral("code_challenge"), QString::fromLatin1(code_challenge) },
473-
{ QStringLiteral("code_challenge_method"), QStringLiteral("S256") },
474-
{ QStringLiteral("scope"), Theme::instance()->openIdConnectScopes() },
475-
{ QStringLiteral("prompt"), Theme::instance()->openIdConnectPrompt() },
476-
{ QStringLiteral("state"), QString::fromUtf8(_state) } };
468+
QUrlQuery query{{QStringLiteral("response_type"), QStringLiteral("code")}, {QStringLiteral("client_id"), _clientId},
469+
{QStringLiteral("redirect_uri"), QStringLiteral("%1:%2").arg(_redirectUrl, QString::number(_server.serverPort()))},
470+
{QStringLiteral("code_challenge"), QString::fromLatin1(code_challenge)}, {QStringLiteral("code_challenge_method"), QStringLiteral("S256")},
471+
{QStringLiteral("scope"), QString::fromUtf8(QUrl::toPercentEncoding(Theme::instance()->openIdConnectScopes()))},
472+
{QStringLiteral("prompt"), QString::fromUtf8(QUrl::toPercentEncoding(Theme::instance()->openIdConnectPrompt()))},
473+
{QStringLiteral("state"), QString::fromUtf8(_state)}};
477474

478475
if (!_davUser.isEmpty()) {
479476
const QString davUser = QString::fromUtf8(QUrl::toPercentEncoding(_davUser)); // Issue #7762;

0 commit comments

Comments
 (0)