rump icon indicating copy to clipboard operation
rump copied to clipboard

tls: add option to connect to redis with enabled tls and auth token

Open lanycrost opened this issue 3 years ago • 4 comments

  • [x] I have read the contributing guide.
  • [x] Issue: https://github.com/stickermule/rump/issues/41

Implementation

Thanks!

lanycrost avatar Jul 28 '21 13:07 lanycrost

It's break auth without TLS support.

For TLS connection, redis url should use rediss:// instead of redis://

yuyuvn avatar Oct 11 '21 04:10 yuyuvn

@yuyuvn it's not braking, I have tested it in both ways, and yes you should use rediss:// for TLS connections which is not available in current version of rump.

lanycrost avatar Oct 15 '21 11:10 lanycrost

@lanycrost Did you test connect to redis server without TLS and have auth? for example: redis://user:[email protected]:6379 strings.Contains(conn, authSeparator) -> this condition will return true, so rump will connect via tls, which is not expected.

and yes you should use rediss:// for TLS connections which is not available in current version of rump. How about implement that in your pull-request? Instead of check @ to determine user want to use TLS or not, check if prefix is rediss or not should be better.

yuyuvn avatar Oct 17 '21 05:10 yuyuvn

@yuyuvn sorry you are right, fixed.

lanycrost avatar Oct 25 '21 07:10 lanycrost