ENG-3418: Block approval of unverified duplicate privacy requests

[JadeCara]

Ticket ENG-3418

Description Of Changes

A duplicate privacy request whose identity was never verified could be approved and processed by an admin, bypassing identity verification entirely. This happened because duplicate was an allowed status for approval (intentional admin override for false-positive duplicates), but there was no secondary check for whether the identity had actually been verified.

The fix adds a guard: duplicate requests can only be approved if identity_verified_at is set. The admin UI also hides the approve button for unverified duplicates. Deny is still allowed for cleanup.

How this happens:

1. Request created from privacy center → identity_unverified
2. Duplicate detection marks it as duplicate before user can verify
3. Admin sees the duplicate in the request list and clicks Approve
4. Request gets processed without identity ever being proven

Code Changes

• src/fides/service/privacy_request/privacy_request_service.py — Added guard in approve_privacy_requests: rejects duplicate + identity_verified_at is None; logs error for blocked attempts
• clients/admin-ui/.../useApproveDenyPrivacyRequest.ts — Hides approve button for unverified duplicates (deny still shown)
• clients/admin-ui/.../types.ts — Fixed identity_verified_at type to string | null to match API response
• tests/.../test_privacy_request_endpoints.py — Added test_approve_unverified_duplicate_rejected and test_bulk_approve_mixed_verified_unverified_duplicates; updated existing duplicate approve tests to set identity_verified_at

Steps to Confirm

1. Enable duplicate request detection, identity verification, and manual approval in settings
2. Configure Mailgun messaging service
3. Submit a DSR from the privacy center → status: identity_unverified
4. Submit a second DSR with the same identity → status: duplicate
5. Log in to the admin UI and navigate to the privacy requests list
6. Confirm the Approve button is not shown for the unverified duplicate request (Deny should still be visible)
7. Via API, attempt to approve the unverified duplicate → blocked with "Cannot approve unverified duplicate request"
8. Verified duplicates CAN still be approved (the admin override for false positives)

E2E results:

Request 1 (pri_c0a2d...): identity_unverified (canonical)
Request 2 (pri_7e71e...): duplicate, identity_verified_at=None
Approve Request 2 → Failed: "Cannot approve unverified duplicate request"  ✓

Pre-Merge Checklist

• Issue requirements met
• All CI pipelines succeeded
• CHANGELOG.md updated
  • Add a db-migration This indicates that a change includes a database migration label to the entry if your change includes a DB migration
  • Add a high-risk This issue suggests changes that have a high-probability of breaking existing code label to the entry if your change includes a high-risk change
  • Updates unreleased work already in Changelog, no new entry necessary
• UX feedback:
  • All UX related changes have been reviewed by a designer
  • No UX review needed
• Followup issues:
  • Followup issues created
  • No followup issues
• Database migrations:
  • Ensure that your downrev is up to date with the latest revision on main
  • Ensure that your downgrade() migration is correct and works
    • If a downgrade migration is not possible for this change, please call this out in the PR description!
  • No migrations
• Documentation:
  • Documentation complete, PR opened in fidesdocs
  • Documentation issue created in fidesdocs
  • If there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
  • No documentation updates required

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments

Project	Deployment	Actions	Updated (UTC)
fides-plus-nightly	Ignored	Preview	Apr 21, 2026 5:02pm
fides-privacy-center	Ignored		Apr 21, 2026 5:02pm

[github-actions]

Title	Lines	Statements	Branches	Functions
admin-ui		6.34% (2799/44080)	5.59% (1402/25046)	4.44% (579/13039)
fides-js		78.98% (1962/2484)	65.55% (1214/1852)	72.57% (336/463)
privacy-center		85.97% (331/385)	81.36% (179/220)	78.87% (56/71)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.94%. Comparing base (b5b98e6) to head (57072af).
⚠️ Report is 10 commits behind head on main.

❌ Your project status has failed because the head coverage (84.94%) is below the target coverage (85.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7972      +/-   ##
==========================================
- Coverage   85.04%   84.94%   -0.11%     
==========================================
  Files         631      630       -1     
  Lines       41213    41086     -127     
  Branches     4807     4769      -38     
==========================================
- Hits        35049    34899     -150     
- Misses       5070     5103      +33     
+ Partials     1094     1084      -10     

☔ 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.

[JadeCara]

/code-review

[claude]

Code Review: Block approval of unverified duplicate privacy requests

This is a well-scoped security fix. The core guard is placed at exactly the right point in approve_privacy_requests — after the status allowlist check but before the DB write — so it cannot be bypassed by manipulating the request body. Backend and frontend defenses are independent of each other (defense in depth), which is the right pattern.

What's done well

• The if block in the service is placed correctly: after the existing PrivacyRequestStatus allowlist check, before the try block that performs the state transition. A caller cannot bypass it.
• The frontend guard in useApproveDenyPrivacyRequest is independent of the backend — the Approve button is simply not rendered for unverified duplicates, not just disabled.
• Two new tests cover both the single-request rejection and the mixed bulk-approval scenario with specific assertions on succeeded/failed counts and the error message.
• The fix to test_approve_privacy_request (setting identity_verified_at for the duplicate parametrize case) is the correct minimal change to keep the existing parametrized test passing under the new invariant.
• Changelog entry is present.

Findings

Suggestion — Missing audit log for blocked approval attempt (see inline comment on privacy_request_service.py): Every successful approval and terminal failure (e.g. wrong status) records an AuditLog entry. The unverified-duplicate case silently continues with no audit trail. Since this is a security-relevant event (an admin attempted to approve a request that bypassed identity verification), it would be valuable to log it for compliance and anomaly-detection purposes.

Nit — identity_verified_at type mismatch in PrivacyRequestEntity (see inline comment on useApproveDenyPrivacyRequest.ts): The field is typed as string | undefined in the entity type, but API responses return string | null. Runtime behaviour is correct since !value handles both falsy values, but updating the type to string | null | undefined would prevent subtle bugs if stricter checks are added later.

Nit — Missing privacy_request.delete(db) at the end of test_approve_unverified_duplicate_rejected for consistency with sibling tests (see inline comment on the test file).

Overall this is a solid, well-tested fix. The audit log suggestion is the only thing worth considering before merge.

🔬 Codegraph: connected (47107 nodes)

---

💡 Write /code-review in a comment to re-run this review.

[claude]

src/fides/service/privacy_request/privacy_request_service.py:665

Suggestion: Consider adding an audit log entry for blocked approval attempts

Every successful approval records an AuditLog entry. The unverified-duplicate case is a security-relevant event (an admin attempted to approve a request that skipped identity verification), but it silently continues with no audit trail. Compared to a normal validation failure (e.g. wrong status), this case is more useful to surface for compliance audits or to detect repeated attempts.

Consider adding an AuditLog.create(...) before the continue, using an appropriate existing AuditLogAction (e.g. rejected):

AuditLog.create(
    db=db,
    data={
        "user_id": requester_id,
        "privacy_request_id": privacy_request.id,
        "action": AuditLogAction.rejected,
        "message": "Cannot approve unverified duplicate request",
    },
)

This keeps it consistent with the rest of the approve flow where all terminal outcomes are logged.

[JadeCara]

Done — added a logger.error() call since this path only fires if someone bypasses the UI and hits the API directly. AuditLog wasn't the right fit since denied means an admin denied the request, whereas this is an invalid API call. The error log captures the request ID and user for investigation.

[gilluminate]

FE looks good!

[daveqnet]

Adding do not merge label and converting to draft for now