Skip to content

Add resource subscription authorization - #30

Draft
dbernheisel wants to merge 1 commit into
mainfrom
resource-subscription-authz
Draft

Add resource subscription authorization#30
dbernheisel wants to merge 1 commit into
mainfrom
resource-subscription-authz

Conversation

@dbernheisel

@dbernheisel dbernheisel commented Aug 13, 2026

Copy link
Copy Markdown
Owner

Closes #29

Summary

  • add a shared authorize_resource_subscriptions/2 callback for subscription requests and update delivery
  • resolve requested resources before authorization and fail closed for unresolved resources, invalid callback results, or callback exceptions
  • add bulk-first Phantom.Tracker.notify_resources_updated/1, grouping subscribed resources per session for one authorization call per batch
  • retain compact resolution metadata with subscriptions to avoid repeating URI path matching during bulk fan-out
  • document application-side update collection and bulk authorization

@dbernheisel

Copy link
Copy Markdown
Owner Author

@merhard would you mind reviewing this to see if this meets your needs re #29

Comment thread lib/phantom/router.ex
Comment on lines +1153 to +1154
rescue
_ -> :error

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no blind rescues.

Comment thread lib/phantom/tracker.ex
Comment on lines +412 to +416
try do
Phoenix.Tracker.get_by_key(__MODULE__, @resources, uri)
rescue
_ -> []
end

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

verify compile without Phoenix.Tracker

@merhard

merhard commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

@merhard would you mind reviewing this to see if this meets your needs re #29

Thanks for doing this! I won't be able to get to the review until Monday, but I should be able to verify it then.

@merhard

merhard commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

@dbernheisel This looks like an approach that would work, but reviewing it raised an interesting question:

Phantom.Router.available_resolved_resources/3 filters the resources list to only those that were permitted using Phantom.Session.allow_resource_templates/2 (or all if that function was not called). If any templates were filtered by that function they would now not be subscribable (or notifiable if a subscription already exists) but they are still reachable by a resources/read call.

Should resources/read also be auto-filtered by the allowed templates to be consistent? Or should subscriptions/notifications not be auto-filtered by the allowed templates leaving all filtering entirely up to the user in application code?

@dbernheisel

Copy link
Copy Markdown
Owner Author

@merhard I don't figure it's Phantoms responsibility for the actual read, because it had to be implemented by the user, whereas the notifications are implemented by Phantom. But let me know if you see a gap.

Are you thinking Phantom wouldn't even call the read callback if the user isn't authorized based on the notification subscription?

@merhard

merhard commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

@dbernheisel I was more just pointing out the inconsistency in case it was unintentional. If anything I would lean more towards none of the three (read, subscribe, notify) auto-referencing the allowed templates and having the user make that call in the hook (subscribe), callback (read), or before the function call (notify). But I think your argument that "it's Phantoms responsibility" because "the notifications are implemented by Phantom" is reasonable.

@dbernheisel

Copy link
Copy Markdown
Owner Author

I like escape hatches, so I'll see if I can add an optional override for notification authz if needed, but otherwise implemented by Phantom by default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

resources/subscribe has no authorization hook for the client-supplied URI

2 participants