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.

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

OF-2827: JDBCAuthProvider should not lowercase/trim provided username #2461

Conversation

guusdk
Copy link
Member

@guusdk guusdk commented May 17, 2024

By lowercasing the input, a username with a capital letter can't be used.

The closely related JDBCUserProvider does not lowercase/trim values. It is probably best that both providers treat usernames in the same way.

I'm wondering if we even need a configuration option to make this behavior configurable. Isn't it just plain 'wrong' to have inconsistent processing of the username values - that's never going to work, is it?

By lowercasing the input, a username with a capital letter can't be used.

The closely related JDBCUserProvider does not lowercase/trim values. It is probably best that both providers treat usernames in the same way.
@guusdk guusdk requested review from akrherz and Fishbowler May 17, 2024 11:32
@guusdk guusdk added the backport 4.8 on merge, GHA will generate a PR with these changes against 4.8 branch label May 17, 2024
@Fishbowler
Copy link
Member

Seems reasonable to maintain the fidelity of the username. I can see an argument to maintain the trim, but I don't know whether alice is a valid username.

@guusdk
Copy link
Member Author

guusdk commented May 17, 2024

I can see an argument to maintain the trim, but I don't know whether alice is a valid username.

It likely isn't, but shouldn't that then be applied to where the user provides the input (eg: the login form)?

@Fishbowler
Copy link
Member

Exactly that. Let's trust the user.

@guusdk
Copy link
Member Author

guusdk commented May 17, 2024

Hehehe, let's not - but lets apply input validation near the input form maybe?

@guusdk guusdk merged commit 07ecd07 into igniterealtime:main Jun 28, 2024
16 checks passed
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 4.8 on merge, GHA will generate a PR with these changes against 4.8 branch
Projects
None yet
2 participants