Looks pretty good. You could optionally merge the user.id into the params earlier in the plug pipeline.

You mean creating a plug to inject the user id inside the parameters?

Personally, I’d pass user to create_book as a first argument. In create_book, I’d do something like this:

def create_book(user, book_params) do
  user
  |> build_assoc(:books)
  |> Book.changeset(book_params)
  |> Repo.create()
end

I would avoid putting user_id in the user supplied params, because then you’d have to allow it in the changeset. If you had another function that reused the changeset, the user might be able to inject a different user_id.

Or

  def create(conn, %{"book" => book_params}, user) do
    with {:ok, %Book{} = book} <- Books.create_book(book_params, for: user) do
      conn
      |> put_status(:created)
      |> put_resp_header("location", book_path(conn, :show, book))
      |> render("show.json", book: book)
    end
  end

def create_book(book_params, for: %User{id: user_id}) do
  %Book{user_id: user_id}
  |> Book.changeset(book_params)
  |> Repo.insert()
end

Hi @acrolink,

I personally like to handle this scenario on the schema itself where we handle the changeset creation and data validation. Just to be consistent where you do some specific actions in your code.

I leave the controller as simple as posible. For example:

defmodule Multiplatform.Documents.Detail do
use Ecto.Schema
import Ecto.Changeset

def changeset(%Detail{invoice_id: nil}, attrs, invoice) do
  Ecto.build_assoc(invoice, :details)
  |> cast(attrs, [:tax, :code, :desc, :up, :invoice_id])
  |> validate_required([:tax, :code, :desc, :up, :invoice_id])
end

def changeset(%Detail{} = detail, attrs, _invoice) do
  detail
  |> cast(attrs, [:tax, :code, :desc, :up, :invoice_id])
  |> validate_required([:tax, :code, :desc, :up, :invoice_id])
  |> mark_for_delete()
end

end

I pass the parent association Invoice to the changeset function to build the association if “invoice_id is nil”.

It is the same result but the difference is where you perform the association and that you have to import the Ecto Library or build_assoc function in the controller in order to use the function. So with this in mind, if I have to handle the association in the controller I would use the Map.put/3 or %{ map | key: value } syntax to add the associated id like you are already using it.

Hope this helps.

To reiterate what @blatyo posted above

I would avoid putting user_id in the user supplied params, because then you’d have to allow it in the changeset. If you had another function that reused the changeset, the user might be able to inject a different user_id.

How do you protect your database from the malicious clients who can potentially supply invalid invoice ids (or valid, but not belonging to them)? Sure, in your case it might be not important, but in general it’s quite dangerous … I’ve seen projects passing shop_id into there changesets, thus allowing me to modify any “shop” in their database without having any privileged access to it.

I totally get it.

In the controller I validate if the user has authorization to execute the action. I use bodyguard for authorization and coherence for authentication purposes.

I also scope queries based on the user and I retrieve user’s information form the users session. I also understand that you can implement a more secure environment using prefixes (for postgres) but this may not be the case.

How are you tackling this issue? or what is the recommended way to do it?

Thanks for your comments.

That’s a good practice, but yet when it comes to the identity of the currently logged in user, as far as I know it is not recommended to do and it is not easily accessible inside the schema definitions.

I leave all authorization logic to the context like here Should user permissions / data correctness logic live in my Ecto validations or Context logic? - #2 by idi527 and raise if something is wrong.