Skip to content

Commit

Permalink
Only use one auth methode
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@TheOneRing
TheOneRing committed May 22, 2024
1 parent 0efe85b commit 40b29d2
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 17 deletions.
5 changes: 5 additions & 0 deletions changelog/unreleased/11646
View file
Original file line number Diff line number Diff line change
@@ -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
31 changes: 22 additions & 9 deletions src/libsync/creds/oauth.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -432,23 +432,26 @@ void OAuth::finalize(const QPointer<QTcpSocket> &socket, const QString &accessTo
emit result(LoggedIn, accessToken, refreshToken);
}

QNetworkReply *OAuth::postTokenRequest(const QList<QPair<QString, QString>> &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<QPair<QString, QString>> { { 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
Expand Down Expand Up @@ -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;
Expand Down
8 changes: 6 additions & 2 deletions src/libsync/creds/oauth.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -95,7 +97,7 @@ class OWNCLOUDSYNC_EXPORT OAuth : public QObject

virtual void fetchWellKnown();

QNetworkReply *postTokenRequest(const QList<QPair<QString, QString>> &queryItems);
QNetworkReply *postTokenRequest(QUrlQuery &&queryItems);


private:
Expand All @@ -111,6 +113,8 @@ class OWNCLOUDSYNC_EXPORT OAuth : public QObject
QString _redirectUrl;
QByteArray _pkceCodeVerifier;
QByteArray _state;

TokenEndpointAuthMethods _endpointAuthMethod = TokenEndpointAuthMethods::client_secret_basic;
};

/**
Expand Down
28 changes: 22 additions & 6 deletions test/testoauth.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}

Expand Down

0 comments on commit 40b29d2

Please sign in to comment.