search

intersystems-community / intersystems-servermanager

VS Code helper extension defining connections to InterSystems servers
https://marketplace.visualstudio.com/items?itemName=intersystems-community.servermanager
MIT License
13 stars 12 forks source link

Use a cookie jar per connection to handle correctly multiple connection definitions to same server #160

Closed gjsjohnmurray closed 2 years ago

gjsjohnmurray commented 2 years ago

This PR fixes #149 and #145.

With this change multiple intersystems.servers entries that point to the same server but specify different usernames will now each connect with the correct username (provided the /api/aterlier webapp doesn't permit Unauthenticated access), and will only show namespaces which the user can access.

As a bonus it also displays the connection's username in the tooltip on the Namespaces folder.

When user logs out of a connection from the VS Code Accounts menu we will attempt to release the server-side session license.

gjsjohnmurray commented 2 years ago

@isc-bsaviano it may be advisable to adopt this technique in the Language Server and main ObjectScript extensions, otherwise if a user creates a multi-root isfs workspace that uses two different usernames to connect to two different namespaces on the same server the extensions could in theory encounter problems by supplying the cookie for userA's session when they actually need to operate in a namespace that only userB has access to.

gjsjohnmurray commented 2 years ago

Doing some more work on this, so have set it to Draft. Please hold off review for now.

gjsjohnmurray commented 2 years ago

@isc-bsaviano @isc-rsingh when you have time please review / test this. The VSIX is at https://github.com/intersystems-community/intersystems-servermanager/suites/8418802188/artifacts/372553351

isc-bsaviano commented 2 years ago

@gjsjohnmurray Neither the main extension nor language server using cookie jars so what action is required to resolve this issue there, if any? I'm pretty sure that they have used the same cookie management system since LS 2.0.0

gjsjohnmurray commented 2 years ago

@isc-bsaviano based on this:

https://github.com/intersystems/language-server/blob/2aff70258587156bf87b770249aab0543cdd72df/client/src/makeRESTRequest.ts#L22

if a multi-root workspace has two isfs folders that use different server names that actually point to the same server but specify different usernames with different permissions on the server, I think the first one to connect will get and store a cookie, then the second one will operate as that user rather than the one specified in the definition.

It's a bit of an edge case, but in fixing it in Server Manager I just wanted to make you aware of it.

isc-bsaviano commented 2 years ago

Ok, so I should add the username to the end of that key?

gjsjohnmurray commented 2 years ago

Yes, I think that will do the trick. One test case would be userA having permission on namespace A but not on B, and userB having permission on B but not A.

isc-bsaviano commented 2 years ago

@gjsjohnmurray I updated the language server code

isc-rsingh commented 2 years ago

@isc-bsaviano @isc-rsingh when you have time please review / test this. The VSIX is at https://github.com/intersystems-community/intersystems-servermanager/suites/8418802188/artifacts/372553351

@gjsjohnmurray can you share with me a .workspace file you used for testing? I'll use that to build my own similar one.

gjsjohnmurray commented 2 years ago

can you share with me a .workspace file you used for testing?

@isc-rsingh you don't really need a .code-workspace file for testing Server Manager. Here are my suggested steps:

  1. On an IRIS instance (local is fine), created databases A and B. During creation, opt to have them controlled by their own resources, %DB_A and %DB_B.
  2. Create namespaces A and B that use those databases.
  3. Edit the security resources %DB_A and %DB_B so they don't grant any public permissions.
  4. Define users testa and testb. Don't grant them %All role. Instead grant them both %Developer, grant %DB_A to testa and %DB_B to testb.
  5. In Server Manager define connections as-a and as-b, both connecting to the server used in the previous steps. Make connection as-a connect as user testa and connection as-b as user testb.
  6. Expand these entries in the Server Manager tree. Expect to be asked to allow access to credentials, and to be prompted for the passwords for testa and testb. Under the Namespaces folder of as-a you should see namespace A but not B, and vice versa for as-b.
  7. From the Accounts icon near the bottom of VS Code's Activity Bar, use the "Sign Out" option on one of the connections. If you previously stored the password it's up to you whether you Keep or Delete it now.
  8. Collapse the as-a and as-b folders.
  9. Click the Refresh button on the INTERSYSTEMS TOOLS: SERVERS view titlebar.
  10. Re-expand as-a and as-b. The one whose account you signed out from should ask for permission again, and for the password if you deleted it during sign-out or never stored it in the first place. The one you didn't sign out from should expand as before without asking anything.
  11. In Portal on the server, go to CSP Sessions under System Operation. Expect to see a session for /api/atelier corresponding to each expanded tree node.
  12. Use Sign Out from Accounts for either testa or testb.
  13. Confirm in Portal that the /api/atelier CSP sessions for the corresponding username has disappeared.
gjsjohnmurray commented 2 years ago

@isc-bsaviano @isc-rsingh any chance of getting this approved today? I'd like to get the pre-release published very soon if we're going to achieve the goal of promoting Server Manager v3 to replace v2 early next week.