platformsh / legacy-cli

This is the legacy version of Platform.sh's command-line interface. The new version is at: https://github.com/platformsh/cli
https://docs.platform.sh/administration/cli.html
MIT License
224 stars 121 forks source link

Refresh tokens are not properly used #324

Closed damz closed 8 years ago

damz commented 8 years ago

I have seen this on platformsh/cli v1.10.2 with commerceguys/guzzle-oauth2-plugin v1.0.0, but we have reports of this happening on platformsh/cli v2.4.1 with commerceguys/guzzle-oauth2-plugin v2.1.0 too.

The symptom is that after a while, the access token expires and the CLI fails to retrieve a new one:

$ platform projects

  [Guzzle\Http\Exception\ClientErrorResponseException]        
  Client error response                                       
  [status code] 401                                           
  [reason phrase] Unauthorized                                
  [url] https://marketplace.commerceguys.com/api/platform/me  

project:list [--pipe] [--refresh[="..."]]

In the state file ($HOME/.platform in my old version), I can see that the access token has been nulled. Doing a new platform login fixes the problem, until the next refresh is due.

Damien

pjcdawkins commented 8 years ago

There used to be CLI bugs and server-side bugs, fixed a while ago.

I wouldn't be surprised if there were still a bug, but it's hard to find without steps to reproduce. Here's a guess:

  1. Run platform project:list
  2. Check you have a reasonable access token saved:

    head ~/.platformsh/.session/sess-cli-default/sess-cli-default.json
  3. Invalidate the access token with something like this:

    sed -i~ 's/"accessToken":"/"accessToken":"nonsense/'  ~/.platformsh/.session/sess-cli-default/sess-cli-default.json
  4. Run platform project:list again - if there is a bug, this should fail

I wonder if there is a server-side (Accounts) issue causing conflicts.

Perhaps, for example, if you are logged in on several machine(s) and they all want to use their refresh tokens. That might explain the rarity of the reports.

pjcdawkins commented 8 years ago

Well, I still can't reproduce this, but I found and fixed a theoretical bug:

https://github.com/commerceguys/guzzle-oauth2-plugin/commit/e6eac03

(line 59 of src/Oauth2Subscriber.php is the main change)

I think the way the API client saved the refresh token in __destruct means that bug wouldn't have had any effect except in a rare case: when a command is interrupted with Ctrl+C just after performing an API call in which it retried after a 401.

pjcdawkins commented 8 years ago

I added another commit to the API client, which should make this more foolproof: https://github.com/platformsh/platformsh-client-php/commit/afa133ec