Azure / azure-functions-signalrservice-extension

Azure Functions bindings for SignalR Service. Project moved to https://github.com/Azure/azure-sdk-for-net/tree/main/sdk/signalr/Microsoft.Azure.WebJobs.Extensions.SignalRService .
MIT License
97 stars 48 forks source link

Multiple jwt claims of the same type not supported? #260

Open rockgecko-development opened 3 years ago

rockgecko-development commented 3 years ago

It seems multiple jwt claims of the same type are not supported in signalr serverless. When I forward such claims to the Negotiate function, it works, connects, I can receive messages in the frontend, but when I try to call any hub methods, I get the following response via websocket message:

Invocation failed, status code 500

and no further information. The method itself in my Azure Function (eg SendToGroup()) is never hit.

I used the BiDirectionChat sample as a starting point: https://github.com/aspnet/AzureSignalR-samples/tree/main/samples/BidirectionChat with the following changes:

Y-Sindo commented 3 years ago

This is a known issue fixed by #151, please update the SignalR extension to the newest version 1.5.0 . The sample will be updated later.

rockgecko-development commented 3 years ago

Thanks, I did update to v1.5.0, but I was still running locally, so the hub methods that were being called were still on the deployed version v1.2.2 from the sample. Deploying fixed the 500 error, but #151 still doesn't allow me to have multiple groups claims. The info in that change says:

If you multiple claims have the same key, only the first one will be reserved.

I'm not quite sure what it reserves, or why it is still limited to the first one, when it could be easily modified to return the same claims that were supplied in Negotiate(name, claims), like this:

public static List<Claim> GetClaimsList(string claims)
        {
            if (string.IsNullOrEmpty(claims))
            {
                return default;
            }

            // The claim string looks like "a: v, b: v"
            return claims.Split(',', StringSplitOptions.RemoveEmptyEntries)
                .Select(p => p.Split(": ", StringSplitOptions.RemoveEmptyEntries)).Where(l => l.Length == 2)
                .Select(c => new Claim(c[0].Trim(), c[1].Trim())).ToList();
        }

usage:

 [FunctionName(nameof(SendToGroup))]
        public async Task SendToGroup([SignalRTrigger]InvocationContext invocationContext, string groupName, string message, ILogger logger)
        {
            var claimsHeader = invocationContext.Headers["X-ASRS-User-Claims"];
                var claims = GetClaimsList(claimsHeader);
                //use claims, eg:
                logger.LogInformation($"Parsed claims from {claimsHeader} to {string.Join(", ", claims.Select(c => $"{c.Type}: {c.Value}"))}");

            await Clients.Group(groupName).SendAsync(NewMessageTarget, new NewMessage(invocationContext, message));
        }

There are still some issues with this too, however. I will need to make sure that none of my users have colons or commas in their names for example, and filter those during Negotiate(), otherwise it will still fail to parse correctly.

zackliu commented 3 years ago

@rockgecko-development The current problem is the claim is now putting in a header and header has different rules from claims. In header, multiple value with the same header key should use comma to separate. However, claim itself can have comma. On the other hand, claim is using Claim.ToString() to serialize, which become a "type: value" schema. It's also not correct. Based on that, we hard to handle it in function binding side correctly. We're investigating how to deal with it more correctly.