Azuredatastudio: Regression: Connections to PGSql are hanging

[borgdylan]

• Azure Data Studio Version: 1.15.0-insider

Steps to Reproduce:

1. Make sure Azure Data Studio is _not_ connected to an Azure account. If it is, remove the account before proceeding.
2. Try to connect to a pgsql database
3. Notice that nothing happens. The connection attempt can be canceled and no error pops up, yet the connection us not successful.

[kburtram]

@swjain23 do you think this might be related to the PG flavor changes? I don't think we've had any other significant changes in this area?

[swjain23]

@kburtram - Taking a look at it

[kburtram]

@swjain23 thanks! FWIW this isn't repro'ing for me on Azure Postgres v10 instance.

[swjain23]

I tried to debug with latest and noticed that it is failing in connectionWidget.ts at validateInputs.
We do let validateAzureAccount = this.validateAzureAccountSelection(); and this is returning false.
I looked at the history of this file and we made a change in validateAzureAccountSelection()

From
```
if (this.authType !== AuthenticationType.AzureMFA) {
return true;
}

to 

if (this.authType !== AuthenticationType.AzureMFA && this.authType !== AuthenticationType.AzureMFAAndUser) {
return true;
}

Also the code is returning false from this piece of code:

let selected = this._azureAccountDropdown.value;
if (selected === '' || selected === this._addAzureAccountMessage) {
if (showMessage) {
this._azureAccountDropdown.showMessage({
content: localize('connectionWidget.invalidAzureAccount', "You must select an account"),
type: MessageType.ERROR
});
}
return false;

```
The value of selected is '' and that causes it to return false. It does not look like related to flavor changes.

[kburtram]

@aaomidi do you have any context on the changes @swjain23 mention in the Azure connection code path?

[aaomidi]

This is probably related to https://github.com/microsoft/azuredatastudio/pull/8434 I'll see if I can debug into this code path

[aaomidi]

@swjain23, so you're saying when you're using normal username and password (not AzureMFAAndUser) this is still failing?

[kburtram]

@rich617 any ideas? For context, the Dec release is planned for Thursday morning and will include #8434. As mentioned previously, I can't repro myself, but it would be great to understand potential impact when we're discussing release readiness tomorrow afternoon.

@aaomidi @swjain23 thanks for investigating!

[aaomidi]

I just tried connecting to a pgsql server, worked without an issue.

[swjain23]

@aaomidi - Yes it is failing with normal username and password, it doesnt show authentication type in the connection pane for me to select AzureMFAAndUser. I tested it on both insider build and the latest codebase and I am not able to connect.

[kburtram]

@swjain23 or @borgdylan could you please provide more details on the environment where connections are failing? I created a new Azure Postgres instance and am connecting using admin username and password without issue.

[aaomidi]

@swjain23 i see that now, looks like the postgres team haven't enabled the AzureMFAAndUser on their side.

Either way I had no issues connecting either, so information about the client and the server would be super useful.

[rich617]

I tried to connect using the existing postgresql extension (without AzureMFAAndUser), and it worked for me using the latest build from master. It doesn't ask for authType, and typing in username and password works fine to connect to both a standalone instance and one running in the Azure environment.

We plan to enable authType for PostgreSQL after the December release of ADS is out.

I agree that we need more information about the environment where this is failing. The repro steps don't result in any kind of hang or bug for me.

[rich617]

Let me see if I can repro without having an Azure account connected. Maybe I broke something there.

[rich617]

Anyone know how I can logout of my Azure account in ADS?

[rich617]

@swjain23 , the authenticationType should always be SqlLogin (or empty/undefined) for PostgreSQL. Are you saying that it came back as === AzureMFA or AzureMFAAndUser? If it did, then we need to figure out how it got into that state because that definitely should never happen.

[rich617]

Not related to the reported issue, but I found a couple of possible areas where I forgot to update the connectionWidget code:

private validateUsername(value: string, isOptionRequired: boolean): boolean {
    let currentAuthType = this._authTypeSelectBox ? this.getMatchingAuthType(this._authTypeSelectBox.value) : undefined;
    **if (!currentAuthType || currentAuthType === AuthenticationType.SqlLogin) {**
        if (!value && isOptionRequired) {
            return true;
        }
    }
    return false;
}

Test should also check for AzureMFAAndUser. Also:

        if (connectionInfo.authenticationType !== null && connectionInfo.authenticationType !== undefined) {
            let authTypeDisplayName = this.getAuthTypeDisplayName(connectionInfo.authenticationType);
            this._authTypeSelectBox.selectWithOptionName(authTypeDisplayName);
        }

        if (this._authTypeSelectBox) {
            this.onAuthTypeSelected(this._authTypeSelectBox.value);
        } else {
            DOM.removeClass(this._tableContainer, 'hide-username');
            DOM.removeClass(this._tableContainer, 'hide-password');
            DOM.addClass(this._tableContainer, 'hide-azure-accounts');
        }

