Ubuntu Online Accounts API

Merge lp:~mardy/online-accounts-api/dbus-api into lp:online-accounts-api

  1. dbus-api
  2. Merge into trunk
Proposed by Alberto Mardegan
Status: Superseded
Proposed branch: lp:~mardy/online-accounts-api/dbus-api
Merge into: lp:online-accounts-api
Diff against target: 481 lines (+157/-158)
6 files modified
src/daemon/aacontext.cpp (+31/-26)
src/daemon/aacontext.h (+2/-2)
src/daemon/main.cpp (+2/-1)
src/daemon/manager.cpp (+77/-64)
src/daemon/manager.h (+26/-22)
src/daemon/manager.xml (+19/-43)
To merge this branch: bzr merge lp:~mardy/online-accounts-api/dbus-api
Reviewer Review Type Date Requested Status
James Henstridge Needs Fixing
Review via email: mp+249089@code.launchpad.net

This proposal has been superseded by a proposal from 2015-02-11.

Commit message

Update D-Bus API, fix warnings

Description of the change

Update D-Bus API, fix warnings

To post a comment you must log in.
Revision history for this message
James Henstridge (jamesh) wrote :

Hi Alberto,

Many of these changes seem to be things that we disagreed about in the meeting previously. I have also been working on updating the interface as we discussed at that meeting, so can we hold off on this?

I've left some inline comments in the diff.

review: Needs Fixing
Revision history for this message
Alberto Mardegan (mardy) wrote :

> Many of these changes seem to be things that we disagreed about in the meeting
> previously. I have also been working on updating the interface as we
> discussed at that meeting, so can we hold off on this?

I'm proposing this exactly in order to discuss about the disagreements. :-)
Keep in mind that what we currently have in trunk has not been agreed upon either. Anyway, I won't merge this until we've got a consensus, or until we come to the conclusion that a consensus cannot be reached.
On the other hand, defining this interface is our top priority, because all the client side will be blocked until we settle this.

lp:~mardy/online-accounts-api/dbus-api updated
5. By Alberto Mardegan

Update

Revision history for this message
Alberto Mardegan (mardy) wrote :

I just pushed a new commit, taking your comments into account. I also replied to them, but in order to see my replies you'll have to select the "r4 into r3" diff into the "Preview Diff" selection box below.

Revision history for this message
James Henstridge (jamesh) wrote :

Replies to r4 comments.

review: Needs Fixing
Revision history for this message
James Henstridge (jamesh) wrote :

Now for r5:

As a general question, what style guide are you trying to convert this code to conform to exactly? I'd really appreciate it if you didn't mix these kinds of code clean ups in with functional changes: all it does is obscure what you're changing.

lp:~mardy/online-accounts-api/dbus-api updated
6. By Alberto Mardegan

merge from style

7. By Alberto Mardegan

Update

8. By Alberto Mardegan

style

9. By Alberto Mardegan

merge style

10. By Alberto Mardegan

from trunk

11. By Alberto Mardegan

