Closed dazinator closed 7 years ago
I am wondering if there is any way / extensibility point I could use to make CSLA use my own backing store for ApplicationContext.User, rather than its own default logic
Oh i think IContextManager
is what I am after
So this is basically what I have got, the only tricky bit is working out what to do with SetUser() - as asp.net core want's a claims principle, but CSLA takes an IPrinciple.
/// <summary>
/// Application context manager that uses HttpContextAccessor whenr esolving HttpContext
/// to store context values.
/// </summary>
public class HttpContextAccessorContextMananger : IContextManager
{
private const string _localContextName = "Csla.LocalContext";
private const string _clientContextName = "Csla.ClientContext";
private const string _globalContextName = "Csla.GlobalContext";
private readonly IServiceProvider _serviceProvider;
public HttpContextAccessorContextMananger(IServiceProvider serviceProvider)
{
_serviceProvider = serviceProvider;
}
protected virtual HttpContext GetHttpContext()
{
var httpContextAccessor = (IHttpContextAccessor)_serviceProvider.GetService(typeof(IHttpContextAccessor));
if (httpContextAccessor != null)
{
return httpContextAccessor.HttpContext;
}
return null;
}
/// <summary>
/// Gets a value indicating whether this
/// context manager is valid for use in
/// the current environment.
/// </summary>
public bool IsValid
{
get { return GetHttpContext() != null; }
}
/// <summary>
/// Gets the current principal.
/// </summary>
public System.Security.Principal.IPrincipal GetUser()
{
return GetHttpContext()?.User;
}
/// <summary>
/// Sets the current principal.
/// </summary>
/// <param name="principal">Principal object.</param>
public void SetUser(System.Security.Principal.IPrincipal principal)
{
GetHttpContext().User = (ClaimsPrincipal)principal;
}
/// <summary>
/// Gets the local context.
/// </summary>
public ContextDictionary GetLocalContext()
{
return (ContextDictionary)GetHttpContext().Items[_localContextName];
}
/// <summary>
/// Sets the local context.
/// </summary>
/// <param name="localContext">Local context.</param>
public void SetLocalContext(ContextDictionary localContext)
{
GetHttpContext().Items[_localContextName] = localContext;
}
/// <summary>
/// Gets the client context.
/// </summary>
public ContextDictionary GetClientContext()
{
return (ContextDictionary)GetHttpContext().Items[_clientContextName];
}
/// <summary>
/// Sets the client context.
/// </summary>
/// <param name="clientContext">Client context.</param>
public void SetClientContext(ContextDictionary clientContext)
{
GetHttpContext().Items[_clientContextName] = clientContext;
}
/// <summary>
/// Gets the global context.
/// </summary>
public ContextDictionary GetGlobalContext()
{
return (ContextDictionary)GetHttpContext().Items[_globalContextName];
}
/// <summary>
/// Sets the global context.
/// </summary>
/// <param name="globalContext">Global context.</param>
public void SetGlobalContext(ContextDictionary globalContext)
{
GetHttpContext().Items[_globalContextName] = globalContext;
}
}
and in startup.cs
Csla.ApplicationContext.ContextManager = new HttpContextAccessorContextMananger(app.ApplicationServices);
ClaimsPrincipal implements IPrincipal so you may be able to cast this as appropriate.
See also http://andrewlock.net/introduction-to-authentication-with-asp-net-core/
Yeah - I am just blindly upcasting the IPrinciple to a ClaimsPrinciple at the moment, as I think it's safe to assume within my partciular application, that we will only use ClaimsPrinciple. If anyone was to implement / use a custom IPrinciple not derived from ClaimsPrinciple then they would have to tweak this:
GetHttpContext().User = (ClaimsPrincipal)principal;
I have a question..
Csla.ApplicationContext.ContextManager
is static (app domain level).
In my asp.net core application, aside from standard MVC stuff, I also have some background hangfire jobs that run, and they run on a background thread, and HttpContext isn't available.
In this instance, CSLA is still making various calls to my Csla.ApplicationContext.ContextManager
- and my implementation can't find a HttpContext and so it does the following:
I am wondering If i need to handle HttpContext being null in my ContextManager implementation and fallback to using the current thread? Or should CSLA do that for me?
My ContextManager also implements IsValid
but it never get's called - so I am wondering what the purpose of IsValid is..
/// <summary>
/// Gets a value indicating whether this
/// context manager is valid for use in
/// the current environment.
/// </summary>
public bool IsValid
{
get
{
return GetHttpContext() != null;
}
}
Doh...
Spotted this:
I should be setting ApplicationContext.WebContextManager
not ContextManager.
Leaving this open, in case CSLA wants to support asp.net core in future, then it will need some support package with a custom IContextManager like this. Unless @rockfordlhotka you see this living in a seperate repo?
I think it would be ideal to have this as part of the Csla.Web.Mvc
assembly for ASP.NET Core, yes.
I plan to have such an assembly - mostly now just waiting for the churn around project.json vs csproj to settle with the release of VS2017. And perhaps wait for netstandard 2.0 as well - no sense putting in lots of work now just to redo it all in a couple months.
Ok nice.
I ended up writing some relevant extension methods in the end to enable CSLA to be more intuitively setup within the context of an asp.net core application's startup class.
Usage from startup.cs:
public override void ConfigureServices(IServiceCollection services)
{
services.ConfigureCsla((options, sp) => {
options.ObjectFactoryLoader = new ServiceProviderObjectFactoryLoader(sp);
// could configure other CSLA hooks / options in future.
});
}
public void Configure(IApplicationBuilder app, IHostingEnvironment env, ILoggerFactory loggerFactory)
{
// ommitted for brevity
app.UseCsla();
}
So in ConfigureServices()
some options can be registered that control what hooks CSLA will use.
Then in Configure()
the UseCsla()
method grabs those options, then set's up CSLA appropriately.
And the extension method itself:
public static class CslaConfiguration
{
public static IServiceCollection ConfigureCsla(this IServiceCollection services, Action<CslaOptions, IServiceProvider> setupAction = null)
{
services.AddSingleton<CslaOptions>((sp) =>
{
var options = new CslaOptions();
setupAction?.Invoke(options, sp);
return options;
});
return services;
}
public static IApplicationBuilder UseCsla(this IApplicationBuilder appBuilder)
{
// grab the options
var options = appBuilder.ApplicationServices.GetRequiredService<CslaOptions>();
// configure csla according to those options.
Csla.Server.FactoryDataPortal.FactoryLoader = options.ObjectFactoryLoader;
Csla.ApplicationContext.WebContextManager = options.WebContextManager ?? new HttpContextAccessorContextMananger(appBuilder.ApplicationServices);
return appBuilder;
}
public class CslaOptions
{
public IObjectFactoryLoader ObjectFactoryLoader { get; set; }
public IContextManager WebContextManager { get; set; }
}
}
The thing I like about this is that it centralises the configuration of CSLA as the consumer sees all relevant configuration options in on place (on the options class)..
The other thing I like is that the consumer is giving access to the IServiceProvider when configuring the csla options, so its easy for them to implement csla extensibility points that rely on an IServiceProvider for DI.
What you've done here ties into #525 I think.
Potentially! I have chucked out a blog post covering this topic for the time being, in case anyone else stumbles accross this problem: http://darrelltunnell.net/blog/2017/01/24/csla-and-asp.net-core/
I'm changing the default (not web) context manager to use AsyncLocal
(#729). I wonder if that would be the right solution for ASP.NET Core as well?
I suppose not really though, because the solution in @dazinator's blog post is probably still the best answer.
If my memory serves, the default context manager should be irrelevent when running under asp.net core because the web context manager should override it (and the web context manager should be set on startup)
Just out of curiosity, why are there 2 context manager properties, why not a single Context Manager property that is set appropriately for the current platform on startup?
I noticed on that line that the web context manager check is compiled out of netstandard2.0 builds. Given that netstandard is implemented by a wide array of platforms (including platforms that run web applications like asp.net core) is it good / safe to assume that web is not appropriate? This problem of whether to include the web property or not would go away if you only had a single Context Manager property.
There's a lot of legacy in that code, and it could use some cleanup. I don't honestly remember why there are two, and it doesn't seem to make a lot of sense looking at it now...
You are right - what I did to the default context manager isn't relevant to the web. I'm just speculating as to whether what I did change would fix the web too, without having to worry about the stuff in your blog post.
The more I think about it though, the more I think you are on the right track. We don't know if AsyncLocal
is safe within ASP.NET Core. If it was safe, then why would ASP.NET Core have the model they do?
We don't know if AsyncLocal is safe within ASP.NET Core. If it was safe, then why would ASP.NET Core have the model they do?
Well.. in asp.net core, we have to get the User / Principle from HttpContext, and the only way to get HttpContext is to be a middleware (i.e like owin middleware, but asp.net abandoned owin, so slightly different) or to use the IHttpContextAccessor
which is registered as a singleton.
As IHttpContextAccessor
is a singleton, I had a quick look to see how it manages thread safety, and low and behold it uses AsyncLocal
:
So in other words, as long as the IContextManager
for asp.net core stores things in HttpContext
(User, Items etc) - like my current implementation does, then as HttpContext
is itself now accessed through / backed by an AsyncLocal
- things should flow as desired!
Excellent!
Sorry - whilst I was looking through that file I saw something unrelated to this discussion but that looked odd - I wondered if this lock statement should have curly braces around both lines:
UPDATE: Looking at that further.. I think you are right this file just needs a general tidy up :-)
@dazinator I figured out/remembered why there might be two "active" context managers at one time (a web one and another).
In regular old ASP.NET it was possible for your code to sometimes have, and sometimes not have, access to HttpContext
. I don't remember the scenario under which HttpContext.Current
would return null
, but it could happen.
As a result, each IContextManager
has an IsValid
property to indicate whether it is currently valid. And the ASP.NET one might return false
, causing the code to fall back to the default manager.
I won't change that code, messy though it seems, because there's no point in causing possible regressions for regular ASP.NET 2.0-4.6 users.
However, I am going to add a new IContextManager
implementation in the ASP.NET Core 2.0 assembly that works as you describe earlier in this thread. Along with the ConfigureCsla
code.
@dazinator I requested a review of my changes via the PR - I suspect you'll get an email to that effect.
If you have time, I'd really appreciate your thoughts on my changes (which are largely pulled from your blog post).
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.
First let me describe the setup!
I have an asp.net core project, with an MVC controller method - the method isn't important aside from the fact it uses authentication via an [Authorize] attribute:
Cookie authentication is pretty common, and in asp.net core it looks like this in Startup.cs
Notice I have set a custom events class - this is an optional thing you can implement if you want to intercept the standard cookie authentication lifetime events.
Here is mine:
Pretty sure I am handling the Validate() method incorrectly at the present, however that shouldn't matter for the purposes of this issue.
Now to the crux..
When the controller method is called, the [Authorise] attribute does it's job, and cookie authentication runs. The CustomCookieAuthenticationEvents class is invoked and sets ApplicationContext.User to a claims principle.
However as soon as the mvc controller method is actually entered, ApplicationContext.User returns a GenericPrinciple again.
I am guessing this is because CSLA is trying to detect HttpContext.Current which may not be working for asp.net core applications which rely on
IHttpContextAccessor
to get hold of HttpContext instance. I have not looked at the CSLA code, but I assume CSLA is then treating the application like a desktop application, and storing the principle on the current thread.