balderdashy / sails

Realtime MVC Framework for Node.js
https://sailsjs.com
MIT License
22.85k stars 1.95k forks source link

"redis" protocol not supported by @sailshq/connect-redis #6866

Open Noitidart opened 5 years ago

Noitidart commented 5 years ago

Brand new sails new myapp.

Node version: v12.10.0 Sails version (sails): 1.2.2 ORM hook version (sails-hook-orm): 2.1.1 Sockets hook version (sails-hook-sockets): 2.0.0 Organics hook version (sails-hook-organics): 0.16.0 Grunt hook version (sails-hook-grunt): UNINSTALLED Uploads hook version (sails-hook-uploads): NOT INSTALLED DB adapter & version (e.g. sails-mysql@5.55.5): sails-postgresql@1.0.2 Skipper adapter & version (e.g. skipper-s3@5.55.5): NOT INSTALLED


My related dependencies:

"@sailshq/connect-redis": "^3.2.1", "@sailshq/socket.io-redis": "^5.2.0", "connect-redis": "3.3.2", "machinepack-redis": "^2.0.2",

I have setup a managed redis cache on digitalocean. Digitalocean gave me a url to use to connect:

url: 'rediss://default:REMOVED_THIS_PASSWORD@my-new-app-sfo2-do-user-5053627-0.db.ondigitalocean.com:25061/0',

I think the solution is we need to implement this into sails - https://github.com/NodeRedis/node_redis/issues/1331#issuecomment-531601022

Somewhere around here - https://github.com/balderdashy/sails/blob/master/lib%2Fhooks%2Fsession%2Findex.js#L300

I created a stackoverflow on the topic, no help yet - https://stackoverflow.com/q/58109419/1828637

sailsbot commented 5 years ago

@Noitidart 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:

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.

Noitidart commented 5 years ago

Oh to fix, I had to add to config/session.js this: tls: {} and it passes it through!

We should document this in the docs. I will try to get to it.

johnabrams7 commented 5 years ago

@Noitidart Thanks for taking the time to explore this and provide a workaround - much appreciated! Yes, updating documentation about this could help. If you'd like, feel free to provide us a PR with the updated information and we'll have it reviewed for updating the docs. All the best!

Noitidart commented 5 years ago

Thank you I will for sure do that! :)

I plan to also write a medium article detailing exactly how to do a deployment of sails onto digital ocean droplet with managed database and managed redis cache. :)

svenemtell commented 3 years ago

I tried connecting the latest sails (1.4.3) to a Heroku Redis add-on (6.2.3 which needs TLS) by adding tls: {} to config/session.js as explained above in this issue, and in the corresponding stackoverflow question https://stackoverflow.com/questions/58109419/managed-digitalocean-redis-instance-giving-redis-aborterror. Adding tls: {} makes it possible to start sails, but no sessions are written to the database when I look at it using RedisInsight. When I use a local redis 6.2.3 without TLS everything works ok. Has anyone used sails with Heroku Redis add-on with TLS? Or does someone have tips on how I can debug the connection between sails and Heroku Redis?

eashaw commented 3 years ago

Hi @svenemtell I was looking at the Heroku Redis docs, and Heroku Redis requires you to add tls: { rejectUnauthorized: false }.

Are you able to connect if you use tls: { rejectUnauthorized: false }?

svenemtell commented 3 years ago

Thanks @eashaw!

I'm a little confused since I'm sure I've read about it in the heroku redis docs and tested it last time... ...but when I tested it again right now it works!

These are my settings (sails.config) if it can help somebody else:

session: {
  adapter: '@sailshq/connect-redis',
  host: <host>,
  port: <port>,
  pass: <pass>,
  db: 0,
  tls: { rejectUnauthorized: false },
  ...
}  
sockets: {
  adapter: '@sailshq/socket.io-redis',
  host: <host>,
  port: <port>,
  pass: <pass>,
  db: 1,
  // NOTE: The tls setting needs to be in adapterOptions, see "Adapter options" in https://github.com/balderdashy/sails-hook-sockets/blob/master/lib/configure.js#L128.
  adapterOptions: {
    tls: { rejectUnauthorized: false },
  },
  ...
}

What would be really great is to have Sails support the 'rediss' scheme in 'session.url' and 'sockets.url' so that an url on the form rediss://:<pass>@<host>:<port> (which is what you get in an environment variable on Heroku) doesn't have to be parsed like above.