Give an account an address to send notifications to

5bdf3bc06ce6 · AtlantisPleb · · parent e57f5ea8b166

Give an account an address to send notifications to

Issue #141 asked for the email channel to start with an address to send to,
and that is what this is: an address, the proof its owner asked for it, and
one notification wired through end to end.

The address is typed into the notification settings rather than taken from
GitHub. Asking OAuth for `user:email` would hand over an address nobody chose
to give this deployment, and the first use of it would be unsolicited mail to
a mailbox its owner never named here.

A typed address is inert. `EmailChannel.verified_address/1` is the only read
any send resolves a recipient through, and it returns nil until a code mailed
to that address comes back. The code is held as a SHA-256 digest, compared in
constant time, expires in thirty minutes, and is retired after five wrong
guesses; resends are bounded to one a minute. The check constraint
`users_notification_email_state_check` refuses a verified timestamp on a row
that names no address, so the gate holds even where this module is wrong.

`email_enabled` is a channel rather than a category, defaults off, and is
backfilled false: turning on a channel that leaves the application for
everybody who chose the inbox is not a default anyone picked. Only mentions
are carried. The other categories follow the traffic on a thread, and one
message per comment is what a digest exists to avoid — that remains open.

The send cannot inherit the durability the inbox gets for free, so
`Delivery.enqueue/1` writes an `email.delivery` effect inside the transaction
that writes the record, keyed to the same `(user_id, dedupe_key)` pair the
unique index uses. `Notifications.email_dispatch/1` then re-decides every
gate at send time — confirmed address, channel, category, and repository
access — because a sent message is the one notification no later
authorization check can withdraw. A refusal completes with its reason instead
of retrying; only an unreachable provider stays pending for the outbox.

The payload names an account and an event and never an address, so no caller
can put a recipient into the queue. A message carries the kind, the actor's
login, the repository path, the issue number, and a link. Not the title.

A deployment with no mail provider says so, and the settings surface declines
to collect an address rather than accepting one it cannot mail to.
`config/runtime.exs` reads Resend, Postmark, or Mailgun credentials from the
environment per RELEASE-002; none of them adds a dependency.

NOTIFY-001 said delivery was in-product only. It now states the four gates
and their proofs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KnhfrafYx5ZGaMbzZEJQ2d
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>

Deploy story

What this commit did to the running system — joined from the forge receipt chain, the part a commit page elsewhere cannot show.

pushed
by user · WAL seq 382 · 2026-08-25T14:38:00.292325Z

Changed files

  • modified INVARIANTS.md
  • modified config/config.exs
  • modified config/prod.exs
  • modified config/runtime.exs
  • modified docs/taxonomy.md
  • modified lib/openagents/accounts/user.ex
  • modified lib/openagents/data_rights/account_export.ex
  • modified lib/openagents/effects/handlers/email_delivery.ex
  • modified lib/openagents/notifications.ex
  • modified lib/openagents/notifications/delivery.ex
  • added lib/openagents/notifications/delivery/mailer_adapter.ex
  • modified lib/openagents/notifications/delivery/null_adapter.ex
  • added lib/openagents/notifications/email.ex
  • added lib/openagents/notifications/email_channel.ex
  • modified lib/openagents/notifications/preference.ex
  • modified lib/openagents_web/live/notifications_live.ex
  • modified priv/migration_lineages/prior-2026-08-19.json
  • added priv/repo/migrations/20260825140544_add_notification_email_address_to_users.exs
  • added priv/repo/migrations/20260825140545_add_email_channel_to_notification_preferences.exs
  • modified test/openagents/notifications/delivery_test.exs
  • added test/openagents/notifications/email_channel_test.exs
  • added test/openagents/notifications/email_delivery_test.exs
  • modified test/openagents_web/live/notifications_live_test.exs

Diff

23 files changed, +1761 -106

INVARIANTS.md modified +47 -4

@@ -5321,13 +5321,56 @@ stays correct past the page the inbox renders and drops repositories the reader

5321 5321
can no longer read. It is keyed by the session's own account and refreshed only
5322 5322
over that account's own topic.
5323 5323
5324
Delivery is in-product only. Accounts carry no email address and no outbound
5325
mail adapter is configured, so no channel here leaves the application.
5324
Delivery is in-product first and by email second, and the second channel is
5325
the only one that leaves the application. Four things gate it, and all four are
5326
decided twice — once when the delivery is queued and again when it is sent —
5327
because a sent message is the one notification no later authorization check can
5328
withdraw.
5329
5330
The address is the first gate. An account has no address until somebody types
5331
one here; GitHub OAuth is not asked for `user:email`, so no address arrives that
5332
its owner did not choose to give this deployment. A typed address is inert:
5333
`OpenAgents.Notifications.EmailChannel.verified_address/1` is the only read any
5334
send resolves a recipient through, and it returns `nil` until a code mailed to
5335
that address comes back. The code is held as a SHA-256 digest, compared in
5336
constant time, expires in thirty minutes, and is retired after five wrong
5337
guesses. The check constraint `users_notification_email_state_check` refuses a
5338
verified timestamp on a row that names no address, so the gate holds
5339
independently of this application's code. The verification message is the one
5340
thing sent to an unconfirmed address, and it carries the code and nothing about
5341
the account.
5342
5343
The channel is the second. `email_enabled` defaults off and is backfilled false
5344
for every account that already exists, because turning on a channel that leaves
5345
the application for people who chose the inbox is not a default anybody picked.
5346
The category is the third: only `mention` is carried, so no other category
5347
becomes an email by widening. Repository access is the fourth, re-read through
5348
`Repositories.readable_by/2` at send time.
5349
5350
The queued half is durable rather than immediate, because a send happens after
5351
the transaction commits and can fail on somebody else's server.
5352
`Delivery.enqueue/1` writes one `email.delivery` effect inside the transaction
5353
that writes the record, keyed to the same `(user_id, dedupe_key)` pair the
5354
unique index uses, so a replayed fan-out is one record and one message.
5355
`OpenAgents.Effects` owns attempts, backoff, and the terminal `failed`. A
5356
refusal at send time is recorded as a completion with its reason rather than
5357
retried, because none of these four answers improves by being asked again. The
5358
payload names an account and an event and never an address, so no caller can
5359
put a recipient into the queue.
5360
5361
A message carries what a record carries — the kind, the actor's login, the
5362
repository path, and the issue number — plus a link. Not the issue's title, and
5363
not a body.
5326 5364
5327 5365
Evidence: `OpenAgents.Notifications`, `OpenAgents.Notifications.Mentions`,
5366
`OpenAgents.Notifications.EmailChannel`, `OpenAgents.Notifications.Delivery`,
5367
`OpenAgents.Notifications.Email`, `OpenAgents.Effects.Handlers.EmailDelivery`,
5328 5368
`OpenAgents.Issues.update_issue/3`, `OpenAgentsWeb.NotificationsLive`,
5329 5369
`OpenAgentsWeb.UserAuth.on_mount/4`, `test/openagents/notifications_test.exs`,
5330
and `test/openagents_web/live/notifications_live_test.exs`.
5370
`test/openagents/notifications/email_channel_test.exs`,
5371
`test/openagents/notifications/email_delivery_test.exs`,
5372
`test/openagents/notifications/delivery_test.exs`, and
5373
`test/openagents_web/live/notifications_live_test.exs`.
5331 5374
5332 5375
### FORGEAPI-001 — One error envelope, and a route inventory derived from the router
5333 5376

@@ -5456,7 +5499,7 @@ contract; the invariant prose above defines the assertion, not the filename.

5456 5499
| WORK-002 | `test/openagents/box_runs_test.exs` |
5457 5500
| PROMISE-001 | `test/openagents/promise_registry_test.exs`, `test/openagents_web/controllers/project_controller_test.exs` |
5458 5501
| PROMISE-002 | `test/openagents/promise_registry_test.exs` |
5459
| NOTIFY-001 | `test/openagents/notifications_test.exs`, `test/openagents_web/live/notifications_live_test.exs` |
5502
| NOTIFY-001 | `test/openagents/notifications_test.exs`, `test/openagents/notifications/email_channel_test.exs`, `test/openagents/notifications/email_delivery_test.exs`, `test/openagents/notifications/delivery_test.exs`, `test/openagents_web/live/notifications_live_test.exs` |
5460 5503
5461 5504
| FORGEAPI-001 | `test/openagents_web/controllers/api_error_contract_test.exs`, `test/openagents_web/controllers/api_extension_controller_test.exs`, `test/openagents_web/api_error_test.exs`, `test/openagents_web/controllers/issue_controller_test.exs` |
5462 5505
| FORGEAPI-002 | `test/openagents_web/api_version_posture_test.exs`, `test/openagents_web/plugs/api_v3_rewrite_test.exs` |
config/config.exs modified +16

@@ -511,6 +511,22 @@ config :phoenix_live_view,

511 511
# at the `config/runtime.exs`.
512 512
config :openagents, OpenAgents.Mailer, adapter: Swoosh.Adapters.Local
513 513
514
# The notification email channel (#141).
515
#
516
# `deliverable` is what the settings surface asks before it offers to take an
517
# address, and it is set here rather than inferred from the Swoosh adapter,
518
# because the adapter answers the question wrongly in both directions: the
519
# local adapter is real delivery in development, where the message lands in the
520
# mailbox preview at "/dev/mailbox", and it is a black hole in production.
521
# `config/prod.exs` turns it off, and `config/runtime.exs` turns it back on for
522
# a deployment that configured a provider.
523
config :openagents, OpenAgents.Notifications.EmailChannel,
524
  from: {"OpenAgents", "notifications@openagents.com"},
525
  deliverable: true
526
527
config :openagents, OpenAgents.Notifications.Delivery,
528
  adapter: OpenAgents.Notifications.Delivery.MailerAdapter
529
514 530
# Product analytics (docs/2026-08-21-posthog-integration-runbook.md). The
515 531
# default supervisor stays off; OpenAgents.Application starts PostHog.Supervisor
516 532
# only when a project token was configured at boot. Error tracking is a
config/prod.exs modified +6

@@ -31,6 +31,12 @@ config :swoosh, api_client: Swoosh.ApiClient.Req

31 31
# Disable Swoosh Local Memory Storage
32 32
config :swoosh, local: false
33 33
34
# No mail provider until `config/runtime.exs` finds one in the environment. The
35
# local adapter is still configured, so a stray send would be swallowed rather
36
# than raised; saying so here means the settings surface declines to collect an
37
# address instead of accepting one this deployment cannot mail (#141).
38
config :openagents, OpenAgents.Notifications.EmailChannel, deliverable: false
39
34 40
# Do not print debug messages in production
35 41
config :logger, level: :info
36 42
config/runtime.exs modified +39

@@ -173,6 +173,45 @@ if config_env() == :prod do

173 173
  config :openagents, :changelog_backfill_on_boot, true
174 174
end
175 175
176
# The mail provider for the notification email channel (#141). Absent by
177
# default: a deployment that sets no provider keeps `deliverable: false` from
178
# `config/prod.exs`, the settings surface declines to collect an address, and
179
# nothing is queued for a channel that cannot send.
180
#
181
# The API key is read here and nowhere else, per RELEASE-002: it enters from the
182
# environment at boot and is absent from source, the build context, and image
183
# arguments. All three providers are HTTP APIs served by `Swoosh.ApiClient.Req`,
184
# so none of them adds a dependency or an SMTP credential.
185
if config_env() == :prod and optional_text.("OPENAGENTS_MAIL_PROVIDER") != nil do
186
  mail_from_address = required_text.("OPENAGENTS_MAIL_FROM")
187
  mail_api_key = required_text.("OPENAGENTS_MAIL_API_KEY")
188
189
  mailer_config =
190
    case required_text.("OPENAGENTS_MAIL_PROVIDER") do
191
      "resend" ->
192
        [adapter: Swoosh.Adapters.Resend, api_key: mail_api_key]
193
194
      "postmark" ->
195
        [adapter: Swoosh.Adapters.Postmark, api_key: mail_api_key]
196
197
      "mailgun" ->
198
        [
199
          adapter: Swoosh.Adapters.Mailgun,
200
          api_key: mail_api_key,
201
          domain: required_text.("OPENAGENTS_MAIL_DOMAIN")
202
        ]
203
204
      _invalid ->
205
        raise "environment variable OPENAGENTS_MAIL_PROVIDER must be resend, postmark, or mailgun"
206
    end
207
208
  config :openagents, OpenAgents.Mailer, mailer_config
