fix: fixes parsing of dex passwords with dollar sign#28027
fix: fixes parsing of dex passwords with dollar sign#28027ppapapetrou76 wants to merge 3 commits into
Conversation
🔴 Preview Environment stopped on BunnyshellSee: Environment Details | Pipeline Logs Available commands (reply to this comment):
|
1ac46a3 to
78c0c4f
Compare
Signed-off-by: Patroklos Papapetrou <ppapapetrou76@gmail.com>
78c0c4f to
02a37d0
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #28027 +/- ##
==========================================
+ Coverage 64.49% 64.60% +0.11%
==========================================
Files 422 423 +1
Lines 58106 58244 +138
==========================================
+ Hits 37473 37630 +157
+ Misses 17114 17098 -16
+ Partials 3519 3516 -3 ☔ View full report in Codecov by Sentry. |
| assert.Equal(t, "spaced$$pass", result["bindPW"]) | ||
| }) | ||
|
|
||
| t.Run("non-reference string value is returned as-is", func(t *testing.T) { |
There was a problem hiding this comment.
Is this intentional behaviour here? If a user puts bindPW: "test$test" in dex.config with no $key reference, the usre is still going to hit the original bug.
EDIT: We should try to escape $ for every string in post-substituion dexCfg and not just resolve references.
There was a problem hiding this comment.
Thanks @nitishfy - yes it was intentionally but probably I was wrong 😄
Thanks for catching this - I have updated the PR changes to also escape both references and passwords.
My initial though was that in real world scenarios only secrete references are used and not passwords
Fell free to re-review when you have time
Signed-off-by: Patroklos Papapetrou <ppapapetrou76@gmail.com>
Signed-off-by: Patroklos Papapetrou <ppapapetrou76@gmail.com>
Closes #27803
Addresses a dex password issue when it contains the dollar sign which was leading to login failures - see details in the linked issue
Checklist: