Cast_assoc is null?

I think most of elixir community vulnerable. Really thanks i see always cast_assocs or id in changesets. This is the first time i see your approach. But on the other hand we already modify user_id from controller so somebody cant put another user_id right ?

Thanks everyone, got it working, have to the community around Elixir is just amazing. Now to the next challenge / task

In your example above, yes. Since you use Map.put(params, "user_id", ...) it replaces the "user_id" provided in the params.

But what if you or someone else who starts working on your codebase forgets to use Map.put/3 or to delete the "user_id" key from params, or uses Map.put_new/3 in some other controller using the same changeset function (which casts user_id), or replaces :user_id with :owner_id in the schema but forgets to update Map.put(params, "user_id", ...) with Map.put(params, "owner_id", ...) in the controller?

Then the "user_id" from params leaks into the changeset function and you have a vulnerability. Any user can send whatever "user_id" they want thus completely bypassing any authentication logic you have.

The only way to avoid it, as I see it, is to not cast user_id (or any other foreign key) inside the changeset functions but put it into the schema structs explicitly when needed.

1 Like

@idi527 would you do the same thing for many_to_many

I have

schema "fluxes" do
    field :description, :string
    field :title, :string
    many_to_many :magnets, Magnify.Magnets.Magnet, join_through: "magnets_fluxes"

    timestamps()
  end

  @doc false
  def changeset(flux, attrs) do
    flux
    |> cast(attrs, [:title, :description])
    |> validate_required([:title, :description])
  end 

And not sure the correct to add magnets use puts_assoc but not sure?

I posted about this topic a few days ago:

The scenario might be similar but keep in mind that using cast_assoc requires you to handle the parent and child at the same time.

Hope this helps.

Best regards,

I’d probably use an ecto multi to insert the flux and create relationships between it and some magnets inside a single database transaction.

Adapted from Is there are way to bulk insert join records of a many-to-many relationship with Ecto?

# in the module containing your business logic

# here I suppose that the magnets already exist
# and we only need to associate them with the 
# created flux
@spec create_flux(%{(String.t | atom) => term}, [magnet_id :: pos_integer]) :: {:ok, %Flux{}} | {:error, Ecto.Changeset.t}
def create_flux(flux_attrs, magnet_ids) do
  alias Ecto.Multi

  flux_changeset = Flux.changeset(%Flux{}, flux_attrs)

  Multi.new()
  |> Multi.insert(:flux, flux_changeset)
  |> Multi.run(:magnet_relationships, fn %{flux: %Flux{id: flux_id}} ->
    # prepare relationships to be inserted into the many-to-many table
    relationships = Enum.map(magnet_ids, fn magnet_id ->
      %{magnet_id: magnet_id, flux_id: flux_id}
    end)
    
    # https://hexdocs.pm/ecto/Ecto.Repo.html#c:insert_all/3
    {:ok, Repo.insert_all("magnets_fluxes", relationships)}
  end)
  |> Repo.transaction()
  |> case do
    {:ok, %{flux: flux}} -> {:ok, flux}
    {:error, :flux, changeset, _changes} -> {:error, changeset}
  end
end

Used like this

flux_attrs = %{title: "a flux", description: "a flux"}
magnet_ids = [1, 2, 3] # I suppose these already exist in the database

create_flux(flux_attrs, magnet_ids)

@idi527 this is great, thanks appreciate it

I’m getting - function Magnify.Magnets.create_flux/1 is undefined or private. Did you mean one of:

Called with 1 arguments
%{"description" => "fff", "magnet_id" => ["2", "3"], "title" => "aff"}

I have the following in my templates

<%= multiple_select f, :magnet_id, @magnets, class: "form-control" %>

and the controller
magnets = Magnets.list_magnets(params) |> Enum.map(&{&1.description, &1.id})

I’m thinking that is were the issue is ?

Ok I know what the issue, is it’s the calling the create which only has case Magnets.create_flux(flux_params) do

just need to figure how to get magent_ids as that is inside flux_params

"Parameters: %{"_csrf_token" => "VxwHMmkfLwMhPhxuHRVcfn0DGTgfEAAA1lDzBRjRLGIVyqsL67MTOA==", "_utf8" => "✓", "flux" => %{"description" => "aaa", "magnet_id" => ["2"], "title" => "aaaa"}}
"

Maybe

magnet_ids = params["magnet_id"] || []
create_flux(params, magnet_ids)

But then you might need to check that the user actually has access to the magnet ids in params.

Well the create is like so

 def create(conn, %{"flux" => flux_params}) do
    IO.inspect flux_params
    case Magnets.create_flux(flux_params) do
      {:ok, flux} ->
        conn
        |> put_flash(:info, "Flux created successfully.")
        |> redirect(to: flux_path(conn, :show, flux))
      {:error, %Ecto.Changeset{} = changeset} ->
        render(conn, "new.html", changeset: changeset)
    end
  end

and flux_params had the keys

"Parameters: %{"_csrf_token" => "VxwHMmkfLwMhPhxuHRVcfn0DGTgfEAAA1lDzBRjRLGIVyqsL67MTOA==", "_utf8" => "✓", "flux" => %{"description" => "aaa", "magnet_id" => ["2"], "title" => "aaaa"}}

  def create(conn, %{"flux" => flux_params}) do
    magnet_ids = flux_params["magnet_id"] || []
    case Magnets.create_flux(flux_params, magnet_ids) do
      {:ok, flux} ->
        conn
        |> put_flash(:info, "Flux created successfully.")
        |> redirect(to: flux_path(conn, :show, flux))
      {:error, %Ecto.Changeset{} = changeset} ->
        render(conn, "new.html", changeset: changeset)
    end
  end

I did that got

Postgrex expected an integer in -9223372036854775808…9223372036854775807, got “1”. Please make sure the value you are passing matches the definition in your table or in your query or convert the value accordingly.

Which is working but the many to many is the wrong format

what I have
many_to_many :magnets, Magnify.Magnets.Magnet, join_through: “magnets_fluxes”

  def create(conn, %{"flux" => flux_params}) do
    magnet_ids = Enum.map(flux_params["magnet_id"] || [], fn magnet_id -> 
      String.to_integer(magnet_id)
    end)
    
    case Magnets.create_flux(flux_params, magnet_ids) do
      {:ok, flux} ->
        conn
        |> put_flash(:info, "Flux created successfully.")
        |> redirect(to: flux_path(conn, :show, flux))
      {:error, %Ecto.Changeset{} = changeset} ->
        render(conn, "new.html", changeset: changeset)
    end
  end

Well It’s getting there I now got

schema Magnify.Magnets.Flux does not have association :magents

Which I have no idea what is going on I added the many to many ?

What’s the file / line where this error is raised? And what’s on that line?

Sweet it’s working, holy shit. I have so much more to learn. It’s failing

 def show(conn, %{"id" => id}) do
    flux = Magnets.get_flux!(id)
    render(conn, "show.html", flux: flux)
  end 

flux_controller.ex:35: MagnifyWeb.FluxController.show/2

which is this part flux = Magnets.get_flux!(id)

Happing after the create which than renders the single view template via show

Sweet it’s working, holy shit. I have so much more to learn. It’s failing

So is it working or is it failing? I don’t see any mention of :magnets association in

 def show(conn, %{"id" => id}) do
    flux = Magnets.get_flux!(id)
    render(conn, "show.html", flux: flux)
  end 

Maybe the problem is in the "show.html.eex" template. Or maybe check your magnets schema again … Maybe there is a typo? Like :magents instead of :magnets judging by

schema Magnify.Magnets.Flux does not have association :magents

Working, amazing thanks for the all help had an issue with the following old code I trying to get working

def get_flux!(id)
      do Repo.get!(Flux, id)
          |> Repo.preload(:magents)
      end

just need to remove |> Repo.preload(:magents)

working with this

def get_flux!(id)
      do Repo.get!(Flux, id)
  end

There’s a type in Repo.preload. :magents instead of :magnets.