Merge lp:~mardy/ubuntuone-credentials/lp1376445-migration into lp:ubuntuone-credentials

[Alberto Mardegan]

Update the ACL of the U1 account in a session migration script

Update the ACL of the U1 account in a session migration script

We use the session-migration facility to update the ACL of the user's U1 account. The script looks for the U1 account, and adds the "unconfined" token to the ACL if it's not already there.

I tested this on my desktop, and it seems to work fine.

It works with the signon-apparmor-extension installed.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:163
http://jenkins.qa.ubuntu.com/job/ubuntuone-credentials-ci/114/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntuone-credentials-vivid-amd64-ci/2
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntuone-credentials-vivid-armhf-ci/2
        deb: http://jenkins.qa.ubuntu.com/job/ubuntuone-credentials-vivid-armhf-ci/2/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntuone-credentials-vivid-i386-ci/2

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntuone-credentials-ci/114/rebuild

review: Approve (continuous-integration)

[dobey (dobey)]

I'm going to disapprove this. The code itself looks ok, but I think a migration script is a bit much to add here. It's more untested code, and more complexity. I don't think we should add it.

After poking at the code a little more, I've raise bug #1282392 to critical, as it is a big source of problems related to the signon-apparmor-extension addition. I will fix it today, and ping to get an approval for it in the next RTM milestone as well. With it fixed, I think we can then just treat the account as invalid, and send the user through the log-in process again, which, combined with the fix to add "unconfined" to the ACL, should be sufficient for migration.

review: Disapprove

Unmerged revisions

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2014-08-11 15:14:54 +0000
+++ CMakeLists.txt	2015-01-15 11:37:05 +0000
@@ -52,6 +52,7 @@
 add_subdirectory(signon-plugin)
 add_subdirectory(qml-credentials-service)
 add_subdirectory(online-accounts-provider)
+add_subdirectory(acl-updater)
 add_subdirectory(po)
 
 # We need to add these here which simply depend on other test rules, as

=== added directory 'acl-updater'
=== added file 'acl-updater/CMakeLists.txt'
--- acl-updater/CMakeLists.txt	1970-01-01 00:00:00 +0000
+++ acl-updater/CMakeLists.txt	2015-01-15 11:37:05 +0000
@@ -0,0 +1,19 @@
+# Qt5 bits
+SET (CMAKE_INCLUDE_CURRENT_DIR ON)
+SET (CMAKE_AUTOMOC ON)
+find_package(Qt5Core REQUIRED)
+
+# The sources for building the library
+FILE (GLOB SOURCES *.cpp)
+# HEADERS only includes the public headers for installation.
+FILE (GLOB HEADERS *.h)
+
+SET (ACL_UPDATER acl-updater)
+
+add_executable (${ACL_UPDATER} ${SOURCES})
+qt5_use_modules (${ACL_UPDATER} Core)
+target_link_libraries (${ACL_UPDATER}
+    ${LIBSIGNON_LDFLAGS}
+)
+
+# We don't install this; dh-migrations will pick it up from the build dir.

