Support for AAD authentication in MSSQL connector

[mycaule]

Description

I needed to use this block to be able to login with Active Directory. This is useful if the database uses a passwordless strategy.

authentication: {
    type: "azure-active-directory-default"
  }

Screenshots

Checklist

• [x] For UI or styling changes, I have added a screenshot or gif showing before & after
• [x] I have added a changeset
• [x] I have added to the docs where applicable

[changeset-bot[bot]]

🦋 Changeset detected

Latest commit: 741fb162026ecc4acf591aff387fff2bdec48177

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages

| Name | Type | | ----------------------------- | ----- | | @evidence-dev/mssql | Minor | | @evidence-dev/db-orchestrator | Patch | | @evidence-dev/components | Patch | | @evidence-dev/evidence | Patch | | my-evidence-project | Patch | | evidence-test-environment | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Mar 10, 2024 8:42am

[netlify[bot]]

❌ Deploy Preview for evidence-development-workspace failed. Why did it fail? →

Name	Link
🔨 Latest commit	741fb162026ecc4acf591aff387fff2bdec48177
🔍 Latest deploy log	https://app.netlify.com/sites/evidence-development-workspace/deploys/66323a529d8e6c00082dfcb2

[archiewood]

Thanks for the PR.

Would this just be for development or would it work in production environments too?

[mycaule]

Thanks for the PR.

Would this just be for development or would it work in production environments too?

Yes on Azure, you can do az login locally or on your VM and then connect to your Azure SQL instance. If your VM uses a managed identity this would work also right away.

The tests are still failing for Postgres, I'll try to investigate later when I have time.

[ItsMeBrianD]

Failing Postgres tests is likely due to a recent change in how workflows run. I'll try to take a look at it later this week or early next week

[timothyhoward]

I gave this a go a while ago as I needed to get AAD authentication working with a client secret. There's a few authentication modes the tedious plugin supports, so would be good to have them all added in. Code for the temporary plugin I made in the link below if it helps: https://github.com/timothyhoward/evidence-connector-mssql

[archiewood]

Hey @timothyhoward This is great!

I've added it to the (just created) Awesome Evidence repo https://github.com/evidence-dev/awesome-evidence

[hughess]

This looks great. Only concern is if having the authentication object in the credentials will override the username/password for people who don't need the additional auth types

[timothyhoward]

I've continued expanding the MSSQL connector with username/password, client secret and access token authentication. I've placed it in a new package at https://www.npmjs.com/package/@timhoward/evidence-connector-mssql so as not to break any installs of the existing package. Again, basically learning JavaScript on the fly here so things may not be ideal :)

[mycaule]

I've continued expanding the MSSQL connector with username/password, client secret and access token authentication. I've placed it in a new package at https://www.npmjs.com/package/@timhoward/evidence-connector-mssql so as not to break any installs of the existing package. Again, basically learning JavaScript on the fly here so things may not be ideal :)

Hi @timothyhoward I tried your plugin, but here in the UI there should be an option to support the default value azure-active-directory-default (Authentication Method and Access Token should not be mandatory)

This code part in unreachable otherwise timothyhoward/evidence-connector-mssql/.../datasource/src/index.cjs#L145-L157

While manually configuring the connection.yaml file, I still get this during my attempt to login with passwordless/secretless strategy.

[!] my_connection failed to connect; Login failed for user '<token-identified principal>'.

I also personally think these features should directly be in the main @evidence-dev/mssql package to make it easier and safer for users, not relying on code hacks. Think of enterprises using Microsoft products.

[netlify[bot]]

❌ Deploy Preview for next-docs-evidence failed. Why did it fail? →

Name	Link
🔨 Latest commit	741fb162026ecc4acf591aff387fff2bdec48177
🔍 Latest deploy log	https://app.netlify.com/sites/next-docs-evidence/deploys/66323a527bad0a0008fb7243

[mycaule]

This looks great. Only concern is if having the authentication object in the credentials will override the username/password for people who don't need the additional auth types

@ItsMeBrianD @hughess

I did the UI changes following the code suggestions from Timothy above. You now have different inputs thanks to the buildConfig method.

[timothyhoward]

I've continued expanding the MSSQL connector with username/password, client secret and access token authentication. I've placed it in a new package at https://www.npmjs.com/package/@timhoward/evidence-connector-mssql so as not to break any installs of the existing package. Again, basically learning JavaScript on the fly here so things may not be ideal :)

