idi527
May 19, 2018, 10:57am
17
tl;dr
Just don’t put it into the params / changeset. Treat the params as an opaque data structure.
# in your controller
def create(conn, %{"magnet" => magnet_params}) do
case Magnets.create_magnet(magnet_params, for: conn.assigns.current_user) do
{:ok, magnet} ->
conn
|> put_flash(:info, "Magnet created successfully.")
|> redirect(to: magnet_path(conn, :show, magnet))
{:error, %Ecto.Changeset{} = changeset} ->
render(conn, "new.html", changeset: changeset)
end
end
# in your business logic module
def create_magnet(attrs, for: %User{id: user_id}) do
%Magnet{user_id: user_id}
|> Magnet.changeset(attrs)
|> Repo.insert()
end
# in the module where you schema / changesets are defined
def changeset(manget, attrs) do
magnet
|> cast(attrs, [:title, :description, :content])
|> validate_required([:title, :description, :content])
end
opened 08:49AM - 09 Mar 18 UTC
feature
👋
Not particularly phoenix related, but is it possible to catch if foreign ke… ys can be set from `params` passed in by a user through a controller action? Abusing different ecto `cast`s down the line (up the stack?) allows attackers to modify resources that don't belong to them.
I've seen many phoenix projects do something like this
```elixir
# controller
def some_action(conn, params) do
# ...
Resources.create(params) # or update, or delete
# ...
end
# schema / changeset
schema "resources" do
# ...
belongs_to(:something, Something)
# ...
end
def changeset(resource, attrs) do
resource
|> cast(attrs, [:something_id, ...])
# or cast_assoc(:something)
# or put_assoc(:something)
end
# resources context
def create(attrs) do
%Resource{}
|> Resource.changeset(attrs)
|> Repo.insert()
end
```
As a possible solution / suggestion:
```elixir
# don't cast foreign keys in Resource.changeset
# but pass them in Resources.create
@spec create(attrs, for: pos_integer) :: {:ok, Resource.t} | {:error, Ecto.Changeset.t}
def create(attrs, for: something_id) do
%Resource{something_id: something_id}
|> Resource.changeset(attrs)
|> Repo.insert()
end
# or
@spec create(attrs, for: Something.t) :: {:ok, Resource.t} | {:error, Ecto.Changeset.t}
def create(attrs, for: %Something{id: something_id}) do
%Resource{something_id: something_id}
|> Resource.changeset(attrs)
|> Repo.insert()
end
# controller action then becomes
def some_action(conn, params) do
# something or something_id are set by some plug (possibly based on user id)
something = conn.assigns.something
# or something_id = conn.assigns.something_id
Resources.create(params, for: something)
# ...
end
```
menu = Cms.Content.create_menu(%{name: "Test Menu"})
What’s returned from Cms.Content.create_menu/1? Is it {:ok, menu}?
I would probably avoid casting foreign keys in changesets (can open you up for vulnerabilities if attrs come from user input), but pass them to the create function “manually”.
@spec create_menu_item(map, for: %Menu{}) :: {:ok, %MenuItem{}} | {:error, Ecto.Changeset.t()}
def create_menu_item(attrs, for: %Menu{id: menu_id}) do
%MenuItem{menu_id: menu_id}
|> MenuItem.chang…
Is there any reason that phoenix does not add these fields by default in validation ?
Casting foreign keys from outside data can open you up for some nasty vulnerabilities.
I saw a project that casted something like shop_id in their changesets and I could do whatever I wanted with them since I was practically “the owner” of every shop in their database.
The fact that it’s so easy to do in ecto + phoenix is a bit worrisome.
Hello folks!
I have a question about how to deal with Context, in particular I see that from the perspective of controller tests all the keys of the params are passed as strings, while from the perspective of context|model tests we pass all the params keys as atoms.
That’s not make a big difference cause what matter is that all the keys of a map have to be of the same type, but …
I need to use a Service inside one of my Context so that based on the result I get from it I need to add a new key…
1 Like
