Skip to content

[PM-21926] Add salt to Admin Console DTOs#7231

Draft
ike-kottlowski wants to merge 5 commits intomainfrom
auth/pm-21926/add-salt-to-ac-dtos
Draft

[PM-21926] Add salt to Admin Console DTOs#7231
ike-kottlowski wants to merge 5 commits intomainfrom
auth/pm-21926/add-salt-to-ac-dtos

Conversation

@ike-kottlowski
Copy link
Contributor

@ike-kottlowski ike-kottlowski commented Mar 16, 2026

🎟️ Tracking

PM-21926

This PR depends on:
#7230

📔 Objective

Add MasterPasswordSalt to DTOs that are owned by Admin Console.

@claude
Copy link
Contributor

claude bot commented Mar 16, 2026

Claude finished @ike-kottlowski's task in 1m 32s —— View job


Code Review: PM-21926 Add salt to Admin Console DTOs

  • Gather context and review PR diff
  • Analyze changed files for correctness, security, and conventions
  • Post review findings

Overall Assessment: ✅ APPROVE

Reviewed 4 changed files adding MasterPasswordSalt to two Admin Console DTOs used in organization user password reset flows.

Summary:

  • OrganizationUserResetPasswordDetails — new MasterPasswordSalt string property added with constructor mapping from user.MasterPasswordSalt, consistent with existing nullable string properties and #nullable disable directive
  • OrganizationUserResetPasswordDetailsResponseModel — maps the new property from the data model to the API response, following the established pattern for KDF parameters
  • Unit and integration tests updated to assert on the new field

No findings. The change is minimal, follows existing patterns, and is well-covered by tests.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 16, 2026

Logo
Checkmarx One – Scan Summary & Detailsfbe0e704-12e7-440a-9ece-00ee8b8a515a


New Issues (3) Checkmarx found the following issues in this Pull Request
# Severity Issue Source File / Package Checkmarx Insight
1 HIGH Path_Traversal /src/Api/Controllers/SelfHosted/SelfHostedOrganizationLicensesController.cs: 56
detailsMethod at line 56 of /src/Api/Controllers/SelfHosted/SelfHostedOrganizationLicensesController.cs gets dynamic data from the model element. This ...
Attack Vector
2 MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1592
detailsMethod at line 1592 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ...
Attack Vector
3 MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1419
detailsMethod at line 1419 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ...
Attack Vector

Fixed Issues (1) Great job! The following issues were fixed in this Pull Request
Severity Issue Source File / Package
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 293

@codecov
Copy link

codecov bot commented Mar 16, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.81%. Comparing base (1bc5f4e) to head (2a3d37b).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7231   +/-   ##
=======================================
  Coverage   61.81%   61.81%           
=======================================
  Files        2035     2035           
  Lines       89647    89651    +4     
  Branches     7994     7994           
=======================================
+ Hits        55413    55416    +3     
- Misses      32288    32289    +1     
  Partials     1946     1946           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant