-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
fix(releases): Allow project:releases scope for org releases endpoint #105130
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
fix(releases): Allow project:releases scope for org releases endpoint #105130
Conversation
src/sentry/api/bases/organization.py
Outdated
| if not has_valid_api_key and request.user and request.user.is_authenticated: | ||
| if request.access.has_scope("project:releases") or request.access.has_scope("org:ci"): | ||
| has_valid_api_key = True |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #105130 +/- ##
========================================
Coverage 80.58% 80.58%
========================================
Files 9438 9438
Lines 404429 404433 +4
Branches 25694 25694
========================================
+ Hits 325912 325917 +5
+ Misses 78048 78047 -1
Partials 469 469 |
| from sentry.hybridcloud.models.apitokenreplica import ApiTokenReplica | ||
|
|
||
| if isinstance(auth, AuthenticatedToken): | ||
| return auth.kind == "api_token" |
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.
How does this line work with the new return type?
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.
TypeGuard, as I understand it from reading the docs, just means that the function returns a bool, but if the bool is True, then the type checker should narrow the type of the first parameter to the type parameter to TypeGuard, which is AuthenticatedToken | ApiToken | ApiTokenReplica here.
This part of the docs is the most relevant for our purposes:
When a conditional statement includes a call to a user-defined type guard function, and that function returns true, the expression passed as the first positional argument to the type guard function should be assumed by a static type checker to take on the type specified in the TypeGuard return type, unless and until it is further narrowed within the conditional code block.
Some built-in type guards provide narrowing for both positive and negative tests (in both the if and else clauses). For example, consider the type guard for an expression of the form x is None. If x has a type that is a union of None and some other type, it will be narrowed to None in the positive case and the other type in the negative case. User-defined type guards apply narrowing only in the positive case (the if clause). The type is not narrowed in the negative case.
Very importantly, the second paragraph means that we are not required to return True when the type matches the type guard, as no inference is made as to the type of the first parameter when we return False. Since we can only return True when the parameter has one of the types specified in the TypeGuard, this is safe.
f3819bd to
a1cb1fe
Compare
- Add logic to check for project:releases and org:ci scopes when determining release permissions - Ensures consistency with project releases endpoint behavior - Add test to verify users with project:releases token can create releases for non-member projects - Fix test to use proper silo mode context for ApiToken creation
Co-authored-by: Sebastian Zivota <loewenheim@users.noreply.github.com>
request.access checks organization membership, and only returns scopes if the token should be able to acccess the given organization.
6daf8f1 to
149b842
Compare
|
@loewenheim would appreciate a re-review. Seer found a potential bug that I fixed with these changes. Does this look good now? |
Changes
project:releasesororg:ciscopes to create releases via the organization releases endpoint, even if they're not direct team members of the projectproject:releasestoken can create releases for projects they're not team members ofassume_test_silo_mode(SiloMode.CONTROL)) forApiTokencreationContext
Previously, the organization releases endpoint only allowed release creation if the user was either a member of the project or was using an org Auth Token with
org:ciscope. This change extends that check to also include users withproject:releasesscope, matching the behavior of the project releases endpoint and providing consistency across the API.Issues