209
210
  config :openagents, OpenAgents.Notifications.EmailChannel,
211
    from: {"OpenAgents", mail_from_address},
212
    deliverable: true
213
end
214
176 215
if config_env() == :prod and runtime_role == :web do
177 216
  runtime_environment =
178 217
    case required_text.("OPENAGENTS_ENVIRONMENT") do
docs/taxonomy.md modified +26

@@ -645,6 +645,32 @@ source of truth for paths and JSON shape.

645 645
**Effect CLI** — the TypeScript CLI (`@openagentsinc/cli`) that calls this
646 646
surface via `openagents api`.
647 647
648
### Notifications
649
650
**Category** — what an account hears about: `mentions`, `issue_comments`,
651
`assignments`, `issue_activity`, `label_changes`. Each names what it delivers,
652
so switching one off has an effect you can predict from its name.
653
654
**Channel** — where it hears about it. Two: the in-product inbox, which has no
655
switch because it is the surface itself, and email, which has `email_enabled`
656
and defaults off. Say category for what and channel for where; a list that
657
mixes them lets a rename move an account from one answer to the other.
658
`OpenAgents.Notifications.Preference` keeps the two apart and `switches/0` is
659
the union.
660
661
**Confirmed address** — an address an account typed into its notification
662
settings and proved by returning a code mailed to it.
663
`OpenAgents.Notifications.EmailChannel.verified_address/1` is the only read a
664
send resolves a recipient through, so an address that is merely recorded
665
reaches nothing. Say confirmed, not verified, of the address; `verified_at` is
666
the column and confirming is what a person does.
667
668
**Delivery** — one queued outbound message,
669
`OpenAgents.Notifications.Delivery`, carried as an `email.delivery` effect in
670
the durable outbox. It is not the notification record, which is written in the
671
transaction and needs no queue, and it is not the message, which is
672
`OpenAgents.Notifications.Email`.
673
648 674
### Pull requests and stacks
649 675
650 676
**Pull request** — a repository-scoped proposal to merge one hosted branch
lib/openagents/accounts/user.ex modified +17

@@ -25,6 +25,18 @@ defmodule OpenAgents.Accounts.User do

25 25
    field :public_leaderboard_opted_out, :boolean, default: false
26 26
    field :browser_key_hash, :binary
27 27
28
    # The notification channel's address, and the proof its owner asked for it.
29
    # Nothing reads `notification_email` as a recipient on its own:
30
    # `OpenAgents.Notifications.EmailChannel.verified_address/1` is the one
31
    # read, and it returns `nil` while `notification_email_verified_at` is,
32
    # so an address typed but never confirmed is inert. The code is held as a
33
    # SHA-256 digest, never as plaintext.
34
    field :notification_email, :string
35
    field :notification_email_verified_at, :utc_datetime_usec
36
    field :notification_email_code_digest, :binary, redact: true
37
    field :notification_email_code_sent_at, :utc_datetime_usec
38
    field :notification_email_code_attempts, :integer, default: 0
39
28 40
    # Not a column: resolved once by `UserAuth.on_mount/4` when it builds the
29 41
    # scope, because the sidebar asks on every render and the answer must not
30 42
    # be a query each time. Defaults to false, so a user loaded by any other

@@ -61,6 +73,11 @@ defmodule OpenAgents.Accounts.User do

61 73
          github_token_rotated_at: DateTime.t() | nil,
62 74
          public_leaderboard_opted_out: boolean(),
63 75
          browser_key_hash: binary() | nil,
76
          notification_email: String.t() | nil,
77
          notification_email_verified_at: DateTime.t() | nil,
78
          notification_email_code_digest: binary() | nil,
79
          notification_email_code_sent_at: DateTime.t() | nil,
80
          notification_email_code_attempts: non_neg_integer(),
64 81
          agent_surfaces?: boolean(),
65 82
          unread_notifications: non_neg_integer(),
66 83
          inserted_at: DateTime.t(),
lib/openagents/data_rights/account_export.ex modified +9 -1

@@ -145,7 +145,15 @@ defmodule OpenAgents.DataRights.AccountExport do

145 145
       "account" => %{
146 146
         "id" => user.id,
147 147
         "github_login" => user.github_login,
148
         "github_name" => user.github_name
148
         "github_name" => user.github_name,
149
         # The one field on the account the person typed rather than the
150
         # provider supplied, so it is the one they are most likely to have
151
         # forgotten giving. Reported whether or not it was confirmed, because
152
         # the question a data export answers is what this deployment holds,
153
         # not what it would act on; the flag beside it says which.
154
         "notification_email" => user.notification_email,
155
         "notification_email_verified" =>
156
           not is_nil(OpenAgents.Notifications.EmailChannel.verified_address(user))
149 157
       },
150 158
       "identities" => identities_export(user, actor_links, refs),
151 159
       "bounds" => bounds(),
lib/openagents/effects/handlers/email_delivery.ex modified +17 -20

@@ -2,35 +2,32 @@ defmodule OpenAgents.Effects.Handlers.EmailDelivery do

2 2
  @moduledoc """
3 3
  Drives an `email.delivery` effect to a terminal state.
4 4
5
  The payload carries the notification `dedupe_key` and optional `data`. When
6
  `data` does not contain a `to` recipient, the handler records a successful
7
  `nothing_to_send_to` terminal outcome. Otherwise it calls the configured
8
  `OpenAgents.Notifications.Delivery` adapter, which is a future seam: no real
9
  send happens unless a provider is configured.
5
  The payload names an account and an event. It does not name an address, and
6
  no caller can put one there: the recipient is resolved on the way out by
7
  `OpenAgents.Notifications.email_dispatch/1`, which returns an address only
8
  when the account confirmed it and asked for mail. An address that was typed
9
  and never confirmed is therefore unreachable from the queue as well as from
10
  the settings surface, and an account that switched the channel off between
11
  the enqueue and the send is not mailed on the strength of what it wanted
12
  earlier.
13
14
  A refusal is a completion, not a failure. Nothing about "this account has no
15
  confirmed address" improves by being tried again in eight seconds, so the
16
  effect records the reason and stops. Only a provider that could not be
17
  reached leaves the effect pending for the outbox to retry.
10 18
  """
11 19
12 20
  @behaviour OpenAgents.Effects.Handler
13 21
14 22
  alias OpenAgents.Effects.Effect
23
  alias OpenAgents.Notifications
15 24
  alias OpenAgents.Notifications.Delivery
16 25
17 26
  @impl OpenAgents.Effects.Handler
18 27
  def run(%Effect{payload: payload}, _idempotency_key) do
19
    data = Map.get(payload, "data", %{})
20
21
    case recipient(data) do
22
      nil ->
23
        {:ok, %{"outcome" => "nothing_to_send_to"}}
24
25
      to ->
26
        Delivery.adapter().deliver(to, data)
27
    end
28
  end
29
30
  defp recipient(data) when is_map(data) do
31
    case Map.get(data, "to") do
32
      value when is_binary(value) and value != "" -> value
33
      _ -> nil
28
    case Notifications.email_dispatch(payload) do
29
      {:ok, recipient, pointer} -> Delivery.adapter().deliver(recipient, pointer)
30
      {:refused, outcome} -> {:ok, %{"outcome" => outcome}}
34 31
    end
35 32
  end
36 33
end
lib/openagents/notifications.ex modified +143 -5

@@ -19,10 +19,23 @@ defmodule OpenAgents.Notifications do

19 19
  `OpenAgents.Repositories.readable_by/2` again, so a recipient who loses
20 20
  membership after the row was written stops seeing it.
21 21
22
  Delivery is in-product only. This deployment has no outbound mail path and
23
  accounts carry no email address, so an email channel would be a second,
24
  unconfigured system. Adding one means adding an address, an adapter, and a
25
  retry schedule, and belongs behind its own change.
22
  ## The second channel
23
24
  Delivery is in-product first and by email second. The inbox is the channel
25
  with no switch, because it is the product surface; email is off until an
26
  account confirms an address (`OpenAgents.Notifications.EmailChannel`) and
27
  turns `email_enabled` on, and it carries mentions only. A mention is the
28
  event addressed to one person by name and the lowest-volume category there
29
  is; the rest wait for a digest, because one message per comment on a busy
