Open divega opened 6 years ago
If you already have the code necessary to obtain the AccessToken, https://github.com/aspnet/EntityFrameworkCore/issues/11928#issuecomment-387420827 describes a clever approach to set the AccessToken on each instance of the DbContext without changing how the DbContext itself gets registered in DI.
@divega Do you have a code sample that I can refer? I don't quite get the approach described in the link.
@Appjunkie sorry, I don't have an example at the moment. I need to work on one, but I am not sure when I will get to it. Perhaps the original poster of the workaround can help. cc @cbrianball.
@Appjunkie I posted some sample code
See also #15247 and consider:
I would love to see this added as a supported feature. In the meantime, I am doing this which seems to work pretty well: https://gist.github.com/ChristopherHaws/b1c54b95838f1513bfb74fa1c8e408f3 Ideally, we wouldn't even need to do this if we could specify something in the connection string and have SqlClient handle it itself.
@ChristopherHaws even with your very nice solution I'm getting an error System.Data.SqlClient.SqlException (0x80131904): Login failed for user 'NT AUTHORITY\ANONYMOUS LOGON'.
. Could you show us the used connection string? I think that might be where I made a mistake. Mine looks like
Server=tcp:correct-server.database.windows.net,1433;Initial Catalog=existingdb;Persist Security Info=False;MultipleActiveResultSets=False;Encrypt=True;TrustServerCertificate=False;
@svrooij It's likely not a connection string issue. The error message you gave is, unfortunately, a very generic one that doesn't help (as is usually the case with security-based exceptions). It likely means the user you are trying to connect with doesn't have permissions to connect to the database.
Assuming you're using an MSI (or user-assigned service identity), and you've configured permissions correctly, then it could simply be a waiting game as permissions propagate (I've had to wait hours before it would work in the past).
If you haven't already, I would recommend using a connection string (separate from the database connection string) for AzureServiceTokenProvider. The reason I recommend this is that if you don't, the API will essentially try each of it's own internal providers until it finds one that works, this added 7+ seconds to obtaining a token in the past.
Thank for the tip, I was already using that. Locally it’s set to RunAs=Developer; DeveloperTool=VisualStudio
and in Azure it’s set to RunAs=App
.
My Azure AD is used in Windows & Visual Studio and can also be used to login to in SQL management studio with the Azure AD with MFA setting, my laptop is Azure AD domain joined.
So it could be either that the token isn’t correct, but I have no way of knowing/checking. Or that the token isn’t transmitted.
Locally it fetches a token from Azure that shows my user ID, has the correct audience and includes the guids of the Azure AD groups.
What claims should be in the token to be valid for SQL?
How can I debug this? At the moment it seems to be a black box to me. Already tried overriding the logging level for Microsoft.EntityFrameworkCore
to Debug
but that doesn’t help much.
@svrooij Sounds like you are doing everything right. If you are using MSI (instead of user-assigned identity) and you want to see the actual token, you can use the following Power Shell script and execute it on the VM/App Service to output the token (you will want to update the resource query string parameter). If you are using an App Service, I use the App Service editor (there's an 'Open Console' menu item) to execute the Power Shell.
$uri = "$($env:MSI_ENDPOINT)?resource=https://graph.microsoft.com/&api-version=2017-09-01"
$ProgressPreference = "SilentlyContinue"
# Get an access token for managed identities for Azure resources
$response = Invoke-WebRequest -UseBasicParsing -Uri $uri ` -Headers @{Secret="$env:MSI_SECRET"}
$responseAsString = [System.Text.Encoding]::ASCII.GetString($response.Content)
echo $responseAsString
I'm not sure what would need to change in the script to get it to work with a user assigned identity (I've never used that feature before).
In the meantime, I am doing this which seems to work pretty well: https://gist.github.com/ChristopherHaws/b1c54b95838f1513bfb74fa1c8e408f3 quoted from: https://github.com/aspnet/EntityFrameworkCore/issues/13261#issuecomment-485025402
@ChristopherHaws Hey, Chris, can you comment on why you override CreateMasterConnection
? Is that a source of problems in some scenarios?
I prefer to modify as little boilerplate as possible in my gap-filling hacks, but maybe some BadThings (tm) will happen if I don't follow your example here. I wonder why the default SqlConnection
implementation doesn't work in Azure, now that SqlClient
has support for AccessToken
; is it not a clone operation under the covers but rather an ignorant ConnectionString
copy? I also wonder if my own application will call CreateMasterConnection
through some scenario I'm not prepared for. If so, then I think other recommended approaches (such as this one) also don't account for those same BadThings.
Would love your knowledge here.
Side note, I also found this implementation from @roysanchez, which adds IsMultipleActiveResultSetsEnabled
and SupportsAmbientTransactions
. These may be Azure-specialized values of interest to other reading this thread as well, I'm not sure.
@syndicatedshannon I need to override it because it needs to return a new instance of itself. The default implementation returns a new SqlServerConnection
and the overridden one returns an AzureSqlServerConnection
. Without overriding it, CreateMasterConnection
won't authenticate with an access token. I agree, I don't like overriding it either. I am hoping for a built in solution soon so I can remove my custom implementation of a pubternal type. :)
Related comment on the necessity of patching CreateMasterConnection
: https://github.com/MicrosoftDocs/azure-docs/issues/38814#issuecomment-531753854
side note: I get sad when I rediscover problems I've already tackled but not solved. 😢
An alternative: https://github.com/Azure-Samples/app-service-msi-entityframework-dotnet
was announced here in April 2019: https://azure.microsoft.com/en-us/blog/securing-azure-sql-databases-with-managed-identities-just-got-easier/
and here's its repo: https://github.com/Azure/azure-sdk-for-net/blob/master/sdk/mgmtcommon/AppAuthentication/Azure.Services.AppAuthentication/SqlAppAuthenticationProvider.cs
Honestly I'm not certain it is viable for .NET Core. The samples are for .NET and web.config. I'm going to look into it further.
@syndicatedshannon Thanks for pointing this out. The second link you provided would be nice, but as you suspected, it is currently only available for .NET Framework. The base class that it relies on does not exist in .NET Standard (or .NET Core); however, Microsoft has released a new API for SQL (Microsoft.Data.SqlClient), and it DOES have the base class that the second link relies on, so even if Microsoft.Azure.Services.AppAuthentication doesn't implement it for .NET Core, you could still roll your own -- I doubt it would be complicated to do so.
For now my app is a .NET Core 2 app, and I use EF Core, which relies on System.Data.SqlClient (it may be possible to switch it out, I don't know), but when I upgrade to .NET Core 3.0, (and EF Core along with it), then I will definitely look into this. Thanks for posting!
Hah! Thanks for the verification. Funny, I hadn't looked any further yesterday, but I just went back today and noticed line 1
#if net472
Nice FAQ on the library @cbrianball mentioned, in the new SqlClient OSS repo. https://github.com/dotnet/SqlClient/wiki/Frequently-Asked-Questions
The NetFx library worked like a charm after setting to build for NetCore against the latest stable Microsoft.Data.SqlClient library. Here are details:
1) Build the https://github.com/Azure/azure-sdk-for-net/blob/master/sdk/mgmtcommon/AppAuthentication/Azure.Services.AppAuthentication/ library against Microsoft.Data.SqlClient instead of System.Data.SqlClient. Note you'll have to remove or change the #if net472
preprocessor directive, and add a netstandard2.0 target framework.
2) The connection string must be edited as described above. The 'SqlAppAuthenticationParameters' class depends on three values: Authority, Resource, UserId. Authority means you must include a server domain name that supports SqlTokenAuthentication. For me, it ends with ".database.windows.net". Also include the following connection string parameters:
UID=AnyString;Authentication=Active Directory Interactive
3) In place of the web.config described above, register your custom SqlAppAuthenticationProvider class as the static provider for SqlAuthenticationMethod.ActiveDirectoryInteractive. You can do that with the following, probably clearest just before your UseSqlServer call:
SqlAuthenticationProvider.SetProvider(SqlAuthenticationMethod.ActiveDirectoryInteractive, new SqlAppAuthenticationProvider());
@syndicatedshannon I try to get a SqlAuthenticationProvider
initialized using your example line of code in ConfigureServices()
in Startup.cs
:
SqlAuthenticationProvider.SetProvider(SqlAuthenticationMethod.ActiveDirectoryInteractive, new FakeSqlAuthenticationProvider());
FakeSqlAuthenticationProvider
returns true
in IsSupported()
and just throws a NotImplementedException
in AcquireTokenAsync()
just to see if it gets used at all before I try to learn what I can use it for. However neither using new SqlConnection(connectionString)
or EF Core triggers the method AcquireTokenAsync()
. Am I missing something?
@OskarKlintrot Sorry, I don't remember the details, and I got my hands full on another thing. I'll be getting back to this issue in about a week, sooner if I'm lucky or good. Will follow up here when I do.
Short story is, I think it wasn't behaving as I expected either, and I meant to get back to it. This PR is still outstanding in our pipeline. Sorry if I led you astray.
No worries, I used @ChristopherHaws gist as inspiration to get it working instead.
Consider renewal scenarios: #19808
@ajcvickers do you have any update on the roadmap for this? Would be really great to see first class support for this to get away from workaround like the one I'm using from @ChristopherHaws
@sebader This issue is in the backlog with consider-for-next-release
. This means it is currently a stretch goal for 5.0. See release planning.
got it! Fingers crossed then. The easier it gets for people to get away from traditional connection strings with passwords etc, the better :)
@ajcvickers Any update on this issue? We'd like to use EF Core with Managed Identity Auth to SQL Server and this one is blocking us.
@syedhassaanahmed Have you seen @ChristopherHaws gist? I have been using EF Core with Managed Identity for a few months now. I used IDbContextOptionsExtension
though, so I did not follow the gist exactly. But it should be working. This issue should not be blocking you.
Thanks @OskarKlintrot I'm looking into it right now. Its a bit unclear how does it handle renewals when the token expire and existing connection lives. Did you manage to solve that part? Do you have an example?
The AzureServiceTokenProvider
renews the token 5 min before it expires and I'm using the DbContext in a WebApi so it's registered as scoped. So for me it's not a problem.
We're also using DbContext in web so its not a problem there due to the reason you mentioned above. Our concern is for long running jobs in the solution which might outlast the token expiry - 1hr. Perhaps what I don't understand is, how would a DbContext which is long-lived, can take advantage of AzureServiceTokenProvider
when it renews the token 5 minutes before the expiry? Don't we need EF Core to update the existing connection with new token?
You could use IDbCommandInterceptor
to be to renew it if needed everytime you try to access the database. The token is cached in a static field so there should not be much of a performance penelty if the cache hasent expired yet. If it has though retriving a new token once an hour during a long running job might not matter that much anyway.
See also #20969 which is a similar issue for Cosmos.
We might not need to do anything here after https://github.com/dotnet/SqlClient/pull/730
Looks nice for production ... a solution using Azure.Identity would avoid having to use a completely different approach for local development.
@sopelt I think you might have a good point there. Probably worth for you to comment on that PR as well, not just here
The MSI solution only runs on Azure based infrastructure. If arc supported MSI I would be a buyer, but you still can't do local development against an azure database using an AccessToken easily which makes me sad.
yeah dotnet/SqlClient#730 is great, but will still need to figure out a good solution for localdev.
For anyone here, this blog post shows how to write a trivial interceptor to use Azure Identity. However, that sample causes GetTokenAsync to get executed every time a context is created, which may have serious perf impacts. It may be possible to cache the token if that's a concern, of use context pooling to mitigate that.
Note to implementor: any solution we'd provide out of the box in EF Core would like make use of that library as well under the hood. This would have the consequence of taking a reference on Azure Identity from the SQL Server provider, which isn't ideal for everyone not using Azure. Some sort of plugin may mitigate that, but I'm not sure we currently have the proper extension point for that.
@roji M.D.S. already depends on https://www.nuget.org/packages/Microsoft.Identity.Client/
Yeah, in that case no worries to add that dependency at the EF Core level, thanks!
Though am not sure what the exact relation is between Microsoft.Identity.Client and Azure.Identity... Seems that the latter depends on the former, so we may need to add an additional dependency (which isn't necessarily a problem).
@roji Some additional information about Azure.Identity and SqlClient https://github.com/dotnet/SqlClient/pull/730#issuecomment-700872728 (probably whole discussion in PR is of interest)
Thanks @ErikEJ!
tl;dr fetching the access token on each and every connection (i.e. calling GetTokenAsync) is a perf anti-pattern for at least some scenarios, since tokens aren't always internally cached by Azure.Identity.
There are currently various samples for setting up an interceptor to set the access token on SqlConnection before opening it (see this great blog post, as well as this docs PR sample). I've investigated if the pattern shown by these resources is OK from a perf perspective, and got the following response from @schaabs:
While many of the credential types in Azure.Identity do in fact cache tokens, not all do, and if you're directly calling GetTokenAsync the best practice is to cache the tokens returned, especially if they are in a hot code path. We've recently updated our documentation around GetTokenAsync to provide better guidence here, adding the following.
This method is called automatically by Azure SDK client libraries. You may call this method directly, but you must also handle token caching and token refreshing.
In this case the example from the blog post is using the EnvironmentCredential and the ManagedIdentityCredential. The EnvironmentCredential will cache tokens it obtains, but the ManagedIdentityCredential will not. The ManagedIdentityCredential does make a network call, but it's a loop back call to a local endpoint so the latency wouldn't be terrible. However, if OpenAsync is called with high frequency it's possible that calls will get throttled.
So interceptors (as well as any feature developed here) should be configured with a user-provided timeout, during which they cache the access token.
Note this very useful discussion on how access tokens are handled between SqlClient and SQL Server.
@roji, I'm a little confused - I thought we'd just be able to use the new connection string formats added by SqlClient; are you planning on adding Managed Identity support in EFCore using Azure.Idendity instead (or in addition)?
@ercisampson if you just want to do stuff via the connection string, there's nothing you need from EF Core here - you can already pass an access token that way. Unless I'm missing something, this is in order to support setting the new SqlConnection.AccessToken property, programmatically.
Importantly, a full solution here needs to support rotation for the access tokens. Once again, if you want to do everything via the connection string, that means it's your responsibility as a user to change that connection string, integrating a new connection string before the current one expires. A more turn-key solution would automatically take care of getting a new access token when necessary, and assigning that to SqlConnection.AccessToken without the user needing to do anything.
Does that make sense?
Note https://github.com/dotnet/SqlClient/issues/771, which is about implementing the bulk of this inside SqlClient, making the Azure.Identity integration available to non-EF Core users as well. Assuming that's done, this issue can track a thin wrapping around that support in the EF SqlServer provider.
SqlConnection.AccessToken is added to SqlClient for .NET Core in 2.2 to enable Azure Active Directory Authentication.
Although it is already possible to use the feature with the current APIs in EF Core and our SQL Server provider, it requires creating the SqlConnection object and passing it to UseSqlServer (or somehow setting the access token just before the connection object is ever opened).
That pattern is a bit more convoluted and forces you to deviate from the common pattern in ASP.NET Core applications that uses AddDbContext with a simple connection string.
This issue is about making this more first-class, for example, by enabling passing the AccessToken in the UseSqlServer call as a separate piece of configuration from the connection string.
We should understand the common patterns to use the feature safely to decide what APIs we actually need: