Fixing up guard access requests

[Vagoasdf]

Ticket [ENG-2440]

Description Of Changes

Fixed a guard clause in the SaaS connector that was not properly skipping async access requests when the policy does not have access rules. Previously, when an async DSR strategy (such as polling) was detected, the code would check if the request was an erasure request and skip the polling on access requests, but it would not exit the retrieve_data method, running the polling access request like a normal request

Regarding Callbacks:
The Current test setup for Callback requires the bypass of the retrieve data guard, due to it requiring the initial access data to set up the future calls. Updating the Callback test was considered, but it escaped the scope an LoC limit for this fix

Code Changes

• Updated retrieve_data() method to check the guard clause only on async requests, and exit the method if it fails the guard
• Added comprehensive unit tests in test_saas_connector.py:
  • test_guard_access_request_with_access_policy: Verifies async access requests run when policy has access rules
  • test_guard_access_request_with_erasure_only_policy: Verifies async access requests are skipped when policy has no access rules

Steps to Confirm

You can use a Polling Async integration, such as AppsFlyer, and verify that there should be no access requests firing when executing an erasure request.

To test:

1. Set up a SaaS connector with async polling configuration (e.g., AppsFlyer)
2. Create an erasure-only policy (no access rules)
3. Submit a privacy request with the erasure-only policy
4. Verify that no access requests are made to the external system
5. Verify that only erasure/masking requests are executed
6. Check logs to confirm "Skipping async access request for policy" message appears when appropriate

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 (i.e. potential for performance impact or unexpected regression) that should be flagged
  • 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	Review	Updated (UTC)
fides-plus-nightly	Ignored	Preview	Dec 18, 2025 1:36pm
fides-privacy-center	Ignored		Dec 18, 2025 1:36pm

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.33%. Comparing base (27068af) to head (eeff6b9).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7130      +/-   ##
==========================================
+ Coverage   87.30%   87.33%   +0.02%     
==========================================
  Files         533      533              
  Lines       35090    35166      +76     
  Branches     4067     4084      +17     
==========================================
+ Hits        30635    30711      +76     
+ Misses       3570     3569       -1     
- Partials      885      886       +1     

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

[greptile-apps]

Greptile Summary

This PR fixes a critical bug in the SaaS connector's async access request handling. Previously, when a policy had no access rules (erasure-only), the guard clause would prevent async access requests from being initiated, but the code would continue executing the regular retrieval flow instead of returning early.

Key Changes:

• Restructured the guard clause logic in retrieve_data() to properly exit when polling async requests should be skipped
• Added AsyncTaskType import to distinguish between polling and callback strategies
• Guard clause now only blocks polling requests when the policy lacks access rules
• Callback requests correctly bypass the guard and proceed with the normal flow
• Added comprehensive test coverage with three new test cases validating different scenarios

Implementation Details:
The fix moves the guard check inside the async strategy conditional and adds explicit handling for polling vs callback strategies. When a polling strategy is detected and the guard fails, the method now returns an empty list immediately. Callback strategies are allowed to fall through to the normal request flow, which is the correct behavior for callback-based async operations.

Test Coverage:

• test_guard_access_request_with_access_policy: Verifies async access requests execute when policy has access rules
• test_guard_access_request_with_erasure_only_policy: Confirms async polling access requests are skipped with erasure-only policies
• test_callback_requests_ignore_guard_clause: Ensures callback requests bypass the guard entirely

The fix properly addresses the issue where async access requests would incorrectly execute during erasure-only operations.

Confidence Score: 4/5

• This PR is safe to merge with minor style issues that don't affect functionality
• The core logic fix is correct and well-tested with three comprehensive test cases covering polling and callback scenarios. The implementation properly distinguishes between async strategy types and handles the guard clause appropriately. However, the score is 4 instead of 5 due to minor style violations: the test cleanup code manually deletes database records, which violates project guidelines stating that the database is automatically cleared between test runs. These cleanup blocks are unnecessary and should be removed, but they don't impact the correctness of the fix.
• No files require special attention - the logic is sound and test coverage is comprehensive

Important Files Changed

Filename	Overview
src/fides/api/service/connectors/saas_connector.py	Fixed guard clause logic to properly skip async polling access requests when policy lacks access rules, and added AsyncTaskType import
tests/ops/service/connectors/test_saas_connector.py	Added comprehensive test coverage for guard clause behavior with polling and callback strategies, though manual cleanup violates testing best practices

[greptile-apps]

_{4 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

style: manual deletion of test records unnecessary - database automatically cleared between test runs

Suggested change

	# Cleanup
	try:
	erasure_rule.delete(db)
	erasure_only_policy.delete(db)
	except Exception:
	pass

Context Used: Rule from dashboard - Do not manually delete database records in test fixtures or at the end of tests, as the database is ... (source)

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

[greptile-apps]

style: manual deletion of test records unnecessary - database automatically cleared between test runs

Suggested change

	# Cleanup
	try:
	erasure_rule.delete(db)
	erasure_only_policy.delete(db)
	except Exception:
	pass

Context Used: Rule from dashboard - Do not manually delete database records in test fixtures or at the end of tests, as the database is ... (source)

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}