Merge lp:~mardy/ubuntu-system-settings-online-accounts/remove-snap-decision into lp:~online-accounts/ubuntu-system-settings-online-accounts/master

[Alberto Mardegan]

Remove snap decision fallback

Remove snap decision fallback

[David Barth (dbarth)]

review: Approve

=== modified file 'online-accounts-service/request-manager.cpp'
--- online-accounts-service/request-manager.cpp	2014-10-13 10:48:54 +0000
+++ online-accounts-service/request-manager.cpp	2015-01-05 14:26:48 +0000
@@ -117,14 +117,16 @@
         return; // Nothing to do
     }
 
+    QObject::connect(request, SIGNAL(completed()),
+                     this, SLOT(onRequestCompleted()));
+
     UiProxy *proxy = new UiProxy(request->clientPid(), this);
     if (Q_UNLIKELY(!proxy->init())) {
         qWarning() << "UiProxy initialization failed!";
-        runQueue(queue);
+        request->fail(OAU_ERROR_PROMPT_SESSION,
+                      "Could not create a prompt session");
         return;
     }
-    QObject::connect(request, SIGNAL(completed()),
-                     this, SLOT(onRequestCompleted()));
     QObject::connect(proxy, SIGNAL(finished()),
                      this, SLOT(onProxyFinished()));
     m_proxies.append(proxy);

=== modified file 'online-accounts-service/ui-proxy.cpp'
--- online-accounts-service/ui-proxy.cpp	2014-11-21 13:50:15 +0000
+++ online-accounts-service/ui-proxy.cpp	2015-01-05 14:26:48 +0000
@@ -53,7 +53,7 @@
     bool init();
     void sendOperation(const QVariantMap &data);
     void sendRequest(int requestId, Request *request);
-    void setupPromptSession();
+    bool setupPromptSession();
 
 private Q_SLOTS:
     void onNewConnection();
@@ -190,28 +190,27 @@
     return m_server.listen(socketDir.filePath(uniqueName));
 }
 
-void UiProxyPrivate::setupPromptSession()
+bool UiProxyPrivate::setupPromptSession()
 {
     Q_Q(UiProxy);
 
-    QProcessEnvironment env = QProcessEnvironment::systemEnvironment();
-    if (!env.value("QT_QPA_PLATFORM").startsWith("ubuntu")) return;
-
     PromptSessionP session =
         MirHelper::instance()->createPromptSession(m_clientPid);
-    if (!session) return;
+    if (!session) return false;
 
     QString mirSocket = session->requestSocket();
     if (mirSocket.isEmpty()) {
-        return;
+        return false;
     }
 
     m_promptSession = session;
     QObject::connect(m_promptSession.data(), SIGNAL(finished()),
                      q, SIGNAL(finished()));
 
+    QProcessEnvironment env = QProcessEnvironment::systemEnvironment();
     env.insert("MIR_SOCKET", mirSocket);
     m_process.setProcessEnvironment(env);
+    return true;
 }
 
 bool UiProxyPrivate::init()
@@ -235,8 +234,9 @@
     arguments.append("--socket");
     arguments.append(m_server.fullServerName());
 
-    if (m_clientPid) {
-        setupPromptSession();
+    QProcessEnvironment env = QProcessEnvironment::systemEnvironment();
+    if (env.value("QT_QPA_PLATFORM").startsWith("ubuntu")) {
+        if (!setupPromptSession()) return false;
     }
 
     m_process.start(processName, arguments);

=== modified file 'online-accounts-ui/globals.h'
--- online-accounts-ui/globals.h	2014-09-15 12:16:50 +0000
+++ online-accounts-ui/globals.h	2015-01-05 14:26:48 +0000
@@ -40,6 +40,8 @@
     QStringLiteral(OAU_ERROR_PREFIX "InvalidParameters")
 #define OAU_ERROR_INVALID_APPLICATION \
     QStringLiteral(OAU_ERROR_PREFIX "InvalidApplication")
+#define OAU_ERROR_PROMPT_SESSION \
+    QStringLiteral(OAU_ERROR_PREFIX "NoPromptSession")
 
 /* SignOnUi service */
 #define SIGNONUI_SERVICE_NAME   QStringLiteral("com.nokia.singlesignonui")

=== modified file 'online-accounts-ui/online-accounts-ui.pro'
--- online-accounts-ui/online-accounts-ui.pro	2014-10-31 12:05:37 +0000
+++ online-accounts-ui/online-accounts-ui.pro	2015-01-05 14:26:48 +0000
@@ -52,7 +52,6 @@
     i18n.cpp \
     ipc.cpp \
     main.cpp \
-    notification.cpp \
     panel-request.cpp \
     provider-request.cpp \
     request.cpp \
@@ -67,7 +66,6 @@
     dialog-request.h \
     i18n.h \
     ipc.h \
-    notification.h \
     panel-request.h \
     provider-request.h \
     request.h \

=== modified file 'online-accounts-ui/signonui-request.cpp'
--- online-accounts-ui/signonui-request.cpp	2014-10-14 09:05:52 +0000
+++ online-accounts-ui/signonui-request.cpp	2015-01-05 14:26:48 +0000
@@ -24,13 +24,7 @@
 #include "debug.h"
 #include "dialog-request.h"
 #include "globals.h"
-#include "notification.h"
 
-#include <Accounts/Account>
-#include <Accounts/Application>
-#include <Accounts/Provider>
-#include <OnlineAccountsPlugin/account-manager.h>
-#include <OnlineAccountsPlugin/application-manager.h>
 #include <OnlineAccountsPlugin/request-handler.h>
 #include <QDBusArgument>
 #include <QPointer>
@@ -52,19 +46,9 @@
     ~RequestPrivate();
 
 private:
-    void setWindow(QWindow *window);
-    Accounts::Account *findAccount();
-
-private Q_SLOTS:
-    void onActionInvoked(const QString &action);
-    void onNotificationClosed();
-
-private:
     mutable Request *q_ptr;
     QVariantMap m_clientData;
     QPointer<RequestHandler> m_handler;
-    OnlineAccountsUi::Notification *m_notification;
-    QWindow *m_window;
 };
 
 } // namespace
@@ -72,9 +56,7 @@
 RequestPrivate::RequestPrivate(Request *request):
     QObject(request),
     q_ptr(request),
-    m_handler(0),
-    m_notification(0),
-    m_window(0)
+    m_handler(0)
 {
     const QVariantMap &parameters = request->parameters();
     if (parameters.contains(SSOUI_KEY_CLIENT_DATA)) {
@@ -87,112 +69,6 @@
 
 RequestPrivate::~RequestPrivate()
 {
-    delete m_notification;
-    m_notification = 0;
-}
-
-void RequestPrivate::setWindow(QWindow *window)
-{
-    Q_Q(Request);
-
-    /* Don't show the window yet: the user must be presented with a
-     * snap-decision, and we'll show the window only if he decides to
-     * authenticate. */
-    Accounts::Account *account = findAccount();
-    if (Q_UNLIKELY(!account)) {
-        QVariantMap result;
-        result[SSOUI_KEY_ERROR] = SignOn::QUERY_ERROR_FORBIDDEN;
-        q->setResult(result);
-        return;
-    }
-
-    OnlineAccountsUi::ApplicationManager *appManager =
-        OnlineAccountsUi::ApplicationManager::instance();
-    Accounts::Application application =
-        appManager->applicationFromProfile(q->clientApparmorProfile());
-
-    OnlineAccountsUi::AccountManager *accountManager =
-        OnlineAccountsUi::AccountManager::instance();
-    Accounts::Provider provider =
-        accountManager->provider(account->providerName());
-
-    QString summary =
-        QString("Please authorize %1 to access your %2 account %3").
-        arg(application.isValid() ? application.displayName() : "Ubuntu").
-        arg(provider.displayName()).
-        arg(account->displayName());
-    m_notification =
-        new OnlineAccountsUi::Notification("Authentication request", summary);
-    m_notification->addAction("cancel", "Cancel");
-    m_notification->addAction("continue", "Authorize...");
-    m_notification->setSnapDecision(true);
-    QObject::connect(m_notification, SIGNAL(actionInvoked(const QString &)),
-                     this, SLOT(onActionInvoked(const QString &)));
-    QObject::connect(m_notification, SIGNAL(closed()),
-                     this, SLOT(onNotificationClosed()));
-    m_notification->show();
-    m_window = window;
-}
-
-Accounts::Account *RequestPrivate::findAccount()
-{
-    Q_Q(Request);
-
-    uint identity = q->identity();
-    if (identity == 0)
-        return 0;
-
-    /* Find the account using this identity.
-     * FIXME: there might be more than one!
-     */
-    OnlineAccountsUi::AccountManager *manager =
-        OnlineAccountsUi::AccountManager::instance();
-    Q_FOREACH(Accounts::AccountId accountId, manager->accountList()) {
-        Accounts::Account *account = manager->account(accountId);
-        if (account == 0) continue;
-
-        QVariant value(QVariant::UInt);
-        if (account->value("CredentialsId", value) != Accounts::NONE &&
-            value.toUInt() == identity) {
-            return account;
-        }
-    }
-
-    // Not found
-    return 0;
-}
-
-void RequestPrivate::onActionInvoked(const QString &action)
-{
-    Q_Q(Request);
-
-    DEBUG() << action;
-
-    QObject::disconnect(m_notification, 0, this, 0);
-    m_notification->deleteLater();
-    m_notification = 0;
-
-    if (action == QStringLiteral("continue")) {
-        q->setWindow(m_window);
-    } else {
-        q->cancel();
-    }
-}
-
-void RequestPrivate::onNotificationClosed()
-{
-    Q_Q(Request);
-
-    DEBUG();
-
-    /* setResult() should have been called by onActionInvoked(), but calling it
-     * twice won't harm because only the first invocation counts. */
-    QVariantMap result;
-    result[SSOUI_KEY_ERROR] = SignOn::QUERY_ERROR_FORBIDDEN;
-    q->setResult(result);
-
-    m_notification->deleteLater();
-    m_notification = 0;
 }
 
 #ifndef NO_REQUEST_FACTORY
@@ -266,22 +142,13 @@
 {
     Q_D(Request);
 
-    /* While a notification is shown, ignore any further calls to
-     * setWindow(). */
-    if (d->m_notification) return;
-
-    /* The first time that this method is called, we handle it by presenting a
-     * snap decision to the user.
-     * Then, if this is called again with the same QWindow, it means that the
-     * snap decision was accepted, and we show the window.
-     */
-    if (window == d->m_window ||
-        /* If we are part of a prompt session, just show the window */
-        !qgetenv("MIR_SOCKET").isEmpty()) {
+    /* Show the window only if we are in a prompt session */
+    if (qgetenv("MIR_SOCKET").isEmpty()) {
+        QVariantMap result;
+        result[SSOUI_KEY_ERROR] = SignOn::QUERY_ERROR_FORBIDDEN;
+        setResult(result);
+    } else {
         OnlineAccountsUi::Request::setWindow(window);
-        d->m_window = 0;
-    } else {
-        d->setWindow(window);
     }
 }
 

=== modified file 'tests/online-accounts-ui/tst_signonui_request.cpp'
--- tests/online-accounts-ui/tst_signonui_request.cpp	2014-10-09 12:24:45 +0000
+++ tests/online-accounts-ui/tst_signonui_request.cpp	2015-01-05 14:26:48 +0000
@@ -20,7 +20,6 @@
 
 #include "globals.h"
 #include "signonui-request.h"
-#include "mock/notification-mock.h"
 #include "mock/request-mock.h"
 #include "mock/ui-server-mock.h"
 
@@ -68,8 +67,6 @@
     void testParameters_data();
     void testParameters();
     void testHandler();
-    void testSnapDecision_data();
-    void testSnapDecision();
 
 private:
     bool mustCreateAccount(uint credentialsId) { return credentialsId > 10; }
@@ -167,171 +164,6 @@
     delete handler;
 }
 
-void SignonuiRequestTest::testSnapDecision_data()
-{
-    QTest::addColumn<uint>("credentialsId");
-    QTest::addColumn<QString>("accountName");
-    QTest::addColumn<QString>("clientProfile");
-    QTest::addColumn<QString>("applicationName");
-    QTest::addColumn<bool>("mustAccept");
-    QTest::addColumn<QVariantMap>("result");
-
-    QVariantMap acceptedResult;
-    acceptedResult.insert("some key", QString("some value"));
-
-    QVariantMap declinedResult;
-    declinedResult.insert(SSOUI_KEY_ERROR, SignOn::QUERY_ERROR_CANCELED);
-
-    QVariantMap errorResult;
-    errorResult.insert(SSOUI_KEY_ERROR, SignOn::QUERY_ERROR_FORBIDDEN);
-
-    QTest::newRow("no account") <<
-        uint(0) <<
-        "[e-mail]" <<
-        "com.ubuntu.tests_application_0.3" <<
-        QString() <<
-        false <<
-        errorResult;
-
-    QTest::newRow("invalid account") <<
-        uint(1) <<
-        "[e-mail]" <<
-        "com.ubuntu.tests_application_0.3" <<
-        QString() <<
-        false <<
-        errorResult;
-
-    QTest::newRow("valid application, accepted") <<
-        uint(14231) <<
-        "[e-mail]" <<
-        "com.ubuntu.tests_application_0.3" <<
-        "Easy Mailer" <<
-        true <<
-        acceptedResult;
-
-    QTest::newRow("valid application, declined") <<
-        uint(14231) <<
-        "[e-mail]" <<
-        "com.ubuntu.tests_application_0.3" <<
-        "Easy Mailer" <<
-        false <<
-        declinedResult;
-
-    QTest::newRow("unconfined application, accepted") <<
-        uint(14235) <<
-        "[e-mail]" <<
-        "unconfined" <<
-        "Ubuntu" <<
-        true <<
-        acceptedResult;
-
-    QTest::newRow("unconfined application, declined") <<
-        uint(14235) <<
-        "[e-mail]" <<
-        "unconfined" <<
-        "Ubuntu" <<
-        false <<
-        declinedResult;
-}
-
-void SignonuiRequestTest::testSnapDecision()
-{
-    QString providerId("cool");
-    QFETCH(uint, credentialsId);
-    QFETCH(QString, accountName);
-    QFETCH(QString, clientProfile);
-    QFETCH(QString, applicationName);
-    QFETCH(bool, mustAccept);
-    QFETCH(QVariantMap, result);
-
-    // First, create an account
-    Accounts::Manager *manager = new Accounts::Manager(this);
-    Accounts::Provider provider = manager->provider(providerId);
-    QVERIFY(provider.isValid());
-    if (mustCreateAccount(credentialsId)) {
-        Accounts::Account *account = manager->createAccount(providerId);
-        QVERIFY(account != 0);
-        account->setEnabled(true);
-        account->setDisplayName(accountName);
-        account->setCredentialsId(credentialsId);
-        account->syncAndBlock();
-    }
-
-    /* Then, create a request referring to the same credentials ID of the
-     * created account. */
-    QVariantMap parameters;
-    parameters.insert(SSOUI_KEY_IDENTITY, credentialsId);
-    parameters.insert(SSOUI_KEY_METHOD, "funnyMethod");
-    parameters.insert(SSOUI_KEY_MECHANISM, "funnyMechanism");
-    TestRequest request(clientProfile, parameters);
-    OnlineAccountsUi::RequestPrivate *mockRequest =
-        OnlineAccountsUi::RequestPrivate::mocked(&request);
-    QSignalSpy failCalled(mockRequest,
-                          SIGNAL(failCalled(const QString&, const QString&)));
-    QSignalSpy setResultCalled(mockRequest,
-                               SIGNAL(setResultCalled(const QVariantMap &)));
-    request.start();
-
-    /* Request to show a window; a snap decision should appear instead */
-    QWindow *window = new QWindow;
-    QSignalSpy setWindowCalled(mockRequest,
-                               SIGNAL(setWindowCalled(QWindow*)));
-    request.setWindow(window);
-    QCOMPARE(setWindowCalled.count(), 0);
-    if (mustCreateAccount(credentialsId)) {
-        QCOMPARE(NotificationPrivate::allNotifications.count(), 1);
-    } else {
-        /* If the account is not found, no notification should appear, and an
-         * error returned to the app */
-        QCOMPARE(NotificationPrivate::allNotifications.count(), 0);
-        QCOMPARE(setResultCalled.count(), 1);
-        QCOMPARE(setResultCalled.at(0).at(0).toMap(), result);
-        return;
-    }
-
-    /* Inspect the snap decision contents */
-    Notification *notification =
-        NotificationPrivate::allNotifications.first();
-    NotificationPrivate *mockNotification =
-        NotificationPrivate::mocked(notification);
-    QCOMPARE(mockNotification->m_summary, QString("Authentication request"));
-    QCOMPARE(mockNotification->m_body,
-             QString("Please authorize %1 to access your %2 account %3").
-             arg(applicationName).arg(provider.displayName()).arg(accountName));
-    QVERIFY(mockNotification->m_isSnapDecision);
-
-    /* Invoke the action on the snap decision */
-    QString action = mustAccept ? "continue" : "cancel";
-    QSignalSpy actionInvoked(notification,
-                             SIGNAL(actionInvoked(const QString &)));
-    mockNotification->invokeAction(action);
-    QCOMPARE(actionInvoked.count(), 1);
-    QCOMPARE(actionInvoked.at(0).at(0).toString(), action);
-
-    /* Here we iterate the main loop because the notification object is
-     * destroyed with deleteLater() */
-    QTest::qWait(5);
-
-    if (mustAccept) {
-        QCOMPARE(setWindowCalled.count(), 1);
-        QCOMPARE(setWindowCalled.at(0).at(0), QVariant::fromValue(window));
-        QCOMPARE(failCalled.count(), 0);
-        QCOMPARE(setResultCalled.count(), 0);
-
-        /* deliver the result */
-        request.sendResult(result);
-        QCOMPARE(setResultCalled.count(), 1);
-        QCOMPARE(setResultCalled.at(0).at(0).toMap(), result);
-    } else {
-        QCOMPARE(setWindowCalled.count(), 0);
-        QCOMPARE(failCalled.count(), 0);
-        QCOMPARE(setResultCalled.count(), 1);
-        QCOMPARE(setResultCalled.at(0).at(0).toMap(), result);
-    }
-    delete window;
-    delete manager;
-}
-
 QTEST_MAIN(SignonuiRequestTest);
 
 #include "tst_signonui_request.moc"

=== modified file 'tests/online-accounts-ui/tst_signonui_request.pro'
--- tests/online-accounts-ui/tst_signonui_request.pro	2014-10-14 11:19:19 +0000
+++ tests/online-accounts-ui/tst_signonui_request.pro	2015-01-05 14:26:48 +0000
@@ -27,18 +27,15 @@
 SOURCES += \
     $${ONLINE_ACCOUNTS_UI_DIR}/debug.cpp \
     $${ONLINE_ACCOUNTS_UI_DIR}/signonui-request.cpp \
-    mock/notification-mock.cpp \
     mock/request-mock.cpp \
     mock/qwindow.cpp \
     mock/ui-server-mock.cpp \
     tst_signonui_request.cpp
 
 HEADERS += \
-    $${ONLINE_ACCOUNTS_UI_DIR}/notification.h \
     $${ONLINE_ACCOUNTS_UI_DIR}/request.h \
     $${ONLINE_ACCOUNTS_UI_DIR}/signonui-request.h \
     $${ONLINE_ACCOUNTS_UI_DIR}/ui-server.h \
-    mock/notification-mock.h \
     mock/request-mock.h \
     mock/ui-server-mock.h \
     window-watcher.h