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

Ability to handle multiple cookies in context #66

Open
stefanoschrs opened this issue Oct 14, 2022 · 5 comments
Open

Ability to handle multiple cookies in context #66

stefanoschrs opened this issue Oct 14, 2022 · 5 comments

Comments

@stefanoschrs
Copy link

Is there an easy way to handle the scenario of multiple cookies in context and not just the default csrf_token ?

It's regarding an iframe integration scenario that the same frame will be included multiple times in the page and the way the library is now, each frame will overwrite the csrf_token meaning if the 1st form submits then it will have a different token than the latest in the context.

For setting the tokens with different names I've managed to simply append the frameId inside the HandlerFunc but the problem is in the verification step where the context is the same.

@justinas
Copy link
Owner

Could you elaborate on the issue? Although the masked token will be different across page (frame) loads, the unmasked token remains the same for the lifetime of the cookie, meaning any of the masked tokens should work when submitted?

@stefanoschrs
Copy link
Author

How can I provide to you more information?

Here's the middleware:

v1ClientRouter.Use(func() gin.HandlerFunc {
  u, _ := url.Parse(os.Getenv("BASE_URL"))
  
  nextHandler, wrapper := adapter.New()
  csrfHandler := nosurf.New(nextHandler)
  csrfHandler.SetBaseCookie(http.Cookie{
    Name:       "csrf_token",
    Path:       u.Path,
    Domain:     u.Host,
    Expires:    time.Now().Add(5 * time.Minute),
    RawExpires: "",
    MaxAge:     60 * 5,
    Secure:     true,
    HttpOnly:   true,
    SameSite:   http.SameSiteNoneMode,
  })
  csrfHandler.ExemptPath(v1ClientRouter.BasePath() + "/action")
  return wrapper(csrfHandler)
}())

I set the token in the template like this:

c.HTML(http.StatusOK, "form.tmpl", gin.H{
  "CSRF":             nosurf.Token(c.Request),
  ...
})

On form submission I include the CSRF token and check like this:

nosurf.VerifyToken(nosurf.Token(c.Request), body.CSRFToken)

It works randomly, that's why I think it's overwriting the token/cookie depending on which iframe loads first on the screen

2022-10-19T10:48:30.377+0200	debug	logging/logging.go:122	NOSURF	{"verification": false, "token": "44ZWy3vzYszGx7VvZWMDRq+9dVwzTliqFwxzyZXrr50/tCtU1smMFw4xw8i8o6UAFmAFrJ0FJO9qvnMRkkXlYw==", "body.CSRFToken": "VhG8BSyab2jMh7dcBp8HNn8whRWRCuupTGIuRe+6TSyAJdx66Vxp6ZFXn7gYRiuPYWlba3AjZMbgbnuGQy0wag=="}
2022-10-19T10:48:35.781+0200	debug	logging/logging.go:122	NOSURF	{"verification": true, "token": "/9d5EIpIsRJ2enCleZS0To6ogx52q55P5uGPy2go1/Aj5QSPJ3Jfyb6MBgKgVBIIN3Xz7tjg4gqbU48Tb4adDg==", "body.CSRFToken": "9i9Eh/ONUH/HDvbE4AwCQmgg13lcZ/o58z8ubk4C9YYqHTkYXre+pA/4gGM5zKQE0f2nifIshnyOjS62Say/eA=="}

@stefanoschrs
Copy link
Author

@justinas any thoughts on this?

@justinas
Copy link
Owner

justinas commented Nov 12, 2022

I think I understand the issue now. nosurf works correctly when the user's first-interaction-ever with the website is not concurrent with any other page load on the website. However, when doing two "initial loads" at the same time, it tries to set two cookies at the same time.

I do not yet have a good fix in mind for this. Is there a way in your case to make some initial request that would set the cookie? Then, you can load the two frames and they will operate on the existing cookie, and not set a new one.


By the way, this seems like it will not work as expected:

    Expires:    time.Now().Add(5 * time.Minute),

This sets all nosurf cookies to expire at the absolute time "5 minutes after the middleware is initially constructed". I am not sure if MaxAge overrides this, if so, this Expires is not even necessary.

In general, setting the cookie to such a short duration, will exacerbate such problems, as the cookies will expire more often. nosurf is designed to work with long-living cookies. That is why MaxAge is set to 1 year by default.

@stefanoschrs
Copy link
Author

I'm afraid there's no possibility of an initial request as it's used by a 3rd party.

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

No branches or pull requests

2 participants