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

Helmet expects an Express request object but koa-helmet passes it a Node request #18

Closed
ide opened this issue Dec 5, 2015 · 1 comment · Fixed by #27
Closed

Helmet expects an Express request object but koa-helmet passes it a Node request #18

ide opened this issue Dec 5, 2015 · 1 comment · Fixed by #27

Comments

@ide
Copy link

ide commented Dec 5, 2015

This is specifically a problem when Helmet is looking for fields on the request that a Node request doesn't have, like request.secure.

You can see this with the hsts() middleware which doesn't send the Strict-Transport-Security header since ctx.req.secure is always undefined. Passing in ctx.request fixes this specific issue, but there might not be a robust solution at hand since Koa request objects aren't guaranteed to be the same as Express request objects.

For the hsts() middleware, an easy workaround is to configure the middleware with { force: ctx.request.secure } but it's a little fragile that Helmet's logic for whether to send the HSTS response header needs to be replicated.

@EvanHahn
Copy link
Contributor

EvanHahn commented Jan 8, 2016

Yeah, this one is tricky.

hsts is the only Helmet module that works better with Express request objects, so luckily it's only for that middleware.

You could wrap hsts and override the default setIf option, but I don't love the idea of special cases.

Happy to change something in Helmet if needed.

venables added a commit that referenced this issue Jan 15, 2017
Several changes:
* Updates Helmet to 3.x major
* Passes along req.secure so the helmet-hsts module knows if the
  request is https or not (it expects an express object)
  Closes #25, closes #18
* Updates documentation for req.secure change
* Switches to use standard.js
* Removes unused devDependencies
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 a pull request may close this issue.

2 participants