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

IFunctionInvocationFilter.OnExecutingAsync priority versus SignalRConnectionInfo #71

Open
CharlieDigital opened this issue Sep 3, 2019 · 7 comments

Comments

@CharlieDigital
Copy link

CharlieDigital commented Sep 3, 2019

Not sure if this is an issue for this project or https://github.com/Azure/azure-webjobs-sdk, so trying here first.

The issue is that the SignalRConnectionInfoAttribute binding is seemingly evaluated before IFunctionInvocationFilter. The symptom is that a header added via IFunctionInvocationFilter.OnExecutingAsync is available in the function method, but is empty when the UserId property is evaluated.

Repro steps

  1. Implement SignalR negotiate endpoint in Azure Functions per documented examples and specify SignalRConnectionInfoAttribute binding with UserId="{headers.x-my-auth-header}"

  2. Implement interface IFunctionInvocationFilter and in method OnExecutingAsync, inject the custom header into the request.

  3. Observe that the following exception is thrown:

System.Private.CoreLib: Exception while executing function: negotiate. Microsoft.Azure.WebJobs.Host: Exception binding parameter 'connectionInfo'. Microsoft.Azure.WebJobs.Host: Error while accessing 'x-my-auth-header': property doesn't exist.
  1. Observe that the header is available in the actual method body when retrieved from HttpRequest.Headers

Expected behavior

For the Functions SignalR parameter bindings to work properly for custom authentication scenarios, my understanding is that the application needs to be able to pre-process the request and extract the user information (for example, from JWT) and inject it into the headers. In order to use Users and Groups, it is necessary that the SignalRConnectionInfo is created with the UserId property.

The x-ms-client-principal-id is not set/not available unless using App Service Authentication.

Actual behavior

Headers injected via execution of IFunctionInvocationFilter.OnExecutingAsync are not available when the SignalRConnectionInfoAttribute is processed.

Known workarounds

Workaround is possible using imperative binding.

[FunctionName("negotiate")]
public async Task<SignalRConnectionInfo> GetSignalRInfo(
    [HttpTrigger(AuthorizationLevel.Anonymous, "post")] HttpRequest req,
    IBinder binder)
{
    string header = req.Headers["x-my-auth-header"]; // This value is here, but not available in the binding for the attribute

    SignalRConnectionInfoAttribute attribute = new SignalRConnectionInfoAttribute
    {
        HubName = "siteActivity",
        UserId = header
    };

    SignalRConnectionInfo connection = await binder.BindAsync<SignalRConnectionInfo>(attribute);

    return connection;
}

Related information

@wanlwanl
Copy link
Member

wanlwanl commented Nov 4, 2019

Could you share your code? By the way, Microsoft.Azure.WebJobs.Host.IFunctionInvocationFilter says "[Obsolete("Filters is in preview and there may be breaking changes in this area.")]"

@CharlieDigital
Copy link
Author

CharlieDigital commented Nov 4, 2019

@realwanpengli

The code below is the default per Microsoft documentation:

https://docs.microsoft.com/en-us/azure/azure-functions/functions-bindings-signalr-service

Note that the SignalRConnectionInfo is attempting to resolve the UserId from the request headers:

public static SignalRConnectionInfo Negotiate(
    [HttpTrigger(AuthorizationLevel.Anonymous)]HttpRequest req,
    [SignalRConnectionInfo
        (HubName = "chat", UserId = "{headers.x-ms-client-principal-id}")]
        SignalRConnectionInfo connectionInfo)
{
    // connectionInfo contains an access key token with a name identifier claim set to the authenticated user
    return connectionInfo;
}

Therefore, we would expect that if we inject the header x-ms-client-principal-id using an IFunctionInvocationFilter, it would be possible for the attribute to resolve the parameter value for UserId.

public abstract class AuthenticatedServiceBase : IFunctionInvocationFilter
{
    /// <summary>
    ///     Pre-execution filter.
    /// </summary>
    /// <remarks>
    ///     This mechanism can be used to extract the authentication information.  Unfortunately, the binding in SignalRConnectionInfoAttribute
    ///     does not pick this up from the headers even when bound.
    /// </remarks>
    public Task OnExecutingAsync(FunctionExecutingContext executingContext, CancellationToken cancellationToken)
    {
        DefaultHttpRequest message = executingContext.Arguments.First().Value as DefaultHttpRequest;
        // Some method to resolve the identity
        string identity = ResolveIdentitySomehow();
        // Now we can add it to headers, but the SignalRConnectionInfoAttribute does not pick this up
        message.Headers.Add("x-ms-client-principal-id", identity);
    }

    // Other code here...
}

@wanlwanl
Copy link
Member

wanlwanl commented Nov 5, 2019

OnExecutingAsync is run after input binding, so you can't add your own headers here.
Could you describe your scenario more? I think there's other ways to custom auth scenario without adding your own header in OnExecutingAsync.

@CharlieDigital
Copy link
Author

@realwanpengli

Yes, of course it is executed after the input binding and I think that's the unfortunate part as it means it is not possible to inject this value into the SignalRConnectionInfoAttribute.

The use case is to allow non-App Gateway authentication to work with SignalR and to make this code ultimately "cleaner" instead of using the imperative binding as shown in the first comment. It would not be obvious at all to users of this code that they should use the imperative binding.

@wanlwanl
Copy link
Member

wanlwanl commented Nov 6, 2019

@CharlieDigital

Since the arguments are binded before running into OnExcutingAsync, you are able to get all the binded arguments. So you don't need to modify http request, instead you can override the connectionInfo in OnExcutingAsync like this sample:

        public override Task OnExecutingAsync(FunctionExecutingContext executingContext, CancellationToken cancellationToken)
        {
            // Some method to resolve the identity
            string identity = "newUserId";//ResolveIdentitySomehow();
            
            // override connectionInfo argument
            var connectionInfo = (SignalRConnectionInfo)executingContext.Arguments["connectionInfo"];
            var serviceManager = StaticServiceHubContextStore.Get().ServiceManager;
            connectionInfo.AccessToken = serviceManager.GenerateClientAccessToken(Constants.HubName, identity);
            
            return Task.CompletedTask;
        }

@CharlieDigital
Copy link
Author

I think that this is not quite as elegant, but gets the job done :) (same with the imperative binding).

@wanlwanl
Copy link
Member

wanlwanl commented Nov 8, 2019

Modifing connectionInfo in OnExecutingAsync can seperate your token validation code with other business logic. Yes, no big difference. But it's the Function limitation. Will give you if any update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants