Merge lp://qastaging/~mardy/ubuntuone-credentials/clear-stored-token into lp://qastaging/ubuntuone-credentials

Replied to inline comments.

On Wed, 2016-06-01 at 08:23 +0000, Alberto Mardegan wrote:
> If the client doesn't want any UI to pop up, it can prevent it by
> setting the UiPolicy flag on the request (the Authenticator class in
> libubuntuoneauth exposes a flag for this).

I made the statement that we don't want to enable UI in various places
on the other MP, and you were arguing that we do need to enable it
there. However, I don't think we ever want online-accounts to be
deciding when it should do that, which is what I mean here.

> I don't understand what you meant with the background process
> example, but maybe it helps to know that signond takes care of
> ensuring that no more than one authentication is going on for the
> same account/method combination at a time. So, between process() and
> the emission of result(), no one else can modify our data.

I mean for example, ubuntu-push. If the token appears to be invalid for
some reason, and the user is for example browsing a random web site, or
is in the middle of a call, we don't want ubuntu-push invalidating that
token to result in a dialog popping up in their face, asking to enter
their U1 credentials again.

> These are the use-cases I wanted to cover:
> 1) Client notices that the token it has received from signond is not
> working, and needs a working one

This needs to be handled differently for different clients. I don't
think we can handle this differently than we currently do, in the scope
for example, due to the way that scopes work. However, implementing
this in the manner done currently in this MP would result in a flow
that would break, due to the stateless nature of scopes.

> 2) Pay-ui: needs a new token

This is a special case that needs to be handled slightly differently
than the other cases for invalidating a token.

> 3) (not sure whether we actually have this case) Client notices that
> the token it has received from signond is not working, but doesn't
> actually need a token right now.

This is I think the behavior we want, whenever we want to invalidate a
token, at least, for now it is, to avoid introducing additional
breakage in the flow.

> And how I'd implement them in the client:
> 1) just add the ForceTokenRefresh to the session data and repeat the
> authentication process
> 2) add the ForceTokenRefresh to the session data and start the
> authentication process
> 3) add the ForceTokenRefresh and the UiPolicy=NoUserInteraction to
> the session data and start the authentication process
>
> This allows to cover all three cases with a single call to process().
> If you are merely suggesting a name change, then I have no big
> objections (except that this is the name we are using in the OAuth
> plugin, and I was hoping we could have standardized on that name for
> all plugins); but if you are also implying that the behaviour should
> change, please be more explicit on how you'd like it to work.

Yes, I think U1 is special here, and the behavior should be different.
As I previously said, we want to just invalidate the token, not
immediately request a new one, for the cases where we need to
invalidate the token. Because of how the credentials are used, and how
those various pieces of the system work...

> Yes, I think U1 is special here, and the behavior should be different.
> As I previously said, we want to just invalidate the token, not
> immediately request a new one, for the cases where we need to
> invalidate the token. Because of how the credentials are used, and how
> those various pieces of the system work, I think we should avoid any
> drastic changes to the existing flow, which would add more confusion
> and possible points of failure.

I think I understand your points, and what I propose goes exactly in that direction. Just, I'm working step by step, first making sure that this plugin can accommodate all the use cases we need, and then I'll modify libubuntuoneauth accordingly. Therefore, while you review this branch please forget about the changes I made in the "signon-plugin-part2" branch: it will obviously need to be updated.

Now, this change I'm proposing here covers all the use cases that we have; it should work both with clients using libubuntuoneauth (which we'll carefully modify to retain most of its old behaviour) and clients directly using this plugin via the OA apis. I think you are confirming this yourself above, when you say that this is case #3 I previously mentioned, but if that's not the case, please correct me.

As for the scopes, I'm well aware of the issues there, because we met the same with the scopes using OAuth authentication; nowadays they should all be fixed, and the OAuth window never pops up unexpectedly, but only in response of the user pressing the "Login" button on the scope. It will be the same for UbuntuOne, don't worry. :-)

FAILED: Continuous integration, rev:248
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntuone-credentials-ci/2/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/85/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/92/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntuone-credentials-ci/2/rebuild

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