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> </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> </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?
