-
Notifications
You must be signed in to change notification settings - Fork 286
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
"Login failed for user ***. Token is expired." when using Azure Active Directory password authentication #617
Comments
@unicrus can you please provide a brief sample of your code that we can setup your environment at our side please? We also have recently released version 2.0.0 of Microsoft.Data.SqlClient. Is it possible to upgrade to that version to see if any errors happen? |
@JRahnama, we already use the version 2.0.0 of Microsoft.Data.SqlClient. The problem is still there. We use JsonApiDotNetCore on top of EF Core, so our environment is not the easiest to replicate. I will try to create a simple app to reproduce the issue. |
Hi @unicrus Could you explain more on how you connect to your server? Do you pass "AccessToken" connection property to SqlConnection class or use "Active Directory Password" authentication keyword in Connection String when connecting with SqlConnection? |
Hi @cheenamalhotra, As I mentioned above, we use Azure Active Directory password authentication. The connection string looks like this: |
@JRahnama, @cheenamalhotra using System;
using System.Threading;
using Microsoft.Data.SqlClient;
namespace ConnectionTest
{
class Program
{
static void Main(string[] args)
{
SqlConnection connection = null;
try
{
while (true)
{
connection = new SqlConnection(@"Data Source=<srv>.database.windows.net;Authentication=Active Directory Password;Initial Catalog=<db>;UID=<user>;PWD=<pass>");
connection.Open();
using (var command = new SqlCommand(@"select count(*) from Organizations", connection))
{
using (var reader = command.ExecuteReader())
{
while (reader.Read())
{
var c = reader.GetInt32(0);
Console.WriteLine($"{DateTime.Now} {c}");
}
}
}
connection.Close();
connection.Dispose();
Thread.Sleep(60 * 1000);
}
} catch (Exception e)
{
Console.WriteLine(e.ToString());
Console.ReadKey();
}
finally
{
if (connection != null)
{
connection.Dispose();
}
}
}
}
} The exception occurred after exactly 10 hours of the app running. Microsoft.Data.SqlClient version: the latest code from this git repo Stack trace:
|
Thank you for reproducing this @unicrus 10 hours is a little unexpected duration.. Since these AD federated auth tokens expire in 1 hour and driver requests a new one when applicable. I'll investigate and let you know! |
Hi @unicrus I'm able to reproduce this issue and to mention this happens from Server side, as no token is generated from client/re-used as there's no login being done when this error occurs. The state of the connection is "Active" and processing results where the server exactly after 10 hours fails with exception:
This State 91 for Error 18456 is also not documented on Microsoft Documentation and seems like this behavior is newly introduced. We're following up with Azure teams for more details and will let you know soon. |
Hi @cheenamalhotra, Is there any update reg. the issue? |
Hi @unicrus We confirmed the issue occurs due to Azure SQL DB revalidating access token on an active physical connection after 10 hours. The driver's current behavior seems flawed and we'll have to ensure pooled connection is not re-used that contains an expired access token. PR #635 fixes this issue, feel free to give it a try! |
HI @cheenamalhotra, I've checked the pull request — it fixed the error. But after 1.5 days of running the test app, I got the following exception. I'm not sure if it's related to the change or not. Will run the app again to check.
|
I've let my test app run for ~5 days without error, so I don't think it's related to this issue. |
Hi @cheenamalhotra, is there an estimated date for the v2.1.0 release with the fix? |
@unicrus This is the current published plan: https://techcommunity.microsoft.com/t5/sql-server/released-microsoft-data-sqlclient-2-0-preview-4/ba-p/1410567 |
@ErikEJ , it cannot be the current plan — it's release notes for 2.0 preview 4, while there is 2.0 release already. |
Did you read the blog post? "Our plan is to provide GA releases twice a year with two preview releases in between. This cadence should provide time for feedback and allow us to deliver features and fixes in a timely manner." |
Hi @unicrus This fix has also been backported to v2.0 (#639) which means v2.0.1 (planned to release soon) will also contain this fix. Also v2.1.0-preview1 shall contain this fix, planned for this month. |
I am having the same issue. About a half day, I'm getting the same error. It is running on this docker container |
@CodeSwimBikeRunner, the fix is in |
Any suggested fixes or work arounds for people on EF 6? aka using System.Data.SqlClient. vs Microsoft.Data.SqlClient? |
@andycoderapptio we've had some preliminary success by extending the public class CustomExecutionStrategy : SqlAzureExecutionStrategy
{
protected override bool ShouldRetryOn(Exception ex)
{
if (ex is SqlException sqlex)
{
foreach (SqlError err in sqlex.Errors)
{
// Error number found here https://github.com/dotnet/SqlClient/issues/617#issuecomment-649686544
if (err.Number == 18456)
{
return true;
}
}
}
return base.ShouldRetryOn(ex);
}
} |
Hi @CodeSwimBikeRunner @andycoderapptio For System.Data.SqlClient, the best recommendation we can provide for this issue is, to fetch and provide access token from your application and ensuring it's always fetched and is "Active" when connection is acquired. You can do something as under as a quick solution. Azure.Identity is the recommended library to fetch tokens from, and it also provides caching capabilities if needed. Eventually you should consider moving to Microsoft.Data.SqlClient, and we are also moving to Azure.Identity soon from MSAL to acquire access tokens, also ensuring that they're always "Active" when reactivating pooled connections. var connString = new SqlConnectionStringBuilder()
{
DataSource = "{server}.database.windows.net",
InitialCatalog = "{db}"
}.ConnectionString;
var scopes = new string[] { "https://database.windows.net/.default" };
// Token Lifetime = 24 hours
var accessToken = new Azure.Identity.ManagedIdentityCredential()
.GetToken(new Azure.Core.TokenRequestContext(scopes)).Token;
using (SqlConnection connection = new SqlConnection(connString))
{
connection.AccessToken = accessToken;
connection.Open();
Console.WriteLine("Connected!");
} Or alternatively, if you still use Credential based login: var connString = new SqlConnectionStringBuilder()
{
DataSource = "{server}.database.windows.net",
InitialCatalog = "{db}"
}.ConnectionString;
var username = "{user}";
var password = "{pass}";
var tenantId = "{tenantId}";
var clientId = "{clientId}";
var scopes = new string[] { "https://database.windows.net/.default" };
// Token Lifetime = 1 hour
var accessToken = new Azure.Identity.UsernamePasswordCredential(username, password, tenantId, clientId)
.GetToken(new Azure.Core.TokenRequestContext(scopes)).Token;
using (SqlConnection connection = new SqlConnection(connString))
{
connection.AccessToken = accessToken;
connection.Open();
Console.WriteLine("Connected!");
} |
at Microsoft.Data.SqlClient.TdsParser.SendFedAuthToken(SqlFedAuthToken fedAuthToken) |
Describe the bug
We use Azure Active Directory password authentication and get the following exception 2-4 times per day:
To reproduce
Unfortunately, I cannot reproduce the issue. I just see the exception in our logs.
I will update the ticket if I manage to reproduce the bug.
Expected behavior
I don't know, how auth token expiration is handled inside Microsoft.Data.SqlClient. But I would expect that the token is refreshed prior to expiration. Or that the expiration exception is properly handled inside the library without throwing it outside.
Further technical details
Microsoft.Data.SqlClient version: 2.0.20168.4. The exception also happened when we used the version 1.10.19324.4.
.NET target: Core 3.1.5
SQL Server version: SQL Server 2014 (12.0.2000.8) on Azure.
Operating system: Docker container based on the image mcr.microsoft.com/dotnet/core/aspnet:3.1
The text was updated successfully, but these errors were encountered: