Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[API] Using isAccessibleBy on candidate endpoint #8824

Merged

Conversation

xlecours
Copy link
Contributor

@xlecours xlecours commented Jul 4, 2023

No description provided.

@xlecours xlecours added the Security PR patches a vulnerability, makes resource access changes, or updates dependencies label Jul 4, 2023
Copy link
Collaborator

@ridz1208 ridz1208 left a comment

Choose a reason for hiding this comment

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

Approving although I realize now that it may be redundant (still not a bad idea though) because all subendpoints (visits) already have the isaccessible by and both hendleGET and handlePUT check for user sites and projects.

I think it's the ad-hoc locations of the site/project checks that is a bit confusing.

Either way the redundancy is welcome here

@driusan
Copy link
Collaborator

driusan commented Jul 4, 2023

1) LorisApiVisitsTest::testPutCandidatesCandidVisit
Failed asserting that 404 matches expected 403.

@xlecours
Copy link
Contributor Author

xlecours commented Jul 4, 2023

interesting. I'll change /candidate/$candid

@xlecours
Copy link
Contributor Author

xlecours commented Jul 4, 2023

@driusan passing

@driusan driusan merged commit f8f123d into aces:24.1-release Jul 4, 2023
9 checks passed
@ridz1208 ridz1208 added this to the 24.1.5 milestone Jul 10, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Security PR patches a vulnerability, makes resource access changes, or updates dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants