tdd/node-demo

xhr "next question" button doesn't respond

Mouvedia opened this issue · 9 comments

express:router errorHandler  : /admin/quizzes/1/next
Error: invalid csrf token
    at createToken (…/node_modules/csurf/index.js:74:19)
etc.

It's undefined at that point. Is there a gotcha with csurf?

tdd commented

Hey there,

Yes, this is due to

$(document).on 'click', 'a#nextQuestion', (e) ->
not injecting the CSRF token, when it should.

Man, this code is old. I would so need a complete revamping… 😕

@tdd Pourquoi fermes-tu le ticket sans un commit pour le régler ?

tdd commented

bin parce que là je dois de toutes façons réécrire complètement le bousin… du coup j'ai en tête une sorte de grosse meta-issue 😕

@tdd Vu que je bosse dessus là tu pourrais me conseiller la meilleure méthode pour retrieve le token ?

En gros si je fais

    a#nextQuestion.btn.btn-primary(
      href="/admin/quizzes/#{quizId}/next",
      data-token=csrfToken
    )

ça ne fonctionne pas.

Si je dois call csrfToken(), ai-je une globale me donnant accès à un objet req disponible depuis quizzes.coffee?

tdd commented

Hello,

Bin oui, un lien n'a pas cette info, et surtout le CSRF est là car notre "next" envoie bien une requête modifiante (en l'espèce, PUT), grâce au code de questions.coffee.

Je suggèrererais deux approches possibles :

  • celle de Ruby On Rails : ton layout publie le token dans une <meta name="csrf-token" content="…"> dans le <head>, et tes scripts client le retrouvent là.
  • celle zéro-JS : au lieu d'un lien, tu fais un formulaire sans style aucun (inline, notamment), avec le bon action, un method="POST", un champ caché pour le token, et le lien devient un <button type="submit" class="btn btn-…">.

Bien à toi,

tdd commented

(ah oui, et un champ caché _method à PUT, aussi)

@tdd par "layout" tu veux dire le template parent layout.jade ?
Vu que tu vas le réécrire je ne PR pas.

En passant #3 peut être fermé.

replace this line with

    $.ajax(
           e.currentTarget.href,
           type: 'post',
           data: { _method: 'put' },
           beforeSend: (xhr) ->
            if !xhr.crossDomain
              xhr.setRequestHeader 'X-CSRF-Token', $('meta[name="csrf-token"]').attr 'content'
          )
tdd commented

Yes, csurf default strategy now uses request headers. That's kinda nice, but not easy to work with outside of JS-send (Ajax) requests, e.g. regular form submissions. I use hidden fields generally, and a custom csurf resolver. I'll probably do that instead.