3

Right now, my user table has a bool called Admin. As the code shows, if user.admin = true, the user is able to see the admin area button and access it.

    @if (Common.UsuarioLogueado.Admin) {
                                <li><a href="@Url.Action("List","ClientesAdmin",new { Area = "Admin" })">Admin control panel</a></li>
                            }

This is working as intended. However, non admin users can still go to the control panel by accessing it´s url http://localhost/appName/admin/ClientesAdmin/list

How do I prevent such thing? I was thinking about showing an error msg

Koni
  • 411
  • 1
  • 4
  • 16
  • 2
    Google AuthorizeAttribute - the attribute used to secure by authenticated users, or by roles. – Brian Mains Aug 02 '17 at 13:38
  • 1
    Create a FilterAttribute that manage the security as you expect – Zinov Aug 02 '17 at 13:38
  • 1
    what you need is `Roles` in ASP MVC and you can find an abundance of samples around the net. – mcy Aug 02 '17 at 13:42
  • Possible duplicate of [Asp.net MVC4: Authorize on both controller and action](https://stackoverflow.com/questions/16709853/asp-net-mvc4-authorize-on-both-controller-and-action) – Travis Tubbs Aug 02 '17 at 13:45

3 Answers3

2

Going along with the other answers about using Roles, and the AuthorizeAttribute.. which in my opinion is the better way to achieve what you're trying to do, there is another way.

You could just simply redirect the user to another page.. preferable an error page saying you don't have access to the requested page, or just a 401 page which the AuthorizeAttribute would do if you weren't authorized.

Alternate Solution

public class ClientesAdmin : Controller {
    // [Authorize(Roles="Admin")]  could do it this way
    public ActionResult List() {
        // or..
         if(!Common.UsuarioLogueado.Admin)
         {
             return new HttpStatusCodeResult(401);
             // or
             // return View("Error") // usually there is an 'Error' view the Shared folder
         }

         return View();
    }
}

This is not the best solution but I don't know how far along your project is, but simply an alternate solution.

Grizzly
  • 5,411
  • 5
  • 39
  • 94
  • Project is literally almost ready :( im a trainee so I had no idea about roles. Thanks for the help, but how do I make my controller check the AdminOnly method when an user tries to log in? – Koni Aug 02 '17 at 13:59
  • @MarianoGianni what do you mean *when a user tries to log in*? The user is either an admin or not an admin because you said that in your `Users` table the Admin Property is a bit (bool). So if a user were to try and access the url `http://localhost/appName/admin/ClientesAdmin/list`.. the flow will go to the `List` Action on the `ClientesAdmin` controller and it will hit that `if` statement – Grizzly Aug 02 '17 at 14:01
  • Never mind that, I was able to achieve my objective with your help. Thank you. – Koni Aug 02 '17 at 14:04
  • 1
    You're welcome. Glad I was able to help. Happy coding! – Grizzly Aug 02 '17 at 14:04
1

This is how I do it. However your membership system needs to be using ASP.Net Roles for this to work properly.

In your controller you just add the data annotation Authorize. for the function to be accessed by the client, they must be logged in and have the roll specified in the function.

This solution may not be direct cut and paste, but you can see the basic usage then perhaps do a little more research on the Authorize functionality.

public class MyController : Controller {
     [Authorize(Roles="Admin")]
     public ActionResult AdminIndex() {
          return View();
     }



    [Authorize(Roles = "basic")]
    public ActionResult BasicUsersIndex() {
         return View();
    }
}
Travis Tubbs
  • 793
  • 1
  • 13
  • 32
1

Ideally you should be using role based access control. By limiting access by the role, rather than a boolean value in a table you could decorate your CientesAdmin controller with an Authorize Attribute like below.

[Authorize(Roles = "Admin")]
public class CientesAdminController : Controller
{ 
}

You could also use razor helpers to check if a user IsInRole("Admin").

There is a lot of help on the net to guide you down this path, but if your app is already developed you probably want to stage your changes. Then the recommendation would be to create your own AuthoriseAttribue. Something like.

public class RestrictAccessToAdmins : AuthorizeAttribute
{
    protected override bool AuthorizeCore(HttpContextBase httpContext)
    {
        //Do the default Authorise Logic (Check if user is loggedin) 
        base.AuthorizeCore(httpContext);
         if (httpContext.User.IsInRole("Admin")) return true;

        var id = httpContext.User.Identity.GetUserId();

        using (ApplicationDbContext context = new ApplicationDbContext())
        {
            //Implement you own DB logic here returning a true or false. 
            return context.Common.First(u => u.userid == id).UsuarioLogueado.Admin;
        }
    }

}

To use the attribute you'd do the following.

[RestrictAccessToAdmins]
public class CientesAdminController : Controller
{ 
}

Then over time, with better understanding of the default authorise attribute and a bit of refactoring you could easily change the attribute to below :)

[RestrictAccessToAdmins(Roles = "Admin")]
public class CientesAdminController : Controller
{ 
}
Phillip Morton
  • 224
  • 2
  • 12
  • This is great, and yeah, app is already developed this way. I will keep it in mind for my next project. – Koni Aug 02 '17 at 13:49
  • The sequence returned by your `where` predicate is not going to have a `UsuarioLogueado` property. Did you mean `Find()` or `FirstOrDefault()`? – Crowcoder Aug 02 '17 at 13:54
  • Yeah I know, it's purely for example purposes :) Feel free to edit as you will. – Phillip Morton Aug 02 '17 at 13:55
  • Agreeing with @Crowcoder .. that line should ideally be `return context.Common.Single(u => u.userid === id).UsuarioLogueado.Admin;`.. if `userid` is **unique** there should only be a **single** record.. so no need to use `First()` or `FirstOrDefault()` – Grizzly Aug 02 '17 at 13:57