bitly / oauth2_proxy

A reverse proxy that provides authentication with Google, Github or other provider
MIT License
5.1k stars 1.21k forks source link

Issues with multiple oauth proxies with different group restrictions sharing cookies. #174

Open jburnham opened 8 years ago

jburnham commented 8 years ago

Once someone successfully authenticates with any oauth2_proxy, theoretically they could successfully log into a different proxy instance that does restricts users to a group that user is not a part of because it uses the same cookie name. This assumes the cookie is valid and doesn't need to be refreshed, therefore the second proxy would forego the need to check group membership. The workaround would be to use different cookie names for group restricted services but that seems hacky. Ideally the same oauth2_proxy cookie payload should be able to be used across your infrastructure, even services that require specific groups. If you try to hit a service where you are not a member of that group you should get a 403 ( I suppose this might cause the cookie to be invalidated, but I don't see a problem with that )

I'm not sure what would be the best solution here. #28 mentioned the ability to pass a header to backend services that shows the list of groups a member is a part of. The main issue that I can think of is how to store the list of groups a member is a part of in a secure, immutable way so that oauth2_proxy can easily check group membership on every request and also pass the group membership as a header.

Other ideas?

jehiah commented 8 years ago

thanks @jburnham I think i like the second path you mentioned.

If we change the group restriction to be per-request (as opposed to the access token re-validation which is periodic) we can keep an in-memory LRU store per oauth2_proxy for the user->group mapping from #127. That would allow multiple oauth2_proxy to share authentication, but have different group restrictions.

One nice to have w/ that would be that the group restriction should probably return 403's that doesn't sign the user out.

jburnham commented 8 years ago

Would we still want a ttl on the cache entry to force group re-verification on a user defined ( or hard coded ) interval?

I was definitely going to look into making group 403's not force the cookie to be deleted since that is much friendlier.

jburnham commented 8 years ago

Can you elaborate on your aversion to storing groups in the cookie? This would allow stateless group authentication as long as the cookie can be validated. This would then allow all instances of oauth2_proxy to send an X-Forwarded-Groups header that downstream services can use for additional checking. Services like GodAuth and variants used by Flickr and Etsy store groups in the cookie. See https://github.com/exflickr/GodAuth/blob/master/mod_perl/GodAuth.pm#L154

This approach could also allow us to add support for oauth2_proxy to always populate groups information but not restrict access to any particular group and just pass the group list down.

jehiah commented 8 years ago

General aversion to putting groups in cookie is to avoid cookie bloat; I also don't think there has been a need to store provider specific info (like groups) in the cookie, so if the google auth provider can leverage the access token and keep it's own state that also seems preferable so that things are more consistent across providers.

jburnham commented 8 years ago

My concern is when using many different instances of oauth2_proxy across your infrastructure, they are each keeping a copy of the provider data and they could each be in a different state. If you instead use some shared state like redis/memcached this adds additional infrastructure that you have to maintain and grant access to for all your auth boxes.

I feel like storing this data in the cookie is closer to the approach of JSON Web Tokens where the client provides everything needed to be granted access to backend resources with the assurance that it hasn't been modified. This makes everything stateless and each provider can choose what it wants to store and send to downstream servers including not using that provider-specific store at all. All of this that is beyond storing the user or email in the session state should be opt-in.

Here's an example scenario I'm picturing.

Multiple oauth2 proxies 1 protecting monitoring (group restriction to operations) 1 protecting build server (no group restriction, but sends group information in X-Forwarded-Groups header) 1 protecting marketing server (group restriction to marketing team and operations)

I can hit any one of these servers as an operations person and I get validated and receive a cookie that includes my groups. I can then log into all the rest without any additional API calls to Google. With the cache per proxy. I have to re-query Google for each other instance to cache the groups data. Also group change recognition can happen just as quickly as deleting the cookie and re-logging in.

I'm happy to go the cache per proxy approach but with modern browsers I feel like cookie bloat has less impact than it once did and the benefits could outweigh the concerns. If we add the ability for providers to add custom data to the session state, I feel like we could add a lot more flexibility to oauth2_proxy and allow more of a separation between authentication and authorization. I'll repeat my example for the Github provider storing your org and team memberships so that downstream services can act differently depending on what they see.

Sorry for the big wall of text :smile:

mrwacky42 commented 8 years ago

Why not just use a different cookie name on different proxy instances? Or are they different instances in say, the same ELB?

jburnham commented 8 years ago

I do different cookie names on different instances now, but with that approach, people have to login to each one. I'd prefer not having people to have to do that. With the group information fetched once, each subsequent request to another instance just checks the cookie for a group which is allowed.

mrwacky42 commented 8 years ago

Point taken. I was assuming disjoint groups.