Update docstring

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/daemon/aacontext.cpp'
--- src/daemon/aacontext.cpp 2015-02-10 08:12:59 +0000
+++ src/daemon/aacontext.cpp 2015-02-10 16:00:33 +0000
@@ -1,47 +1,52 @@
1#include <QDBusConnection>
2#include <QDBusMessage>
3#include <QDebug>
1#include "aacontext.h"4#include "aacontext.h"
2#include <stdexcept>
3#include <sys/apparmor.h>5#include <sys/apparmor.h>
4#include <QDebug>6
5#include <QDBusConnection>7const int maxCacheSize = 50;
6#include <QDBusMessage>8
79AppArmorContext::AppArmorContext()
8const int max_cache_size = 50;10{
911}
10AppArmorContext::AppArmorContext() {12
11}13AppArmorContext::~AppArmorContext()
1214{
13AppArmorContext::~AppArmorContext() {15}
14}16
1517QString AppArmorContext::getPeerSecurityContext(const QDBusConnection &bus,
16QString AppArmorContext::getPeerSecurityContext(const QDBusConnection &bus, const QDBusMessage &message) {18 const QDBusMessage &message)
19{
17 if (!aa_is_enabled()) {20 if (!aa_is_enabled()) {
18 return "unconfined";21 return "unconfined";
19 }22 }
2023
21 QString peer_address = message.service();24 QString peer_address = message.service();
22 try {25 QString &context = m_contexts[peer_address];
23 return contexts.at(peer_address);26 /* If the peer_address was now known, it's now been added as an empty
24 } catch (const std::out_of_range &e) {27 * QString. */
28 if (!context.isEmpty()) {
29 return context;
25 }30 }
2631
27 QDBusMessage msg = QDBusMessage::createMethodCall(32 QDBusMessage msg =
28 "org.freedesktop.DBus", "/org/freedesktop/DBus",33 QDBusMessage::createMethodCall("org.freedesktop.DBus",
29 "org.freedesktop.DBus", "GetConnectionAppArmorSecurityContext");34 "/org/freedesktop/DBus",
35 "org.freedesktop.DBus",
36 "GetConnectionAppArmorSecurityContext");
30 msg << peer_address;37 msg << peer_address;
31 QDBusMessage reply = bus.call(msg, QDBus::Block);38 QDBusMessage reply = bus.call(msg, QDBus::Block);
3239
33 QString context;
34 if (reply.type() == QDBusMessage::ReplyMessage) {40 if (reply.type() == QDBusMessage::ReplyMessage) {
35 context = reply.arguments().value(0).value<QString>();41 context = reply.arguments().value(0).value<QString>();
36 } else {42 } else {
37 qWarning() << "Could not determine AppArmor context: "43 qWarning() << "Could not determine AppArmor context: " <<
38 << reply.errorName() << ": " << reply.errorMessage();44 reply.errorName() << ": " << reply.errorMessage();
39 }45 }
4046
41 // If the context cache has hit the maximum size, clear it47 // If the context cache has hit the maximum size, clear it
42 if (contexts.size() >= max_cache_size) {48 if (m_contexts.size() >= maxCacheSize) {
43 contexts.clear();49 m_contexts.clear();
44 }50 }
45 contexts[peer_address] = context;
46 return context;51 return context;
47}52}
4853
=== modified file 'src/daemon/aacontext.h'
--- src/daemon/aacontext.h 2015-02-09 05:18:22 +0000
+++ src/daemon/aacontext.h 2015-02-10 16:00:33 +0000
@@ -1,9 +1,9 @@
1#ifndef AACONTEXT_H1#ifndef AACONTEXT_H
2#define AACONTEXT_H2#define AACONTEXT_H
33
4#include <map>
5#include <QDBusConnection>4#include <QDBusConnection>
6#include <QDBusMessage>5#include <QDBusMessage>
6#include <QHash>
7#include <QString>7#include <QString>
88
9class AppArmorContext {9class AppArmorContext {
@@ -14,7 +14,7 @@
14 QString getPeerSecurityContext(const QDBusConnection &bus, const QDBusMessage &message);14 QString getPeerSecurityContext(const QDBusConnection &bus, const QDBusMessage &message);
1515
16private:16private:
17 std::map<QString,QString> contexts;17 QHash<QString,QString> m_contexts;
18};18};
1919
20#endif20#endif
2121
=== modified file 'src/daemon/main.cpp'
--- src/daemon/main.cpp 2015-02-10 06:24:38 +0000
+++ src/daemon/main.cpp 2015-02-10 16:00:33 +0000
@@ -1,7 +1,8 @@
1#include "manager.h"1#include "manager.h"
2#include "manageradaptor.h"2#include "manageradaptor.h"
33
4int main(int argc, char **argv) {4int main(int argc, char **argv)
5{
5 QCoreApplication app(argc, argv);6 QCoreApplication app(argc, argv);
67
7 qDBusRegisterMetaType<AccountInfo>();8 qDBusRegisterMetaType<AccountInfo>();
89
=== modified file 'src/daemon/manager.cpp'
--- src/daemon/manager.cpp 2015-02-10 06:24:38 +0000
+++ src/daemon/manager.cpp 2015-02-10 16:00:33 +0000
@@ -1,40 +1,48 @@
1#include "manager.h"1#include <QDBusMessage>
2#include <QDebug>2#include <QDebug>
3#include <QDBusMessage>
4#include "aacontext.h"3#include "aacontext.h"
54#include "manager.h"
6using namespace std;
75
8static const char FORBIDDEN_ERROR[] = "com.ubuntu.OnlineAccounts.Error.Forbidden";6static const char FORBIDDEN_ERROR[] = "com.ubuntu.OnlineAccounts.Error.Forbidden";
97
10QDBusArgument &operator<<(QDBusArgument &argument, const AccountInfo &info) {8QDBusArgument &operator<<(QDBusArgument &argument, const AccountInfo &info)
11 argument.beginStructure();9{
12 argument << info.account_id << info.details;10 argument.beginStructure();
13 argument.endStructure();11 argument << info.accountId << info.details;
14 return argument;12 argument.endStructure();
15}13 return argument;
1614}
17const QDBusArgument &operator>>(const QDBusArgument &argument, AccountInfo &info) {15
18 argument.beginStructure();16const QDBusArgument &operator>>(const QDBusArgument &argument,
19 argument >> info.account_id >> info.details;17 AccountInfo &info)
20 argument.endStructure();18{
21 return argument;19 argument.beginStructure();
22}20 argument >> info.accountId >> info.details;
2321 argument.endStructure();
2422 return argument;
25struct Manager::Private {23}
26 AppArmorContext apparmor;24
25class ManagerPrivate {
26public:
27 AppArmorContext m_apparmor;
27};28};
2829
29Manager::Manager(QObject *parent)30Manager::Manager(QObject *parent):
30 : QObject(parent), p(new Private) {31 QObject(parent),
31}32 d_ptr(new ManagerPrivate)
3233{
33Manager::~Manager() {34}
34}35
3536Manager::~Manager()
36bool Manager::canAccess(const QString &service_id) {37{
37 QString context = p->apparmor.getPeerSecurityContext(connection(), message());38}
39
40bool Manager::canAccess(const QString &serviceId)
41{
42 Q_D(Manager);
43
44 QString context = d->m_apparmor.getPeerSecurityContext(connection(),
45 message());
38 // Could not determine peer's AppArmor context, so deny access46 // Could not determine peer's AppArmor context, so deny access
39 if (context.isEmpty()) {47 if (context.isEmpty()) {
40 return false;48 return false;
@@ -55,51 +63,56 @@
55 // Do the same on the service ID: we are only dealing with63 // Do the same on the service ID: we are only dealing with
56 // confined apps at this point, so only $pkgname prefixed64 // confined apps at this point, so only $pkgname prefixed
57 // services are accessible.65 // services are accessible.
58 pos = service_id.indexOf('_');66 pos = serviceId.indexOf('_');
59 if (pos < 0) {67 if (pos < 0) {
60 return false;68 return false;
61 }69 }
62 return service_id.left(pos) == pkgname;70 return serviceId.left(pos) == pkgname;
63}71}
6472
65bool Manager::checkAccess(const QString &service_id) {73bool Manager::checkAccess(const QString &serviceId)
66 bool has_access = canAccess(service_id);74{
67 if (!has_access) {75 bool hasAccess = canAccess(serviceId);
68 sendErrorReply(FORBIDDEN_ERROR, QString("Access to service ID %1 forbidden").arg(service_id));76 if (!hasAccess) {
69 }77 sendErrorReply(FORBIDDEN_ERROR,
70 return has_access;78 QString("Access to service ID %1 forbidden").arg(serviceId));
71}79 }
7280 return hasAccess;
73QList<AccountInfo> Manager::GetAccounts(const QStringList &service_ids) {81}
74 for (const auto &service_id : service_ids) {82
75 if (!checkAccess(service_id)) {83QList<AccountInfo> Manager::GetAccounts(const QVariantMap &filters)
76 return QList<AccountInfo>();84{
77 }85 Q_UNUSED(filters);
78 }86
7987 return QList<AccountInfo>();
80 return QList<AccountInfo>({AccountInfo(0, QVariantMap())});88}
81}89
8290QVariantMap Manager::Authenticate(quint32 accountId, const QString &serviceId,
83AccountInfo Manager::GetAccountInfo(const QString &service_id, uint account_id) {91 bool interactive, bool invalidate,
84 if (!checkAccess(service_id)) {92 const QVariantMap &parameters)
85 return AccountInfo();93{
86 }94 Q_UNUSED(accountId);
8795 Q_UNUSED(interactive);
88 return AccountInfo(account_id, QVariantMap());96 Q_UNUSED(invalidate);
89}97 Q_UNUSED(parameters);
9098
91QVariantMap Manager::Authenticate(const QString &service_id, uint account_id, bool interactive, bool invalidate) {99 if (!checkAccess(serviceId)) {
92 if (!checkAccess(service_id)) {
93 return QVariantMap();100 return QVariantMap();
94 }101 }
95102
96 return QVariantMap();103 return QVariantMap();
97}104}
98105
99AccountInfo Manager::Register(const QString &service_id, QVariantMap &credentials) {106AccountInfo Manager::RequestAccess(const QString &serviceId,
100 if (!checkAccess(service_id)) {107 const QVariantMap &parameters,
108 QVariantMap &credentials)
109{
110 Q_UNUSED(parameters);
111
112 if (!checkAccess(serviceId)) {
101 return AccountInfo();113 return AccountInfo();
102 }114 }
103115
116 credentials = QVariantMap();
104 return AccountInfo();117 return AccountInfo();
105}118}
106119
=== modified file 'src/daemon/manager.h'
--- src/daemon/manager.h 2015-02-10 06:24:38 +0000
+++ src/daemon/manager.h 2015-02-10 16:00:33 +0000
@@ -1,50 +1,54 @@
1
2#ifndef MANAGER_H1#ifndef MANAGER_H
3#define MANAGER_H2#define MANAGER_H
43
5#include <memory>4#include <QDBusArgument>
5#include <QDBusContext>
6#include <QList>6#include <QList>
7#include <QObject>7#include <QObject>
8#include <QVariantMap>8#include <QVariantMap>
9#include <QDBusArgument>
10#include <QDBusContext>
119
12struct AccountInfo {10struct AccountInfo {
13 uint account_id = 0;11 quint32 accountId;
14 QVariantMap details;12 QVariantMap details;
1513
16 AccountInfo() {}14 AccountInfo(): accountId(0) {}
17 AccountInfo(uint account_id, const QVariantMap &details)15 AccountInfo(uint accountId, const QVariantMap &details):
18 : account_id(account_id), details(details) {}16 accountId(accountId), details(details) {}
19
20};17};
21Q_DECLARE_METATYPE(AccountInfo)18Q_DECLARE_METATYPE(AccountInfo)
2219
23QDBusArgument &operator<<(QDBusArgument &argument, const AccountInfo &info);20QDBusArgument &operator<<(QDBusArgument &argument, const AccountInfo &info);
24const QDBusArgument &operator>>(const QDBusArgument &argument, AccountInfo &info);21const QDBusArgument &operator>>(const QDBusArgument &argument,
22 AccountInfo &info);
2523
26class Manager : public QObject, protected QDBusContext {24class ManagerPrivate;
25class Manager: public QObject, protected QDBusContext
26{
27 Q_OBJECT27 Q_OBJECT
28 struct Private;28
29public:29public:
30 explicit Manager(QObject *parent=nullptr);30 explicit Manager(QObject *parent = 0);
31 ~Manager();31 ~Manager();
3232
33public Q_SLOTS:33public Q_SLOTS:
34 QList<AccountInfo> GetAccounts(const QStringList &service_ids);34 QList<AccountInfo> GetAccounts(const QVariantMap &filters);
35 AccountInfo GetAccountInfo(const QString &service_id, uint account_id);35 QVariantMap Authenticate(quint32 accountId, const QString &serviceId,
36 QVariantMap Authenticate(const QString &service_id, uint account_id, bool interactive, bool invalidate);36 bool interactive, bool invalidate,
37 AccountInfo Register(const QString &service_id, QVariantMap &credentials);37 const QVariantMap &parameters);
38 AccountInfo RequestAccess(const QString &serviceId,
39 const QVariantMap &parameters,
40 QVariantMap &credentials);
3841
39Q_SIGNALS:42Q_SIGNALS:
40 void AccountChanged(const QString &service_id, uint account_id, bool enabled);43 void AccountChanged(AccountInfo accountInfo);
41 void CredentialsChanged(const QString &service_id, uint account_id);
4244
43private:45private:
44 bool canAccess(const QString &service_id);46 bool canAccess(const QString &serviceId);
45 bool checkAccess(const QString &service_id);47 bool checkAccess(const QString &serviceId);
46 QString getPeerSecurityContext();48 QString getPeerSecurityContext();
47 std::unique_ptr<Private> p;49
50 Q_DECLARE_PRIVATE(Manager)
51 ManagerPrivate *d_ptr;
48};52};
4953
50#endif54#endif
5155
=== modified file 'src/daemon/manager.xml'
--- src/daemon/manager.xml 2015-02-10 07:33:36 +0000
+++ src/daemon/manager.xml 2015-02-10 16:00:33 +0000
@@ -5,27 +5,18 @@
5 <interface name="com.ubuntu.OnlineAccounts.Manager">5 <interface name="com.ubuntu.OnlineAccounts.Manager">
66
7 <!--7 <!--
8 GetAccounts: returns a list containing account information for8 GetAccounts: returns a list of account IDs that satisfy the given
9 accounts that provide one of a list of service IDs.9 filters.
10 -->10 -->
11 <method name="GetAccounts">11 <method name="GetAccounts">
12 <arg name="service_ids" type="as" direction="in" />12 <arg name="filters" type="a{sv}" direction="in" />
13 <arg name="accounts" type="a(ua{sv})" direction="out" />13 <arg name="accounts" type="a(ua{sv})" direction="out" />
14 <annotation name="org.qtproject.QtDBus.QtTypeName.In0" value="QVariantMap"/>
14 <annotation name="org.qtproject.QtDBus.QtTypeName.Out0"15 <annotation name="org.qtproject.QtDBus.QtTypeName.Out0"
15 value="QList&lt;AccountInfo&gt;"/>16 value="QList&lt;AccountInfo&gt;"/>
16 </method>17 </method>
1718
18 <!--19 <!--
19 GetAccountInfo: return information about a given account ID
20 -->
21 <method name="GetAccountInfo">
22 <arg name="service_id" type="s" direction="in" />
23 <arg name="account_id" type="u" direction="in" />
24 <arg name="details" type="(ua{sv})" direction="out" />
25 <annotation name="org.qtproject.QtDBus.QtTypeName.Out0" value="AccountInfo"/>
26 </method>
27
28 <!--
29 Authenticate: request authentication credentials for the given20 Authenticate: request authentication credentials for the given
30 account ID in the context of a particualr service.21 account ID in the context of a particualr service.
3122
@@ -38,54 +29,39 @@
38 provider.29 provider.
39 -->30 -->
40 <method name="Authenticate">31 <method name="Authenticate">
41 <arg name="service_id" type="s" direction="in" />32 <arg name="accountId" type="u" direction="in" />
42 <arg name="account_id" type="u" direction="in" />33 <arg name="serviceId" type="s" direction="in" />
43 <arg name="interactive" type="b" direction="in" />34 <arg name="interactive" type="b" direction="in" />
44 <arg name="invalidate" type="b" direction="in" />35 <arg name="invalidate" type="b" direction="in" />
45 <!-- <arg name="reply_socket" type="h" direction="in" /> -->36 <arg name="parameters" type="a{sv}" direction="in" />
46 <arg name="credentials" type="a{sv}" direction="out" />37 <arg name="credentials" type="a{sv}" direction="out" />
38 <annotation name="org.qtproject.QtDBus.QtTypeName.In4" value="QVariantMap"/>
47 <annotation name="org.qtproject.QtDBus.QtTypeName.Out0" value="QVariantMap"/>39 <annotation name="org.qtproject.QtDBus.QtTypeName.Out0" value="QVariantMap"/>
48 </method>40 </method>
4941
50 <!--42 <!--
51 Register: register a new account for use with the given43 RequestAccess: register a new account for use with the given
52 service.44 service.
53 -->45 -->
54 <method name="Register">46 <method name="RequestAccess">
55 <arg name="service_id" type="s" direction="in" />47 <arg name="serviceId" type="s" direction="in" />
56 <!-- <arg name="reply_socket" type="h" direction="in" /> -->48 <arg name="parameters" type="a{sv}" direction="in" />
57 <arg name="account" type="(ua{sv})" direction="out" />49 <arg name="account" type="(ua{sv})" direction="out" />
50 <arg name="credentials" type="a{sv}" direction="out" />
51 <annotation name="org.qtproject.QtDBus.QtTypeName.In1" value="QVariantMap"/>
58 <annotation name="org.qtproject.QtDBus.QtTypeName.Out0" value="AccountInfo"/>52 <annotation name="org.qtproject.QtDBus.QtTypeName.Out0" value="AccountInfo"/>
59 <arg name="credentials" type="a{sv}" direction="out" />
60 <annotation name="org.qtproject.QtDBus.QtTypeName.Out1" value="QVariantMap"/>53 <annotation name="org.qtproject.QtDBus.QtTypeName.Out1" value="QVariantMap"/>
61 </method>54 </method>
6255
63 <!--56 <!--
64 AccountChanged: emitted when account details are changed.57 AccountChanged: emitted when account details are changed.
65
66 This signal will be emitted when new accounts are enabled, and
67 when existing accounts are disabled. Clients can detect these
68 cases by checking the "enabled" flag and comparing the account
69 ID with the list of accounts they currently know about.
70
71 The actual changed account details can be retrieved with
72 GetAccountInfo().
73 -->58 -->
74 <signal name="AccountChanged">59 <signal name="AccountChanged">
75 <arg name="service_id" type="s" />60 <!-- the dictionary ontains a changeType key, type "u", whose value is
76 <arg name="account_id" type="u" />61 enum { enabled, disabled, changed }
77 <arg name="enabled" type="b" />62 -->
78 </signal>63 <arg name="account" type="(ua{sv})" />
7964 <annotation name="org.qtproject.QtDBus.QtTypeName.In0" value="AccountInfo"/>
80 <!--
81 CredentialsChanged: emitted when the credentials for the given
82 service ID on the given account change.
83
84 The new credentials can be retrieved with Authenticate().
85 -->
86 <signal name="CredentialsChanged">
87 <arg name="service_id" type="s" />
88 <arg name="account_id" type="u" />
89 </signal>65 </signal>
9066
91 </interface>67 </interface>

Subscribers

People subscribed via source and target branches