30
  issue is how a channel gets filtered to a folder nobody opens (#141).
31
32
  Email cannot inherit the durability the inbox gets for free. The record is
33
  written in the transaction, but the send is not, so `Delivery.enqueue/1`
34
  writes an `email.delivery` effect in that same transaction and
35
  `OpenAgents.Effects` owns the attempts. `email_dispatch/1` is the other half:
36
  it re-decides, at send time, everything the enqueue decided — confirmed
37
  address, channel, category, and the recipient's access to the repository — so
38
  a queued message cannot outlive the consent it was queued under.
26 39
  """
27 40
28 41
  import Ecto.Query, warn: false

@@ -30,6 +43,8 @@ defmodule OpenAgents.Notifications do

30 43
  alias OpenAgents.Accounts.User
31 44
  alias OpenAgents.Issues.Comment
32 45
  alias OpenAgents.Issues.Issue
46
  alias OpenAgents.Notifications.Delivery
47
  alias OpenAgents.Notifications.EmailChannel
33 48
  alias OpenAgents.Notifications.IssueSubscription
34 49
  alias OpenAgents.Notifications.Mentions
35 50
  alias OpenAgents.Notifications.Notification

@@ -45,6 +60,12 @@ defmodule OpenAgents.Notifications do

45 60
46 61
  @notifications_per_page 50
47 62
63
  # The kinds the email channel carries. One, deliberately. A mention names a
64
  # person, so it is the category whose volume is bounded by how often somebody
65
  # types your login; every other kind follows the traffic on a thread, and
66
  # mailing one message per comment is what a digest exists to avoid (#141).
67
  @email_kinds ["mention"]
68
48 69
  def per_page, do: @notifications_per_page
49 70
50 71
  ## Announcements

@@ -296,6 +317,7 @@ defmodule OpenAgents.Notifications do

296 317
    |> Enum.filter(&enabled?(&1, kind))
297 318
    |> Enum.map(fn user ->
298 319
      insert_notification(user, issue, nil, kind, actor_login, dedupe_key)
320
      enqueue_email(user, issue, kind, actor_login, dedupe_key)
299 321
      user.id
300 322
    end)
301 323
  end

@@ -344,10 +366,34 @@ defmodule OpenAgents.Notifications do

344 366
    |> Enum.filter(fn {user, _kind} -> readable?(repository, user) end)
345 367
    |> Enum.map(fn {user, kind} ->
346 368
      insert_notification(user, issue, comment, kind, actor_login, dedupe_key)
369
      enqueue_email(user, issue, kind, actor_login, dedupe_key)
347 370
      user.id
348 371
    end)
349 372
  end
350 373
374
  # The email half, asked for in the same transaction as the record it
375
  # announces. The four conditions are re-decided at send time by
376
  # `email_dispatch/1`; checking them here as well keeps the queue from filling
377
  # with effects for accounts that were never going to be mailed.
378
  defp enqueue_email(%User{} = user, %Issue{} = issue, kind, actor_login, dedupe_key) do
379
    if email_wanted?(user, kind) do
380
      Delivery.enqueue(
381
        dedupe_key: dedupe_key,
382
        user_id: user.id,
383
        issue_id: issue.id,
384
        kind: kind,
385
        actor_login: actor_login
386
      )
387
    end
388
389
    :ok
390
  end
391
392
  defp email_wanted?(%User{} = user, kind) do
393
    kind in @email_kinds and EmailChannel.deliverable?() and
394
      not is_nil(EmailChannel.verified_address(user)) and preferences(user).email_enabled
395
  end
396
351 397
  defp insert_notification(user, issue, comment, kind, actor_login, dedupe_key) do
352 398
    %Notification{}
353 399
    |> Notification.changeset(%{

@@ -526,7 +572,7 @@ defmodule OpenAgents.Notifications do

526 572
    case existing do
527 573
      %Preference{id: nil} ->
528 574
        Repo.insert(changeset,
529
          on_conflict: {:replace, Preference.categories() ++ [:updated_at]},
575
          on_conflict: {:replace, Preference.switches() ++ [:updated_at]},
530 576
          conflict_target: [:user_id]
531 577
        )
532 578

@@ -539,4 +585,96 @@ defmodule OpenAgents.Notifications do

539 585
  def preferences_recorded?(%User{} = user) do
540 586
    Repo.exists?(from preference in Preference, where: preference.user_id == ^user.id)
541 587
  end
588
589
  ## Outbound
590
591
  @doc """
592
  Resolves a queued `email.delivery` payload to a recipient and a pointer.
593
594
  Every question the enqueue answered is asked again here, against the database
595
  as it is now rather than as it was when the comment landed: is the account
596
  still active, is its address still confirmed, is the channel still on, is the
597
  category still on, and can it still read the repository. A queued message
598
  therefore cannot outlive the consent or the access it was queued under, which
599
  matters precisely because a sent message is the one notification no later
600
  authorization check can withdraw.
601
602
  A `{:refused, outcome}` is a completion, not a failure: none of these answers
603
  changes by being asked again in eight seconds. The outcome string is recorded
604
  on the effect, so the queue says why it sent nothing.
605
606
  The pointer carries what NOTIFY-001 lets a notification carry — the kind, the
607
  actor's login, the repository path, and the issue number — plus the two URLs
608
  a message needs to be useful. The issue's title is not in it.
609
  """
610
  @spec email_dispatch(map()) :: {:ok, String.t(), map()} | {:refused, String.t()}
611
  def email_dispatch(payload) when is_map(payload) do
612
    with {:ok, user} <- dispatch_user(payload["user_id"]),
613
         {:ok, address} <- dispatch_address(user),
614
         preferences = preferences(user),
615
         :ok <- dispatch_channel(preferences),
616
         :ok <- dispatch_category(preferences, payload["kind"]),
617
         {:ok, issue} <- dispatch_issue(payload["issue_id"]),
618
         {:ok, repository} <- dispatch_repository(issue, user) do
619
      {:ok, address, pointer(payload, issue, repository)}
620
    end
621
  end
622
623
  defp dispatch_user(user_id) when is_binary(user_id) do
624
    with {:ok, uuid} <- Ecto.UUID.cast(user_id),
625
         %User{status: "active"} = user <- Repo.get(User, uuid) do
626
      {:ok, user}
627
    else
628
      _absent -> {:refused, "recipient_gone"}
629
    end
630
  end
631
632
  defp dispatch_user(_user_id), do: {:refused, "recipient_gone"}
633
634
  defp dispatch_address(user) do
635
    case EmailChannel.verified_address(user) do
636
      nil -> {:refused, "no_verified_address"}
637
      address -> {:ok, address}
638
    end
639
  end
640
641
  defp dispatch_channel(%Preference{email_enabled: true}), do: :ok
642
  defp dispatch_channel(%Preference{}), do: {:refused, "channel_off"}
643
644
  defp dispatch_category(preferences, kind) when kind in @email_kinds do
645
    if category_enabled(preferences, kind), do: :ok, else: {:refused, "category_off"}
646
  end
647
648
  defp dispatch_category(_preferences, _kind), do: {:refused, "kind_not_carried"}
649
650
  # The issue's key is an integer, so the payload carries one and this reads it
651
  # back as one. Anything else is a payload this release did not write.
652
  defp dispatch_issue(issue_id) when is_integer(issue_id) do
653
    case Repo.get(Issue, issue_id) do
654
      %Issue{} = issue -> {:ok, issue}
655
      nil -> {:refused, "issue_gone"}
656
    end
657
  end
658
659
  defp dispatch_issue(_issue_id), do: {:refused, "issue_gone"}
660
661
  defp dispatch_repository(%Issue{repository_id: repository_id}, user) do
662
    repository = Repo.get(Repository, repository_id)
663
664
    if readable?(repository, user), do: {:ok, repository}, else: {:refused, "not_readable"}
665
  end
666
667
  defp pointer(payload, issue, repository) do
668
    base = String.trim_trailing(OpenAgentsWeb.Endpoint.url(), "/")
669
    path = "#{repository.owner}/#{repository.name}"
670
671
    %{
672
      "kind" => payload["kind"],
673
      "actor_login" => payload["actor_login"],
674
      "repository" => path,
675
      "issue_number" => issue.number,
676
      "url" => "#{base}/#{path}/issues/#{issue.number}",
677
      "settings_url" => "#{base}/notifications"
678
    }
679
  end
542 680
end
lib/openagents/notifications/delivery.ex modified +34 -35

@@ -1,64 +1,63 @@

1 1
defmodule OpenAgents.Notifications.Delivery do
2 2
  @moduledoc """
3
  Durable outbound email delivery seam for notifications.
3
  The durable outbound half of a notification.
4 4
5
  Enqueues one `email.delivery` effect keyed to a notification `dedupe_key`.
6
  A recipient is optional in the caller's data; if none is provided the handler
7
  records a terminal `nothing_to_send_to` outcome instead of a failure.
5
  In-product delivery needs no queue: the record is written in the transaction
6
  that writes the comment it announces, so it exists exactly when the event
7
  does. Email cannot work that way. A send happens after that transaction
8
  commits and can fail on somebody else's server, so the asking has to be
9
  durable even though the doing is not.
8 10
9
  No real send happens here. The adapter is a future seam: the default
10
  `OpenAgents.Notifications.Delivery.NullAdapter` refuses unless a real
11
  provider is configured, and the caller supplies a `to` address.
11
  `enqueue/1` is called inside the same transaction as the notification row and
12
  writes one `email.delivery` effect. From there `OpenAgents.Effects` owns the
13
  schedule: a lease per attempt, exponential backoff between them, and a
14
  terminal `failed` at `maximum_attempts`. The effect's idempotency key is the
15
  recipient and the notification's `dedupe_key`, which is the same key the
16
  unique index on `notifications` uses, so a replayed fan-out produces one
17
  record and one send rather than two of each.
18
19
  ## What the payload may carry
20
21
  Identifiers, and no address. The recipient is resolved at send time by
22
  `OpenAgents.Notifications.email_dispatch/1` from the account's confirmed
23
  address, so a queued delivery cannot outlive the consent it was queued under:
24
  an account that removes its address, switches the channel off, or loses
25
  access to the repository stops being mailed, including for effects already
26
  sitting in the queue.
12 27
  """
13 28
14 29
  alias OpenAgents.Effects
15
  alias OpenAgents.Notifications.Notification
16 30
17 31
  @kind "email.delivery"
18 32
  @source_kind "notification"
19 33
20 34
  @doc """
21
  Enqueue an email delivery for a `Notification` or a bare `dedupe_key`.
35
  Enqueue the email half of one notification.
22 36
23
  Accepted shapes:
24
    * `%Notification{}`
25
    * a map or keyword with `:dedupe_key` and optional `:data`, `:user_id`,
26
      `:notification_id`, and `:maximum_attempts`
27
28
  When a `user_id` is known, the idempotency key is scoped to that user so the
29
  same `dedupe_key` for two different accounts stays two distinct deliveries.
37
  Requires `:dedupe_key`, `:user_id`, `:issue_id`, and `:kind`, and accepts
38
  `:actor_login` and `:maximum_attempts`. The idempotency key is scoped to the
39
  account, so the same event for two recipients is two deliveries.
30 40
  """
31
  @spec enqueue(Notification.t() | map() | keyword()) ::
32
          {:ok, Effects.Effect.t()} | {:error, term()}
33
  def enqueue(%Notification{} = notification) do
34
    enqueue(%{
35
      dedupe_key: notification.dedupe_key,
36
      user_id: notification.user_id,
37
      notification_id: notification.id,
38
      data: %{}
39
    })
40
  end
41
41
  @spec enqueue(map() | keyword()) :: {:ok, Effects.Effect.t()} | {:error, term()}
42 42
  def enqueue(attrs) when is_list(attrs), do: enqueue(Map.new(attrs))
43 43
44 44
  def enqueue(attrs) when is_map(attrs) do
45 45
    dedupe_key = fetch!(attrs, :dedupe_key)
46
    user_id = Map.get(attrs, :user_id)
47
    notification_id = Map.get(attrs, :notification_id)
48
    data = Map.get(attrs, :data) || %{}
49
50
    idempotency_source = if user_id, do: "#{user_id}/#{dedupe_key}", else: dedupe_key
46
    user_id = fetch!(attrs, :user_id)
47
    issue_id = fetch!(attrs, :issue_id)
48
    kind = fetch!(attrs, :kind)
51 49
52 50
    Effects.enqueue(@kind, %{
53 51
      payload: %{
54 52
        "dedupe_key" => dedupe_key,
55 53
        "user_id" => user_id,
56
        "notification_id" => notification_id,
57
        "data" => data
54
        "issue_id" => issue_id,
55
        "kind" => kind,
56
        "actor_login" => Map.get(attrs, :actor_login)
58 57
      },
59 58
      source_kind: @source_kind,
60 59
      source_id: dedupe_key,
61
      idempotency_key: Effects.idempotency_key(@kind, @source_kind, idempotency_source),
60
      idempotency_key: Effects.idempotency_key(@kind, @source_kind, "#{user_id}/#{dedupe_key}"),
62 61
      maximum_attempts: Map.get(attrs, :maximum_attempts, 5)
63 62
    })
64 63
  end
lib/openagents/notifications/delivery/mailer_adapter.ex added +31

@@ -0,0 +1,31 @@

1
defmodule OpenAgents.Notifications.Delivery.MailerAdapter do
2
  @moduledoc """
3
  The delivery adapter that actually sends, through `OpenAgents.Mailer`.
4
5
  It is deliberately thin. Everything that decides whether a message may be
6
  sent — the confirmed address, the channel switch, the category, and the
7
  recipient's current access to the repository — is settled by
8
  `OpenAgents.Notifications.email_dispatch/1` before the adapter is reached, so
9
  this module has no authority to add or to withhold. It turns a resolved
10
  pointer into a message and reports what the provider said.
11
12
  A provider refusal comes back as `{:error, reason}`, which leaves the effect
13
  pending: the durable outbox counts the attempt, backs off, and stops at
14
  `maximum_attempts` with a terminal `failed`. That is the whole retry policy,
15
  and it lives in `OpenAgents.Effects` rather than here.
16
  """
17
18
  @behaviour OpenAgents.Notifications.Delivery.Adapter
19
20
  alias OpenAgents.Notifications.Email
21
22
  @impl true
23
  def deliver(recipient, data) when is_binary(recipient) do
24
    case Email.deliver_notification(recipient, data) do
25
      {:ok, _metadata} -> {:ok, %{"outcome" => "sent"}}
26
      {:error, reason} -> {:error, reason}
27
    end
28
  end
29
30
  def deliver(_recipient, _data), do: {:error, :no_recipient}
31
end
lib/openagents/notifications/delivery/null_adapter.ex modified +7 -5

@@ -1,11 +1,13 @@

1 1
defmodule OpenAgents.Notifications.Delivery.NullAdapter do
2 2
  @moduledoc """
3
  Default no-op email delivery adapter.
3
  The adapter a deployment that configured none gets.
4 4
5
  No provider is configured, so a delivery that has a recipient cannot be sent.
6
  The handler returns an error and the durable outbox retries up to its
7
  `maximum_attempts`. A delivery without a recipient is handled before the
8
  adapter is ever called.
5
  It refuses every send, so a delivery that reached it — meaning the account
6
  confirmed an address and asked for mail — is retried by the durable outbox
7
  and then recorded as terminally failed rather than quietly dropped. A
8
  deployment in that state should be saying so on the settings surface too:
9
  `OpenAgents.Notifications.EmailChannel.deliverable?/0` is the switch that
10
  stops an address being collected in the first place.
9 11
  """
10 12
11 13
  @behaviour OpenAgents.Notifications.Delivery.Adapter
lib/openagents/notifications/email.ex added +93

@@ -0,0 +1,93 @@

1
defmodule OpenAgents.Notifications.Email do
2
  @moduledoc """
3
  The two messages this application sends, and the mailer it sends them with.
4
5
  Both are plain text. A notification is a pointer with a link on it, which
6
  needs no layout, and a text part renders in every client without a fallback
7
  to maintain.
8
9
  ## What a notification message may say
10
11
  `NOTIFY-001` says a notification record stores identifiers, a kind, and the
12
  actor's login, never a title or a body. The message obeys the same rule for
13
  the same reason: it leaves the application, so it is the one copy of a
14
  notification that no later authorization check can withdraw. The repository
15
  path and the issue number travel because a link is useless without them and
16
  because the recipient's access to that repository was rechecked on the way
17
  out. The issue's title did not travel, and does not.
18
19
  ## Where they are sent from
20
21
  One `from` for both, configured under
22
  `OpenAgents.Notifications.EmailChannel`, so a deployment sets its sending
23
  identity in one place and no call site invents one.
24
  """
25
26
  import Swoosh.Email
27
28
  alias OpenAgents.Mailer
29
  alias OpenAgents.Notifications.EmailChannel
30
31
  @doc """
32
  Mails a verification code to an address nobody has confirmed yet.
33
34
  This is the one message that goes to an unverified address, and it is what
35
  makes verification possible at all. It carries the code and nothing about the
36
  account: an address typed by mistake learns that somebody asked, not who.
37
  """
38
  @spec deliver_verification(String.t(), String.t()) :: {:ok, term()} | {:error, term()}
39
  def deliver_verification(address, code) when is_binary(address) and is_binary(code) do
40
    new()
41
    |> to(address)
42
    |> from(EmailChannel.from())
43
    |> subject("Your OpenAgents verification code: #{code}")
44
    |> text_body("""
45
    Somebody asked to send OpenAgents notifications to this address.
46
47
    Your verification code is:
48
49
        #{code}
50
51
    Enter it on the notifications page to confirm the address. The code is good
52
    for 30 minutes.
53
54
    If this was not you, ignore this message. Nothing is sent to an address that
55
    has not been confirmed, so no further mail will arrive.
56
    """)
57
    |> Mailer.deliver()
58
  end
59
60
  @doc """
61
  Mails one notification to a confirmed address.
62
63
  `pointer` is what `OpenAgents.Notifications.email_dispatch/1` resolved: the
64
  kind, the actor's login, the repository path, the issue number, and the URL.
65
  Nothing else reaches this function, so nothing else can reach the message.
66
  """
67
  @spec deliver_notification(String.t(), map()) :: {:ok, term()} | {:error, term()}
68
  def deliver_notification(address, pointer) when is_binary(address) and is_map(pointer) do
69
    reference = "#{pointer["repository"]}##{pointer["issue_number"]}"
70
71
    new()
72
    |> to(address)
73
    |> from(EmailChannel.from())
74
    |> subject("#{headline(pointer)} on #{reference}")
75
    |> text_body("""
76
    #{headline(pointer)} on #{reference}.
77
78
    #{pointer["url"]}
79
80
    You are receiving this because email delivery is on for your account. Turn
81
    it off, or change the address, on the notifications page:
82
83
    #{pointer["settings_url"]}
84
    """)
85
    |> Mailer.deliver()
86
  end
87
88
  defp headline(%{"kind" => "mention", "actor_login" => login}) when is_binary(login),
89
    do: "#{login} mentioned you"
90
91
  defp headline(%{"kind" => "mention"}), do: "You were mentioned"
92
  defp headline(_pointer), do: "There is activity"
93
end
lib/openagents/notifications/email_channel.ex added +326

@@ -0,0 +1,326 @@

1
defmodule OpenAgents.Notifications.EmailChannel do
2
  @moduledoc """
3
  The address an account is willing to receive mail at, and the proof of it.
4
5
  ## Why the address is typed rather than taken
6
7
  GitHub OAuth can be asked for `user:email` and hand over whatever address the
8
  provider holds. This deployment does not ask. An address taken from a
9
  provider is an address nobody chose to give this application, and the first
10
  thing it would be used for is unsolicited mail to a mailbox its owner never
11
  named here. So the address is typed into the notification settings, by the
12
  person who wants mail, on purpose.
13
14
  ## The gate
15
16
  `verified_address/1` is the only function anything sends to, and it returns
17
  `nil` unless the account both names an address and confirmed it. Confirmation
18
  is a code mailed to the address and typed back, which is the only evidence
19
  this application can have that the person asking controls the mailbox.
20
21
  Three things make the gate hold rather than merely exist:
22
23
    * The code is held as a SHA-256 digest. A database read is not enough to
24
      claim somebody else's mailbox.
25
    * Guesses are counted and bounded at five. A short code with free guesses
26
      is not a secret.
27
    * The check constraint `users_notification_email_state_check` refuses a
28
      verified timestamp on a row with no address, so the gate survives a bug
29
      in this module.
30
31
  Changing the address clears the verification, because the evidence was about
32
  the old mailbox. Removing it clears everything, including the outstanding
33
  code.
34
35
  ## What a deployment without a mail provider does
36
37
  `deliverable?/0` reads one configuration key rather than inferring from the
38
  Swoosh adapter, because the inference is wrong in both directions: the local
39
  adapter is real delivery in development — the mailbox preview at
40
  `/dev/mailbox` — and it is a black hole in production. A deployment that
41
  configures no provider says so, and the settings surface offers no address
42
  field rather than accepting one it cannot mail to.
43
  """
44
45
  alias OpenAgents.Accounts.User
46
  alias OpenAgents.Notifications.Email
47
  alias OpenAgents.Repo
48
49
  # Long enough that five guesses are hopeless, short enough to retype from a
50
  # phone. Crockford's alphabet without I, L, O and U: no character in it can
51
  # be confused with another in a proportional font, and none of them spell
52
  # anything.
53
  @code_alphabet ~c"0123456789ABCDEFGHJKMNPQRSTVWXYZ"
54
  @code_length 8
55
56
  @code_lifetime_seconds 1_800
57
  @resend_after_seconds 60
58
  @maximum_attempts 5
59
60
  @typedoc "What the settings surface needs to render the address, and nothing more."
61
  @type state :: %{address: String.t() | nil, verified?: boolean(), pending?: boolean()}
62
63
  @typedoc "Why an address or a code was refused."
64
  @type refusal ::
65
          :not_deliverable
66
          | :invalid_address
67
          | :too_soon
68
          | :nothing_pending
69
          | :expired
70
          | :incorrect_code
71
          | :too_many_attempts
72
73
  @doc """
74
  The address this account may be mailed at, or `nil`.
75
76
  Every outbound notification resolves its recipient here. An address that was
77
  typed but never confirmed returns `nil`, which is what makes an unverified
78
  address unreachable rather than merely discouraged.
79
  """
80
  @spec verified_address(User.t() | nil) :: String.t() | nil
81
  def verified_address(%User{notification_email: address, notification_email_verified_at: at})
82
      when is_binary(address) and not is_nil(at),
83
      do: address
84
85
  def verified_address(_user), do: nil
86
87
  @doc "Whether this deployment can send at all. See the module note on `deliverable?/0`."
88
  @spec deliverable?() :: boolean()
89
  def deliverable?, do: Keyword.get(configuration(), :deliverable, false)
90
91
  @doc "The `{name, address}` every message this channel sends is from."
92
  @spec from() :: {String.t(), String.t()}
93
  def from, do: Keyword.fetch!(configuration(), :from)
94
95
  @doc """
96
  What the settings surface renders: the address, whether it is confirmed, and
97
  whether a code is outstanding.
98
99
  Never the code, and never its digest.
100
  """
101
  @spec state(User.t()) :: state()
102
  def state(%User{} = user) do
103
    %{
104
      address: user.notification_email,
105
      verified?: not is_nil(user.notification_email_verified_at),
106
      pending?: pending?(user)
107
    }
108
  end
109
110
  @doc """
111
  Records an address and mails a code to it.
112
113
  The address is inert until the code comes back. Re-recording the address an
114
  account already confirmed changes nothing rather than quietly unverifying it,
115
  because retyping what you already proved is not a withdrawal of the proof.
116
  """
117
  @spec set_address(User.t(), String.t()) :: {:ok, User.t()} | {:error, refusal()}
118
  def set_address(%User{} = user, address) when is_binary(address) do
119
    with :ok <- require_deliverable(),
120
         {:ok, normalized} <- normalize(address) do
121
      if normalized == verified_address(user) do
122
        {:ok, user}
123
      else
124
        issue_code(user, normalized)
125
      end
126
    end
127
  end
128
129
  @doc """
130
  Mails another code to the address already on the account.
131
132
  Bounded by `#{@resend_after_seconds}` seconds since the last one, so the send
133
  button cannot be turned into a way to mail somebody repeatedly. Issuing a new
134
  code retires the old one and resets the attempt count: the person is asking
135
  again, not guessing again.
136
  """
137
  @spec resend_code(User.t()) :: {:ok, User.t()} | {:error, refusal()}
138
  def resend_code(%User{notification_email: address} = user) when is_binary(address) do
139
    with :ok <- require_deliverable(),
140
         :ok <- require_resend_window(user),
141
         do: issue_code(user, address)
142
  end
143
144
  def resend_code(%User{}), do: {:error, :nothing_pending}
145
146
  @doc """
147
  Confirms the address with the code that was mailed to it.
148
149
  A correct code marks the address verified and clears the outstanding one, so
150
  the same code cannot be replayed. A wrong one counts, and at the fifth the
151
  code is retired entirely: the next step is a fresh send, not another guess.
152
153
  The comparison is constant-time over digests, so a caller cannot learn the
154
  code one character at a time.
155
  """
156
  @spec verify(User.t(), String.t()) :: {:ok, User.t()} | {:error, refusal()}
157
  def verify(%User{} = user, code) when is_binary(code) do
158
    with :ok <- require_pending(user),
159
         :ok <- require_unexpired(user),
160
         :ok <- require_attempts_left(user) do
161
      if Plug.Crypto.secure_compare(
162
           user.notification_email_code_digest,
163
           digest(normalize_code(code))
164
         ) do
165
        confirm(user)
166
      else
167
        count_failure(user)
168
      end
169
    end
170
  end
171
172
  @doc """
173
  Forgets the address, the verification, and any outstanding code.
174
175
  One update rather than a soft delete: there is nothing here worth keeping
176
  once the account has said to stop mailing it.
177
  """
178
  @spec remove_address(User.t()) :: {:ok, User.t()} | {:error, Ecto.Changeset.t()}
179
  def remove_address(%User{} = user) do
180
    update(user, %{
181
      notification_email: nil,
182
      notification_email_verified_at: nil,
183
      notification_email_code_digest: nil,
184
      notification_email_code_sent_at: nil,
185
      notification_email_code_attempts: 0
186
    })
187
  end
188
189
  ## Internals
190
191
  # The row and the message have to agree, and only one of them is
192
  # transactional. Writing first is the order that cannot mail a code the
193
  # database does not hold; rolling back on a refused send is what keeps the
194
  # other direction from mattering, so a provider hiccup while changing an
195
  # address does not leave the account with its previous verification quietly
196
  # withdrawn.
197
  defp issue_code(user, address) do
198
    code = generate_code()
199
200
    Repo.transaction(fn ->
201
      case update(user, %{
202
             notification_email: address,
203
             notification_email_verified_at: nil,
204
             notification_email_code_digest: digest(code),
205
             notification_email_code_sent_at: DateTime.utc_now(),
206
             notification_email_code_attempts: 0
207
           }) do
208
        {:ok, updated} ->
209
          case Email.deliver_verification(address, code) do
210
            {:ok, _delivery} -> updated
211
            {:error, _reason} -> Repo.rollback(:not_deliverable)
212
          end
213
214
        {:error, %Ecto.Changeset{}} ->
215
          Repo.rollback(:invalid_address)
216
      end
217
    end)
218
  end
219
220
  defp confirm(user) do
221
    update(user, %{
222
      notification_email_verified_at: DateTime.utc_now(),
223
      notification_email_code_digest: nil,
224
      notification_email_code_attempts: 0
225
    })
226
  end
227
228
  defp count_failure(user) do
229
    attempts = user.notification_email_code_attempts + 1
230
231
    attributes =
232
      if attempts >= @maximum_attempts do
233
        %{notification_email_code_digest: nil, notification_email_code_attempts: attempts}
234
      else
235
        %{notification_email_code_attempts: attempts}
236
      end
237
238
    case update(user, attributes) do
239
      {:ok, _updated} when attempts >= @maximum_attempts -> {:error, :too_many_attempts}
240
      {:ok, _updated} -> {:error, :incorrect_code}
241
      {:error, _changeset} -> {:error, :incorrect_code}
242
    end
243
  end
244
245
  defp update(user, attributes) do
246
    user
247
    |> Ecto.Changeset.change(attributes)
248
    |> Ecto.Changeset.check_constraint(:notification_email,
249
      name: :users_notification_email_state_check
250
    )
251
    |> Repo.update()
252
  end
253
254
  defp require_deliverable do
255
    if deliverable?(), do: :ok, else: {:error, :not_deliverable}
256
  end
257
258
  defp require_pending(user) do
259
    if pending?(user), do: :ok, else: {:error, :nothing_pending}
260
  end
261
262
  defp require_unexpired(%User{notification_email_code_sent_at: sent_at}) do
263
    if DateTime.diff(DateTime.utc_now(), sent_at) <= @code_lifetime_seconds do
264
      :ok
265
    else
266
      {:error, :expired}
267
    end
268
  end
269
270
  defp require_attempts_left(%User{notification_email_code_attempts: attempts}) do
271
    if attempts < @maximum_attempts, do: :ok, else: {:error, :too_many_attempts}
272
  end
273
274
  defp require_resend_window(user) do
275
    case resend_available_at(user) do
276
      nil ->
277
        :ok
278
279
      available_at ->
280
        if DateTime.compare(DateTime.utc_now(), available_at) == :lt do
281
          {:error, :too_soon}
282
        else
283
          :ok
284
        end
285
    end
286
  end
287
288
  defp pending?(%User{notification_email_code_digest: digest}), do: is_binary(digest)
289
290
  defp resend_available_at(%User{notification_email_code_sent_at: nil}), do: nil
291
292
  defp resend_available_at(%User{notification_email_code_sent_at: sent_at}),
293
    do: DateTime.add(sent_at, @resend_after_seconds, :second)
294
295
  # Deliberately conservative, and deliberately not a full RFC 5322 grammar. The
296
  # address is not being parsed, it is being refused early: one at-sign, no
297
  # whitespace, a dot in the domain, and a length a column and a provider both
298
  # accept. Anything this admits that the provider rejects fails at the send,
299
  # which the outbox already handles.
300
  defp normalize(address) do
301
    normalized = address |> String.trim() |> String.downcase()
302
303
    if String.match?(normalized, ~r/\A[^\s@]+@[^\s@.]+(\.[^\s@.]+)+\z/) and
304
         String.length(normalized) <= 254 do
305
      {:ok, normalized}
306
    else
307
      {:error, :invalid_address}
308
    end
309
  end
310
311
  defp normalize_code(code), do: code |> String.trim() |> String.upcase()
312
313
  defp generate_code do
314
    size = length(@code_alphabet)
315
316
    @code_length
317
    |> :crypto.strong_rand_bytes()
318
    |> :binary.bin_to_list()
319
    |> Enum.map(&Enum.at(@code_alphabet, rem(&1, size)))
320
    |> List.to_string()
321
  end
322
323
  defp digest(value), do: :crypto.hash(:sha256, value)
324
325
  defp configuration, do: Application.get_env(:openagents, __MODULE__, [])
326
end
lib/openagents/notifications/preference.ex modified +22 -2

@@ -18,6 +18,20 @@ defmodule OpenAgents.Notifications.Preference do

18 18
  can predict from its name. That is why closing an issue is `issue_activity`
19 19
  rather than a quiet widening of `issue_comments`: somebody who switched off
20 20
  comments did not thereby ask to stop hearing that the issue closed.
21
22
  ## Categories and channels are different questions
23
24
  A category is *what* an account hears about. A channel is *where*. They are
25
  kept apart because they fail differently: switching off a category means
26
  wanting less, switching off a channel means wanting it somewhere else, and a
27
  list that mixed them would let a rename quietly move an account from one
28
  answer to the other.
29
30
  There is one channel switch, `email_enabled`, and it defaults off. The inbox
31
  has no switch because the inbox is the product surface; email leaves the
32
  application, so nobody gets it without asking. It is also inert on its own —
33
  `OpenAgents.Notifications.EmailChannel.verified_address/1` still has to
34
  return an address somebody confirmed.
21 35
  """
22 36
23 37
  use Ecto.Schema

@@ -31,12 +45,15 @@ defmodule OpenAgents.Notifications.Preference do

31 45
  @categories ~w(mentions_enabled issue_comments_enabled assignments_enabled
32 46
                 issue_activity_enabled label_changes_enabled)a
33 47
48
  @channels ~w(email_enabled)a
49
34 50
  schema "notification_preferences" do
35 51
    field :mentions_enabled, :boolean, default: true
36 52
    field :issue_comments_enabled, :boolean, default: true
37 53
    field :assignments_enabled, :boolean, default: true
38 54
    field :issue_activity_enabled, :boolean, default: true
39 55
    field :label_changes_enabled, :boolean, default: false
56
    field :email_enabled, :boolean, default: false
40 57
41 58
    belongs_to :user, User
42 59

@@ -46,11 +63,14 @@ defmodule OpenAgents.Notifications.Preference do

46 63
  @doc "Every category this account can switch, in the order the form lists them."
47 64
  def categories, do: @categories
48 65
66
  @doc "Every switch on this row, category and channel alike."
67
  def switches, do: @categories ++ @channels
68
49 69
  @doc false
50 70
  def changeset(preference, attrs) do
51 71
    preference
52
    |> cast(attrs, @categories)
53
    |> validate_required(@categories)
72
    |> cast(attrs, switches())
73
    |> validate_required(switches())
54 74
    |> unique_constraint(:user_id)
55 75
    |> foreign_key_constraint(:user_id)
56 76
  end
lib/openagents_web/live/notifications_live.ex modified +189

@@ -28,10 +28,25 @@ defmodule OpenAgentsWeb.NotificationsLive do

28 28
  One page is `OpenAgents.Notifications.per_page/0` rows. There is no older
29 29
  page yet; the inbox is a recent-activity surface, and an archive is a
30 30
  separate question from delivery.
31
32
  ## Email
33
34
  This page is also where an account gives an address and confirms it. The
35
  address is typed here rather than taken from GitHub, and it does nothing
36
  until a code mailed to it comes back — see
37
  `OpenAgents.Notifications.EmailChannel`. Only then does the channel switch
38
  appear, because a switch that turns on a channel with no reachable address
39
  would be a control that does nothing.
40
41
  A deployment with no mail provider says so and offers no address field. That
42
  is honest rather than defensive: collecting an address it cannot mail to
43
  would be collecting a secret for no purpose.
31 44
  """
32 45
  use OpenAgentsWeb, :live_view
33 46
47
  alias OpenAgents.Accounts.User
34 48
  alias OpenAgents.Notifications
49
  alias OpenAgents.Notifications.EmailChannel
35 50
  alias OpenAgents.Notifications.Preference
36 51
  alias OpenAgents.Repositories
37 52

@@ -69,16 +84,85 @@ defmodule OpenAgentsWeb.NotificationsLive do

69 84
    end
70 85
  end
71 86
87
  def handle_event("set_email_address", %{"email" => %{"address" => address}}, socket) do
88
    socket.assigns.email_account
89
    |> EmailChannel.set_address(address)
90
    |> resolve(socket, "Check that address for a verification code.")
91
  end
92
93
  def handle_event("resend_email_code", _params, socket) do
94
    socket.assigns.email_account
95
    |> EmailChannel.resend_code()
96
    |> resolve(socket, "Another code is on its way.")
97
  end
98
99
  def handle_event("verify_email_code", %{"email" => %{"code" => code}}, socket) do
100
    socket.assigns.email_account
101
    |> EmailChannel.verify(code)
102
    |> resolve(socket, "Address confirmed.")
103
  end
104
105
  def handle_event("remove_email_address", _params, socket) do
106
    socket.assigns.email_account
107
    |> EmailChannel.remove_address()
108
    |> resolve(socket, "Address removed. Nothing else will be mailed to it.")
109
  end
110
111
  def handle_event("update_email_channel", %{"channel" => params}, socket) do
112
    attrs = %{email_enabled: checked?(params["email_enabled"])}
113
114
    case Notifications.update_preferences(socket.assigns.current_scope, attrs) do
115
      {:ok, _preferences} ->
116
        {:noreply, socket |> put_flash(:info, "Notification settings saved.") |> load()}
117
118
      {:error, _changeset} ->
119
        {:noreply, put_flash(socket, :error, "Notification settings could not be saved.")}
120
    end
121
  end
122
123
  defp resolve({:ok, %User{} = user}, socket, message) do
124
    {:noreply,
125
     socket
126
     |> assign(:email_account, user)
127
     |> put_flash(:info, message)
128
     |> load()}
129
  end
130
131
  defp resolve({:error, reason}, socket, _message) do
132
    {:noreply, socket |> put_flash(:error, refusal(reason)) |> load()}
133
  end
134
135
  defp refusal(:invalid_address), do: "That does not look like an email address."
136
  defp refusal(:not_deliverable), do: "This deployment cannot send email."
137
  defp refusal(:too_soon), do: "A code went out a moment ago. Wait a minute and ask again."
138
  defp refusal(:nothing_pending), do: "There is no code waiting to be confirmed."
139
  defp refusal(:expired), do: "That code has expired. Ask for another one."
140
  defp refusal(:incorrect_code), do: "That code is not right."
141
  defp refusal(:too_many_attempts), do: "Too many wrong codes. Ask for a new one."
142
  defp refusal(_reason), do: "That could not be saved."
143
72 144
  defp checked?("true"), do: true
73 145
  defp checked?("on"), do: true
74 146
  defp checked?(_value), do: false
75 147
76 148
  defp load(socket) do
77 149
    user = socket.assigns.current_scope
150
    account = Map.get(socket.assigns, :email_account, user)
78 151
    notifications = Notifications.list_notifications(user)
79 152
    preferences = Notifications.preferences(user)
80 153
81 154
    socket
155
    |> assign(:email_account, account)
156
    |> assign(:email_deliverable?, EmailChannel.deliverable?())
157
    |> assign(:email_state, EmailChannel.state(account))
158
    |> assign(
159
      :email_form,
160
      to_form(%{"address" => account.notification_email, "code" => ""}, as: :email)
161
    )
162
    |> assign(
163
      :email_channel_form,
164
      to_form(%{"email_enabled" => preferences.email_enabled}, as: :channel)
165
    )
82 166
    |> assign(:unread_count, Notifications.unread_count(user))
83 167
    |> assign(:notifications_empty?, notifications == [])
84 168
    |> assign(

@@ -226,6 +310,111 @@ defmodule OpenAgentsWeb.NotificationsLive do

226 310
            />
227 311
          </.form>
228 312
        </section>
313
314
        <section aria-labelledby="notifications-email-heading" class="flex flex-col gap-4">
315
          <h2 id="notifications-email-heading" class="text-base font-medium text-foreground">
316
            Email
317
          </h2>
318
319
          <p
320
            :if={!@email_deliverable?}
321
            id="notifications-email-unavailable"
322
            class="text-sm text-muted-foreground"
323
          >
324
            This deployment has no mail provider configured, so there is nowhere to
325
            send to. Notifications arrive in the inbox above.
326
          </p>
327
328
          <%= if @email_deliverable? do %>
329
            <p class="text-sm text-muted-foreground">
330
              Give an address and confirm it with the code that arrives, and mentions
331
              can also reach you by email. Nothing is sent to an address that has not
332
              been confirmed, and the channel stays off until you turn it on. Only
333
              mentions travel this way for now — the other categories are waiting on a
334
              digest, because one message per comment is not a channel anybody keeps.
335
            </p>
336
337
            <.form
338
              for={@email_form}
339
              id="notification-email-address-form"
340
              phx-submit="set_email_address"
341
              class="flex flex-col gap-3"
342
            >
343
              <.input
344
                field={@email_form[:address]}
345
                type="email"
346
                label="Email address"
347
                autocomplete="email"
348
              />
349
350
              <div class="flex items-center gap-2">
351
                <.button id="notification-email-save" type="submit" size={:sm}>
352
                  {if @email_state.verified?, do: "Change address", else: "Send code"}
353
                </.button>
354
355
                <.button
356
                  :if={@email_state.address}
357
                  id="notification-email-remove"
358
                  variant={:ghost}
359
                  size={:sm}
360
                  tone={:danger}
361
                  phx-click="remove_email_address"
362
                >
363
                  Remove
364
                </.button>
365
              </div>
366
            </.form>
367
368
            <.form
369
              :if={@email_state.pending?}
370
              for={@email_form}
371
              id="notification-email-code-form"
372
              phx-submit="verify_email_code"
373
              class="flex flex-col gap-3"
374
            >
375
              <.input
376
                field={@email_form[:code]}
377
                type="text"
378
                label="Verification code"
379
                autocomplete="one-time-code"
380
              />
381
382
              <div class="flex items-center gap-2">
383
                <.button id="notification-email-confirm" type="submit" size={:sm}>
384
                  Confirm
385
                </.button>
386
387
                <.button
388
                  id="notification-email-resend"
389
                  variant={:ghost}
390
                  size={:sm}
391
                  phx-click="resend_email_code"
392
                >
393
                  Send another code
394
                </.button>
395
              </div>
396
            </.form>
397
398
            <div :if={@email_state.verified?} class="flex flex-col gap-3">
399
              <p id="notification-email-verified" class="text-sm text-muted-foreground">
400
                <.badge variant={:info}>Confirmed</.badge>
401
                {@email_state.address}
402
              </p>
403
404
              <.form
405
                for={@email_channel_form}
406
                id="notification-email-channel-form"
407
                phx-change="update_email_channel"
408
              >
409
                <.input
410
                  field={@email_channel_form[:email_enabled]}
411
                  type="checkbox"
412
                  label="Send mentions to this address"
413
                />
414
              </.form>
415
            </div>
416
          <% end %>
417
        </section>
229 418
      </div>
230 419
    </Layouts.app>
231 420
    """
priv/migration_lineages/prior-2026-08-19.json modified +3 -1

@@ -303,7 +303,9 @@

303 303
    20260825102408,
304 304
    20260825113454,
305 305
    20260825113459,
306
    20260825120000
306
    20260825120000,
307
    20260825140544,
308
    20260825140545
307 309
  ],
