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

Don't convert user-type email to upn #19515

Merged
merged 2 commits into from
Nov 20, 2019

Conversation

jvlcek
Copy link
Member

@jvlcek jvlcek commented Nov 14, 2019

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

Summary:
Fix user_type==mail support in MiqLdap#get_user_object

Details:

The user_type is not always set, requiring MiqLdap#get_user_object to attempt to determine the user_type based on the format of the user_name

The provider VM provisioning screens in the UI provide for the ability to lookup a user by email address in the Idp. The MiqLdap client was erroneously converting the user_type to User Principle Name (UPN) when the username format matched string@string

This PR addresses this issue by:
1 - Explicitly specifying the user-type of mail in MiqRequestWorkflow#retrieve_ldap, which is used by the UI when doing user lookup by email.
2 - Ensuring the user_type is not set to UPN when user_type is mail.

@jvlcek
Copy link
Member Author

jvlcek commented Nov 14, 2019

@miq-bot add_label wip

@jvlcek
Copy link
Member Author

jvlcek commented Nov 14, 2019

@miq-bot add_label bug
@miq-bot add_label authentication

@Fryguy
Copy link
Member

Fryguy commented Nov 15, 2019

@jvlcek You have a bad BZ link in the commit and in the OP comment here.

@jvlcek jvlcek force-pushed the bz1769517_provision_user_mail branch from f40a8fc to e46d92f Compare November 18, 2019 12:20
@jvlcek
Copy link
Member Author

jvlcek commented Nov 18, 2019

Thank you for pointing that out @Fryguy. It should be correct now.

@jvlcek jvlcek force-pushed the bz1769517_provision_user_mail branch from e46d92f to d3fa94b Compare November 18, 2019 19:57
@miq-bot
Copy link
Member

miq-bot commented Nov 18, 2019

Checked commits jvlcek/manageiq@d3fa94b~...8607fae with ruby 2.5.5, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
3 files checked, 1 offense detected

app/models/miq_request_workflow.rb

@chessbyte
Copy link
Member

@jvlcek The BZ link above is probably wrong and is unrelated to ManageIQ

@jvlcek
Copy link
Member Author

jvlcek commented Nov 19, 2019

@chessbyte Thank you. I had fixed that once, not sure how my fix got reverted.

@jvlcek jvlcek changed the title [WIP] Don't convert user-type email to upn Don't convert user-type email to upn Nov 19, 2019
@jvlcek
Copy link
Member Author

jvlcek commented Nov 19, 2019

@miq-bot remove_label wip

@miq-bot miq-bot removed the wip label Nov 19, 2019
@jvlcek
Copy link
Member Author

jvlcek commented Nov 19, 2019

I've tested end-to-end against OpenLdap and Active Directory.

@jvlcek
Copy link
Member Author

jvlcek commented Nov 19, 2019

@gtanzillo Please review.

@jvlcek
Copy link
Member Author

jvlcek commented Nov 19, 2019

@miq-bot assign @abellotti

@jvlcek
Copy link
Member Author

jvlcek commented Nov 19, 2019

@hstastna FYI
Thank you for the help with this. Turns out no UI changes will be necessary.

@gtanzillo
Copy link
Member

@abellotti This looks good to me. Do you want to give it a look before I merge?

@gtanzillo gtanzillo added this to the Sprint 125 Ending Nov 25, 2019 milestone Nov 20, 2019
@gtanzillo gtanzillo merged commit 3fa0b49 into ManageIQ:master Nov 20, 2019
@jvlcek jvlcek deleted the bz1769517_provision_user_mail branch January 24, 2020 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants