Skip to content

Commit

Permalink
Add guide on conditional fill and security concerns (#1790)
Browse files Browse the repository at this point in the history
* Extend guide on fill and validation

* Extend guide

* Fix controller test

* Apply suggestions from code review

* Switch to If/ Then
  • Loading branch information
amitaibu authored Aug 14, 2023
1 parent 63c1872 commit 4589034
Show file tree
Hide file tree
Showing 4 changed files with 117 additions and 13 deletions.
4 changes: 3 additions & 1 deletion Guide/controller.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ action MyAction = do

Rarely you might want to work with a custom scalar value which is not yet supported with [`param`](https://ihp.digitallyinduced.com/api-docs/IHP-Controller-Param.html#v:param). Define a custom [`ParamReader`](https://ihp.digitallyinduced.com/api-docs/IHP-Controller-Param.html#t:ParamReader) instance to be able to use the [`param`](https://ihp.digitallyinduced.com/api-docs/IHP-Controller-Param.html#v:param) functions with your custom value type. [For that, take a look at the existing instances of `ParamReader`.](https://ihp.digitallyinduced.com/api-docs/IHP-Controller-Param.html#t:ParamReader)

### Records
### Populating Records from From Data with `fill`

When working with records, use [`fill`](https://ihp.digitallyinduced.com/api-docs/IHP-Controller-Param.html#v:fill) instead of [`param`](https://ihp.digitallyinduced.com/api-docs/IHP-Controller-Param.html#v:param). [`fill`](https://ihp.digitallyinduced.com/api-docs/IHP-Controller-Param.html#v:fill) automatically deals with validation failure when e.g. a field value needs to be an integer, but the submitted value is not numeric.

Expand All @@ -196,6 +196,8 @@ action CreatePostAction = do
redirectTo PostsAction
```

In the above example we are creating a new post record and then we are filling it with the values from the request. If the request contains invalid data, we are rendering the `NewView` again, so the user can fix the errors. If the request contains valid data, we are creating the post record and redirecting to the `PostsAction`.

## Lifecycle

The Controller instance provides a [`beforeAction`](https://ihp.digitallyinduced.com/api-docs/IHP-ControllerSupport.html#v:beforeAction) function, which is called before the [`action`](https://ihp.digitallyinduced.com/api-docs/IHP-ControllerSupport.html#v:action) function is called. Common request handling logic like authentication is often placed inside [`beforeAction`](https://ihp.digitallyinduced.com/api-docs/IHP-ControllerSupport.html#v:beforeAction) to protect all actions of the controller against unprotected access.
Expand Down
111 changes: 111 additions & 0 deletions Guide/validation.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,117 @@ user

In this example, when the [`validateIsUnique`](https://ihp.digitallyinduced.com/api-docs/IHP-ValidationSupport-ValidateIsUnique.html#v:validateIsUnique) function adds an error to the user, the message `Email Has Already Been Used` will be used instead of the default `This is already in use`.

## Security Concerns and Conditional `fill`

It's important to remember that any kind of validations you might have on the form level are not enough to ensure the security of your application. You should always have validations on the backend as well. The user might manipulate the form data and send invalid data to your application.

So if a field is disabled or has an integer field with a min/max value, you should always have a validation on the backend.

Another point is that you don't have to always `fill` all fields in one go. Sometimes you'd like to conditionally `fill` based on the current user or based on the current logic.

Let's see those examples in action. Let's say we have a `Comment` record that has a `postId` that references a `Post`, a `body` field, and a moderation field allowing admin users to indicate if they are approved or rejected.

Here's an excerpt from the `Schema.sql`:

```sql
-- Schema.sql

-- Add a moderation status column to the comments table
CREATE TYPE comment_moderation AS ENUM ('comment_moderation_pending', 'comment_moderation_approved', 'comment_moderation_rejected');

CREATE TABLE comments (
id UUID DEFAULT uuid_generate_v4() PRIMARY KEY NOT NULL,
post_id UUID NOT NULL,
body TEXT NOT NULL,
created_at TIMESTAMP WITH TIME ZONE DEFAULT NOW() NOT NULL
comment_moderation comment_moderation NOT NULL
);
```

Once we generate a controller for the `Comment` record, we'll the `buildComment` function that is called from both the `CreateCommentAction` and the `UpdateCommentAction`:

```haskell
-- Controller/Comments.hs

buildComment comment = comment
|> fill @'["postId", "body", "commentModeration"]
```

We'll start with the `postId`. Once a comment is refernecing a post it will never have the reference change. So it means we should fill it only upon creation.

```haskell
buildComment comment = comment
|> fill @'["body", "commentModeration"]
|> fillIsNew
where
fillIsNew record =
if isNew record
-- Record is new, so fill the `postId`.
then fill @'["postId"] record
-- Otherwise, leave the record as is.
else record
```

Next, imagine we have a `currentUserIsAdmin` indicating if the current user is an admin. We'd like to allow only admins to set the moderation status of a comment. So we'll allow `fill` on the `commentModeration` field only in that case:

```haskell
-- A fake implementation of `currentUserIsAdmin`.
-- Will return true if the user's email is `admin@example.com`.
currentUserIsAdmin :: (?context :: ControllerContext) => Bool
currentUserIsAdmin =
case currentUserOrNothing of
Just user -> user.email == "admin@example.com"
Nothing -> False

buildComment comment = comment
|> fill @'["body"]
|> fillIsNew
|> fillCurrentUserIsAdmin
where
fillIsNew record =
if isNew record
-- Record is new, so fill the `postId`.
then fill @'["postId"] record
-- Otherwise, leave the record as is.
else record

fillCurrentUserIsAdmin record =
if currentUserIsAdmin
then fill @'["commentModeration"] record
else record
```

Let's finish with a final example. Let's assume there was also a `score` integer field between 1 - 5 that only the admin could set. As mentioned, we'd need to have a validation on the backend to ensure that the user didn't manipulate the form data. And use `fill` to ensure that a non-admin user can't set the score in the first place. Here's the final code, where we conditionally `fill` the `score` field only if the user is an admin, and perform validation on it:

```haskell
buildComment comment = comment
|> fill @'["body"]
|> fillIsNew
|> fillCurrentUserIsAdmin
where
fillIsNew record =
if isNew record
-- Record is new, so fill the `postId`.
then fill @'["postId"] record
-- Otherwise, leave the record as is.
else record

fillCurrentUserIsAdmin record =
if currentUserIsAdmin
then fill @'["commentModeration", "score"] record
-- Make sure that star can be only between 1 and 5.
|> validateField #score (isInRange (1, 5))
else record


currentUserIsAdmin :: (?context :: ControllerContext) => Bool
currentUserIsAdmin =
case currentUserOrNothing of
Just user -> user.email == "admin@example.com"
Nothing -> False
```


## Internals

IHP's validation is built with a few small operations.
Expand Down
2 changes: 1 addition & 1 deletion IHP/IDE/CodeGen/ControllerGenerator.hs
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ generateController schema config =
<> "build" <> singularName <> " " <> modelVariableSingular <> " = " <> modelVariableSingular <> "\n"
<> " |> fill " <> toTypeLevelList modelFields <> "\n"

toTypeLevelList values = "@" <> (if length values < 2 then "'" else "") <> (values |> tshow |> Text.replace "," ", ")
toTypeLevelList values = "@'" <> (values |> tshow |> Text.replace "," ", ")
in
""
<> "module " <> moduleName <> " where" <> "\n"
Expand Down
13 changes: 2 additions & 11 deletions Test/IDE/CodeGeneration/ControllerGenerator.hs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ tests = do
, unlogged = False
}
]

it "should build a controller with name \"pages\"" do
let rawControllerName = "pages"
let controllerName = tableNameToControllerName rawControllerName
Expand Down Expand Up @@ -95,9 +96,6 @@ tests = do
, AddImport {filePath = "Web/Controller/Pages.hs", fileContent = "import Web.View.Pages.Edit"}
]




it "should build a controller with name \"page\"" do
let rawControllerName = "page"
let controllerName = tableNameToControllerName rawControllerName
Expand Down Expand Up @@ -126,7 +124,6 @@ tests = do
, AddImport {filePath = "Web/Controller/Page.hs", fileContent = "import Web.View.Page.Edit"}
]


it "should build a controller with name \"page_comment\"" do
let rawControllerName = "page_comment"
let controllerName = tableNameToControllerName rawControllerName
Expand Down Expand Up @@ -185,9 +182,6 @@ tests = do
, AddImport {filePath = "Web/Controller/PageComment.hs", fileContent = "import Web.View.PageComment.Edit"}
]




it "should build a controller with name \"people\"" do
let rawControllerName = "people"
let controllerName = tableNameToControllerName rawControllerName
Expand All @@ -197,7 +191,7 @@ tests = do
let builtPlan = ControllerGenerator.buildPlan' schema applicationName controllerName modelName pagination

builtPlan `shouldBe`
[ CreateFile {filePath = "Web/Controller/People.hs", fileContent = "module Web.Controller.People where\n\nimport Web.Controller.Prelude\nimport Web.View.People.Index\nimport Web.View.People.New\nimport Web.View.People.Edit\nimport Web.View.People.Show\n\ninstance Controller PeopleController where\n action PeopleAction = do\n people <- query @Person |> fetch\n render IndexView { .. }\n\n action NewPersonAction = do\n let person = newRecord\n render NewView { .. }\n\n action ShowPersonAction { personId } = do\n person <- fetch personId\n render ShowView { .. }\n\n action EditPersonAction { personId } = do\n person <- fetch personId\n render EditView { .. }\n\n action UpdatePersonAction { personId } = do\n person <- fetch personId\n person\n |> buildPerson\n |> ifValid \\case\n Left person -> render EditView { .. }\n Right person -> do\n person <- person |> updateRecord\n setSuccessMessage \"Person updated\"\n redirectTo EditPersonAction { .. }\n\n action CreatePersonAction = do\n let person = newRecord @Person\n person\n |> buildPerson\n |> ifValid \\case\n Left person -> render NewView { .. } \n Right person -> do\n person <- person |> createRecord\n setSuccessMessage \"Person created\"\n redirectTo PeopleAction\n\n action DeletePersonAction { personId } = do\n person <- fetch personId\n deleteRecord person\n setSuccessMessage \"Person deleted\"\n redirectTo PeopleAction\n\nbuildPerson person = person\n |> fill @[\"name\", \"email\"]\n"}
[ CreateFile {filePath = "Web/Controller/People.hs", fileContent = "module Web.Controller.People where\n\nimport Web.Controller.Prelude\nimport Web.View.People.Index\nimport Web.View.People.New\nimport Web.View.People.Edit\nimport Web.View.People.Show\n\ninstance Controller PeopleController where\n action PeopleAction = do\n people <- query @Person |> fetch\n render IndexView { .. }\n\n action NewPersonAction = do\n let person = newRecord\n render NewView { .. }\n\n action ShowPersonAction { personId } = do\n person <- fetch personId\n render ShowView { .. }\n\n action EditPersonAction { personId } = do\n person <- fetch personId\n render EditView { .. }\n\n action UpdatePersonAction { personId } = do\n person <- fetch personId\n person\n |> buildPerson\n |> ifValid \\case\n Left person -> render EditView { .. }\n Right person -> do\n person <- person |> updateRecord\n setSuccessMessage \"Person updated\"\n redirectTo EditPersonAction { .. }\n\n action CreatePersonAction = do\n let person = newRecord @Person\n person\n |> buildPerson\n |> ifValid \\case\n Left person -> render NewView { .. } \n Right person -> do\n person <- person |> createRecord\n setSuccessMessage \"Person created\"\n redirectTo PeopleAction\n\n action DeletePersonAction { personId } = do\n person <- fetch personId\n deleteRecord person\n setSuccessMessage \"Person deleted\"\n redirectTo PeopleAction\n\nbuildPerson person = person\n |> fill @'[\"name\", \"email\"]\n"}
, AppendToFile {filePath = "Web/Routes.hs", fileContent = "\ninstance AutoRoute PeopleController\n\n"}
, AppendToFile {filePath = "Web/Types.hs", fileContent = "\ndata PeopleController\n = PeopleAction\n | NewPersonAction\n | ShowPersonAction { personId :: !(Id Person) }\n | CreatePersonAction\n | EditPersonAction { personId :: !(Id Person) }\n | UpdatePersonAction { personId :: !(Id Person) }\n | DeletePersonAction { personId :: !(Id Person) }\n deriving (Eq, Show, Data)\n"}
, AppendToMarker {marker = "-- Controller Imports", filePath = "Web/FrontController.hs", fileContent = "import Web.Controller.People"}
Expand All @@ -215,6 +209,3 @@ tests = do
, CreateFile {filePath = "Web/View/People/Edit.hs", fileContent = "module Web.View.People.Edit where\nimport Web.View.Prelude\n\ndata EditView = EditView { person :: Person }\n\ninstance View EditView where\n html EditView { .. } = [hsx|\n {breadcrumb}\n <h1>Edit Person</h1>\n {renderForm person}\n |]\n where\n breadcrumb = renderBreadcrumb\n [ breadcrumbLink \"People\" PeopleAction\n , breadcrumbText \"Edit Person\"\n ]\n\nrenderForm :: Person -> Html\nrenderForm person = formFor person [hsx|\n {(textField #name)}\n {(textField #email)}\n {submitButton}\n\n|]"}
, AddImport {filePath = "Web/Controller/People.hs", fileContent = "import Web.View.People.Edit"}
]



0 comments on commit 4589034

Please sign in to comment.