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

Allow setting axios client options per request #631

Closed
wants to merge 4 commits into from

Conversation

djmadeira
Copy link

No description provided.

@felixmosh
Copy link
Owner

Hi man, this approach has similar disadvantage as the previous one, you set the options of first html render, since the app is SPA, there will no other html renders at all, this means the csrf token is "static" again.

The best approach would be to add some cookie (name in the uiConfig) which will tell the client to add as header in each request, with a middlewere you can update it at each api call)

@felixmosh
Copy link
Owner

createBullBoard({
  queues: [
    new BullAdapter(someQueue, {uiConfig: {csrf: {cookie: 'name-here', header: 'other-name-here'}}}),
  ],
  serverAdapter
})

// ... express server configuration

app.use(basePath, 
  (req, res, next) => {
    if(req.path.includes('/api')) {
      res.cookie('name-here', 'secret-value')
    }
    next();
  },
  serverAdapter.getRouter());

Then the only thing you need to add is on the client side, if this value exists, attach it to the requests...

@djmadeira
Copy link
Author

djmadeira commented Nov 28, 2023

@felixmosh unfortunately your recommendation won't work for the CSRF protection method used by the library we use, and doesn't actually provide much protection against CSRF attacks.

csrf-csrf uses a double submit cookie pattern, which generates two tokens, one of which is passed as a Set-Cookie header, and one of which is passed in the response body. The server doesn't actually keep track of the values in a session variable, it just generates the hashes using a shared secret and checks that the two values match and match its secret.

Submitting a cookie alone doesn't provide much protection, since a browser will always submit the cookies it had previously saved for a domain. It's much harder for an attacker to intercept a secret value passed through an HTTPS request.

It's not as much of a concern that the token will become stale, since the server doesn't actually maintain any state about the token, just compares the hashes it had previously generated. This would only fail in the case where a developer had configured the cookie to expire after some time, which is common. Ideally there would be a way to configure how to fetch a fresh token if it becomes stale, but that would require a lot more config:

  • How to know when an API error is related to CSRF token being invalid
  • How to fetch a fresh token (what path to fetch it at, how to get it out of the response body, whether at a certain JSON path, a hidden HTML form item, etc)
  • How to pass the token on subsequent requests (http header, post body, etc)

Since implementations vary, I wanted to make the config as flexible as possible without complicating the library too much.

@felixmosh
Copy link
Owner

felixmosh commented Nov 29, 2023

@djmadeira thank you for explaining me that... but, you didn't got my idea, I didn't told you to add the csrf only to the cookie.

Anyway, Axios do support the double submit mechanism,
image

All you need to do, is add the XSRF-TOKEN cookie, and axios will send it as X-XSRF-TOKEN header on every request.

Exposing AxiosConfig to this lib users will couple the lib to implementation detail (won't allow me to change the underline http client in the future).

I'll add an example using CSRF today (hopefully).

@felixmosh
Copy link
Owner

I've added an example with csrf-csrf lib,

I've made a PR which approved & will release soon, to allow disabling httpOnly flag (which is not necessary), in meanwhile, the example sets an additional cookie.

@felixmosh felixmosh closed this Nov 30, 2023
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

Successfully merging this pull request may close these issues.

2 participants