2
\$\begingroup\$

Background

One of the projects I'm working on is an Intranet website which is a front-end for several kinds of data, each aimed at a different set of users. Access to the website is handled through ADFS; users are put into one or more Active Directory groups and it is this membership that determines what pages they get access to.

The website was an afterthought to a larger project, and while usually we build Angular SPAs with an ASP.NET Web API back-end, it was decided that this should be a fairly simple ASP.NET MVC website.

User management

Managing Active Directory groups using the usual Windows tools is a rather cumbersome excperience, and thus we usually implement an Admin-only management page in our projects where we can easily search for users and add them to or remove them from the various Active Directory groups. We usually copy-paste these implementations and change the necessary variables (group names etc.), but that couldn't be done now since the front-end is in Angular (the code of the ASP.NET Web API back-end could largely be re-used).

And thus I quickly whipped up a simple implementation in ASP.NET MVC. However, while it does work, I'm not happy with one part of the implementation. I suspect I'm not well-versed enough in ASP.NET MVC, and thus I'm hoping someone here might come up with a better solution.

User

public class User
{
    public User(string userName, string displayName)
    {
        UserName = userName;
        DisplayName = displayName;
    }

    public string UserName { get; set; }

    public string DisplayName { get; set; }
}

Role

public class Role
{
    public Role(string name, string shortName)
    {
        Name = name;
        ShortName = shortName;
        Users = new List<User>();
    }

    public string Name { get; set; }

    public string ShortName { get; set; }

    public List<User> Users { get; set; }

    public static string GetShortName(string name, string domainAndPrefix)
    {
        return name.Replace(domainAndPrefix, string.Empty);
    }

    public static string GetProperName(string name, string domainAndPrefix)
    {
        if (name.StartsWith(domainAndPrefix))
        {
            return name;
        }

        return domainAndPrefix + name;
    }
}

ActiveDirectory

public static class ActiveDirectory
{
    public const string Domain = "MyDomain";
    public const string Prefix = "MyProject-";
    public const string DomainAndPrefix = Domain + @"\" + Prefix;

    public static class Role
    {
        public const string Administrators = DomainAndPrefix + "Administrators";
        public const string Readers = DomainAndPrefix + "Readers";
        public const string Editors = DomainAndPrefix + "Editors";

        public static string[] All = { Administrators, Readers, Editors };
    }

    public static class RoleGroup
    {
        public const string Admins = Role.Administrators;
        public const string ReadersAndAdmins = Role.Readers + "," + Role.Administrators;
        public const string EditorsAndAdmins = Role.Editors + "," + Role.Administrators;
    }
}

ISecurityFacade

public interface ISecurityFacade
{
    UserDataResponse GetAll(UserDataRequest request);
}

SecurityFacade

public class SecurityFacade : ISecurityFacade
{
    public UserDataResponse GetAll(UserDataRequest request)
    {
        var roles = ActiveDirectory.Role.All
            .Select(GetRole)
            .OrderBy(x => x.ShortName)
            .ToList();

        if (string.IsNullOrEmpty(request.RoleName))
        {
            request.RoleName = roles.First().ShortName;
        }

        var response = new UserDataResponse(request)
        {
            Roles = roles
        };

        return response;
    }

    private Role GetRole(string group)
    {
        return new Role(group, Role.GetShortName(group, ActiveDirectory.DomainAndPrefix))
        {
            Users = UsersRetriever.GetUsersInRole(group)
        };
    }
}

AdminController

public class AdminController : AdfsController
{
    private readonly ISecurityFacade _securityFacade;

    public AdminController()
    {
        _securityFacade = new SecurityFacade();
    }

    [Authorize(Roles = ActiveDirectory.RoleGroup.Admins)]
    [HttpGet]
    public ActionResult Index(string roleName)
    {
        ViewBag.Title = "Users & roles";

        if (string.IsNullOrEmpty(roleName))
        {
            // hacky solution that is necessary due to data binding
            roleName = Request.QueryString["Request.RoleName"];

            if (string.IsNullOrEmpty(roleName))
            {
                roleName = Role.GetShortName(ActiveDirectory.Role.Administrators, ActiveDirectory.DomainAndPrefix);
            }
        }

        var request = new UserDataRequest { RoleName = roleName };

        var response = _securityFacade.GetAll(request);

        return View(response);
    }
}

AdfsController

public class AdfsController : Controller
{
    protected override void OnAuthentication(AuthenticationContext filterContext)
    {
        base.OnAuthentication(filterContext);

        SetCurrentUser(filterContext.Principal);
    }

