Skip to content
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

Split Organization.LimitCollectionCreationDeletion into two separate business rules #11223

Merged

Conversation

addisonbeck
Copy link
Contributor

@addisonbeck addisonbeck commented Sep 24, 2024

🎟️ Tracking

🧵 Jira Ticket: PM-10863

📚 Stacked PRs

  1. server: Split LimitCollectionCreationDeletion into two database columns server#4709
  2. server: Split Organization.LimitCollectionCreationDeletion into two separate business rules server#4730
  3. clients: Split Organization.LimitCollectionCreationDeletion into two separate business rules #11223
    ⬆️ YOU ARE HERE
  4. server: Turn on LimitCollectionCreationDeletionSplit for self host server#4808
  5. server: [PM-14821] [PM-14822] Remove LimitCollectionCreationDeletionSplit feature flag server#4809
  6. clients: [PM-14821] Remove LimitCollectionCreationDeletionSplit feature flag #11258
  7. server: Drop LimitCollectionCreationDeletion from the database server#4810

📔 Objective

🤳 The Bigger Picture

There is an option in the Admin Console that removes collection creation and
deletion controls from all organization members that are not in the Owner and
Admin roles. This PR stack splits this conjoined option into two separate
controls: one to block create operations and one to block delete
operations. This is a part of an effort to make these settings better align
with what customers expect it to do.

👉 This Pull Request 👍

In the previous PR we implemented the new functionality requested from this
effort in server. On this PR we apply those new rules to clients. This
involves adding the feature toggle to clients and implementing two new
boolean settings in the admin console behind that feature toggle.

This PR, its predecessor in server, and the next PR in the stack encompass
all of the functional changes to the system implemented by this PR stack. The
first pull request and the last two in the stack are for maintenance to
support evolutionary database design and feature toggling.

🧮 Side Effects

  • Because this work included removing LimitCollectionCreationDeletion and
    AllowAdminsAccessToAllCollections from the license file used for self
    hosted: the new controls are not disabled for self hosted organizations.

📸 Screenshots

Several recorded demos can be found in Jira

⏰ Reminders before review

  • Contributor guidelines followed
  • All formatters and local linters executed and passed
  • Written new unit and / or integration tests where applicable
  • Protected functional changes with optionality (feature flags)
  • Used internationalization (i18n) for all UI strings
  • CI builds passed
  • Communicated to DevOps any deployment requirements
  • Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

  • 👍 (:+1:) or similar for great changes
  • 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
  • ❓ (:question:) for questions
  • 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
  • 🎨 (:art:) for suggestions / improvements
  • ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
  • 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
  • ⛏ (:pick:) for minor or nitpick changes

Copy link

codecov bot commented Sep 24, 2024

Codecov Report

Attention: Patch coverage is 21.87500% with 25 lines in your changes missing coverage. Please review.

Project coverage is 33.14%. Comparing base (178a418) to head (1a7244a).
Report is 15 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...onsole/organizations/settings/account.component.ts 0.00% 21 Missing ⚠️
...n-console/models/response/organization.response.ts 0.00% 2 Missing ⚠️
...e/src/common/collections/models/collection.view.ts 0.00% 1 Missing ⚠️
...on/src/admin-console/models/domain/organization.ts 66.66% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #11223   +/-   ##
=======================================
  Coverage   33.14%   33.14%           
=======================================
  Files        2779     2779           
  Lines       86321    86342   +21     
  Branches    16449    16453    +4     
=======================================
+ Hits        28615    28622    +7     
- Misses      55432    55446   +14     
  Partials     2274     2274           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

github-actions bot commented Sep 24, 2024

Logo
Checkmarx One – Scan Summary & Detailsb39857bb-91c5-4d30-bcfc-c6c41ebae9fc

New Issues

Severity Issue Source File / Package Checkmarx Insight
MEDIUM Client_Privacy_Violation /apps/browser/src/autofill/popup/settings/autofill.component.html: 104 Attack Vector
MEDIUM Client_Privacy_Violation /apps/browser/src/autofill/popup/settings/autofill.component.html: 79 Attack Vector
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.ts: 18 Attack Vector
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.ts: 18 Attack Vector
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.ts: 18 Attack Vector
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.ts: 18 Attack Vector
MEDIUM Client_Privacy_Violation /libs/tools/send/send-ui/src/send-form/components/options/send-options.component.ts: 52 Attack Vector

Fixed Issues

Severity Issue Source File / Package
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.html: 53
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.html: 31
MEDIUM Client_Privacy_Violation /libs/tools/generator/components/src/username-generator.component.html: 3

@addisonbeck addisonbeck force-pushed the ac/addison/pm-10863/limit-collection-creation-deletion-split branch 3 times, most recently from e78c97b to c712b07 Compare October 4, 2024 23:26
@addisonbeck addisonbeck requested a review from eliykat October 7, 2024 18:51
eliykat
eliykat previously approved these changes Oct 8, 2024
@addisonbeck addisonbeck force-pushed the ac/addison/pm-10863/limit-collection-creation-deletion-split branch 6 times, most recently from f009b0a to 8f74e2c Compare October 15, 2024 22:19
@addisonbeck addisonbeck force-pushed the ac/addison/pm-10863/limit-collection-creation-deletion-split branch from 8f74e2c to 1a7244a Compare October 15, 2024 23:41
@addisonbeck addisonbeck marked this pull request as ready for review October 15, 2024 23:42
@addisonbeck addisonbeck requested review from a team as code owners October 15, 2024 23:42
Copy link
Contributor

@JaredSnider-Bitwarden JaredSnider-Bitwarden left a comment

Choose a reason for hiding this comment

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

Auth change looks good

@addisonbeck addisonbeck merged commit 073ee47 into main Oct 17, 2024
66 of 67 checks passed
@addisonbeck addisonbeck deleted the ac/addison/pm-10863/limit-collection-creation-deletion-split branch October 17, 2024 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants