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.

Sign up for GitHub

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

Jump to bottom

Fix crud operations permission in update relationships #300

Merged
merged 11 commits into from
Aug 21, 2023
Merged

Fix crud operations permission in update relationships #300

merged 11 commits into from
Aug 21, 2023

Conversation

fanatic75
Copy link
Contributor

@fanatic75 fanatic75 commented Jul 31, 2023 •
edited
Loading

  • Skipping authorization for crud operations which are integral parts of logic but not done by the user
  • Added a test case.

@fanatic75
fixes comparison check and permission issues in relationships
89b111c 
Skipping authorization for the db calls that are only made for updating document logic but not required by user. Fixes the bug in comparing new document and old document in updateDocument method. Adds a test case to verify.
@fanatic75 fanatic75 requested review from stnguyen90 and abnegate July 31, 2023 20:13
@github-actions github-actions bot mentioned this pull request Aug 1, 2023
Closed
stnguyen90
stnguyen90 requested changes Aug 2, 2023
View reviewed changes
src/Database/Database.php Outdated Show resolved Hide resolved
src/Database/Database.php Outdated Show resolved Hide resolved
@fanatic75 fanatic75 requested a review from stnguyen90 August 2, 2023 06:36
@fanatic75 fanatic75 marked this pull request as draft August 10, 2023 10:59
stnguyen90
stnguyen90 reviewed Aug 17, 2023
View reviewed changes
Copy link
Contributor

@stnguyen90 stnguyen90 left a comment

Choose a reason for hiding this comment

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

Is this ready for review? Also, it looks like there are merge conflicts.

@abnegate
Copy link
Member

Closing as for now we don't want to allow relating to documents without read permission

@abnegate abnegate closed this Aug 18, 2023
@fanatic75 fanatic75 reopened this Aug 18, 2023
@fanatic75 fanatic75 marked this pull request as ready for review August 18, 2023 15:52
@fanatic75 fanatic75 requested a review from stnguyen90 August 18, 2023 15:53
@fanatic75
Copy link
Contributor Author

@abnegate
I have removed the ability to skip authentication when fetching related documents. However, we still need to perform certain operations, such as updating removed documents from the relation without authentication.

Let's consider this scenario: Level 1 has a one-to-many relationship with Level 2. If we create new documents in Level 2 using Level 1, we should not need update permission in Level 2. However, due to updating old documents, we are unable to create new documents without update permission. This pull request resolves this issue.

abnegate
abnegate reviewed Aug 21, 2023
View reviewed changes
src/Database/Database.php Outdated Show resolved Hide resolved
tests/Database/Base.php Outdated Show resolved Hide resolved
@abnegate abnegate requested review from stnguyen90 and removed request for stnguyen90 August 21, 2023 19:10
@abnegate abnegate merged commit 7f10223 into main Aug 21, 2023
@github-actions github-actions bot mentioned this pull request Sep 1, 2023
Closed
@abnegate abnegate deleted the fix-crud-operations-permission-in-update-relationships branch November 23, 2023 01:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abnegate abnegate abnegate approved these changes

@stnguyen90 stnguyen90 stnguyen90 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fanatic75 @abnegate @stnguyen90