ubuntuone-credentials

Merge lp:~mardy/ubuntuone-credentials/signon-plugin-part2 into lp:ubuntuone-credentials

  1. signon-plugin-part2
  2. Merge into trunk
Proposed by Alberto Mardegan on 2016-04-28
Status: Rejected
Rejected by: Natalia Bidart on 2017-05-19
Proposed branch: lp:~mardy/ubuntuone-credentials/signon-plugin-part2
Merge into: lp:ubuntuone-credentials
Prerequisite: lp:~mardy/ubuntuone-credentials/signon-plugin-part1
Diff against target: 793 lines (+430/-121)
13 files modified
data/ubuntuone.provider (+2/-2)
debian/control (+1/-1)
libubuntuoneauth/CMakeLists.txt (+4/-0)
libubuntuoneauth/authenticator.cpp (+179/-0)
libubuntuoneauth/authenticator.h (+71/-0)
libubuntuoneauth/keyring.cpp (+52/-67)
libubuntuoneauth/keyring.h (+3/-3)
libubuntuoneauth/libubuntuoneauth.symbols (+2/-1)
libubuntuoneauth/ssoservice.cpp (+35/-13)
signon-plugin/tests/test_plugin.cpp (+58/-7)
signon-plugin/ubuntuone-plugin.cpp (+19/-26)
signon-plugin/ubuntuone-plugin.h (+0/-1)
signon-plugin/ubuntuonedata.h (+4/-0)
To merge this branch: bzr merge lp:~mardy/ubuntuone-credentials/signon-plugin-part2
Reviewer Review Type Date Requested Status
unity-api-1-bot continuous-integration Needs Fixing on 2016-06-28
dobey (community) 2016-04-28 Needs Fixing on 2016-05-26
PS Jenkins bot continuous-integration Needs Fixing on 2016-04-29
Review via email: mp+293217@code.launchpad.net

Commit Message

Authenticate via the new signon plugin

Description of the Change

Authenticate via the new signon plugin

This adds a couple of new internal classes, under the "Internal" namespace. API and ABI compatibility is preserved.

To post a comment you must log in.
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
dobey (dobey) :
lp:~mardy/ubuntuone-credentials/signon-plugin-part2 updated on 2016-04-29
248. By Alberto Mardegan on 2016-04-29

Move plugin dependency to lib, add version number

PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
dobey (dobey) wrote :

Some comments in-line, and where are the tests for the new code?

review: Needs Fixing
lp:~mardy/ubuntuone-credentials/signon-plugin-part2 updated on 2016-05-27
249. By Alberto Mardegan on 2016-05-27

Use Q_DECL_DEPRECATED

250. By Alberto Mardegan on 2016-05-27

handleSessionData is deprecated too

251. By Alberto Mardegan on 2016-05-27

include order

252. By Alberto Mardegan on 2016-05-27

Remove accountmanager singleton

253. By Alberto Mardegan on 2016-05-27

No UI for login()

Alberto Mardegan (mardy) wrote :

I added a few commits where I did all the easy/agreeable changes. Other remarks of you require more information.

I developed this branch trying hard not only not to break the API signature, but also to retain its functionality. However, in one of your comments you suggested to empty the Keyring::storeToken() method out and deprecate it, which would break any clients using it. If changes like this are allowed, then I'd need to know what are the methods which we need to keep functional, and those which could be removed. I can do some research myself, but maybe you can help me by giving some pointers on which projects I should be considering. Do I understand right, that the pay-ui repo is deprecated and that everything lives inside pay-service?

I see that the SSOService::login() method is used in pay-service to double-check the user's credentials before a payment, and by the account plugin to create the account. As I understand it, this method performs a couple of different operations:
1) Logs into the U1 account, to check that the given params are valid
2) If they are valid, ensures that the account exists (if not, creates it)
In the case of pay-service, #2 wouldn't normally be needed, but it's an essential functionality for the account plugin, so either we preserve it, or we move it to the account plugin code. Any preference?

Then, about invalidating credentials: this thing was not required in my first implementation, because the signon-plugin was always checking the token validity before returning it, so it would automatically attempt to re-validate it if it was expired. But indeed, now we have to do this in the library. The only small issue with it, is that we need an additional member in the Keyring class to store this information, and we cannot add it without breaking the ABI.

So, three options:
1) we just add it and break ABI, if you are confident that we have no client using the Keyring class directly.
2) use QObject::setProperty() and store this flag as a dynamic property;
3) The Keyring private data currently are Accounts::Manager and Accounts::Account *. We could move enable PIMPL for this class if we replace these two private members with a union:

   union {
       struct {
           Accounts::Manager m;
           Accounts::Account *a;
       } oldPrivateData;
       KeyringPrivate *d_ptr;
   };

Of course, we wouldn't use the members in oldPrivateData, they are needed just to keep the class size constant. That's also ugly, but then we don't have to worry when adding private members anymore.
Which one has your vote?

As for the unit tests, I'll add them later, once you are happy with the code (we'll have plenty of time while this thing is being tested in a silo).

Alberto Mardegan (mardy) wrote :

Ouch, I realized one important thing :-)
Now, when login() is called, we invoke the signon-plugin, which will (unless an OTP is given, but the OTP is not enabled for all U1 accounts) just return the cached token and not try to actually login with the given credentials. So, in the case of pay-service, the verification would actually be skipped, except for accounts having 2fa enabled.

