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

lookupd: support filtering response based on auth state #370

Merged
merged 3 commits into from
Jun 17, 2014

Conversation

jehiah
Copy link
Member

@jehiah jehiah commented Jun 12, 2014

This adds lookupd support to filter responses to just the topics/channels a client is allowed to connect to.

(also, this fixes a bug with specifying more than one lookupd)

WIP cc: @mreiferson

@jehiah
Copy link
Member Author

jehiah commented Jun 12, 2014

So i ran into a challenge with the way i was thinking about things. I wanted to get to a point where clients would use the same secret in querying the lookupd and would only get a filtered response back about the topics they have permission to subscribe to. This is important for running nsqd as a public service where you don't want to disclose all possible nsqd endpoints, only the ones that already have a topic.

This turns out to be a challenge because on the nsqlookupd http side it would be both handling requests from nsqd (unauthenticated) and requests from clients (authenticated). The split makes it difficult to enforce 100% authentication there because we don't have a way to configure and manage authentication for nsqd's themselves.

nsqd's uses the /channels api endpoint and clients use the /lookup endpoint, so we could have different auth rules for those two, but that would imply we require people to expose nsqlookupd behind a service that only allows the single endpoint through, and if we do that we might as well just to auth filtering there (though that does change options from just fronting with nginx, vs something else).

@mreiferson I'm inclined to land this refactoring and continue work elsewhere, but i want to sleep on it. thoughts?

@mreiferson
Copy link
Member

For one, this refactoring looks good.

Regarding your comments, I suggest that we keep this first pass simple and not deal with nsqlookupd at all. I'm pretty sure in your use case you don't need to expose nsqlookupd, right? The questions you raise are complicated and I think we need time (and production experience) to come up with good solutions.

To make things more complicated, as you know, I've been experimenting with a path forward where nsqlookupd might become irrelevant, so I'm hesitant to spend too much time worrying about it right now if it's not absolutely necessary.

@jehiah
Copy link
Member Author

jehiah commented Jun 14, 2014

squashed down, but i'd like to bump go-nsq before merging to pickup nsqio/go-nsq#43

@jehiah
Copy link
Member Author

jehiah commented Jun 17, 2014

updated go-nsq. RFM @mreiferson

mreiferson added a commit that referenced this pull request Jun 17, 2014
lookupd: support filtering response based on auth state
@mreiferson mreiferson merged commit 012c6b7 into nsqio:master Jun 17, 2014
@jehiah jehiah deleted the lookup_authd_filter_370 branch June 27, 2014 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants