Fix accessing public datasets #8242
Conversation
📝 WalkthroughWalkthroughThe changes in this pull request primarily focus on the Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
app/controllers/DatasetController.scala (1)
Line range hint
479-487: LGTM! Comprehensive validation for segment anything mask parameters.The validation checks are well-implemented:
- Size constraints prevent resource exhaustion
- Equal long edges requirement ensures proper image processing
- Maximum depth limit of 16 maintains performance
Consider extracting these validation constants into a configuration object for easier maintenance and reuse. For example:
object SegmentAnythingConfig { val MaxSize = 1024 val MaxDepth = 16 // ... other constants }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
app/controllers/DatasetController.scala(1 hunks)
🔇 Additional comments (2)
app/controllers/DatasetController.scala (2)
Line range hint 488-495: LGTM! Well-structured validation for interaction parameters.
The validation logic correctly ensures that:
- For bounding box interactions: all required coordinates are provided
- For point interactions: X and Y coordinates are provided
267-267: LGTM! The fix correctly addresses the public dataset access regression.
Using GlobalAccessContext for organization retrieval is appropriate here since the dataset's public status has already been verified via datasetDAO.findOne(datasetIdValidated)(ctx).
Let's verify that this change doesn't affect private dataset access:
✅ Verification successful
Based on the search results, I can now generate the final response:
Using GlobalAccessContext for organization lookup is consistent with the codebase patterns
The usage of GlobalAccessContext when retrieving organization information is a well-established pattern across the codebase, particularly in scenarios involving public access or basic entity information retrieval. This includes:
- Dataset-related operations in
DatasetService.scala - Team information retrieval in
Team.scala - User profile data in
UserService.scala - Authentication flows in
AuthenticationController.scala
The change aligns with existing practices and correctly implements public dataset access while maintaining security for private resources through the earlier dataset access check.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other instances where organization access is checked
rg "organizationDAO.findOne.*GlobalAccessContext" -A 5
Length of output: 22790
Regression from #8075 – Public datasets could not be accessed without user credentials.
I tested locally that this helps. Looking up the organization bypassing its read access check here is fine, since we already checked that the dataset is public in the line above.
Let’s also test this. #8243