From fb0d31c5326fad6638c7d809f91a585fa7abb026 Mon Sep 17 00:00:00 2001
From: Hannah von Reth <hannah.vonreth@owncloud.com>
Date: Wed, 29 Jan 2020 19:54:12 +0100
Subject: [PATCH] [OAuth] Improve error message on login

---
 src/libsync/creds/oauth.cpp | 29 ++++++++++++-----------------
 src/libsync/creds/oauth.h   |  2 --
 2 files changed, 12 insertions(+), 19 deletions(-)

diff --git a/src/libsync/creds/oauth.cpp b/src/libsync/creds/oauth.cpp
index 1ff5cc3f0b..31cc8a7274 100644
--- a/src/libsync/creds/oauth.cpp
+++ b/src/libsync/creds/oauth.cpp
@@ -117,7 +117,12 @@ void OAuth::startAuthentification()
                     if (reply->error() != QNetworkReply::NoError || jsonParseError.error != QJsonParseError::NoError
                         || !fieldsError.isEmpty()
                         || tokenType != "bearer") {
-                        QString errorReason = errorFromJson(json);
+                        // do we have error message suitable for users?
+                        QString errorReason = json[QStringLiteral("error_description")].toString();
+                        if (errorReason.isEmpty()) {
+                            // fall back to technical error
+                            errorReason = json[QStringLiteral("error")].toString();
+                        }
                         if (!errorReason.isEmpty()) {
                             errorReason = tr("Error returned from the server: <em>%1</em>")
                                               .arg(errorReason.toHtmlEscaped());
@@ -140,7 +145,7 @@ void OAuth::startAuthentification()
                         } else {
                             errorReason = tr("Unknown Error");
                         }
-                        qCWarning(lcOauth) << "Error when getting the accessToken" << jsonData << errorReason;
+                        qCWarning(lcOauth) << "Error when getting the accessToken" << errorReason << "received data:" << jsonData;
                         httpReplyAndClose(socket, "500 Internal Server Error",
                             tr("<h1>Login Error</h1><p>%1</p>").arg(errorReason).toUtf8().constData());
                         emit result(Error);
@@ -181,12 +186,13 @@ void OAuth::refreshAuthentification(const QString &refreshToken)
             QJsonParseError jsonParseError;
             // https://developer.okta.com/docs/reference/api/oidc/#response-properties-2
             const QJsonObject json = QJsonDocument::fromJson(jsonData, &jsonParseError).object();
-            const QString errorReason = errorFromJson(json);
-            if (!errorReason.isEmpty()) {
-                if (errorReason == QStringLiteral("invalid_grant")) {
+            const QString error = json.value(QLatin1String("error")).toString();
+            if (!error.isEmpty()) {
+                if (error == QLatin1String("invalid_grant") ||
+                    error == QLatin1String("invalid_request")) {
                     newRefreshToken.clear();
                 } else {
-                    qCWarning(lcOauth) << tr("Error while refreshing the token: %1").arg(errorReason);
+                    qCWarning(lcOauth) << tr("Error while refreshing the token: %1 : %2").arg(error, json.value(QLatin1String("error_description")).toString());
                 }
             } else if (reply->error() != QNetworkReply::NoError) {
                 qCWarning(lcOauth) << tr("Error while refreshing the token: %1 : %2").arg(reply->errorString(), QString::fromUtf8(jsonData));
@@ -274,17 +280,6 @@ QVariant OAuth::getRequiredField(const QJsonObject &json, const QString &s, QStr
     return *out;
 }
 
-QString OAuth::errorFromJson(const QJsonObject &json)
-{
-    if (json.isEmpty()) {
-        return {};
-    }
-    QString errorFromJson = json[QStringLiteral("error_description")].toString();
-    if (errorFromJson.isEmpty())
-        errorFromJson = json[QStringLiteral("error")].toString();
-    return errorFromJson;
-}
-
 QUrl OAuth::authorisationLink() const
 {
     Q_ASSERT(_server.isListening());
diff --git a/src/libsync/creds/oauth.h b/src/libsync/creds/oauth.h
index d98386c63a..7bd49bc246 100644
--- a/src/libsync/creds/oauth.h
+++ b/src/libsync/creds/oauth.h
@@ -97,8 +97,6 @@ private:
 
     QVariant getRequiredField(const QJsonObject &json, const QString &s, QString *error);
 
-    QString errorFromJson(const QJsonObject &json);
-
     Account* _account;
     QTcpServer _server;
     bool _wellKnownFinished = false;