HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Listing users in Azure Active Directory

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
directoryactivelistingazureusers

Problem

The code below is the Index Action from a User Controllers. Basically this code returns a list of users from Azure Active Directory using Azure Authentication Library (ADAL).

However I need to show 2 custom properties, in Azure AD, they are called Schema Extensions, they are just custom attributes where I saved custom data in a comma delimited format. On the first column I have the IDs of the companies the user works for, and on the second column I have the IDs of the Modules the user has access to.

The index controller action is easy:

```
public async Task Index()
{
var userList = new List();
try
{
var client = AuthenticationHelper.GetActiveDirectoryClient();
var pagedCollection = await client.Users.ExecuteAsync();
while (pagedCollection != null)
{
var usersList = pagedCollection.CurrentPage.ToList();
userList.AddRange(usersList.Cast());
pagedCollection = await pagedCollection.GetNextPageAsync();
}
}
catch (Exception)
{
if (Request.QueryString["reauth"] == "True")
{
//
// Send an OpenID Connect sign-in request to get a new set of tokens.
// If the user still has a valid session with Azure AD, they will not be prompted for their credentials.
// The OpenID Connect middleware will return to this controller after the sign-in response has been handled.
//
HttpContext.GetOwinContext()
.Authentication.Challenge(OpenIdConnectAuthenticationDefaults.AuthenticationType);
}

//
// The user needs to re-authorize. Show them a message to that effect.
//
ViewBag.ErrorMessage = "AuthorizationRequired";

Solution

Talking about public async Task Index()

While looking at this

while (pagedCollection != null)
{
    var usersList = pagedCollection.CurrentPage.ToList();
    userList.AddRange(usersList.Cast());
    pagedCollection = await pagedCollection.GetNextPageAsync();
}


I thought what is the sense of getting the userList and then calling AddRange() passing itself as parameter. After looking more closely I could spot the tiny little difference between usersList and userList.

Because it happened to me, it will also happen for Sam the maintainer of this code. Sam would be happy if one of these variables would get a different name so it could be seen at first glance that these variables are different.

Instead of comparing Request.QueryString["reauth"] to the string "True" you should take advantage of the bool.TrueString property like so

if (Request.QueryString["reauth"] == bool.TrueString)


return View(userList);
    }
    return View(userList);


the first return is superfluous and does not add any value, hence it should be removed.

Code Snippets

while (pagedCollection != null)
{
    var usersList = pagedCollection.CurrentPage.ToList();
    userList.AddRange(usersList.Cast<Microsoft.Azure.ActiveDirectory.GraphClient.User>());
    pagedCollection = await pagedCollection.GetNextPageAsync();
}
if (Request.QueryString["reauth"] == bool.TrueString)
return View(userList);
    }
    return View(userList);

Context

StackExchange Code Review Q#99142, answer score: 4

Revisions (0)

No revisions yet.