OData Delta Patch Security

Question

I have a working PATCH for my user class with Delta in Web API 2. By using the .patch method I can easily detect only the changes that were sent over and then update accordingly, rather than have to receive the entire user!

The problem is there are several fields that I want to protect so they are never updated.

I saw one example on SO but it didn't leverage Delta rather seemed to be slightly more dated and practically wrote all of the patch code by hand. Is there not a way to easily tell OData's patch to skip over properties you designate (maybe I need to override patch and tell it to avoid some properties)?

How would I even begin to go about doing this (or what should I search for / research to get started)? Do action filters / validation have a role here? Do I look into model binding? Is it overriding patch?

Thanks!

Answer 1

There's a couple of possibilities, depending on you use case...

1. You want to exclude the changes if they are supplied
2. You want to throw an error if non-editable fields are updated.

Start with an attribute to mark appropriate properties

/// <summary>
/// Marks a property as non-editable.
/// </summary>
[AttributeUsage(AttributeTargets.Property, AllowMultiple = false, Inherited = true)]
public class NonEditableAttribute : Attribute
{
}

Then we can write some extensions against Delta to take advantage of this

public static class PatchExtensions
{
    /// <summary>
    /// Get the properties of a type that are non-editable.
    /// </summary>
    /// <param name="type"></param>
    /// <returns></returns>
    public static IList<string> NonEditableProperties(this Type type)
    {
        return type.GetProperties().Where(x => Attribute.IsDefined(x, typeof(NonEditableAttribute))).Select(prop => prop.Name).ToList();
    }

    /// <summary>
    /// Get this list of non-editable changes in a <see cref="Delta{T}"/>.
    /// </summary>
    /// <typeparam name="T"></typeparam>
    /// <param name="delta"></param>
    /// <returns></returns>
    public static IList<string> NonEditableChanges<T>(this Delta<T> delta)
        where T : class
    {
        var nec = new List<string>();
        var excluded = typeof(T).NonEditableProperties();

        nec.AddRange(delta.GetChangedPropertyNames().Where(x => excluded.Contains(x)));

        return nec;
    }

    /// <summary>
    /// Exclude changes from a <see cref="Delta{T}"/> based on a list of property names
    /// </summary>
    /// <typeparam name="T"></typeparam>
    /// <param name="delta"></param>
    /// <param name="excluded"></param>
    /// <returns></returns>
    public static Delta<T> Exclude<T>(this Delta<T> delta, IList<string> excluded)
        where T : class
    {
        var changed = new Delta<T>();

        foreach (var prop in delta.GetChangedPropertyNames().Where(x => !excluded.Contains(x)))
        {
            object value;
            if (delta.TryGetPropertyValue(prop, out value))
            {
                changed.TrySetPropertyValue(prop, value);
            }
        }

        return changed;
    }

    /// <summary>
    /// Exclude changes from a <see cref="Delta{T}"/> where the properties are marked with <see cref="NonEditableAttribute"/>
    /// </summary>
    /// <typeparam name="T"></typeparam>
    /// <param name="delta"></param>
    /// <returns></returns>
    public static Delta<T> ExcludeNonEditable<T>(this Delta<T> delta)
        where T : class
    {
        var excluded = typeof(T).NonEditableProperties();
        return delta.Exclude(excluded);
    }
}

And a domain class

public class Customer 
{
    public int Id { get; set; }

    public string Name { get; set; }

    [NonEditable]
    public string SecurityId { get; set; }
}

Finally your controller can then take advantage of this in the Patch method

public async Task<IHttpActionResult> Patch([FromODataUri] int key, Delta<Customer> delta)
{
    var patch = delta.ExcludeNonEditable();

    // TODO: Your patching action here
}

or

public async Task<IHttpActionResult> Patch([FromODataUri] int key, Delta<Customer> delta)
{
    var nonEditable = delta.NonEditableChanges();
    if (nonEditable.Count > 0)
    {
        throw new HttpException(409, "Cannot update as non-editable fields included");
    }

    // TODO: Your patching action here
}

Answer 2

Depending on what you want to do if someone tries to update protected fields you can either:

1. Update only fields that can be modified. For this you can construct new Delta with only these fields like this:

   Delta<User> filteredDelta = new Delta<User>();
   if (originalDelta.GetChangedPropertyNames().Contains("FirstName"))
   {
       filteredDelta.TrySetPropertyValue("FirstName", originalDelta.GetEntity().FirstName);
   }
   if (originalDelta.GetChangedPropertyNames().Contains("LastName"))
   {
       filteredDelta.TrySetPropertyValue("LastName", originalDelta.GetEntity().LastName);
   }    
   filteredDelta.Patch(selectedUser);
   

2. Fail the PATCH request (I would say this is preferred and least surprising way to deal with such requests). This would be even simpler:

   if (originalDelta.GetChangedPropertyNames().Contains("ModifiedDate"))
   {
       return InternalServerError(new ArgumentException("Attribue is read-only", "ModifiedDate"));
   }
   

Answer 3

I had the same need and I ended up writing an extension method to Delta that accepts additional parameters to limit which fields to update (similar to TryUpDateModel)

