Oauth: Display a branded html page instead of a white sheet with textwork/oath_error

author: Hannah von Reth <hannah.vonreth@owncloud.com> 2022-06-22 16:06:58 +0300
committer: Hannah von Reth <hannah.vonreth@owncloud.com> 2022-06-22 17:43:43 +0300
commit: 5dd03fccb8068763c35418c5c33ec0ab2e4f5eed (patch)
tree: 837bae247e7d3838d6d21371f4484dafdeec7bcb
parent: 9ec922b559008c3a21f24bb28ef56bdf25486e5b (diff)
5 files changed, 112 insertions, 19 deletions
diff --git a/changelog/unreleased/9772 b/changelog/unreleased/9772
new file mode 100644
index 000000000..a6450459f
--- /dev/null
+++ b/changelog/unreleased/9772
@@ -0,0 +1,7 @@
+Enhancement: Display a correct error when the wrong user was authenticated
+
+When the wrong user was authenticated using oauth we used to display a misleading message.
+We now also style the html response the client provides to the file browser.
+
+https://github.com/owncloud/client/issues/9772
+https://github.com/owncloud/client/pull/9813
diff --git a/client.qrc b/client.qrc
index 9acf066fd..1042827be 100644
--- a/client.qrc
+++ b/client.qrc
@@ -16,7 +16,6 @@
         <file alias="resources/light/check.svg">resources/font-awesome/dark/check-solid.svg</file>
         <file alias="resources/light/step-forward.svg">resources/font-awesome/dark/step-forward-solid.svg</file>
         <file alias="resources/light/clipboard.svg">resources/font-awesome/dark/clipboard-solid.svg</file>
-
         <file alias="resources/dark/folder-sync.svg">resources/font-awesome/dark/folder-solid.svg</file>
         <file alias="resources/dark/settings.svg">resources/font-awesome/dark/cog-solid.svg</file>
         <file alias="resources/dark/activity.svg">resources/font-awesome/dark/bolt-solid.svg</file>
@@ -33,7 +32,7 @@
         <file alias="resources/dark/check.svg">resources/font-awesome/dark/check-solid.svg</file>
         <file alias="resources/dark/step-forward.svg">resources/font-awesome/dark/step-forward-solid.svg</file>
         <file alias="resources/dark/clipboard.svg">resources/font-awesome/dark/clipboard-solid.svg</file>
-
         <file alias="resources/wizard/style.qss">resources/wizard/style.qss</file>
+        <file>resources/oauth/oauth.html.in</file>
     </qresource>
 </RCC>
diff --git a/resources/oauth/oauth.html.in b/resources/oauth/oauth.html.in
new file mode 100644
index 000000000..0c365d969
--- /dev/null
+++ b/resources/oauth/oauth.html.in
@@ -0,0 +1,43 @@
+<!DOCTYPE html>
+
+<html lang="en">
+
+<head>
+<title>@{TITLE}</title>
+<style>
+html, body {
+    height: 100%;
+    width: 100%;
+    margin: 0;
+}
+
+body {
+    background-color: @{BACKGROUND_COLOR};
+    color: @{FONT_COLOR};
+    font-family: "Noto Sans", OpenSans, Verdana, Helvetica, Arial, sans-serif;
+    display: flex;
+    flex-direction: column;
+    align-items: center;
+}
+
+.row {
+    display: flex;
+    flex-direction: row;
+    align-items: center;
+    height: 100%;
+}
+
+.content {
+    text-align: center;
+}
+</style>
+</head>
+
+<body>
+<div class="row">
+    <div class="content">
+        <img src="data:image/png;base64,@{ICON}" />
+        @{CONTENT}
+    </div>
+</div>
+</body>
diff --git a/src/common/utility.cpp b/src/common/utility.cpp
index 444e31284..797773fff 100644
--- a/src/common/utility.cpp
+++ b/src/common/utility.cpp
@@ -614,7 +614,13 @@ QString Utility::renderTemplate(QString templ, const QMap<QString, QString> &val
         while (it.hasNext()) {
             const auto match = it.next();
             Q_ASSERT(match.lastCapturedIndex() == 1);
-            Q_ASSERT(values.contains(match.captured(1)));
+            Q_ASSERT([&] {
+                if (!values.contains(match.captured(1))) {
+                    qCCritical(lcUtility) << "Unknow key:" << match.captured(1);
+                    return false;
+                }
+                return true;
+            }());
             templ.replace(match.captured(0), values.value(match.captured(1)));
         }
     };
