Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I have a class called AzureActiveDirectoryController. This class has some methods: one to create users, one to get the app name, one to register schema extensions (AD custom properties, etc).

However, I have to repear the same piece of code on each method because I need an adClient instance for all operations.

I want to refactor this class to make it more readable and maintenable.

The lines I am talking about are this:

Uri serviceRoot = new Uri(azureAdGraphApiEndPoint);

ActiveDirectoryClient adClient = new ActiveDirectoryClient(
 serviceRoot,
 async () => await GetAppTokenAsync());

Application app = (Application)adClient.Applications.Where(
    a => a.AppId == clientId).ExecuteSingleAsync().Result;
if (app == null)
{
    throw new ApplicationException("Unable to get a reference to application in Azure AD.");
}

The complete controller class:

using Microsoft.Azure.ActiveDirectory.GraphClient;
using Microsoft.IdentityModel.Clients.ActiveDirectory;
using Microsoft.IdentityModel.Protocols;
using System;
using System.Collections.Generic;
using System.Configuration;
using System.Globalization;
using System.Linq;
using System.Net.Http;
using System.Threading.Tasks;
using System.Web;
using System.Web.Mvc;

namespace PruebasAD.Controllers
{
    public class ActiveDirectoryController : Controller
    {
        private static string azureAdGraphApiEndPoint = ConfigurationManager.AppSettings["ida:AzureAdGraphApiEndPoint"];
        private static string clientId = ConfigurationManager.AppSettings["ida:ClientId"];
        private static string appKey = ConfigurationManager.AppSettings["ida:AppKey"];

        // GET: ActiveDirectory
        public ActionResult GetAzureAadApp()
        {
            // Instantiate an instance of ActiveDirectoryClient.
            Uri serviceRoot = new Uri(azureAdGraphApiEndPoint);
            ActiveDirectoryClient adClient = new ActiveDirectoryClient(
                serviceRoot,
                async () => await GetAppTokenAsync());


            Application app =(Application)adClient.Applications.Where(
                    a => a.AppId == clientId).ExecuteSingleAsync().Result;

            if (app == null)
            {
                throw new ApplicationException("Unable to get a reference to application in Azure AD.");
            }

            PruebasAD.Models.Application application = new PruebasAD.Models.Application();
            application.ApplicationName = app.DisplayName;

            return View(application);
        }

        public ActionResult CreateProperty()
        {
            // Create the extension property
            string extPropertyName = "CompInfo";
            ExtensionProperty extensionProperty = new ExtensionProperty()
            {
                Name = extPropertyName,
                DataType = "String",
                TargetObjects = { "User" }
            };
            Uri serviceRoot = new Uri(azureAdGraphApiEndPoint);

            ActiveDirectoryClient adClient = new ActiveDirectoryClient(
             serviceRoot,
             async () => await GetAppTokenAsync());

            Application app =(Application)adClient.Applications.Where(
                a => a.AppId == clientId).ExecuteSingleAsync().Result;
            if (app == null)
            {
                throw new ApplicationException("Unable to get a reference to application in Azure AD.");
            }

            // Register the extension property
            app.ExtensionProperties.Add(extensionProperty);
            app.UpdateAsync();
            Task.WaitAll();

            // Apply the change to Azure AD
            app.GetContext().SaveChanges();
            ViewBag.Message = "Extension Created.";

            return View();
        }

        public ActionResult CreateUser()
        {
            Uri serviceRoot = new Uri(azureAdGraphApiEndPoint);

            ActiveDirectoryClient adClient = new ActiveDirectoryClient(
             serviceRoot,
             async () => await GetAppTokenAsync());

            Application app = (Application)adClient.Applications.Where(
                a => a.AppId == clientId).ExecuteSingleAsync().Result;
            if (app == null)
            {
                throw new ApplicationException("Unable to get a reference to application in Azure AD.");
            }
            var newUser = new User()
            {
                // Required settings
                DisplayName = "Jay Hamlin",
                UserPrincipalName = "[email protected]",
                PasswordProfile = new PasswordProfile()
                {
                    Password = "H@ckMeNow!",
                    ForceChangePasswordNextLogin = false
                },
                MailNickname = "JayHamlin",
                AccountEnabled = true,

                // Some (not all) optional settings
                GivenName = "Jay",
                Surname = "Hamlin",
                JobTitle = "Programmer",
                Department = "Development",
                City = "Dallas",
                State = "TX",
                Mobile = "214-123-1234",
            };

            // Add the user to the directory
            adClient.Users.AddUserAsync(newUser).Wait();

            ViewBag.Message = "User Created.";

            return View();
        }

        public ActionResult TestRestCall()
        {
            Uri serviceRoot = new Uri(azureAdGraphApiEndPoint);

            ActiveDirectoryClient adClient = new ActiveDirectoryClient(
             serviceRoot,
             async () => await GetAppTokenAsync());

            Application app = (Application)adClient.Applications.Where(
                a => a.AppId == clientId).ExecuteSingleAsync().Result;
            if (app == null)
            {
                throw new ApplicationException("Unable to get a reference to application in Azure AD.");
            }

            HttpClient hc = new HttpClient();
            hc.DefaultRequestHeaders.Authorization = new System.Net.Http.Headers.AuthenticationHeaderValue(
                 "Bearer", );

        }
        public ActionResult GetProperties()
        {
            Uri serviceRoot = new Uri(azureAdGraphApiEndPoint);

            ActiveDirectoryClient adClient = new ActiveDirectoryClient(
             serviceRoot,
             async () => await GetAppTokenAsync());

            Application app = (Application)adClient.Applications.Where(
                a => a.AppId == clientId).ExecuteSingleAsync().Result;
            if (app == null)
            {
                throw new ApplicationException("Unable to get a reference to application in Azure AD.");
            }

            IEnumerable<IExtensionProperty> appExtProperties = app.ExtensionProperties;
            appExtProperties.ToList();

            return View(appExtProperties);
        }

        private static async Task<string> GetAppTokenAsync()
        {
            string clientId = ConfigurationManager.AppSettings["ida:ClientId"];
            string appKey = ConfigurationManager.AppSettings["ida:AppKey"];
            string aadInstance = ConfigurationManager.AppSettings["ida:AADInstance"];
            string tenant = ConfigurationManager.AppSettings["ida:Tenant"];
            string postLogoutRedirectUri = ConfigurationManager.AppSettings["ida:PostLogoutRedirectUri"];
            string azureAdGraphApiEndPoint = ConfigurationManager.AppSettings["ida:AzureAdGraphApiEndPoint"];
            // This is the resource ID of the AAD Graph API.  We'll need this to request a token to call the Graph API.
            string graphResourceId = ConfigurationManager.AppSettings["ida:GraphResourceId"];

            string Authority = String.Format(CultureInfo.InvariantCulture, aadInstance, tenant);

            // Instantiate an AuthenticationContext for my directory (see authString above).
            AuthenticationContext authenticationContext = new AuthenticationContext(Authority, false);

            // Create a ClientCredential that will be used for authentication.
            // This is where the Client ID and Key/Secret from the Azure Management Portal is used.
            ClientCredential clientCred = new ClientCredential(clientId, appKey);

            // Acquire an access token from Azure AD to access the Azure AD Graph (the resource)
            // using the Client ID and Key/Secret as credentials.
            AuthenticationResult authenticationResult = await authenticationContext.AcquireTokenAsync(graphResourceId, clientCred);

            // Return the access token.
            return authenticationResult.AccessToken;
        }
    }


    public class CompanyInfo
    {
        public int Nit;
        public string Nombre;
    }
}
share|improve this question
    
You've already identified the problem. Why not just extract out the common code into a private method? Asking for a review is tantamount to asking for the work to be done for you. – 200_success May 22 '15 at 23:51
    
you re wrong, if I extract the common code in a method, then I have to call that method anyways from each method and thats not what I want, that method is an expensive call to azure rest AAD Apis, I want some kinda of design pattern, where I can get the adClient once and use it everywhere. but I am not sure how? – Esteban V May 22 '15 at 23:57

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Browse other questions tagged or ask your own question.