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 showAll toggle and relax saving constraints #2

Merged
merged 2 commits into from
Jul 25, 2024
Merged

Fix showAll toggle and relax saving constraints #2

merged 2 commits into from
Jul 25, 2024

Conversation

szymonrw
Copy link
Collaborator

@szymonrw szymonrw commented Jul 24, 2024

Two fixes:

  1. Fix page decreasing when toggling the "show all" checkbox added previously

  2. Relax saving constraints

    So far making changes to other user's assignments was only possible with allow_all_users flag. This flag is not very safe though because anyone with an account could enumerate predictable data ids. Explicit list of people for each project is safer because each new user needs to be manually added.

    But on the other hand, we want to allow correcting other's work within a given project. For that we can relax the permission check slightly. Few lines above the removed part there's already a check whether the user is part of the project and this should be enough for our purposes.

Previous check already checks if the user is assigned to the project which should be good enough for our case.
@szymonrw szymonrw self-assigned this Jul 24, 2024
@szymonrw szymonrw marked this pull request as ready for review July 25, 2024 02:46
@szymonrw szymonrw requested a review from bearice July 25, 2024 02:46
@bearice
Copy link

bearice commented Jul 25, 2024

lgtm

@bearice bearice merged commit d104491 into master Jul 25, 2024
2 checks passed
@szymonrw szymonrw deleted the fixes branch July 25, 2024 03:14
@szymonrw
Copy link
Collaborator Author

Thank you for checking!

@bearice
Copy link

bearice commented Jul 25, 2024

deployed

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.

2 participants