Problem with changesets, which way is the best?

Phoenix Forum Questions / Help
, ,
1 
Hello everyone. I have Phoenix API app which has the following schema:

defmodule TattooBackend.Accounts.Studio do
  @moduledoc """
  Contains schema and changesets for Studio resource.
  """

  use Ecto.Schema

  import Ecto.Changeset
  import Ecto.Query

  alias TattooBackend.Accounts.Account
  alias TattooBackend.Accounts.Address

  schema "studios" do
    field :name,        :string
    field :description, :string

    belongs_to :account, Account
    has_one    :address, Address

    timestamps()
  end

  def changeset(studio, attributes \\ %{}) do
    studio
    |> cast(attributes, [:name, :account_id, :description])
    |> validate_required([:name, :account_id])
    |> unique_constraint(:name)
    |> foreign_key_constraint(:account_id)
  end
end

and in controller I have the following action that creates studios with all of its resources:

  def create(_conn, params) do
    account_params  = params["account"]  || %{}
    studio_params   = params["studio"]   || %{}
    address_params  = params["address"]  || %{}

    multi =
      Multi.new
      |> Multi.insert(:account, Account.changeset(%Account{}, account_params))
      |> Multi.run(:studio, fn %{account: account} ->
        %Studio{account_id: account.id}
        |> Studio.changeset(studio_params)
        |> Repo.insert
      end)
      |> Multi.run(:address, fn %{studio: studio} ->
        %Address{studio_id: studio.id}
        |> Address.changeset(address_params)
        |> Repo.insert
      end)

    case Repo.transaction(multi) do
      {:ok, result} ->
        result.account
        |> AccountMailer.account_confirmation()
        |> Mailer.deliver_later()

        {:ok, :created}

      {:error, _, changeset, %{}} ->
        {:error, changeset}
    end
  end

This code have one very dangerous problem. Studio.changeset/2 is casting account_id parameter so user can inject account_id into my params then new Studio will be created for existing account which is very bad. Solution for this problem is of course creating another changeset for creating and updating. I was also thinking about strong params way. I used the following library: https://github.com/vic/params and updated my controller action to something like this:

defparams create_params %{
    account: %{
      email: :string,
      password: :string,
      password_confirmation: :string,
    },
    studio: %{
      name: :string,
      description: :string,
    },
    address: %{
      street: :string,
      city: :string,
      zip_code: :string,
    }
  }

  def create(_conn, params) do
    new_params = params |> create_params |> Params.to_map

    account_params  = new_params[:account]  || %{}
    studio_params   = new_params[:studio]   || %{}
    address_params  = new_params[:address]  || %{}


    multi =
      Multi.new
      |> Multi.insert(:account, Account.changeset(%Account{}, account_params))
      |> Multi.run(:studio, fn %{account: account} ->
        %Studio{account_id: account.id}
        |> Studio.changeset(studio_params)
        |> Repo.insert
      end)
      |> Multi.run(:address, fn %{studio: studio} ->
        %Address{studio_id: studio.id}
        |> Address.changeset(address_params)
        |> Repo.insert
      end)

    case Repo.transaction(multi) do
      {:ok, result} ->
        result.account
        |> AccountMailer.account_confirmation()
        |> Mailer.deliver_later()

        {:ok, :created}

      {:error, _, changeset, %{}} ->
        {:error, changeset}
    end
  end

With that in place my action was secure from injecting data from malicious users. What do You think about this? What are best practices in Phoenix to solve this types of problems?

Thanks in advance!

2

I mean, all you need to do is have a changeset without :account_id on the cast list, and then it won’t be cast. You’re in fact encouraged to have different changeset functions for different purposes. Alternatively you can just always do %Studio{account_id: account.id} and remove it from the main changeset function cast list.

1 Like