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.

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

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

Open
rockgecko-development opened this issue Jul 21, 2021 · 3 comments
Open

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

rockgecko-development opened this issue Jul 21, 2021 · 3 comments
Labels
bug Something isn't working

Comments

@rockgecko-development
Copy link

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:

  • Update to latest C# and JS signalr versions
  • Modify index.html generateAccessToken method to send my own token containing multiple groups claims:
var data = {
          "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/nameidentifier": userName,
          "exp": 1699819025,
            "admin": isAdmin,
          "groups":["first","second","third"]
        };
  • Modify Function.cs Negotiate endpoint:
public SignalRConnectionInfo Negotiate([HttpTrigger(AuthorizationLevel.Anonymous)]HttpRequest req)
        {
            var claims = GetClaims(req.Headers["Authorization"]);
            var username = claims.First(c=>c.Type=="http://schemas.xmlsoap.org/ws/2005/05/identity/claims/nameidentifier").Value;
            return Negotiate(username, claims);
        }

When running the sample, I can receive broadcast chat messages, but I get the above error when I try to send.
Removing the "groups":["first","second","third"] line (or changing it to a single group item) fixes the problem.

@Y-Sindo
Copy link
Member

Y-Sindo commented Jul 21, 2021

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
Copy link
Author

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
Copy link
Contributor

zackliu commented Jul 22, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants