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

AUTH Support #21

Closed
jehiah opened this issue Jul 2, 2014 · 2 comments
Closed

AUTH Support #21

jehiah opened this issue Jul 2, 2014 · 2 comments

Comments

@jehiah
Copy link
Contributor

jehiah commented Jul 2, 2014

@dlecocq I did an audit to understand how far away nsq-py is in terms of supporting nsq AUTH (added in nsqio/nsq#356 and supported in other client libraries nsqio/pynsq#72 nsqio/pynsq#77 and nsqio/go-nsq#35 nsqio/go-nsq#43). I wanted to drop notes here for you as to what i found.

One item i noticed is that the identify response from nsq doesn't currently trigger snappy/tls socket upgrades. That needs to happen before the AUTH command is sent across. (auth will almost always require TLS). Because there are additional round trips involved in connection upgrade and AUTH there are some changes needed to adjust when that connection is "ready" and gets it's SUB sent across and added to the client/reader and marked as available.

The second part is that we adjusted clients so that they could be given a fully qualified path to lookupd with existing url parameters. This would allow lookupd to be specified as an SSL endpoint on an existing api platform, and would allow authentication information for lookupd to be embedded in the URL and the client would just add the topic parameter to any existing parameters.

@dlecocq
Copy link
Owner

dlecocq commented Jul 2, 2014

You are right -- currently snappy, deflate and TLS are unsupported, but they are specifically called out when initializing a connection. I had a branch where I was hacking on TLS support, but was finding it difficult to support TLS from the mock server internal to the testing infrastructure. Some of the tests are already integration tests and so there's no reason for the TLS tests to not be -- I just haven't found the time.

I had been content putting it off, but with AUTH on the horizon we'll be prioritizing this work accordingly.

@jehiah
Copy link
Contributor Author

jehiah commented Jul 24, 2014

closed by #25

@jehiah jehiah closed this as completed Jul 24, 2014
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

No branches or pull requests

2 participants