=== added file 'acl-updater/acl-updater.cpp'
--- acl-updater/acl-updater.cpp	1970-01-01 00:00:00 +0000
+++ acl-updater/acl-updater.cpp	2015-01-15 11:37:05 +0000
@@ -0,0 +1,94 @@
+/*
+ * Copyright 2015 Canonical Ltd.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 3 of the GNU Lesser General Public
+ * License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the
+ * Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ * Boston, MA 02110-1301, USA.
+ */
+
+#include <Accounts/Account>
+#include <Accounts/Manager>
+
+#include <QCoreApplication>
+#include <QDebug>
+
+#include <SignOn/Identity>
+#include <SignOn/IdentityInfo>
+
+#include "acl-updater.h"
+
+AclUpdater::AclUpdater(quint32 credentialsId, QObject *parent):
+    QObject(parent),
+    identity(0)
+{
+    qDebug() << "Loading identity" << credentialsId;
+
+    identity = SignOn::Identity::existingIdentity(credentialsId, this);
+    QObject::connect(identity, SIGNAL(info(const SignOn::IdentityInfo&)),
+                     this, SLOT(onInfoReady(const SignOn::IdentityInfo&)));
+    QObject::connect(identity, SIGNAL(error(const SignOn::Error&)),
+                     this, SLOT(onError(const SignOn::Error&)));
+    identity->queryInfo();
+}
+
+void AclUpdater::onInfoReady(const SignOn::IdentityInfo &info)
+{
+    QStringList currentAcl = info.accessControlList();
+    QString unconfined = QStringLiteral("unconfined");
+
+    if (currentAcl.contains(unconfined)) {
+        // nothing to do
+        qDebug() << "ACL already contains 'unconfined'";
+        Q_EMIT finished();
+        return;
+    }
+
+    qDebug() << "Updating identity";
+    SignOn::IdentityInfo updatedInfo(info);
+    updatedInfo.setAccessControlList(currentAcl << unconfined);
+
+    QObject::connect(identity, SIGNAL(credentialsStored(const quint32)),
+                     this, SIGNAL(finished()));
+    identity->storeCredentials(updatedInfo);
+}
+
+void AclUpdater::onError(const SignOn::Error &error)
+{
+    qWarning() << "Error updating identity" << identity->id() <<
+        ":" << error.message();
+    Q_EMIT finished();
+}
+
+int main(int argc, char **argv)
+{
+    QCoreApplication app(argc, argv);
+
+    Accounts::Manager manager;
+    AclUpdater *updater = 0;
+
+    Q_FOREACH(Accounts::AccountId accountId, manager.accountList()) {
+        Accounts::Account *account = manager.account(accountId);
+        if (account->providerName() == QStringLiteral("ubuntuone")) {
+            updater = new AclUpdater(account->credentialsId());
+            break;
+        }
+    }
+
+    if (updater) {
+        QObject::connect(updater, SIGNAL(finished()),
+                         &app, SLOT(quit()));
+        return app.exec();
+    } else {
+        return EXIT_SUCCESS;
+    }
+}

=== added file 'acl-updater/acl-updater.h'
--- acl-updater/acl-updater.h	1970-01-01 00:00:00 +0000
+++ acl-updater/acl-updater.h	2015-01-15 11:37:05 +0000
@@ -0,0 +1,48 @@
+/*
+ * Copyright 2015 Canonical Ltd.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 3 of the GNU Lesser General Public
+ * License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the
+ * Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ * Boston, MA 02110-1301, USA.
+ */
+
+#ifndef _ACL_UPDATER_H_
+#define _ACL_UPDATER_H_
+
+#include <QObject>
+
+namespace SignOn {
+    class Error;
+    class Identity;
+    class IdentityInfo;
+};
+
+class AclUpdater : public QObject
+{
+    Q_OBJECT
+
+public:
+    AclUpdater(quint32 credentialsId, QObject *parent = 0);
+
+Q_SIGNALS:
+    void finished();
+
+private Q_SLOTS:
+    void onInfoReady(const SignOn::IdentityInfo &info);
+    void onError(const SignOn::Error &error);
+
+private:
+    SignOn::Identity *identity;
+};
+
+#endif // _ACL_UPDATER_H_

=== modified file 'debian/control'
--- debian/control	2014-08-20 14:33:41 +0000
+++ debian/control	2015-01-15 11:37:05 +0000
@@ -4,6 +4,7 @@
 Build-Depends:
  cmake,
  debhelper (>= 9),
+ dh-migrations,
  libaccounts-qt5-dev,
  liboauth-dev,
  libsignon-qt5-dev,

=== added file 'debian/libubuntuoneauth-2.0-0.migrations'
--- debian/libubuntuoneauth-2.0-0.migrations	1970-01-01 00:00:00 +0000
+++ debian/libubuntuoneauth-2.0-0.migrations	2015-01-15 11:37:05 +0000
@@ -0,0 +1,1 @@
+obj-*/acl-updater/acl-updater

=== modified file 'debian/rules'
--- debian/rules	2014-07-24 14:12:32 +0000
+++ debian/rules	2015-01-15 11:37:05 +0000
@@ -7,7 +7,7 @@
 
 
 %:
-	dh $@ --buildsystem cmake --fail-missing
+	dh $@ --buildsystem cmake --fail-missing --with migrations
 
 override_dh_auto_configure:
 	dh_auto_configure -- -DLIB_SUFFIX=/$(DEB_HOST_MULTIARCH)