RANT : Hard Coded Security Roles

:::::::::::sigh:::::::::::::

If I come across one more hard coded security roles brownfield application, I think I’m gonna throw my chair out the window.

Today, I came across an application that does type-checking to determine security.

The code goes something like this:

                        AbstractUser user = SomeGetUserMethod("myname", "mypassword");
                       /* the above method returns 1 of a few concrete classes which implement AbstractUser */

                        // type-checking time!!
                        if (user.GetType() == typeof(BAL.Domain.AdminUser))
                        {
                            MyWebPage.btnDeleteAllEmployees.Visible = true;
                            MyWebPage.btnUpdateMyOwnProfile.Visible = true;
                            MyWebPage.btnLogOut.Visible = true;
                        }

                        if (user.GetType() ==
typeof(BAL.Domain.NormalUser))
                        {
                            MyWebPage.btnDeleteAllEmployees.Visible = false;
                            MyWebPage.btnUpdateMyOwnProfile.Visible =
true;
                            MyWebPage.btnLogOut.Visible = true;
                        }

or maybe you have seen (bool representing 1 of a few hard coded roles)

                        bool isAdmin = SomeMethodToFigureOutHardCodedRoles();
                        bool isNormalUser =
SomeMethodToFigureOutHardCodedRoles();
                        if (isAdmin)
                        {
                            MyWebPage.btnDeleteAllEmployees.Visible =
true;
                            MyWebPage.btnUpdateMyOwnProfile.Visible =
true;
                            MyWebPage.btnLogOut.Visible = true;
                        }
                        if (isNormalUser)

                        {

                            MyWebPage.btnDeleteAllEmployees.Visible =false ; // NormalUser cannot do this! So hide the button.

                            MyWebPage.btnUpdateMyOwnProfile.Visible =
true;

                            MyWebPage.btnLogOut.Visible = true;

                        }

You know the drill. 

And the pre-project-starts-to-be-constructed discussion goes something like this:
"Today, we have 3 roles, lets base all our security off those 3 (hard coded) roles.  Those roles will ~~never~~ change.

And my thoughts on a finite set of hard coded Role(s).
That’s fine for your kid’s soccer club fan page.
That is NOT fine for a professional DotNet developer creating a business application.

I’ve written an example on how NOT do it (so you can see the pattern clearly).

The problem will always end up being that the 3 (N) number of roles will never be sufficient.
A DAY WILL ARRIVE WHEN THE BUSINESS OWNERS WANT A SLIGHTLY DIFFERENT ROLE.
They’ll say that want to "tweak" an existing role, but what they really mean is that they want a new role that is very close to an existing role.
But "a tad bit different" is still different.
But you (or your "architect" if you want to blame someone else) did not account for this at the beginning of the project.

You can check out:
http://www.lhotka.net/weblog/CommentView,guid,9efcafc7-68a2-4f8f-bc64-66174453adfd.aspx
for a discussion.
I can’t blame just the developer(s).  Microsoft and its "super easy p-easy" ".IsInRole()" method helped propagate this ugliness.
And then this kind of code:
[PrincipalPermission(SecurityAction.Demand,Role="Teller")]

And I completely agree with the article above and its assertion "At runtime, when the user is actually using the application, the roles are entirely meaningless".
Listen people (aka all you developers) ..  software cares about permissions (or "rights").  Stop coding to roles, start coding to permissions (or rights).

The article above tackles the issue using the existing (available) objects in DotNet.

Here is my custom IPrincipal solution, if the above workaround rubs you the wrong way.
While I have "Role" methods, I never use them ***, except for the AllRoles collection, which I use to show humans what role they are in.
However, I never show just the Roles, I show the Permissions/Rights, because that is more important.

    public interface IRolesAndRightsPrincipal : System.Security.Principal.IPrincipal
    {

        bool IsInRole(System.Guid role);

        bool IsInAnyRole(System.Guid[] roles);

        bool IsInAllRoles(System.Guid[] roles);

        bool HasRight(System.Guid right);

        bool HasAnyRight(System.Guid[] rights);

        bool HasAllRights(System.Guid[] rights);
       
        ISecurityRoleCollection AllRoles
        {
            get;
        }

        ISecurityRightCollection AllRights
        {
            get;
        }
    }

(*** The one place I might use them (though I never have) is backwards compatibility when refactoring an existing application.  If the current application is "all roled up" then that would be a stepping stone to getting to permissions/rights based security.)

You might be saying "What is the HasAnyRight method all about?"

Well, take for instance that you have a menu link called "Manage Employees".
This link will take you to a separate page that allows you to ADDNEW, UPDATE, and/or DELETE an employee.
(These are 3 distinct permission(s)/rights(s)).

So how do you decide if you should show this menu link called "Manage Employees", since its not based on a single permission/right.

And there ya go:  Use the HasAnyRight() method
menuItemManageEmployees.Visible = customPrinc.HasAnyRight ( /* throw the Guids here which represent ADDNEW, UPDATE, DELETE */ );
You show the link if the use has one (any) of the 3 permissions/rights.
Then when you get to that new page, you show buttons/links/etc based on the individual permissions/rights.

Side note, my concrete IRolesAndRightsPrincipal ( adaptly named "RolesAndRightsPrincipal" takes all your roles,rights in its constructor, and then becomes a look-up holder from that point on.  Obviously, if you do on the fly permissions/rights changes, you’ll have to refresh it.
Currently, I just go with a "You gotta re-login to get your fresh permissions/rights", since the project (almost) never takes away any permissions/rights and seldom changes them.  But you’ll have to conquer that design decision on your own.

:::::::::Call to all Developers::::::::::::

If you’re developing a new project, and starting out with N number of hard coded roles, please stop.  Please stop.  I’m begging you, please stop.

If you’re in a brownfield application, then ask your manager for some time to re-factor the security.
If your manager is fair, then he/she will find some time for you that does not involve late nights or weekends.
If your manager thinks re-factoring (without scope change) is a waste of time since "it works, so its OK", then find a new job.

This oldie-but-goodie article has the basic meat of a good solution.
http://www.codeproject.com/KB/security/cgsecurity.aspx
I would take his DDL, update it to your standards, and "framework up" his "Managers"(.cs code).

It is OK to make a Role, but ONLY in the sense to logically group a set of permissions/rights.
You should be checking permission(s)/right(s) when you actually are interested in the question
                 "Can this IIdentity perform this certain thing?"

Please stop coding hard-coded-roles into your application(s). 
The developers who have to maintain your code after you’ve left will thank you.
..

I have also posted an "Anti Example" C# code example here.  (<<Right-Click , "Save As" usually works best).

//End Rant

About these ads
This entry was posted in Software Development. Bookmark the permalink.

2 Responses to RANT : Hard Coded Security Roles

  1. Pingback: NetSqlAzMan vs AzMan vs (?????) | DIGG LINK

  2. Another frustrated pro says:

    i entirely agree with the rant, but the only link to to the garbage example; where’s the REAL code??

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s