First if block should be inside of the if block that tests for _authTypeSelectBox is defined.

Neither of these would explain the behavior that is reported here.

From my reading of the code _authTypeSelectBox would never be set if there is no authType connection option. Therefore authTypeDisplayName and authType will always be undefined for PostgreSQL (as it exists currently), and therefore validateAzureAccountSelection should always return true.

In short, no idea.

[swjain23]

@rich617 - I did not login to any azure account in ADS. I just installed the latest insider build, got the PG extension and clicked on the new connection and tried to connect to an Azure PG instance. Yes, when I was debugging it the auth type returned was AzureMFA and not SQLLogin.

Also, I don't know if it makes a difference but I completely removed everything from previous insiders build (deleted the folder %userprofile%.azuredatastudio-insiders) and then installed the latest version.

[swjain23]

@rich617 - So I looked further and noticed this:

private get authType(): AuthenticationType {
let authDisplayName: string = this.getAuthTypeDisplayName(this.authenticationType);
return this.getMatchingAuthType(authDisplayName);
}

In this method, authDisplayName is undefined but then when we call method getMatchingAuthType, it returns AzureMFA as:

private getMatchingAuthType(displayName: string): AuthenticationType {
return find(ConnectionWidget._authTypes, authType => this.getAuthTypeDisplayName(authType) === displayName);
}

The first element of ConnectionWidget._authTypes is AzureMFA and when we call getAuthDisplayName(AzureMFA), it returns undefined because this._optionsMaps[ConnectionOptionSpecialType.authType] is not set and finally we get authType as AzureMFA as it matches our undefined displayName.

[borgdylan]

@kburtram I am trying to connect to an on-premise pgsql server. Connections from c# code are working.

[rich617]

@swjain23 , that is a bug, and probably what is causing this. If displayName is undefined, then get MatchingAuthType must return undefined—not AzureMFA. We need to fix this before the December release goes out. Do you want me to create a PR for it? Since you can repro, it might be easier for you to do it. Let me know.

[rich617]

@borgdylan @kburtram , looks like @swjain23 found the issue. If should be a quick fix in getMatchingAuthType, something like:

if (!displayName)
     return undefined;

at the top of the function.

[swjain23]

@rich617 I can do the change and send a PR. But is any of you able to repro it? I am just wondering why it is working for some of us.

[kburtram]

@swjain23 @rich617 we're currently planning to release tomorrow morning at 11am. Please send a PR asap so we can merge and get a new build out. People are starting to take holiday leave so we don't have a lot of room to slip the schedule this week.

[rich617]

@swjain23 , seems like if you have never logged in to an Azure account it fails. If you are logged in to an Azure account, it works.

If there is some way to disconnect ADS from the Azure account, we could probably repro, but I can’t find any option to do that, and I don’t know where this state is stored in the filesystem.

[kburtram]

@rich617 you should be able to delete the Azure accounts by clicking on the little person icon in the bottom left of the status bar then using the Account dialog. You can also run the Clear Azure Account Token Cache command. Let me try to see if I can repro.

[swjain23]

@kburtram - Yes just testing it out, will send a PR in 10 mins.

[swjain23]

@rich617 - After applying the fix that you suggested, I am able to connect to a PostgreSQL server.

@kburtram - FYI I am getting an error when ADS is loading extensions "Book initialize failed: Failed to recognize the structure." Source: Notebook Core Extensions. But I am getting this even without the fix. So not related to this change but I was not getting it yesterday.

[rich617]

@kburtram @swjain23 , I followed the instructions to remove the Azure account from ADS, and was able to verify the behavior described. Attempting to connect to PostgreSQL hangs. You can do this to repro / validate the fix. I updated the repro instructions at the top.

[kburtram]

I was able to repro after removing my Azure account. And I was able to verify this is fixed in the Dec release candidate build. Here are fwlinks for tomorrow's release in case anyone else wants to verify the fix.

Platform | Link
-- | --
Windows User EXE | https://go.microsoft.com/fwlink/?linkid=2113530
Windows System EXE | https://go.microsoft.com/fwlink/?linkid=2113628
Windows ZIP | https://go.microsoft.com/fwlink/?linkid=2113529
MacOS Zip | https://go.microsoft.com/fwlink/?linkid=2113528
Linux tar.gz | https://go.microsoft.com/fwlink/?linkid=2113627
Linux rpm | https://go.microsoft.com/fwlink/?linkid=2113718
Linux deb | https://go.microsoft.com/fwlink/?linkid=2113344