taoensso / sente

Realtime web comms library for Clojure/Script
https://www.taoensso.com/sente
Eclipse Public License 1.0
1.73k stars 193 forks source link

Example of what to pass for CSRF client argument for node.js #406

Closed dmg46664 closed 1 year ago

dmg46664 commented 1 year ago

The node js example is from 1.11.0 https://github.com/theasp/sente-nodejs-example/blob/master/project.clj#L20

The API now insists on a CSRF argument for the client: https://github.com/ptaoussanis/sente/blob/master/src/taoensso/sente.cljc#L1670

What are you supposed to pass here? nil? I'm getting other errors and unsure if this is the issue. I am migrating client code from https://www.npmjs.com/package/ws (their api and core.async) and they don't insist on passing any equivalent argument.

Thanks.

ptaoussanis commented 1 year ago

@dmg46664 Hi Daniel,

Please see the README Getting Started section - it includes info on CSRF. Would suggest searching the README in general for "csrf" - since there's a few different places that CSRF is discussed.

Hope that helps! Cheers :-)

dmg46664 commented 1 year ago

@ptaoussanis

I looked at the example on the readme.

  (when-let [el (.getElementById js/document "sente-csrf-token")]
    (.getAttribute el "data-csrf-token")))

This code will fail for node.js as there is no DOM for node.js ReferenceError: document is not defined

Anything on the page relating to CSRF and middleware isn't relevant for me as I'm not running a server. I.e. the following isn't relevant.

Security: CSRF protection?
This is important. Sente has support, and use is strongly recommended. You'll need to use middleware like [ring-anti-forgery](https://github.com/ring-clojure/ring-anti-forgery) or [ring-defaults](https://github.com/ring-clojure/ring-defaults) to generate and check CSRF codes. The ring-ajax-post handler should be covered (i.e. protected).

Please suggest where else I'm supposed to look?

dmg46664 commented 1 year ago

CSRF is only related to interaction in the browser based on my understanding. Therefore my expectation is if connecting from an app outside the browser (node.js or java), I would be able to ignore this argument. Hopefully you can point out where I've made a mistake. I'm only interested in make-channel-socket-client! 🙏

ptaoussanis commented 1 year ago

Hi Daniel, can you explain in a little more detail exactly what you're doing and what error you're seeing?

Anything on the page relating to CSRF and middleware isn't relevant for me as I'm not running a server.

What is your Sente client meant to connect to?

If CSRF isn't relevant for your use-case, it should be possible to disable the CSRF checks as per the API docs here and here, i.e.:

It's very possible I'm misunderstanding something about your use case though.

dmg46664 commented 1 year ago

@ptaoussanis I can't see where it states you can pass a nil to ?csrf-token-or-fn in the link you've shared. If it said that I'd be happy and therefore my error is somewhere else. Please can we update the docs?

image

In fact it's under the required section of arguments, which adds to the confusion. Seems like it would be good to move it to a key in the map.

ptaoussanis commented 1 year ago

Apologies for the confusion, I use the convention that ?arg implies an argument that can be nil. This is also mirrored in the server API:

  :csrf-token-fn     ; ?(fn [ring-req]) -> CSRF-token for Ajax POSTs and WS handshake.
                     ; CSRF check will be skipped iff nil (NOT RECOMMENDED!).

Can definitely see that that could be confusing. PR welcome to clarify the docstrings, otherwise will do myself next time I'm doing batched Sente work 👍

Still, it would be helpful to know what error you're seeing to help rule out any issue on Sente's end. I never use nor test Sente with NodeJS myself, so it's conceivable something broke for Node somewhere along the line.

ptaoussanis commented 1 year ago

Just updated docstrings on master, hope that's better?

dmg46664 commented 1 year ago

Great! Clear! Thanks 👍