Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for subscription initial payload #25

Closed
theikkila opened this issue Aug 3, 2017 · 15 comments · Fixed by #94
Closed

Support for subscription initial payload #25

theikkila opened this issue Aug 3, 2017 · 15 comments · Fixed by #94
Assignees
Milestone

Comments

@theikkila
Copy link
Contributor

When client opens websocket channel into server and issues first connection_init-message it can additionally supply any data in payload of that message.

This can be used ie. authenticating the client and so it should be available in subscription handler context.

It could be feasible to implement this as part of the connection-loop
(https://github.com/walmartlabs/lacinia-pedestal/blob/master/src/com/walmartlabs/lacinia/pedestal/subscriptions.clj#L83)

I'm waiting for comments before implementing this

@hlship
Copy link
Member

hlship commented Aug 5, 2017

I'll think about this over the weekend. It's nice having someone with experience on the client; I'm behind on that!

@theikkila
Copy link
Contributor Author

Apollo GraphQL-client is sending connectionParams as that payload:
ie.

const wsClient = new SubscriptionClient(`wss://graphql-server/graphql-ws`, {
    reconnect: true,
    connectionParams: {
      authorization: "some auth token"
    }
});

@hlship
Copy link
Member

hlship commented Aug 10, 2017

I'll have a chance to look at this tomorrow, I hope.

@theikkila
Copy link
Contributor Author

ping @hlship

@hlship
Copy link
Member

hlship commented Aug 31, 2017

Back from vacation ...

@hlship
Copy link
Member

hlship commented Mar 27, 2018

I've lost track over whether this has actually been implemented yet.

@hlship
Copy link
Member

hlship commented Mar 27, 2018

Looks like #41 in 0.6.0 fixes this?

@gabrielnau
Copy link
Contributor

Hello @hlship, indeed #41 tackles the use case of storing authentication token stored in cookies, but the matter exposed by this github issue and the proposal in #63 is more about implementing Apollo's contract.

Another way to put it is that it's a needed change to gather a context for the subscription, stored in cookies or user-agent for example, but it doesn't fix the use case of providing an initial context through the connection_init payload for example.

On one hand, the connectionParams exposed by Apollo are not in GraphQL spec as far as I know, so it's it shouldn't be in Lacinia spec, on the other hand I would propose Lacinia exposes such flexibility, at a cost I perceive as being little.

@a613
Copy link

a613 commented Oct 10, 2018

Yup, this is a big problem for anyone using the apollo client. The streamer has access to the connection-params, but any regular resolvers (for fields of the subscribed object) do not and neither do the subscription interceptors, meaning that any authorization logic in regular resolvers fail because they don't see the connection-params.

I had a minor success by using with-context in the streamer, in order to get the connection-params to the regular resolver, and that worked but only for the first level of regular resolvers, not further nested ones, which didn't have the added context for some reason despite the docs saying they should.

@hlship
Copy link
Member

hlship commented Nov 13, 2018

Can I close this issue? I believe it is fixed by #63.

@ghost
Copy link

ghost commented Nov 20, 2018

Can I close this issue? I believe it is fixed by #63.

Is there any documentation on how to use this? Did the default behaviour change or do I need to adjust my application to make use of this?

@a613
Copy link

a613 commented Nov 20, 2018

Hi @urzds , all I did was in my streamer functions, instead of returning the streamed value directly, I wrapped it in (lacinia.resolve/with-context value <context map>). Really, I am forcing the context to look how I need it to look in the subfield resolvers.

How it works:

  • websocket subscription requested by a client
  • streamer invoked and starts watching a core.async channel
  • when a value is received, the streamer yields the value wrapped with a forced context map
  • lacinia resolves the arbitrary fields requested by the original subscription query
  • the forced context is used in each of those resolvers

As for getting values from the subscription connection payload, it already works, I think the keys aren't keywordized though so I had to look up the values as strings.

;; this is called from the `:enter` part of an interceptor,
;; where we look it up to find its user and assoc that onto the context map
(defn auth-token
  "Extract auth token from the request headers."
  [context]
  (let [headers (:headers (:request context))
        params (:connection-params context)]
    (or (not-empty (get headers "token"))
        (not-empty (get params :token))
        (not-empty (some-> headers
                           (get "authorization")
                           (str/split #"\s")
                           second)))))

@ghost
Copy link

ghost commented Apr 8, 2019

Could someone please provide a complete example on how to set this up?

I am trying the following, but it appears my interceptor is never being called for subscriptions / WebSocket connections (but it is being called for queries / HTTP requests):

  (let [options {:port          graphql-port
                 :graphiql      true
                 :subscriptions true}
        default-interceptors (lp/default-interceptors compiled-schema options)
        interceptors (-> default-interceptors
                         (lp/inject
                           interceptors/authorize
                           :after
                           ::lp/inject-app-context))
        default-subscription-interceptors (lpsubs/default-subscription-interceptors compiled-schema (:app-context options))
        subscription-interceptors (-> default-subscription-interceptors
                                      (lp/inject
                                        interceptors/authorize
                                        :after
                                        ::lpsubs/inject-app-context))
        service-map (lp/service-map
                     compiled-schema
                     (assoc options
                       :interceptors interceptors
                       :subscription-interceptors subscription-interceptors))
        server (-> service-map
                   (assoc ::http/host graphql-host ...)
                   http/create-server
                   http/start)]
   ...)

I think having some complete but minimal sample code would help a lot.

@a613
Copy link

a613 commented Apr 8, 2019

Sorry @urzds I haven't looked at this code since December so I'm rusty, but here are our :interceptors:

[lac.ped/json-response-interceptor
 lac.ped/graphql-data-interceptor
 lac.ped/status-conversion-interceptor
 lac.ped/missing-query-interceptor
 (lac.ped/query-parser-interceptor compiled-schema)
 lac.ped/disallow-subscriptions-interceptor
 lac.ped/prepare-query-interceptor
 (lac.ped/inject-app-context-interceptor app-context)
 ... custom ones...
 lac.ped/query-executor-handler]

And our :subscription-interceptors:

[subscriptions/exception-handler-interceptor
 subscriptions/send-operation-response-interceptor
 (subscriptions/query-parser-interceptor compiled-schema)
 (subscriptions/inject-app-context-interceptor app-context)
 ... custom ones...
 subscriptions/execute-operation-interceptor]

Remember to set the :keep-alive-ms to 8000 or less so Chrome doesn't reset the websocket too early.

In our streamers, we use with-context as I mentioned above:

(defn- base-streamer
  [mult yield xform]
  ...)

(defn something-happened
  [{:keys [stream]} {:keys [objId]} yield]
  (base-streamer (:mult stream) yield
                 (keep (fn [{:keys [datomic-db event value]}]
                         (when (and (= ::events/something-happened event)
                                    (= objId (::data/objId value)))
                           (lacinia.resolve/with-context value
                                                         {:db datomic-db}))))))

@ghost
Copy link

ghost commented Apr 9, 2019

Thanks! It turned out I had a bug in my frontend code, that prevented the subscription from being submitted correctly. The code I pasted above works.

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

Successfully merging a pull request may close this issue.

4 participants