Hierarchies of Visitors modifying the behavior of parent. Is it fine with Liskov?

Question

There is a class IUser. It has a function which takes a Visitor and allows changes to the public properties.

public IUser
{
    public PermissionMatrix Permissions { get; set; }    

    public void Authorizations(IAuthManager manager)
    {
       manager.SetRoles(this);
    }
}

Now, it can be visited by class hierarchies of IAuthManager

public IAuthManager
{
   public void SetRoles(IUser user);
}

public InternalAuthManager : IAuthManager
{
   public virtual void SetRoles(IUser user)
   {
     // sets permissions in user for internal security
     // according to a complex logic
   }
}

public RestrictInternalAuthManager : InternalAuthManager
{
   public override void SetRoles(IUser user)
   {
     base.SetRoles(user); // need to use complex logic of parent
     // then reverts few permissions based on conditions
   }
}

I want to evaluate if class RestrictInternalAuthManager is violating Liskov Substitution Principle. I have been arguing for both yes and no,

No : There is no check for type of IAuthManager.

Yes : RestrictInternalAuthManager is changing the post-conditions of InternalAuthManager.

Can this be left as it is, or the classes require refactoring? Any help is appreciated.

Answer 1

Let Φ(x) be a property provable about objects x of type T. Then Φ(y) should be true for objects y of type S where S is a subtype of T.

Now the problem is that you haven't really specified the behavior of the setRoles method; if one would infer "it sets permissions", then you are not really changing behavior. If you would say, "this method does it in such and such way" and in the subclass you would change it, then yes, you would violate LSP. I.e. if you would say "get() retrieves an item" for both Queue and Stack, than it would be ok, but if you say "Queue.get() retrieves oldest item" and "Stack.get() retrieves newest item" than they are different.

The idea behind LSP is, that the expected behavior doesn't change, so for example if you were to write tests for base class, then all the tests must also pass if you provide the subclass instead.

Another important question is: am I subtyping, or subclassing? If I change the behavior of the superclass, would I need to make changes in the subclass? Am I merely reusing the implementation in the superclass out of convenience? If so, then it is a violation.

Think also about what would happen if you needed yet another manager that adds just one more thing; this could get out of hand very quickly. If you are unsure whether to inherit, it is often better to use decomposition instead.