Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[v23.2.x] Validation and url_decode for SCRAM usernames #15282

Merged
merged 4 commits into from
Dec 4, 2023
Merged

[v23.2.x] Validation and url_decode for SCRAM usernames #15282

merged 4 commits into from
Dec 4, 2023

Conversation

oleiman
Copy link
Member

@oleiman oleiman commented Dec 4, 2023 •
edited
Loading

Backport of PR #15253

Fixes #15279

@oleiman
admin/security: Add SCRAM username validation to create_user_handler
b8af403 
Includes simple validator against SASLNAME regex in scram_algorithm.cc

(cherry picked from commit 75b0cb6)
@oleiman
scram_test: Add test case for excluded username chars
21666db 
(cherry picked from commit 4350ec2)
@oleiman
admin/security: Unescape characters in username path params
311bf0a 
- `DELETE /v1/security/users/{user}`
- `PUT /v1/security/users/{user}`

(cherry picked from commit 93d575f)
@oleiman
scram_test: Add tests for escaped usernames
9a81162 
- Create usernames with characters that will require URL escaping.
- Verify that these users can be updated/deleted

(cherry picked from commit 8f40bf0)
@oleiman oleiman added this to the v23.2.x-next milestone Dec 4, 2023
@oleiman oleiman added the kind/backport PRs targeting a stable branch label Dec 4, 2023
@oleiman oleiman requested a review from BenPope December 4, 2023 01:03
@oleiman oleiman self-assigned this Dec 4, 2023
BenPope
BenPope approved these changes Dec 4, 2023
View reviewed changes
Copy link
Member

@BenPope BenPope left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The conflict was the already included header in admin_server.cc

@michael-redpanda michael-redpanda marked this pull request as ready for review December 4, 2023 14:36
@oleiman
Copy link
Member Author

oleiman commented Dec 4, 2023

The conflict was the already included header in admin_server.cc

For posterity, the conflict was that these handlers (and others) moved admin_server.cc --> admin/security.cc. Generally pretty clean though.

@oleiman
Copy link
Member Author

oleiman commented Dec 4, 2023

CI Failure: https://buildkite.com/redpanda/redpanda/builds/42194#018c35cb-b464-4234-9445-02c7389028dc

Doesn't look related (and only failed the release build), but I'm going to re-run ducktape to err on the safe side.

@oleiman oleiman merged commit 46e9283 into redpanda-data:v23.2.x Dec 4, 2023
25 checks passed
@oleiman oleiman mentioned this pull request Dec 5, 2023
Closed
@piyushredpanda piyushredpanda modified the milestones: v23.2.x-next, v23.2.18 Dec 7, 2023
@github-actions github-actions bot mentioned this pull request Dec 13, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BenPope BenPope BenPope approved these changes

@michael-redpanda michael-redpanda Awaiting requested review from michael-redpanda

Assignees

@oleiman oleiman

Labels
area/redpanda kind/backport PRs targeting a stable branch
Projects
None yet
Milestone
v23.2.18
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@oleiman @BenPope @piyushredpanda