Active Directory Query Application

Question

This application is designed to query an active directory, and at the moment, performs only two tasks:

1. Save a list of all users to a file.
2. Save a list of all groups that all users are in to a file.

I tried to implement a print all groups method, but ended up removing it. I believe I removed all references to it from this code, but if you see one, please ignore (or fuss at me for not scrutinizing enough).

The method that saves all user groups uses a background worker due to how much longer it takes to run. I understand I should probably add a background worker for the save users method too.

Users are able to change both the domain and the organizational units using text boxes.

ActiveDirectoryTool.cs:

public partial class ActiveDirectoryTool : Form
{
    private Backend backend;

    public ActiveDirectoryTool()
    {
        InitializeComponent();
        backend = new Backend();
        UpdateDisplay();
    }

    private void getAllUsers_Click(object sender, EventArgs e)
    {
        this.Enabled = false;
        if (backend.PrintAllUsersToFile())
        {
            this.Enabled = true;
        }
    }

    private void printAllUserGroups_Click(object sender, EventArgs e)
    {
        if (!backgroundWorker1.IsBusy)
        {
            this.Enabled = false;
            backgroundWorker1.RunWorkerAsync();
        }
    }

    private void backgroundWorker1_DoWork(object sender, DoWorkEventArgs e)
    {
        backend.PrintAllUserGroupsToFile();
    }

    private void backgroundWorker1_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
    {
        this.Enabled = true;
    }

    private void UpdateBackend()
    {
        backend.Domain = domainBox.Text;
        backend.OrganizationalUnits = organizationalUnitBox.Text;
        backend.WriteSettings();
    }

    private void UpdateDisplay()
    {
        domainBox.Text = backend.Domain;
        organizationalUnitBox.Text = backend.OrganizationalUnits;
        scopeDisplay.Text = backend.Scope;
    }

    private void organizationalUnitBox_TextChanged(object sender, EventArgs e)
    {
        UpdateBackend();
        UpdateDisplay();
    }

    private void domainBox_TextChanged(object sender, EventArgs e)
    {
        UpdateBackend();
        UpdateDisplay();
    }
}

Backend.cs:

