Skip to content

Conversation

@iamrajjoshi
Copy link
Collaborator

@iamrajjoshi iamrajjoshi commented Jul 22, 2024

Created migration to add bit flag to Organization. Will need to follow this up with a migration to add the flag so Hybrid Cloud services can handle syncing.

Glossary
Data secrecy mode: Disallows any kind of superuser access into an organization

Enable/Disable Data secrecy mode: Persistently enable/disable data secrecy for an organization

Waive Data secrecy mode: Temporarily disable data secrecy for an organizations
Reinstate Data secrecy mode: Re-enable data secrecy after a temporary waiver

This flag handles the enable/disable function.

spec

@iamrajjoshi iamrajjoshi requested a review from a team July 22, 2024 23:34
@iamrajjoshi iamrajjoshi self-assigned this Jul 22, 2024
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jul 22, 2024
with self.options(
{"system.url-prefix": "http://example.com"}
), self.tasks(), outbox_runner():
with (
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of these diffs are auto-formatting from IDE

).values_list("organization_id", "user_id")

for (org_id, user_id) in queried_owner_ids:
for org_id, user_id in queried_owner_ids:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto formatting from IDE

disable_member_project_creation: bool

# Disable superuser access to an organization
disable_superuser_access: bool
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a negative since the default is False and we do not want to have data secrecy enabled by default.

@iamrajjoshi iamrajjoshi marked this pull request as ready for review July 22, 2024 23:36
@iamrajjoshi iamrajjoshi requested review from a team as code owners July 22, 2024 23:36
@github-actions
Copy link
Contributor

This PR has a migration; here is the generated SQL for src/sentry/migrations/0744_add_disable_superuser_access_bitflag.py ()

--
-- Alter field flags on organization
--
-- (no-op)

Copy link
Member

@cathteng cathteng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the wording for this project is difficult for sure

"require_2fa": org.flags.require_2fa.is_set,
"codecov_access": org.flags.codecov_access.is_set,
"disable_member_project_creation": org.flags.disable_member_project_creation.is_set,
"disable_superuser_access": org.flags.disable_superuser_access.is_set,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i've had trouble with options using the negative before... i would suggest calling this allow_superuser_access

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a negative since the default is False and we do not want to have data secrecy enabled by default.
If i make this allow, I will need to write a script to make everyone's allow_data_secrecy True by default and we would have to do this for every new org creation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about prevent_superuser_access? That would avoid needing to use negation 🤷

),
"avatar": avatar,
"allowMemberProjectCreation": not obj.flags.disable_member_project_creation,
"allowSuperuserAccess": not obj.flags.disable_superuser_access,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would customers understand what this means? can we specify that this means sentry employees?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can add a description in the frontend for this

@iamrajjoshi iamrajjoshi changed the title feat(data-secrecy): Created Migration to Add disable_superuser_access Bit Flag feat(data-secrecy): Migration to Add disable_superuser_access Bit Flag Jul 23, 2024
@codecov
Copy link

codecov bot commented Jul 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.15%. Comparing base (26e6464) to head (92a3146).
Report is 9 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #74700   +/-   ##
=======================================
  Coverage   78.14%   78.15%           
=======================================
  Files        6730     6731    +1     
  Lines      300110   300162   +52     
  Branches    51623    51627    +4     
=======================================
+ Hits       234529   234582   +53     
+ Misses      59256    59254    -2     
- Partials     6325     6326    +1     
Files Coverage Δ
src/sentry/api/endpoints/organization_details.py 88.55% <100.00%> (+0.07%) ⬆️
src/sentry/api/serializers/models/organization.py 94.50% <ø> (ø)
src/sentry/models/organization.py 87.35% <100.00%> (+0.04%) ⬆️

... and 11 files with indirect coverage changes

@iamrajjoshi iamrajjoshi force-pushed the raj/ds/bitflag-mig-1 branch from c23631e to 924557b Compare July 23, 2024 17:43
@github-actions
Copy link
Contributor

This PR has a migration; here is the generated SQL for src/sentry/migrations/0745_add_prevent_superuser_access_bitflag.py ()

--
-- Alter field flags on organization
--
-- (no-op)

@iamrajjoshi iamrajjoshi changed the title feat(data-secrecy): Migration to Add disable_superuser_access Bit Flag feat(data-secrecy): Migration to Add prevent_superuser_access Bit Flag Jul 23, 2024
Copy link
Member

@wedamija wedamija left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

migration lgtm, bitflag changes are a no-op

@iamrajjoshi iamrajjoshi merged commit 568c329 into master Jul 23, 2024
@iamrajjoshi iamrajjoshi deleted the raj/ds/bitflag-mig-1 branch July 23, 2024 20:23
iamrajjoshi added a commit that referenced this pull request Jul 25, 2024
…es (#74891)

I recently merged a
[migration](#74700) to add
`prevent_superuser_access` to the Organization bitflag and now need to
add it to Hybrid Cloud services to sync since I need it to exist on
`RpcOrganization`.

In this PR, I also add the `disable_member_project_creation` flag since
it existed above the new flag I created.

My flag is still not used in our logic, but the
`disable_member_project_creation` is currently being used, so we need to
make sure this pr doesn't mess around with its value.
Christinarlong pushed a commit that referenced this pull request Jul 26, 2024
…es (#74891)

I recently merged a
[migration](#74700) to add
`prevent_superuser_access` to the Organization bitflag and now need to
add it to Hybrid Cloud services to sync since I need it to exist on
`RpcOrganization`.

In this PR, I also add the `disable_member_project_creation` flag since
it existed above the new flag I created.

My flag is still not used in our logic, but the
`disable_member_project_creation` is currently being used, so we need to
make sure this pr doesn't mess around with its value.
@github-actions github-actions bot locked and limited conversation to collaborators Aug 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants