SQL Permission Handling in WinForms

For my application, I've opted to use Integrated Security with Windows Authentication to access the SQL database. I've created AD security groups for each role and given them GRANT permissions on the tables and views they need to do their job.

To further keep the end users from getting to the controls I'm checking the SQL permissions using HAS_PERMS_BY_NAME() and then disabling the relevant controls.

In a static class, I have defined astatic method to get a dictionary of permissions.

namespace PartDataManager
{
    public static class Globals
    {
        public static Dictionary GetPermissions()
        {
            Dictionary Permissions = new Dictionary();
            string query = "SELECT \r\n";
            foreach( string table in new string[]{"appearance","appearanceSpecs","cutTemplates","cutVinyl"} )
            {
                foreach( string perm in new string[]{"INSERT","UPDATE","DELETE"} )
                {
                    query += String.Format("\t{0}HAS_PERMS_BY_NAME('{1}','OBJECT','{2}') AS {1}_{2}\r\n",(query.Length>10?",":""),table,perm);
                }
            }
            try
            {
                using( SqlConnection sql = new SqlConnection(ConnectionString) )
                using( SqlCommand cmd = new SqlCommand(query,sql) )
                {
                    sql.Open();
                    using( SqlDataReader data = cmd.ExecuteReader() )
                    {
                        data.Read();
                        for( int i = 0; i < data.FieldCount; i++ )
                            Permissions.Add(data.GetName(i), data[i].Equals(1));
                    }
                }
            }
            catch
            {
                throw;
            }
            return Permissions;
        }

        // ...

    }
}

I then use the method on loading of the form to disable the relevant controls like so (wrapped in a try... catch)...

```
Dictionary Permissions = Globals.GetPermissions()

So, any tips that reduce lines of code..

As a start you can remove this useless try..catch because you are only rethrowing the exception which is senseless.

Using += with strings in a loop is a bad habit because each time a new string will be created so better use a StringBuilder.

Speeking about the composition of the query variable you don't need to add new lines and tabs in it. You won't show it, you only execute it as a sql query.

In its current state this method is not flexible enough. Each time you want to query different tables or need only specific permissions you need to change this method.

By passing IEnumerable tables and IEnumerable permissionType you can beautify your method and add a lot flexibility to it.

Let us talk about style. If it comes to style one should stick to a choosen style.

If you choose to use braces {} for loops you should always use them. Mixing styles is a bad habit, don't do it.

Method scoped variables should be named using camelCase casing. PascalCase casing is used for class-, method- and propertynames. So Dictionary Permissions -> Dictionary permissions.

EDIT Based on the great comment of @d347hm4n stating


In the above code, in all cases, there will be a comma on the end that needs to be removed. So removing the last character from the string builder is sufficient. If the sql generated does change in the future, using a TrimEnd instead gives you the extra intelligence to not remove the comma on the end if it's not there.

I changed the former removing of the comma from

sb.Length -= 1;  
string query = sb.ToString();

to

string query = sb.ToString().TrimEnd(',');

Applying the mentioned points lead to

public static Dictionary GetPermissions(IEnumerable tables, IEnumerable permissionTypes)
{
    if (!tables.Any() || !permissionTypes.Any())
    {
        return new Dictionary();
    }

    StringBuilder sb = new StringBuilder(1024);
    sb.Append("SELECT ");
    foreach (string table in tables)
    {
        foreach (string perm in permissionTypes)
        {
            sb.AppendFormat("HAS_PERMS_BY_NAME('{0}','OBJECT','{1}') AS {0}_{1},", table, perm);
        }
    }

    string query = sb.ToString().TrimEnd(',');

    Dictionary permissions = new Dictionary();

    using (SqlConnection sql = new SqlConnection(ConnectionString))
    using (SqlCommand cmd = new SqlCommand(query, sql))
    {
        sql.Open();
        using (SqlDataReader data = cmd.ExecuteReader())
        {
            data.Read();
            for (int i = 0; i < data.FieldCount; i++)
            {
                permissions.Add(data.GetName(i), data[i].Equals(1));
            }
        }
    }

    return permissions;
}

and can be then called like this

IEnumerable tables = new string[] { "appearance", "appearanceSpecs", "cutTemplates", "cutVinyl" };
IEnumerable permissionTypes = new string[] { "INSERT", "UPDATE", "DELETE" };
Dictionary permissions = GetPermissions(tables, permissionTypes);

StackExchange Code Review Q#102633, answer score: 4