internal class Backend
{
    private const string DefaultDomain = "dnet.domtar";
    private const string CommaSeparatedValuesExtension = ".csv";
    private const string PlainTextExtension = ".txt";
    private const string TabSeparatedValuesExtension = ".tsv";
    private const string DefaultExtension = PlainTextExtension;
    private const string DateTimeFormat = "yyyyMMddTHHmmss";
    private const char Hyphen = '-';
    private const char Tab = '\t';
    private const char Comma = ',';
    private const string UserListHeader = "Last\tFirst\tDisplay Name\tID\tActive\tLocked\tDescription\tHome Drive\tHome Folder\tLogin Script\tEmail\tStreet\tCity\tState\tPhone\tDistinguished Name";
    private const string GroupListHeader = "Group Name\tGroup ID\tManaged By\tDescription\tDistinguished Name";
    private const string UserGroupListHeader = "User ID\tGroup\tUser Full Name\tUser Distinguished Name";
    private string path = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.MyDocuments), "ActiveDirectoryTool");

    public Backend()
    {
        if (!Directory.Exists(path))
        {
            Directory.CreateDirectory(path);
        }
        ReadSettings();
    }

    private string domain;
    private string domainContext;
    private string organizationalUnits;

    public string Domain
    {
        get
        {
            return domain;
        }

        set
        {
            domain = value;
        }
    }

    public string OrganizationalUnits
    {
        get
        {
            return organizationalUnits.Replace("OU=", "").Replace(",", " ");
        }

        set
        {
            organizationalUnits = "OU=" + value.Replace(" ", ",OU=");
        }
    }

    private DirectoryEntry DefaultRootDirectory
    {
        get
        {
            return new DirectoryEntry("LDAP://RootDSE");
        }
    }

    private DirectoryEntry RootDirectory
    {
        get
        {
            if (!String.IsNullOrWhiteSpace(Domain) && !Domain.Equals(DefaultDomain))
            {
                return new DirectoryEntry("LDAP:://" + Domain);
            }
            else
            {
                return DefaultRootDirectory;
            }
        }
    }

    public string DomainContext
    {
        get
        {
            if (String.IsNullOrWhiteSpace(domainContext))
            {
                domainContext = GetDefaultDomainContext();
            }
            return domainContext;
        }
    }

    public string Scope
    {
        get
        {
            return organizationalUnits + Comma + DomainContext;
        }
    }

    private string GetDefaultDomainContext()
    {
        return RootDirectory.Properties["defaultNamingContext"].Value as string;
    }

    private PrincipalContext GetPrincipalContext()
    {
        return new PrincipalContext(ContextType.Domain, Domain, Scope);
    }

    public void WriteSettings()
    {
        ConfigurationManager.AppSettings["domain"] = domain;
        ConfigurationManager.AppSettings["organizationalUnits"] = organizationalUnits;
    }

    private void ReadSettings()
    {
        domain = ConfigurationManager.AppSettings["domain"];
        organizationalUnits = ConfigurationManager.AppSettings["organizationalUnits"];
    }

    private void ShowException(Exception e)
    {
        MessageBox.Show("Exception: " + e.Message + "\n" + e.StackTrace);
    }

    private string GenerateFilename(string fileType)
    {
        return fileType + Hyphen + DateTime.Now.ToString(DateTimeFormat) + DefaultExtension;
    }

    private bool IsActive(DirectoryEntry de)
    {
        if (de.NativeGuid == null) return false;

        int flags = (int)de.Properties["userAccountControl"].Value;

        return !Convert.ToBoolean(flags & 0x0002);
    }

    private string ConcatenateWithTabs(params string[] strings)
    {
        string concatenated = "";
        foreach (string s in strings)
        {
            concatenated += s + Tab;
        }
        return concatenated;
    }

    private string UserAsString(UserPrincipal user)
    {
        DirectoryEntry entry = user.GetUnderlyingObject() as DirectoryEntry;
        return ConcatenateWithTabs(
            user.Surname,
            user.GivenName,
            user.DisplayName,
            user.SamAccountName,
            IsActive(entry).ToString(),
            user.IsAccountLockedOut().ToString(),
            user.Description,
            user.HomeDrive,
            user.HomeDirectory,
            user.ScriptPath,
            user.EmailAddress,
            (string)entry.Properties["streetAddress"].Value,
            (string)entry.Properties["l"].Value,
            (string)entry.Properties["st"].Value,
            user.VoiceTelephoneNumber,
            user.DistinguishedName);
    }

    private string UserGroupAsString(UserPrincipal user, GroupPrincipal group)
    {
        return ConcatenateWithTabs(
            user.SamAccountName,
            group.SamAccountName,
            user.Name,
            user.DistinguishedName);
    }

    public bool PrintAllUsersToFile()
    {
        try
        {
            int count = 0;
            string filename = Path.Combine(path, GenerateFilename("AllUsers"));
            using (var searcher = new PrincipalSearcher(new UserPrincipal(GetPrincipalContext())))
            {
                using (StreamWriter sr = new StreamWriter(filename))
                {
                    sr.WriteLine(UserListHeader);
                    foreach (var result in searcher.FindAll())
                    {
                        sr.WriteLine(UserAsString((UserPrincipal)result));
                        count++;
                    }
                }
            }
            MessageBox.Show("All (" + count + ") users saved to file " + filename);
        }
        catch (Exception e)
        {
            ShowException(e);
        }
        return true;
    }

    public bool PrintAllUserGroupsToFile()
    {
        try
        {
            int count = 0;
            string filename = Path.Combine(path, GenerateFilename("UserGroups"));
            using (var searcher = new PrincipalSearcher(new UserPrincipal(GetPrincipalContext())))
            {
                using (StreamWriter sr = new StreamWriter(filename))
                {
                    sr.WriteLine(UserGroupListHeader);
                    foreach (UserPrincipal user in searcher.FindAll())
                    {
                        foreach (GroupPrincipal group in user.GetGroups())
                        {
                            sr.WriteLine(UserGroupAsString(user, group));
                        }
                        count++;
                    }
                }
            }
            MessageBox.Show("All (" + count + ") users with groups saved to file " + filename);
        }
        catch (Exception e)
        {
            ShowException(e);
        }
        return true;
    }
}

Answer 1

"Backend" is a compound word and thus the correct capitalization should be "BackEnd". However, that would still be a fairly meaningless name for a class, even for a namespace.

