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

Always check for userid as UPN #16069

Merged

Conversation

jvlcek
Copy link
Member

@jvlcek jvlcek commented Sep 28, 2017

Always check for userid in UPN format, even when "Get User Groups
from External Authentication (httpd)" is not checked

https://bugzilla.redhat.com/show_bug.cgi?id=1496979

The recent changes to the authentication code space to normalize userid to UPN format
resulted in an edge case failure.

A user entry in the database would not be found if the userid is in UPN format when
"Get User Groups from External Authentication (httpd)" is not checked

This PR addresses this edge case.

@jvlcek
Copy link
Member Author

jvlcek commented Sep 28, 2017

@abellotti I discovered this edge case while rerunning some miqldap_to_sssd test scenarios while documenting it. Please take a look. Thank you!

Always check for userid in UPN format, even when "Get User Groups
from External Authentication (httpd)" is not checked

https://bugzilla.redhat.com/show_bug.cgi?id=1496979
@jvlcek jvlcek force-pushed the bz1496979_ext_auth_no_get_groups_upn branch from 1f321f3 to 2ba8f0c Compare September 29, 2017 14:47
@miq-bot
Copy link
Member

miq-bot commented Sep 29, 2017

Checked commit jvlcek@2ba8f0c with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
3 files checked, 0 offenses detected
Everything looks fine. ⭐

@abellotti
Copy link
Member

LGTM!! @gtanzillo can we borrow your 👀 ? Thanks.

one concern was the line https://github.com/ManageIQ/manageiq/pull/16069/files#diff-5214aab14508eac8c281a77da26e8cd2R94
note, this was not changed here so not an issue in this PR. General concern was the second % wildcard, in case there's both a cn=jvlcek,ou=people,dc=example,dc=com and cn=jvlcek,ou=people,ou=engineering,dc=example,dc=com, they may be 2 different users sharing the same rdn, the code may match the wrong one. Something to look for, verify scenario with the sssd_to_ldap conversion script.

Copy link
Member

@gtanzillo gtanzillo left a comment

Choose a reason for hiding this comment

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

LGTM too 👍

@gtanzillo gtanzillo added this to the Sprint 71 Ending Oct 16, 2017 milestone Oct 4, 2017
@gtanzillo gtanzillo merged commit d5edc2f into ManageIQ:master Oct 4, 2017
@gtanzillo gtanzillo added the bug label Oct 17, 2017
@jvlcek jvlcek deleted the bz1496979_ext_auth_no_get_groups_upn branch November 10, 2017 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants