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

Fix permission issues #155

Open
wants to merge 16 commits into
base: dev
Choose a base branch
from
Open

Fix permission issues #155

wants to merge 16 commits into from

Conversation

yld-weng
Copy link
Collaborator

This PR primarily addresses permission issues and includes some basic documentation.

Related issues: #93, #78, and #102 (fixed).

"""Get user's highest role across project's organisations"""
"""
Get user's highest role across project's organisations
TODO: this assuming a project can be linked to multiple organisations, check if still the case
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is this still true?

Copy link
Member

Choose a reason for hiding this comment

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

@yld-weng Thanks for this Yuliang. From my understanding, a project should only be linked to one org. But the PI (and their team) should have access to all projects/orgs who agree to share their data with us.

Copy link
Member

Choose a reason for hiding this comment

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

Also just to clarify - all managers in a given organisation should have access to all of the org's projects.

@@ -63,7 +66,7 @@ def can_edit(self, user: User, project: Project) -> bool:
if role == ROLE_ADMIN:
return True
elif role == ROLE_PROJECT_MANAGER:
permission = self.get_user_permission(project, user)
permission = self.get_user_permission(user, project)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fix for #102.

Copy link
Member

@f-allian f-allian left a comment

Choose a reason for hiding this comment

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

@yld-weng Apart from my comment above re. the PI's admin access, everything else looks good. Thanks

@f-allian f-allian added the enhancement New feature or request label Feb 27, 2025
Copy link
Contributor

@twinkarma twinkarma left a comment

Choose a reason for hiding this comment

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

The can_create in BasePermissionService should also have an additional parameter. In order to be able to create a project, you have to check if the user belongs to the organisation they are trying to create the project in, same for creating a survey.

@yld-weng
Copy link
Collaborator Author

yld-weng commented Mar 7, 2025

I have removed ProjectOrganisation and ProjectManagerPermission as a project should only be linked to one organisation.

@yld-weng yld-weng requested a review from twinkarma March 7, 2025 18:07
@twinkarma
Copy link
Contributor

Made a PR to this branch in #166 to address various issues and add proper permissions to the survey service

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants