I am trying to change the way our existing c# application handles security. The end result is that a we will use a new "user" object that looks different from the current user object we have. But we want to introduce this user model in a way that won't break existing code. Until we finish migrating all our old code, we will have 2 ways of handling security / determining what functionality a user is allowed to access.
How the Code Works Today
As a part of our authentication process, when a user logs in, we load a their profile from our database, and this profile includes a list of application permissions.
Here's a sample controller that would trigger the security logic:
[Route("me/permissions")]
[HttpGet]
[SecuredEndpoint(WidgetAppPermissions.General)]
public string DoSomethingAmazing(GetAmazingDataRequest request)
{
try
{
var x = someAmazingObject.DoSomething()
return JsonConvert.SerializeObject(x);
}
catch (Exception ex)
{
return ex.Message.ToString();
}
}
This is what the GetAmazingDataRequest class looks like:
public class GetAmazingDataRequest : Request
{
}
And the logic behind the SecuredEndpoint attribute looks like this: (note: it's a public sealed class)
public sealed class SecuredEndpointAttribute : ActionFilterAttribute, IAuthorizationFilter
{
private readonly WidgetAppPermissions[] functions;
public SecuredEndpointAttribute(params WidgetAppPermissions[] functions)
{
this.functions = functions;
}
public override void OnActionExecuting(ActionExecutingContext context)
{
//it's inside this method that eventually we call GetUserProfile to load the user
profile from the database, including the list of permissions.
var user = userRepo.GetUserProfile(context.HttpContext.User);
The idea is that the OnActionExecuting() logic checks the user's profile that was retrieved from the database to ensure that he / she has, in this case, a permission called "General" included in their profile. If they do, then the execution returns and the system actually tries the logic inside the DoSomethingAmazing() method.
What I Tried
The system runs the SecuredEndPoint attribute logic before actually running the logic inside the DoSomethingAmazing(). But while debugging I noticed that the name of the request class is included inside the "context" object that's passed into OnActionExecuting()
So I was thinking that for each method in any controller that we want to 'migrate', we could change the request object to include a new property like
public class Request
{
public virtual bool securityVersionBeta {get; set;} = false;
}
public class GetAmazingDataRequest : Request
{
public virtual bool securityVersionBeta {get; set;} = true;
}
And then I can hijack the existing OnActionExecuting to include a new check:
public override void OnActionExecuting(ActionExecutingContext context)
{
if (((GetAmazingDataRequest )context.ActionArguments["Request"]).securityVersionBeta)
{
newProfileLogic(context);
} else
{
Originalcode(context);
}
Questions
Do you see anything glaring bad / wrong about this approach? Is there another approach that would allow us to migrate our code in a piecemeal fashion? Any other suggestions would also be appreciated.