Skip to content

fix: update shadow credential detection to exclude forcechangepassword rights#332

Merged
l50 merged 1 commit into
mainfrom
refactor/shadow-creds-exclude-forcechangepassword
May 18, 2026
Merged

fix: update shadow credential detection to exclude forcechangepassword rights#332
l50 merged 1 commit into
mainfrom
refactor/shadow-creds-exclude-forcechangepassword

Conversation

@l50
Copy link
Copy Markdown
Contributor

@l50 l50 commented May 18, 2026

Key Changes:

  • Corrects shadow credential logic to exclude ForceChangePassword as a candidate
  • Updates documentation to clarify exclusion and rationale for this right
  • Refines unit tests to match updated candidate logic

Changed:

  • Candidate logic for shadow credentials now excludes forcechangepassword and
    acl_forcechangepassword to prevent false positives, reflecting that this
    right only allows password reset, not the required property write
  • Documentation comments enhanced to clarify that ForceChangePassword is routed
    to password reset abuse, not shadow credential abuse
  • Unit tests updated to assert that ForceChangePassword and its variants are not
    considered valid shadow credential candidates, and expanded with relevant
    negative cases

Removed:

  • Inclusion of forcechangepassword and acl_forcechangepassword from the set
    of shadow credential candidate types in logic and tests

…ection

**Changed:**

- Updated documentation and logic to clarify that `forcechangepassword` and its
  ACL-prefixed form are not valid for shadow credential abuse, as they only
  grant password reset, not property writes for msDS-KeyCredentialLink
- Modified `is_shadow_cred_candidate` to remove checks for
  `forcechangepassword` and `acl_forcechangepassword`
- Updated tests to remove assertions for `forcechangepassword` and add negative
  tests confirming it is not accepted as a shadow credential candidate
@codecov
Copy link
Copy Markdown

codecov Bot commented May 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.03%. Comparing base (383db3a) to head (8f6e97e).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #332      +/-   ##
==========================================
- Coverage   80.03%   80.03%   -0.01%     
==========================================
  Files         433      433              
  Lines      125578   125577       -1     
==========================================
- Hits       100502   100501       -1     
  Misses      25076    25076              
Files with missing lines Coverage Δ
.../src/orchestrator/automation/shadow_credentials.rs 72.90% <100.00%> (-0.06%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@l50 l50 merged commit db1c4ee into main May 18, 2026
12 checks passed
@l50 l50 deleted the refactor/shadow-creds-exclude-forcechangepassword branch May 18, 2026 05:51
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.

1 participant