Aspnetcore: Should SignalR pass a resource into IPolicyEvaluator?

[HaoK]

MVC passes in the AuthorizationFilterContext. SignalR should decide what resource is appropriate (if any)

[anurse]

Yes, we should :). cc @BrennanConroy

[anurse]

The context should be ConnectionContext, initialized with as much data as we have at the time. During negotiate, that will basically just be the HttpContext. During a connection request (SSE, LP, WS), we'll know the transport type, etc. This is going to mean some fairly hairy refactors to HttpConnectionDispatcher.

[DaRosenberg]

As requested by @BrennanConroy in #2831 here's our scenario and the reason why we need this.

Our authorization story involves providing a JWT token and user ID in the query string. If your JWT token is for scope A, you are only allowed to connect with the same user ID as the subject of your token (act as yourself). On the other hand, if your token is for a more privileged scope B, you are allowed to connect as any user ID, i.e. pass any user ID on the query string (act as someone else). This impersonation is necessary for several back-end processes where there is no feasible way to do on-behalf-on stuff because they involve operations not initiated by a user.

This authorization logic is evaluated in an authorization handler, and applies equally both to our controller methods and to the SignalR hub, to which users can connect to receive notifications for data changes.

Without access to the HTTP context (and thereby to the query string parameters used to connect to the hub) the authorization filter has no way of evaluating this logic.

As such we are block on this, unless someone has a creative workaround to suggest.

[anurse]

Thanks for the scenario, that helps a lot! Can you use IHttpContextAccessor in your auth handler to work around this?

[DaRosenberg]

@anurse Using IHttpContextAccessor works like a charm. Don't know why I didn't think of that. Thanks!

[muratg]

@Tratcher Thoughts on this?

[Tratcher]

@muratg it sounds reasonable.

[anurse]

I don't think this is much work. We should just look at what MVC passes in and pass a similar thing that contains SignalR-related state. For the HTTP connection level we can't pass much extra in, but when authenticating a specific invocation we can.

[rynowak]

Pass in the endpoint IMO. We're converging around that as the thing that has all of the infoZ.

[anurse]

Is that true for SignalR though? There's all the connection metadata on ConnectionContext and I don't know if that's replicated on the endpoint.

[rynowak]

If it's truly metadata then you should add it to the endpoint metadata. If it's part of the request, then I dunno 🤷‍♂️

[anurse]

Triage decision:

• Add relevant metadata to the endpoint
• Pass the endpoint in as the resource

[anurse]

We don't have Hub-level auth any more (it's in the auth middleware), but we do still do method-level auth. We need to prepare a resource and pass it in. We can't just use the endpoint here because we need to include the method being executed (and the args?)

[rynowak]

How does that work if you're doing middleware-based auth?

[anurse]

We still do that auth ourselves by manually calling AuthorizeAsync. The change we made to disable our custom auth only applies to the front-line hub level auth attributes.

[rynowak]

Ah gotcha. I saw you reply and had a minor panic because I thought I understood what you were doing here already. It makes sense since the middleware won't run on arbitrary signalR messages.

[BrennanConroy]

Acceptance checklist (check one item)

• [ ] We decided not to take this fix.
• [ ] The fix is tests-only.
• [x] The fix contains product changes (check all items below).
  
  
  
  • [x] Relevant XML documentation comments for new public APIs are present.
  
  
  • [x] Narrative docs (docs.microsoft.com) are updated. (check one item below)
    
    
    
    
    
    • [x] The change requires a new article. An issue with an outline has been filed here: [aspnet/AspNetCore.Docs/issues/13050]
    
    
    
    • [ ] The change requires a change to an existing article. A docs PR with these changes is linked here: [PR LINK]
    
    
    
    • [ ] The change requires no docs changes.
    
    
    
  
  
  • [x] Verification has been completed. (check one item below)
    
    
    
    
    
    • [x] The change is in the shared framework and was verified against the following versions
      
      
      
      
      
      
      
      • SDK installer: [3.0.100-preview7-012687]
      
      
      
      
      • ASP.NET Core Runtime: [3.0.0-preview7.19326.10]
      
      
      
      
      • .NET Core Runtime: [3.0.0-preview7-27826-04]
      
      
      
      
    
    
    
    • [ ] The change is in an OOB NuGet/NPM/JAR package and was verified against the following version of that package: [PACKAGE ID] [VERSION]