Terminate if statement without terminating child code

Question

I have a page which acts as both a viewing page and an edit page. Obviously for security I only want users to access the edit functionality if they are authorised. In order to submit the edited data to the server, I've wrapped a form around the page with the Html.BeginForm method. My problem is that I need to only create that form if the user is authorised. Here is my code:

if(UserIsAuthorised)
{
    using(Html.BeginForm("update","page"))
    {
        //Code that happens regardless of whether or not the user is authorised
    }
}

By wrapping the whole thing in an if statement, it means the entire page is blank unless the user is authorised, which is not how it should be. Any user should be able to view the page, but only authorised users can view it.

I know I could just write out the form tags manually but I was hoping there was an easier way Hopefully this is clear enough.

The normal pattern would be to have a display view, which include a link to redirect to an edit view (with the link only visible to authorized users) — , Mar 22 '15 at 22:18

Darren · Answer 1 · 2015-03-22T14:31:46.500

It's a little unclear why you want unauthorised users to see the Edit page, however if it is a requirement I would personally try and refactor this logic from the View and determine if the User is authenticated at the Controller level. You could then serve a different View to the client.

For instance:

public ActionResult Edit() 
{
    if (UserIsAuthorised) 
    {
       return View("AuthenticatedEdit");
    }

    return View("UnauthenticatedEdit");
}

Alternatively you could use the Authorize attribute on the Controller.

To answer your question more specifically you could use an else if statement to accommodate for what you need to do.

if (UserIsAuthorised)
{
    using(Html.BeginForm("update","page"))
    {
        //Code that happens regardless of whether or not the user is authorised
    }
} 
else if (!UserIsAuthorised) {
     //Code that happens regardless of whether or not the user is authorised
}

However this introduces logic within the View which may lead to coupled and untestable code.

score 0 · Answer 2 · edited Apr 12 '17 at 07:31

I would actually do a couple of things different, and hopefully you're early enough in your project where you can accomplish this.

First I would not mix areas of the project that can only be accessed by Authorized users, with areas that should be view only. So beyond just locking down an action with the [Authorize] attribute, I'd split up concerns and responsibilities.

However, if you cannot accomplish this I still would not suggest putting all that logic into one action. It will make it more difficult to maintain the project down the road. So split that up. I'd suggest creating two actions. As such:

namespace My.Sample
{
    /// <summary>
    /// Settings controller used to maintain settings stuff
    /// </summary>
    public class SettingsController : Controller
    {
        [Authorize]
        public ActionResult EditSettings()
        {

            return View();
        }

        public ActionResult ViewSettings()
        {
            return View();
        }
    }
}

If your concern is that you will duplicate HTML markup between the two Actions, I'd say don't worry about that so much. It's much more likely that those two views will start to differ drastically as time goes on and then you'll have to wonder how you can maintain that as one, and you'll end up with a huge cshmtl file. Split it up, it will make life much easier later on.

You're probably not looking for a theological answer, but... you should read the following:

https://softwareengineering.stackexchange.com/questions/73065/what-are-dry-kiss-solid-etc-classified-as

If you're still convinced on going the current direction, you can return a different view from an action and pass in a different model. As such:

        [Authorize]
        public ActionResult EditSettings()
        {
            var myModel = Services.SettingsService.GetSettings();
            return View("ViewSettings", myModel);
        }

So, both actions point to one view, one is authorized and one is not. The model passed in can then reflect different states, etc. Your Razor markup can do whatever needs to be done based on that. I still would not recommend doing things this way though, separate those Actions.

Thanks for the long answer kroolk. The way it's set up currently is the `view` view is combined to also allow authroized users to edit it. This is done by giving the user a button if they are authorized. This button will produce textboxes when clicked using Jquery, and the user can use those textboxes to edit the data, then this is sent off via AJAX and the recipe is updated. There is still server side authentication before the AJAX request is processed, but my point is that I'm not just allowing any user to edit the page, they will only get the edit button if they are authorized. — Frayt, Mar 22 '15 at 14:57
What about using this answer: http://stackoverflow.com/questions/2204766/how-to-check-if-user-is-authorized-inside-action You could check if the user is authenticated inside the View that way as well. — virtualadrian, Mar 22 '15 at 15:10

Terminate if statement without terminating child code

2 Answers2