SailsJS 0.12.14 Cookie SameSite Update Needed

[crh3675]

Node version: 10.16.3 Sails version (sails): 0.12.4

---

Need an update to the 0.12.4 branch for sameSite Cookie option. Currently get error:

TypeError: option sameSite = None is invalid ./node_modules/sails/node_modules/express-session/node_modules/cookie/index.js:174:15)

We cannot upgrade to Sails v1. Please assist.

[sailsbot]

@crh3675 Thanks for posting! We'll take a look as soon as possible.

In the mean time, there are a few ways you can help speed things along:

• look for a workaround. (Even if it's just temporary, sharing your solution can save someone else a lot of time and effort.)
• tell us why this issue is important to you and your team. What are you trying to accomplish? (Submissions with a little bit of human context tend to be easier to understand and faster to resolve.)
• make sure you've provided clear instructions on how to reproduce the bug from a clean install.
• double-check that you've provided all of the requested version and dependency information. (Some of this info might seem irrelevant at first, like which database adapter you're using, but we ask that you include it anyway. Oftentimes an issue is caused by a confluence of unexpected factors, and it can save everybody a ton of time to know all the details up front.)
• read the code of conduct.
• if appropriate, ask your business to sponsor your issue. (Open source is our passion, and our core maintainers volunteer many of their nights and weekends working on Sails. But you only get so many nights and weekends in life, and stuff gets done a lot faster when you can work on it during normal daylight hours.)
• let us know if you are using a 3rd party plugin; whether that's a database adapter, a non-standard view engine, or any other dependency maintained by someone other than our core team. (Besides the name of the 3rd party package, it helps to include the exact version you're using. If you're unsure, check out this list of all the core packages we maintain.)

  ---

Please remember: never post in a public forum if you believe you've found a genuine security vulnerability. Instead, disclose it responsibly.

For help with questions about Sails, click here.

[whichking]

Hi, @crh3675—

Thanks for letting us know about the error you're getting! Would you mind sharing more about the configuration you're trying to use? We'd love to take a closer look.

[crh3675]

I think I found a way around the issue using "Lax". Not really sure how to test (until/unless it breaks). https://blog.chromium.org/2019/10/developers-get-ready-for-new.html

[whichking]

Glad you found a workaround, @crh3675!

[dcolley]

This is still a problem in sails@1.2.3, which bundles "cookie": "0.3.1" I understand sameSite=None was implemented in cookie@0.4.0 (which is now bundled with express@4.17.1) I added the following to node_modules/sails/node_modules/cookie/index.js:174, and it worked for me:

      case 'none':
        str += '; SameSite=None';
        break;

Now, how am I going to remember that when I deploy to production... {scratching chin}

[dcolley]

Just an FYI, between last night and tonight I took an update to Chrome {Version 80.0.3987.122 (Official Build) (64-bit)} and then it started rejecting SameSite=Lax (this is the default if SameSite is not specified) for my API...

[dcolley]

I think this article about peerDependencies is a possible solution.

https://medium.com/angular-in-depth/npm-peer-dependencies-f843f3ac4e7f

Move

  "dependencies": {
        "cookie": "0.3.1",
  }

down to

  "peerDependencies": {
        "cookie": "^0.4.0",
  }

this way we don't get a duplicate module if a compatible cookie version is already installed?

[NachtRitter]

@dcolley peerDependency isn't a good solution, especially if package is used inside of the code. If compatible versions are installed they wouldn't be duplicated. npm flattens the dependency tree on the fly during the installation. You may read about it here: https://docs.npmjs.com/cli/dedupe

upd If you run command npm list in any of your projects you will see dependencies tree and which dependencies were moved to the up.

[dcolley]

@NachtRitter re. "if package is used inside of the code" - it does not make sense to have a package in package.json and then not use it..?

The bigger issue here is that sails specifies "0.3.1" exactly, and hence node won't use 0.4.0 even though it's available in the parent project. And the installation process, sails new my-app does not cater for pre-installation of modules into target directory - it fails if the my-app directory exists...

[peter-gyulavari]

Is there a good solution to this? We have a sails API in production, and we would like to ship new features with the next update but chrome already sets SameSite to Lax in the test environment and users can't login to the website. Should we just change "node_modules/sails/node_modules/cookie/index.js" ?

[dcolley]

One way would be to amend the code in node_modules (you need to remember to do this each time you deploy...

Another way to do this is to

• fork https://github.com/balderdashy/sails
• update [forked] package.json to use
  • "cookie": "0.4.0",
  • "express": "4.17.1",
  • commit your changes to the fork
• update your project package.json to point to your own fork
• run npm install

After amending the you should be able to set a cookie in your controller

    fn: async function (inputs, exits, env) {

        let {req, res} = env
        let myvalue = 1
        // ... blah ...
        res.cookie('mycookie', myvalue, {
            // domain: config.domain,
            path: '/admin',
            httpOnly: true,
            // sameSite: true,
            sameSite: "none",
            secure: true,
        }); 
        res.redirect(installUrl);
    }

[peter-gyulavari]

I see, thanks for the help. Other possible solution we thought of is to create a DNS A record pointing to our "real API", so browsers might not think it's a cross site request.

according to web.dev: "If the user is on www.web.dev and requests an image from static.web.dev then that is a same-site request."

so if we do: frontend: www.mysite.com backend: api.mysite.com (which points to our real API's ip address) it might work.

I'm not really sure we can "cheat" browsers like this though.

[kriss1897]

@dcolley @ewol123 You can directly use cookie@0.4.x in your project to serialize cookies, and set them using res.set('set-cookie', [serializedCookies]). If you want to sign the cookies, then you can use cookie-signature as the project dependency as well.

[LouAdrien]

Is there anyway to just override the default behaviour through a hook without redoing the cookie logic? Declared http middleware in config/http still seem to be called before any Set cookie headers are set on the requests. What the right way to create a callback that'll modify the headers (to add the proper same site attribute?

I think this should be a highly critical issue as this will break existing project in production, as many apps will probably count on cookies being accepted from 3rd parties.

[kriss1897]

@LouAdrien The only way is to use the latest cookie module. The session middleware in HTTP headers is the one that sets session cookies. Probably you can override the handling of res.cookie in an HTTP middleware which is called before the session middleware

[LouAdrien]

I did not want to rely on have to mess with sails internal dependencies, so I ended implementing a hook that uses the onHeaders callback to do what I need. I hardcoded a few thing for my use case but you can modify it to fit your own. Make sure to register it before the session hook.

order: [
      'customCookieSet',
      'cookieParser',
      'session',
      'bodyParser',
      'compress',
      'poweredBy',
      'router',
      'www',
      'favicon',
    ],

customCookieSet : function (req,res,next) {
        // Using express onHeader callback to make sure we modify the cookie just before they are sent. Carefull this requires that this hook be registered
        // BEFORE the session hook (or any other hook working with cookies) as cookies are also set with onHeaders and onHeaders callback are called in reverse order of subscription
        // https://www.npmjs.com/package/on-headers
        onHeaders(res, function(){
          // There can be multiple set-cookie instructions
          var currentCookieSets = res.get("set-cookie");
          // get always returns an array, contrary to what the documentation says.
          if(currentCookieSets && currentCookieSets.length){
            for (var i = 0; i < currentCookieSets.length; i++) {
              var currentCookieSet = currentCookieSets[i];
              if( currentCookieSet.indexOf("SameSite") === -1){
                  currentCookieSet += '; SameSite=None';
              }else {
                currentCookieSet = currentCookieSet.replace("SameSite=Strict","SameSite=None");
                currentCookieSet = currentCookieSet.replace("SameSite=Lax","SameSite=None");
              }

              if( currentCookieSet.indexOf("Secure") === -1 ) {
                currentCookieSet += '; Secure';
              }
              currentCookieSets[i] = currentCookieSet;
            }
            res.setHeader('set-cookie',currentCookieSets);
          }
          // console.log("onHeader() --> ", res.get("set-cookie"));
        })
        return next();
    },

[crh3675]

@LouAdrien Is that proposed solution for 1.x or 0.12?

[LouAdrien]

1.x

[crh3675]

For our needs, I am testing with 0.12 using a slight modification and it seems to work as well:

    cookieSameSiteHandler: function(req, res, next) {
      onHeaders(res, function() {
        const currentCookieSets = res.get('set-cookie');

        if (currentCookieSets && currentCookieSets.length) {
          for (let i = 0; i < currentCookieSets.length; i++) {
            let currentCookieSet = currentCookieSets[i];
            if (sails.config.session.cookie.secure) {
              currentCookieSet += '; Secure; SameSite=None';
            } else {
              currentCookieSet += '; SameSite=Strict';
            }
            currentCookieSets[i] = currentCookieSet;
          }
          res.setHeader('set-cookie', currentCookieSets);
        }
      });
      return next();
    }

[crh3675]

I added this code to our config/http.js using the same pattern that @LouAdrien recommended. You need to add a require at the top of that file: const onHeaders = require('on-headers')

[Muhammadinaam]

I am following @LouAdrien or @crh3675 solution. But for me, res.get('set-cookie') returns undefined. How can I fix it?

[Muhammadinaam]

This issue is open for 7 months... 😲 😲

[eashaw]

Hi @Muhammadinaam, whose solution are you trying to use and what version of Sails is your app running?

[crh3675]

@Muhammadinaam, if set-cookie is undefined, are you sure you have sessions enabled and you are passing a cookie to the browser?

[Muhammadinaam]

I am following @LouAdrien or @crh3675 solution. But for me, res.get('set-cookie') returns undefined.

sails version is 1.0.2. I am working on an already developed project. @LouAdrien's solution is actually adding a middleware. But in my project, previous developers have commented out middleware code in http.js file.

// order: [ // 'cookieParser', // 'session', // 'bodyParser', // 'compress', // 'poweredBy', // 'router', // 'www', // 'favicon', // ],

So I tried to implement @LouAdrien solution using hooks. But for me, res.get('set-cookie') returns undefined

[dflupu]

The following code works for sails < 0.12. Note that it needs to be added AFTER the session middleware.

    cookieAddSameSite: function(req, res, next) {
      res.on('header', function() {
        var currentCookieSets = res.get('set-cookie');

        if (typeof(currentCookieSets) === 'undefined') {
          return;
        }

        if (typeof(currentCookieSets) === 'string') {
          currentCookieSets = [currentCookieSets];
        }

        if (currentCookieSets && currentCookieSets.length) {
          for (var i = 0; i < currentCookieSets.length; i++) {
            var currentCookieSet = currentCookieSets[i];

            if (sails.config.session.cookie.secure) {
              currentCookieSets[i] = currentCookieSet + '; SameSite=None; Secure';
            } else {
              currentCookieSets[i] = currentCookieSet + '; SameSite=Strict';
            }
          }

          res.removeHeader('set-cookie');
          res.setHeader('set-cookie', currentCookieSets);
        }
      });

      return next();
    }

[eashaw]

@crh3675 @dcolley @ewol123 @LouAdrien @dflupu @Muhammadinaam This issue is fixed as of Sails v1.4.0