Therefore I need to add some parameter to the signon plugin to tell it whether the cached token should be discarded.
And thinking of this, it occurs to me that this could also be the solution to the last issue I raised in my previous comment: the Keyring::invalidateCredentials() method could perform an authentication with this flag specified, so that the signon-plugin would delete its cached token and the next time an app requires to authenticate, it will go through the full flow. I'll create a separate MP for this, OK?

dobey (dobey) wrote :

On Fri, 2016-05-27 at 12:51 +0000, Alberto Mardegan wrote:
> I added a few commits where I did all the easy/agreeable changes.
> Other remarks of you require more information.
>
> I developed this branch trying hard not only not to break the API
> signature, but also to retain its functionality. However, in one of
> your comments you suggested to empty the Keyring::storeToken() method
> out and deprecate it, which would break any clients using it. If
> changes like this are allowed, then I'd need to know what are the
> methods which we need to keep functional, and those which could be
> removed. I can do some research myself, but maybe you can help me by
> giving some pointers on which projects I should be considering. Do I
> understand right, that the pay-ui repo is deprecated and that
> everything lives inside pay-service?

Functional here is a relative term. Keyring::storeToken shouldn't be
getting used directly by anything which links to libubuntuoneauth.
Really, the only classes which external apps/libs should be using are
SSOService and Token. Also, I hope we can do further cleanup, after
this lands, and break the API/ABI and get rid a lot of this old cruft
that's been left around. However, I wanted to avoid the work to add the
signon plug-in, and start using it, turning into a giant branch that
basically rewrites everything. So it's fine to make some things outside
the SSOService and Token classes be no-ops and deprecate them.

Yes, lp:pay-ui is dead. The pay-ui code is in pay-service now.

> I see that the SSOService::login() method is used in pay-service to
> double-check the user's credentials before a payment, and by the
> account plugin to create the account. As I understand it, this method
> performs a couple of different operations:
> 1) Logs into the U1 account, to check that the given params are valid
> 2) If they are valid, ensures that the account exists (if not,
> creates it)
> In the case of pay-service, #2 wouldn't normally be needed, but it's
> an essential functionality for the account plugin, so either we
> preserve it, or we move it to the account plugin code. Any
> preference?

The use of login() in pay-ui isn't a "double check" of the credentials.
It is a login. We require a fresh token when making purchases, so if
the user has not logged in during the past 15 minutes, we require them
to log in again to refresh the token. However, the design for the
purchase process requires that the login be part of that purchase flow,
and not a separate window that we pop up. We only pop up the external
U1 login window when there is no U1 account.

I'm not quite sure what you mean in #2 here. My understanding is that
all the actual work to do the login and store the token in the signon
db is done in the signon plug-in already. We need to continue calling
login from pay-service/pay-ui in order for the designed flow to work.

dobey (dobey) wrote :

On Fri, 2016-05-27 at 13:07 +0000, Alberto Mardegan wrote:
> Ouch, I realized one important thing :-)
> Now, when login() is called, we invoke the signon-plugin, which will
> (unless an OTP is given, but the OTP is not enabled for all U1
> accounts) just return the cached token and not try to actually login
> with the given credentials. So, in the case of pay-service, the
> verification would actually be skipped, except for accounts having
> 2fa enabled.

This must be fixed. The pay-service usage here isn't verification, it
is to refresh the token.

> Therefore I need to add some parameter to the signon plugin to tell
> it whether the cached token should be discarded.

It should always be replaced by a new token if login is successful. I
don't think we need additional parameters for that, but to just fix the
code so that it works this way.

> And thinking of this, it occurs to me that this could also be the
> solution to the last issue I raised in my previous comment: the
> Keyring::invalidateCredentials() method could perform an
> authentication with this flag specified, so that the signon-plugin
> would delete its cached token and the next time an app requires to
> authenticate, it will go through the full flow. I'll create a
> separate MP for this, OK?

I think Keyring::deleteToken() should probably be made no-op and
flagged as deprecated, and SSOService::invalidateCredentials() should
do whatever is necessary to mark the credentials as invalid.

lp:~mardy/ubuntuone-credentials/signon-plugin-part2 updated on 2016-05-30
254. By Alberto Mardegan on 2016-05-30

Merge clear-cached-token branch

[ Alberto Mardegan ]
* Be more explicit about which headers are installed. Move the symbol
  export map to LINK_FLAGS on the target.
* Complete the UbuntuOne authentication plugin
* Make Token::isValid() return false on tokens created out of empty
  strings. (LP: #1572943)

255. By Alberto Mardegan on 2016-05-30

Implement credentials invalidation

Alberto Mardegan (mardy) wrote :

On Fri, 2016-05-27 at 12:51 +0000, Alberto Mardegan wrote:
> I'm not quite sure what you mean in #2 here. My understanding is that
> all the actual work to do the login and store the token in the signon
> db is done in the signon plug-in already. We need to continue calling
> login from pay-service/pay-ui in order for the designed flow to work.

Weird, I thought I had replied, but something must have gone wrong. Trying again.

The signon-plugin does not create the account, it only cares about the credentials and other authentication data.
The account itself must be created by the account plugin; in its current incarnation, the account plugin is relying on libubuntuoneauth to create the account. This was triggered by calling SSOService::login(), and precisely in Keyring::storeToken().

So the question is: do we keep things like this, or should I investigate the possibility of moving the account creation directly in the account plugin?

unity-api-1-bot (unity-api-1-bot) wrote :
review: Needs Fixing (continuous-integration)
Natalia Bidart (nataliabidart) wrote :

Started a massive cleanup of old MPs, closing this given its age, please update and re-open if still valid.

Unmerged revisions

255. By Alberto Mardegan on 2016-05-30

Implement credentials invalidation

254. By Alberto Mardegan on 2016-05-30

Merge clear-cached-token branch

[ Alberto Mardegan ]
* Be more explicit about which headers are installed. Move the symbol
  export map to LINK_FLAGS on the target.
* Complete the UbuntuOne authentication plugin
* Make Token::isValid() return false on tokens created out of empty
  strings. (LP: #1572943)

253. By Alberto Mardegan on 2016-05-27

No UI for login()

252. By Alberto Mardegan on 2016-05-27

Remove accountmanager singleton

251. By Alberto Mardegan on 2016-05-27

include order

250. By Alberto Mardegan on 2016-05-27

handleSessionData is deprecated too

249. By Alberto Mardegan on 2016-05-27

Use Q_DECL_DEPRECATED

248. By Alberto Mardegan on 2016-04-29

Move plugin dependency to lib, add version number

247. By Alberto Mardegan on 2016-04-28

Inject times into token

246. By Alberto Mardegan on 2016-04-28

WIP -- use plugin in libubuntuoneauth

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'data/ubuntuone.provider'
2--- data/ubuntuone.provider 2016-04-19 15:04:15 +0000
3+++ data/ubuntuone.provider 2016-05-30 11:56:38 +0000
4@@ -8,8 +8,8 @@
5
6 <template>
7 <group name="auth">
8- <setting name="method">password</setting>
9- <setting name="mechanism">password</setting>
10+ <setting name="method">ubuntuone</setting>
11+ <setting name="mechanism">ubuntuone</setting>
12 </group>
13 </template>
14 </provider>
15
16=== modified file 'debian/control'
17--- debian/control 2016-04-27 14:07:35 +0000
18+++ debian/control 2016-05-30 11:56:38 +0000
19@@ -76,7 +76,7 @@
20 ${misc:Pre-Depends},
21 Depends:
22 account-plugin-tools,
23- signon-plugin-password,
24+ signon-plugin-ubuntuone (= ${source:Version}),
25 sqlite3,
26 ubuntuone-credentials-common (= ${source:Version}),
27 ${misc:Depends},
28
29=== modified file 'libubuntuoneauth/CMakeLists.txt'
30--- libubuntuoneauth/CMakeLists.txt 2016-04-21 09:25:58 +0000
31+++ libubuntuoneauth/CMakeLists.txt 2016-05-30 11:56:38 +0000
32@@ -13,6 +13,10 @@
33 SET (LIB_TYPE STATIC)
34 ENDIF (BUILD_STATIC_LIBS)
35
36+# Some slots are deprecated; disable warnings on deprecations, because
37+# moc-generated files are using these methods
38+SET (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-deprecated-declarations")
39+
40 # The sources for building the library
41 FILE (GLOB SOURCES *.cpp)
42 # HEADERS only includes the public headers for installation.
43
44=== added file 'libubuntuoneauth/authenticator.cpp'
45--- libubuntuoneauth/authenticator.cpp 1970-01-01 00:00:00 +0000
46+++ libubuntuoneauth/authenticator.cpp 2016-05-30 11:56:38 +0000
47@@ -0,0 +1,179 @@
48+/*
49+ * Copyright 2016 Canonical Ltd.
50+ *
51+ * This library is free software; you can redistribute it and/or
52+ * modify it under the terms of version 3 of the GNU Lesser General Public
53+ * License as published by the Free Software Foundation.
54+ *
55+ * This program is distributed in the hope that it will be useful,
56+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
57+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
58+ * General Public License for more details.
59+ *
60+ * You should have received a copy of the GNU Lesser General Public
61+ * License along with this library; if not, write to the
62+ * Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
63+ * Boston, MA 02110-1301, USA.
64+ */
65+
66+#include "authenticator.h"
67+#include "../signon-plugin/ubuntuonedata.h"
68+
69+#include <Accounts/Account>
70+#include <Accounts/Service>
71+#include <SignOn/AuthSession>
72+#include <SignOn/Identity>
73+#include <SignOn/IdentityInfo>
74+
75+#include <QDebug>
76+
77+using namespace Internal;
78+using namespace UbuntuOne;
79+
80+Authenticator::Authenticator(Accounts::Manager *manager, QObject *parent):
81+ QObject(parent),
82+ m_manager(manager),
83+ m_invalidate(false),
84+ m_uiAllowed(true)
85+{
86+ if (!m_manager) {
87+ m_manager = new Accounts::Manager(this);
88+ }
89+}
90+
91+void Authenticator::handleError(const SignOn::Error &e)
92+{
93+ qCritical() << "Authentication error:" << e.message();
94+ Q_EMIT error(AuthenticationError);
95+}
96+
97+void Authenticator::handleSessionData(const SignOn::SessionData &data)
98+{
99+ PluginData reply = data.data<PluginData>();
100+
101+ auto errorCode = PluginData::ErrorCode(reply.U1ErrorCode());
102+ if (errorCode != PluginData::NoError) {
103+ switch (errorCode) {
104+ case PluginData::OneTimePasswordRequired:
105+ qDebug() << "Error: OTP required";
106+ Q_EMIT error(OneTimePasswordRequired);
107+ break;
108+ case PluginData::InvalidPassword:
109+ qDebug() << "Error: invalid password";
110+ Q_EMIT error(InvalidPassword);
111+ break;
112+ default:
113+ qWarning() << "Unknown error code" << errorCode;
114+ Q_EMIT error(AuthenticationError);
115+ }
116+ return;
117+ }
118+
119+ Token token(reply.TokenKey(), reply.TokenSecret(),
120+ reply.ConsumerKey(), reply.ConsumerSecret(),
121+ reply.DateCreated(), reply.DateUpdated());
122+ if (token.isValid()) {
123+ Q_EMIT authenticated(token);
124+ } else {
125+ QString message("Failed to convert result to Token object.");
126+ qCritical() << message;
127+ Q_EMIT error(AuthenticationError);
128+ }
129+}
130+
131+quint32 Authenticator::credentialsId()
132+{
133+ QString providerId("ubuntuone");
134+ Accounts::AccountIdList accountIds = m_manager->accountList(providerId);
135+
136+ if (accountIds.isEmpty()) {
137+ qDebug() << "authenticate(): No UbuntuOne accounts found";
138+ Q_EMIT error(AccountNotFound);
139+ return 0;
140+ }
141+
142+ if (Q_UNLIKELY(accountIds.count() > 1)) {
143+ qWarning() << "authenticate(): Found '" << accountIds.count() <<
144+ "' accounts. Using first.";
145+ }
146+
147+ qDebug() << "authenticate(): Using account '" << accountIds[0] << "'.";
148+
149+ auto account = m_manager->account(accountIds[0]);
150+ if (Q_UNLIKELY(!account)) {
151+ qDebug() << "Couldn't load account";
152+ /* This could either happen because the account was deleted right while
153+ * we were loading it, or because the accounts DB was locked by another
154+ * app. Let's just return an authentication error here, so the client
155+ * can retry.
156+ */
157+ Q_EMIT error(AuthenticationError);
158+ return 0;
159+ }
160+
161+ /* Here we should check that the account service is enabled; but since the
162+ * old code was not doing this check, and that from the API there is no way
163+ * of knowing which service we are interested in, let's leave it as a TODO.
164+ */
165+
166+ return account->credentialsId();
167+}
168+
169+void Authenticator::authenticate(const QString &tokenName,
170+ const QString &userName,
171+ const QString &password,
172+ const QString &otp)
173+{
174+ SignOn::Identity *identity;
175+ if (userName.isEmpty()) {
176+ // Use existing account
177+ quint32 id = credentialsId();
178+ if (Q_UNLIKELY(!id)) return;
179+
180+ identity = SignOn::Identity::existingIdentity(id, this);
181+ if (Q_UNLIKELY(!identity)) {
182+ qCritical() << "authenticate(): unable to load credentials" << id;
183+ Q_EMIT error(AccountNotFound);
184+ return;
185+ }
186+ } else {
187+ identity = SignOn::Identity::newIdentity(SignOn::IdentityInfo(), this);
188+ }
189+
190+ auto session = identity->createSession(QStringLiteral("ubuntuone"));
191+ if (Q_UNLIKELY(!session)) {
192+ qCritical() << "Unable to create AuthSession.";
193+ Q_EMIT error(AuthenticationError);
194+ return;
195+ }
196+
197+ connect(session, SIGNAL(response(const SignOn::SessionData&)),
198+ this, SLOT(handleSessionData(const SignOn::SessionData&)));
199+ connect(session, SIGNAL(error(const SignOn::Error&)),
200+ this, SLOT(handleError(const SignOn::Error&)));
201+
202+ PluginData data;
203+ data.setTokenName(tokenName);
204+ data.setUserName(userName);
205+ data.setSecret(password);
206+ data.setOneTimePassword(otp);
207+ int uiPolicy = m_uiAllowed ?
208+ SignOn::DefaultPolicy : SignOn::NoUserInteractionPolicy;
209+ data.setUiPolicy(uiPolicy);
210+ if (m_invalidate) {
211+ data.setForceTokenRefresh(true);
212+ m_invalidate = false;
213+ }
214+
215+ session->process(data, QStringLiteral("ubuntuone"));
216+}
217+
218+void Authenticator::invalidateCredentials()
219+{
220+ m_invalidate = true;
221+}
222+
223+void Authenticator::setUiAllowed(bool allowed)
224+{
225+ m_uiAllowed = allowed;
226+}
227
228=== added file 'libubuntuoneauth/authenticator.h'
229--- libubuntuoneauth/authenticator.h 1970-01-01 00:00:00 +0000
230+++ libubuntuoneauth/authenticator.h 2016-05-30 11:56:38 +0000
231@@ -0,0 +1,71 @@
232+/*
233+ * Copyright 2016 Canonical Ltd.
234+ *
235+ * This library is free software; you can redistribute it and/or
236+ * modify it under the terms of version 3 of the GNU Lesser General Public
237+ * License as published by the Free Software Foundation.
238+ *
239+ * This program is distributed in the hope that it will be useful,
240+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
241+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
242+ * General Public License for more details.
243+ *
244+ * You should have received a copy of the GNU Lesser General Public
245+ * License along with this library; if not, write to the
246+ * Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
247+ * Boston, MA 02110-1301, USA.
248+ */
249+#ifndef _U1_AUTHENTICATOR_H_
250+#define _U1_AUTHENTICATOR_H_
251+
252+#include <Accounts/Manager>
253+#include <SignOn/Identity>
254+
255+#include <QObject>
256+
257+#include "token.h"
258+
259+namespace Internal {
260+
261+class Authenticator : public QObject
262+{
263+ Q_OBJECT
264+
265+public:
266+ enum ErrorCode {
267+ NoError = 0,
268+ AccountNotFound,
269+ OneTimePasswordRequired,
270+ InvalidPassword,
271+ AuthenticationError, // will create more specific codes if needed
272+ };
273+
274+ explicit Authenticator(Accounts::Manager *manager = 0, QObject *parent = 0);
275+
276+ void authenticate(const QString &tokenName,
277+ const QString &userName = QString(),
278+ const QString &password = QString(),
279+ const QString &otp = QString());
280+ void invalidateCredentials();
281+ void setUiAllowed(bool allowed);
282+
283+Q_SIGNALS:
284+ void authenticated(const UbuntuOne::Token& token);
285+ void error(Internal::Authenticator::ErrorCode code);
286+
287+private:
288+ quint32 credentialsId();
289+
290+private Q_SLOTS:
291+ void handleError(const SignOn::Error &error);
292+ void handleSessionData(const SignOn::SessionData &data);
293+
294+private:
295+ Accounts::Manager *m_manager;
296+ bool m_invalidate;
297+ bool m_uiAllowed;
298+};
299+
300+} /* namespace */
301+
302+#endif /* _U1_AUTHENTICATOR_H_ */
303
304=== modified file 'libubuntuoneauth/keyring.cpp'
305--- libubuntuoneauth/keyring.cpp 2016-04-19 15:04:15 +0000
306+++ libubuntuoneauth/keyring.cpp 2016-05-30 11:56:38 +0000
307@@ -23,7 +23,9 @@
308
309 #include <QDebug>
310
311+#include "authenticator.h"
312 #include "keyring.h"
313+#include "../signon-plugin/ubuntuonedata.h"
314
315 using namespace Accounts;
316 using namespace SignOn;
317@@ -46,58 +48,32 @@
318
319 void Keyring::handleSessionData(const SignOn::SessionData &data)
320 {
321- QString secret = data.Secret();
322-
323- if (secret.length() == 0) {
324- QString msg("Could not read credentials secret value.");
325- qCritical() << msg;
326- emit keyringError(msg);
327- return;
328- }
329-
330- Token *token = Token::fromQuery(secret);
331- if (token->isValid()) {
332- emit tokenFound(*token);
333- } else {
334- QString message("Failed to convert result to Token object.");
335- qCritical() << message;
336- emit keyringError(message);
337- }
338- delete token;
339+ /* UNUSED */
340+ Q_UNUSED(data);
341 }
342
343 void Keyring::findToken()
344 {
345- QString _acctName("ubuntuone");
346- AccountIdList _ids = _manager.accountList(_acctName);
347- Identity *identity;
348- Account *account;
349-
350- if (_ids.length() > 0) {
351- if (_ids.length() > 1) {
352- qDebug() << "findToken(): Found '" << _ids.length() << "' accounts. Using first.";
353- }
354- account = _manager.account(_ids[0]);
355- qDebug() << "findToken(): Using Ubuntu One account '" << _ids[0] << "'.";
356- identity = Identity::existingIdentity(account->credentialsId());
357- if (identity == NULL) {
358- qCritical() << "findToken(): disabled account " << _acctName << _ids[0];
359- emit tokenNotFound();
360- return;
361- }
362- AuthSession *session = identity->createSession(QStringLiteral("password"));
363- if (session != NULL) {
364- connect(session, SIGNAL(response(const SignOn::SessionData&)),
365- this, SLOT(handleSessionData(const SignOn::SessionData&)));
366- connect(session, SIGNAL(error(const SignOn::Error&)),
367- this, SLOT(handleError(const SignOn::Error&)));
368- session->process(SessionData(), QStringLiteral("password"));
369- return;
370- }
371- qCritical() << "Unable to create AuthSession.";
372- }
373- qDebug() << "findToken(): No accounts found matching " << _acctName;
374- emit tokenNotFound();
375+ using namespace Internal;
376+
377+ auto authenticator = new Authenticator(&_manager);
378+ authenticator->setUiAllowed(true);
379+
380+ connect(authenticator, &Authenticator::authenticated,
381+ [=](const Token &token) {
382+ Q_EMIT tokenFound(token);
383+ authenticator->deleteLater();
384+ });
385+ connect(authenticator, &Authenticator::error,
386+ [=](Authenticator::ErrorCode code) {
387+ if (code == Authenticator::AccountNotFound) {
388+ Q_EMIT tokenNotFound();
389+ } else {
390+ Q_EMIT keyringError("Authentication failed");
391+ }
392+ authenticator->deleteLater();
393+ });
394+ authenticator->authenticate(Token::buildTokenName());
395 }
396
397 void Keyring::handleCredentialsStored(const quint32 id)
398@@ -167,37 +143,46 @@
399
400 void Keyring::handleAccountRemoved()
401 {
402+ /* UNUSED */
403 emit tokenDeleted();
404 }
405
406 void Keyring::handleDeleteError(const SignOn::Error &error)
407 {
408+ /* UNUSED */
409 // Just log the error here, as we don't want to infinite loop.
410 qWarning() << "Error deleting token:" << error.message();
411 }
412
413 void Keyring::deleteToken()
414 {
415- QString _acctName("ubuntuone");
416- AccountIdList _ids = _manager.accountList(_acctName);
417- if (_ids.length() > 0) {
418- if (_ids.length() > 1) {
419- qDebug() << "deleteToken(): Found '" << _ids.length() << "' accounts. Using first.";
420+ using namespace Internal;
421+
422+ auto authenticator = new Authenticator(&_manager);
423+ authenticator->setUiAllowed(false);
424+ authenticator->invalidateCredentials();
425+
426+ connect(authenticator, &Authenticator::authenticated,
427+ [=](const Token &token) {
428+ Q_EMIT tokenDeleted();
429+ authenticator->deleteLater();
430+ });
431+ connect(authenticator, &Authenticator::error,
432+ [=](Authenticator::ErrorCode code) {
433+ /* We are expecting the authentication to return an error, given
434+ * that we are deleting the cached token and preventing the UI from
435+ * opening.
436+ * However, we need to handle the AccountNotFound error separately,
437+ * since this error means that the token was not actually deleted.
438+ */
439+ if (code == Authenticator::AccountNotFound) {
440+ Q_EMIT tokenNotFound();
441+ } else {
442+ Q_EMIT tokenDeleted();
443 }
444- Account *account = _manager.account(_ids[0]);
445- qDebug() << "deleteToken(): Using Ubuntu One account '" << _ids[0] << "'.";
446- Identity *identity = Identity::existingIdentity(account->credentialsId());
447- connect(account, SIGNAL(removed()),
448- this, SLOT(handleAccountRemoved()));
449- connect(identity, SIGNAL(error(const SignOn::Error&)),
450- this, SLOT(handleDeleteError(const SignOn::Error&)));
451-
452- identity->remove();
453- account->remove();
454- account->sync();
455- return;
456- }
457- emit tokenNotFound();
458+ authenticator->deleteLater();
459+ });
460+ authenticator->authenticate(Token::buildTokenName());
461 }
462
463 } // namespace UbuntuOne
464
465=== modified file 'libubuntuoneauth/keyring.h'
466--- libubuntuoneauth/keyring.h 2016-04-21 09:25:58 +0000
467+++ libubuntuoneauth/keyring.h 2016-05-30 11:56:38 +0000
468@@ -50,10 +50,10 @@
469
470 private Q_SLOTS:
471 void handleError(const SignOn::Error &error);
472- void handleSessionData(const SignOn::SessionData &data);
473+ Q_DECL_DEPRECATED void handleSessionData(const SignOn::SessionData &data);
474 void handleCredentialsStored(const quint32 id);
475- void handleAccountRemoved();
476- void handleDeleteError(const SignOn::Error &error);
477+ Q_DECL_DEPRECATED void handleAccountRemoved();
478+ Q_DECL_DEPRECATED void handleDeleteError(const SignOn::Error &error);
479
480 private:
481 Accounts::Manager _manager;
482
483=== modified file 'libubuntuoneauth/libubuntuoneauth.symbols'
484--- libubuntuoneauth/libubuntuoneauth.symbols 2013-07-22 15:54:02 +0000
485+++ libubuntuoneauth/libubuntuoneauth.symbols 2016-05-30 11:56:38 +0000
486@@ -1,7 +1,8 @@
487 {
488 global:
489 extern "C++" {
490- *UbuntuOne::*;
491+ UbuntuOne::*;
492+ *for?UbuntuOne::*;
493 };
494 qt_*;
495 local:
496
497=== modified file 'libubuntuoneauth/ssoservice.cpp'
498--- libubuntuoneauth/ssoservice.cpp 2016-04-19 15:04:15 +0000
499+++ libubuntuoneauth/ssoservice.cpp 2016-05-30 11:56:38 +0000
500@@ -22,6 +22,7 @@
501 #include <QNetworkRequest>
502 #include <QUrlQuery>
503
504+#include "authenticator.h"
505 #include "logging.h"
506 #include "ssoservice.h"
507 #include "requests.h"
508@@ -64,9 +65,6 @@
509 this, SLOT(accountPinged(QNetworkReply*)));
510
511 connect(&(_provider),
512- SIGNAL(OAuthTokenGranted(const OAuthTokenResponse&)),
513- this, SLOT(tokenReceived(const OAuthTokenResponse&)));
514- connect(&(_provider),
515 SIGNAL(AccountGranted(const AccountResponse&)),
516 this, SLOT(accountRegistered(const AccountResponse&)));
517 connect(&(_provider),
518@@ -116,12 +114,32 @@
519
520 void SSOService::login(QString email, QString password, QString twoFactorCode)
521 {
522- OAuthTokenRequest request(getAuthBaseUrl(),
523- email, password,
524- Token::buildTokenName(), twoFactorCode);
525- _tempEmail = email;
526-
527- _provider.GetOAuthToken(request);
528+ using namespace Internal;
529+
530+ auto authenticator = new Authenticator;
531+ /* The caller of this API is assumed to have his own UI, so no support
532+ * from SignOn UI is needed or even desired. */
533+ authenticator->setUiAllowed(false);
534+
535+ connect(authenticator, &Authenticator::authenticated,
536+ [=](const Token &token) {
537+ _keyring->storeToken(token, email);
538+ authenticator->deleteLater();
539+ });
540+ connect(authenticator, &Authenticator::error,
541+ [=](Authenticator::ErrorCode code) {
542+ if (code == Authenticator::AccountNotFound) {
543+ Q_EMIT credentialsNotFound();
544+ } else if (code == Authenticator::OneTimePasswordRequired) {
545+ Q_EMIT twoFactorAuthRequired();
546+ } else {
547+ /* TODO: deliver a proper error response. */
548+ Q_EMIT requestFailed(ErrorResponse());
549+ }
550+ authenticator->deleteLater();
551+ });
552+ authenticator->authenticate(Token::buildTokenName(),
553+ email, password, twoFactorCode);
554 }
555
556 void SSOService::handleTwoFactorAuthRequired()
557@@ -147,10 +165,14 @@
558
559 void SSOService::tokenReceived(const OAuthTokenResponse& token)
560 {
561- Token realToken = Token(token.token_key(), token.token_secret(),
562- token.consumer_key(), token.consumer_secret(),
563- token.date_created(), token.date_updated());
564- _keyring->storeToken(realToken, _tempEmail);
565+ // Not used anymore
566+
567+ /* The following two lines are needed to ensure that the
568+ * OAuthTokenRequest::~OAuthTokenRequest() symbol is exported by the
569+ * library.
570+ */
571+ OAuthTokenRequest request;
572+ qDebug() << request.serialize();
573 }
574
575 void SSOService::accountPinged(QNetworkReply*)
576
577=== modified file 'signon-plugin/tests/test_plugin.cpp'
578--- signon-plugin/tests/test_plugin.cpp 2016-04-27 08:41:03 +0000
579+++ signon-plugin/tests/test_plugin.cpp 2016-05-30 11:56:38 +0000
580@@ -28,6 +28,7 @@
581
582 #include <SignOn/uisessiondata_priv.h>
583
584+#include "token.h"
585 #include "ubuntuone-plugin.h"
586
587 using namespace SignOn;
588@@ -232,8 +233,8 @@
589
590 QTest::newRow("empty") <<
591 sessionData.toMap() <<
592- int(Error::MissingData) <<
593- false << QVariantMap() << QVariantMap();
594+ -1 <<
595+ true << QVariantMap() << QVariantMap();
596
597 sessionData.setTokenName("helloworld");
598 sessionData.setSecret("consumer_key=aAa&consumer_secret=bBb&name=helloworld&token=cCc&token_secret=dDd");
599@@ -249,10 +250,60 @@
600 sessionData.toMap() <<
601 -1 <<
602 false << response.toMap() << stored.toMap();
603- sessionData = UbuntuOne::PluginData();
604- response = UbuntuOne::PluginData();
605- stored = UbuntuOne::PluginData();
606- storedData.clear();
607+
608+ sessionData = UbuntuOne::PluginData();
609+ QString tokenName = UbuntuOne::Token::buildTokenName();
610+ sessionData.setStoredData(QVariantMap {
611+ { tokenName, QVariantMap {
612+ { "ConsumerKey", "ck" },
613+ { "ConsumerSecret", "cs" },
614+ { "TokenKey", "tk" },
615+ { "TokenSecret", "ts" },
616+ }},
617+ });
618+ response = UbuntuOne::PluginData();
619+ response.setConsumerKey("ck");
620+ response.setConsumerSecret("cs");
621+ response.setTokenKey("tk");
622+ response.setTokenSecret("ts");
623+ response.setTokenName(tokenName);
624+ stored = UbuntuOne::PluginData();
625+ storedData.clear();
626+ QTest::newRow("stored, valid") <<
627+ sessionData.toMap() <<
628+ -1 <<
629+ false << response.toMap() << stored.toMap();
630+
631+ sessionData = UbuntuOne::PluginData();
632+ sessionData.setStoredData(QVariantMap {
633+ { tokenName, QVariantMap {
634+ { "ConsumerKey", "ck" },
635+ { "ConsumerSecret", "cs" },
636+ { "TokenKey", "tk" },
637+ { "TokenSecret", "ts" },
638+ }},
639+ });
640+ sessionData.setForceTokenRefresh(true);
641+ response = UbuntuOne::PluginData();
642+ stored = UbuntuOne::PluginData();
643+ storedData.clear();
644+ stored.setStoredData(storedData);
645+ QTest::newRow("clearing token") <<
646+ sessionData.toMap() <<
647+ -1 <<
648+ true << response.toMap() << stored.toMap();
649+
650+ sessionData = UbuntuOne::PluginData();
651+ sessionData.setSecret("consumer_key=aAa&consumer_secret=bBb&name=helloworld&token=cCc&token_secret=dDd");
652+ sessionData.setForceTokenRefresh(true);
653+ response = UbuntuOne::PluginData();
654+ stored = UbuntuOne::PluginData();
655+ storedData.clear();
656+ stored.setStoredData(storedData);
657+ QTest::newRow("in secret, clearing") <<
658+ sessionData.toMap() <<
659+ -1 <<
660+ true << response.toMap() << stored.toMap();
661 }
662
663 void PluginTest::testStoredToken()
664@@ -382,10 +433,10 @@
665 response.setTokenSecret("dDd");
666 response.setDateUpdated("2013-01-11 12:43:23");
667 response.setDateCreated("2013-01-11 12:43:23");
668+ response.setTokenName(sessionData.TokenName());
669 QVariantMap storedData;
670 storedData[sessionData.TokenName()] = response.toMap();
671 stored.setStoredData(storedData);
672- response.setTokenName(sessionData.TokenName());
673 QTest::newRow("no OTP needed, 201") <<
674 sessionData.toMap() <<
675 -1 <<
676
677=== modified file 'signon-plugin/ubuntuone-plugin.cpp'
678--- signon-plugin/ubuntuone-plugin.cpp 2016-04-27 14:07:35 +0000
679+++ signon-plugin/ubuntuone-plugin.cpp 2016-05-30 11:56:38 +0000
680@@ -69,18 +69,6 @@
681 {
682 }
683
684- bool SignOnPlugin::validateInput(const PluginData &data,
685- const QString &mechanism)
686- {
687- Q_UNUSED(mechanism);
688-
689- if (data.TokenName().isEmpty()) {
690- return false;
691- }
692-
693- return true;
694- }
695-
696 bool SignOnPlugin::respondWithStoredData()
697 {
698 QVariantMap storedData = m_data.StoredData();
699@@ -119,6 +107,9 @@
700 m_data.setSecret(QString());
701 }
702 delete token;
703+ } else {
704+ /* Always use the same token name for now */
705+ m_data.setTokenName(Token::buildTokenName());
706 }
707
708 /* Check if we have stored data for this token name */
709@@ -167,18 +158,20 @@
710 PluginData response;
711 m_data = inData.data<PluginData>();
712
713- if (!validateInput(m_data, mechanism)) {
714- qWarning() << "Invalid parameters passed";
715- Q_EMIT error(SignOn::Error(SignOn::Error::MissingData));
716- return;
717- }
718-
719- /* It may be that the stored token is valid; however, do the check only
720- * if no OTP was provided (since the presence of an OTP is a clear
721- * signal that the caller wants to get a new token). */
722- if (m_data.OneTimePassword().isEmpty() &&
723- respondWithStoredData()) {
724- return;
725+ if (m_data.ForceTokenRefresh()) {
726+ qDebug() << "Clearing stored token";
727+ PluginData pluginData;
728+ pluginData.setStoredData(QVariantMap());
729+ Q_EMIT store(pluginData);
730+ m_data.setStoredData(QVariantMap());
731+ } else {
732+ /* It may be that the stored token is valid; however, do the check
733+ * only if no OTP was provided (since the presence of an OTP is a
734+ * clear signal that the caller wants to get a new token). */
735+ if (m_data.OneTimePassword().isEmpty() &&
736+ respondWithStoredData()) {
737+ return;
738+ }
739 }
740
741 getCredentialsAndCreateNewToken();
742@@ -208,6 +201,7 @@
743 token.setTokenSecret(object.value("token_secret").toString());
744 token.setDateCreated(Token::dateStringToISO(object.value("date_created").toString()));
745 token.setDateUpdated(Token::dateStringToISO(object.value("date_updated").toString()));
746+ token.setTokenName(tokenName);
747
748 /* Store the token */
749 QVariantMap storedData;
750@@ -216,7 +210,6 @@
751 pluginData.setStoredData(storedData);
752 Q_EMIT store(pluginData);
753
754- token.setTokenName(tokenName);
755 Q_EMIT result(token);
756 } else if (statusCode == 401 && error == ERR_INVALID_CREDENTIALS) {
757 m_data.setSecret(QString());
758@@ -253,7 +246,7 @@
759 QJsonObject formData;
760 formData.insert("email", m_data.UserName());
761 formData.insert("password", m_data.Secret());
762- formData.insert("token_name", m_data.TokenName());
763+ formData.insert("token_name", Token::buildTokenName());
764 if (!m_data.OneTimePassword().isEmpty()) {
765 formData.insert("otp", m_data.OneTimePassword());
766 }
767
768=== modified file 'signon-plugin/ubuntuone-plugin.h'
769--- signon-plugin/ubuntuone-plugin.h 2016-04-26 08:45:29 +0000
770+++ signon-plugin/ubuntuone-plugin.h 2016-05-30 11:56:38 +0000
771@@ -54,7 +54,6 @@
772 void userActionFinished(const SignOn::UiSessionData &data) Q_DECL_OVERRIDE;
773
774 private:
775- bool validateInput(const PluginData &data, const QString &mechanism);
776 bool respondWithStoredData();
777 void emitErrorFromReply(QNetworkReply *reply);
778 void createNewToken();
779
780=== modified file 'signon-plugin/ubuntuonedata.h'
781--- signon-plugin/ubuntuonedata.h 2016-04-27 08:36:19 +0000
782+++ signon-plugin/ubuntuonedata.h 2016-05-30 11:56:38 +0000
783@@ -46,6 +46,10 @@
784 SIGNON_SESSION_DECLARE_PROPERTY(QString, DateCreated);
785 SIGNON_SESSION_DECLARE_PROPERTY(QString, DateUpdated);
786
787+ // Set this to true if the token returned by the previous
788+ // authentication is invalid.
789+ SIGNON_SESSION_DECLARE_PROPERTY(bool, ForceTokenRefresh);
790+
791 // Error code
792 enum ErrorCode {
793 NoError = 0,

Subscribers

People subscribed via source and target branches