Password hash redaction policy for historical user rows

uwe.admin commented

2026-05-06 12:58:30 +02:00

Owner

Context

authkit#14 PR 4 makes the users table temporal (entity_id + valid_from + valid_until). Per owner directive, password_hash rides the temporal row — every password change creates a historical row with the prior hash preserved, plus a new live row carrying the new hash.

This raises a security question: historical password hashes should not stay readable indefinitely. They give an attacker who breaches the DB everything they need to brute-force every password a user has ever used.

What needs to ship

A redaction policy applied at password-change time. Owner suggested: blank the actual hash on the historical row, but keep the row so the change-history (timestamps, who changed it) remains auditable.

Concrete shape (proposed):

When TemporalRepository<UserDto>::save decides this is an UPDATE that closes the prior live row, run an extra step: on the historical clone, set password_hash = NULL (or to a fixed sentinel like redacted).
The live row keeps the new hash as usual.
valid_from / valid_until / username / role / tls_cert_dn stay on the historical row — only password_hash is redacted.
Net effect: an audit query SELECT username, valid_from, valid_until FROM users WHERE entity_id = X ORDER BY valid_from shows the full change history (when did this user exist, when was their password rotated, when were they deactivated), but the hashes themselves only exist on the live row.

Open questions

Where does the redaction live? Three options:
- On TemporalRepository<UserDto> directly via a specialisation/hook — clean but couples the generic decorator to a user-specific concern.
- On a new RedactedFieldRepository<TDto> decorator that takes a list of "redact on close" field names — generic, composes cleanly above TemporalRepository.
- On ConcreteUserRepository::save — hand-rolled, not reusable.
What about tls_cert_dn? Same security argument — a leaked old cert DN is less catastrophic than a hash, but still a privilege-escalation footprint. Worth thinking about whether the same policy applies.
Backfill? When this lands, existing historical rows from the migration of the non-temporal table will already have password_hash populated. Does the migration redact them retroactively, or do we accept the legacy hashes (acknowledging they were already in plaintext or bcrypt format on the live row before the migration anyway)?

Effort

Probably Small for option 2 (RedactedFieldRepository) once the design is locked. The actual code is a thin decorator that introspects DTO fields by name and zeroes them out on the historical clone.

Refs oatpp-authkit#14 (parent migration tracker, now closed).

## Context `authkit#14` PR 4 makes the `users` table temporal (`entity_id` + `valid_from` + `valid_until`). Per owner directive, `password_hash` rides the temporal row — every password change creates a historical row with the prior hash preserved, plus a new live row carrying the new hash. This raises a security question: **historical password hashes should not stay readable indefinitely**. They give an attacker who breaches the DB everything they need to brute-force every password a user has ever used. ## What needs to ship A redaction policy applied at password-change time. Owner suggested: blank the actual hash on the historical row, but keep the row so the change-history (timestamps, who changed it) remains auditable. Concrete shape (proposed): 1. When `TemporalRepository<UserDto>::save` decides this is an UPDATE that closes the prior live row, run an extra step: on the historical clone, set `password_hash = NULL` (or to a fixed sentinel like `redacted`). 2. The live row keeps the new hash as usual. 3. `valid_from` / `valid_until` / `username` / `role` / `tls_cert_dn` stay on the historical row — only `password_hash` is redacted. 4. Net effect: an audit query `SELECT username, valid_from, valid_until FROM users WHERE entity_id = X ORDER BY valid_from` shows the full change history (when did this user exist, when was their password rotated, when were they deactivated), but the hashes themselves only exist on the live row. ## Open questions - **Where does the redaction live?** Three options: - On `TemporalRepository<UserDto>` directly via a specialisation/hook — clean but couples the generic decorator to a user-specific concern. - On a new `RedactedFieldRepository<TDto>` decorator that takes a list of "redact on close" field names — generic, composes cleanly above `TemporalRepository`. - On `ConcreteUserRepository::save` — hand-rolled, not reusable. - **What about `tls_cert_dn`?** Same security argument — a leaked old cert DN is less catastrophic than a hash, but still a privilege-escalation footprint. Worth thinking about whether the same policy applies. - **Backfill?** When this lands, existing historical rows from the migration of the non-temporal table will already have `password_hash` populated. Does the migration redact them retroactively, or do we accept the legacy hashes (acknowledging they were already in plaintext or `bcrypt` format on the live row before the migration anyway)? ## Effort Probably **Small** for option 2 (`RedactedFieldRepository`) once the design is locked. The actual code is a thin decorator that introspects DTO fields by name and zeroes them out on the historical clone. Refs `oatpp-authkit#14` (parent migration tracker, now closed).

uwe.admin commented

2026-05-06 12:59:49 +02:00

Author

Owner

Agent Evaluation

Feasibility: Low risk. The decorator stack is already in place (PR 0 of authkit#14); a new RedactedFieldRepository<TDto> slots above TemporalRepository without touching anything that ships today. Field-by-name introspection on a DTO is straightforward via oatpps reflective property dispatcher (used by cloneDto in TemporalRepository).

Impact: High in the security sense, low in the operational sense. Brute-forcing one historical password hash gives an attacker a known-plaintext oracle for the users password rotation patterns and likely future passwords. Closing that off is a clean win and doesnt cost runtime performance (one extra field-write per password change, never on read paths).

Effort: Small. Maybe 3-4 hours of code: a new repo/RedactedFieldRepository.hpp header, a couple of unit tests, README update, version bump.

Recommendation: Accept. Strong case from a defence-in-depth standpoint, and the cost is genuinely small.

Implementation plan

Add repo/RedactedFieldRepository.hpp — generic decorator templated on <TDto, FieldNames…>. On save, when the inner TemporalRepository decides this is an UPDATE that closes the prior live row, hook in to set the named fields to nullptr on the historical clone before its persisted. Mechanism: shadow the TemporalRepository save path with our own version, or expose a small "before-historical-save" hook on the temporal decorator that this layer plugs into. The hook is the cleaner shape — keeps redaction policy out of the temporal decorator.
Wire into makeUserRepository: RedactedFieldRepository<UserDto, "passwordHash">(TemporalRepository<UserDto>(ConcreteUserRepository(udb))).
Add tests: save-update-close-historical-blank-passwordHash, save-fresh-no-redaction, multi-field redaction list, ensure live row keeps the new value.
Document the convention in README.md alongside the other decorators.

Open design choices (decision needed below)

The three open questions in the issue body resolve as follows in this evaluation:

Where does the redaction live? → Option 2 (new RedactedFieldRepository<TDto>). The hook-based approach above is the cleanest shape.
tls_cert_dn? → Same redaction policy applies. Cert DNs identify which old credential was active; an attacker who breaches the historical DB shouldnt get a privilege-escalation map. Cheap to add, makes the security story coherent.
Backfill on the migration from non-temporal users? → Dont retroactively redact. Existing rows pre-migration only had a single live hash anyway; no historical hashes to redact. Once migrated, the redaction policy applies to all future changes.

Decision needed

Check one (edit this comment):

Option A — RedactedFieldRepository decorator + redact passwordHash only — cleanest scope; defer tls_cert_dn to a separate consideration.
Option B — RedactedFieldRepository decorator + redact passwordHash AND tlsCertDn — single PR, full security story.
Option C — Need clarification first — prefer to discuss the hook mechanism / introspection approach before implementation.

## Agent Evaluation **Feasibility:** Low risk. The decorator stack is already in place (PR 0 of authkit#14); a new `RedactedFieldRepository<TDto>` slots above `TemporalRepository` without touching anything that ships today. Field-by-name introspection on a DTO is straightforward via oatpps reflective property dispatcher (used by `cloneDto` in `TemporalRepository`). **Impact:** High in the security sense, low in the operational sense. Brute-forcing one historical password hash gives an attacker a known-plaintext oracle for the users password rotation patterns and likely future passwords. Closing that off is a clean win and doesnt cost runtime performance (one extra field-write per password change, never on read paths). **Effort:** **Small.** Maybe 3-4 hours of code: a new `repo/RedactedFieldRepository.hpp` header, a couple of unit tests, README update, version bump. **Recommendation:** Accept. Strong case from a defence-in-depth standpoint, and the cost is genuinely small. ### Implementation plan 1. Add `repo/RedactedFieldRepository.hpp` — generic decorator templated on `<TDto, FieldNames…>`. On `save`, when the inner `TemporalRepository` decides this is an UPDATE that closes the prior live row, hook in to set the named fields to `nullptr` on the historical clone before its persisted. Mechanism: shadow the `TemporalRepository` save path with our own version, or expose a small "before-historical-save" hook on the temporal decorator that this layer plugs into. The hook is the cleaner shape — keeps redaction policy out of the temporal decorator. 2. Wire into `makeUserRepository`: `RedactedFieldRepository<UserDto, "passwordHash">(TemporalRepository<UserDto>(ConcreteUserRepository(udb)))`. 3. Add tests: save-update-close-historical-blank-passwordHash, save-fresh-no-redaction, multi-field redaction list, ensure live row keeps the new value. 4. Document the convention in README.md alongside the other decorators. ### Open design choices (decision needed below) The three open questions in the issue body resolve as follows in this evaluation: - **Where does the redaction live?** → Option 2 (new `RedactedFieldRepository<TDto>`). The hook-based approach above is the cleanest shape. - **`tls_cert_dn`?** → Same redaction policy applies. Cert DNs identify *which* old credential was active; an attacker who breaches the historical DB shouldnt get a privilege-escalation map. Cheap to add, makes the security story coherent. - **Backfill on the migration from non-temporal users?** → Dont retroactively redact. Existing rows pre-migration only had a single live hash anyway; no historical hashes to redact. Once migrated, the redaction policy applies to all *future* changes. ### Decision needed Check one (edit this comment): - [ ] **Option A — RedactedFieldRepository decorator + redact `passwordHash` only** — cleanest scope; defer `tls_cert_dn` to a separate consideration. - [x] **Option B — RedactedFieldRepository decorator + redact `passwordHash` AND `tlsCertDn`** — single PR, full security story. - [ ] **Option C — Need clarification first** — prefer to discuss the hook mechanism / introspection approach before implementation.

uwe.admin added the

evaluated

label 2026-05-06 13:00:08 +02:00

uwe.admin commented

2026-05-06 13:00:19 +02:00

Author

Owner

Evaluated #15 (effort: Small; recommendation: Accept) — picked Option 2 (RedactedFieldRepository decorator) with A/B/C decision-needed for the redaction scope (passwordHash only / + tlsCertDn / discuss first).

u.schuster added the

accepted

label 2026-05-06 20:47:43 +02:00

uwe.admin referenced this issue from a commit

2026-05-06 20:52:08 +02:00

#15: RedactedFieldRepository — null credentials on historical rows

uwe.admin closed this issue

2026-05-06 20:52:08 +02:00

uwe.admin commented

2026-05-06 20:52:14 +02:00

Author

Owner

Implemented #15 — RedactedFieldRepository landed in commit 52449e4, tagged v0.13.0. Both passwordHash and tlsCertDn redact on historical rows; live row keeps its values. 14/14 tests pass.

Implemented #15 — RedactedFieldRepository landed in commit `52449e4`, tagged `v0.13.0`. Both `passwordHash` and `tlsCertDn` redact on historical rows; live row keeps its values. 14/14 tests pass.

Rows
Columns

Password hash redaction policy for historical user rows #15

Context

What needs to ship

Open questions

Effort

Agent Evaluation

Implementation plan

Open design choices (decision needed below)

Decision needed