@jonallured: only allow jwt tokens from trusted apps #322
Conversation
|
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. |
|
So far apps have simply used |
app/controllers/api_controller.rb
Outdated
| rescue JWT::DecodeError | ||
| payload = JWT.decode(token, secret).first | ||
| validate_payload(payload) | ||
| rescue JWT::DecodeError, NoMethodError |
There was a problem hiding this comment.
I agree - any reason you didn't like the return false unless line @cavvia?
There was a problem hiding this comment.
I agree - Rubocop was giving me an AbcSize error, though I may have fixed that with the validate_payload extraction too. So can re-introduce and see if that works.
spec/graphql/search_spec.rb
Outdated
| @@ -3,7 +3,7 @@ | |||
| describe GraphqlController, type: :controller do | |||
| let(:jwt_token) do | |||
| JWT.encode( | |||
| { aud: Rails.application.secrets.artsy_application_id }, | |||
| { aud: Rails.application.secrets.artsy_application_id, roles: 'trusted' }, | |||
There was a problem hiding this comment.
I'd like to see another test case here without the trusted role that shows it doesn't work - are you good with adding that?
|
Thank you @cavvia!! 🥇 |
|
@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 |
|
FYI, I opened an issue over there artsy/artsy-auth#6 with some thoughts. |
|
FYI, this is deployed to production. |
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