MarimerLLC / cslaforum

Discussion forum for CSLA .NET
https://cslanet.com
Other
31 stars 6 forks source link

Does Csla.ApplicationContext.User CustomPrincipal work differently in NetCore3.1? #879

Open russblair opened 4 years ago

russblair commented 4 years ago

Question Why would the Csla.ApplicationContext.User CustomPrincipal behave differently after upgrading a CSLA 4.11 NetCore2.2 application to CSLA 5.1 NetCore3.1?

My custom identity and principal classes were modeled after the ProjectTracker example. The following unit test works in NetCore2.2 but not in NetCore3.1.

    [TestMethod]
    public async Task LoginAsync_ValidCreds()
    {
        // arrange
        var username = "LoginName1";
        var password = "test";

        // act
        await LbPrincipal.LoginAsync(username, password);

        // assert
        Assert.IsTrue(Csla.ApplicationContext.User is LbPrincipal);
        Assert.IsTrue(Csla.ApplicationContext.User.Identity is ClaimsIdentity);
        Assert.IsTrue(Csla.ApplicationContext.User.Identity.IsAuthenticated);
    }

By stepping into the LoginAsync method I am able to confirm that the ApplicationContext.User is being correctly set to an authenticated LbPrincipal object but the ApplicationContext.User on the first Assert line is incorrectly set to the UnauthenticatedPrincpal.

Version and Platform CSLA version: 5.1.0 OS: Windows Platform: NetCore3.1, NetStandard2.1

rockfordlhotka commented 4 years ago

Is this in a console app or aspnetcore? They are quite different, because aspnetcore now requires the use of ClaimsPrincipal so you can no longer create custom principal types.

russblair commented 4 years ago

This is aspnetcore. Here are my class signatures: public class LbPrincipal : CslaClaimsPrincipal public class LbIdentity : CslaIdentityBaseCore<LbIdentity> public abstract class CslaIdentityBaseCore<T> : CslaIdentityBase<T> where T : CslaIdentityBase<T>

I followed the suggestions in #422 and #496 when I moved this application to CSLA 4.7 and it worked. It continued to work through to CSLA 4.11.2 NetCore2.2 but broke when I moved to CSLA 5.1 NetCore3.1.

rockfordlhotka commented 4 years ago

The thing is, aspnetcore does not allow custom principal types. The assumption is that ClaimsPrincipal does what you need, because you can add custom claims to ClaimsIdentity all you want.

So any extra fields of data that you'd have put into your custom principal/identity types in the past, you should be able to implement as claims today.

rockfordlhotka commented 4 years ago

(maybe I'm wrong, but I spent hours trying to figure out how to do custom types, and they always reset - just like what you are seeing - so I've come the conclusion that aspnetcore just doesn't like them)

rockfordlhotka commented 4 years ago

I should be more precise. afaik aspnetcore doesn't allow custom principal types, but it does allow custom identity types.

For example, in the Blazor client in ProjectTracker:

  private async void LoginUser()
  {
    var identity = await Library.Security.PTIdentity.GetPTIdentityAsync(vm.Model.Username, vm.Model.Password);

    var baseidentity = new ClaimsIdentity(identity.AuthenticationType);
    baseidentity.AddClaim(new Claim(ClaimTypes.Name, identity.Name));
    if (identity.Roles != null)
      foreach (var item in identity.Roles)
        baseidentity.AddClaim(new Claim(ClaimTypes.Role, item));
    var principal = new ClaimsPrincipal(baseidentity);

    if (identity.IsAuthenticated)
    {
      userService.CurrentUser = principal;
      NavigationManager.NavigateTo("/");
    }
    else
    {
      ErrorText = "Invalid credentials";
      StateHasChanged();
    }
  }
russblair commented 4 years ago

I replaced my code with yours and still no joy. I then noticed that my logout unit test was returning a ClaimsPrincipal. The difference between the login and logout methods is that the login is async. Next I created a LogoutAsync method and unit test with exactly the same code as the synchronous logout and it fails too!

Unit tests

        [TestMethod]
        public async Task Logout()    // this one passes
        {
            LbPrincipal.Logout();
            Assert.IsTrue(Csla.ApplicationContext.User is ClaimsPrincipal);
        }

        [TestMethod]
        public async Task LogoutAsync()    // this one fails
        {
            await LbPrincipal.LogoutAsync();
            Assert.IsTrue(Csla.ApplicationContext.User is ClaimsPrincipal);
        }

Logout methods

        public static void Logout()
        {
            var claimsIdentity = new ClaimsIdentity(new List<Claim>());
            var principal = new ClaimsPrincipal(claimsIdentity);
            ApplicationContext.User = principal;
            OnNewUser();
        }

        public static async Task LogoutAsync()
        {
            var claimsIdentity = new ClaimsIdentity(new List<Claim>());
            var principal = new ClaimsPrincipal(claimsIdentity);
            ApplicationContext.User = principal;
            OnNewUser();
        }

        public static event Action NewUser;
        private static void OnNewUser()
        {
            NewUser?.Invoke();
        }
russblair commented 4 years ago

After much digging, I was unable to discover if thread pool management of async/await was changed in netcore 3.1.

Since the logout sync worked and async did not, I created a sync login method, copied the contents of the async login method, changed the awaited FetchAsync to a Fetch, created a new unit test and, sure enough, the synchronous login method works.

It seems the combination of Csla.ApplicationContext.User and async/await on netcore 3.1 stubbornly refuses to bring back a ClaimsPrincipal.

rockfordlhotka commented 4 years ago

I'd recommend taking CSLA out of the equation and figuring it out with raw aspnetcore.

ApplicationContext.User is just a wrapper over the top of aspnetcore identity - so it is just adding confusion, where I suspect the issue is that aspnetcore itself isn't maintaining the identity - and in my experience that's often due to some missing config step in Startup.

rockfordlhotka commented 4 years ago

@russblair here's an excerpt from the Blazor book where I discuss the ClaimsPrincipal and ClaimsIdentity issue. I misspoke earlier in this thread - I don't think a custom identity is allowed anymore either. I think you must use the claims types from .NET.

===

The onclick event is handled by a VerifyCredentials method that does the login operation.

  private async void VerifyCredentials()
  {
    var identity = await DataPortal.FetchAsync<CustomIdentity>(vm.Model);
    var baseidentity = new ClaimsIdentity(identity.AuthenticationType);
    baseidentity.AddClaim(new Claim(ClaimTypes.Name, identity.Name));
    if (identity.Roles != null)
      foreach (var item in identity.Roles)
        baseidentity.AddClaim(new Claim(ClaimTypes.Role, item));
    var principal = new System.Security.Claims.ClaimsPrincipal(baseidentity);
    userService.CurrentUser = principal;
    StateHasChanged();
    nav.NavigateTo("/");
  }

The data portal is used to fetch a CustomIdentity instance. As I discussed earlier in this chapter, the resulting object will contain information about whether the credentials are or are not valid.

Modern .NET, and Blazor, are designed with the assumption that the user identity is a ClaimsPrincipal containing a ClaimsIdentity. The challenge is that ClaimsIdentity is not serializable, so the server-side code is creating an instance of CustomIdentity and the VerifyCredentials method needs to translate the identity information into a new ClaimsIdentity. That is the purpose of this block of code:

    var baseidentity = new ClaimsIdentity(identity.AuthenticationType);
    baseidentity.AddClaim(new Claim(ClaimTypes.Name, identity.Name));
    if (identity.Roles != null)
      foreach (var item in identity.Roles)
        baseidentity.AddClaim(new Claim(ClaimTypes.Role, item));

A .NET ClaimsPrincipal is created, initializing it using the new ClaimsIdentity object. Then that principal object is set to represent the current user by setting the CurrentUser property value of the CslaUserService object.

russblair commented 4 years ago

Thanks for the help @rockfordlhotka.

Further testing has revealed that calling the LoginAsync method from my netcore3.1 RazorPages project successfully returns a ClaimsPrincipal however calling the same method from my netcore3.1 unit tests project does not. I agree, it does seem like this is a project startup/environment issue.

My unit test project very simply tests the CSLA business objects against a MockDb.

The only NuGet packages in the unit test project are:

    <PackageReference Include="Csla" Version="5.1.0" />
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.5.0" />
    <PackageReference Include="MSTest.TestAdapter" Version="2.1.0" />
    <PackageReference Include="MSTest.TestFramework" Version="2.1.0" />

I created a startup.cs file to initialize the DI container as follows:

        public static ServiceProvider ServiceProvider { get; private set; }

        [AssemblyInitialize]
        public static void AssemblyInitialize(TestContext tc)
        {
            var services = new ServiceCollection();

            var loadedAssemblies = GetLoadedAssemblies();
            RegisterPackages(services, loadedAssemblies);

            services.AddCsla();

            ServiceProvider = services.BuildServiceProvider();
        }

The same login and logout code worked in netcore2.2 and it works in the aspnetcore3.1 razorpages project but not in the netcore3.1 unit tests project. I'm not sure what to try next.

russblair commented 4 years ago

I misspoke before when I said this issue was occurring in an aspnetcore project. The problem is in the unit test project not the aspnetcore project.

Is there a CSLA sample solution that includes a unit test project showing how to properly configure CSLA?

rockfordlhotka commented 4 years ago

There are unit tests for the configuration subsystem.

The trick is that the config necessary to unit test CSLA itself isn't the same as the config necessary to test your code in any given scenario.

I expect you'll need to configure .NET itself with some required services, plus configure CSLA to use a context manager that mirrors your expected behavior.

The default CSLA context manager now uses AsyncLocal, which should provide thread isolation, and so should work in most console app scenarios.

However, your test runner host might use threads differently from ASP.NET Core or a standard console app. They usually do, which adds complexity for some unit test scenarios (like using a custom principal in the old days).

using Csla.Configuration;

...

      CslaConfiguration.Configure()
        .ContextManager(new Csla.Core.ApplicationContextManager());

In v5.1 there are two built-in managers - this default (with AsyncLocal) and ApplicationContextManagerTls that uses thread-local storage.

I'm pretty sure that both of them get the User property from System.Threading though, so for user/principal/identity stuff they should be the same.

russblair commented 4 years ago

Thanks Rocky. I have run out of time and cannot go any deeper down this rabbit-hole. For now I have replaced all async authentication calls in the unit test project with sync calls and it is working. Maybe if if get some time I will create a ProjectTracker unit test project to see if I can recreate this issue.