Custom policy removes validation of security configuration

[marcelopetersen]

Hi,

I've created a custom policy, and when I configure it on fluent security, validations of missing configuration stop to work (I didn't setup configuration.Advanced.IgnoreMissingConfiguration()).

-> My custom policy:

public class PasswordExpiredPolicy : ISecurityPolicy
    {
        public PolicyResult Enforce(ISecurityContext context)
        {
            return (SessionManager.UserPasswordIsExpired() ?
                PolicyResult.CreateFailureResult(this, "User need to change password!") :
                PolicyResult.CreateSuccessResult(this));
        }
    }

-> Configuration:

configuration.ForAllControllers().AddPolicy(new PasswordExpiredPolicy());

Trying to access an action missing configuration, the behavior is:

-> Before add custom policy

Security has not been configured for controller AppTest.Controllers.TestsController, action Index
Area: (not set)
Controller: Tests
Action: Index

-> After add my custom policy Contents of specified view is shown.

I'm doing something wrong to use custom policies? The version of fluent security which I'm using is 2.1.0

Regards

[marcelopetersen]

Its possible to have more than one policy configured by controller?

Looking on source code, I noted that is not possible to configure DenyAnonymousAccess and RequireAllRoles on the same controller (always only one of them will be executed and the last wins).

The policy RequireAllRoles verify if user is authenticated, and if not, return error, but don't throws DenyAnonymousAccessException. It's not possible to redirect user to login page if it's not authenticated without override the RequireAllRoles handler and do this validation again.

Probably the issue with missing configuration and custom policy describeb before is that.

[kristofferahl]

If a controller action get's a policy applied to it, it should no longer display the message "Security has not been configured for controller...". It does not matter what policy you apply to it, it should never warn once it has a policy.

In regards to DenyAnonymousAccess and RequireAllRoles, yes it is true. You can only have one of them. This is because the DefaultPolicyAppender clears all other policies when applying any of the policies shipped with FluentSecurity. You can create a custom IPolicyAppender to change this behavior and then apply it like this.

configuration.PolicyAppender = new MyCustomPolicyAppender();

You are also correct in that the RequireAllRoles policy first check if the user is authenticated before checking roles. This is because it is a very common case that you want to do both. So you should not have to apply both DenyAnonymousAccessPolicy and RequireAllRolesPolicy to get this behavior. Does this make sense?

I hope this explains things for you? I will leave this issue open for a while and then close it if you don't have objections.

[marcelopetersen]

Hi, thanks for your answer.

I'll check how to create a custom appender because is a very bad thing lose this validation.

About DenyAnonymousAccessPolicy and RequireAllRolesPolicy, I think that exists a point: when the user is not authenticated, and is applied only the DenyAnonymousAccessPolicy, the behavior will be redirect user to login page. But when I check for user roles, if the user is not authenticated, is thrown the exception "Anonymous access denied" instead redirect user.

In my opinion, it's a strange behavior once I have a policy to handle anonymous access. The RequireAllRolesPolicy needs to check if the user is authenticated, but if not, it should delegate the validation to DenyAnonymousAccessPolicy. We have two different behaviors to same validation.

What do you think about that?

[kristofferahl]

I agree with you that it can be considered a strange behavior and it might be something we can fix for FluentSecurity 3.0 and onwards. But for legacy reasons we can't change this behavior without breaking older implementations. I'll include this change as something we should do for FluentSecurity 3.0.

[marcelopetersen]

OK Kristoffer ... thanks a lot !!!

[kristofferahl]

I'll leave it open for now as a reminder that this is something we should have a look at. Thanks!