mwoods79 · Apr 29, 2026 · Apr 25, 2026 · Apr 25, 2026 · Apr 25, 2026 · Apr 25, 2026
diff --git a/config/dev.exs b/config/dev.exs
@@ -46,3 +46,6 @@ config :huddlz, Huddlz.Repo,
 
 # Adapters (compile-time - modules must exist at compile)
 config :huddlz, :storage, adapter: Huddlz.Storage.Local
+
+# CORS — allow all origins in development
+config :huddlz, :cors_origins, :all
@@ -101,4 +101,13 @@ if config_env() == :prod do
   config :huddlz, :storage,
     bucket: required!("BUCKET_NAME"),
     endpoint: required!("AWS_ENDPOINT_URL_S3")
+
+  cors_origins =
+    "CORS_ORIGINS"
+    |> optional("#{scheme}://#{host}")
+    |> String.split(",", trim: true)
+    |> Enum.map(&String.trim/1)
+    |> Enum.reject(&(&1 == ""))
+
+  config :huddlz, :cors_origins, cors_origins
 end
@@ -31,3 +31,6 @@ config :huddlz, :geocoding, adapter: Huddlz.MockGeocoding
 config :huddlz, :places, adapter: Huddlz.MockPlaces
 config :huddlz, geocoding_req_plug: {Req.Test, Huddlz.Geocoding.Google}
 config :huddlz, Huddlz.Repo, pool: Ecto.Adapters.SQL.Sandbox
