Better validation of notification orders

Altinn / altinn-notifications

Altinn platform microservice for handling notifications (mail, sms, etc)

MIT License

3 stars 3 forks source link

Better validation of notification orders #474

Closed martinothamar closed 4 months ago

martinothamar commented 5 months ago

Description

Implements validation of more order fields based on constraints from downstream providers. I came across this since I was wondering what the input constraints was when developing the SMS client in apps

Related Issue(s)

N/A

Verification

[ ] Your code builds clean without any errors or warnings
[ ] Manual testing done (required)
[ ] Relevant automated test added (if you find this hard, leave it and we'll help out)
[ ] All tests run green

Documentation

[ ] User documentation is updated with a separate linked PR in altinn-studio-docs. (if applicable)

martinothamar commented 5 months ago

Btw I had to comment the line endings config in .editorconfig to avoid a huge diff. My editor is trying to adhere to line endings config but the actual files I touched are LF it seems

martinothamar commented 5 months ago

@acn-sbuad since this would be a breaking change I'm not sure if this is something you want. But I think a change like this is good because

Users get bad request errors early (as opposed to only noticing when reading the status, I'm assuming errors due to service limits would appear somewhere in the status/delivery reports).
Unbounded input is not a good idea in terms of security, upstream applications could affect service availability and performance (either due to bugs or security issues) I think

If we do something like this, we should document the limits as well