Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature | Introduce "Active Directory Default" authentication mode #1043

Merged
merged 3 commits into from May 17, 2021

Conversation

cheenamalhotra
Copy link
Member

@cheenamalhotra cheenamalhotra commented Apr 20, 2021

This PR introduces a new SQL Authentication method "Active Directory Default". With this mode of authentication, the driver widens possibilities of user authentication, extending login solutions to Client environment, Visual Studio Code, Visual Studio, Azure CLI etc. It enhances developer experience for applications that will be deployed to Azure.

With this authentication mode, the driver uses "DefaultAzureCredential" from Azure Identity library to acquire access token. The driver enables below mentioned credential types, to be attempted to acquire access token, in below order:

  • EnvironmentCredential
    • Enables authentication to Azure Active Directory using client and secret, or username and password, details configured in the following environment variables: AZURE_TENANT_ID, AZURE_CLIENT_ID, AZURE_CLIENT_SECRET, AZURE_CLIENT_CERTIFICATE_PATH, AZURE_USERNAME, AZURE_PASSWORD (More details)
  • ManagedIdentityCredential
    • Attempts authentication to Azure Active Directory using a managed identity that has been assigned to the deployment environment. "Client Id" of "User Assigned Managed Identity" continues to be read from "User Id" connection property.
  • SharedTokenCacheCredential
    • Authenticates using tokens in the local cache shared between Microsoft applications.
  • VisualStudioCredential
    • Enables authentication to Azure Active Directory using data from Visual Studio
  • VisualStudioCodeCredential
    • Enables authentication to Azure Active Directory using data from Visual Studio Code.
  • AzureCliCredential
    • Enables authentication to Azure Active Directory using Azure CLI to obtain an access token.

Note that InteractiveBrowserCredential is disabled in the driver implementation of "Active Directory Default", and "Active Directory Interactive" continues to be the only method to acquire token using MFA/Interactive authentication.

Additional changes:

  • Fixed unavailable "Microsoft.Dotnet.RemoteExecutor" package errors while building tests.

@cheenamalhotra cheenamalhotra added this to In Progress in SqlClient v3.0 via automation Apr 20, 2021
@cheenamalhotra cheenamalhotra added this to the 3.0.0-preview3 milestone Apr 20, 2021
@cheenamalhotra cheenamalhotra added the 🆕 Public API Use this label for new API additions to the driver. label Apr 20, 2021
@Wraith2
Copy link
Contributor

Wraith2 commented Apr 20, 2021

That's a lot of boilerplate and repetition. It's be nice if it wasn't quite as complicated to plug in new authentication schemes.

@cheenamalhotra
Copy link
Member Author

I will be working on merge for AAD stuff in another PR

@ErikEJ
Copy link
Contributor

ErikEJ commented Apr 21, 2021

What is "Active Directory Default" - is that the one that works with Visual Studio etc?

@cheenamalhotra
Copy link
Member Author

cheenamalhotra commented Apr 21, 2021

Yes. We acquire credential using DefaultAzureCredential from Azure Identity. This API uses all non-interactive token credential implementations sequentially that provide password-less solution for best security.

It can also be seen as an extension to Managed Identity based authentication to allow developers to continue using same connection string in both dev and deploy environments. We named it Active Directory Default following the DefaultAzureCredential nomenclature.

I'll update PR description tomorrow and we will definitely be documenting officially once it's released in GA 🙂

@mderriey
Copy link

mderriey commented May 7, 2021

That's amazing!

I've been building manual token acquisition and caching solutions in my apps to leverage AAD auth with managed identity, so it's fantastic to see it supported natively by the SQL driver.

My only "grip" with this PR is that it doesn't provide the capability to select which sources to use: Visual Studio, VS Code, or the Azure CLI.

I don't know in what order Azure.Identity tries them, but I could be logged in to the wrong account in the first source, and Azure.identity would pick this one.

I understand it might be super hard / impossible to expose a way to "pick" sources through the connection string, but has it been considered? Something similar to ChainedTokenCredential which allows the consumer of Azure.Identity to explicitly pick the sources and the order they're tried?

@cheenamalhotra
Copy link
Member Author

cheenamalhotra commented May 7, 2021

My only "grip" with this PR is that it doesn't provide the capability to select which sources to use: Visual Studio, VS Code, or the Azure CLI.

Yes, as of this feature, this is only going to allow default settings of "DefaultAzureCredential", if you need more specialized authentication support, you may acquire token yourself with all the customization options this class already gives and pass the token directly to "SqlConnection.AccessToken" property.

@mderriey
Copy link

mderriey commented May 9, 2021

OK, thanks.

as of this feature

Does that mean that in the future more configuration options might be exposed through the SQL driver?

@ErikEJ
Copy link
Contributor

ErikEJ commented May 10, 2021

@mderriey seems out of scope for SQL client, since you can always obtain access token yourself.

@mderriey
Copy link

[...] since you can always obtain access token yourself.

One could argue that's also the case for the managed identity case, and the development time case that this PR addresses.

The massive benefit of having it taken care of by the SQL client is that it also performs token caching, which Azure Identity doesn't do.

All good, though, it's still going to make it so much easier for so many peeps, thanks for the great work.

@cheenamalhotra
Copy link
Member Author

Hi @mderriey

One could argue that's also the case for the managed identity case, and the development time case that this PR addresses.

This was chosen to be implemented as many customers migrating from System.Data.SqlClient were using App Authentication library which already handled multiple credentials, and migration lead to many issues (attached to PR) as there was no alternative to same experience. To universalize the authentication experience using Azure.Identity and multi-credential support using Active Directory Default our objective is to ease out migration and provide a developer friendly passwordless solution.

Co-authored-by: Johnny Pham <v-jopha@microsoft.com>
@cheenamalhotra cheenamalhotra moved this from In Progress to Review In Progress in SqlClient v3.0 May 15, 2021
SqlClient v3.0 automation moved this from Review In Progress to Reviewer Approved May 17, 2021
@cheenamalhotra cheenamalhotra merged commit 4316474 into dotnet:main May 17, 2021
SqlClient v3.0 automation moved this from Reviewer Approved to Done May 17, 2021
@ericsampson
Copy link

@cheenamalhotra thank you for this!

Would a PR be accepted to allow the user to control which sources to use? One reason is that we (and other users) have to disable the VS/VSCode token source because of a bug in them when using an Azure tenant as an AAD Guest. Using Default as this is currently hardcoded won't work for us. Cheers!!!

@ericsampson
Copy link

Here's some background (there are a number of issues created that have the same root cause):
Azure/azure-sdk-for-net#19511

@cheenamalhotra
Copy link
Member Author

cheenamalhotra commented May 18, 2021

@ericsampson

Please feel free to open a new feature request in this repo with all this information to request this support. We will prioritize based on community interest. 👍

@ericsampson
Copy link

ericsampson commented May 19, 2021

@cheenamalhotra it looks like the issue that we (and others) are experiencing is planned on being fixed in Azure.Identity in Q3.
Should I create some sort of 'future TODO' feature request to pull this version into SqlClient when it's released?

Then the original question about allowing the ability to customize sources isn't needed.

@cheenamalhotra
Copy link
Member Author

Sure, sounds good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🆕 Public API Use this label for new API additions to the driver.
Projects
No open projects
7 participants