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

Cookies Samesite attribute handling #322

Closed
suren-khatana opened this issue Feb 3, 2022 · 4 comments · Fixed by #330
Closed

Cookies Samesite attribute handling #322

suren-khatana opened this issue Feb 3, 2022 · 4 comments · Fixed by #330
Labels
enhancement New feature or request

Comments

@suren-khatana
Copy link

suren-khatana commented Feb 3, 2022

Describe the problem

I have a SPA application that talks to the backend BFF layer (which handles all OAuth flows using express-openid-connect lib). I would like to set samesite =strict for the appSession cookie containing the tokens so that the cookie is protected and only sent to my BFF component.

In order to achieve this , I have set the cookieconfigparams as cookie: { httpOnly: true, sameSite: 'Strict', secure: true } but the problem is that this configuration also sets the samesite =strict for auth_verification cookie as well. This prevents the auth_verification cookie to be sent to the /callback since it is redirected from a third party auth server domain and that causes the following error :

BadRequestError: checks.state argument is missing at /home/ec2-user/bff-code/bff-layer/node_modules/express-openid-connect/middleware/auth.js:121:31 at processTicksAndRejections (node:internal/process/task_queues:96:5)

I have following questions :

  1. How to set the samesite =strict for the encrypted cookie but keep samesite =lax for the auth_verification cookie ?
  2. How to set __Host- for the appSession cookie using cookieconfigparams ?
  3. Is there any support available for CSRF protection of appSession cookie containing the tokens ?
@BitPatty
Copy link
Contributor

BitPatty commented Feb 4, 2022

Regarding 1) Experiencing the same issue, looks like the configuration for the cookie is being shared and there's no builtin way to override the behaviour:

transient.store('auth_verification', req, res, {
sameSite:
options.authorizationParams.response_mode === 'form_post'
? 'None'
: config.session.cookie.sameSite,
value: JSON.stringify(authVerification),
});

@adamjmcgrath
Copy link
Contributor

Hi @surensinghkhatana - thanks for raising this

How to set the samesite =strict for the encrypted cookie but keep samesite =lax for the auth_verification cookie ?

Currently the cookie config is shared for the session and transaction cookies. But your use case sounds like a reasonable one, so I'll keep this open as a possible enhancement.

How to set __Host- for the appSession cookie using cookieconfigparams ?

You can configure the cookie name using the session.name config

Is there any support available for CSRF protection of appSession cookie containing the tokens ?

The library doesn't provide csrf protection (other than the default cookie config SameSite=Lax). This is a separate concern from login and you should use one of the available express csrf protection middlewares if you want to add this.

@adamjmcgrath adamjmcgrath added the enhancement New feature or request label Feb 4, 2022
@suren-khatana
Copy link
Author

suren-khatana commented Feb 4, 2022

Thanks @adamjmcgrath for quick response. Appreciate your help.

You can configure the cookie name using the session.name config

I tried this session: { name: '__Host-BFFCookie', absoluteDuration: 150, cookie: { httpOnly: true, secure: true, } }` but it doesn't accept that.

`/home/ec2-user/bff-code/bff-layer/node_modules/express-openid-connect/lib/config.js:209
throw new TypeError(error.details[0].message);
^

TypeError: "session.name" must only contain alpha-numeric and underscore characters_
    at module.exports.get (/home/ec2-user/bff-code/bff-layer/node_modules/express-openid-connect/lib/config.js:209:11)
    at auth (/home/ec2-user/bff-code/bff-layer/node_modules/express-openid-connect/middleware/auth.js:28:18)
    at Object.<anonymous> (/home/ec2-user/bff-code/bff-layer/src/bff.js:16:3)
    at Module._compile (node:internal/modules/cjs/loader:1101:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)

@adamjmcgrath
Copy link
Contributor

TypeError: "session.name" must only contain alpha-numeric and underscore characters_

Ah - thanks for testing that @surensinghkhatana, when I pick up this ticket I'll look into that limitation as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants