HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Custom Authentication Attribute

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
customauthenticationattribute

Problem

I was trying to find a way to redirect to different pages on authorization and authentication failure. I found this to be a possible solution.

However, I ended with a different solution by myself. It seems to work fine, however, I am not sure if it is the right thing to do.

I created a custom Authorize Attribute that redirects to an action if the request is Authenticated but not Authorized.

```
public class HandleAuthorizeAttribute : AuthorizeAttribute {
public static string GlobalUnAuthorizationUrl { get; set; }

private const string DefaultUnAuthorizationUrl = "~/Account/UnAuthorized";
private static readonly char[] RolesSeparator = { ',' };

public string UnAuthorizedUrl { get; set; }

protected override bool AuthorizeCore(HttpContextBase httpContext) {
if(httpContext.User.Identity.IsAuthenticated) {
if(string.IsNullOrEmpty(Roles)) {
return true;
} else {
var rolesOfUser = System.Web.Security.Roles.GetRolesForUser(httpContext.User.Identity.Name);
var authorizedRoles = Roles.Split(RolesSeparator);

var common = rolesOfUser.Intersect(authenticatedRoles);

if(common.Count() == 0) {
httpContext.Response.Redirect(
string.Format("{0}?{1}={2}", ActiveUnAuthorizedUrl, "requestUrl", httpContext.Request.Url.AbsoluteUri));
return false;
}

return true;
}
} else {
return false;
}
}

private string ActiveUnAuthorizedUrl {
get {
if(!string.IsNullOrEmpty(UnAuthorizedUrl)) {
return UnAuthorizedUrl;
}
if(!string.IsNullOrEmpty(GlobalUnAuthorizationUrl)) {
return GlobalUnAuthorizationUrl;
}

Solution

There is something wrong with this Method that makes it a bit confusing.

protected override bool AuthorizeCore(HttpContextBase httpContext) {
        if(httpContext.User.Identity.IsAuthenticated) {
            if(string.IsNullOrEmpty(Roles)) {
                return true;
            } else {
                var rolesOfUser = System.Web.Security.Roles.GetRolesForUser(httpContext.User.Identity.Name);
                var authorizedRoles = Roles.Split(RolesSeparator);

                var common = rolesOfUser.Intersect(authenticatedRoles);

                if(common.Count() == 0) {
                    httpContext.Response.Redirect(
                        string.Format("{0}?{1}={2}", ActiveUnAuthorizedUrl, "requestUrl", httpContext.Request.Url.AbsoluteUri));
                    return false;
                }

                return true;
            }
        } else {
            return false;
        }
    }


first thing that I noticed is that the authorizedRoles variable isn't being used, then I saw that you use a variable that isn't declared or initialized anywhere in your code, authenticatedRoles, I am assuming that these are supposed to be the same variable when I removed the common variable like this

protected override bool AuthorizeCore(HttpContextBase httpContext) {
        if(httpContext.User.Identity.IsAuthenticated) {
            if(string.IsNullOrEmpty(Roles)) {
                return true;
            } else {
                var rolesOfUser = System.Web.Security.Roles.GetRolesForUser(httpContext.User.Identity.Name);
                var authorizedRoles = Roles.Split(RolesSeparator);

                if((rolesOfUser.Intersect(authorizedRoles)).Count == 0) {
                    httpContext.Response.Redirect(
                        string.Format("{0}?{1}={2}", ActiveUnAuthorizedUrl, "requestUrl", httpContext.Request.Url.AbsoluteUri));
                    return false;
                }
                return true;
            }
        } else {
            return false;
        }
    }


I removed some of the else statements that were making this code a little cluttered as well, I was tempted to remove some of these return statements and replace it with a single bool and then just return at the end of the method, but I think that would have made this code messier and harder to read, so this is what I came up with, and it should do the same thing as your code assuming that those two variables I mentioned earlier are the same variable.

protected override bool AuthorizeCore(HttpContextBase httpContext) {
    if(httpContext.User.Identity.IsAuthenticated) {
        if(string.IsNullOrEmpty(Roles)) {
            return true;
        } 
        var rolesOfUser = System.Web.Security.Roles.GetRolesForUser(httpContext.User.Identity.Name);
        var authorizedRoles = Roles.Split(RolesSeparator);

        if((rolesOfUser.Intersect(authorizedRoles)).Count == 0) {
            httpContext.Response.Redirect(
                string.Format("{0}?{1}={2}", ActiveUnAuthorizedUrl, "requestUrl", httpContext.Request.Url.AbsoluteUri));
            return false;
        }
        return true;
    } 
    return false;
}

Code Snippets

protected override bool AuthorizeCore(HttpContextBase httpContext) {
        if(httpContext.User.Identity.IsAuthenticated) {
            if(string.IsNullOrEmpty(Roles)) {
                return true;
            } else {
                var rolesOfUser = System.Web.Security.Roles.GetRolesForUser(httpContext.User.Identity.Name);
                var authorizedRoles = Roles.Split(RolesSeparator);

                var common = rolesOfUser.Intersect(authenticatedRoles);

                if(common.Count() == 0) {
                    httpContext.Response.Redirect(
                        string.Format("{0}?{1}={2}", ActiveUnAuthorizedUrl, "requestUrl", httpContext.Request.Url.AbsoluteUri));
                    return false;
                }

                return true;
            }
        } else {
            return false;
        }
    }
protected override bool AuthorizeCore(HttpContextBase httpContext) {
        if(httpContext.User.Identity.IsAuthenticated) {
            if(string.IsNullOrEmpty(Roles)) {
                return true;
            } else {
                var rolesOfUser = System.Web.Security.Roles.GetRolesForUser(httpContext.User.Identity.Name);
                var authorizedRoles = Roles.Split(RolesSeparator);

                if((rolesOfUser.Intersect(authorizedRoles)).Count == 0) {
                    httpContext.Response.Redirect(
                        string.Format("{0}?{1}={2}", ActiveUnAuthorizedUrl, "requestUrl", httpContext.Request.Url.AbsoluteUri));
                    return false;
                }
                return true;
            }
        } else {
            return false;
        }
    }
protected override bool AuthorizeCore(HttpContextBase httpContext) {
    if(httpContext.User.Identity.IsAuthenticated) {
        if(string.IsNullOrEmpty(Roles)) {
            return true;
        } 
        var rolesOfUser = System.Web.Security.Roles.GetRolesForUser(httpContext.User.Identity.Name);
        var authorizedRoles = Roles.Split(RolesSeparator);

        if((rolesOfUser.Intersect(authorizedRoles)).Count == 0) {
            httpContext.Response.Redirect(
                string.Format("{0}?{1}={2}", ActiveUnAuthorizedUrl, "requestUrl", httpContext.Request.Url.AbsoluteUri));
            return false;
        }
        return true;
    } 
    return false;
}

Context

StackExchange Code Review Q#13838, answer score: 5

Revisions (0)

No revisions yet.