OAuth2 QOAuthHttpServerReplyHandler HTTPS/SSL support?

[Alex94102]

@Alex94102 I suspect the OAuth2 library is broken. It doesn't URL Encode the POST request body, I verified that. The technique used doesn't work properly.

[SGaist]

Hi,

Would it be possible for you to provide a minimal compilable example that reproduces that behaviour ?

[Alex94102]

Hi,

so essentially I went down that path and the service call for retrieving the refresh_token was failing and I couldn't figure out why. To diagnose that I tested that with an echo server to print what was being sent and compare that with a working CURL call. What I noticed is that the parameters that were placed in the body were not URL encoded but they were left as is. Then I looked at the source code (see below) and this is the method where he's doing the POST (the body of the request is created by this line const QString data = query.toString(QUrl::FullyEncoded);

By replicating this functionality and properly encoding the parameter values only with QUrl::toPercentEncoding() everything works as it should. Let me know if you need more details...:

void QOAuth2AuthorizationCodeFlow::requestAccessToken(const QString &code)
{
Q_D(QOAuth2AuthorizationCodeFlow);
using Key = QAbstractOAuth2Private::OAuth2KeyString;

QVariantMap parameters;
QNetworkRequest request(d->accessTokenUrl);
QUrlQuery query;
parameters.insert(Key::grantType, QStringLiteral("authorization_code"));
parameters.insert(Key::code, QUrl::toPercentEncoding(code));
parameters.insert(Key::redirectUri, QUrl::toPercentEncoding(callback()));
parameters.insert(Key::clientIdentifier, QUrl::toPercentEncoding(d->clientIdentifier));
if (!d->clientIdentifierSharedKey.isEmpty())
    parameters.insert(Key::clientSharedSecret, d->clientIdentifierSharedKey);
if (d->modifyParametersFunction)
    d->modifyParametersFunction(Stage::RequestingAccessToken, &parameters);
query = QAbstractOAuthPrivate::createQuery(parameters);
request.setHeader(QNetworkRequest::ContentTypeHeader,
                  QStringLiteral("application/x-www-form-urlencoded"));

const QString data = query.toString(QUrl::FullyEncoded);
QNetworkReply *reply = d->networkAccessManager()->post(request, data.toUtf8());
d->currentReply = reply;
QAbstractOAuthReplyHandler *handler = replyHandler();
QObject::connect(reply, &QNetworkReply::finished,
                 [handler, reply] { handler->networkReplyFinished(reply); });
QObjectPrivate::connect(d->replyHandler.data(), &QAbstractOAuthReplyHandler::tokensReceived, d,
                        &QOAuth2AuthorizationCodeFlowPrivate::_q_accessTokenRequestFinished,
                        Qt::UniqueConnection);
QObjectPrivate::connect(d->networkAccessManager(),
                        &QNetworkAccessManager::authenticationRequired,
                        d, &QOAuth2AuthorizationCodeFlowPrivate::_q_authenticate,
                        Qt::UniqueConnection);

}

[SGaist]

It looks like you found a bug :)

Did you already check the bug report system to see if there was something related ?

If not, please consider opening a new report providing all the information you have here (do not just link this thread, it's better to have all there).

Since you also find a solution, would you consider submitting a patch directly ?

[Alex94102]

Hi,

to be honest, I don't know how to submit a patch, I've never done it before. Would that be a patch file? I am a little lost. But essentially the code above where the "data" variable is populated by that statement needs to be replaced by something like this (just to get the idea). In my case, I used a QVariantMap for storing my parameter values, and then I build the QByteArray for the request using the method below. :

QByteArray MyOwnOAuth2::buildBodyPayload()
{
bool first=true;
QByteArray body;
for(QVariantMap::const_iterator iter = mBodyParameters.begin(); iter != mBodyParameters.end(); ++iter) {
qDebug() << iter.key() << iter.value();
if (!first) {
body.append(QString("&").toUtf8());
}
QString name = iter.key();
QString value = QUrl::toPercentEncoding(iter.value().toString());
body.append(name.toUtf8());
body.append(QString("=").toUtf8());
body.append(value.toUtf8());
first = false;
}
return body;
}

[aha_1980]

@Alex94102 said in OAuth2 QOAuthHttpServerReplyHandler HTTPS/SSL support?:

to be honest, I don't know how to submit a patch, I've never done it before. Would that be a patch file?

A patch is the difference in code between the original and your new version: https://en.wikipedia.org/wiki/Patch_(computing)

The preferred way to get patches into Qt is through Gerrit, however small patches may also be attached to the bug report. In any case, we can help you with that, preferred in a separate discussion.

body.append(QString("&").toUtf8());

That (and the similar line) should be simplified to: body.append("&");

PS: Please add a link to the bug report here, so we can follow. Thanks!

[Alex94102]

Sorry, I was out of town. I think I am familiar with the patch utility, my question is more around the whole process. Maybe someone can get back to me directly so that we are not polluting this topic with these kind of general questions. At the present time the code that I put together is not using this library anymore as it was defective, so for me to properly test I would have to make some changes to my code and to make it use this patched library instead.

What I am looking for is some pointers (not c++) on how to get this done (example):

1. File a JIRA bug report
2. Prepare the patch if bug confirmed
3. This is where I am struggling, would I send the simple patch file to the maintainer? I guess so, submit that in JIRA?
4. Would I need also submit unit tests for this?
5. Anythig else?

Thanks a lot,
alex

[SGaist]

1. Yes if not already existing
2. Since you are experiencing this and have a solution, say it in the report.
3. The nicest and fastest would be to submit the patch yourself for review. See the gerrit introcution.
4. Since you are solving a specific problem, it would be best since it's not already covered by a test
5. Be patient, sometime patches can get reviewed quickly while some needs more time. It has nothing to do with the fact that its your first submission.

[Alex94102]

@SGaist Ok, I have enough info to get started. Thank you.

[SGaist]

You're welcome !

One last thing: if you open a new report, please post a link to it here so people can more easily find it :)

[testmonkey]

bump, bit of a necropost, but still an issue.

@JonB said in OAuth2 QOAuthHttpServerReplyHandler HTTPS/SSL support?:

@Alex94102
Here you go: https://bugreports.qt.io/browse/QTBUG-64615

any luck with getting QSslSocket bits into qoauthhttpserverreplyhandler ?? I am trying to interact with a service that has just started enforcing https for callbacks.