From 40b29d239b75533d0d777edf66702d6687167b45 Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Wed, 22 May 2024 13:11:46 +0200 Subject: [PATCH] Only use one auth methode Historically the client used both basic auth and the post body to provide the client id and secret. Some idp's however are picky and only accept one. --- changelog/unreleased/11646 | 5 +++++ src/libsync/creds/oauth.cpp | 31 ++++++++++++++++++++++--------- src/libsync/creds/oauth.h | 8 ++++++-- test/testoauth.cpp | 28 ++++++++++++++++++++++------ 4 files changed, 55 insertions(+), 17 deletions(-) create mode 100644 changelog/unreleased/11646 diff --git a/changelog/unreleased/11646 b/changelog/unreleased/11646 new file mode 100644 index 00000000000..5030a79d160 --- /dev/null +++ b/changelog/unreleased/11646 @@ -0,0 +1,5 @@ +Enhancement: Support `Active Directory Federation Service` as identity provider + +We changed the OAuth workflow to support `Active Directory Federation Service` + +https://github.com/owncloud/client/issues/11646 diff --git a/src/libsync/creds/oauth.cpp b/src/libsync/creds/oauth.cpp index d2f5041267d..cbb89625565 100644 --- a/src/libsync/creds/oauth.cpp +++ b/src/libsync/creds/oauth.cpp @@ -432,23 +432,26 @@ void OAuth::finalize(const QPointer &socket, const QString &accessTo emit result(LoggedIn, accessToken, refreshToken); } -QNetworkReply *OAuth::postTokenRequest(const QList> &queryItems) +QNetworkReply *OAuth::postTokenRequest(QUrlQuery &&queryItems) { const QUrl requestTokenUrl = _tokenEndpoint.isEmpty() ? Utility::concatUrlPath(_serverUrl, QStringLiteral("/index.php/apps/oauth2/api/v1/token")) : _tokenEndpoint; QNetworkRequest req; req.setTransferTimeout(defaultTimeoutMs()); - const QByteArray basicAuth = QStringLiteral("%1:%2").arg(_clientId, _clientSecret).toUtf8().toBase64(); - req.setRawHeader("Authorization", "Basic " + basicAuth); + switch (_endpointAuthMethod) { + case TokenEndpointAuthMethods::client_secret_basic: + req.setRawHeader("Authorization", "Basic " + QStringLiteral("%1:%2").arg(_clientId, _clientSecret).toUtf8().toBase64()); + break; + case TokenEndpointAuthMethods::client_secret_post: + queryItems.addQueryItem(QStringLiteral("client_id"), _clientId); + queryItems.addQueryItem(QStringLiteral("client_secret"), _clientSecret); + break; + } req.setHeader(QNetworkRequest::ContentTypeHeader, QStringLiteral("application/x-www-form-urlencoded; charset=UTF-8")); req.setAttribute(HttpCredentials::DontAddCredentialsAttribute, true); - QUrlQuery arguments; - arguments.setQueryItems(QList> { { QStringLiteral("client_id"), _clientId }, - { QStringLiteral("client_secret"), _clientSecret }, - { QStringLiteral("scope"), Theme::instance()->openIdConnectScopes() } } - << queryItems); + queryItems.addQueryItem(QStringLiteral("scope"), Theme::instance()->openIdConnectScopes()); req.setUrl(requestTokenUrl); - return _networkAccessManager->post(req, arguments.toString(QUrl::FullyEncoded).toUtf8()); + return _networkAccessManager->post(req, queryItems.toString(QUrl::FullyEncoded).toUtf8()); } QByteArray OAuth::generateRandomString(size_t size) const @@ -532,6 +535,16 @@ void OAuth::fetchWellKnown() _tokenEndpoint = QUrl::fromEncoded(data[QStringLiteral("token_endpoint")].toString().toUtf8()); _registrationEndpoint = QUrl::fromEncoded(data[QStringLiteral("registration_endpoint")].toString().toUtf8()); _redirectUrl = QStringLiteral("http://127.0.0.1"); + + const auto authMethods = data.value(QStringLiteral("token_endpoint_auth_methods_supported")).toArray(); + if (authMethods.contains(QStringLiteral("client_secret_basic"))) { + _endpointAuthMethod = TokenEndpointAuthMethods::client_secret_basic; + } else if (authMethods.contains(QStringLiteral("client_secret_post"))) { + _endpointAuthMethod = TokenEndpointAuthMethods::client_secret_post; + } else { + OC_ASSERT_X(false, qPrintable(QStringLiteral("Unsupported token_endpoint_auth_methods_supported: %1").arg(QDebug::toString(authMethods)))); + } + qCDebug(lcOauth) << "parsing .well-known reply successful, auth endpoint" << _authEndpoint << "and token endpoint" << _tokenEndpoint << "and registration endpoint" << _registrationEndpoint; diff --git a/src/libsync/creds/oauth.h b/src/libsync/creds/oauth.h index 5f5c5e4e98d..055ae302ee4 100644 --- a/src/libsync/creds/oauth.h +++ b/src/libsync/creds/oauth.h @@ -54,7 +54,9 @@ class OWNCLOUDSYNC_EXPORT OAuth : public QObject Q_OBJECT public: enum Result { NotSupported, LoggedIn, Error, ErrorInsecureUrl }; - Q_ENUM(Result); + Q_ENUM(Result) + enum class TokenEndpointAuthMethods { client_secret_basic, client_secret_post }; + Q_ENUM(TokenEndpointAuthMethods) OAuth(const QUrl &serverUrl, const QString &davUser, QNetworkAccessManager *networkAccessManager, const QVariantMap &dynamicRegistrationData, QObject *parent); ~OAuth() override; @@ -95,7 +97,7 @@ class OWNCLOUDSYNC_EXPORT OAuth : public QObject virtual void fetchWellKnown(); - QNetworkReply *postTokenRequest(const QList> &queryItems); + QNetworkReply *postTokenRequest(QUrlQuery &&queryItems); private: @@ -111,6 +113,8 @@ class OWNCLOUDSYNC_EXPORT OAuth : public QObject QString _redirectUrl; QByteArray _pkceCodeVerifier; QByteArray _state; + + TokenEndpointAuthMethods _endpointAuthMethod = TokenEndpointAuthMethods::client_secret_basic; }; /** diff --git a/test/testoauth.cpp b/test/testoauth.cpp index 31b38f2f490..5df5dfcfaf4 100644 --- a/test/testoauth.cpp +++ b/test/testoauth.cpp @@ -415,7 +415,9 @@ private slots: QJsonDocument jsondata(QJsonObject{ {QStringLiteral("authorization_endpoint"), QJsonValue(QStringLiteral("oauthtest://openidserver") + sOAuthTestServer.path() + QStringLiteral("/index.php/apps/oauth2/authorize"))}, - {QStringLiteral("token_endpoint"), QStringLiteral("oauthtest://openidserver/token_endpoint")}}); + {QStringLiteral("token_endpoint"), QStringLiteral("oauthtest://openidserver/token_endpoint")}, + {QStringLiteral("token_endpoint_auth_methods_supported"), QJsonArray{QStringLiteral("client_secret_post")}}, + }); return new FakePayloadReply(op, req, jsondata.toJson(), fakeAm); } @@ -490,7 +492,9 @@ private slots: {QStringLiteral("authorization_endpoint"), QJsonValue(QStringLiteral("oauthtest://openidserver") + sOAuthTestServer.path() + QStringLiteral("/index.php/apps/oauth2/authorize"))}, {QStringLiteral("token_endpoint"), QStringLiteral("oauthtest://openidserver/token_endpoint")}, - {QStringLiteral("registration_endpoint"), QStringLiteral("%1/clients-registrations").arg(localHost)}}); + {QStringLiteral("registration_endpoint"), QStringLiteral("%1/clients-registrations").arg(localHost)}, + {QStringLiteral("token_endpoint_auth_methods_supported"), QJsonArray{QStringLiteral("client_secret_basic")}}, + }); return new FakePayloadReply(op, req, jsondata.toJson(), fakeAm); } @@ -531,7 +535,10 @@ private slots: {QStringLiteral("authorization_endpoint"), QJsonValue(QStringLiteral("oauthtest://openidserver") + sOAuthTestServer.path() + QStringLiteral("/index.php/apps/oauth2/authorize"))}, {QStringLiteral("token_endpoint"), QStringLiteral("oauthtest://openidserver/token_endpoint")}, - {QStringLiteral("registration_endpoint"), QStringLiteral("%1/clients-registrations").arg(localHost)}}); + {QStringLiteral("registration_endpoint"), QStringLiteral("%1/clients-registrations").arg(localHost)}, + {QStringLiteral("token_endpoint_auth_methods_supported"), + QJsonArray{QStringLiteral("client_secret_basic"), QStringLiteral("client_secret_post")}}, + }); return new FakePayloadReply(op, req, jsondata.toJson(), fakeAm); } @@ -582,7 +589,10 @@ private slots: {QStringLiteral("authorization_endpoint"), QJsonValue(QStringLiteral("oauthtest://openidserver") + sOAuthTestServer.path() + QStringLiteral("/index.php/apps/oauth2/authorize"))}, {QStringLiteral("token_endpoint"), QStringLiteral("oauthtest://openidserver/token_endpoint")}, - {QStringLiteral("registration_endpoint"), QStringLiteral("%1/clients-registrations").arg(localHost)}}); + {QStringLiteral("registration_endpoint"), QStringLiteral("%1/clients-registrations").arg(localHost)}, + {QStringLiteral("token_endpoint_auth_methods_supported"), QJsonArray{QStringLiteral("client_secret_basic")}}, + + }); return new FakePayloadReply(op, req, jsondata.toJson(), fakeAm); } @@ -636,7 +646,10 @@ private slots: {QStringLiteral("authorization_endpoint"), QJsonValue(QStringLiteral("oauthtest://openidserver") + sOAuthTestServer.path() + QStringLiteral("/index.php/apps/oauth2/authorize"))}, {QStringLiteral("token_endpoint"), QStringLiteral("oauthtest://openidserver/token_endpoint")}, - {QStringLiteral("registration_endpoint"), QStringLiteral("%1/clients-registrations").arg(localHost)}}); + {QStringLiteral("registration_endpoint"), QStringLiteral("%1/clients-registrations").arg(localHost)}, + // this test explicitly check for the client secret in the post body + {QStringLiteral("token_endpoint_auth_methods_supported"), QJsonArray{QStringLiteral("client_secret_post")}}, + }); return new FakePayloadReply(op, req, jsondata.toJson(), fakeAm); } @@ -712,7 +725,10 @@ private slots: {QStringLiteral("authorization_endpoint"), QJsonValue(QStringLiteral("oauthtest://openidserver") + sOAuthTestServer.path() + QStringLiteral("/index.php/apps/oauth2/authorize"))}, {QStringLiteral("token_endpoint"), QStringLiteral("oauthtest://openidserver/token_endpoint")}, - {QStringLiteral("registration_endpoint"), QStringLiteral("%1/clients-registrations").arg(localHost)}}); + {QStringLiteral("registration_endpoint"), QStringLiteral("%1/clients-registrations").arg(localHost)}, + // this test explicitly check for the client secret in the post body + {QStringLiteral("token_endpoint_auth_methods_supported"), QJsonArray{QStringLiteral("client_secret_post")}}, + }); return new FakePayloadReply(op, req, jsondata.toJson(), fakeAm); }