---

Your Backend class starts out with a dozen const; I'd put those in a separate (static) class.

---

Strings like "domain" and "organizationalUnits" and "OU=" get used multiple times, so they too should be const strings, and of course then moved to that static class mentioned earlier.

---

Why is there a method ShowException in your back-end code? Separate your UI from your logic.

The contents of that method are also up for debate: showing the stack trace?

---

You do a lot of string concatenation where perhaps string.Format() would be more appropriate, e.g. return fileType + Hyphen + DateTime.Now.ToString(DateTimeFormat) + DefaultExtension;.

---

ConcatenateWithTabs seems to reinvent string.Join() for some reason?

---

MessageBox.Show also is present in PrintAllUserGroupsToFile and PrintAllUsersToFile, and it shoudln't be: decouple your UI from your back-end.

Answer 2

First thing that I saw that you could make a little easier to read and less verbose is this

private void getAllUsers_Click(object sender, EventArgs e)
{
    this.Enabled = false;
    if (backend.PrintAllUsersToFile())
    {
        this.Enabled = true;
    }
}

you are setting this.Enabled to whatever backend.PrintAllUsersToFile() returns, which I am assuming is a boolean value.

So just set this.Enabled to backend.PrintAllUsersToFile() like this

private void getAllUsers_Click(object sender, EventArgs e)
{
    this.Enabled = backend.PrintAllUsersToFile();
}

but it appears that you want the button disabled between the click and the function return so we need to have the this.Enabled = false; in there.

private void getAllUsers_Click(object sender, EventArgs e)
{
    this.Enabled = false;
    this.Enabled = backend.PrintAllUsersToFile();
}

---

Also, I noticed somewhere that you could use some syntactic sugar to reduce indentations

public bool PrintAllUsersToFile()
{
    try
    {
        int count = 0;
        string filename = Path.Combine(path, GenerateFilename("AllUsers"));
        using (var searcher = new PrincipalSearcher(new UserPrincipal(GetPrincipalContext())))
        {
            using (StreamWriter sr = new StreamWriter(filename))
            {
                sr.WriteLine(UserListHeader);
                foreach (var result in searcher.FindAll())
                {
                    sr.WriteLine(UserAsString((UserPrincipal)result));
                    count++;
                }
            }
        }
        MessageBox.Show("All (" + count + ") users saved to file " + filename);
    }
    catch (Exception e)
    {
        ShowException(e);
    }
    return true;
}

When you have 2 using statements in a row you can "group" them together and use a single set of curly braces like this

public bool PrintAllUsersToFile()
{
    try
    {
        int count = 0;
        string filename = Path.Combine(path, GenerateFilename("AllUsers"));
        using (var searcher = new PrincipalSearcher(new UserPrincipal(GetPrincipalContext())))
        using (StreamWriter sr = new StreamWriter(filename))
        {
            sr.WriteLine(UserListHeader);
            foreach (var result in searcher.FindAll())
            {
                sr.WriteLine(UserAsString((UserPrincipal)result));
                count++;
            }
        }
        MessageBox.Show("All (" + count + ") users saved to file " + filename);
    }
    catch (Exception e)
    {
        ShowException(e);
    }
    return true;
}

---

if you are using C# 6.0 you can use some more syntactic sugar in the way of string interpolation on some of your messages or string concatenation, it may make things a little less cluttered.

MessageBox.Show("All (" + count + ") users saved to file " + filename);

Would become this instead

MessageBox.Show($"All ({count}) users saved to file {filename}");

And your ShowException Method

private void ShowException(Exception e)
{
    MessageBox.Show("Exception: " + e.Message + "\n" + e.StackTrace);
}

would look like this

private void ShowException(Exception e)
{
    MessageBox.Show($"Exception: {e.Message}\n {e.StackTrace}");
}

---

I also noticed that in your Constructor for the Backend class that you can get rid of the if statement as well

public Backend()
{
    if (!Directory.Exists(path))
    {
        Directory.CreateDirectory(path);
    }
    ReadSettings();
}

turns out that the Directory.CreateDirectory() Method creates a directory unless it already exists, so you don't need the if statement at all.

public Backend()
{
    Directory.CreateDirectory(path);
    ReadSettings();
}