Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

return more perms from /api/access/datafile/{id}/userPermissions #11267

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

pdurbin
Copy link
Member

@pdurbin pdurbin commented Feb 14, 2025

What this PR does / why we need it:

API users (especially the frontend team, which is writing the new UI) need more granularity when checking file permissions.

Which issue(s) this PR closes:

Special notes for your reviewer:

Instead of adding these four new permissions, the API user could simply check the existing "canEditOwnerDataset" instead. I'm curious where these permissions come from? JSF? They aren't in our main Permission class.

I cleaned up existing assertions and added a new "no perms" user and additional assertions.

Suggestions on how to test this:

Create some users. Give them various permissions. Check the /api/access/datafile/{id}/userPermissions API. Does it return the right results?

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

No.

Is there a release notes update needed for this change?:

Additional documentation:

@pdurbin pdurbin added Type: Feature a feature request Feature: API Size: 3 A percentage of a sprint. 2.1 hours. SPA These changes are required for the Dataverse SPA GREI Re-arch Issues related to the GREI Dataverse rearchitecture SPA.Q1 Not related to any specific Q1 feature FY25 Sprint 17 FY25 Sprint 17 (2025-02-12 - 2025-02-26) labels Feb 14, 2025
@coveralls
Copy link

coveralls commented Feb 14, 2025

Coverage Status

coverage: 22.745% (-0.002%) from 22.747%
when pulling 5a8ad0a on 11226-file-perms
into d3cba3f on develop.

@@ -1711,6 +1711,10 @@ public Response getUserPermissionsOnFile(@Context ContainerRequestContext crc, @
jsonObjectBuilder.add("canDownloadFile", permissionService.userOn(requestUser, dataFile).has(Permission.DownloadFile));
jsonObjectBuilder.add("canManageFilePermissions", permissionService.userOn(requestUser, dataFile).has(Permission.ManageFilePermissions));
jsonObjectBuilder.add("canEditOwnerDataset", permissionService.userOn(requestUser, dataFile.getOwner()).has(Permission.EditDataset));
jsonObjectBuilder.add("canEditFileMetadata", permissionService.userOn(requestUser, dataFile.getOwner()).has(Permission.EditDataset));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the code returns the answer to Permission.EditDataset 5 times in a row?

Copy link
Contributor

@g-saracca g-saracca Feb 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @pdurbin and @qqmyers, if you think that with the canEditOwnerDataset property we are already covered to handle the allowed actions on a file I think this PR is not really going to be needed.

This comment has been minimized.

1 similar comment
Copy link

📦 Pushed preview images as

ghcr.io/gdcc/dataverse:11226-file-perms
ghcr.io/gdcc/configbaker:11226-file-perms

🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature: API FY25 Sprint 17 FY25 Sprint 17 (2025-02-12 - 2025-02-26) GREI Re-arch Issues related to the GREI Dataverse rearchitecture Size: 3 A percentage of a sprint. 2.1 hours. SPA.Q1 Not related to any specific Q1 feature SPA These changes are required for the Dataverse SPA Type: Feature a feature request
Projects
Status: Ready for Review ⏩
Development

Successfully merging this pull request may close these issues.

User Permissions on a File - Extend API information
4 participants