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

Validation and url_decode for SCRAM usernames #15253

Merged
merged 4 commits into from
Dec 3, 2023
Merged

Validation and url_decode for SCRAM usernames #15253

merged 4 commits into from
Dec 3, 2023

Conversation

oleiman
Copy link
Member

@oleiman oleiman commented Dec 1, 2023

This PR introduces a simple validator for SCRAM usernames (applied on the create_user path) and url decoding for usernames on the delete_user and update_user paths, along with associated tests.

Fixes #14034

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v23.2.x
  • v23.1.x
  • v22.3.x

Release Notes

Bug Fixes

  • Fixes an issue where lookup would fail for URL encoded username parameter (DELETE/PUT /v1/security/users/{user})

@oleiman oleiman self-assigned this Dec 1, 2023
@vbotbuildovich
Copy link
Collaborator

new failures in https://buildkite.com/redpanda/redpanda/builds/42099#018c23d9-e78e-47fe-aa2d-510aa048f508:

"rptest.tests.topic_delete_test.TopicDeleteCloudStorageTest.topic_delete_cloud_storage_test.disable_delete=False.cloud_storage_type=CloudStorageType.S3"

@oleiman
Copy link
Member Author

oleiman commented Dec 1, 2023

Note to reviewers: I may have badly abused some terminology here, so please keep an eye out for that!

michael-redpanda
michael-redpanda reviewed Dec 1, 2023
View reviewed changes
Copy link
Contributor

@michael-redpanda michael-redpanda left a comment

Choose a reason for hiding this comment

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

🔥

src/v/security/scram_algorithm.cc Outdated Show resolved Hide resolved
src/v/security/scram_algorithm.cc Outdated Show resolved Hide resolved
@oleiman
admin/security: Add SCRAM username validation to create_user_handler
75b0cb6 
Includes simple validator against SASLNAME regex in scram_algorithm.cc
@oleiman
scram_test: Add test case for excluded username chars
4350ec2
@oleiman
admin/security: Unescape characters in username path params
93d575f 
- `DELETE /v1/security/users/{user}`
- `PUT /v1/security/users/{user}`
@oleiman
scram_test: Add tests for escaped usernames
8f40bf0 
- Create usernames with characters that will require URL escaping.
- Verify that these users can be updated/deleted
@oleiman oleiman force-pushed the bug/14034/del-user-param-unescape branch from 39575f3 to 8f40bf0 Compare December 1, 2023 19:53
@oleiman
Copy link
Member Author

oleiman commented Dec 1, 2023

force push to remove extraneous struct saslname_match.

michael-redpanda
michael-redpanda approved these changes Dec 1, 2023
View reviewed changes
Copy link
Contributor

@michael-redpanda michael-redpanda left a comment

Choose a reason for hiding this comment

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

lgtm

@oleiman
Copy link
Member Author

oleiman commented Dec 2, 2023

/ci-repeat 1

@vbotbuildovich
Copy link
Collaborator

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/42155#018c281c-d3c3-4c5e-84ab-e009ab568868

@oleiman oleiman merged commit ac725d5 into redpanda-data:dev Dec 3, 2023
20 checks passed
@vbotbuildovich
Copy link
Collaborator

/backport v23.2.x

@vbotbuildovich
Copy link
Collaborator

/backport v23.1.x

@vbotbuildovich
Copy link
Collaborator

/backport v22.3.x

@vbotbuildovich
Copy link
Collaborator

Failed to create a backport PR to v23.2.x branch. I tried:

git remote add upstream https://github.com/redpanda-data/redpanda.git
git fetch --all
git checkout -b backport-pr-15253-v23.2.x-718 remotes/upstream/v23.2.x
git cherry-pick -x 75b0cb6bf4ea12e0574e014e89c143e3fd41c105 4350ec2cd1259c5bf26d0d08a71c165217e8ab64 93d575f427f38e2af86d4bd3a6f91b8369e9ebd2 8f40bf0c272d495bd865774ab2d2cd3d4a05aa72

Workflow run logs.

@vbotbuildovich vbotbuildovich mentioned this pull request Dec 3, 2023
Closed
@vbotbuildovich
Copy link
Collaborator

Failed to create a backport PR to v23.1.x branch. I tried:

git remote add upstream https://github.com/redpanda-data/redpanda.git
git fetch --all
git checkout -b backport-pr-15253-v23.1.x-428 remotes/upstream/v23.1.x
git cherry-pick -x 75b0cb6bf4ea12e0574e014e89c143e3fd41c105 4350ec2cd1259c5bf26d0d08a71c165217e8ab64 93d575f427f38e2af86d4bd3a6f91b8369e9ebd2 8f40bf0c272d495bd865774ab2d2cd3d4a05aa72

Workflow run logs.

@vbotbuildovich
Copy link
Collaborator

Failed to create a backport PR to v22.3.x branch. I tried:

git remote add upstream https://github.com/redpanda-data/redpanda.git
git fetch --all
git checkout -b backport-pr-15253-v22.3.x-514 remotes/upstream/v22.3.x
git cherry-pick -x 75b0cb6bf4ea12e0574e014e89c143e3fd41c105 4350ec2cd1259c5bf26d0d08a71c165217e8ab64 93d575f427f38e2af86d4bd3a6f91b8369e9ebd2 8f40bf0c272d495bd865774ab2d2cd3d4a05aa72

Workflow run logs.

This was referenced Dec 3, 2023
Closed
Closed
This was referenced Dec 4, 2023
Merged
Merged
Closed
@michael-redpanda michael-redpanda mentioned this pull request Jan 17, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michael-redpanda michael-redpanda michael-redpanda approved these changes

@BenPope BenPope Awaiting requested review from BenPope

Assignees

@oleiman oleiman

Labels
area/redpanda
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Admin-API: URL Parameters must be unescaped
3 participants
@oleiman @vbotbuildovich @michael-redpanda