Repository navigation
fix(security): filter unreadable items/bitstreams from public usage statistics - #1540
Merged
Merged
Conversation
added 2 commits
October 27, 2026 13:21
Site-level TopItems and item TotalDownloads reports returned the name and UUID of items/files without checking the caller's READ permission, leaking withdrawn/embargoed/restricted content to anonymous users. Inject AuthorizeService and BitstreamService into UsageReportUtils and only add a point when the current user has READ on the resolved item/bitstream. Drop the unresolved raw-handle fallback in the site report.
Use BitstreamService#findByIdOrLegacyId with a null/blank guard instead of UUID.fromString so migrated statistics rows with legacy numeric ids do not crash the TotalDownloads report. Add a test proving a non-admin eperson with an explicit READ policy still sees the restricted bitstream while another eperson without it does not.
There was a problem hiding this comment.
🟡 Changes recommended
Serialize resolved legacy bitstreams using canonical UUIDs and add regression coverage for numeric legacy IDs.
1 open issue
What changed in this PR
This pull request prevents public usage-statistics reports from exposing unreadable items and bitstreams while preserving access for authorized users.
Changes:
- Applies per-user READ authorization to site TopItems results.
- Resolves bitstreams using UUIDs or legacy IDs before filtering TotalDownloads results.
- Adds integration tests for anonymous, administrator, and explicitly authorized users.
| File | Summary |
|---|---|
dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/UsageReportUtils.java |
Filters unreadable items and bitstreams from usage reports. A moderate issue remains: legacy numeric IDs should be serialized as the resolved bitstream UUID. A nit remains to add regression coverage for legacy numeric IDs. |
dspace-server-webapp/src/test/java/org/dspace/app/rest/StatisticsRestRepositoryIT.java |
Adds coverage for restricted item and bitstream visibility across user roles; legacy numeric-ID resolution is not yet covered. |
Review effort: Lite
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.
Problem
Public usage reports could disclose the identity of objects the caller cannot read:
Part of dataquest-dev/dspace-customers#1104 (R15), see #1083.
Fix
Check the caller's READ permission before including each item or bitstream in these reports. Skip missing objects instead of returning an unresolved URL. Resolve historical numeric bitstream IDs as well as UUIDs; return the resolved bitstream's canonical UUID.
The existing report structure, configuration and aggregation are preserved. Authorization applies to the requesting user, including non-admin users with explicit READ policies.
Tests
The two previously failing download tests now reload the existing fixtures into the current persistence context. They no longer create files through a detached collection.
Local validation: all 49 tests in StatisticsRestRepositoryIT passed (0 failures/errors), including existing report behavior and the UUID/legacy authorization cases. Production/test compilation and Checkstyle passed. Full GitHub CI must pass on the updated commit before merge.