ViewComponent => User as ClaimsPrincipal

[wangkanai]

HomeController inherit ControllerBase gives User with ClaimsPrincipal

Controller : ControllerBase {
    public ClaimsPrincipal User { get; }
}

DemoViewComponent inherit ViewComponent give User with IPrincipal

ViewComponent {
    public IPrincipal User { get; }
}

Is there any reason for such implementation?

[rynowak]

The only reason I can think of is that HttpContext.User used to be IPrincipal and is now ClaimsPrincipal. It looks like controller was updated when that changed and view component was not.

[wangkanai]

This is how I overcome the difficulty right now.

public class OwnerViewComponent : ViewComponent
{
    public IViewComponentResult Invoke()
    {
        var user = User as ClaimsPrincipal;
        if (user == null) return null;
        var owner = new Owner
        {
            NameIdentifier = user.FindFirst(ClaimTypes.NameIdentifier)?.Value,
            Name = user.FindFirst(ClaimTypes.Name)?.Value,
            GivenName = user.FindFirst(ClaimTypes.GivenName)?.Value,
            Surname = user.FindFirst(ClaimTypes.Surname)?.Value
        };
        return View(owner);
    }
}
public class Owner
{
    public string NameIdentifier { get; set; }
    public string Name { get; set; }
    public string GivenName { get; set; }
    public string Surname { get; set; }
}

Would anything break if you guys change User in ViewComponent to inherit from ClaimsPrincipal?

[rynowak]

From https://github.com/dotnet/corefx/blob/master/Documentation/coding-guidelines/breaking-change-rules.md#behavioral-changes

Returning a value of a more derived type for a property, field, return or out value

Note, again, that the static type cannot change. e.g. it is OK to return a string instance where an object was returned previously, but it is not OK to change the return type from object to string.

This would be a binary breaking change

[tuespetre]

😨 oops!

Now I feel like an 🅰️⭕, I was working with someone on a ViewComponent a while back (maybe February?) and noticed this but I guess I forget to ever report it or anything.

[brockallen]

This would be a binary breaking change

Mark this as a TODO for 2.0 and yes, make the breaking change #semver.

[dealdiane]

This was always going to be a breaking change. It's been like this since at least RC1 (I know! I should've reported this earlier.. ). I think 2.0 is too far off so why not mark this as todo for like 1.1 or something?

[Eilon]

Assuming the right data all exists, let's add a new property to ViewComponent that returns the right data:

ViewComponent {
    public ClaimsPrincipal UserClaimsPrincipal { get; }
}

It's kind of lame to have two properties that return the same instance, but this is a good and easy solution.

[dazinator]

Consider marking User property as obsolete? This way people can switch over to the new ClaimsPrincipal user property gracefully, and then the older property can be removed from a future release?

[Eilon]

@dazinator we generally don't mark things as obsolete unless we really think they shouldn't be used, but in this case I think it's fine, no?