secure_headers icon indicating copy to clipboard operation
secure_headers copied to clipboard

Stricter validation on samesite configuration

Open vcsjones opened this issue 7 years ago • 1 comments

Hi! 👋

We started using SameSite configuration on a few of our cookies, I misread the docs, goofed, and did this:

samesite: {
  strict: ['butter_cookie']
}

This does not cause a startup error, it just doesn't work, silently.

In other cases when I have made mistakes, secure_headers told me I was doing something wrong, e.g. Unknown config directive: dogs=["better"]. This caused a bit of head scratching because I have come to expect nice errors from here when I make a mistake (thanks for that!)

Is there a use where samesite.strict can be set to an array and do something useful? If not, does it make sense to validate that the strict key can only be assigned to a TrueClass, FalseClass, or a Hash via is_a??

Along the same lines, if it is a hash, it can also silently fail like so

samesite: {
  strict: {
    rly: ['biscotti_cookie']
  }
}

So that leads me to a question of what is the philosophy around validation for configuration? I'm happy to take a stab at a PR to improve validation around cookie configuration, but I wanted to get my head around what should be done, first.

vcsjones avatar Sep 28 '18 16:09 vcsjones

I'm sorry, the correct answer was "oreo_cookie". That's what unlocks all security features :trollface:

Historically, the validation was incredibly valuable. Nowadays, because the validation hasn't kept up with the set of valid configurations, it's limits functionality that can only be fixed in a pull request. It's a trade off.

Currently, the only validation is that a hash is supplied: https://github.com/twitter/secure_headers/blob/90597531a705ddb5fe9cd3ec222191786068dc27/lib/secure_headers/utils/cookies_config.rb#L23

"Surely you wrote a test ensuring the functionality works as expected and your cookies are extra secured" but I don't like calling people names. I like validation. I don't see this spec changing too frequently. Philosophically I think adding validation for this would be nice. It would be really nice to move away from a nested hash structure to an object but I'll settle for something basic.

oreoshake avatar Sep 29 '18 00:09 oreoshake