Skip to content

Conversation

@mzieniukbw
Copy link
Contributor

@mzieniukbw mzieniukbw commented Sep 21, 2025

🎟️ Tracking

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

📔 Objective

Updated change kdf component for Forced update KDF settings work.
See Jira for figma designs.

Visual changes:

  • New, more user friendly wording
  • More spacing
  • A help tooltip over "Algorithm" field now opens a popover with user friendly message with link to Bitwarden help docs.
  • Removed help tooltip over "KDF iterations".
  • When pm-23995-no-logout-on-kdf-change feature flag is enabled:
    • The 'logout' warning banner is no longer visible in the component and confirmation dialog.
    • Once updated, does not show a message about being logged out in the success toast.
  • (bug, fixed) KDF iterations, memory, parallelism valitators did not load on init (fields were not required and did not had min/max checks)

Functional changes:

  • When pm-23995-no-logout-on-kdf-change feature flag is enabled, does not log out the user.

Dependent on #16515 , which moves the change KDF component into KM ownership

📸 Screenshots

Feature flag on:

Video:

2025-10-04.12-10-41.mp4

PBKDF2:

image

Argon2Id:

image

Confirmation dialog:

image

About encryption algorithms popover:

image

Error cases:

Kdf iterations field missing:

Screenshot 2025-10-04 at 12 15 08

Kdf iterations below minimum:

image

Feature flag off:

image

Confirmation dialog:

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

@mzieniukbw mzieniukbw requested a review from a team as a code owner September 21, 2025 10:07
@mzieniukbw mzieniukbw requested review from Thomas-Avery and removed request for a team September 21, 2025 10:07
@github-actions
Copy link
Contributor

github-actions bot commented Sep 21, 2025

Logo
Checkmarx One – Scan Summary & Detailsbcfb0513-9ac3-4e6b-9f38-1612b678026f

Great job! No new security vulnerabilities introduced in this pull request

@codecov
Copy link

codecov bot commented Sep 21, 2025

Codecov Report

❌ Patch coverage is 94.33962% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.37%. Comparing base (a15d686) to head (3c6bca4).
⚠️ Report is 31 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
.../key-management/change-kdf/change-kdf.component.ts 94.73% 1 Missing and 1 partial ⚠️
...app/key-management/change-kdf/change-kdf.module.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #16516      +/-   ##
==========================================
+ Coverage   39.25%   39.37%   +0.11%     
==========================================
  Files        3467     3468       +1     
  Lines       98629    98658      +29     
  Branches    14799    14802       +3     
==========================================
+ Hits        38719    38848     +129     
+ Misses      58231    58129     -102     
- Partials     1679     1681       +2     

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Base automatically changed from km/pm-23995-move-change-kdf-to-km-team to main September 22, 2025 18:55
@mzieniukbw mzieniukbw requested a review from a team as a code owner September 22, 2025 18:55
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 Auth changes are good

# Conflicts:
#	apps/web/src/app/key-management/change-kdf/change-kdf.component.html
#	apps/web/src/app/key-management/change-kdf/change-kdf.module.ts
#	apps/web/src/locales/en/messages.json
@Patrick-Pimentel-Bitwarden
Copy link
Contributor

Not seeing any auth changes, removing myself from the review, please add me back if that's not the case.

@Patrick-Pimentel-Bitwarden Patrick-Pimentel-Bitwarden removed their request for review September 29, 2025 19:37
@mzieniukbw mzieniukbw requested a review from a team as a code owner September 29, 2025 22:03
Signed-off-by: Maciej Zieniuk <[email protected]>
@mzieniukbw mzieniukbw removed request for a team and addisonbeck September 29, 2025 22:08
Copy link
Contributor

@Thomas-Avery Thomas-Avery left a comment

Choose a reason for hiding this comment

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

Looking good, a few minor things to check out from my side.

Thomas-Avery
Thomas-Avery previously approved these changes Oct 3, 2025
Copy link
Contributor

@Thomas-Avery Thomas-Avery left a comment

Choose a reason for hiding this comment

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

LGTM

# Conflicts:
#	apps/web/src/locales/en/messages.json
Thomas-Avery
Thomas-Avery previously approved these changes Oct 6, 2025
# Conflicts:
#	apps/web/src/locales/en/messages.json
#	libs/common/src/enums/feature-flag.enum.ts
@sonarqubecloud
Copy link

@mzieniukbw mzieniukbw merged commit 8154613 into main Oct 22, 2025
52 checks passed
@mzieniukbw mzieniukbw deleted the km/pm-23995-new-change-kdf-component branch October 22, 2025 18:29
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