ash-project / ash-project/ash

Proposal(4.0): Make adding a `transform` required when defining a pubsub notification

Open
#1,792 0 comments 5 reactions 0 assignees View on GitHub
enhancement
Dominant language
Elixir
Stars
2.5k
Forks
426
Avg merge
23h 26m
Merged PRs (30d)
46

Description

**Is your feature request related to a problem? Please describe.**

When using default pubsub notifications, eg. with code like the following:

```elixir
pub_sub do
prefix "artist_followers"
module TunezWeb.Endpoint
publish :create, [:artist_id, "create"]
publish :destroy, [:artist_id, "destroy"]
end
```

The payload for the pubsub broadcast (no matter which [broadcast type](https://hexdocs.pm/ash/dsl-ash-notifier-pubsub.html#broadcast-types) is configured) is large and contains a lot of unnecessary information, eg. for a create action the payload is:

```elixir
%Ash.Notifier.Notification{
resource: Tunez.Music.ArtistFollower,
domain: Tunez.Music,
action: %Ash.Resource.Actions.Create{...},
data: #Tunez.Music.ArtistFollower<...>,
changeset: #Ash.Changeset<...>,
actor: #Tunez.Accounts.User<...>,
for: [Ash.Notifier.PubSub],
from: #PID<0.886.0>,
metadata: %{}
}
```

It includes the full data definition of the action, the record being created, the changeset from calling the action, and also the actor details.

This means that recipients of the pubsub messages may be receiving data that they don't have access to read, because by design any liveview can subscribe to any endpoint, and we have no way of knowing who will be receiving the data in the notification.

This is _especially_ true for actor data; the notification contains the full actor resource (along with any other data that was loaded on the actor when they called the action).

**Describe the solution you'd like**

Ash recently added support for adding a PubSub [transform](https://hexdocs.pm/ash/dsl-ash-notifier-pubsub.html#pub_sub-transform) - a function that can pre-process a notification before it is sent. This can be used for unloading or otherwise scrubbing any potentially sensitive data before the notification is sent.

It isn't currently required to use a transform when defining a pubsub notifier, but it might be a good idea to make it so. We should be encouraging users to use them!

We can also provide some built-in transformers, such as `scrub_actor` to unload the actor data (or replace it with only the `id` of the actor).

**Describe alternatives you've considered**

We could also create a new data type such as a much-slimmer `Broadcast` that gets sent instead of the full `Notification`, perhaps only including the record being modified. We can then use a default transformer to convert the Notification to a Broadcast, before broadcasting it.

If users want to provide their own transform function, that function could receive the full Notification, and be required to return a map that is then used as the record in the Broadcast struct.

**Additional context**

Either implementation would definitely be a breaking change, so only possible for Ash 4.0.

The PubSub docs can be updated *now* to point out the potential pitfalls of not using a transform function.

Contributor guide

Open the contributing guide

Research direction

Start with the Ash PubSub notifier transform documentation linked in the issue and the existing pubsub notification behavior. Resolve the Ash 4.0 breaking-change direction—required transforms, built-in scrubbing, or a slimmer Broadcast—and define what completion means. Update the PubSub docs now to warn about unsanitized notification data.

Written by the indexing model from the issue text.

Assessment

Tech stack
elixir
Domain
backend, distributed-systems, documentation
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.