Action passing null instead of string in asp.net mvc application

Question

I am absolutely stuck on this, I have created an action to add a new Identity Role to my database using the Identity Framework. Unfortunately whenever it is executed, it returns that it has failed and after I inserted a breakpoint, I established that I am passing null as the value for newRole and not superadmin as would be expected.

The below is the URL I am using, exactly as I am using it:

https://localhost:44344/account/addrole/superadmin

And this is the corresponding action:

[Authorize(Roles = RoleNames.CanAddUsers)]
public ActionResult AddRole(string newRole)
{
    var roleStore = new RoleStore<IdentityRole>(new ApplicationDbContext());
    var roleManager = new RoleManager<IdentityRole>(roleStore);
    var result = roleManager.Create(new IdentityRole(newRole));

    return result.Succeeded 
        ? Content(newRole + " added to database.") 
        : Content("Failed to add " + newRole + ".");
}

Now I have checked and double checked that the user I am logged in as whilst testing this has the role CanAddUsers and I am not being redirected to login but simply receiving Failed to add .

To confirm, the following is my route and I cannot find anything else suggesting my code may be routed in a different manner.

routes.MapRoute(
    name: "Default",
    url: "{controller}/{action}/{id}",
    defaults: new { controller = "Home", action = "Index", id = UrlParameter.Optional }
);

I am sure at this point it is probably an incredibly obvious mistake but I cannot find it!

asp.net identity version: 2.2.1
asp.net mvc version: 5.2.3
entity framework 6.3.1

instead of addrole/superadmin maybe should be addrole?newRole=superadmin or create an view and do the post because it is a "get" call — Carlos Cocom, Jul 14 '17 at 16:38
You shouldn't be using a GET for creating/modifying data. If you must, add [HttpGet("{newRole}")] to your action — Mardoxx, Jul 14 '17 at 16:52
Or else just replace `newRole` with `id` in your controller action. — Rafalon, Jul 14 '17 at 17:00
`addrole?newRole=superadmin` worked perfectly and then that lead me to understand why I could not make it work... which is exactly what Rafalon said! Thank you all! — Stu, Jul 14 '17 at 17:02
@Stu as Mardoxx pointed out you should really be performing a POST and not a GET — maccettura, Jul 14 '17 at 17:05
@Mardoxx can I ask why that should be POST when the action is only actionable by an authorised user? Is this simply down to standard practice or is their a practical difference also. — Stu, Jul 14 '17 at 17:43
https://stackoverflow.com/a/1254147/3515174 I mean sure chances of someone targeting you are very slim but still it's usually best to follow standards! — Mardoxx, Jul 14 '17 at 18:37

score 3 · Accepted Answer · answered Jul 14 '17 at 17:05

If you want to use the MapRoute only, then you should know that the name of the optional parameter (id in your case) will be used.

Then you should either rename your Action's parameter to id, or explicitly say that you use the newRole variable by writing addrole?newRole=superadmin.

Or else, as others pointed out, you may prefer to use POST instead of GET for this.

Action passing null instead of string in asp.net mvc application

1 Answers1