Hi @timothyhoward I tried your plugin, but here in the UI there should be an option to support the default value azure-active-directory-default (Authentication Method and Access Token should not be mandatory)

This code part in unreachable otherwise

timothyhoward/evidence-connector-mssql/.../datasource/src/index.cjs#L145-L157

While manually configuring the connection.yaml file, I still get this during my attempt to login with passwordless/secretless strategy.

[!] my_connection failed to connect; Login failed for user '<token-identified principal>'.

I also personally think these features should directly be in the main @evidence-dev/mssql package to make it easier and safer for users, not relying on code hacks. Think of enterprises using Microsoft products.

Thanks for the feedback!

I agree it should be included directly in the '@evidence-dev/mssql' package as well. I had to improvise the plugin to get it working with our architecture, as I am working in an enterprise (government) environment that depends on Microsoft products, and trying to push Evidence as a solution.

It's currently working in an Azure DevOps workflow that fetches a token and stores it as a secret variable, but I too would like to see this integrated into the Evidence codebase as a default option. The code contributed was rushed, mostly untested, and I don't have a background in JavaScript. I'm only sharing it to assist and it's by no means a production solution.

[mycaule]

The rebase was a mess, I should have done PR against main in the first place, I made a new clean PR in https://github.com/evidence-dev/evidence/pull/1953

[timothyhoward]

I've continued expanding the MSSQL connector with username/password, client secret and access token authentication. I've placed it in a new package at https://www.npmjs.com/package/@timhoward/evidence-connector-mssql so as not to break any installs of the existing package. Again, basically learning JavaScript on the fly here so things may not be ideal :)

Hi @timothyhoward I tried your plugin, but here in the UI there should be an option to support the default value azure-active-directory-default (Authentication Method and Access Token should not be mandatory)

This code part in unreachable otherwise

timothyhoward/evidence-connector-mssql/.../datasource/src/index.cjs#L145-L157

While manually configuring the connection.yaml file, I still get this during my attempt to login with passwordless/secretless strategy.

[!] my_connection failed to connect; Login failed for user '<token-identified principal>'.

I also personally think these features should directly be in the main @evidence-dev/mssql package to make it easier and safer for users, not relying on code hacks. Think of enterprises using Microsoft products.

I forgot to mention, due to security requirements we cannot utilise the 'azure-active-directory-default' authentication flow. We're usually utilising a service connector and the username/password or secret is never shared. Rather, a token is usually requested manually through the SC and shared as a secret to the plugin (via the Access Token), hence why it's an option. If that option wasn't specially provided I'd probably need to keep using my stopgap solution.

[mycaule]

I forgot to mention, due to security requirements we cannot utilise the 'azure-active-directory-default' authentication flow. We're usually utilising a service connector and the username/password or secret is never shared. Rather, a token is usually requested manually through the SC and shared as a secret to the plugin (via the Access Token), hence why it's an option. If that option wasn't specially provided I'd probably need to keep using my stopgap solution.

The main problem I have is you duplicate the code And there are potentially risks of exploits for enterprise data during the execution of your runQuery method if someday someone modifies it without verifying. https://github.com/timothyhoward/evidence-connector-mssql/blob/main/datasource/src/index.cjs#L162-L191

[timothyhoward]

I forgot to mention, due to security requirements we cannot utilise the 'azure-active-directory-default' authentication flow. We're usually utilising a service connector and the username/password or secret is never shared. Rather, a token is usually requested manually through the SC and shared as a secret to the plugin (via the Access Token), hence why it's an option. If that option wasn't specially provided I'd probably need to keep using my stopgap solution.

The main problem I have is you duplicate the code And there are potentially risks of exploits for enterprise data during the execution of your runQuery method if someday someone modifies it without verifying. https://github.com/timothyhoward/evidence-connector-mssql/blob/main/datasource/src/index.cjs#L162-L191

@mycaule

I agree that the code duplication is not ideal - it's more of a quick and dirty code hack to get things working. I'm not comfortable with JavaScript, so I was working things out on the fly and hoping someone with experience (like yourself) could finesse it into a production piece of code that could sit in @evidence/mssql. I didn't do a pull request due to this, and instead set it aside in a package so that I could use it personally, but also use it as a reference point for this pull request.

If we could include azure-active-directory-access-token authentication (and ideally the other types) I could move away from the custom connector :)