+
+# CORS — allow all origins so tests don't have to configure them
+config :huddlz, :cors_origins, :all
@@ -0,0 +1,213 @@
+# API follow-ups
+
+Things consciously deferred while landing the JSON:API + GraphQL surface on
+`feat/api`. Each entry captures *why* it was skipped and *what to do* when
+it's time to come back to it, so we don't have to reconstruct the context.
+
+> **Status as of latest sweep:**
+> Items addressed in the "make `feat/api` shippable" pass:
+> the slug ergonomics fix, group destroy cascade, ProfilePicture upload
+> exposure, the GroupMember.remove_member JSON:API route, the real
+> multipart HTTP test, and the image-upload error mapping. Removed from
+> this doc. The remaining items below are genuinely deferred.
+
+## Pagination sweep
+
+**Status:** deferred.
+**Why:** Adding `pagination keyset?: true` to a read action changes the
+response shape — the result becomes a `KeysetPageOf<Type>` wrapper instead
+of a bare list, so any test or call site that pattern-matches on
+`{:ok, [list]}` breaks. The actions that ship paginated already (`Huddl.search`,
+`Huddl.by_group`) needed their tests written around `results { ... }`
+unwrapping. Sweeping the rest at the same time would have multiplied
+breakage well beyond the API change.
+**To do:**
+
+- Add `pagination keyset?: true, offset?: true, countable: true, required?: false`
+  to: `Huddl.upcoming`, `Huddl.past`, `Group.read`, `Group.search`,
+  `GroupMember.get_by_group`, `GroupMember.get_by_user`,
+  `HuddlAttendee.by_huddl`, `HuddlAttendee.by_user`,
+  `GroupLocation.by_group`.
+- Update each action's tests to wrap result-set assertions in `results { … }`
+  (GraphQL) or expect `meta.page` envelopes (JSON:API).
+- Add a per-resource `default_limit` (20) and a max so large lists don't
+  fall back to "stream the whole table".
+
+## OpenAPI / GraphiQL prose polish
+
+**Status:** deferred.
+**Why:** Descriptions exist on actions and arguments but they were written
+for resource-internal callers. The phrasing reads awkwardly when surfaced
+to API consumers via Swagger UI or GraphiQL. This is a writing task more
+than a code task.
+**To do:**
+
+- Walk through `/api/json/swaggerui` and rewrite descriptions on each
+  exposed action to address API consumers ("Create a huddl" not
+  "Create a huddl with the given attributes via the create action").
+- Add `@deprecated` directives on the legacy `:create` actions for
+  `HuddlImage` / `GroupImage` (they're now superseded by `:upload` for
+  API use; the manual-create actions should stay for internal callers
+  that pre-compute storage paths).
+- Add example bodies to the OpenAPI spec via `OpenApiSpex` example
+  schemas.
+
+## User update mutations use `id:` arg
+
+**Status:** functional but un-ergonomic. Attempted fix in the
+shippable-API sweep; deferred again after the AshGraphql/Ash 3.x
+interaction proved more involved than expected.
+**Why:** `updateDisplayName(id: $id, input: { displayName: $name })`
+takes the user id explicitly even though the policy already
+restricts the action to `^actor(:id)`. The client can pass any id
+and the policy rejects mismatches; passing the id is noise.
+
+The clean fix — `update :update_display_name, :update_display_name,
+read_action: :me, identity: false` — produces an Ecto error at run
+time:
+
+> `update_all` does not allow subqueries in `from`
+
+because `:me` filters on `^actor(:id)`, which Ash's atomic update
+path turns into a subquery wrapping the update statement. Postgres
+rejects that. Setting `require_atomic? false` on the update action
+didn't bypass the path either.
+
+**To do:**
+
+- Investigate AshGraphql 1.9's mutation pipeline and figure out
+  whether there's a `read_action`-compatible shape that doesn't go
+  through bulk update (a `before_action` lookup, or a custom
+  resolver, or an Ash patch).
+- Alternative: build a generic action wrapper (`update_self`) that
+  takes only the input fields, loads the actor explicitly, and
+  delegates to the underlying update. Expose that action as the
+  mutation. More code than the `read_action` shortcut but works
+  around the atomic-update constraint.
+
+## API key strategy: scopes, rotation, audit
+
+**Status:** v1 ships with full-permission keys and manual revocation.
+**Why:** Sufficient for a single first-party SPA / one trusted
+machine consumer. Adding scopes / fine-grained permissions before
+there's a second consumer is premature.
+**To do (when needed):**
+
+- Per-key scopes: `read_only`, `events:write`, `groups:admin`. Store
+  on the `ApiKey` resource as an array of atoms; check via a custom
+  policy.
+- Rotation: a `rotate_api_key` mutation that creates a new key and
+  marks the old one for deletion in N days.
+- Audit log: record every API-key authentication event so a
+  compromised key can be traced.
+
+## Rate limiting
+
+**Status:** none.
+**Why:** No threat model yet — only first-party clients in flight.
+**To do:**
+
+- Add `:hammer` (Redis-backed in prod, ETS in dev) plug in front of
+  `/api/auth/*` first (brute-force protection on register and
+  sign_in).
+- Per-user-or-key buckets on `/api/json/*` and `/gql`. Start
+  permissive — the goal is to stop runaway scripts, not to throttle
+  legitimate use.
+
+## RemoveMemberByIds membership-existence leak
+
+**Status:** known, acceptable for v1.
+**Why:** `Huddlz.Communities.GroupMember.Actions.RemoveMemberByIds`
+does `Ash.read_one(authorize?: false)` on `(group_id, user_id)` and
+then dispatches `:remove_member` with the actor. The destroy is the
+real auth gate, so unauthorized callers can't change state — but the
+response shape distinguishes "membership not found" (404) from
+"found but forbidden" (403), letting any authenticated actor probe
+membership pairs. Group membership in a public group isn't very
+secret; private groups would leak.
+**To do:**
+
+- Fold into the API-key scopes work above. Once a `groups:admin`
+  scope exists, the lookup can be filtered to "groups owned by the
+  actor" and the probe closes naturally.
+- Cheaper interim fix: collapse 404 and 403 to a single 404 response
+  shape so the failure modes are indistinguishable.
+
+## User-FK cascade asymmetry
+
+**Status:** deferred.
+**Why:** Migration `20260427030225_cascade_destroy_groups_and_huddls`
+cascades `groups → group_members / group_images / huddlz` and
+`huddlz → huddl_images / huddl_attendees`, but the three FKs back to
+`users` are **not** cascaded: `group_members.user_id`,
+`huddl_attendees.user_id`, `huddlz.creator_id`. There's no
+delete-account flow today, so this isn't blocking.
+**To do (when delete-account ships):**
+
+- Follow-up migration. `group_members` and `huddl_attendees` are
+  pure join rows — `on_delete: :delete_all` is fine.
+- `huddlz.creator_id` needs a product call **before** the FK
+  changes: either soft-delete and anonymize the creator, or
+  transfer ownership to the group owner. Don't `:delete_all` huddlz
+  when their creator deletes their account.
+
+## sign_out path doesn't apply to API-key auth
+
+**Status:** functional but UX-confusing.
+**Why:** `DELETE /api/auth/sign_out` only revokes JWT bearers
+(it calls `TokenResource.Actions.revoke/2`). An actor authenticated
+via an API key gets the same generic
+`{"error": "Authentication required"}` 401 as an unauthenticated
+caller, with no hint that the real revoke endpoint is
+`DELETE /api/auth/api_keys/:id`.
+**To do (when there's a real API-key consumer):**
+
+- Detect the auth method on the conn and either
+  (a) return a 400 with a pointer to `/api/auth/api_keys/:id`, or
+  (b) destroy the active API key when sign_out is called with one.
+  (a) is simpler and probably better — sign_out and key revocation
+  are conceptually different.
+
+## Cucumber `.feature` for the happy path
+
+**Status:** end-to-end test ships as a single ExUnit case
+(`test/huddlz_web/api/end_to_end_test.exs`).
+**Why:** A Cucumber `.feature` file is the more readable format
+for product/QA review, but writing the step definitions for a
+multi-bearer, multi-protocol flow (JSON:API + GraphQL with token
+swaps) is its own project.
+**To do:**
+
+- Port `end_to_end_test.exs` to a `.feature` file with step
+  definitions. Probably easier once we have a small step library
+  for `Given I am authenticated as ...` / `When I POST ... with body ...` /
+  `Then the response is ...`.
+
+## Flaky `profile_picture.feature` member-avatar step
+
+**Status:** intermittent failure observed during the PR-113 review
+sweep.
+**Why:** The Cucumber scenario "Profile picture appears in group
+members section" (`test/features/profile_picture.feature:45`) fails
+the `Then I should see the member avatar with image` step under some
+seeds — the selector `main img[src*='_thumb.jpg']` finds nothing.
+Re-running the same file with `--seed 0` is green, so it's a timing
+or fixture-ordering flake rather than a bug in the assertion. Looks
+unrelated to feat/api code, but it surfaces while the suite is hot
+on the branch.
+**To do:**
+
+- Reproduce with a sticky seed: bisect via `--seed <n>` to find a
+  failing one, then capture the rendered HTML at the failing step.
+- Likely culprits: profile-picture upload running through a Mox
+  expectation that another async test consumed first, or
+  `current_profile_picture_url` aggregate not having loaded by the
+  time the Liveview renders. The `Mox.stub_with` setup in
+  `test/features/support/hooks.exs` is the place to verify.
+- Once root-caused, either pin the scenario's stubs explicitly or
+  await the avatar render in the step definition before asserting.
+
+---
+
+When picking one of these up, tag the commit message with the section
+heading so we can crosswalk back to this doc.
@@ -4,7 +4,8 @@ defmodule Huddlz.Accounts do
   """
 
   use Ash.Domain,
-    otp_app: :huddlz
+    otp_app: :huddlz,
+    extensions: [AshJsonApi.Domain]
 
   resources do
     resource Huddlz.Accounts.Token
@@ -28,5 +29,7 @@ defmodule Huddlz.Accounts do
       define :delete_profile_picture, action: :destroy
       define :soft_delete_profile_picture, action: :soft_delete
     end
+
+    resource Huddlz.Accounts.ApiKey
   end
 end
@@ -0,0 +1,89 @@
+defmodule Huddlz.Accounts.ApiKey do
+  @moduledoc """
+  API keys for machine-to-machine authentication.
+
+  Plaintext keys are returned only at create time via the
+  `plaintext_api_key` metadata; only the hash is persisted.
+  """
+
+  use Ash.Resource,
+    otp_app: :huddlz,
+    domain: Huddlz.Accounts,
+    data_layer: AshPostgres.DataLayer,
+    authorizers: [Ash.Policy.Authorizer]
+
+  postgres do
+    table "api_keys"
+    repo Huddlz.Repo
+
+    references do
+      reference :user, on_delete: :delete
+    end
+
+    custom_indexes do
+      index [:user_id]
+    end
+  end
+
+  actions do
+    defaults [:read, :destroy]
+
+    create :create do
+      primary? true
+      accept [:expires_at]
+
+      change relate_actor(:user)
+
+      change {AshAuthentication.Strategy.ApiKey.GenerateApiKey,
+              prefix: :huddlz, hash: :api_key_hash}
+    end
+  end
+
+  policies do
+    bypass AshAuthentication.Checks.AshAuthenticationInteraction do
+      authorize_if always()
+    end
+
+    policy action(:create) do
+      description "Authenticated users can create API keys for themselves"
+      authorize_if actor_present()
+    end
+
+    policy action(:read) do
+      description "Users can read their own API keys"
+      authorize_if expr(user_id == ^actor(:id))
+    end
+
+    policy action(:destroy) do
+      description "Users can revoke their own API keys"
+      authorize_if expr(user_id == ^actor(:id))
+    end
+  end
+
+  attributes do
+    uuid_primary_key :id
+
+    attribute :api_key_hash, :binary do
+      allow_nil? false
+      sensitive? true
+    end
+
+    attribute :expires_at, :utc_datetime_usec do
+      allow_nil? false
+    end
+  end
+
+  relationships do
+    belongs_to :user, Huddlz.Accounts.User do
+      allow_nil? false
+    end
+  end
+
+  calculations do
+    calculate :valid, :boolean, expr(expires_at > now())
+  end
+
+  identities do
+    identity :unique_api_key, [:api_key_hash]
+  end
+end