[2.x] fix(gdpr): harden export download with audit trail, single-use, and active expiry check - #4642
Merged
Merged
Conversation
…ctive expiry check - Active expiry check in Export::byFile() so logically-expired tokens stop working immediately, even if the daily cleanup cron has not run. - Single-use enforcement via downloaded_at — second use returns 404. - Audit metadata captured on download (timestamp, IP, UA truncated to 255 chars). - Two-stage cleanup: cron now deletes the ZIP and nulls the file column rather than removing the row, preserving the audit record. Already-downloaded exports get their artifact swept early. - Replace FileNotFoundException (500) with ModelNotFoundException (404) so missing/expired/used tokens return a proper status. - byFile() rejects null/empty tokens and requires file IS NOT NULL as defense in depth against requests against cleaned-up rows. Exporter::destroy() is renamed to Exporter::expire() to reflect the new semantics.
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
1 participant
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.
Summary
Hardens the GDPR data-export download flow in response to a security report flagging the
GET /gdpr/export/{file}route as having no authorization (CWE-862) and no enforced session expiration (CWE-613).The token-as-bearer model is preserved for now (the link still works without a session — that's a UX decision to revisit in a follow-up with a UI-driven download), but the practical exploit surface is closed:
Export::byFile()now requiresdestroys_at > now(). Previously, expiry depended entirely on the dailygdpr:destroy-exportscron sweep, so a logically-expired token kept working until that ran (or indefinitely on instances where the scheduler isn't wired up).downloaded_atand subsequent requests with the same token return 404.downloaded_at,downloaded_ip, anddownloaded_user_agent(truncated to 255 chars).filecolumn rather than dropping the row outright. The audit row persists for usage analysis. The cron also picks up already-downloaded exports early, since their artifacts are no longer needed.FileNotFoundException(which fell through to a 500) replaced withModelNotFoundExceptionso missing/expired/used tokens return a proper 404.byFile()rejects null/empty tokens and requiresfile IS NOT NULL, so a request with an empty?file=cannot match a row whose token has been nulled by cleanup.Schema
New migration adds three nullable columns to
gdpr_exports:downloaded_at(datetime)downloaded_ip(varchar 45, IPv6-safe)downloaded_user_agent(varchar 255)Behaviour change for operators
Exporter::destroy()is renamed toExporter::expire()and no longer deletes the row — it deletes the ZIP, nullsfile, and marks notifications deleted. Audit rows accumulate over time but are cheap (one row per export request) and can be useful for understanding feature usage.