feat(context-center): file visibility, tabular profiles on assets, files in query runner requests - #34173
feat(context-center): file visibility, tabular profiles on assets, files in query runner requests#34173tomasmontielp wants to merge 10 commits into
Conversation
✅ PR checks passedThe linked issue has a description and all required Shipping project fields set. Thanks! |
github-actions
Bot
added
the
safe to test
❌ UI Checkstyle Failed❌ ESLint + Prettier + Organise Imports (src)One or more source files have linting or formatting issues. Affected files
Fix locally (fast - only checks files changed in this branch): make ui-checkstyle-changed |
✅ Generated Sources Auto-UpdatedThe generated TypeScript types ( |
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| @Context SecurityContext securityContext, | ||
| @PathParam("id") UUID id, | ||
| @Valid jakarta.json.JsonPatch patch) { | ||
| // Sharing is edited through this endpoint, so the guard runs here too: a caller who cannot see | ||
| // a document must not be able to change who else can. | ||
| ContextFileVisibility.enforceVisibility( | ||
| getInternal( | ||
| uriInfo, | ||
| securityContext, | ||
| id, | ||
| ContextFileVisibility.guardFields(""), | ||
| Include.NON_DELETED), | ||
| securityContext); | ||
| return patchInternal(uriInfo, securityContext, id, patch); |
There was a problem hiding this comment.
🚨 Bug: shareConfig changes via PATCH are never persisted
ContextFileUpdater.entitySpecificUpdate (ContextFileRepository.java:261-271) records fileType, processingStatus, extractedText, folder and a few more, but not shareConfig. When a PATCH changes only shareConfig, which is exactly what the new visibility menu sends, entityChanged and versionChanged both stay false. storeUpdate then takes the no-change branch and never writes the row. The UI shows a success toast, but the file keeps its old visibility in the DB and in search. ContextMemoryRepository's updater does record this field. The fix is to add recordChange("shareConfig", original.getShareConfig(), updated.getShareConfig(), true); to the file updater.
Record shareConfig changes so the updater persists them:
recordChange("pageCount", original.getPageCount(), updated.getPageCount());
recordChange("shareConfig", original.getShareConfig(), updated.getShareConfig(), true);
updateFolder();
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
| public static boolean isVisibleToUser(ContextFile file, String userName, boolean isAdmin) { | ||
| if (isAdmin) { | ||
| return true; | ||
| } | ||
| MemoryShareConfig share = file.getShareConfig(); | ||
| if (share == null || share.getVisibility() == null) { | ||
| return true; | ||
| } | ||
| if (isOwnedBy(file, userName)) { | ||
| return true; | ||
| } | ||
| if (share.getVisibility() == MemoryVisibility.ENTITY) { | ||
| return true; | ||
| } | ||
| return share.getVisibility() == MemoryVisibility.SHARED |
There was a problem hiding this comment.
⚠️ Bug: Private locks out the uploader since Drive files have no owners
The upload endpoint builds the file with ContextFileMapper.createToEntity(createFile, user) and never sets owners, so Drive uploads are ownerless. ContextFileVisibility.isVisibleToUser only lets owners (or admins) through for Private, and the search filter works the same way. When a non-admin picks "Private — visible only to you" (DocumentsView.component.tsx:316-323), the document disappears for that user and for everyone else except admins. The same happens to any editor who restricts a file they don't own. Fix options: make the uploader the owner at upload time, add createdBy to the check, or refuse Private/Shared on a file that has no owner the caller matches.
Make the uploader the owner so Private stays visible to them:
ContextFile file = mapper.createToEntity(createFile, user);
if (nullOrEmpty(file.getOwners())) {
file.setOwners(List.of(Entity.getEntityReferenceByName(Entity.USER, user, Include.NON_DELETED)));
}
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
| ContextFile file = | ||
| getInternal(uriInfo, securityContext, id, ContextFileVisibility.guardFields(""), include); | ||
| ContextFileVisibility.enforceVisibility(file, securityContext); |
There was a problem hiding this comment.
⚠️ Security: Bulk download and move skip the new file visibility guard
This commit adds ContextFileVisibility.enforceVisibility to get, getByName, patch, the single-file download and delete. bulkDownloadFiles (around line 620) still calls getInternal(uriInfo, securityContext, id, "", include) and streams every file's bytes into the zip without checking visibility. A user who is not the owner or a named sharer can POST a Private file's id to /bulk/download and get its content, even though GET /{id}/download returns 403 for the same file. moveFile and bulkMoveFiles also only check EDIT_ALL, so a user who cannot see a private file can still move it. Bulk delete is safe because it goes through delete(). Fix: in the bulk-download loop, fetch with guardFields("") and call enforceVisibility, and add the same check before repository.moveContextFile in both move endpoints.
Enforce visibility in the bulk-download resolve loop (apply the same pattern before moveContextFile in moveFile/bulkMoveFiles):
for (UUID id : ids) {
ContextFile file =
getInternal(uriInfo, securityContext, id, ContextFileVisibility.guardFields(""), include);
ContextFileVisibility.enforceVisibility(file, securityContext);
Asset asset = resolveAsset(file);
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
Jest test CoverageUI tests summary
|
GitHub Actions
Failure
|
This branch was successfully deployed
1 participant
Schema and service groundwork for analyzing uploaded spreadsheets from AskCollate.
maxUploadsPerUserPerDay, the per-user daily upload cap the chat enforces.Needed by open-metadata/openmetadata-collate#6932, which merges after this one. Nothing here waits on another PR.
Fixes #34175