Don't use WindowsPrincipal in .NET 6 code

[rockfordlhotka]

In some cases (such as Csla.Xaml), there is code like this:

    public override IPrincipal GetUser()
    {
      if (_principal == null)
      {
        if (ApplicationContext.AuthenticationType == "Windows")
#pragma warning disable CA1416 // Validate platform compatibility
          SetUser(new WindowsPrincipal(WindowsIdentity.GetCurrent()));
#pragma warning restore CA1416 // Validate platform compatibility
        else
          SetUser(new System.Security.Claims.ClaimsPrincipal());
      }
      return _principal;
    }

"Modern" platforms no longer support the WindowsPrincipal type, and so this code fails at runtime.

We might resolve this by adding another CSLA authentication type beyond "Windows" to distinguish the case where we do not want auto-impersonation but where we do want a ClaimsPrincipal by default.

Right now the "Windows" authn type does two things:

1. Tells the data portal not to flow the client identity to the server
2. Tells some platform-specific code to use WindowsPrincipal

A new authentication type might:

1. Tell the data portal not to flow the client identity to the server
2. Use ClaimsPrincipal like we do by default

Another way to resolve this might be to use compiler directives in the platform-specific code to avoid using the WindowsPrincipal type. This may be a simpler solution if the right build constants exist.

[rockfordlhotka]

Perhaps a better solution: https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1416

[TheCakeMonster]

I was wondering whether doing #3043 sooner rather than later might help resolve this?

Separating out the use of the "Windows" authentication type and the flowing of user context from client to server are both done with the same thing at the moment. If we separate them, nobody needs to use "Windows" unless it's helpful in their environment. We want to do that task at some point, and there seems to be a synergy to me.

[rockfordlhotka]

The problem is taht #3043 is a breaking change, so needs to wait until CSLA 7, and we know that at least one person is being affected by this bug right now.

[TheCakeMonster]

It is a breaking change, but it's not a per-class breaking change. Instead it raises a single, per application change to fluent config (at both ends.) As we did in #2922, we sometimes introduce breaking changes in point releases when it is worth it. Because this is security-related, it feels worth it to me.

If you remain worried then the other option is to do the change I have suggested in #3043 but set the default the other way, for the time being - set the default to having the flow enabled. That means we make a step in the right direction yet avoid the breaking change.

The only extra work we would have to do on #3043 to avoid the break would be to add an additional method to allow people to turn off the flow, over and above the method we want to add to turn it on in the future.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.