308 310
  "required_tables": [
309 311
    "users",
priv/repo/migrations/20260825140544_add_notification_email_address_to_users.exs added +45

@@ -0,0 +1,45 @@

1
defmodule OpenAgents.Repo.Migrations.AddNotificationEmailAddressToUsers do
2
  @moduledoc """
3
  An address to send to, and the proof that its owner asked for it.
4
5
  GitHub OAuth is not asked for `user:email` here. The address is typed into
6
  the notification settings by the person who wants mail, and it is inert until
7
  a code sent to that mailbox comes back. So the account carries four facts:
8
  the address, when it was verified, the digest of the code currently
9
  outstanding, and when that code went out.
10
11
  The code is stored as a SHA-256 digest, never as plaintext, for the same
12
  reason a token is: a database read must not be enough to claim somebody
13
  else's mailbox. `notification_email_code_attempts` bounds guessing at the
14
  address, since a short code is guessable if the guesses are free.
15
16
  The check constraint is the durable half of the gate. `verified_at` cannot be
17
  set without an address, and a code cannot be outstanding without one either,
18
  so no row can claim a verified mailbox it does not name. The application
19
  refuses first; this refuses when the application is wrong.
20
21
  Expand-only: every column is nullable or defaulted, so a node running the
22
  previous release keeps writing rows this migration accepts.
23
  """
24
25
  use Ecto.Migration
26
27
  def change do
28
    alter table(:users) do
29
      add :notification_email, :text
30
      add :notification_email_verified_at, :utc_datetime_usec
31
      add :notification_email_code_digest, :binary
32
      add :notification_email_code_sent_at, :utc_datetime_usec
33
      add :notification_email_code_attempts, :integer, null: false, default: 0
34
    end
35
36
    create constraint(:users, :users_notification_email_state_check,
37
             check: """
38
             notification_email IS NOT NULL
39
               OR (notification_email_verified_at IS NULL
40
                   AND notification_email_code_digest IS NULL
41
                   AND notification_email_code_sent_at IS NULL)
42
             """
43
           )
44
  end
45
end
priv/repo/migrations/20260825140545_add_email_channel_to_notification_preferences.exs added +22

@@ -0,0 +1,22 @@

1
defmodule OpenAgents.Repo.Migrations.AddEmailChannelToNotificationPreferences do
2
  @moduledoc """
3
  The second delivery channel, off for every account that already exists.
4
5
  The five columns beside this one are categories: what an account hears about.
6
  This one is a channel: where it hears about it. In-product delivery has no
7
  switch because it is the inbox itself; email does, and it defaults to false.
8
9
  Turning a channel on for every existing account without asking would mail
10
  people who chose the inbox and nothing else, so `false` is the only honest
11
  backfill. `NOT NULL DEFAULT false` fills the existing rows in place, and a
12
  node running the previous release neither reads nor writes the column.
13
  """
14
15
  use Ecto.Migration
16
17
  def change do
18
    alter table(:notification_preferences) do
19
      add :email_enabled, :boolean, null: false, default: false
20
    end
21
  end
22
end
test/openagents/notifications/delivery_test.exs modified +75 -32

@@ -1,10 +1,11 @@

1 1
defmodule OpenAgents.Notifications.DeliveryTest do
2 2
  @moduledoc """
3
  Proofs for the durable, decision-independent outbound email seam.
3
  The outbox half of the email channel: identity, retry, and giving up.
4 4
5
  These tests intentionally do not configure a real provider. They exercise the
6
  outbox invariants (idempotency, retry, terminal failure, and no-recipient
7
  handling) through the `email.delivery` effect.
5
  These tests deliberately configure an adapter that always refuses, so they
6
  exercise the schedule rather than the send. What arrives at a mailbox, and
7
  who is allowed to have one, is
8
  `OpenAgents.Notifications.EmailDeliveryTest`.
8 9
  """
9 10
10 11
  use OpenAgents.DataCase, async: false

@@ -15,6 +16,7 @@ defmodule OpenAgents.Notifications.DeliveryTest do

15 16
  alias OpenAgents.Notifications.Delivery
16 17
17 18
  defmodule FailingAdapter do
19
    @moduledoc false
18 20
    @behaviour OpenAgents.Notifications.Delivery.Adapter
19 21
20 22
    @impl true

@@ -22,41 +24,54 @@ defmodule OpenAgents.Notifications.DeliveryTest do

22 24
  end
23 25
24 26
  setup do
27
    # Restored rather than deleted. Deleting it would take the configured
28
    # adapter with it and leave every later test in the run talking to the
29
    # NullAdapter, which is a failure mode that reads like a bug in the code
30
    # under test.
31
    configured = Application.get_env(:openagents, OpenAgents.Notifications.Delivery)
25 32
    Application.put_env(:openagents, OpenAgents.Notifications.Delivery, adapter: FailingAdapter)
26 33
27 34
    on_exit(fn ->
28
      Application.delete_env(:openagents, OpenAgents.Notifications.Delivery)
35
      Application.put_env(:openagents, OpenAgents.Notifications.Delivery, configured)
29 36
    end)
30 37
31 38
    :ok
32 39
  end
33 40
34 41
  describe "enqueue/1" do
35
    test "records one delivery per dedupe key, no matter how many times it is enqueued" do
36
      {:ok, first} = Delivery.enqueue(dedupe_key: "issue:1:opened", user_id: "user-a")
37
      {:ok, second} = Delivery.enqueue(dedupe_key: "issue:1:opened", user_id: "user-a")
42
    test "records one delivery per recipient and dedupe key, however often it is asked" do
43
      {:ok, first} = enqueue("issue:1:opened", "user-a")
44
      {:ok, second} = enqueue("issue:1:opened", "user-a")
38 45
39 46
      assert first.id == second.id
40 47
      assert Repo.aggregate(Effect, :count) == 1
41 48
      assert first.payload["dedupe_key"] == "issue:1:opened"
42 49
    end
43 50
44
    test "a different user with the same dedupe key gets a distinct delivery" do
45
      {:ok, first} = Delivery.enqueue(dedupe_key: "issue:1:opened", user_id: "user-a")
46
      {:ok, second} = Delivery.enqueue(dedupe_key: "issue:1:opened", user_id: "user-b")
51
    test "a different recipient with the same dedupe key gets a distinct delivery" do
52
      {:ok, first} = enqueue("issue:1:opened", "user-a")
53
      {:ok, second} = enqueue("issue:1:opened", "user-b")
47 54
48 55
      refute first.id == second.id
49 56
    end
57
58
    test "carries identifiers and no address, so no caller can name a recipient" do
59
      {:ok, effect} = enqueue("issue:1:opened", "user-a")
60
61
      assert Map.keys(effect.payload) |> Enum.sort() ==
62
               ~w(actor_login dedupe_key issue_id kind user_id)
63
    end
64
65
    test "refuses to queue a delivery that names no account or no event" do
66
      assert_raise ArgumentError, fn -> Delivery.enqueue(dedupe_key: "x", kind: "mention") end
67
      assert_raise ArgumentError, fn -> Delivery.enqueue(user_id: "user-a", kind: "mention") end
68
    end
50 69
  end
51 70
52 71
  describe "handler dispatch" do
53 72
    test "a failed attempt increments attempts and reschedules the next try" do
54
      {:ok, effect} =
55
        Delivery.enqueue(
56
          dedupe_key: "fail-once",
57
          user_id: "user-a",
58
          data: %{"to" => "test@example.com"}
59
        )
73
      {user_id, issue_id} = deliverable()
74
      {:ok, effect} = enqueue("fail-once", user_id, issue_id: issue_id)
60 75
61 76
      before = DateTime.utc_now()
62 77
      assert %{completed: 0, failed: 1} = Worker.run_once(identity: "worker-a")

@@ -73,13 +88,8 @@ defmodule OpenAgents.Notifications.DeliveryTest do

73 88
    end
74 89
75 90
    test "attempts stop at a terminal failed state" do
76
      {:ok, effect} =
77
        Delivery.enqueue(
78
          dedupe_key: "doomed",
79
          user_id: "user-a",
80
          data: %{"to" => "test@example.com"},
81
          maximum_attempts: 2
82
        )
91
      {user_id, issue_id} = deliverable()
92
      {:ok, effect} = enqueue("doomed", user_id, issue_id: issue_id, maximum_attempts: 2)
83 93
84 94
      assert %{completed: 0, failed: 1} = Worker.run_once(identity: "worker-a")
85 95
      first = Effects.get(effect.id)

@@ -101,22 +111,55 @@ defmodule OpenAgents.Notifications.DeliveryTest do

101 111
      assert Effects.claim_batch("worker-c", now: far_future) == []
102 112
    end
103 113
104
    test "a delivery with no recipient records nothing to send to, not a failure" do
105
      {:ok, effect} =
106
        Delivery.enqueue(
107
          dedupe_key: "no-recipient",
108
          user_id: "user-a",
109
          data: %{"subject" => "hello"}
110
        )
114
    test "a delivery whose account cannot be resolved completes, and is not retried" do
115
      {:ok, effect} = enqueue("nobody", "user-a")
111 116
112 117
      assert %{completed: 1, failed: 0} = Worker.run_once(identity: "worker-a")
113 118
114 119
      done = Effects.get(effect.id)
115 120
      assert done.status == "done"
116 121
      assert done.attempts == 1
117
      assert done.result == %{"outcome" => "nothing_to_send_to"}
122
      assert done.result == %{"outcome" => "recipient_gone"}
118 123
      assert done.last_error == nil
119 124
      assert done.completed_at != nil
120 125
    end
121 126
  end
127
128
  defp enqueue(dedupe_key, user_id, options \\ []) do
129
    defaults = [
130
      dedupe_key: dedupe_key,
131
      user_id: user_id,
132
      issue_id: 1,
133
      kind: "mention",
134
      actor_login: "someone"
135
    ]
136
137
    Delivery.enqueue(Keyword.merge(defaults, options))
138
  end
139
140
  # An account and an event the adapter will actually be asked about: a
141
  # confirmed address, the channel on, and an issue in a repository the account
142
  # can read. Everything `email_dispatch/1` checks has to pass, or the effect
143
  # is refused before the adapter is reached and there is no schedule to test.
144
  defp deliverable do
145
    author = OpenAgents.AccountsFixtures.repository_user_fixture("outbox-author-#{unique()}")
146
    reader = OpenAgents.AccountsFixtures.repository_user_fixture("outbox-reader-#{unique()}")
147
    repository = OpenAgents.AccountsFixtures.repository_with_member_fixture(author)
148
    {:ok, _member} = OpenAgents.Repositories.add_member(repository, reader, "contributor")
149
    {:ok, issue} = OpenAgents.Issues.create_issue(repository, %{title: "a title"}, author)
150
151
    {:ok, pending} =
152
      OpenAgents.Notifications.EmailChannel.set_address(reader, "outbox-#{unique()}@example.com")
153
154
    assert_receive {:email, %Swoosh.Email{subject: subject}}
155
    code = hd(Regex.run(~r/[0-9A-Z]{8}/, subject))
156
    {:ok, verified} = OpenAgents.Notifications.EmailChannel.verify(pending, code)
157
158
    {:ok, _preferences} =
159
      OpenAgents.Notifications.update_preferences(verified, %{email_enabled: true})
160
161
    {verified.id, issue.id}
162
  end
163
164
  defp unique, do: System.unique_integer([:positive])
122 165
end
test/openagents/notifications/email_channel_test.exs added +206

@@ -0,0 +1,206 @@

1
defmodule OpenAgents.Notifications.EmailChannelTest do
2
  @moduledoc """
3
  Proofs for the gate an address has to pass before anything is sent to it.
4
5
  The claim under test is narrow and absolute: `verified_address/1` is the only
6
  read any send resolves its recipient through, and it returns `nil` for every
7
  address a code has not come back from. The database says the same thing
8
  independently, so the last test here goes around this module entirely and
9
  asks PostgreSQL.
10
  """
11
12
  use OpenAgents.DataCase, async: false
13
14
  import OpenAgents.AccountsFixtures
15
  import Swoosh.TestAssertions
16
17
  alias OpenAgents.Accounts.User
18
  alias OpenAgents.Notifications.EmailChannel
19
20
  setup do
21
    %{user: repository_user_fixture("addressee-#{System.unique_integer([:positive])}")}
22
  end
23
24
  describe "recording an address" do
25
    test "stores it unverified, and an unverified address is not a recipient", %{user: user} do
26
      assert {:ok, updated} = EmailChannel.set_address(user, "Someone@Example.com")
27
28
      assert updated.notification_email == "someone@example.com"
29
      assert is_nil(updated.notification_email_verified_at)
30
      assert EmailChannel.verified_address(updated) == nil
31
32
      assert EmailChannel.state(updated) == %{
33
               address: "someone@example.com",
34
               verified?: false,
35
               pending?: true
36
             }
37
    end
38
39
    test "mails the code to the address, and holds only its digest", %{user: user} do
40
      assert {:ok, updated} = EmailChannel.set_address(user, "someone@example.com")
41
42
      assert_email_sent(fn email ->
43
        assert email.to == [{"", "someone@example.com"}]
44
        assert [code] = Regex.run(~r/[0-9A-Z]{8}/, email.subject)
45
        assert email.text_body =~ code
46
        assert updated.notification_email_code_digest == :crypto.hash(:sha256, code)
47
      end)
48
    end
49
50
    test "refuses an address that is not one", %{user: user} do
51
      for bad <- ["", "someone", "someone@example", "two@parts@example.com", "a b@example.com"] do
52
        assert EmailChannel.set_address(user, bad) == {:error, :invalid_address}
53
      end
54
55
      assert_no_email_sent()
56
    end
57
58
    test "refuses to collect an address at all where nothing can be sent", %{user: user} do
59
      configured = Application.get_env(:openagents, EmailChannel)
60
      Application.put_env(:openagents, EmailChannel, Keyword.put(configured, :deliverable, false))
61
      on_exit(fn -> Application.put_env(:openagents, EmailChannel, configured) end)
62
63
      refute EmailChannel.deliverable?()
64
      assert EmailChannel.set_address(user, "someone@example.com") == {:error, :not_deliverable}
65
      assert_no_email_sent()
66
    end
67
  end
68
69
  describe "confirming an address" do
70
    setup %{user: user} do
71
      {:ok, pending} = EmailChannel.set_address(user, "someone@example.com")
72
      %{pending: pending, code: sent_code()}
73
    end
74
75
    test "the right code makes it a recipient", %{pending: pending, code: code} do
76
      assert {:ok, verified} = EmailChannel.verify(pending, code)
77
78
      assert EmailChannel.verified_address(verified) == "someone@example.com"
79
      assert verified.notification_email_verified_at != nil
80
      assert EmailChannel.state(verified).verified?
81
    end
82
83
    test "the code is spent, so the same one cannot be replayed", %{pending: pending, code: code} do
84
      assert {:ok, verified} = EmailChannel.verify(pending, code)
85
      assert EmailChannel.verify(verified, code) == {:error, :nothing_pending}
86
    end
87
88
    test "case and surrounding space do not matter", %{pending: pending, code: code} do
89
      assert {:ok, verified} = EmailChannel.verify(pending, "  #{String.downcase(code)} ")
90
      assert EmailChannel.verified_address(verified) == "someone@example.com"
91
    end
92
93
    test "a wrong code is counted, and the fifth retires the code", %{pending: pending} do
94
      user =
95
        Enum.reduce(1..4, pending, fn attempt, current ->
96
          assert EmailChannel.verify(current, "00000000") == {:error, :incorrect_code}
97
          reloaded = Repo.get!(User, current.id)
98
          assert reloaded.notification_email_code_attempts == attempt
99
          reloaded
100
        end)
101
102
      assert EmailChannel.verify(user, "00000000") == {:error, :too_many_attempts}
103
104
      spent = Repo.get!(User, user.id)
105
      assert is_nil(spent.notification_email_code_digest)
106
      assert EmailChannel.verified_address(spent) == nil
107
    end
108
109
    test "an expired code is refused, and the address stays unverified", %{
110
      pending: pending,
111
      code: code
112
    } do
113
      stale =
114
        pending
115
        |> Ecto.Changeset.change(
116
          notification_email_code_sent_at: DateTime.add(DateTime.utc_now(), -1_801, :second)
117
        )
118
        |> Repo.update!()
119
120
      assert EmailChannel.verify(stale, code) == {:error, :expired}
121
      assert EmailChannel.verified_address(Repo.get!(User, stale.id)) == nil
122
    end
123
124
    test "another code cannot be asked for within the minute", %{pending: pending} do
125
      assert EmailChannel.resend_code(pending) == {:error, :too_soon}
126
    end
127
128
    test "a fresh code retires the old one and resets the guesses", %{
129
      pending: pending,
130
      code: code
131
    } do
132
      assert {:error, :incorrect_code} = EmailChannel.verify(pending, "00000000")
133
134
      guessed =
135
        User
136
        |> Repo.get!(pending.id)
137
        |> Ecto.Changeset.change(
138
          notification_email_code_sent_at: DateTime.add(DateTime.utc_now(), -120, :second)
139
        )
140
        |> Repo.update!()
141
142
      assert {:ok, reissued} = EmailChannel.resend_code(guessed)
143
      reissued_code = sent_code()
144
145
      assert reissued.notification_email_code_attempts == 0
146
      assert EmailChannel.verify(reissued, code) == {:error, :incorrect_code}
147
148
      assert {:ok, verified} = EmailChannel.verify(Repo.get!(User, pending.id), reissued_code)
149
      assert EmailChannel.verified_address(verified) == "someone@example.com"
150
    end
151
  end
152
153
  describe "changing and removing an address" do
154
    setup %{user: user} do
155
      {:ok, pending} = EmailChannel.set_address(user, "first@example.com")
156
      {:ok, verified} = EmailChannel.verify(pending, sent_code())
157
      %{verified: verified}
158
    end
159
160
    test "a new address is not a recipient until it is confirmed too", %{verified: verified} do
161
      assert {:ok, changed} = EmailChannel.set_address(verified, "second@example.com")
162
163
      assert changed.notification_email == "second@example.com"
164
      assert EmailChannel.verified_address(changed) == nil
165
166
      assert {:ok, reverified} = EmailChannel.verify(changed, sent_code())
167
      assert EmailChannel.verified_address(reverified) == "second@example.com"
168
    end
169
170
    test "retyping the confirmed address is not a withdrawal of the proof", %{verified: verified} do
171
      assert {:ok, unchanged} = EmailChannel.set_address(verified, "FIRST@example.com")
172
173
      assert EmailChannel.verified_address(unchanged) == "first@example.com"
174
      assert_no_email_sent()
175
    end
176
177
    test "removal takes the address, the proof, and any outstanding code", %{verified: verified} do
178
      assert {:ok, removed} = EmailChannel.remove_address(verified)
179
180
      assert is_nil(removed.notification_email)
181
      assert is_nil(removed.notification_email_verified_at)
182
      assert is_nil(removed.notification_email_code_digest)
183
      assert EmailChannel.verified_address(removed) == nil
184
      assert EmailChannel.resend_code(removed) == {:error, :nothing_pending}
185
    end
186
  end
187
188
  describe "the database's own half of the gate" do
189
    test "a verified timestamp cannot stand on a row that names no address", %{user: user} do
190
      changeset =
191
        Ecto.Changeset.change(user, notification_email_verified_at: DateTime.utc_now())
192
193
      assert_raise Ecto.ConstraintError, ~r/users_notification_email_state_check/, fn ->
194
        Repo.update(changeset)
195
      end
196
    end
197
  end
198
199
  # The code exists in exactly one place a test may read it: the message that
200
  # was sent. Reading it from the row would prove nothing, because the row holds
201
  # a digest.
202
  defp sent_code do
203
    assert_receive {:email, %Swoosh.Email{subject: subject}}
204
    hd(Regex.run(~r/[0-9A-Z]{8}/, subject))
205
  end
206
end
test/openagents/notifications/email_delivery_test.exs added +283

@@ -0,0 +1,283 @@

1
defmodule OpenAgents.Notifications.EmailDeliveryTest do
2
  @moduledoc """
3
  The email channel, end to end and at its edges.
4
5
  One mention travels the whole path: a comment names somebody, the fan-out
6
  writes a record and an `email.delivery` effect in the same transaction, the
7
  outbox worker claims it, and a message arrives addressed to the mailbox that
8
  account confirmed. The rest of the file is the other half of the claim —
9
  everybody who should get nothing, gets nothing:
10
11
    * an address that was typed but never confirmed
12
    * an account that never turned the channel on
13
    * an account that switched it off, or removed the address, after the enqueue
14
    * a kind the channel does not carry
15
    * a recipient who lost access to the repository after the enqueue
16
17
  The last three matter most. A sent message is the one notification no later
18
  authorization check can withdraw, so the checks have to run on the way out
19
  rather than only on the way in.
20
  """
21
22
  use OpenAgents.DataCase, async: false
23
24
  import OpenAgents.AccountsFixtures
25
  import Swoosh.TestAssertions
26
27
  alias OpenAgents.Effects
28
  alias OpenAgents.Effects.Effect
29
  alias OpenAgents.Effects.Worker
30
  alias OpenAgents.Issues
31
  alias OpenAgents.Notifications
32
  alias OpenAgents.Notifications.EmailChannel
33
  alias OpenAgents.Repositories
34
35
  setup do
36
    author = repository_user_fixture("author-#{System.unique_integer([:positive])}")
37
    reader = repository_user_fixture("reader-#{System.unique_integer([:positive])}")
38
    repository = repository_with_member_fixture(author)
39
    {:ok, _member} = Repositories.add_member(repository, reader, "contributor")
40
    # Distinctive on purpose. "a title" could be absent from a message by
41
    # accident; this cannot, so the refutation below means something.
42
    title = "unrepeatable-title-#{System.unique_integer([:positive])}"
43
    {:ok, issue} = Issues.create_issue(repository, %{title: title}, author)
44
45
    %{author: author, reader: reader, repository: repository, issue: issue}
46
  end
47
48
  describe "a mention to a confirmed address" do
49
    setup %{reader: reader} do
50
      %{reader: subscribe_by_email(reader, "reader@example.com")}
51
    end
52
53
    test "is queued in the transaction that writes the record", %{
54
      author: author,
55
      reader: reader,
56
      issue: issue
57
    } do
58
      {:ok, _comment} = mention(issue, author, reader)
59
60
      assert [notification] = Notifications.list_notifications(reader)
61
      assert notification.kind == "mention"
62
63
      assert [effect] = deliveries()
64
      assert effect.payload["dedupe_key"] == notification.dedupe_key
65
      assert effect.payload["user_id"] == reader.id
66
      assert effect.payload["issue_id"] == issue.id
67
      assert effect.payload["kind"] == "mention"
68
      refute Map.has_key?(effect.payload, "to")
69
    end
70
71
    test "arrives at the confirmed mailbox, carrying a pointer and no title", %{
72
      author: author,
73
      reader: reader,
74
      repository: repository,
75
      issue: issue
76
    } do
77
      {:ok, _comment} = mention(issue, author, reader)
78
79
      drain()
80
81
      assert_receive {:email, email}
82
      assert email.to == [{"", "reader@example.com"}]
83
      assert email.from == {"OpenAgents", "notifications@openagents.com"}
84
      assert email.subject =~ author.github_login
85
      assert email.subject =~ "#{repository.owner}/#{repository.name}##{issue.number}"
86
      assert email.text_body =~ "/#{repository.owner}/#{repository.name}/issues/#{issue.number}"
87
      refute email.subject =~ issue.title
88
      refute email.text_body =~ issue.title
89
90
      assert [%{status: "done", result: %{"outcome" => "sent"}}] = deliveries()
91
    end
92
93
    test "a replayed fan-out is one record and one send", %{
94
      author: author,
95
      reader: reader,
96
      issue: issue
97
    } do
98
      {:ok, comment} = mention(issue, author, reader)
99
      Notifications.comment_created(issue, comment, author)
100
101
      assert length(Notifications.list_notifications(reader)) == 1
102
      assert length(deliveries()) == 1
103
104
      drain()
105
106
      assert_receive {:email, %Swoosh.Email{to: [{"", "reader@example.com"}]}}
107
      assert_no_email_sent()
108
    end
109
  end
110
111
  describe "who gets nothing" do
112
    test "an address that was typed but never confirmed", %{
113
      author: author,
114
      reader: reader,
115
      issue: issue
116
    } do
117
      {:ok, unconfirmed} = EmailChannel.set_address(reader, "reader@example.com")
118
      {:ok, _preferences} = Notifications.update_preferences(unconfirmed, %{email_enabled: true})
119
      assert_receive {:email, %Swoosh.Email{}}
120
121
      {:ok, _comment} = mention(issue, author, reader)
122
123
      assert [_notification] = Notifications.list_notifications(reader)
124
      assert deliveries() == []
125
      assert_no_email_sent()
126
    end
127
128
    test "an account that never turned the channel on", %{
129
      author: author,
130
      reader: reader,
131
      issue: issue
132
    } do
133
      confirm_address(reader, "reader@example.com")
134
135
      {:ok, _comment} = mention(issue, author, reader)
136
137
      assert [_notification] = Notifications.list_notifications(reader)
138
      assert deliveries() == []
139
      assert_no_email_sent()
140
    end
141
142
    test "an account that switched the channel off after the enqueue", %{
143
      author: author,
144
      reader: reader,
145
      issue: issue
146
    } do
147
      reader = subscribe_by_email(reader, "reader@example.com")
148
      {:ok, _comment} = mention(issue, author, reader)
149
      {:ok, _preferences} = Notifications.update_preferences(reader, %{email_enabled: false})
150
151
      drain()
152
153
      assert [%{status: "done", result: %{"outcome" => "channel_off"}}] = deliveries()
154
      assert_no_email_sent()
155
    end
156
157
    test "an account that removed the address after the enqueue", %{
158
      author: author,
159
      reader: reader,
160
      issue: issue
161
    } do
162
      reader = subscribe_by_email(reader, "reader@example.com")
163
      {:ok, _comment} = mention(issue, author, reader)
164
      {:ok, _removed} = EmailChannel.remove_address(reader)
165
166
      drain()
167
168
      assert [%{status: "done", result: %{"outcome" => "no_verified_address"}}] = deliveries()
169
      assert_no_email_sent()
170
    end
171
172
    test "a recipient who lost access to the repository after the enqueue", %{author: author} do
173
      dropped = repository_user_fixture("dropped-#{System.unique_integer([:positive])}")
174
      private = repository_with_member_fixture(author, %{visibility: "private"})
175
      {:ok, _member} = Repositories.add_member(private, dropped, "contributor")
176
      {:ok, issue} = Issues.create_issue(private, %{title: "a title"}, author)
177
178
      dropped = subscribe_by_email(dropped, "dropped@example.com")
179
      {:ok, _comment} = mention(issue, author, dropped)
180
      assert [_queued] = deliveries()
181
182
      :ok = Repositories.remove_member(private, author, dropped.id)
183
184
      drain()
185
186
      assert [%{status: "done", result: %{"outcome" => "not_readable"}}] = deliveries()
187
      assert_no_email_sent()
188
    end
189
190
    test "a kind the channel does not carry", %{
191
      author: author,
192
      reader: reader,
193
      issue: issue
194
    } do
195
      reader = subscribe_by_email(reader, "reader@example.com")
196
      {:ok, _subscription} = Notifications.subscribe(issue, reader, "manual")
197
198
      {:ok, _comment} = Issues.create_comment(issue, %{"body" => "no names here"}, author)
199
200
      assert [notification] = Notifications.list_notifications(reader)
201
      assert notification.kind == "issue_comment"
202
      assert deliveries() == []
203
      assert_no_email_sent()
204
    end
205
  end
206
207
  describe "a provider that cannot be reached" do
208
    defmodule FailingAdapter do
209
      @moduledoc false
210
      @behaviour OpenAgents.Notifications.Delivery.Adapter
211
212
      @impl true
213
      def deliver(_recipient, _data), do: {:error, :provider_unreachable}
214
    end
215
216
    setup do
217
      configured = Application.get_env(:openagents, OpenAgents.Notifications.Delivery)
218
219
      Application.put_env(:openagents, OpenAgents.Notifications.Delivery,
220
        adapter: __MODULE__.FailingAdapter
221
      )
222
223
      on_exit(fn ->
224
        Application.put_env(:openagents, OpenAgents.Notifications.Delivery, configured)
225
      end)
226
227
      :ok
228
    end
229
230
    test "leaves the delivery pending, and the outbox tries it again", %{
231
      author: author,
232
      reader: reader,
233
      issue: issue
234
    } do
235
      reader = subscribe_by_email(reader, "reader@example.com")
236
      {:ok, _comment} = mention(issue, author, reader)
237
238
      before = DateTime.utc_now()
239
      drain()
240
241
      assert [failed] = deliveries()
242
      assert failed.status == "pending"
243
      assert failed.attempts == 1
244
      assert failed.last_error =~ "provider_unreachable"
245
      assert DateTime.compare(failed.available_at, before) == :gt
246
247
      later = DateTime.add(failed.available_at, 1, :second)
248
      assert [retried] = Effects.claim_batch("worker-b", now: later)
249
      assert retried.attempts == 2
250
    end
251
  end
252
253
  defp mention(issue, author, reader) do
254
    Issues.create_comment(
255
      issue,
256
      %{"body" => "@#{reader.github_login} would you look at this"},
257
      author
258
    )
259
  end
260
261
  defp deliveries do
262
    Repo.all(from effect in Effect, where: effect.kind == "email.delivery")
263
  end
264
265
  defp drain do
266
    _pass = Worker.run_once(identity: "worker-#{System.unique_integer([:positive])}")
267
    :ok
268
  end
269
270
  defp subscribe_by_email(user, address) do
271
    verified = confirm_address(user, address)
272
    {:ok, _preferences} = Notifications.update_preferences(verified, %{email_enabled: true})
273
    verified
274
  end
275
276
  defp confirm_address(user, address) do
277
    {:ok, pending} = EmailChannel.set_address(user, address)
278
    assert_receive {:email, %Swoosh.Email{subject: subject}}
279
    code = hd(Regex.run(~r/[0-9A-Z]{8}/, subject))
280
    {:ok, verified} = EmailChannel.verify(pending, code)
281
    verified
282
  end
283
end
test/openagents_web/live/notifications_live_test.exs modified +105 -1

@@ -1,10 +1,13 @@

1 1
defmodule OpenAgentsWeb.NotificationsLiveTest do
2
  use OpenAgentsWeb.ConnCase, async: true
2
  use OpenAgentsWeb.ConnCase, async: false
3 3
4 4
  import Phoenix.LiveViewTest
5 5
6
  alias OpenAgents.Accounts.User
6 7
  alias OpenAgents.Issues
7 8
  alias OpenAgents.Notifications
9
  alias OpenAgents.Notifications.EmailChannel
10
  alias OpenAgents.Repo
8 11
  alias OpenAgents.Repositories
9 12
10 13
  defp setup_thread(conn, visibility) do

@@ -156,6 +159,107 @@ defmodule OpenAgentsWeb.NotificationsLiveTest do

156 159
    end
157 160
  end
158 161
162
  describe "the email channel" do
163
    test "an address is taken, and is not a recipient until the code comes back", %{conn: conn} do
164
      %{conn: conn, reader: reader} = setup_thread(conn, "public")
165
166
      {:ok, view, _html} = live(conn, ~p"/notifications")
167
168
      assert has_element?(view, "#notification-email-address-form")
169
      refute has_element?(view, "#notification-email-code-form")
170
      refute has_element?(view, "#notification-email-channel-form")
171
172
      view
173
      |> form("#notification-email-address-form", %{"email" => %{"address" => "me@example.com"}})
174
      |> render_submit()
175
176
      pending = Repo.get!(User, reader.id)
177
      assert pending.notification_email == "me@example.com"
178
      assert EmailChannel.verified_address(pending) == nil
179
180
      # The switch that would turn the channel on is not offered yet, because
181
      # there is nowhere for it to send.
182
      assert has_element?(view, "#notification-email-code-form")
183
      refute has_element?(view, "#notification-email-channel-form")
184
185
      assert_receive {:email, %Swoosh.Email{subject: subject}}
186
      code = hd(Regex.run(~r/[0-9A-Z]{8}/, subject))
187
188
      view
189
      |> form("#notification-email-code-form", %{"email" => %{"code" => code}})
190
      |> render_submit()
191
192
      assert EmailChannel.verified_address(Repo.get!(User, reader.id)) == "me@example.com"
193
      assert has_element?(view, "#notification-email-channel-form")
194
      assert has_element?(view, "#notification-email-verified")
195
    end
196
197
    test "a wrong code leaves the address unverified", %{conn: conn} do
198
      %{conn: conn, reader: reader} = setup_thread(conn, "public")
199
200
      {:ok, view, _html} = live(conn, ~p"/notifications")
201
202
      view
203
      |> form("#notification-email-address-form", %{"email" => %{"address" => "me@example.com"}})
204
      |> render_submit()
205
206
      html =
207
        view
208
        |> form("#notification-email-code-form", %{"email" => %{"code" => "00000000"}})
209
        |> render_submit()
210
211
      assert html =~ "not right"
212
      assert EmailChannel.verified_address(Repo.get!(User, reader.id)) == nil
213
    end
214
215
    test "the channel switch is off until the account turns it on", %{conn: conn} do
216
      %{conn: conn, reader: reader} = setup_thread(conn, "public")
217
218
      {:ok, pending} = EmailChannel.set_address(reader, "me@example.com")
219
      assert_receive {:email, %Swoosh.Email{subject: subject}}
220
      {:ok, _verified} = EmailChannel.verify(pending, hd(Regex.run(~r/[0-9A-Z]{8}/, subject)))
221
222
      refute Notifications.preferences(reader).email_enabled
223
224
      {:ok, view, _html} = live(conn, ~p"/notifications")
225
226
      view
227
      |> form("#notification-email-channel-form", %{"channel" => %{"email_enabled" => "true"}})
228
      |> render_change()
229
230
      assert Notifications.preferences(reader).email_enabled
231
    end
232
233
    test "removing the address takes the switch with it", %{conn: conn} do
234
      %{conn: conn, reader: reader} = setup_thread(conn, "public")
235
236
      {:ok, pending} = EmailChannel.set_address(reader, "me@example.com")
237
      assert_receive {:email, %Swoosh.Email{subject: subject}}
238
      {:ok, _verified} = EmailChannel.verify(pending, hd(Regex.run(~r/[0-9A-Z]{8}/, subject)))
239
240
      {:ok, view, _html} = live(conn, ~p"/notifications")
241
      assert has_element?(view, "#notification-email-channel-form")
242
243
      view |> element("#notification-email-remove") |> render_click()
244
245
      refute has_element?(view, "#notification-email-channel-form")
246
      assert EmailChannel.verified_address(Repo.get!(User, reader.id)) == nil
247
    end
248
249
    test "a deployment with no mail provider collects no address at all", %{conn: conn} do
250
      configured = Application.get_env(:openagents, EmailChannel)
251
      Application.put_env(:openagents, EmailChannel, Keyword.put(configured, :deliverable, false))
252
      on_exit(fn -> Application.put_env(:openagents, EmailChannel, configured) end)
253
254
      %{conn: conn} = setup_thread(conn, "public")
255
256
      {:ok, view, _html} = live(conn, ~p"/notifications")
257
258
      assert has_element?(view, "#notifications-email-unavailable")
259
      refute has_element?(view, "#notification-email-address-form")
260
    end
261
  end
262
159 263
  describe "the subscribe control on an issue" do
160 264
    test "an author can unsubscribe and subscribe again", %{conn: conn} do
161 265
      %{conn: conn, reader: reader, repository: repository, issue: issue} =

This page updates live while a promote is in flight · changelog