Skip to content

Commit e87d666

Browse files
erikjvTheOneRing
authored andcommitted
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 19e0dc4 commit e87d666

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/creds/httpcredentialsgui.cpp

-5
Original file line numberDiff line numberDiff line change
@@ -172,11 +172,6 @@ void HttpCredentialsGui::restartOAuth()
172172
tr("The account %1 is currently logged out.\n\nPlease authenticate using your browser.").arg(_account->displayName()));
173173

174174
auto *contentWidget = qobject_cast<OAuthLoginWidget *>(_loginRequiredDialog->contentWidget());
175-
connect(contentWidget, &OAuthLoginWidget::copyUrlToClipboardButtonClicked, _loginRequiredDialog, [](const QUrl &url) {
176-
// TODO: use authorisationLinkAsync
177-
qApp->clipboard()->setText(url.toString());
178-
});
179-
180175
connect(contentWidget, &OAuthLoginWidget::openBrowserButtonClicked, this, &HttpCredentialsGui::openBrowser);
181176
connect(_loginRequiredDialog, &LoginRequiredDialog::rejected, this, &HttpCredentials::requestLogout);
182177

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
@@ -420,9 +420,8 @@ QNetworkReply *OAuth::postTokenRequest(const QList<QPair<QString, QString>> &que
420420
req.setAttribute(HttpCredentials::DontAddCredentialsAttribute, true);
421421

422422
QUrlQuery arguments;
423-
arguments.setQueryItems(QList<QPair<QString, QString>> { { QStringLiteral("client_id"), _clientId },
424-
{ QStringLiteral("client_secret"), _clientSecret },
425-
{ QStringLiteral("scope"), Theme::instance()->openIdConnectScopes() } }
423+
arguments.setQueryItems(QList<QPair<QString, QString>>{{QStringLiteral("client_id"), _clientId}, {QStringLiteral("client_secret"), _clientSecret},
424+
{QStringLiteral("scope"), QString::fromUtf8(QUrl::toPercentEncoding(Theme::instance()->openIdConnectScopes()))}}
426425
<< queryItems);
427426
req.setUrl(requestTokenUrl);
428427
return _networkAccessManager->post(req, arguments.toString(QUrl::FullyEncoded).toUtf8());
@@ -443,14 +442,12 @@ QUrl OAuth::authorisationLink() const
443442

444443
const QByteArray code_challenge = QCryptographicHash::hash(_pkceCodeVerifier, QCryptographicHash::Sha256)
445444
.toBase64(QByteArray::Base64UrlEncoding | QByteArray::OmitTrailingEquals);
446-
QUrlQuery query { { QStringLiteral("response_type"), QStringLiteral("code") },
447-
{ QStringLiteral("client_id"), _clientId },
448-
{ QStringLiteral("redirect_uri"), QStringLiteral("%1:%2").arg(_redirectUrl, QString::number(_server.serverPort())) },
449-
{ QStringLiteral("code_challenge"), QString::fromLatin1(code_challenge) },
450-
{ QStringLiteral("code_challenge_method"), QStringLiteral("S256") },
451-
{ QStringLiteral("scope"), Theme::instance()->openIdConnectScopes() },
452-
{ QStringLiteral("prompt"), Theme::instance()->openIdConnectPrompt() },
453-
{ QStringLiteral("state"), QString::fromUtf8(_state) } };
445+
QUrlQuery query{{QStringLiteral("response_type"), QStringLiteral("code")}, {QStringLiteral("client_id"), _clientId},
446+
{QStringLiteral("redirect_uri"), QStringLiteral("%1:%2").arg(_redirectUrl, QString::number(_server.serverPort()))},
447+
{QStringLiteral("code_challenge"), QString::fromLatin1(code_challenge)}, {QStringLiteral("code_challenge_method"), QStringLiteral("S256")},
448+
{QStringLiteral("scope"), QString::fromUtf8(QUrl::toPercentEncoding(Theme::instance()->openIdConnectScopes()))},
449+
{QStringLiteral("prompt"), QString::fromUtf8(QUrl::toPercentEncoding(Theme::instance()->openIdConnectPrompt()))},
450+
{QStringLiteral("state"), QString::fromUtf8(_state)}};
454451

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

0 commit comments

Comments
 (0)