search

artsy / bearden

A simple database of organizations
MIT License
3 stars 8 forks source link

@jonallured: only allow jwt tokens from trusted apps #322

Closed cavvia closed 7 years ago

cavvia commented 7 years ago

It turns out gravity has an Auth API endpoint that deals out JWT tokens to untrusted apps (like force) with a short expiry. This can theoretically be used to produce valid JWT tokens for Bearden knowing only the Bearden application ID, which is a security problem.

While the Gravity API endpoint looks like a security hole, it can be useful for some apps who wish to allow access to a set of users from an untrusted app (like Impulse, which wants to allow some Force users access to their conversations inbox data, as Force is an untrusted app in this scheme).

We've decided it's up to the destination app to validate app roles in its auth if it wishes to restrict access to trusted apps. This is the case with Bearden, so I've added the additional checks in our auth method to ensure that the app which requested the token is trusted. We use 'roles' embedded in the JWT to determine whether an app is trusted (see gravity code).

cc @joeyAghion @mzikherman

dblock commented 7 years ago

This highlights the need for a shared middleware across Artsy for these! cc: @orta

I think the Gravity API is OK, it hands a token for a given app that can then be used to identify the user from gravity. That's authentication. Authorization is definitely the burden of this app and it should reject users it doesn't want.

I did raise this in #security on Slack btw. And please note that bearden is public, so this discussion is public as well.

joeyAghion commented 7 years ago

So far apps have simply used jwt rather than shared middleware. Maybe it can be an extension of https://github.com/artsy/artsy-auth, which is session-oriented and has hooks for asserting user roles explicitly, but could be extended to encapsulate the JWT validation and expose the embedded app and role data.

jonallured commented 7 years ago

Thank you @cavvia!! 🥇

cavvia commented 7 years ago

@dblock @joeyAghion Agreed a shared middleware would be good. It would need to support these trusted app tokens. I think we can probably get it into artsy-auth, even if that is more focused on client devs.

jonallured commented 7 years ago

FYI, I opened an issue over there artsy/artsy-auth#6 with some thoughts.

jonallured commented 7 years ago

FYI, this is deployed to production.