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

made authenticated user check case insensitive #293

Closed
wants to merge 2 commits into from

Conversation

jlaacke
Copy link
Contributor

@jlaacke jlaacke commented Apr 25, 2018

LDAP login is not case sensitive, so a user may successfully login but cannot be identified when user id in LDAP differs. As a result the user is logged in, but has no profile information (like mail address) and is not assigned to any LDAP groups. Therefore the check has to be case insentive too.

@koevskinikola
Copy link
Member

Hi @jlaacke ,

Thank you for the contribution! I started looking into it.

We are doing a minor release this month and we have more or less defined what it will deliver. So it's possible that your contribution might be skipped for this version and be merged with the next one. I'll keep you updated on our decisions.

However, I noticed that you don't provide a test case to reproduce the problem that you're fixing. Would you be willing to create one?

Best,
Nikola

@jlaacke
Copy link
Contributor Author

jlaacke commented May 3, 2018

Hi Nikola,

thanks for looking at my pull request. ;)

You are absolutely right, I added two test cases. One to proof, that login itself is case insensitive (which is also successful without my fix) and the other to ensure that filtering for logged in user also is case insensitive (which is what I fixed).

Cheers,
Jonas

@koevskinikola
Copy link
Member

Hey Jonas,

That's great! Thank you.

At the moment we can't merge it with our codebase since it has already been decided what will be included/fixed by the minor release at the end of this month.

However, I'll be extremely happy if your contribution is still available for a merge after the release. Would it be a problem for you to wait that long?

Best,
Nikola

@jlaacke
Copy link
Contributor Author

jlaacke commented May 4, 2018

That's absolutely fine, I will keep my custom code just until it's merged. ;)

Cheers,
Jonas

@koevskinikola
Copy link
Member

Hi @jlaacke,

I finally merged your pull request with the master. :)

Thank you for the contribution!

Best,
Nikola

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants