patterncsharpMinor
Listing users in Azure Active Directory
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";
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
While looking at this
I thought what is the sense of getting the
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
the first return is superfluous and does not add any value, hence it should be removed.
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.