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.

Sign up for GitHub

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

Fix Async thread blocking on SqlConnection open for AAD modes #1213

Merged
merged 7 commits into from
Aug 23, 2021
Merged

Fix Async thread blocking on SqlConnection open for AAD modes #1213

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
48daafe
Fix Async thread blocking on SqlConnection open for AAD modes
cheenamalhotra Aug 16, 2021
9adeb8c
back to result
cheenamalhotra Aug 17, 2021
cf15d35
Fix AAD Tests
cheenamalhotra Aug 17, 2021
4d69e29
More changes
cheenamalhotra Aug 17, 2021
de5c963
Fix for Unix
cheenamalhotra Aug 17, 2021
75471d4
Wrap internal exceptions into SqlException
cheenamalhotra Aug 18, 2021
f25e4f7
Add comments
cheenamalhotra Aug 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Wrap internal exceptions into SqlException
  • Loading branch information
@cheenamalhotra
cheenamalhotra committed Aug 18, 2021
commit 75471d4ae04b93423f3f72f659c840035bc89e67
8 changes: 7 additions & 1 deletion ...Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2444,8 +2444,9 @@ internal SqlFedAuthToken GetFedAuthToken(SqlFedAuthInfo fedAuthInfo)
|| _timeout.MillisecondsRemaining <= sleepInterval)
{
SqlClientEventSource.Log.TryTraceEvent("<sc.SqlInternalConnectionTds.GetFedAuthToken.MSALException error:> {0}", msalException.ErrorCode);

// Error[0]
SqlErrorCollection sqlErs = new SqlErrorCollection();
SqlErrorCollection sqlErs = new();
sqlErs.Add(new SqlError(0, (byte)0x00, (byte)TdsEnums.MIN_ERROR_CLASS, ConnectionOptions.DataSource, StringsHelper.GetString(Strings.SQL_MSALFailure, username, ConnectionOptions.Authentication.ToString("G")), ActiveDirectoryAuthentication.MSALGetAccessTokenFunctionName, 0));

// Error[1]
Expand All @@ -2467,6 +2468,11 @@ internal SqlFedAuthToken GetFedAuthToken(SqlFedAuthInfo fedAuthInfo)
Thread.Sleep(sleepInterval);
sleepInterval *= 2;
}
// All other exceptions from MSAL/Azure Identity APIs
catch (Exception e)
{
throw SqlException.CreateException(new() { new(0, (byte)0x00, (byte)TdsEnums.FATAL_ERROR_CLASS, ConnectionOptions.DataSource, e.Message, ActiveDirectoryAuthentication.MSALGetAccessTokenFunctionName, 0) }, "", this, e);
}
}

Debug.Assert(_fedAuthToken != null, "fedAuthToken should not be null.");
Expand Down
5 changes: 5 additions & 0 deletions src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2906,6 +2906,11 @@ internal SqlFedAuthToken GetFedAuthToken(SqlFedAuthInfo fedAuthInfo)
Thread.Sleep(sleepInterval);
sleepInterval *= 2;
}
// All other exceptions from MSAL/Azure Identity APIs
catch (Exception e)
{
throw SqlException.CreateException(new() { new(0, (byte)0x00, (byte)TdsEnums.FATAL_ERROR_CLASS, ConnectionOptions.DataSource, e.Message, ActiveDirectoryAuthentication.MSALGetAccessTokenFunctionName, 0) }, "", this, e);
}
}

Debug.Assert(fedAuthToken != null, "fedAuthToken should not be null.");
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/AADConnectionTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ public static void EmptyCredInConnStrAADPasswordAnyUnix()
// connection fails with expected error message.
string[] removeKeys = { "User ID", "Password", "UID", "PWD" };
string connStr = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, removeKeys) + "User ID=; Password=;";
PlatformNotSupportedException e = Assert.Throws<PlatformNotSupportedException>(() => ConnectAndDisconnect(connStr));
SqlException e = Assert.Throws<SqlException>(() => ConnectAndDisconnect(connStr));

string expectedMessage = "MSAL cannot determine the username (UPN) of the currently logged in user.For Integrated Windows Authentication and Username/Password flows, please use .WithUsername() before calling ExecuteAsync().";
Assert.Contains(expectedMessage, e.Message);
Expand Down Expand Up @@ -400,7 +400,7 @@ public static void ActiveDirectoryManagedIdentityWithInvalidUserIdMustFail(strin
string connStrWithNoCred = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, credKeys) +
$"Authentication=Active Directory Managed Identity; User Id={userId}";

Azure.Identity.CredentialUnavailableException e = Assert.Throws<Azure.Identity.CredentialUnavailableException>(() => ConnectAndDisconnect(connStrWithNoCred));
SqlException e = Assert.Throws<SqlException>(() => ConnectAndDisconnect(connStrWithNoCred));

string expectedMessage = "ManagedIdentityCredential authentication unavailable";
Assert.Contains(expectedMessage, e.GetBaseException().Message);
Expand Down