HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Separation of concerns for security checks

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
checksconcernssecurityforseparation

Problem

I needed a nice typesafe way to implement security checks for various domain objects and actions that can be performed on them.

I had an idea how to do it, so I implemented it as a simple proof of concept.

The core is the ActionSelector class that receives two objects, securityChecker that checks for security, and endpointFactory which is used to create real endpoint for action.
Both securityChecker and endpointFactory implement generic interface which defines a list of possible actions.
securityChecker implementation specifies T as SecurityCheckResult and endpointFactory specifies T as HtmlEndpoint.
ActionSelector takes in same generic interface, but with T as an object. If the securityChecker passes, the result from endpointFactory is returned. If it does not pass, it uses disabledEndpointFactory to return disabled endpoint representation.

```
public class ActionSelector
{
private readonly Action initalize;
private readonly TCommonInterface securityChecker;
private readonly TCommonInterface endpointFactory;
private readonly IDisabledEndpointFactory disabledEndpointFactory;

public ActionSelector(
IDisabledEndpointFactory disabledEndpointFactory,
TCommonInterface securityChecker,
TCommonInterface endpointFactory,
Action initialize)
{
this.initalize = initialize;
this.disabledEndpointFactory = disabledEndpointFactory;
this.securityChecker = securityChecker;
this.endpointFactory = endpointFactory;
}

public bool IsActionAllowed(Func> methodSelector)
{
this.initalize(securityChecker);
var performSecurityCheck = methodSelector(securityChecker);
return ((SecurityCheckResult) performSecurityCheck()).Allowed;
}

public TFormat Action(Func> methodSelector)
{
if (IsActionAllowed(methodSelector))
{
this.initalize(this.endpointFactory);
var generateActionInvocation = me

Solution

General

If you have something like

if (condition)
{
    return something;
}
else
{
   some more code
}


the else is not necessary.

So this

public TFormat Action(Func> methodSelector)
{
    if (IsActionAllowed(methodSelector))
    {
        this.initalize(this.endpointFactory);
        var generateActionInvocation = methodSelector(this.endpointFactory);
        return (TFormat) generateActionInvocation();
    }
    else
    {
        return disabledEndpointFactory.Create();
    }
}


should be

public TFormat Action(Func> methodSelector)
{
    if (IsActionAllowed(methodSelector))
    {
        this.initalize(this.endpointFactory);
        var generateActionInvocation = methodSelector(this.endpointFactory);
        return (TFormat) generateActionInvocation();
    }

    return disabledEndpointFactory.Create();
}


SecurityCheckResult

IMHO, this class should be sealed and also the constructor should be private. The class has only two states Allowed || !Allowed which already are represented by the static fields Ok and Fail.

Code Snippets

if (condition)
{
    return something;
}
else
{
   some more code
}
public TFormat Action(Func<TCommonInterface, Func<object>> methodSelector)
{
    if (IsActionAllowed(methodSelector))
    {
        this.initalize(this.endpointFactory);
        var generateActionInvocation = methodSelector(this.endpointFactory);
        return (TFormat) generateActionInvocation();
    }
    else
    {
        return disabledEndpointFactory.Create();
    }
}
public TFormat Action(Func<TCommonInterface, Func<object>> methodSelector)
{
    if (IsActionAllowed(methodSelector))
    {
        this.initalize(this.endpointFactory);
        var generateActionInvocation = methodSelector(this.endpointFactory);
        return (TFormat) generateActionInvocation();
    }

    return disabledEndpointFactory.Create();
}

Context

StackExchange Code Review Q#62256, answer score: 2

Revisions (0)

No revisions yet.