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 might 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\$
1

1 Answer 1

1
\$\begingroup\$

If I'm reading things right, then a simple solution would be to change the parameter of your Index action method from (string roleName) to (UserDataResponse model). The model binder will stuff your Request.RoleName query string parameter value into model.Request.RoleName. Perhaps, it would be better to rename your UserDataResponse class to AdminIndexViewModel, for example.

I find that the MVC HTML "Helper" methods often get in the way and make things harder than they need to be. I think what you really wanted was just a radio button named roleName rather than one named Request.RoleName, which is what you got with your @Html.RadioButtonFor() code.

\$\endgroup\$

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.