Ubuntu Online Accounts API

Merge lp:~mardy/online-accounts-api/password-1628473 into lp:online-accounts-api

  1. password-1628473
  2. Merge into trunk
Proposed by Alberto Mardegan
Status: Merged
Approved by: Alberto Mardegan
Approved revision: 28
Merged at revision: 32
Proposed branch: lp:~mardy/online-accounts-api/password-1628473
Merge into: lp:online-accounts-api
Diff against target: 435 lines (+131/-42)
7 files modified
src/lib/OnlineAccountsDaemon/authenticator.cpp (+37/-3)
src/lib/OnlineAccountsDaemon/authenticator.h (+2/-0)
src/lib/OnlineAccountsDaemon/manager.cpp (+2/-21)
tests/daemon/functional_tests/data/com.ubuntu.tests_coolshare.service (+7/-0)
tests/daemon/functional_tests/fake_signond.h (+4/-0)
tests/daemon/functional_tests/functional_tests.cpp (+69/-16)
tests/daemon/functional_tests/signond.py (+10/-2)
To merge this branch: bzr merge lp:~mardy/online-accounts-api/password-1628473
Reviewer Review Type Date Requested Status
David Barth (community) Approve
Review via email: mp+307026@code.launchpad.net

Commit message

Fix reply of password-based authentication

The keys for the username and password are different from those used by signond, and need to be translated.

Description of the change

Fix reply of password-based authentication

The keys for the username and password are different from those used by signond, and need to be translated.

To post a comment you must log in.
lp:~mardy/online-accounts-api/password-1628473 updated
28. By Alberto Mardegan

From trunk

* debian/control:
  - Depend on qttools5-dev-tools for qdoc (LP: #1608814)
* debian/rules:
  - No need to invoke dh_python

Revision history for this message
David Barth (dbarth) wrote :

LGTM. Nice code reorg as well.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/lib/OnlineAccountsDaemon/authenticator.cpp'
2--- src/lib/OnlineAccountsDaemon/authenticator.cpp 2016-02-12 07:44:03 +0000
3+++ src/lib/OnlineAccountsDaemon/authenticator.cpp 2016-10-06 06:56:41 +0000
4@@ -71,6 +71,7 @@
5 SignOn::AuthSession *m_authSession;
6 SignOn::Identity *m_identity;
7 QVariantMap m_parameters;
8+ int m_authMethod;
9 QVariantMap m_reply;
10 QVariantMap m_extraReplyData;
11 QString m_errorName;
12@@ -85,6 +86,7 @@
13 QObject(q),
14 m_authSession(0),
15 m_identity(0),
16+ m_authMethod(ONLINE_ACCOUNTS_AUTH_METHOD_UNKNOWN),
17 m_invalidateCache(false),
18 q_ptr(q)
19 {
20@@ -111,11 +113,14 @@
21 mergeMaps(authData.parameters(), mergeMaps(parameters, m_parameters));
22 QString mechanism = authData.mechanism();
23
24+ m_authMethod = Authenticator::authMethod(authData);
25+
26 if (m_invalidateCache) {
27 /* This works for OAuth 1.0 and 2.0; other authentication plugins should
28 * implement a similar flag. */
29 allSessionData["ForceTokenRefresh"] = true;
30- if (authData.method() == "password" || authData.method() == "sasl") {
31+ if (m_authMethod == ONLINE_ACCOUNTS_AUTH_METHOD_PASSWORD ||
32+ m_authMethod == ONLINE_ACCOUNTS_AUTH_METHOD_SASL) {
33 uint uiPolicy = allSessionData.value("UiPolicy").toUInt();
34 if (uiPolicy != SignOn::NoUserInteractionPolicy) {
35 allSessionData["UiPolicy"] = SignOn::RequestPasswordPolicy;
36@@ -124,7 +129,7 @@
37 }
38
39 m_extraReplyData.clear();
40- if (mechanism == "HMAC-SHA1" || mechanism == "PLAINTEXT") {
41+ if (m_authMethod == ONLINE_ACCOUNTS_AUTH_METHOD_OAUTH1) {
42 /* For OAuth 1.0, let's return also the Consumer key and secret along
43 * with the reply. */
44 m_extraReplyData[ONLINE_ACCOUNTS_AUTH_KEY_CONSUMER_KEY] =
45@@ -139,7 +144,17 @@
46 void AuthenticatorPrivate::onAuthSessionResponse(const SignOn::SessionData &sessionData)
47 {
48 Q_Q(Authenticator);
49- m_reply = mergeMaps(m_extraReplyData, sessionData.toMap());
50+ QVariantMap signonReply;
51+
52+ /* Perform some method-specific translation of reply keys */
53+ if (m_authMethod == ONLINE_ACCOUNTS_AUTH_METHOD_PASSWORD) {
54+ signonReply[ONLINE_ACCOUNTS_AUTH_KEY_USERNAME] = sessionData.UserName();
55+ signonReply[ONLINE_ACCOUNTS_AUTH_KEY_PASSWORD] = sessionData.Secret();
56+ } else {
57+ signonReply = sessionData.toMap();
58+ }
59+
60+ m_reply = mergeMaps(m_extraReplyData, signonReply);
61 Q_EMIT q->finished();
62 }
63
64@@ -231,4 +246,23 @@
65 return d->m_errorMessage;
66 }
67
68+int Authenticator::authMethod(const Accounts::AuthData &authData)
69+{
70+ QString method = authData.method();
71+ QString mechanism = authData.mechanism();
72+ if (method == "oauth2") {
73+ if (mechanism == "web_server" || mechanism == "user_agent") {
74+ return ONLINE_ACCOUNTS_AUTH_METHOD_OAUTH2;
75+ } else if (mechanism == "HMAC-SHA1" || mechanism == "PLAINTEXT") {
76+ return ONLINE_ACCOUNTS_AUTH_METHOD_OAUTH1;
77+ }
78+ } else if (method == "sasl") {
79+ return ONLINE_ACCOUNTS_AUTH_METHOD_SASL;
80+ } else if (method == "password") {
81+ return ONLINE_ACCOUNTS_AUTH_METHOD_PASSWORD;
82+ }
83+
84+ return ONLINE_ACCOUNTS_AUTH_METHOD_UNKNOWN;
85+}
86+
87 #include "authenticator.moc"
88
89=== modified file 'src/lib/OnlineAccountsDaemon/authenticator.h'
90--- src/lib/OnlineAccountsDaemon/authenticator.h 2015-08-28 14:25:23 +0000
91+++ src/lib/OnlineAccountsDaemon/authenticator.h 2016-10-06 06:56:41 +0000
92@@ -51,6 +51,8 @@
93 QString errorName() const;
94 QString errorMessage() const;
95
96+ static int authMethod(const Accounts::AuthData &authData);
97+
98 Q_SIGNALS:
99 void finished();
100
101
102=== modified file 'src/lib/OnlineAccountsDaemon/manager.cpp'
103--- src/lib/OnlineAccountsDaemon/manager.cpp 2016-01-13 14:23:12 +0000
104+++ src/lib/OnlineAccountsDaemon/manager.cpp 2016-10-06 06:56:41 +0000
105@@ -33,6 +33,7 @@
106 #include <QSet>
107 #include "access_request.h"
108 #include "authentication_request.h"
109+#include "authenticator.h"
110 #include "client_registry.h"
111 #include "dbus_constants.h"
112 #include "manager_adaptor.h"
113@@ -76,7 +77,6 @@
114 const QString &serviceName,
115 const QStringList &clients);
116
117- int authMethod(const Accounts::AuthData &authData);
118 AccountInfo readAccountInfo(const Accounts::AccountService *as);
119 QList<AccountInfo> getAccounts(const QVariantMap &filters,
120 const CallContext &context);
121@@ -317,25 +317,6 @@
122 return activeAccount;
123 }
124
125-int ManagerPrivate::authMethod(const Accounts::AuthData &authData)
126-{
127- QString method = authData.method();
128- QString mechanism = authData.mechanism();
129- if (method == "oauth2") {
130- if (mechanism == "web_server" || mechanism == "user_agent") {
131- return ONLINE_ACCOUNTS_AUTH_METHOD_OAUTH2;
132- } else if (mechanism == "HMAC-SHA1" || mechanism == "PLAINTEXT") {
133- return ONLINE_ACCOUNTS_AUTH_METHOD_OAUTH1;
134- }
135- } else if (method == "sasl") {
136- return ONLINE_ACCOUNTS_AUTH_METHOD_SASL;
137- } else if (method == "password") {
138- return ONLINE_ACCOUNTS_AUTH_METHOD_PASSWORD;
139- }
140-
141- return ONLINE_ACCOUNTS_AUTH_METHOD_UNKNOWN;
142-}
143-
144 AccountInfo ManagerPrivate::readAccountInfo(const Accounts::AccountService *as)
145 {
146 QVariantMap info;
147@@ -343,7 +324,7 @@
148 info[ONLINE_ACCOUNTS_INFO_KEY_DISPLAY_NAME] = as->account()->displayName();
149 info[ONLINE_ACCOUNTS_INFO_KEY_SERVICE_ID] = as->service().name();
150
151- info[ONLINE_ACCOUNTS_INFO_KEY_AUTH_METHOD] = authMethod(as->authData());
152+ info[ONLINE_ACCOUNTS_INFO_KEY_AUTH_METHOD] = Authenticator::authMethod(as->authData());
153 QString settingsPrefix(QStringLiteral(ONLINE_ACCOUNTS_INFO_KEY_SETTINGS));
154 /* First, read the global settings */
155 Accounts::Account *a = as->account();
156
157=== modified file 'tests/daemon/functional_tests/data/com.ubuntu.tests_coolshare.service'
158--- tests/daemon/functional_tests/data/com.ubuntu.tests_coolshare.service 2015-07-08 15:09:56 +0000
159+++ tests/daemon/functional_tests/data/com.ubuntu.tests_coolshare.service 2016-10-06 06:56:41 +0000
160@@ -4,4 +4,11 @@
161 <name>Cool Share</name>
162 <icon>general_otherservice</icon>
163 <provider>cool</provider>
164+
165+ <template>
166+ <group name="auth">
167+ <setting name="method">password</setting>
168+ <setting name="mechanism">password</setting>
169+ </group>
170+ </template>
171 </service>
172
173=== modified file 'tests/daemon/functional_tests/fake_signond.h'
174--- tests/daemon/functional_tests/fake_signond.h 2015-07-06 14:51:30 +0000
175+++ tests/daemon/functional_tests/fake_signond.h 2016-10-06 06:56:41 +0000
176@@ -37,6 +37,10 @@
177 mockedAuthService().call("AddIdentity", id, info);
178 }
179
180+ void setNextReply(uint identity, const QVariantMap &reply) {
181+ mockedAuthService().call("SetNextReply", identity, reply);
182+ }
183+
184 private:
185 OrgFreedesktopDBusMockInterface &mockedAuthService() {
186 return m_mock->mockInterface("com.google.code.AccountsSSO.SingleSignOn",
187
188=== modified file 'tests/daemon/functional_tests/functional_tests.cpp'
189--- tests/daemon/functional_tests/functional_tests.cpp 2016-07-22 05:08:11 +0000
190+++ tests/daemon/functional_tests/functional_tests.cpp 2016-10-06 06:56:41 +0000
191@@ -186,6 +186,7 @@
192 EnvSetup m_env;
193 DBusService *m_dbus;
194 int m_firstAccountId;
195+ int m_account2CredentialsId;
196 int m_account3CredentialsId;
197 };
198
199@@ -206,6 +207,7 @@
200 FunctionalTests::FunctionalTests():
201 QObject(),
202 m_dbus(0),
203+ m_account2CredentialsId(41),
204 m_account3CredentialsId(35)
205 {
206 clearDb();
207@@ -230,6 +232,7 @@
208 QVERIFY(account2 != 0);
209 account2->setEnabled(true);
210 account2->setDisplayName("CoolAccount 2");
211+ account2->setCredentialsId(m_account2CredentialsId);
212 account2->selectService(coolMail);
213 account2->setEnabled(false);
214 account2->selectService(coolShare);
215@@ -262,6 +265,9 @@
216 {
217 m_dbus = new DBusService();
218 m_dbus->startServices();
219+
220+ /* Uncomment next line to debug DBus calls */
221+ // QProcess::startDetached("/usr/bin/dbus-monitor");
222 }
223
224 void FunctionalTests::cleanup()
225@@ -321,16 +327,19 @@
226 QTest::addColumn<bool>("interactive");
227 QTest::addColumn<bool>("invalidate");
228 QTest::addColumn<QVariantMap>("authParams");
229+ QTest::addColumn<QVariantMap>("signonReply");
230 QTest::addColumn<QVariantMap>("expectedCredentials");
231 QTest::addColumn<QString>("errorName");
232
233 QVariantMap authParams;
234+ QVariantMap signonReply;
235 QVariantMap credentials;
236 QTest::newRow("invalid account ID") <<
237 12412341 <<
238 "coolmail" <<
239 false << false <<
240 authParams <<
241+ signonReply <<
242 credentials <<
243 ONLINE_ACCOUNTS_ERROR_PERMISSION_DENIED;
244
245@@ -341,6 +350,7 @@
246 "coolmail" <<
247 false << false <<
248 authParams <<
249+ signonReply <<
250 credentials <<
251 "com.ubuntu.OnlineAccounts.Error.Network";
252 authParams.clear();
253@@ -354,6 +364,7 @@
254 "coolmail" <<
255 false << false <<
256 authParams <<
257+ signonReply <<
258 credentials <<
259 QString();
260
261@@ -363,6 +374,7 @@
262 "coolmail" <<
263 true << false <<
264 authParams <<
265+ signonReply <<
266 credentials <<
267 QString();
268
269@@ -372,6 +384,7 @@
270 "coolmail" <<
271 true << true <<
272 authParams <<
273+ signonReply <<
274 credentials <<
275 QString();
276
277@@ -385,6 +398,7 @@
278 "oauth1auth" <<
279 true << false <<
280 authParams <<
281+ signonReply <<
282 credentials <<
283 QString();
284
285@@ -399,6 +413,25 @@
286 "oauth1auth" <<
287 true << false <<
288 authParams <<
289+ signonReply <<
290+ credentials <<
291+ QString();
292+
293+ authParams.clear();
294+ credentials = QVariantMap {
295+ { ONLINE_ACCOUNTS_AUTH_KEY_USERNAME, "my name" },
296+ { ONLINE_ACCOUNTS_AUTH_KEY_PASSWORD, "don't tell" },
297+ };
298+ signonReply = QVariantMap {
299+ { "UserName", "my name" },
300+ { "Secret", "don't tell" },
301+ };
302+ QTest::newRow("password") <<
303+ 2 <<
304+ "com.ubuntu.tests_coolshare" <<
305+ true << false <<
306+ authParams <<
307+ signonReply <<
308 credentials <<
309 QString();
310 }
311@@ -410,10 +443,16 @@
312 QFETCH(bool, interactive);
313 QFETCH(bool, invalidate);
314 QFETCH(QVariantMap, authParams);
315+ QFETCH(QVariantMap, signonReply);
316 QFETCH(QVariantMap, expectedCredentials);
317 QFETCH(QString, errorName);
318
319+ m_dbus->signond().addIdentity(m_account2CredentialsId, QVariantMap());
320 m_dbus->signond().addIdentity(m_account3CredentialsId, QVariantMap());
321+ if (!signonReply.isEmpty()) {
322+ m_dbus->signond().setNextReply(m_account2CredentialsId, signonReply);
323+ m_dbus->signond().setNextReply(m_account3CredentialsId, signonReply);
324+ }
325
326 DaemonInterface *daemon = new DaemonInterface(m_dbus->sessionConnection());
327
328@@ -425,8 +464,10 @@
329 if (errorName.isEmpty()) {
330 QVERIFY2(!reply.isError(), reply.error().message().toUtf8().constData());
331 QVariantMap credentials = reply.argumentAt<0>();
332- // Add the requestor PID
333- expectedCredentials["requestorPid"] = getpid();
334+ if (credentials.contains("requestorPid")) {
335+ // Add the requestor PID
336+ expectedCredentials["requestorPid"] = getpid();
337+ }
338 QCOMPARE(credentials, expectedCredentials);
339 } else {
340 QVERIFY(reply.isError());
341@@ -561,11 +602,14 @@
342
343 QCOMPARE(serviceId, coolShare.name());
344 QCOMPARE(accountInfo.id(), account->id());
345- QVariantMap expectedAccountInfo;
346- expectedAccountInfo["authMethod"] = ONLINE_ACCOUNTS_AUTH_METHOD_UNKNOWN;
347- expectedAccountInfo["changeType"] = ONLINE_ACCOUNTS_INFO_CHANGE_ENABLED;
348- expectedAccountInfo["displayName"] = "New account";
349- expectedAccountInfo["serviceId"] = "com.ubuntu.tests_coolshare";
350+ QVariantMap expectedAccountInfo {
351+ { "authMethod", ONLINE_ACCOUNTS_AUTH_METHOD_PASSWORD },
352+ { "changeType", ONLINE_ACCOUNTS_INFO_CHANGE_ENABLED },
353+ { "displayName", "New account" },
354+ { "serviceId", "com.ubuntu.tests_coolshare" },
355+ { "settings/auth/mechanism", "password" },
356+ { "settings/auth/method", "password" },
357+ };
358 QCOMPARE(accountInfo.data(), expectedAccountInfo);
359
360 /* Change a setting */
361@@ -579,11 +623,15 @@
362
363 QCOMPARE(serviceId, coolShare.name());
364 QCOMPARE(accountInfo.id(), account->id());
365- expectedAccountInfo["authMethod"] = ONLINE_ACCOUNTS_AUTH_METHOD_UNKNOWN;
366- expectedAccountInfo["changeType"] = ONLINE_ACCOUNTS_INFO_CHANGE_UPDATED;
367- expectedAccountInfo["settings/color"] = "blue";
368- expectedAccountInfo["displayName"] = "New account";
369- expectedAccountInfo["serviceId"] = "com.ubuntu.tests_coolshare";
370+ expectedAccountInfo = QVariantMap {
371+ { "authMethod", ONLINE_ACCOUNTS_AUTH_METHOD_PASSWORD },
372+ { "changeType", ONLINE_ACCOUNTS_INFO_CHANGE_UPDATED },
373+ { "settings/color", "blue" },
374+ { "displayName", "New account" },
375+ { "serviceId", "com.ubuntu.tests_coolshare" },
376+ { "settings/auth/mechanism", "password" },
377+ { "settings/auth/method", "password" },
378+ };
379 QCOMPARE(accountInfo.data(), expectedAccountInfo);
380
381 /* Delete the account */
382@@ -597,10 +645,15 @@
383
384 QCOMPARE(serviceId, coolShare.name());
385 QCOMPARE(accountInfo.id(), account->id());
386- expectedAccountInfo["authMethod"] = ONLINE_ACCOUNTS_AUTH_METHOD_UNKNOWN;
387- expectedAccountInfo["changeType"] = ONLINE_ACCOUNTS_INFO_CHANGE_DISABLED;
388- expectedAccountInfo["displayName"] = "New account";
389- expectedAccountInfo["serviceId"] = "com.ubuntu.tests_coolshare";
390+ expectedAccountInfo = QVariantMap {
391+ { "authMethod", ONLINE_ACCOUNTS_AUTH_METHOD_PASSWORD },
392+ { "changeType", ONLINE_ACCOUNTS_INFO_CHANGE_DISABLED },
393+ { "displayName", "New account" },
394+ { "serviceId", "com.ubuntu.tests_coolshare" },
395+ { "settings/color", "blue" },
396+ { "settings/auth/mechanism", "password" },
397+ { "settings/auth/method", "password" },
398+ };
399 QCOMPARE(accountInfo.data(), expectedAccountInfo);
400
401 delete manager;
402
403=== modified file 'tests/daemon/functional_tests/signond.py'
404--- tests/daemon/functional_tests/signond.py 2015-07-23 13:57:48 +0000
405+++ tests/daemon/functional_tests/signond.py 2016-10-06 06:56:41 +0000
406@@ -48,7 +48,11 @@
407 return (path, self.identities[identity])
408
409
410-def auth_session_process(identity, params, method):
411+def auth_session_process(self, params, method):
412+ auth_service = dbusmock.get_object(MAIN_OBJ)
413+ if self.identity in auth_service.auth_replies:
414+ return auth_service.auth_replies[self.identity]
415+
416 if 'errorName' in params:
417 raise dbus.exceptions.DBusException('Authentication error',
418 name=params['errorName'])
419@@ -63,7 +67,7 @@
420 path = '/AuthSession%s' % self.sessions_counter
421 self.sessions_counter += 1
422 self.AddObject(path, AUTH_SESSION_IFACE, {}, [
423- ('process', 'a{sv}s', 'a{sv}', 'ret = self.auth_session_process(self.identity, args[0], args[1])'),
424+ ('process', 'a{sv}s', 'a{sv}', 'ret = self.auth_session_process(self, args[0], args[1])'),
425 ])
426
427 auth_session = dbusmock.get_object(path)
428@@ -91,3 +95,7 @@
429 def AddIdentity(self, identity, data):
430 self.identities[identity] = data
431
432+@dbus.service.method(MOCK_IFACE, in_signature='ua{sv}', out_signature='')
433+def SetNextReply(self, identity, reply):
434+ self.auth_replies[identity] = reply
435+

Subscribers

People subscribed via source and target branches