-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Bug/70979 api v3 allows reading writing internal comments when the enable internal comments project setting is disabled #21800
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
Bug/70979 api v3 allows reading writing internal comments when the enable internal comments project setting is disabled #21800
Conversation
The API v3 was allowing reading/writing internal comments when both the enterprise token doesn't allow it and when the project setting is disabled. Reading (GET): - Added EnterpriseToken check to internal_visible scope Writing (POST): - Added project.enabled_internal_comments check to CreateNoteContract
fdc3236 to
3adc142
Compare
3adc142 to
80b111d
Compare
Adds `GET /api/v3/projects/:id/configuration` endpoint that returns all global configuration properties plus project-specific settings. This allows client apps to check both enterprise token features (availableFeatures) and project settings (enabledInternalComments) in a single API call.
80b111d to
8dbac61
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request fixes a security vulnerability where the API v3 allowed reading and writing internal comments even when the enabled_internal_comments project setting was disabled or when the Enterprise Token doesn't support the feature.
Changes:
- Added enterprise token check to the
internal_visiblescope inWorkPackage::Journalizedto prevent reading internal comments when the token doesn't allow it - Added project setting validation to
CreateNoteContractto prevent creating internal comments when disabled at the project level - Introduced a new
/api/v3/projects/:id/configurationendpoint that returns global configuration plus project-specific settings likeenabledInternalComments
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| app/models/work_package/journalized.rb | Added EnterpriseToken check to internal_visible scope to enforce authorization when reading internal comments |
| app/contracts/work_packages/create_note_contract.rb | Added project setting validation to prevent creating internal comments when disabled for the project |
| lib/api/v3/projects/configuration/project_configuration_api.rb | New API endpoint for project-scoped configuration |
| lib/api/v3/projects/configuration/project_configuration_representer.rb | Representer for project configuration extending global configuration with project settings |
| lib/api/v3/workspaces/nested_apis.rb | Mounted ProjectConfigurationAPI under workspaces |
| lib/api/v3/utilities/path_helper.rb | Added path helper methods for project and workspace configuration endpoints |
| docs/api/apiv3/paths/project_configuration.yml | OpenAPI documentation for the new project configuration endpoint |
| docs/api/apiv3/components/schemas/project_configuration_model.yml | Schema definition for project configuration model |
| docs/api/apiv3/openapi-spec.yml | Updated OpenAPI spec to include new project configuration endpoint and schema |
| config/locales/en.yml | Added translation for feature_disabled_for_project error message |
| spec/requests/api/v3/projects/configuration/project_configuration_resource_spec.rb | Comprehensive tests for the new configuration API endpoint |
| spec/requests/api/v3/activities_by_work_package_resource_spec.rb | Added test for creating internal comments when project setting is disabled |
| spec/contracts/work_packages/create_note_contract_spec.rb | Added test for project setting validation in contract |
| spec/models/work_package/work_package_acts_as_journalized_spec.rb | Updated tests to use with_ee flag and verify enterprise token enforcement |
| spec/services/work_packages/activities_tab/paginator_spec.rb | Updated test to use with_ee flag for internal comments |
| spec/requests/api/v3/emoji_reactions/emoji_reactions_by_work_package_comments_api_spec.rb | Updated test to use with_ee flag |
| spec/models/concerns/emoji_reactions/grouped_queries_spec.rb | Updated tests to use with_ee flags |
| spec/helpers/work_packages_helper_spec.rb | Updated test to use with_ee flag |
| spec/features/activities/work_package/activities_spec.rb | Updated test suite to use with_ee flag |
| spec/controllers/work_packages_controller_spec.rb | Updated test to use with_ee flag |
spec/requests/api/v3/projects/configuration/project_configuration_resource_spec.rb
Show resolved
Hide resolved
spec/requests/api/v3/projects/configuration/project_configuration_resource_spec.rb
Outdated
Show resolved
Hide resolved
|
@copilot open a new pull request to apply changes based on the comments in this thread |
e9928c4 to
4ca0236
Compare
94f9e7f to
1bca833
Compare
judithroth
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! I have only one small question about the documentation
Ticket
https://community.openproject.org/wp/70979
What are you trying to accomplish?
Fix internal comments authorization bug where the API v3 allows reading/writing internal comments when the enabled_internal_comments project setting is disabled or when the EnterpriseToken doesn't allow it.
What approach did you choose and why?
Enforcement Layer: Added the missing checks at the appropriate layers:
Configuration API: Extended the existing Configuration API with a project-scoped endpoint rather than modifying the Capabilities API because:
Merge checklist