DC-394: honor per-case-type address+view IdProofingRequirements when masking household data#301
Open
ShaynaCummings wants to merge 1 commit into
Open
DC-394: honor per-case-type address+view IdProofingRequirements when masking household data#301ShaynaCummings wants to merge 1 commit into
address+view IdProofingRequirements when masking household data#301ShaynaCummings wants to merge 1 commit into
Conversation
…usehold data
PII visibility for address/email/phone was resolved against an empty case
list every time, which is correct only for uniform requirements. For
per-case-type IdProofingRequirements (e.g., the granular `address+view` form CO
plans to adopt when co-loading begins in 2027) it silently degraded to
IAL1 for every request — `IalRequirement.Resolve([])` short-circuits to
IAL1 ("no cases = no case-derived reason to require elevated IAL"), so
the per-case-type "highest wins" rule was never applied. `household+view`
didn't have this gap because its evaluator already received the real
cases; this change brings PII visibility into the same shape.
Changes:
- Add `IPiiVisibilityService.GetVisibility(UserIalLevel, IReadOnlyList<SummerEbtCase>)`
overload; existing case-less overload delegates by passing `[]` so
callers without case context keep their prior behavior.
- `IdProofingService.EvaluateView` now takes cases and passes them to
`IalRequirement.Resolve`, so per-case-type view requirements actually
resolve against the user's cases.
- Extract masking from `HouseholdRepository.ApplyPiiVisibility` into a
shared `HouseholdPiiFilter.Apply` utility (Core.Utilities) so the
use-case layer can re-apply visibility after fetching.
- `GetHouseholdDataQueryHandler` now fetches with full PII, runs the
existing `household+view` check against real cases, then resolves
case-aware PII visibility and applies masking before returning. The
visibility decision now happens once, against the data we're actually
about to return.
Out of scope (worth a follow-up if we want full symmetry):
- `UpdateAddressCommandHandler` still uses the case-less overload. It
doesn't expose household PII back to the client, so its visibility
decision is functionally inert.
- `MockHouseholdRepository.CreateCopy` duplicates the masking logic
alongside its defensive deep copy; left untouched here to keep the
diff scoped to the bug.
Tests:
- New `GetVisibility_PerCaseTypeAddressView_ResolvesAgainstActualCases`
documents the gap (was a compile-error red before the overload was
added; green now).
- `Handle_WhenIdentifierResolvedAndHouseholdExistsButNotIdVerified_ReturnsSuccessWithoutAddress`
reworked to provide a real address and assert it gets masked to "****"
through the new code path, instead of relying on the test's pre-null
AddressOnFile to make masking a no-op.
- Default mocks added in `GetHouseholdDataQueryHandlerTests` and
`HouseholdControllerTests` so existing tests pick up full PII from the
new overload without per-test wiring.
- A few `Assert.Same` checks against the repository's returned instance
replaced with behavior-focused assertions, since the handler now
produces a new record instance via `with`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
address+view IdProofingRequirements when masking household data
jamesmblair
reviewed
May 26, 2026
| // can be resolved against the household's actual cases below. The | ||
| // case-aware visibility computed after the fetch decides what's masked | ||
| // before returning to the caller. | ||
| var fullPiiVisibility = new PiiVisibility(IncludeAddress: true, IncludeEmail: true, IncludePhone: true); |
Member
There was a problem hiding this comment.
Small/nit sort of thing, but this feels like a special case deserving of a singleton instance or a factory (i.e., PiiVisibility.Full or PiiVisibility.Full())
jamesmblair
approved these changes
May 26, 2026
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.
🔗 Jira ticket
https://codeforamerica.atlassian.net/browse/DC-394
✍️ Description
PII visibility for address/email/phone was resolved against an empty case list every time, which is correct only for uniform requirements. For per-case-type IdProofingRequirements (e.g., the granular
address+viewform CO plans to adopt when co-loading begins in 2027) it silently degraded to IAL1 for every request —IalRequirement.Resolve([])short-circuits to IAL1 ("no cases = no case-derived reason to require elevated IAL"), so the per-case-type "highest wins" rule was never applied.household+viewIdProofingRequirements didn't have this gap because its evaluator already received the real cases; this change brings PII visibility into the same shape.Changes:
IPiiVisibilityService.GetVisibility(UserIalLevel, IReadOnlyList<SummerEbtCase>)overload; existing case-less overload delegates by passing[]so callers without case context keep their prior behavior.IdProofingService.EvaluateViewnow takes cases and passes them toIalRequirement.Resolve, so per-case-type view requirements actually resolve against the user's cases.HouseholdRepository.ApplyPiiVisibilityinto a sharedHouseholdPiiFilter.Applyutility (Core.Utilities) so the use-case layer can re-apply visibility after fetching.GetHouseholdDataQueryHandlernow fetches with full PII, runs the existinghousehold+viewcheck against real cases, then resolves case-aware PII visibility and applies masking before returning. The visibility decision now happens once, against the data we're actually about to return.Out of scope (worth a follow-up if we want full symmetry):
UpdateAddressCommandHandlerstill uses the case-less overload. It doesn't expose household PII back to the client, so its visibility decision is functionally inert.MockHouseholdRepository.CreateCopyduplicates the masking logic alongside its defensive deep copy; left untouched here to keep the diff scoped to the bug.Tests:
GetVisibility_PerCaseTypeAddressView_ResolvesAgainstActualCasesdocuments the gap (was a compile-error red before the overload was added; green now).Handle_WhenIdentifierResolvedAndHouseholdExistsButNotIdVerified_ReturnsSuccessWithoutAddressreworked to provide a real address and assert it gets masked to "****" through the new code path, instead of relying on the test's pre-null AddressOnFile to make masking a no-op.GetHouseholdDataQueryHandlerTestsandHouseholdControllerTestsso existing tests pick up full PII from the new overload without per-test wiring.Assert.Samechecks against the repository's returned instance replaced with behavior-focused assertions, since the handler now produces a new record instance viawith.Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com
✅ Completion tasks