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

Check for permissions in rel add/delete mutations #5772

Merged
merged 4 commits into from
Feb 18, 2025

Conversation

gmazoyer
Copy link
Contributor

This should patch a security issue where user could add relationships between nodes without going through the permission check process.

In the add/delete relationship mutations, with this change, we identify the kinds of nodes that are involved in the mutations and infer what global and object permissions are required to mutate the nodes.

@github-actions github-actions bot added the group/backend Issue related to the backend (API Server, Git Agent) label Feb 17, 2025
Copy link

codspeed-hq bot commented Feb 17, 2025

CodSpeed Performance Report

Merging #5772 will not alter performance

Comparing gma-20250217-relmutation-permissions (127462d) with stable (a663a5f)

Summary

✅ 11 untouched benchmarks

@gmazoyer gmazoyer requested a review from a team February 17, 2025 20:22
@gmazoyer gmazoyer marked this pull request as ready for review February 17, 2025 20:25
Copy link
Contributor

@ajtmccarty ajtmccarty left a comment

Choose a reason for hiding this comment

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

I'm not sure how hard it is to write a test for this, but it would be good to have a couple
maybe it can fit in with the existing permissions tests?

Copy link
Contributor

@ogenstad ogenstad left a comment

Choose a reason for hiding this comment

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

LGTM, as Aaron says it would be good with some tests in place. If we don't have that infrastructure in place I think things might fail when we bring this back to the develop branch as some of the tests there would be setting an account_session within the context to get the event stuff to work.

@gmazoyer gmazoyer force-pushed the gma-20250217-relmutation-permissions branch from 3dc4af7 to 0445e5b Compare February 18, 2025 11:08
@gmazoyer gmazoyer force-pushed the gma-20250217-relmutation-permissions branch from 0445e5b to 127462d Compare February 18, 2025 12:53
@gmazoyer gmazoyer merged commit 8b64597 into stable Feb 18, 2025
33 checks passed
@gmazoyer gmazoyer deleted the gma-20250217-relmutation-permissions branch February 18, 2025 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
group/backend Issue related to the backend (API Server, Git Agent)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants