Query generation fails for multiple permission clauses with same association but different conditions

[vincentvanbush]

Description

When defining multiple permission clauses for the same action and resource that reference the same association with different conditions (intended as OR alternatives), Permit.Ecto generates incorrect queries due to duplicate join bindings.

Expected Behavior

The following permission definition should create a query with OR'd conditions on a single join:

def can(%User{id: user_id}) do
  permit()
  |> create(Comment, note: [user_id: user_id])
  |> create(Comment, note: [public: true])
end

Expected SQL:

SELECT * FROM comments
INNER JOIN notes ON notes.id = comments.note_id
WHERE (notes.user_id = 123 OR notes.public = true)

Actual Behavior

The query generation fails or produces incorrect results because both disjunctions attempt to create joins with the same binding name (`"comment_note"`), causing:

1. Duplicate binding errors from Ecto
2. Second join overwriting the first
3. Loss of one condition, breaking the OR logic
4. Incorrect authorization results

Root Cause

The issue lies in `Permit.Ecto.Permissions.DynamicQueryJoiner`:

1. Association Path Extraction (`extract_assocs/1`, lines 48-56)

• Each disjunction's conditions are examined separately
• Association paths are extracted: `[note: [...], note: [...]]`
• No deduplication occurs at this stage

2. Join Construction (`add_joins/2` and `add_join/3`, lines 58-82)

Both disjunctions produce the same binding name:

defp add_join(root, key, acc) when is_atom(key) do
  binding = "#{root}_#{key}"  # Both produce "comment_note"
  join(acc, :inner, [{^root, p}], _ in assoc(p, ^key), as: ^binding)
end

The second call attempts to create a join with `as: "comment_note"`, conflicting with the first.

3. Missing Logic

• No check for existing bindings
• No special handling for duplicate associations across disjunctions
• The disjunctive (OR) semantic between clauses is lost during join construction

Minimal Reproduction

# Schema definitions
defmodule Note do
  use Ecto.Schema
  
  schema "notes" do
    field :user_id, :integer
    field :public, :boolean
    has_many :comments, Comment
  end
end

defmodule Comment do
  use Ecto.Schema
  
  schema "comments" do
    field :content, :string
    belongs_to :note, Note
  end
end

# Permissions
defmodule TestPermissions do
  use Permit.Ecto.Permissions, actions_module: Permit.Actions.CrudActions

  def can(%User{id: user_id}) do
    permit()
    |> create(Comment, note: [user_id: user_id])
    |> create(Comment, note: [public: true])
  end
end

# Attempting to construct query
user = %User{id: 123}
{:ok, query} = Permit.Ecto.Permissions.construct_query(
  TestPermissions.can(user),
  :create,
  Comment,
  user,
  Permit.Actions.CrudActions
)
# This will fail or produce incorrect results

Impact

This limitation prevents expressing common authorization patterns such as:

• "User can edit if they own it OR it's public"
• "User can view if they're a member OR it's in a public group"
• "User can comment if they authored the post OR commenting is enabled"

Current Workarounds

None that maintain the intended OR semantics with association conditions. Users must either:

1. Use function-based conditions (loses query optimization and database-level filtering)
2. Denormalize data to avoid associations
3. Handle authorization in application code instead of query level
4. Write custom authorization logic outside Permit.Ecto

Test Coverage Gap

Examined test/permit/permissions/query_construction_test.exs:

• ✅ Tests exist for multiple associations in a single clause
• ✅ Tests exist for nested associations
• ❌ NO tests for duplicate associations across multiple disjunctions

Environment

• Elixir version: 1.17.0
• OTP version: 27
• Permit.Ecto version: 0.1.x (current main branch)
• Ecto version: 3.12.x

Suggested Solution Direction

A proper fix might involve:

1. Detecting shared associations across disjunctions during extract_assocs/1
2. Creating a single join for shared associations
3. Distributing conditions across the OR'd dynamic query fragments
4. Ensuring binding uniqueness while maintaining query correctness

This would require refactoring how joins are constructed to be aware of the disjunctive structure, possibly by:

• Building joins once for unique association paths
• Applying different conditions to the same join binding in different OR branches
• Ensuring the dynamic query construction uses the correct bindings

Additional Context

This issue was discovered while implementing authorization for a notes application where comments can be created on notes that are either owned by the user or marked as public. The current workaround requires bypassing Permit.Ecto's query construction for this use case.

Related Code References

• lib/permit_ecto/permissions/dynamic_query_joiner.ex:48-56 - Association path extraction
• lib/permit_ecto/permissions/dynamic_query_joiner.ex:58-82 - Join construction
• lib/permit_ecto/permissions/dynamic_query_joiner.ex:23-45 - DNF to dynamic query conversion