dotnet / orleans

Cloud Native application framework for .NET
https://docs.microsoft.com/dotnet/orleans
MIT License
10.07k stars 2.02k forks source link

Orleans & .Net Security #8442

Open Tyler-Harker opened 1 year ago

Tyler-Harker commented 1 year ago

The Problem

Currently within orleans there doesn't seem to be a standard way to handle authorization in orleans. I stumbled across this interesting library https://github.com/Async-Hub/Orleans.Security that i believe there are a lot of cool benefits of incorporating something similar to this in the main Orleans repo.

I've been recently working with blazor and with this the concept of needing a WebApi to be a middleman for my blazor wasm application & Orleans cluster seems like overkill for simple POC's. It leads to a lot more boilerplate code to be managed, and more resources to be consumed when they aren't needed. I was looking at ways of securing the Orleans cluster so that I could publicly expose it, and call it directly from my client applications.

The Proposed Solution

The forementioned repo does a lot of things good, but there are some things I would change.

Things I would change

  1. Firstly I would not re-create all the aspnetcore security objects from Microsoft.AspNetCore.Authorization, I would instead reference this package from orleans. Since orleans already depends on other aspnetcore packages, I feel adding this to the security module wouldn't be that far of a leap.

How it would work

Users would define security attributes on their grain interfaces. like so

    [Authorize(Policy = "ManagerPolicy")]
    public interface IAuthorizationTestGrain : IGrainWithStringKey
    {
        [Authorize(Policy = "EmailVerifiedPolicy")]
        Task<string> TakeForEmailVerifiedPolicy(string someString);

        [Authorize(Roles = "Developer")]
        [Authorize(Roles = "Manager")]
        Task<string> TakeForCombinedRoles(string someString);

        [Authorize(Roles = "FemaleAdminPolicy")]
        Task<string> TakeForFemaleAdminPolicy(string someString);

        [Authorize(Policy = "FemaleManagerPolicy")]
        Task<string> TakeForFemaleManagerPolicy(string someString);

        [Authorize(Policy = "FemaleManagerPolicy")]
        [Authorize(Roles = "Admin")]
        Task<string> TakeForFemaleManagerPolicyAndAdminRole(string someString);

        [Authorize(Policy = "AdminPolicy")]
        [Authorize(Roles = "Admin")]
        Task<string> TakePrivateData(string someString);

        [AllowAnonymous]
        Task<string> TakePublicData(string someString);
    }

Because these policies would be defined in the grain interfaces we could do context validation both on the silo side & on the client side utilizing the various filters: IIncomingGrainCallFilter & IOutgoingGrainCallFilter

This means that we can reduce load on the silo because the client can check if they pass the validation / security requirements before the request is issued; but it will also actually validate the context on the silo side.

ReubenBond commented 1 year ago

This kind of thing is best as an extension, even if it's incorporated into the main repo. I agree that it's useful to have a relatively standardized way of dealing with AuthZ in grain calls. The typical strategy is to use RequestContext and an IIncomingGrainCallFilter and (if needed) IOutgoingGrainCallFilter.

For now, I would recommend submitting PRs to the Orleans.Security library which you mentioned.

jkonecki commented 1 year ago

@ReubenBond https://github.com/Async-Hub/Orleans.Security seems to be abandoned. What is more, that project uses a different set of attributes (essentially a copy of ASP.NET attributes).

I believe being able to use existing ASP.NET security will allow for easy adoption.

Security is a very important feature, especially in multi-tenant systems. Right now Orleans doesn't seem to have any security features in place.

I understand that it may be too early to move this code to main repo. However, I would love to see it in OrleansContrib. I think it will be beneficial to discuss the overall design and any areas of concern to ensure the final implementation will adhere to Orleans principles and long-term vision, and could be easily integrated into main repo should such decision been made.

PS. I can assist @Tyler-Harker with this work.

ReubenBond commented 1 year ago

@jkonecki your proposal sounds good to me. Do you have access to create a new repo in OrleansContrib? Ideally, if we can get approval from the existing repo owner, then we can ask them to transfer it to OrleansContrib (and help with that). It's not strictly necessary, but it's preferable.

jkonecki commented 1 year ago

@ReubenBond yes, I do. Happy to create it. We will try to contact the owner of the other repo. I understand your viewpoint and I'm glad you don't deem it strictly necessary. There will be significant changes between the existing and desired code anyway. @Tyler-Harker let's start with contacting the owner and asking for migration to OrleansContrib. I should be able to facilitate it and - if we don't get a response - I can create a new repo.

Reuben, do you have any concerns regarding the proposed design?

I have one worry related to incoming grain call filters (I'm not 100% sure if the order of filter execution can be controlled) - I would expect the security to be applied before any other call filter. Otherwise, the caller may trigger some processing and/or receive some response from the other call filter even without having permission to perform the call. I wonder if this requirement triggers any changes to the Orleans runtime.

ReubenBond commented 1 year ago

No concerns with the design as expressed. The potential for a pipeline ordering issue you mentioned exists today in ASP.NET, as far as I'm aware. I think documentation is the right approach for now (eg, ensure that the AddAuthorization() call happens first). Alternatively, the call can reorder registrations in the container to ensure it's first.

jkonecki commented 1 year ago

That's a fair argument, I agree.

Could we keep this issue open till we provision the other repo one way or the other, please?

schneiBoy commented 1 year ago

This would be a game-changer, and totally overhaul how we design solutions in our shop. Most people I talk with have always thought clients should be actors as well.

I guess my only concern would be the orders of complexity involved with maintaining the identity claim tokens across every single grain. With web servers the methods of keeping permissions up to date is simple enough in part because the number of service instances is small, but the number of grains can potentially get cumbersome when aggregating the checks for claim tokens.

The added authentication would also need a way of bringing the missing token information back to the user/client in the event a redirect to the IdP is needed.

If this happens you would all be my heroes.