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

[PM-11404] Account Management: Prevent a verified user from purging their vault #11411

Merged

Conversation

r-tome
Copy link
Contributor

@r-tome r-tome commented Oct 4, 2024

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-11404

📔 Objective

If the Account Deprovisioning feature flag is enabled and the user is managed by an organization then hide the Purge vault button.

Server PR: bitwarden/server#4853

📸 Screenshots

Feature flag enabled and account managed:
image

⏰ 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
Contributor

github-actions bot commented Oct 4, 2024

Logo
Checkmarx One – Scan Summary & Details4c1d8c58-958d-46b7-8d88-e5b9b1beeb82

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/tools/access-intelligence/password-health.component.html: 45 Attack Vector
MEDIUM Client_Privacy_Violation /apps/web/src/app/tools/access-intelligence/password-health.component.ts: 54 Attack Vector
MEDIUM Client_Privacy_Violation /apps/web/src/app/tools/access-intelligence/password-health.component.ts: 48 Attack Vector
MEDIUM Client_Privacy_Violation /apps/web/src/app/tools/access-intelligence/password-health.component.ts: 52 Attack Vector
MEDIUM Client_Privacy_Violation /libs/tools/send/send-ui/src/send-form/components/options/send-options.component.ts: 52 Attack Vector
MEDIUM Unpinned Actions Full Length Commit SHA /release-cli.yml: 59 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /build-desktop.yml: 288 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /version-bump.yml: 507 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /release-cli.yml: 44 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /crowdin-pull.yml: 41 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /version-bump.yml: 187 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /version-bump.yml: 499 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /build-web.yml: 297 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /deploy-web.yml: 270 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /release-desktop-beta.yml: 248 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /release-desktop-beta.yml: 668 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /release-desktop.yml: 44 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /version-bump.yml: 180 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /publish-web.yml: 44 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /release-desktop-beta.yml: 51 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /version-bump.yml: 48 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /build-desktop.yml: 1293 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /release-desktop.yml: 94 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /build-desktop.yml: 891 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /release-desktop.yml: 71 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /version-bump.yml: 359 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /release-browser.yml: 96 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /publish-desktop.yml: 196 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /version-bump.yml: 523 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /version-bump.yml: 82 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /publish-cli.yml: 141 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /release-web.yml: 57 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /release-browser.yml: 44 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /version-bump.yml: 162 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /retrieve-current-desktop-rollout.yml: 23 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /release-web.yml: 41 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /build-cli.yml: 405 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /brew-bump-desktop.yml: 26 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /build-desktop.yml: 1339 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /build-browser.yml: 408 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /version-bump.yml: 247 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /publish-cli.yml: 181 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /version-bump.yml: 194 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /version-bump.yml: 515 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /version-auto-bump.yml: 21 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /deploy-web.yml: 222 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /publish-desktop.yml: 122 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /publish-cli.yml: 104 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /crowdin-pull.yml: 35 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /staged-rollout-desktop.yml: 29 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /build-web.yml: 342 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /version-bump.yml: 201 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /publish-desktop.yml: 245 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /build-web.yml: 267 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /build-web.yml: 191 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /build-browser.yml: 367 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /version-bump.yml: 297 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /deploy-web.yml: 191 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
LOW Unpinned Actions Full Length Commit SHA /version-bump.yml: 359 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
LOW Unpinned Actions Full Length Commit SHA /version-bump.yml: 162 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
LOW Unpinned Actions Full Length Commit SHA /version-bump.yml: 247 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
LOW Unpinned Actions Full Length Commit SHA /build-desktop.yml: 1293 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
LOW Unpinned Actions Full Length Commit SHA /version-bump.yml: 194 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
LOW Unpinned Actions Full Length Commit SHA /version-bump.yml: 515 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
LOW Unpinned Actions Full Length Commit SHA /publish-cli.yml: 104 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
LOW Unpinned Actions Full Length Commit SHA /release-desktop-beta.yml: 668 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
LOW Unpinned Actions Full Length Commit SHA /version-bump.yml: 297 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
LOW Unpinned Actions Full Length Commit SHA /build-browser.yml: 367 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
LOW Unpinned Actions Full Length Commit SHA /version-bump.yml: 201 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
LOW Unpinned Actions Full Length Commit SHA /version-bump.yml: 180 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
LOW Unpinned Actions Full Length Commit SHA /version-bump.yml: 82 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
LOW Unpinned Actions Full Length Commit SHA /version-bump.yml: 187 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
LOW Unpinned Actions Full Length Commit SHA /version-bump.yml: 507 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
LOW Unpinned Actions Full Length Commit SHA /build-desktop.yml: 1339 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
LOW Unpinned Actions Full Length Commit SHA /version-bump.yml: 48 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
LOW Unpinned Actions Full Length Commit SHA /publish-desktop.yml: 122 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
LOW Unpinned Actions Full Length Commit SHA /retrieve-current-desktop-rollout.yml: 23 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
LOW Unpinned Actions Full Length Commit SHA /version-bump.yml: 499 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
LOW Unpinned Actions Full Length Commit SHA /publish-web.yml: 44 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
LOW Unpinned Actions Full Length Commit SHA /publish-desktop.yml: 245 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
LOW Unpinned Actions Full Length Commit SHA /build-cli.yml: 405 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
LOW Unpinned Actions Full Length Commit SHA /build-desktop.yml: 891 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
LOW Unpinned Actions Full Length Commit SHA /build-desktop.yml: 288 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
LOW Unpinned Actions Full Length Commit SHA /publish-cli.yml: 181 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
LOW Unpinned Actions Full Length Commit SHA /publish-cli.yml: 141 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
LOW Unpinned Actions Full Length Commit SHA /version-bump.yml: 523 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
LOW Unpinned Actions Full Length Commit SHA /build-browser.yml: 408 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
LOW Unpinned Actions Full Length Commit SHA /publish-desktop.yml: 196 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...

Fixed Issues

Severity Issue Source File / Package
MEDIUM Client_Privacy_Violation /apps/web/src/app/tools/access-intelligence/password-health.component.html: 50
MEDIUM Client_Privacy_Violation /apps/web/src/app/tools/access-intelligence/password-health.component.html: 45
MEDIUM Client_Privacy_Violation /apps/web/src/app/tools/access-intelligence/password-health.component.html: 40
MEDIUM Client_Privacy_Violation /apps/web/src/app/tools/access-intelligence/password-health.component.html: 45
MEDIUM Client_Privacy_Violation /libs/tools/generator/components/src/username-generator.component.html: 3

@r-tome r-tome marked this pull request as ready for review October 7, 2024 09:37
@r-tome r-tome requested review from a team as code owners October 7, 2024 09:37
Copy link

codecov bot commented Oct 7, 2024

Codecov Report

Attention: Patch coverage is 58.33333% with 5 lines in your changes missing coverage. Please review.

Project coverage is 33.20%. Comparing base (e8f0135) to head (342595c).
Report is 3 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...src/app/auth/settings/account/account.component.ts 44.44% 4 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11411      +/-   ##
==========================================
- Coverage   33.20%   33.20%   -0.01%     
==========================================
  Files        2782     2782              
  Lines       86474    86484      +10     
  Branches    16481    16484       +3     
==========================================
- Hits        28715    28714       -1     
- Misses      55474    55483       +9     
- Partials     2285     2287       +2     

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

coroiu
coroiu previously approved these changes Oct 7, 2024
Copy link
Contributor

@coroiu coroiu left a comment

Choose a reason for hiding this comment

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

approving platform owned code

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.

Per our call, I am requesting that we do the following:

  1. Move managedByOrganizationId out of the AccountService and into its own OrgUser domain service (name is up to you) with a reactive interface as we want to keep the base AccountInfo type applicable to all users.
  2. Rename managedByOrganizationId to a more clear name to match the server naming refactor we discussed.

@r-tome r-tome requested a review from a team as a code owner October 8, 2024 17:45
@r-tome r-tome requested a review from jrmccannon October 8, 2024 17:45
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.

LGTM! Appreciate the docs on managesActiveUser as well!

jrmccannon
jrmccannon previously approved these changes Oct 10, 2024
@r-tome r-tome changed the title [PM-11404] Account Deprovisioning: Prevent a verified user from purging their vault [PM-11404] Account Management: Prevent a verified user from purging their vault Oct 11, 2024
@r-tome r-tome requested a review from coroiu October 14, 2024 08:57
Copy link
Contributor

@coroiu coroiu left a comment

Choose a reason for hiding this comment

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

I don't think Platform owns any files in this PR anymore? You should be able to merge without my approval if you make any additional changes

@r-tome r-tome merged commit 97e195c into main Oct 17, 2024
65 of 66 checks passed
@r-tome r-tome deleted the ac/pm-11404/prevent-a-verified-user-from-purging-their-vault branch October 17, 2024 15:06
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.

4 participants