Azure / azure-signalr

Azure SignalR Service SDK for .NET
https://aka.ms/signalr-service
MIT License
425 stars 100 forks source link

Include NameIdentifier claim in Azure function negotiate method using TypeScript #493

Closed jrmcdona closed 5 years ago

jrmcdona commented 5 years ago

How do we include NameIdentifier claim in Azure function negotiate method using TypeScript?

I am calling negotiate API from an Angular app that has an authenticated user. When I generate an access token my negotiate method, I am not sure how to add in nameIdentifier.

I will be using REST API to send message to single clients.

JialinXin commented 5 years ago

@jrmcdona are you using Azure SignalR function extension binding? We provide an attribute UserId in SignalRConnectionInfo for negotiate use. See https://github.com/Azure/azure-functions-signalrservice-extension/blob/dev/src/SignalRServiceExtension/SignalRConnectionInfoAttribute.cs#L21-#L32.

So you can assign this value in functions.json like below. You may take our js sample for some reference. Let me know if you have any further questions.

{
      "type": "signalRConnectionInfo",
      "name": "connectionInfo",
      "userId": "{headers.x-ms-signalr-userid}",
      "hubName": "simplechat",
      "direction": "in"
}
jrmcdona commented 5 years ago

@JialinXin So in order to populate "userId": "{headers.x-ms-signalr-userid}" with a value we need to add App service authentication to our Azure function?

I want to add my client is only receiving messages. My clients is not sending any messages. I will be using REST API to send messages to the client/user. https://docs.microsoft.com/en-us/azure/azure-signalr/signalr-quickstart-rest-api#send-user

In your sample, it looks like the userid value is getting added when a message is being sent via function method. But in the REST API how would this look like?

I am going to try it like this with a header in my Angular app - see the getSignalRConnetion method where I pass x-ms-signalr-userid header:

  public init() {
    console.log('Init SignalR');
    this.getSignalRConnection().subscribe(con => {
      console.log('SignalR con: ' + con);
      const options = {
        accessTokenFactory: () => con.accessToken
      };
      this.hubConnection = new SignalR.HubConnectionBuilder()
        .withUrl(con.url, options)
        .configureLogging(SignalR.LogLevel.Information)
        .build();

      this.hubConnection.on("UpdateProfile", data => {
        console.log(data);
      });

      this.hubConnection.start().catch(error => console.error(error));
    });
  }

  private getSignalRConnection(): Observable<SignalRConnection> {
    return this.http.get<SignalRConnection>(
      'https://localhost:7071/api/negotiate', {headers: new HttpHeaders().set('x-ms-signalr-userid', '00034001CA87777') }
    );
  }
JialinXin commented 5 years ago

@jrmcdona Yeah. You did it in the right way. You can name x-ms-signalr-userid as you like, just to make sure to pass it in function binding signalRConnectionInfo UserId during negotiate. Then it will generate AccessToken based on it and build connection with Azure SignalR Service. And later when client send message to user/groups via RestApi, the same AccessToken will be used. We put the set part in our client code here.

jrmcdona commented 5 years ago

@JialinXin cool, I was able to get it working!

jrmcdona commented 5 years ago

@JialinXin one additional thing. Do you feel this is secure enough? Or do we need another level of security in the Function apps?

I don't want anybody to grab endpoints from the client side and generate all of the necessary tokens to be able to hit the Azure SignalR service.

Thanks

anthonychu commented 5 years ago

From what I can tell, this is not secure.

Even without thinking about SignalR, how does your function app authenticate it’s users (how does it know the user is who they say they are)?

jrmcdona commented 5 years ago

@anthonychu I agree. This is for a 1st party Microsoft app and I think the Authentication providers are not made for 1st party apps. I would say certainly Microsoft Account is not. I think it may be easier once we get Azure AD converged endpoints rolled out for 1st party but they are not.

Let me know if you know anything more about this topic.

I will see if I can get some auth going in the function. In a traditional web API asp.net project or core project you set everyhing up in startup.cs such as the RPS bits needed for Microsoft accounts.

jrmcdona commented 5 years ago

@anthonychu @JialinXin So I am looking more into this for AAD. Since this is an API call we do not want to use App service authentication. Instead we would be using Bearer token authentication, something like this. Unless I am missing somthing? Something like this: https://blog.wille-zone.de/post/secure-azure-functions-with-jwt-token/

However, what I do not know is in this case how to get the NameIdentifier passed on to the SignalR service from the Negotiate method?

anthonychu commented 5 years ago

Because SignalRConnectionInfo is an input binding, what you are hoping to do is a bit awkward but possible.

If you'd like to chat about it, I'm currently on vacation but feel free to set up a meeting with me later on this week. We can talk through it and update this issue with the outcome.

chenkennt commented 5 years ago

Right now the userId in function.json isn't really useful, as in a real scenario you shouldn't trust whoever the client claim he is. In a real world scenario your negotiate should be protected by some authentication mechanism by yourself and in the negotiate you should get the identity from the authenticated user and put it to the SignalR access token. Take the JWT token way you mentioned as an example, you should first validate the JWT token of the negotiate function, get the userId from the claims, then put it to the negotiate result. The problem is it's not easy to finish it in a simple expression in function.json. For now please refer to this example for how to do it in javascript:

https://github.com/aspnet/AzureSignalR-samples/issues/42

jrmcdona commented 5 years ago

@chenkennt @anthonychu Hello, I am trying this with C# and I am unable to Negotiate with SignalR. In my Azure function for negotiate I am first manually validating the AAD Bearer token sent in the headers. That is working fine.

This also give me the ClaimsPrincipal if the user is authorized and allows me to get the Name Identifier.

Once that is done, I am trying to GenerateAccessToken and add the Name identifier claim but the token is Unauthorized:

https://github.com/aspnet/AzureSignalR-samples/pull/39/files#diff-577b0ba7bbffc0f105ca22d9947f3191R33

I am also using this sample too that was mentioned: aspnet/AzureSignalR-samples#42

Error: POST https://css-ambs-sigr.service.signalr.net/client/negotiate?hub=ambassadors 401 (Unauthorized) Utils.js:203 [2019-04-23T20:38:46.666Z] Error: Failed to complete negotiation with the server: Error: Unauthorized

Can you all see anything I am doing wrong here?

                ClaimsPrincipal principal = await Security.ValidateTokenAsync(authHeader.ToString(), req.HttpContext);

                if (principal == null || (principal.Identity == null || !principal.Identity.IsAuthenticated))
                {
                    return new UnauthorizedResult();
                }

                log.LogInformation(principal.Identity.Name);

                ServiceUtils _serviceUtils = new ServiceUtils(System.Environment.GetEnvironmentVariable("AzureSignalRConnectionString"));
                var sigRConnToken = _serviceUtils.GenerateAccessToken(_serviceUtils.Endpoint, principal.Identity.Name);
                connectionInfo.AccessToken = sigRConnToken;

                return new OkObjectResult(connectionInfo);
jrmcdona commented 5 years ago

I needed to use the URL of the Hub and not the Endpoint. var sigRConnToken = _serviceUtils.GenerateAccessToken(connectionInfo.Url, principal.Identity.Name); So my audience was wrong on the token.

anthonychu commented 5 years ago

@jrmcdona If you’re using C#, you can also use the binding to generate the token with a user ID like this: https://gist.github.com/ErikAndreas/72c94a0c8a9e6e632f44522c41be8ee7

It might be better than doing this yourself in case anything changes in the future.