feat: Introduce pluggable authentication providers.

[arthurschreiber]

In preparation of a new tedious release, I was going through all changes since v2.0.1. One thing I want to cleanup before the release is the integration with sspi-client.

I'm not super happy about the current situation where sspi-client is a hard dependency of tedious, and seeing that SQL Server actually supports more authentication methods and that there is another authentication related pull request in flight (#612) that adds node-kerberos as another hard dependency, I feel that the current approach for authentication methods is not scalable.

In this pull request, I explore the addition of what I call "authentication" providers (they're called "Security Support Providers" in the MS-TDS specification). The whole pull request is still very much in flux and a work in progress, but I believe the direction proposed here to be sound.

Authentication providers are simply objects that implement a .handshake(input, callback) method that is responsible for taking in authentication handshake data coming from SQL Server, and calling the passed callback with the handshake data that should be sent back to the SQL Server.

The long-term goal is to ship tedious with only a "default" dummy provider that does not actually do anything. Other providers like for NTLM, Kerberos or the native SSPI support will be extracted into separate npm modules organization. This way, we can extract both the implementation of these providers out of tedious, and keep tedious free from dependencies that are not required for all users.

The authentication provider to use for connecting to SQL Server will be specifiable via a new option when establishing a connection, and each provider will be able to handle options that are specific to it in it's own way.

For example, when using NTLM (domain) authentication I imagine connection creation could look like this:

new Connection({
  "server": "localhost",
  "userName": "sa",
  "password": "yourStrong(!)Password",
  "authProvider": require("tedious-auth-ntlm")({
    "domain": "MYDOMAIN.COM",
    "workstation": "FOOBARBAZ"
  }),
  "options": {
    "port": 1433,
    "database": "master"
  }
});

While native SSPI could look like this:

new Connection({
  "server": "localhost",
  "userName": "sa",
  "password": "yourStrong(!)Password",
  "authProvider": require("tedious-auth-native")({
    "packageName": "negotiate"
  }),
  "options": {
    "port": 1433,
    "database": "master"
  }
});

@tvrprasad @v-suhame What do you think?

[Suraiya-Hameed]

@arthurschreiber sound like a good idea! Later we can also bring these tedious specific authentication providers dependencies under tediousjs organization. I'll start reviewing PR 👍

[tvrprasad]

@arthurschreiber Couple of quick high level comments:

1. Further abstracting out Authentication sounds like general goodness.
2. How does this get rid of dependency on sspi-client? It seems to me like you still need to install it with Tedious. May be I'm missing something.

[arthurschreiber]

@tvrprasad The idea would be to have at least 3 separate packages:

• tedious-auth-native to expose native system SSPI packages on Windows Systems.
• tedious-auth-ntlm to expose NTLM auth (regardless of operating system).
• tedious-auth-kerberos to expose Kerberos auth (regardless of operating system).

I think that'd cover all SSPI types SQL Server supports. I've seen that there is also some sort of Federated Authentication support documented in the latest TDS specifications, but I've not really looked into how it works - but it seems different enough from the SSPI authentication mechanisms that there's probably no sane way to reconcile the APIs.

tedious would not depend directly on any of these packages, so the dependencies like node-kerberos and sspi-client would not automatically be pulled in when installing tedious and using the standard TDS authentication mechanism.

[arthurschreiber]

@v-suhame Thanks! I'd love to have tests for NTLM and Kerberos auth as part of the CI builds (first inside tedious itself, later as part of the individual packages). Is there some "straightforward" way to set up a CI system for this? Maybe via Azure SQL Server and Azure AD? 🤔 I do have an Azure account and could set everything up, but some pointers would be great.

(/cc @tvrprasad - I know you might not have time, but maybe you have a good idea here.)

[Suraiya-Hameed]

Federated Authentication was the former name for Azure AD, TDS docs still seems to use the old name. We have long term plan to add Azure AD support to Tedious 😉 For CI, the only option I can think of is to set an Azure VM and test integrated auth and NTLM in there. @meet-bhagdev @tvrprasad Is there better alternative?

[meet-bhagdev]

@v-suhame that seems like the best idea right now. @Hadis-Fard any thoughts on this? How do we do this for PHP?

[Hadis-Knj]

@meet-bhagdev +1 to @v-suhame's idea. As she said Tedious doesn't support AzureAD yet and is in our future plans to support it. In PHP, we are testing the Azure AD scenarios that can be tested using sql server docker (UID and PWD authentication), and added a variable to tests for other cases as we couldn't reveal credentials on GitHub , the variable is replaced with a valid credential when we pull tests from GitHub to internal test lab before connecting to sqlDB

[tvrprasad]

I'd love to have tests for NTLM and Kerberos auth as part of the CI builds (first inside tedious itself, later as part of the individual packages).

There are tests inside Tedious for NTLM, both for sspi-client and the non-sspi-client version. I think they run as part of CI but not 100% sure.

sspi-client is applicable only for Windows and that's where the tests run. sspi-client is not just NTLM, it's NTLM, Kerberos and Negotiate protocols. But since it uses Microsoft proprietary API, it'll only work on Windows.

Kerberos auth code path that @v-suhame has in PR, I think, should only be enabled for Linux.

[Suraiya-Hameed]

--Updated-- SQL Server image in docker only supports username/password auth, so current CI doesn't test any NTLM or sspi-client related integration tests. Only feasible alternative (I can think of) is to setup Azure VM with SQL Server.

And yes, Kerberos auth code path is reachable only for non-Windows based system.

[meet-bhagdev]

@v-suhame how woulad Azure integrated auth work with the SQL Server docker image? I am not sure if that is a supported scenario. Am I missing something?

[Suraiya-Hameed]

Sorry! I meant SQL Server in Azure VM.

[Suraiya-Hameed]

@arthurschreiber I've committed changes, based on my review comments.

[tvrprasad]

@v-suhame Could you install box version of SQL Server on Azure VM that's added to an AD domain to test for AD authentication, right? Or does that also only work with Azure AD?

[tvrprasad]

tedious would not depend directly on any of these packages, so the dependencies like node-kerberos and sspi-client would not automatically be pulled in when installing tedious and using the standard TDS authentication mechanism.

So, node-Kerberos and sspi-client would be external dependencies that will need to be installed independent of Tedious, may be globally. Is that the idea? Trying to understand when and how exactly these packages will be pulled in.

[Suraiya-Hameed]

I have pushed some more changes for Windows Integrated Auth.

@tvrprasad sspi-client should be installed in client's application to use Windows Integrated Auth.

new Connection({
  "server": "localhost",
  "domain": "local",
  "authProvider": require("sspi-client"),
  "options": {
    "port": 1433,
    "database": "test"
  }
});

[Suraiya-Hameed]

Could you install box version of SQL Server on Azure VM that's added to an AD domain to test for AD authentication, right? Or does that also only work with Azure AD?

Yes, that setup should work without AAD.

[arthurschreiber]

@v-suhame @chdh I think this is mostly ready.

I stepped back from my idea to extract the NTLM and native auth providers - I think for now we should keep them internal and only extract them in the next major tedious version, once we have a better understanding on whether a common interface can be shared between sspi auth and federated auth providers somehow.

[arthurschreiber]

What's left here is:

• [ ] Extend the NTLM auth test cases to cover some missing cases.
• [ ] Extract https://github.com/tediousjs/tedious/pull/624/commits/168b845f12a283d0fdb13f5dfdc0690f29c5808a into a separate PR. This fix is unrelated to the auth provider changes, so I want it to be merged separately.

I'll try to resolve those two open points today, and as soon as I have an approval for these changes in this pull request I'll get it merged and will release a new version of tedious to npm under the next tag. I'll open another issue to track feedback for v2.1.0 and will promote it to latest after we gained a tiny bit of confidence.

How does this sound?

[chdh]

@arthurschreiber Does this mean that the next Tedious release will have a dependency on the package sspi-client and require node-gyp?

[Suraiya-Hameed]

I would prefer not having to require node-gyp, it brings in list of other dependencies, it can be challenging to get it going.

Also, should revert commit 38ed20f - I haven't looked close into the details of sppi-client implementation, but without these checks we get Single invocation of getNextBlob per instance of SspiClient may be in flight error from sspi_client.

[arthurschreiber]

@arthurschreiber Does this mean that the next Tedious release will have a dependency on the package sspi-client and require node-gyp?

I would prefer not having to require node-gyp, it brings in list of other dependencies, it can be challenging to get it going.

Thanks for the feedback! ❤️ Let's do the following then: we extract 2 new packages, tedious-auth-ntlm and tedious-auth-native. tedious-auth-ntlm will be included as a dependency of tedious (for backwards-compatibility), while tedious-auth-native will be available for everyone who wants to use tedious on windows and use native windows authentication.

In one of the next releases we will deprecate the domain configuration option, and then in the next major version, we will stop bundling tedious-auth-ntlm as part of tedious.

Also, should revert commit 38ed20f - I haven't looked close into the details of sppi-client implementation, but without these checks we get Single invocation of getNextBlob per instance of SspiClient may be in flight error from sspi_client.

Thanks for letting me know. To be honest, I've not done a lot of testing on the native auth code yet - but if we extract this into a separate module anyway, we can make sure this works properly before releasing it (independent of any tedious release).

Thanks @v-suhame and @chdh!

[Suraiya-Hameed]

@arthurschreiber If there is anything I can help with to get the npm release going, I'll be glad to help :) If you have a stub in place, I can help with tedious-auth-native and some testing.

[arthurschreiber]

Ugh, it looks like I accidentally released 2.1.0. 😞 I won't mention the addition of integrated authentication in the release notes and we will just keep quiet about it until 2.2.0 ships with the pluggable authentication support finished.

Sorry about this! 🙇

[Suraiya-Hameed]

Node.js web app in Azure fails with 2.1.0, due to sspi-client dependencies :worried:

I did a quick test by moving native.js from here into sspi-client, it is working. What do you think about publishing 2.1.1 maybe, with just this pluggable-auth in it?

[Suraiya-Hameed]

@arthurschreiber @tvrprasad It's right time to transfer sspi-client repository to tediousjs organization, what do you think?

[tvrprasad]

It's right time to transfer sspi-client repository to tediousjs organization, what do you think?

I agree. It doesn't make sense to keep making commits under my account as I won't be tracking the repository.

[Hadis-Knj]

@arthurschreiber I have finished AAD (fedauth) authentication usign username password based on current master, and able to connect. I was wondering when would you merge this PR, so I can adapt this design, or should i just send the AAD PR as is? 😕

[arthurschreiber]

@Hadis-Fard Would be great if you could send the current version of your PR. 👍 This would allow us to take a look and see whether we can integrate the different auth approaches somehow.

[Suraiya-Hameed]

@arthurschreiber Is there anything you need assistance with to wrap up this PR? 😄 Looks like it is almost ready to merge.

[David-Engel]

@arthurschreiber Is there anything we can do to help move this PR forward? We'd really like to be able to add additional authentication methods in tedious using this method. A lot of customers are migrating to Azure hosted services and supporting them is a high priority for us. 😄