Skip to content

fix(portal): collection share revoke and shared thumbnail access#485

Merged
cjimti merged 2 commits into
mainfrom
fix/portal-ui-fixes
May 27, 2026
Merged

fix(portal): collection share revoke and shared thumbnail access#485
cjimti merged 2 commits into
mainfrom
fix/portal-ui-fixes

Conversation

@cjimti
Copy link
Copy Markdown
Member

@cjimti cjimti commented May 27, 2026

Summary

  • Collection share DELETE returned 404 because revokeShare assumed every share had an AssetID and verified ownership via the asset. Collection shares have CollectionID instead, so the asset lookup failed.
  • Shared collection thumbnails returned 403 because getCollectionThumbnail checked authentication but not share-based authorization.
  • Tidied code comments in the admin package.

What changed

pkg/portal/handler.go

revokeShare now branches on share.CollectionID: if set, ownership is verified against the collection via verifyCollectionOwner; otherwise the existing asset-ownership path runs. The store's Revoke method was already generic (operates on share ID), so no store changes were needed.

pkg/portal/collection_handler.go

getCollectionThumbnail now checks coll.OwnerID != user.UserID and falls back to collectionSharePermission before serving the image, matching the pattern used by getCollection.

pkg/admin/system.go, pkg/admin/system_test.go

Removed vendor-specific names from code comments.

Test plan

  • TestRevokeCollectionShare/owner_can_revoke - owner successfully revokes a collection share
  • TestRevokeCollectionShare/non_owner_forbidden - non-owner gets 403
  • TestRevokeCollectionShare/collection_not_found - missing collection gets 404
  • TestGetCollectionThumbnail_SharedUser/shared_user_can_view - user with share grant gets the thumbnail
  • TestGetCollectionThumbnail_SharedUser/non_shared_user_forbidden - user without share grant gets 403
  • All existing revoke and thumbnail tests pass unchanged
  • make verify core checks pass (fmt, test/race, lint, security, coverage, patch coverage 91.2%)

cjimti added 2 commits May 26, 2026 17:04
The revokeShare handler assumed every share had an AssetID and verified
ownership via the asset. Collection shares have CollectionID instead,
so the asset lookup returned 404 on every DELETE attempt.

The collection thumbnail endpoint checked authentication but not
authorization, so users with valid share grants got 403 when loading
thumbnails for shared collections.
@codecov
Copy link
Copy Markdown

codecov Bot commented May 27, 2026

Codecov Report

❌ Patch coverage is 88.88889% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.18%. Comparing base (6bab1a3) to head (f62b50d).

Files with missing lines Patch % Lines
pkg/portal/handler.go 86.36% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #485   +/-   ##
=======================================
  Coverage   86.18%   86.18%           
=======================================
  Files         237      237           
  Lines       33005    33025   +20     
=======================================
+ Hits        28444    28464   +20     
  Misses       3301     3301           
  Partials     1260     1260           

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

@cjimti cjimti merged commit 43843e0 into main May 27, 2026
9 checks passed
@cjimti cjimti deleted the fix/portal-ui-fixes branch May 27, 2026 00:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant