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

feat(organizations): add new asset endpoint for owners and admins TASK-960 #5218

Open
wants to merge 26 commits into
base: main
Choose a base branch
from

Conversation

noliveleger
Copy link
Contributor

@noliveleger noliveleger commented Oct 31, 2024

Summary:

Added a new endpoint to retrieve organization assets, accessible to both owners and admins.

Description:

This update introduces a dedicated endpoint that allows organization owners and admins to access a complete list of assets associated with their organization. Owners and admins have unrestricted access to these assets by virtue of their roles, without needing explicit permissions.

Notes:

As part of this update, a refactor was implemented in the unit test suite to minimize code duplication when testing organization admin access across various endpoints. This refactor allows us to test admin access without explicitly assigning permissions, ensuring that the tests more accurately reflect role-based access.

Testing

  • Register three different users
  • From the shell, add user2 and user3 to user1 org
organization = user1.organization
organization.mmo_override = True
organization.save()
organization.add_user(user2, is_admin=True)
organization.add_user(user3, is_admin=False)
  • The UI is not aware yet about org's admin role, so you will receive Access Denied until you patch the JS code (see below) when you try to access project details but the API call should work.

user2 is now an admin and should be able to see and to do whatever they want on user1's organization projects, they should be able to access the org assets list: /api/v2/<organization_uid>/assets/
user3 is a regular member, they cannot access /api/v2/<organization_uid>/assets/

organization_uid can be retrieve from /me in organization property.

  • Create a project with user3

    1. user3 should see it in the regular asset list (i.e. : /api/v2/assets/) and should have been assigned manage_asset permission
    2. user2 should see it in the org asset list but not in the regular asset list. No permissions assigned.
    3. user1 should see it in the org asset list and in the regular asset list (until the latter is filtered in a future PR see TASK-1187). They are the owner.
  • Create a project with user2

    1. user3 should not see it in the regular asset list (and should be able to access its details either)
    2. user2 should see it in the org asset list and in the regular asset list and should have been assigned manage_asset permission
    3. user1 should see it in the org asset list and in the regular asset list. They are the owner.
  • Create a project with user1

    1. user3 should not see it in the regular asset list (and should be able to access its details either)
    2. user2 should see it in the org asset list but not in the regular asset list. No permissions assigned.
    3. user1 should see it in the org asset list and in the regular asset list. They are the owner.

By-pass front-end checks to allow org admins access project details

  • Apply this diff BUT DO NOT commit it.
  • Use npm run watch to compile front-end build on the fly (with kobo-install, ./run.py -cf run -p 3000:3000 --rm kpi npm run watch)
  • Try to access the project created for user3 with user2 session

When this work, you can test the admin role by navigating in the project detail. The API responses should always be 2xx.

@noliveleger noliveleger added API Changes related to API endpoints Back end labels Oct 31, 2024
@noliveleger noliveleger self-assigned this Oct 31, 2024
@noliveleger noliveleger force-pushed the task-960-organization-assets-endpoint branch from 855905d to 12a1425 Compare October 31, 2024 22:06
@noliveleger noliveleger force-pushed the task-960-organization-assets-endpoint branch from 12a1425 to 58163fd Compare October 31, 2024 22:13
Copy link

@Akuukis
Copy link
Contributor

Akuukis commented Nov 4, 2024

Few comments according to the new PR workflow:

  • use camelCase for the scope ("feat(Oorganizations): ...")
  • consider splitting such PRs next time, depends on reviewers but they have all the rights to require splitting if this is too hard to review. To me from a quick glance this looks like a bundle of 3 things - feature itself, test refactor, and a linter on unrelated lines.

@noliveleger noliveleger changed the title feat(Organizations): add new asset endpoint for owners and admins TASK-960 feat(organizations): add new asset endpoint for owners and admins TASK-960 Nov 4, 2024
@magicznyleszek
Copy link
Member

magicznyleszek commented Nov 6, 2024

@noliveleger should the /api/v2/<organization_uid>/assets/ in the PR description be /api/v2/organizations/<organization_uid>/assets/ (missing /organizations part)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Changes related to API endpoints Back end
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants