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

FAILED: Continuous integration, rev:237
http://jenkins.qa.ubuntu.com/job/ubuntuone-credentials-ci/144/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntuone-credentials-wily-amd64-ci/21/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntuone-credentials-wily-armhf-ci/21/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntuone-credentials-wily-i386-ci/21/console

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

Replied to inline comments.

On Wed, 2016-04-20 at 08:14 +0000, Alberto Mardegan wrote:
> Because Token::isValid() is currently broken; either we add a check
> there to see that every variable is non empty, or we make sure we
> don't add empty variables into the map when we construct the token.

Please file a bug about this issue, and revert this change from this
branch. You don't fix all cases here, and I think this is probably the
wrong fix anyway. It's an issue separate from the signon-plugin, so
let's treat it as such and deal with it more appropriately.

> > +            if (pair.count() < 2) continue;
> Yes, it helps the migration from using the current password plugin to
> the new one: the new one won't store the encoded token in the
> password field, which will be used for the password instead.
> However, to ease the migration, the new plugin first checks is the
> password fields contains an encoded token: to do that, it tries to
> load the password into a Token with Token::fromQuery, and then checks
> whether the token is valid. If the password field does contain a
> password and not a token, this would break. (see lines 974-1000 of
> this diff)

So the plug-in can read the data from a different plug-in type to
convert it? If the line is required, please make it a 3 line addition
with open/close curly braces.

> Answering in random order:
> No, this won't affect the current account plugin: with the code I'm
> going to propose, the account plugin disables all signon UI
> interactions, which means that this code path will not be taken and
> that the account plugin will receive error codes instead, so that it
> will be able to adapt its UI and query the user itself (exactly like
> it's currently doing). So, no additional dialogs there.
>
> This code is mainly for the case of the Click scope, as well as PayUI
> (unless you can handle UI requests there, in which case we could do a
> similar thing as we are doing for the account plugin, of disabling
> signon UI in there too) and any third party app using the UbuntuOne
> account (a launchpad client, for example).

Can you clarify how this would work exactly for the click scope? A flow
chart of screen grabs or such perhaps? I think design wants us to
always show branded UI for cases where we require the user to log in
again.

> What happens in that case is that the client will receive the U1
> token data, if available, and no UI interactions will be required.
> If, however, the token data is not available (because the existing
> token has been invalidated via the website, or because the client is
> using a different TokenName and therefore a new token needs to be
> created instead), there might be the need to show the login dialog to
> the user. In that case, this authentication plugin asks signond to
> popup a UI to request the needed info.

How does online-accounts know the token has been invalidated via the
web site or such? We would need to change the clients to call some
other methods than they are currently calling, for these cases?

> The parameters you see being passed here into the "data" dictionary
> describe what are the needed fields, and it's possible to override
> the dialog title and each field's descr...

On 20/04/2016 23:15, Rodney Dawes wrote:
> Please file a bug about this issue, and revert this change from this
> branch. You don't fix all cases here, and I think this is probably the
> wrong fix anyway. It's an issue separate from the signon-plugin, so
> let's treat it as such and deal with it more appropriately.

https://bugs.launchpad.net/bugs/1572943

>>> + if (pair.count() < 2) continue;
[...]
> So the plug-in can read the data from a different plug-in type to
> convert it? If the line is required, please make it a 3 line addition
> with open/close curly braces.

OK.

[...]
> Can you clarify how this would work exactly for the click scope? A flow
> chart of screen grabs or such perhaps? I think design wants us to
> always show branded UI for cases where we require the user to log in
> again.

I can do that of course, but IMHO the best thing is whether you would
test the silo and see for yourself.
There is not much branded in the current authentication UI (apart from
the page title), but in principle this is something which can be
improved easily.

> How does online-accounts know the token has been invalidated via the
> web site or such? We would need to change the clients to call some
> other methods than they are currently calling, for these cases?

The authentication plugins checks the validity of a token before
returning it to the client: see the
SignOnPlugin::respondWithStoredData() method: near the end it calls
SignOnPlugin::checkTokenValidity() and if the token is invalid it won't
return it; instead, it will try to get a new one.

> Does this UI request open the Main.qml plug-in we are providing? Or it
> only opens generic UI provided by online-accounts itself?

The latter. It's a generic UI, configurable with a few parameters.

FAILED: Continuous integration, rev:238
http://jenkins.qa.ubuntu.com/job/ubuntuone-credentials-ci/146/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntuone-credentials-wily-amd64-ci/23/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntuone-credentials-wily-armhf-ci/23/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntuone-credentials-wily-i386-ci/23/console

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

To clarify: I'm talking about silo 79 (https://requests.ci-train.ubuntu.com/#/ticket/1049). No matter what the train says, the packages in the silo PPA appear to be working as expected, and testing is quite easy: create an U1 plugin, then go to the website to revoke the token and finally try to install an app from the click scope.
You will get a dialog like this beauty: http://www.mardy.it/archivos/ubuntuone.png
If you enter the correct credentials, the package installation will start; otherwise, the dialog will reappear. And indeed, if you type the right credentials but you have 2fa enabled, the dialog will reappear and will have an additional field for the 2fa.

Before you say it: I'm aware of the uglyness of the dialog, and also that it would be a much better experience if the dialog didn't close at every attempt, but instead remain on screen with a spinner until the authentication has completed.
Both these things are solvable; the first with some UI designers input, the latter with some non trivial changes to ussoa, which I didn't start doing given that U1 was the only account requiring these changes, and I was not sure whether the U1 authentication plugin was ever going to land.
However, now that it appears that we are going to have an owncloud plugin, I might get started on them anyway.

FAILED: Continuous integration, rev:239
http://jenkins.qa.ubuntu.com/job/ubuntuone-credentials-ci/147/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntuone-credentials-wily-amd64-ci/24/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntuone-credentials-wily-armhf-ci/24/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntuone-credentials-wily-i386-ci/24/console

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

OK, I don't know what you did, but trying to merge your library-symbols branch on top of this one, or vice versa, results in a bunch of conflicts.

Also, it looks like you forgot to bzr mv tst_plugin.cpp to test_plugin.cpp.

FAILED: Continuous integration, rev:240
http://jenkins.qa.ubuntu.com/job/ubuntuone-credentials-ci/148/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntuone-credentials-wily-amd64-ci/25/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntuone-credentials-wily-armhf-ci/25/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntuone-credentials-wily-i386-ci/25/console

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