microsoft / azuredatastudio

Azure Data Studio is a data management and development tool with connectivity to popular cloud and on-premises databases. Azure Data Studio supports Windows, macOS, and Linux, with immediate capability to connect to Azure SQL and SQL Server. Browse the extension library for more database support options including MySQL, PostgreSQL, and MongoDB.
https://learn.microsoft.com/sql/azure-data-studio
MIT License
7.5k stars 882 forks source link

Authentication not working correctly when switching databases #25074

Open gotdablues opened 7 months ago

gotdablues commented 7 months ago

Type: Bug

I have two databases -

  1. Open a new query window (one that contains the Database: dropdown in the toolbar)
  2. Using DB1, select from a table which you have access. Ensure you get back results so you can compare.
  3. Using DB2, use the same query as step 1. Notice theresults are returned. They should be the same as step 1. However, this result is incorrect. The user should have received a 'No Access' error.

When I perform the same steps in SQL Server Mgmt Studio, I get the expected results (i.e., no access message).

Azure Data Studio version: azuredatastudio 1.47.0 (c7c2b2f21505562d21972d4c135119d00806db4f, 2023-11-07T17:17:45.669Z) OS version: Windows_NT x64 10.0.22621 Restricted Mode: No Preview Features: Enabled Modes:

System Info |Item|Value| |---|---| |CPUs|12th Gen Intel(R) Core(TM) i7-1265U (12 x 2688)| |GPU Status|2d_canvas: enabled
canvas_oop_rasterization: enabled_on
direct_rendering_display_compositor: disabled_off_ok
gpu_compositing: enabled
multiple_raster_threads: enabled_on
opengl: enabled_on
rasterization: enabled
raw_draw: disabled_off_ok
video_decode: enabled
video_encode: enabled
vulkan: disabled_off
webgl: enabled
webgl2: enabled
webgpu: enabled| |Load (avg)|undefined| |Memory (System)|31.44GB (14.73GB free)| |Process Argv|| |Screen Reader|no| |VM|0%|
Extensions (1) Extension|Author (truncated)|Version ---|---|--- sql-prompt|Red|0.2.12
cheenamalhotra commented 6 months ago

I can confirm that issue is reproducible and could reproduce it even in 1.42.1. To add, if trying to query directly to a non-accessible DB, hangs query editor.

cheenamalhotra commented 6 months ago

The first part of the problem is that ChangeDatabaseRequest is dispatching 2-3 times for one dropdown event.

Consider a case where open connection is targeted to database "Northwind" by user "north_user" Another database "ASomeDB" is restricted to user "north_user", and when changing to it, we observe below behavior:

<!-- Two changedatabase requests are sent from ADS !! -->
[Trace - 5:41:31 PM] Sending request 'connection/changedatabase - (7)'.
Params: {
    "ownerUri": "untitled:SQLQuery_2",
    "newDatabase": "ASomeDB"
}

[Trace - 5:41:34 PM] Sending request 'connection/changedatabase - (8)'.
Params: {
    "ownerUri": "untitled:SQLQuery_2",
    "newDatabase": "ASomeDB"
}

<!-- True response is received for second request, while first is already in process, and conn is assumed as changed. -->
[Trace - 5:41:34 PM] Received response 'connection/changedatabase - (8)' in 6ms.
Result: true

[Trace - 5:41:34 PM] Received notification 'connection/connectionchanged'.
Params: {
    "ownerUri": "untitled:SQLQuery_2",
    "connection": {
        "serverName": "localhost\\MSSQLSERVER01",
        "databaseName": "ASomeDB",
        "userName": "north_user"
    }
}

<!-- User gets no error, so executes query on this connection -->
[Trace - 5:41:38 PM] Sending request 'query/executeDocumentSelection - (10)'.
Params: {
    "ownerUri": "untitled:SQLQuery_2",
    "executionPlanOptions": {}
}

<!-- For non-cloud connections, query execution continues to reference old connection, as connection is changed, not dropped -->

<!-- If we change code to force drop connection and recreate, query execution gets stuck as connection is no longer available. -->
<!-- This time the first change database response comes back to ADS with error -->
[Trace - 5:41:41 PM] Received response 'connection/changedatabase - (7)' in 10197ms. Request failed: Cannot open database "ASomeDB" requested by the login. The login failed.
Login failed for user 'north_user'. (0).
[Error - 5:41:41 PM] Request connection/changedatabase failed.
  Message: Cannot open database "ASomeDB" requested by the login. The login failed.
Login failed for user 'north_user'.
  Code: 0 

This seems like something we can prevent from happening by preventing multiple events from triggering, but I'm not sure why the first request response didn't even go through ADS immediately..?

Any ideas you may have @alanrenmsft @Charles-Gagnon ?

Charles-Gagnon commented 6 months ago

The connection error doesn't show up if you let it sit for a bit? I don't know offhand why it wouldn't immediately return the error as soon as it happened - did you debug into it to see why that isn't happening?

cheenamalhotra commented 6 months ago

Yeah, I did debug and the thread wouldn't return the response with exception to GUI even if I let it do its job for couple minutes. Only when running a query, we get this response.

I'm looking further to see where's the first request thread stuck and why it doesn't get to respond first.

Charles-Gagnon commented 6 months ago

I do agree that the two changedatabase commands being sent seems like a bug that should be fixed though, so maybe start there?