-
-
Notifications
You must be signed in to change notification settings - Fork 4
feat(auth): jwt validation and auth-aware service wrapper #199
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
base: main
Are you sure you want to change the base?
Conversation
30ee1a2 to
d64060f
Compare
1a291df to
a1cbd07
Compare
505562a to
bb79ee1
Compare
| .headers | ||
| .get(header::AUTHORIZATION) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to be forward compatible with "pre-signed urls", maybe also fall back to a query string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes ideally we should support passing the auth info in the query string.
This is for pre-signed urls but also for the proxy we'll have in the monolith, it's going to require an org token which is going to be passed in the authorization header. So we need a different way to pass the auth info for objectstore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ahh i will think about this
i don't know much about our pre-signed URL plans, but i think typically they work a bit differently. some client which can auth the normal way makes a request specifically to ask for pre-signed URLs. the service verifies its auth and then generates URLs with a different signature (which is much smaller and simpler than the JWT) and returns them. the original requestor then shares the pre-signed URLs with whoever they want to share access with.
so, if we want the sentry UI to expose end-user download links, pre-signed URLs work great. if we want to support something like CLI uploads from customer CI, it might be better for us to implement auth based on DSN or an api key or something ourselves. otherwise we would need django/relay to expose an endpoint for getting objectstore urls and the cli would have to make an extra request before it can actually upload anything (which is how codecovcli worked)
no idea what this "org token" thing is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am primarily thinking of symbolicator, which is just handed a url, and it downloads that with its own internal http client, without using our client:
https://github.com/getsentry/sentry/blob/195cecd0b58f6cde6e4f4fe2bbef2900d4e0043f/src/sentry/lang/native/symbolicator.py#L188-L196
As symbolicator is an internal service, we could very well teach it to properly auth, or change it in such away that it is using our client. I don‘t really care either way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
which is much smaller and simpler than the JWT
Let's just ensure that the URL/signature can always be validated in a stateless way. For simple pre-signed URLs we could sign the path + expiry with HMAC, then base64-encode it. That would be much shorter than the entire JWT.
This form of pre-signed URLs could become a bit repetitive, though. We will have a couple of use cases that will call domain specific endpoints in Sentry API to first search for records/files, and then download some or all of them. All of them could use the same JWT - sending back a different URL+token might blow up the response size.
I'm probably overthinking that last point though, so let's follow best practices.
eae5445 to
bd52010
Compare
bd52010 to
84cc526
Compare
introduces JWT-based authorization to objectstore. implements the objectstore side of the "[RFC] Sentry Permission Tokens" proposal doc
with this in place, objectstore api endpoints should add
service: AuthAwareServiceto their handler function args and use that to access the underlying service. no other changes are neededmost of the "meat" is in
AuthContext.AuthContext::from_encoded_jwt()verifies the JWT and puts the usecase, scope, and permissions into anAuthContext.kidheader field populated. we will use thekidto load a list of possible keys (usually one, unless rotating) from our config and attempt verification with all of themexpset. we will fail validation if theexpfield's timestamp has passedAuthContext::assert_authorized(permission, path)ensures that the requested permission and path are covered by the auth contextAuthAwareService::get_object(path)callsauth_context.assert_authorized(Permission::ObjectRead, path)to make sure the current request has read permission for that specific objectAuthContextwith a scope oforg.123/proj.456covers objects whose scope isorg.123/proj.456AuthContextwith a scope of justorg.123covers all objects whose scope starts withorg.123(i.e., objects from any project in that org)there is a config setting
auth.enforceto control enforcement. when it's false, auth checks are still performed/logged but failures don't block requests.