Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VSCode reconnection issues for remote connections #1776

Closed
tobespc opened this issue Jan 16, 2020 · 18 comments
Closed

VSCode reconnection issues for remote connections #1776

tobespc opened this issue Jan 16, 2020 · 18 comments

Comments

@tobespc
Copy link
Contributor

@tobespc tobespc commented Jan 16, 2020

To recreate

  • Create a remote connection (can be docker desktop) from VScode
  • Swap your network connection from IBM to IBM Intranet
  • Wait a few seconds
  • Swap network back to IBM
  • connection is now lost in VScode and we fail to reauthenticate

To fix on Mac, bring up the keychain and delete the most recent codewind refresh token, then stop and start the remote connection to force a login.

@tobespc

This comment has been minimized.

Copy link
Contributor Author

@tobespc tobespc commented Jan 16, 2020

we are investigating this from a portal pov but could have a vscode view as well

@tetchel

This comment has been minimized.

Copy link
Contributor

@tetchel tetchel commented Jan 16, 2020

Refreshing the connection doesn't fix it?

@tobespc

This comment has been minimized.

Copy link
Contributor Author

@tobespc tobespc commented Jan 17, 2020

nope, even deleting the connection and recreating it doesn't work. Might be a docker desktop issue

@markcor11

This comment has been minimized.

Copy link
Contributor

@markcor11 markcor11 commented Jan 23, 2020

It sounds like the IDE has cached a bad / expired access token and tries to use that when re-establishing a connection to the UI socket. I noticed when I manually deleted the access_token and refresh_token from the keychain that they were not re-created when toggling the connection off/on from the connections list button. (only going through the connections screen and re-saving the password did that - which then makes the connection work again). So that suggests that the IDE is not calling the cwctl to get a new set of tokens when re-starting the connection.

@tetchel

This comment has been minimized.

Copy link
Contributor

@tetchel tetchel commented Jan 23, 2020

It makes sense to refresh the access token when the connection is refreshed. I can check that out.

@micgibso micgibso added the area/docs label Jan 23, 2020
@jopit

This comment has been minimized.

Copy link

@jopit jopit commented Jan 23, 2020

Doesn't the ability to refresh the access token rely on the current access token being valid? If it does, and this scenario is causing the access token to be invalidated (as opposed to just being expired), then refreshing the token won't be enough, we'll need to re-authenticate.

I assume that underlying this is that when the gatekeeper sees the current access token being presented from an IP address that differs from the one it was issued on, it rejects/invalidates it.

@markcor11

This comment has been minimized.

Copy link
Contributor

@markcor11 markcor11 commented Jan 23, 2020

@jopit the refresh-token contains enough information for Keycloak to determine the user and works independently of any previously issued access-tokens. The refresh-token can be used instead of a username:password to request an access-token through cwctl but unlike username:passwords the refresh token has a much shorter lifespan making them more suitable for session based caching.

Refresh tokens should be sent to cwctl sectoken refresh only. This command will fetch and return a new access-token and a new refresh-token. The refresh-token should be stashed until needed, the new access-token should then be used in future API calls until such time as it expires. Once the access-token has or is about to expire, the refresh cycle should kick in with the IDE asking cwctl for a new access-token first by calling cwctl sectoken refresh and if that fails (because the refresh might also have expired) calling the cwctl sectoken get which will perform a regular username:password auth request with credentials from the keychain.

Important points here are:

access-tokens expire before refresh-tokens
refresh-tokens are only to be used to request access-tokens
Codewind API requests should include access-tokens not refresh-tokens

For Socket IO connections the behaviour is a little different.

When the IDE connects to the Codewind UISocket it must emit an authentication event with a access-token payload. The access-token must be valid (not expired) and must have originated from a Keycloak service trusted by the Codewind deployment. Codewind will check the access-token and validate it using the realm and codewind-client public key stored in Keycloak. Assuming the access-token is good, the socket is considered authenticated and streaming of socket messages can proceed until such time as the socket connection disconnects.

Important point here is : socket IO connections remain authenticated beyond the lifespan of the access-token until the connection drops.

When the socket drops (which could be hours after the original connection) there is a very high chance the access-token originally used would have now expired. The socket IO protocol does not consume a refresh token. (refreshing a token is a cwctl task) and for that reason, a new access token should be requested via cwctl and presented in the follow-up socket reconnection attempts.

The IDE still has a choice of using the cwctl sectoken refresh or cwctl sectoken get but must call cwctl to get a new access-token either way.

You might ask, what's the point of using cwctl sectoken refresh first (which might fail due to an expired refresh token) when I could just use cwctl sectoken get only ? The reason is session management. Refreshing an existing token re-uses the same session where as sectoken get will generate a whole new session in both Keycloak and in Codewind every time its called and until the single-sign-on session expires. Using fewer sessions makes them easier to revoke, avoids duplication and provides less overhead in both systems.

So in short, when socket connections drop (for whatever reason) please use cwctl to request a new access_token and use that new access-token when reconnecting.

@tobespc tobespc removed the area/portal label Jan 24, 2020
@jopit

This comment has been minimized.

Copy link

@jopit jopit commented Jan 24, 2020

@tetchel @eharris369 Please take a look at Mark's comment above: #1776 (comment)

@jopit

This comment has been minimized.

Copy link

@jopit jopit commented Jan 24, 2020

Toby was saying that even restarting the IDE doesn't fix this problem -- does that mean the IDE wasn't refreshing the access token when it started up?

@tetchel

This comment has been minimized.

Copy link
Contributor

@tetchel tetchel commented Jan 24, 2020

does that mean the IDE wasn't refreshing the access token when it started up?

It must refresh it on startup, the access token is only stored in extension RAM

@markcor11

This comment has been minimized.

Copy link
Contributor

@markcor11 markcor11 commented Jan 24, 2020

Could it be that quitting vscode doesn’t actually kill all its related processes and flush memory ? Just like if you had 2 chrome browser windows opened, closed 1 of them but leaving the other one able to retain its access to shared session storage ?

@tetchel

This comment has been minimized.

Copy link
Contributor

@tetchel tetchel commented Jan 24, 2020

Each window has its own instance of the extension, and there is definitely nothing (related to this) cached when you close all windows.

@tetchel

This comment has been minimized.

Copy link
Contributor

@tetchel tetchel commented Jan 28, 2020

#1870 related?

edit: I see that it will not impact this behaviour, but should improve session management behaviour.

@tetchel

This comment has been minimized.

Copy link
Contributor

@tetchel tetchel commented Jan 28, 2020

I will make this change too:

So in short, when socket connections drop (for whatever reason) please use cwctl to request a new access_token and use that new access-token when reconnecting.

tetchel added a commit to tetchel/codewind-vscode that referenced this issue Jan 29, 2020
- eclipse/codewind#1776
    - Send auth over socket after connect, not after enable
    - Refresh projects list on reconnect
    - Fix undefined error when getting metrics status fails

Signed-off-by: Tim Etchells <timetchells@ibm.com>
tetchel added a commit to tetchel/codewind-vscode that referenced this issue Jan 29, 2020
- eclipse/codewind#1776
    - Send auth over socket after connect, not after enable
    - Refresh projects list on reconnect
    - Fix undefined error when getting metrics status fails

Signed-off-by: Tim Etchells <timetchells@ibm.com>
@markcor11

This comment has been minimized.

Copy link
Contributor

@markcor11 markcor11 commented Jan 29, 2020

@tetchel I created #1870 because it will simplify the IDE calls to cwctl in that IDEs will only need to call sectoken get rather than first call sectoken refresh (which may fail because the supplied refresh_token has expired), then have to try sectoken get anyway as a fallback.

We'll keep both commands available in cwctl as it assists with troubleshooting tokens, but once #1870 lands IDEs should just use sectoken get for authenticating.

jopit pushed a commit to eclipse/codewind-vscode that referenced this issue Jan 29, 2020
- eclipse/codewind#1776
    - Send auth over socket after connect, not after enable
    - Refresh projects list on reconnect
    - Fix undefined error when getting metrics status fails

Signed-off-by: Tim Etchells <timetchells@ibm.com>
@tetchel

This comment has been minimized.

Copy link
Contributor

@tetchel tetchel commented Jan 29, 2020

Guys, this should be fixed now. Give it a try and verify if you don't see it anymore.

@jagraj

This comment has been minimized.

Copy link

@jagraj jagraj commented Jan 31, 2020

Our team verified this and we can not reproduce this problem any more.

@malincoln

This comment has been minimized.

Copy link

@malincoln malincoln commented Feb 3, 2020

@tobespc do you need to verify before closing?

@tobespc tobespc closed this Feb 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.