I know there must be a better way to do this, but for now this works.

I had to recreate some of the Delta private methods and classes. Most of the code is from https://github.com/mono/aspnetwebstack/blob/master/src/System.Web.Http.OData/OData/Delta.cs, https://github.com/ASP-NET-MVC/aspnetwebstack/blob/master/OData/src/System.Web.Http.OData/OData/PropertyAccessor.cs and https://github.com/mono/aspnetwebstack/blob/master/src/System.Web.Http.OData/OData/CompiledPropertyAccessor.cs (or similar, these are not the exact url's I copied from)

using System;
using System.Collections.Generic;
using System.Dynamic;
using System.Linq;
using System.Web;
using System.Reflection;
using System.Linq.Expressions;

namespace MyProject.ODataExtensions
{
public static class ODataExtensions
{

    public static void Patch<TEntityType>(this System.Web.OData.Delta<TEntityType> d, TEntityType original, String[] includedProperties, String[] excludedProperties) where TEntityType : class
    {
        Dictionary<string, PropertyAccessor<TEntityType>> _propertiesThatExist = InitializePropertiesThatExist<TEntityType>();

        var changedProperties = d.GetChangedPropertyNames();
        if (includedProperties != null) changedProperties = changedProperties.Intersect(includedProperties);
        if (excludedProperties != null) changedProperties = changedProperties.Except(excludedProperties);

        PropertyAccessor<TEntityType>[] array = (
                from s in changedProperties
                select _propertiesThatExist[s]).ToArray();

        var array2 = array;

        for (int i = 0; i < array2.Length; i++)
        {
            PropertyAccessor<TEntityType> propertyAccessor = array2[i];
            propertyAccessor.Copy(d.GetEntity(), original);
        }

    }

    private static Dictionary<string, PropertyAccessor<T>> InitializePropertiesThatExist<T>() where T : class
    {
        Type backingType = typeof(T);
        return backingType.GetProperties()
                            .Where(p => p.GetSetMethod() != null && p.GetGetMethod() != null)
                            .Select<PropertyInfo, PropertyAccessor<T>>(p => new CompiledPropertyAccessor<T>(p))
                            .ToDictionary(p => p.Property.Name);
    }

    internal abstract class PropertyAccessor<TEntityType> where TEntityType : class
    {
        protected PropertyAccessor(PropertyInfo property)
        {
            if (property == null)
            {
                throw new System.ArgumentException("Property cannot be null","property");
            }
            Property = property;
            if (Property.GetGetMethod() == null || Property.GetSetMethod() == null)
            {
                throw new System.ArgumentException("Property must have public setter and getter", "property");
            }
        }

        public PropertyInfo Property
        {
            get;
            private set;
        }

        public void Copy(TEntityType from, TEntityType to)
        {
            if (from == null)
            {
                throw new System.ArgumentException("Argument cannot be null", "from");
            }
            if (to == null)
            {
                throw new System.ArgumentException("Argument cannot be null", "to");
            }
            SetValue(to, GetValue(from));
        }

        public abstract object GetValue(TEntityType entity);

        public abstract void SetValue(TEntityType entity, object value);
    }

    internal class CompiledPropertyAccessor<TEntityType> : PropertyAccessor<TEntityType> where TEntityType : class
    {
        private Action<TEntityType, object> _setter;
        private Func<TEntityType, object> _getter;

        public CompiledPropertyAccessor(PropertyInfo property)
            : base(property)
        {
            _setter = MakeSetter(Property);
            _getter = MakeGetter(Property);
        }

        public override object GetValue(TEntityType entity)
        {
            if (entity == null)
            {
                throw new ArgumentNullException("entity");
            }
            return _getter(entity);
        }

        public override void SetValue(TEntityType entity, object value)
        {
            if (entity == null)
            {
                throw new ArgumentNullException("entity");
            }

            _setter(entity, value);
        }

        private static Action<TEntityType, object> MakeSetter(PropertyInfo property)
        {
            Type type = typeof(TEntityType);
            ParameterExpression entityParameter = Expression.Parameter(type);
            ParameterExpression objectParameter = Expression.Parameter(typeof(Object));
            MemberExpression toProperty = Expression.Property(Expression.TypeAs(entityParameter, property.DeclaringType), property);
            UnaryExpression fromValue = Expression.Convert(objectParameter, property.PropertyType);
            BinaryExpression assignment = Expression.Assign(toProperty, fromValue);
            Expression<Action<TEntityType, object>> lambda = Expression.Lambda<Action<TEntityType, object>>(assignment, entityParameter, objectParameter);
            return lambda.Compile();
        }

        private static Func<TEntityType, object> MakeGetter(PropertyInfo property)
        {
            Type type = typeof(TEntityType);
            ParameterExpression entityParameter = Expression.Parameter(type);
            MemberExpression fromProperty = Expression.Property(Expression.TypeAs(entityParameter, property.DeclaringType), property);
            UnaryExpression convert = Expression.Convert(fromProperty, typeof(Object));
            Expression<Func<TEntityType, object>> lambda = Expression.Lambda<Func<TEntityType, object>>(convert, entityParameter);
            return lambda.Compile();
        }
    }
}

}