Update katalogus client, input sanitization / validation
Changes
This add's some input validation and sanitization for the string fields for the katalogus name, and plugin id's, as those are used verbatim in the authenticated calls to the katalogus url's .
Issue link
Closes ... TBA
Demo
Please add some proof in the form of screenshots or screen recordings to show (off) new functionality, if there are interesting new features for end-users.
QA notes
Messing around with bogus plugin names or organization names should not be possible. all other functions should remain functional.
Code Checklist
- [ ] All the commits in this PR are properly PGP-signed and verified.
- [ ] This PR only contains functionality relevant to the issue.
- [ ] I have written unit tests for the changes or fixes I made.
- [ ] I have checked the documentation and made changes where necessary.
- [ ] I have performed a self-review of my code and refactored it to the best of my abilities.
- [ ] Tickets have been created for newly discovered issues.
- [ ] For any non-trivial functionality, I have added integration and/or end-to-end tests.
- [ ] I have informed others of any required
.envchanges files if required and changed the.env-distaccordingly. - [ ] I have included comments in the code to elaborate on what is not self-evident from the code itself, including references to issues and discussions online, or implicit behavior of an interface.
Checklist for code reviewers:
Copy-paste the checklist from the docs/source/templates folder into your comment.
Checklist for QA:
Copy-paste the checklist from the docs/source/templates folder into your comment.
In my opinion input validation should be done where input enters the system.
For the organisation code this validation already happens in the model. Everything that does something with an organization must fetch the organisation from the database, so as far as I know it isn't possible to have an organization code in rocky that isn't the correct format because OrganizationView takes care of that. We could improve this by also adding a path converter to the urlpatterns.
For the plugin id we can also easily add the validation in the urlpatterns by using the slug path converter or define our own path converter if the slug path converter is not what we want. For the new create boefje view we should do the validation in the form, so that the error will also be correctly displayed next to the form field.
I agree that we should try and validate as soon as possible. However, we rely on validation of many fields inside the Objects that we instantiate based on all sorts of input. (be it url's, forms, ooi-data etc). We should also try and clamp down on the input in or url-patterns, but it would be naive to only validate in the url's as there are plenty of forms that post a plugin id or organizaton id. The OrganizationView mixing works for now, as we always asume the organizaton to be present in the url, but this is going to change for users who perform actions on more than one organizaton (as is requested as a feature by many of our stakeholders). Tldr, security comes on layers.
As discussed, we should move these validators out of the client (which is not where it should be handled), but onto the, currently non existent, plugin model in Rocky. Next to that, the client should do urlencoding, and the url and form handlers should do input validation.
Quality Gate passed
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
82.1% Coverage on New Code
0.0% Duplication on New Code