ViewModelBase & Per-Instance Edit/Delete Authorization Rules

ViewModelBase & Per-Instance Edit/Delete Authorization Rules

Old forum URL: forums.lhotka.net/forums/t/10544.aspx


Peran posted on Thursday, July 21, 2011

I have added some instance authorization rules that allow Edit & Delete based on the state of a BusinessBase object. A shorthand example:

 

protected override void AddBusinessRules()
{
    this.BusinessRules.AddRule(new MyEditRule(AuthorizationActions.EditObject));
    this.BusinessRules.AddRule(new MyDeleteRule(AuthorizationActions.DeleteObject));
}

private class MyEditRule : Csla.Rules.AuthorizationRule
{
        public MyEditRule()  : base(AuthorizationActions.EditObject)
        {
        } 

    protected override void Execute(AuthorizationContext context)
    {

        var target = (MyEditObject)context.Target;

        if (target != null)
        {
            context.HasPermission = target.Status != ObjectStatus.Locked;
        }
    }
}

 

I found that ViewModelBase.SetProperties() only took account of the Per-Type rules and my Save & Delete buttons were still enabled when the state of the object dictated that they should have been disabled. This would be a suggested changed to SetProperties()

 

 

            // Does Model instance implement ITrackStatus 

            if (targetObject != null)

            {

                var canEditInstance = Csla.Rules.BusinessRules.HasPermission(Csla.Rules.AuthorizationActions.EditObject, targetObject);

                var canDeleteInstance = Csla.Rules.BusinessRules.HasPermission(Csla.Rules.AuthorizationActions.DeleteObject, targetObject);

                CanSave = CanEditObject && targetObject.IsSavable && !isObjectBusy && canEditInstance;

                CanCancel = CanEditObject && targetObject.IsDirty && !isObjectBusy;

                CanCreate = CanCreateObject && !targetObject.IsDirty && !isObjectBusy;

                CanDelete = CanDeleteObject && !isObjectBusy && canDeleteInstance;

                CanFetch = CanGetObject && !targetObject.IsDirty && !isObjectBusy;

I also found I needed a new property ViewModelBase.CanEdit, that could be bound to my control IsEnabled property, so that all the UI text boxes could be disabled in one hit.  ViewModelBase.CanSave can not be used because it is False when a fetched object that is not dirty (but editable) is displayed, therefore disabling the UI text boxes.  This would also be updated in ViewModelBase.SetProperties()

                CanEdit = CanEditObject && !isObjectBusy && canEditInstance;

On another note I did notice is that MyEditRule.Execute() is called multiple times regardless of the value of MyEditRule.CacheResult.

Would this be a sensible change?

 

Regards

 

Peran

JonnyBee replied on Thursday, July 21, 2011

Hmmmmmm....

Could this have something to do with events that ViewModelBase hook into (or missing an event) or that you should also add Authz rules for CreateObject? .

At first glance ViewModelBase only checks the Per-Type Authz rules but IsSavable checks the Per-Instance rules.
Please note -  as implemented in Csla you should probably add the MyEditRule for both CreateObject and EditObject actions.

CanDelete is (IMO) coded to map in the same way as DataPortal.Delete (immediate deletion) which does not know the actual object.
Deferred deletion occurs in Save and IsSavable will test for authorization.

Delete in BusinessBase does not check for type authz rules. I don't know why - maybe Rocky will provide his thougts?

Your per-instance Authz rules must also set CanCache to false (in order to refresh)

    public override bool CacheResult
    {
      get
      {
        return false;
      }
    }

And this should enforce the per-type authz rules in Csla.Core.BusinessBase.cs:

    public virtual bool IsSavable
    {
      get 
      {
        bool auth;
        if (IsDeleted)
          auth = Csla.Rules.BusinessRules.HasPermission(Rules.AuthorizationActions.DeleteObjectthis);
        else if (IsNew)
          auth = Csla.Rules.BusinessRules.HasPermission(Rules.AuthorizationActions.CreateObjectthis);
        else
          auth = Csla.Rules.BusinessRules.HasPermission(Rules.AuthorizationActions.EditObjectthis);
        return (auth && IsDirty && IsValid && !IsBusy); 
      }
    }

Peran replied on Thursday, July 21, 2011

Hi Jonny,

 

Thanks for your quick reply.

 

You are correct with CanSave, my additional check is not necessary as it is already called within IsSavable().  I was going by the ITrackStatus.IsSavable comment and did not think to check the implementation!

I think there is confusion (I get confused anyway!) because ViewModel.Delete/ViewModelBase.DoDelete call the (deferred deletion) Delete() method on an *instance*, whereas CanDelete is set on a (immediate deletion) *type* basis.  I think as ViewModel.Delete/ViewModelBase.DoDelete work on an instance, CanDelete should also be set based on that instance.  

ViewModel.cs:

    /// <summary>
    /// Marks the Model for deletion (if it is an
    /// editable root object).
    /// </summary>
    public virtual void Delete(object sender, ExecuteEventArgs e)    {      DoDelete();    }

ViewModelBase.cs:

        /// <summary>
        /// Marks the Model for deletion (if it is an
        /// editable root object).
        /// </summary>
        protected virtual void DoDelete()        {            ((Csla.Core.IEditableBusinessObject)Model).Delete();        }

I will pull out your latest version of ViewModelBase from svn and that will allow me to override CanDelete as I think appropriate.

 

With regard to CacheResult, I left this as the default True value in my auth rule (the rule is based on a state that does not change after it has been pulled from the database).  I did notice that AuthorizationRule.Execute was being called each time ViewModelBase.SetProperties (and therefore  ITrackStatus.IsSavable) was called even though CacheResult was true.  I'm not sure if this is by design, but I was not expecting it, so I though it was worth mentioning.

 

 

Regards

 

Peran

JonnyBee replied on Thursday, July 21, 2011

http://www.lhotka.net/cslabugs/edit_bug.aspx?id=940Yes, I had to look more deeply into it here too.

The Authorization caching is actually builtin to the

so each time you call BusinessRules.HasPermission the rules are always executed (no matter what CacheResult says).

I agree - It's easy to get confused about the immediate/deferred deletion handling.
Maybe it would be appropriate to have separate CanDeleteXYZ flags for each type of Delete or follow your code when Model is set?

Anyway - you can now easily override this yourself , so at least there is a workaround/extension point available.

I do agree that Csla should test for instance rules on CanDelete in ViewModelBase.

Added to Issue tracker: http://www.lhotka.net/cslabugs/edit_bug.aspx?id=940

Peran replied on Thursday, July 21, 2011

Thanks.

 

By the way:

I have download the latest version of ViewModelBase from svn.  Line 796 is calling SetProperties() when I think it should be calling *On*SetProperties()

 

Regards

 

Peran

 

Peran replied on Thursday, July 21, 2011

Hi Jonny,

2 more things...

 

The setters for the virtualized CanXXX properties should be changed from private to protected so the value can be updated from a derived class.

 

(Cheeky one :-) ) Now that I have you thinking CanDelete should be set based on the instance, you may like to reconsider also taking instance.IsNew into account, as you can't really delete an object that IsNew.

 

                CanDelete = CanDeleteObject && !isObjectBusy && !targetObject.IsNew
                                        && Csla.Rules.BusinessRules.HasPermission(Csla.Rules.AuthorizationActions.DeleteObject, targetObject);

Cheers
Peran

Copyright (c) Marimer LLC