Skip to content

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

Description

@merhard

Summary

resources/subscribe accepts any URI string from any connected session and never consults the server author. There is no callback, no plug, and no overridable function between the client's uri parameter and the subscription being created. A server whose resources are per-user cannot restrict who subscribes to what. Verified against main at 0.5.1.

Impact

Affected servers are those that configure pubsub, call Phantom.Tracker.notify_resource_updated/1 for user-scoped resources, and rely on resources/read authorization to scope them. Nothing internal to Phantom emits notifications/resources/updated, so a server that never calls it has no leak - an unauthorized subscription just sits there. The gap is real only once notifications flow.

For a server that does notify, a session can subscribe to any URI it can name and learn:

  1. Timing. notifications/resources/updated carries a URI and no body, so the subscriber learns when a resource it may not read changed, at the resolution of the server's write path.
  2. Existence. The reply to subscribe is identical for a URI that resolves and one that does not, but subsequent notification traffic is not. Receiving an update notification for a URI confirms that the resource exists, so a session can probe for resources it cannot read by subscribing and waiting.

Note that the subscribe surface is not opt-in: router.ex:1067 advertises the capability as subscribe: not is_nil(session.pubsub), so it is exposed by any server configured with pubsub for SSE streams or logging, whether or not it intended to offer subscriptions.

Every comparable surface has a hook; this one doesn't

Method Author's control point
resources/read handler function per resource; returning nil yields resource_not_found
resources/list list_resources/2, in defoverridable (router.ex:530)
resources/templates/list Session.allow_resource_templates/2, applied by Cache.list/3 (cache.ex:86-93)
tools/call handler function per tool
prompts/get handler function per prompt
resources/subscribe none

resources/subscribe is the only method that takes a client-supplied resource identifier and acts on it without passing through author code. The read path and the subscribe path accept the same uri parameter, and only one of them is authorized.

Not fixable downstream

dispatch_method/4 isn't in defoverridable (router.ex:529-535). The options available today are a plug in front of Phantom.Plug that re-parses the JSON-RPC body, or a vendored fork. We shipped the plug; it duplicates knowledge of the request shape including the _json batch form, and has to reject a whole batch when one subscribe in it is unauthorized.

Suggested fix

One new overridable callback, defaulting to permit, so the change is additive and no existing server behaves differently after upgrading.

Behaviour, near list_resources/2 (router.ex:106-131):

@callback authorize_subscription(uri :: String.t(), Session.t()) :: :ok | {:error, map()}

Docs worth stating on it: the URI has not been matched against any declared template; rejecting with Phantom.Request.resource_not_found/1 keeps an unauthorized subscription indistinguishable from one to a nonexistent URI; servers with per-user resources should implement it because authorizing resources/read alone does not restrict who may subscribe.

Default implementation alongside the other injected defaults:

@impl true
def authorize_subscription(_uri, _session), do: :ok

Guard the clause (router.ex:434):

 def dispatch_method("resources/subscribe", %{"uri" => uri} = _params, request, session) do
   if is_nil(session.pubsub) do
     {:error, Request.not_found(), session}
   else
-    case Session.subscribe_to_resource(session, uri) do
-      :ok ->
-        {:reply, %{}, session}
-
-      _ ->
-        {:error, Request.not_found("SSE stream not open"), session}
+    with :ok <- authorize_subscription(uri, session),
+         :ok <- Session.subscribe_to_resource(session, uri) do
+      {:reply, %{}, session}
+    else
+      {:error, error} -> {:error, error, session}
+      _ -> {:error, Request.not_found("SSE stream not open"), session}
     end
   end
 end

Extend defoverridable (router.ex:530):

       defoverridable list_resources: 2,
+                     authorize_subscription: 2,
                      server_info: 1,

Open question: raw URI or resolved template?

The signature above hands the author a raw URI, so every implementation must resolve it before it can authorize it. Downstream we needed a resolver that maps a URI back to the %Phantom.ResourceTemplate{} it addresses plus the path params it supplies. Phantom has no public equivalent.

@callback authorize_subscription(uri :: String.t(), Session.t()) :: :ok | {:error, map()}
# or
@callback authorize_subscription(ResourceTemplate.t(), params :: map(), Session.t()) :: :ok | {:error, map()}

The raw-URI form is proposed because it is the smaller change and cannot break on a URI that matches no template. The resolved form is friendlier but forces a decision about unresolvable URIs before the author is consulted, and would want reverse resolution exposed as a public function in its own right. Happy to follow whichever you prefer.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions