-
Notifications
You must be signed in to change notification settings - Fork 0
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
fix: Correctly encode user given content, such as passwords #571
Conversation
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.
Can you please also try to break the integration tests by setting as many weird characters in e.g. ldap bind credentials or internal passwords?
running a test 🤞 |
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.
Thanks! LGTM when tests pass
Just a meta comment: product-utils is coming to an release soon-ish. I think it would make sense to use that tool everywhere for consistency reasons [and not rely on Druid features that might do thing slightly differently]. |
You might be right there. I can also see an argument for using less parts, but maybe the consistency is better to have here. Happy to have other opinions, maybe @soenkeliebau |
b09971e
to
36f3169
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.
lgtm
Description
fixes #219
Integration tests were successful: https://ci.stackable.tech/view/06%20Replicated/job/druid-operator-it-replicated/8/console
Definition of Done Checklist
Author
Reviewer
Acceptance