azure-signalr
azure-signalr copied to clipboard
AuthType=aad should default to DefaultAzureCredential
Describe the bug
When the signalr connection string says AuthType=aad and nothing else related to auth, it should use DefaultAzureCredential. This credential automatically figures out a lot of stuff and can be configured through environment variables etc.. It automatically works locally or in a managed identity context. It is the default for all other Azure services.
Somehow Azure SignalR tries to determine for itself which credential type it should use, which is not the expected behavior given all the other Azure SDKs (e.g. storage, service bus, sql, etc).
To Reproduce
Given this snippet:
var mgr = new ServiceManagerBuilder()
.WithOptions(o =>
{
o.ConnectionString = "Endpoint=https://example.service.signalr.net;AuthType=aad;Version=1.0;";
})
.BuildServiceManager();
I expect the service manager to use DefaultAzureCredential when generating tokens to authenticate with the service. It currently appears to use ManagedIdentityCredential which only works in a managed identity context.
Exceptions (if any)
AzureSignalRAccessTokenNotAuthorizedException: The given AzureAD identity don't have the permission to generate access token.
at Microsoft.Azure.SignalR.AadAccessKey.<GenerateAccessTokenAsync>d__23.MoveNext()
While the identity used by DefaultAzureCredential is SignalR Service Owner. The same code does not throw in a managed identity context.
Further technical details
Microsoft.Azure.SignalR.Managementv1.17.0
@joukevandermaas Hello, it's not recommended to use connection string to set up AAD identity anymore.
https://docs.microsoft.com/en-us/azure/azure-signalr/signalr-howto-authorize-application#app-server

@terencefan thank you for the reply. We have indeed settled on that solution.
However, the connection string is shown in Azure Portal. Currently it shows both the key and the AAD connection strings (as it does for other services).
Either way I think this is a very easy thing to fix and would help a lot. I can make a draft PR if you like to show how what I mean. It should not be a breaking change.
PS. I personally think the connection string approach is nice, it allows you to switch on the fly between the two auth approaches should there be an issue with one of them.
#1616 Hello, I've already created a PR to introduce a new type "azure" for fixing this. It should be in our next release.