fix: Restrict deactivated enterprise user access #910
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #910 +/- ##
=======================================
Coverage 96.25% 96.25%
=======================================
Files 826 826
Lines 19048 19051 +3
=======================================
+ Hits 18334 18337 +3
Misses 714 714
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. 📢 Thoughts on this report? Let us know! |
suejung-sentry
changed the title
| # per product spec, if guestAccess is off for the environment, the current enterpriseUser | ||
| # must be "activated" in the given target owner (e.g., "codecov" org) in order to see things | ||
| target = await get_owner(service, username) | ||
| if user.ownerid not in target.plan_activated_users: |
There was a problem hiding this comment.
you can use this helper function: current_user_part_of_org
There was a problem hiding this comment.
Oh nice thanks!
Looking over that helper - Are we diligent about updating an owner (say suejung-sentry)'s organizations when we adjust the other source of truth on the organization (say codecov)'s plan_activated_users?
From what I could tell the term activated relates to whether the ownerid in the plan_activated_users list, and I haven't dug yet into what the owner.organizations is, but I thought it had to do with accounts?
This stuff makes it seem like they are 2 separate concepts, so using the helper will result in a different effect
There was a problem hiding this comment.
sounds like the helper may not be equivalent or what's desired here in this chat
| @@ -50,11 +52,17 @@ def resolve_owner(_, info, username): | |||
| if not user or not user.is_authenticated: | |||
| raise UnauthorizedGuestAccess() | |||
|
|
|||
| return get_owner(service, username) | |||
| # per product spec, if guestAccess is off for the environment, the current enterpriseUser | |||
There was a problem hiding this comment.
nit: I think the comment states something that can be inferred from the code / PR if needed so not sure if there's much value in having it tbh
Restrict what deactivated Enterprise users can see when the environment has guest access turned off
Closes codecov/engineering-team#1859