diff --git a/src/libsync/creds/oauth.cpp b/src/libsync/creds/oauth.cpp
index ff3409b20..5587f7567 100644
--- a/src/libsync/creds/oauth.cpp
+++ b/src/libsync/creds/oauth.cpp
@@ -25,10 +25,12 @@
 #include <QApplication>
 #include <QBuffer>
 #include <QDesktopServices>
+#include <QIcon>
 #include <QJsonArray>
 #include <QJsonDocument>
 #include <QJsonObject>
 #include <QNetworkReply>
+#include <QPixmap>
 #include <QRandomGenerator>
 #include <QScopeGuard>
 #include <QTimer>
@@ -41,6 +43,30 @@ Q_LOGGING_CATEGORY(lcOauth, "sync.credentials.oauth", QtInfoMsg)
 
 namespace {
 
+QString renderHttpTemplate(const QString &title, const QString &content)
+{
+    const QString tmpl = [] {
+        QFile f(QStringLiteral(":/client/resources/oauth/oauth.html.in"));
+        OC_ASSERT(f.open(QFile::ReadOnly));
+        return QString::fromUtf8(f.readAll());
+    }();
+
+    const QString icon = [] {
+        const auto img = Theme::instance()->aboutIcon().pixmap(256).toImage();
+        QByteArray out;
+        QBuffer buffer(&out);
+        img.save(&buffer, "PNG");
+        return QString::fromUtf8(out.toBase64());
+    }();
+    return Utility::renderTemplate(tmpl, {
+                                             { QStringLiteral("TITLE"), title }, //
+                                             { QStringLiteral("CONTENT"), content }, //
+                                             { QStringLiteral("ICON"), icon }, //
+                                             { QStringLiteral("BACKGROUND_COLOR"), Theme::instance()->wizardHeaderBackgroundColor().name() }, //
+                                             { QStringLiteral("FONT_COLOR"), Theme::instance()->wizardHeaderTitleColor().name() } //
+                                         });
+}
+
 auto defaultTimeout()
 {
     // as the oauth process can be interactive we don't want 5min of inactivity
@@ -68,13 +94,24 @@ QVariant getRequiredField(const QVariantMap &json, const QString &s, QString *er
     return *out;
 }
 
-void httpReplyAndClose(const QPointer<QTcpSocket> &socket, const QByteArray &code, const QByteArray &html,
-    const QByteArray &moreHeaders = {})
+void httpReplyAndClose(const QPointer<QTcpSocket> &socket, const QString &code, const QString &title, const QString &body = {}, const QStringList &additionalHeader = {})
 {
-    if (!socket)
+    if (!socket) {
         return; // socket can have been deleted if the browser was closed
-    // clang format has issues with the next line...
-    const QByteArray msg = QByteArrayLiteral("HTTP/1.1 ") % code % QByteArrayLiteral("\r\nContent-Type: text/html; charset=utf-8\r\nConnection: close\r\nContent-Length: ") % QByteArray::number(html.length()) % (!moreHeaders.isEmpty() ? QByteArrayLiteral("\r\n") % moreHeaders : QByteArray()) % QByteArrayLiteral("\r\n\r\n") % html;
+    }
+
+    const QByteArray content = renderHttpTemplate(title, body.isEmpty() ? title : body).toUtf8();
+    QString header = QStringLiteral("HTTP/1.1 %1\r\n"
+                                    "Content-Type: text/html; charset=utf-8\r\n"
+                                    "Connection: close\r\n"
+                                    "Content-Length: %2\r\n")
+                         .arg(code, QString::number(content.length()));
+
+    if (!additionalHeader.isEmpty()) {
+        const QString nl = QStringLiteral("\r\n");
+        header += additionalHeader.join(nl) + nl;
+    }
+    const QByteArray msg = header.toUtf8() + QByteArrayLiteral("\r\n") + content;
     qCDebug(lcOauth) << msg;
     socket->write(msg);
     socket->disconnectFromHost();
@@ -233,13 +270,13 @@ void OAuth::startAuthentication()
                 qCDebug(lcOauth) << "Server provided:" << peek;
                 const auto getPrefix = QByteArrayLiteral("GET /?");
                 if (!peek.startsWith(getPrefix)) {
-                    httpReplyAndClose(socket, QByteArrayLiteral("404 Not Found"), QByteArrayLiteral("<html><head><title>404 Not Found</title></head><body><center><h1>404 Not Found</h1></center></body></html>"));
+                    httpReplyAndClose(socket, QStringLiteral("404 Not Found"), QStringLiteral("404 Not Found"));
                     return;
                 }
                 const auto endOfUrl = peek.indexOf(' ', getPrefix.length());
                 const QUrlQuery args(QUrl::fromPercentEncoding(peek.mid(getPrefix.length(), endOfUrl - getPrefix.length())));
                 if (args.queryItemValue(QStringLiteral("state")).toUtf8() != _state) {
-                    httpReplyAndClose(socket, QByteArrayLiteral("400 Bad Request"), QByteArrayLiteral("<html><head><title>400 Bad Request</title></head><body><center><h1>400 Bad Request</h1></center></body></html>"));
+                    httpReplyAndClose(socket, QStringLiteral("400 Bad Request"), QStringLiteral("400 Bad Request"));
                     return;
                 }
 
@@ -295,8 +332,8 @@ void OAuth::startAuthentication()
                             errorReason = tr("Unknown Error");
                         }
                         qCWarning(lcOauth) << "Error when getting the accessToken" << errorReason;
-                        httpReplyAndClose(socket, QByteArrayLiteral("500 Internal Server Error"),
-                            tr("<h1>Login Error</h1><p>%1</p>").arg(errorReason).toUtf8());
+                        httpReplyAndClose(socket, QStringLiteral("500 Internal Server Error"),
+                            tr("Login Error"), tr("<h1>Login Error</h1><p>%1</p>").arg(errorReason));
                         emit result(Error);
                         return;
                     }
@@ -305,8 +342,8 @@ void OAuth::startAuthentication()
 
                     connect(job, &CoreJob::finished, this, [=]() {
                         if (!job->success()) {
-                            httpReplyAndClose(socket, QByteArrayLiteral("500 Internal Server Error"),
-                                tr("<h1>Login Error</h1><p>%1</p>").arg(job->errorMessage()).toUtf8());
+                            httpReplyAndClose(socket, QStringLiteral("500 Internal Server Error"),
+                                tr("Login Error"), tr("<h1>Login Error</h1><p>%1</p>").arg(job->errorMessage()));
                             emit result(Error);
                         } else {
                             auto result = job->result().value<FetchUserInfoResult>();
@@ -393,16 +430,17 @@ void OAuth::finalize(const QPointer<QTcpSocket> &socket, const QString &accessTo
                                    "<p>You logged-in with user <em>%1</em>, but must login with user <em>%2</em>.<br>"
                                    "Please return to the %3 client and restart the authentication.</p>")
                                     .arg(userName, _davUser, Theme::instance()->appNameGUI());
-        httpReplyAndClose(socket, QByteArrayLiteral("403 Forbidden"), message.toUtf8());
+        httpReplyAndClose(socket, QStringLiteral("403 Forbidden"), tr("Wrong user"), message);
         emit result(Error);
         return;
     }
-    const auto loginSuccessfullHtml = QByteArrayLiteral("<h1>Login Successful</h1><p>You can close this window.</p>");
+    const QString loginSuccessfullHtml = tr("<h1>Login Successful</h1><p>You can close this window.</p>");
+    const QString loginSuccessfullTitle = tr("Login Successful");
     if (messageUrl.isValid()) {
-        httpReplyAndClose(socket, QByteArrayLiteral("303 See Other"), loginSuccessfullHtml,
-            QByteArrayLiteral("Location: ") + messageUrl.toEncoded());
+        httpReplyAndClose(socket, QStringLiteral("303 See Other"), loginSuccessfullTitle, loginSuccessfullHtml,
+            { QStringLiteral("Location: %1").arg(QString::fromUtf8(messageUrl.toEncoded())) });
     } else {
-        httpReplyAndClose(socket, QByteArrayLiteral("200 OK"), loginSuccessfullHtml);
+        httpReplyAndClose(socket, QStringLiteral("200 OK"), loginSuccessfullTitle, loginSuccessfullHtml);
     }
     emit result(LoggedIn, userName, accessToken, displayName, refreshToken);
 }
author	Hannah von Reth <hannah.vonreth@owncloud.com>	2022-06-22 16:06:58 +0300
committer	Hannah von Reth <hannah.vonreth@owncloud.com>	2022-06-22 17:43:43 +0300
commit	5dd03fccb8068763c35418c5c33ec0ab2e4f5eed (patch)
tree	837bae247e7d3838d6d21371f4484dafdeec7bcb
parent	9ec922b559008c3a21f24bb28ef56bdf25486e5b (diff)