    private void SetCurrentUser(IPrincipal principal)
    {
        var claimsIdentity = (ClaimsIdentity)((ClaimsPrincipal)principal).Identity;

        SetNameIdentifier(claimsIdentity);
        SetRoles(claimsIdentity);
    }

    private void SetNameIdentifier(ClaimsIdentity claimsIdentity)
    {
        var name = claimsIdentity.Claims.Single(x => x.Type == ClaimTypes.Name);
        var nameIdentifier = claimsIdentity.Claims.SingleOrDefault(x => x.Type == ClaimTypes.NameIdentifier);

        if (string.IsNullOrEmpty(nameIdentifier?.Value))
        {
            claimsIdentity.AddClaim(new Claim(ClaimTypes.NameIdentifier, name.Value));
        }
    }

    private void SetRoles(ClaimsIdentity claimsIdentity)
    {
        var roles = User.Identity.GetEliseForwardRoles();

        foreach (var role in roles)
        {
            claimsIdentity.AddClaim(new Claim(ClaimTypes.Role, role));
        }
    }
}

UserDataRequest

public class UserDataRequest
{
    public string RoleName { get; set; }

    public int Page { get; set; }

    public int ItemsPerPage { get; set; }
}

UserDataResponse

public class UserDataResponse
{
    public UserDataResponse(UserDataRequest request)
    {
        Request = request;
    }

    public UserDataRequest Request { get; }

    public List<Role> Roles { get; set; }

    public List<User> Users
    {
        get { return Roles.Single(x => x.ShortName == Request.RoleName).Users; }
    }

    public int Count { get; set; }
}

Views / Admin / Index.cshtml

@model UserDataResponse

<h2>@ViewBag.Title</h2>

<div>
    @using (Html.BeginForm("Index", "Admin", FormMethod.Get))
    {
        foreach (var role in Model.Roles)
        {
            @Html.RadioButtonFor(x => x.Request.RoleName, role.ShortName) @Html.Label(role.ShortName) <span>&nbsp;</span>
        }

        <input type="submit" name="submit" value="Filter" class="btn btn-default" />
    }
</div>

<div>
    <table class="table table-striped table-bordered table-hover">
        <thead>
            <tr>
                <th>Name</th>
                <th>Login</th>
                <th>&nbsp;</th>
            </tr>
        </thead>

        <tbody>
            @if (Model.Users.Any())
            {
                foreach (var user in Model.Users)
                {
                    <tr>
                        <td>@Html.DisplayFor(modelItem => user.DisplayName)</td>
                        <td>@Html.DisplayFor(modelItem => user.UserName)</td>
                        <td>@Html.ActionLink("Remove", "Remove", new { userName = user.UserName, displayName = user.DisplayName, roleName = Model.Request.RoleName })</td>
                    </tr>
                }
            }
            else
            {
                <tr>
                    <td colspan="3">Found no results.</td>
                </tr>
            }
        </tbody>

        <tfoot>
            <tr>
                <td colspan="3" class="pager">
                    @Html.Pager(Model.Request.ItemsPerPage, Model.Request.Page, Model.Count).Options(x =>
                             x.AddRouteValue("Request.RoleName", Model.Request.RoleName)
                                 .DisplayFirstAndLastPage()
                                 .SetFirstPageText("First")
                                 .SetLastPageText("Last")
                                 .SetPreviousPageText("Prev")
                                 .SetNextPageText("Next")
                             )
                </td>
            </tr>
        </tfoot>
    </table>
</div>

<div>
    @Html.ActionLink("Add user", "AddUser", new { roleName = Model.Request.RoleName })
</div>

What I am unhappy with

As you can guess, my main issue is with public ActionResult Index(string roleName): the parameter roleName is actually never filled in and its value is actually in a querystring parameter called "Request.RoleName". Of course, when you first enter the page there is no value and thus a default one is assigned (that way you don't end up with a blank page):

roleName = Role.GetShortName(ActiveDirectory.Role.Administrators, ActiveDirectory.DomainAndPrefix);

To me the whole of this feels "not right":

if (string.IsNullOrEmpty(roleName))
{
    // hacky solution that is necessary due to data binding
    roleName = Request.QueryString["Request.RoleName"];

    if (string.IsNullOrEmpty(roleName))
    {
        roleName = Role.GetShortName(ActiveDirectory.Role.Administrators, ActiveDirectory.DomainAndPrefix);
    }
}

var request = new UserDataRequest { RoleName = roleName };

Where did I go wrong? Is there a more elegant way to bind the radio buttons in the View to the code in the AdminController?

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.