Add "dual_account_mode" to LDAP secrets engine allowing blue/green; also adds AD integration testing#226
Closed
andybaran wants to merge 8 commits intohashicorp:mainfrom
Closed
Add "dual_account_mode" to LDAP secrets engine allowing blue/green; also adds AD integration testing#226andybaran wants to merge 8 commits intohashicorp:mainfrom
andybaran wants to merge 8 commits intohashicorp:mainfrom
Conversation
Add support for managing two LDAP service accounts per static role with blue/green rotation and configurable grace periods, enabling zero-downtime credential rotation for enterprise LDAP environments. New fields on static roles: - dual_account_mode (bool): enables dual-account rotation - username_b / dn_b (string): second LDAP account identity - grace_period (duration): overlap period where both credentials are valid Rotation state machine: - active: one account is active, standby is idle - grace_period: both accounts' credentials returned after rotation On rotation trigger, the standby account's password is rotated, active designation flips, and the role enters grace_period. After the grace period expires, the role returns to active state. Initial setup sets both accounts' passwords without entering grace_period, so applications see a clean active state from the start. Reuses all existing infrastructure: LDAP client, WAL crash recovery, priority queue rotation, password policies, SealWrap, and HA forwarding. Includes 10 new unit tests covering validation, happy paths, managed user tracking, immutability, credential reads, state transitions, grace period expiry, skip_import_rotation, and username conflict detection.
Add integration tests that verify dual-account (blue/green) rotation works against a real Active Directory domain controller: - TestAD_SingleAccountRotation: baseline single-account rotation against AD - TestAD_DualAccountRotation: full dual-account rotation with grace period verification, LDAP bind validation for both accounts - TestAD_DualAccountRotation_SecondRotation: complete A→B→A rotation cycle with grace period expiry - TestAD_DualAccountUsernameOnlyMode: username-based (no DN) dual-account mode Tests are gated by AD_URL, AD_BIND_DN, AD_BIND_PW, AD_USER_DN, AD_DOMAIN environment variables and will be skipped when not set. Tested against Windows Server 2022 DC with AD CS (LDAPS) in AWS.
Verify that dual-account roles and Service Account Library (check-in/ check-out) correctly prevent username conflicts in both directions: Unit tests (dual_account_test.go): - TestDualAccountRole_LibrarySetConflict (5 subtests): - library_user_blocks_dual_account_username_b - library_user_blocks_dual_account_primary_username - dual_account_blocks_library_set_username_b - dual_account_blocks_library_set_primary_username - delete_dual_account_frees_both_usernames_for_library OpenLDAP Docker integration tests (openldap_integration_test.go): - TestOpenLDAP_LibrarySetConflict (3 subtests): - library_blocks_dual_account_username_b - dual_account_blocks_library_set - delete_dual_account_frees_usernames_for_library AD integration tests (ad_integration_test.go): - TestAD_LibrarySetConflict (2 subtests): - library_blocks_dual_account_username_b - dual_account_blocks_library_set All tests verify the managedUsers tracking works correctly for both username and username_b fields, ensuring no conflicts between static roles, dual-account roles, and library sets.
…racking Fix 6 issues identified in PR #2 code review: 1. path_rotate.go: Guard against nil resp dereference in manual rotation success path. Restructured conditional to only access resp.RotationTime when resp is non-nil. 2. rotation.go: Extend setCredentialsWAL struct with dual-account fields (NewPasswordB, UsernameB, DNB) so initial setup WAL records both account passwords for complete crash recovery. 3. rotation.go: Guard against zero-value GracePeriodEnd causing immediate incorrect transition. If GracePeriodEnd is zero, recompute it from LastVaultRotation + GracePeriod before checking expiry. 4. rotation.go: Fix LastRotationB asymmetry - now tracked symmetrically for both A and B account rotations. 5. rotation.go: Add WAL password reuse logic for dual-account normal rotation, matching single-account behavior. On retry after crash, reuses password from existing WAL instead of generating a new one. 6. openldap_integration_test.go: Return descriptive error when LDAP entry count validation fails instead of returning nil error.
Pre-upstream code quality improvements: 1. Add string constants for state machine values (rotationStateActive, rotationStateGracePeriod, activeAccountA, activeAccountB) to eliminate magic strings across rotation.go, path_static_roles.go, path_static_creds.go, and tests. 2. Fix LastRotationB bug: remove incorrect update in B→A rotation branch. LastRotationB should only update when account B's password is actually rotated (A→B branch and initial setup), not when switching away from B. 3. Block skip_import_rotation + dual_account_mode: dual-account mode requires initial rotation to set both passwords. API-level validation returns an error. Config-level SkipStaticRoleImportRotation is silently overridden for dual-account roles with a debug log. 4. Add WAL password reuse in initial setup path: if Vault crashes after setting account A's password but before B, recovery now reuses the WAL-stored passwords instead of generating new ones (matching the pattern in single-account and normal dual-account rotation). 5. Update help text: staticRoleHelpDescription now documents dual-account mode parameters, grace period behavior, immutability rules, and skip_import_rotation incompatibility. 6. Add grace_period sanity warning: logs a warning if grace_period > 80% of rotation_period, as this leaves very little time in active state. All unit tests (11 dual-account + existing) pass. OpenLDAP Docker integration tests (3 subtests) pass. AD integration tests (5 tests against Windows Server 2022 DC) pass.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Align map literal field spacing (dn, dn_b) to match gofmt standards. Fixes Go checks CI failure.
feat: Add dual-account (blue/green) rotation for static LDAP roles
Author
|
I have added the terraform code to create an Active Directory domain controller in AWS for Active Directory integration testing here |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Add support for managing two LDAP service accounts per static role with blue/green rotation and configurable grace periods. This enables zero-downtime credential rotation for enterprise LDAP environments, particularly in financial services where even brief authentication failures during rotation cause cascading outages.
Problem
When Vault rotates a single LDAP service account password, there is a window where applications holding the old credential fail authentication. For organizations operating thousands of services across global data centers, this creates:
Current workarounds (maintenance windows, application-level retry logic, external scripts) are operationally expensive and don't scale.
Solution
Dual-account rotation manages two LDAP accounts and alternates between them:
activewith account A designated activegrace_periodNew API Fields
Static Role (
POST /static-role/:name):dual_account_mode(bool) — enable dual-account rotationusername_b/dn_b(string) — second LDAP account identitygrace_period(duration) — overlap period where both credentials are validCredential Read (
GET /static-cred/:nameduring grace period):username/password— active account credentialsstandby_username/standby_password/standby_dn/standby_last_password— previous active accountactive_account— which account is active (aorb)rotation_state—activeorgrace_periodgrace_period_end— when the grace period expiresDesign Decisions
staticAccountstruct; reuses all existing infrastructure (LDAP client, WAL, priority queue, password policies, SealWrap, HA forwarding)dual_account_modeis immutable — Set at creation time only, prevents state corruptiongrace_periodis required — No default; operators must explicitly configure itmanagedUsers— Prevents conflicts with library check-in/check-outFiles Changed
path_static_roles.gostaticAccountstruct (12 new fields),staticFields()(4 new schemas), role CRUD with dual-account validation and immutabilitypath_static_creds.gorotation.gosetDualAccountPassword()with initial setup + normal rotation paths; extendedrotateCredential()for grace period expiry; extendedpopulateQueue()path_rotate.gobackend.goloadManagedUsersto trackUsernameBdual_account_test.goad_integration_test.goopenldap_integration_test.goTotal: +2049 / -15 lines across 8 files. Zero changes to
client.go,path_config.go,path_dynamic_*.go,path_checkout*.go, orcmd/.Testing
Unit Tests (11 new, all pass)
TestDualAccountRole_CreateValidation(7 sub-tests) — validation errorsTestDualAccountRole_HappyPath— full creation with DNTestDualAccountRole_UsernameOnly— username-only modeTestDualAccountRole_ManagedUserTracking— both usernames trackedTestDualAccountRole_ImmutableFields— cannot change mode/username_b/dn_b after creationTestDualAccountCreds_Read— active account returned as primaryTestDualAccountRotation_StateTransition— full rotation cycle (a→b→a)TestDualAccountGracePeriodExpiry— grace period → active transitionTestDualAccountRole_SkipImportRotation— skip_import_rotation respectedTestDualAccountRole_UsernameB_AlreadyManaged— username conflict detectionTestDualAccountRole_LibrarySetConflict(5 sub-tests) — dual-account ↔ library set conflict preventionAD Integration Tests (5 new, all pass)
Gated by env vars (
AD_URL,AD_BIND_DN,AD_BIND_PW,AD_USER_DN,AD_DOMAIN):TestAD_SingleAccountRotation— single-account rotation against AD with LDAP bind verificationTestAD_DualAccountRotation— dual-account initial setup, rotation, grace period, both accounts bindableTestAD_DualAccountRotation_SecondRotation— full A→B→A cycle with automatic grace period expiryTestAD_DualAccountUsernameOnlyMode— username-only (no DN) dual-account mode against ADTestAD_LibrarySetConflict(2 sub-tests) — dual-account ↔ library set conflict prevention against real ADOpenLDAP Docker Integration Tests (3 new subtests, all pass)
TestOpenLDAP_LibrarySetConflict— dual-account ↔ library set conflict prevention with real OpenLDAP Docker containerPre-existing Test Failures (not related to this PR)
Test_UpdateDNPasswordandTest_UpdateUserPassword— these fail onmainbranch; they require a live LDAP server.Test Output (not run in CI)
Unit + OpenLDAP Docker Test Output
Click to expand unit + OpenLDAP Docker test output
AD Integration Test Output (Windows Server 2022 DC with LDAPS)
Click to expand AD integration test output
How to Test
Compatibility
API Backward Compatibility & Client SDK Impact
All API changes are purely additive. Existing applications require ZERO code changes, even for single-account roles in the same environment. The new fields (
dual_account_mode,active_account,rotation_state,standby_*,grace_period_end) only appear whendual_account_mode=true.SDK-by-SDK Analysis
Each SDK was researched from source code to verify backward compatibility:
1. hvac (Python)
client.secrets.ldap.generate_static_credentials(name)callsGET /v1/{mount}/static-cred/{name}and returns raw JSONresponse['data']['password'],response['data']['username'], etc.2. Spring Vault / Spring Cloud Vault
VaultTemplate.read(path)returningVaultResponseresponse.getData().get("password")Mapnaturally accommodates extra keys3. vault-java-driver (jopenlibs)
vault.logical().read("ldap/static-cred/my-role")returningLogicalResponse4. vaultgo (mittwald)
map[string]interface{}naturally accommodates extra fieldsMapMapmap[string]interface{}Contributor Checklist
PCI Review Checklist
I have documented a clear reason for, and description of, the change I am making.
If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.
Revert plan: This change is purely additive — all new fields are optional with zero-value defaults, no schema migrations are involved, and no existing fields are modified or removed. Reverting the PR reverts all changes completely. Existing single-account static roles created while this feature is active will continue to work after revert (dual-account fields are simply ignored when absent). Dual-account roles created while the feature is active would need to be deleted and recreated as single-account roles after revert.
If applicable, I've documented the impact of any changes to security controls.
Security controls impact: Dual-account passwords are stored under the existing
staticRolePathwhich already hasSealWrapStorage: true, ensuring passwords are encrypted at rest via Vault's seal mechanism. Bothusernameandusername_bare tracked in themanagedUsersmap, preventing conflicts with the Service Account Library (check-in/check-out) system. All write operations inherit the existingForwardPerformanceStandby: trueandForwardPerformanceSecondary: truesettings on their path definitions, ensuring correct behavior in HA/DR configurations. Password generation uses the existingGeneratePasswordmethod which respects configured password policies. WAL (Write-Ahead Log) entries are extended withNewPasswordB,UsernameB, andDNBfields for crash recovery during dual-account rotation, ensuring LDAP and Vault never diverge.