azure-signalr icon indicating copy to clipboard operation
azure-signalr copied to clipboard

AuthType=aad should default to DefaultAzureCredential

Open joukevandermaas opened this issue 3 years ago • 3 comments

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.Management v1.17.0

joukevandermaas avatar May 09 '22 08:05 joukevandermaas

@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

image

terencefan avatar May 19 '22 03:05 terencefan

@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.

joukevandermaas avatar May 19 '22 07:05 joukevandermaas

#1616 Hello, I've already created a PR to introduce a new type "azure" for fixing this. It should be in our next release.

terencefan avatar May 23 '22 03:05 terencefan