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

A tool for converting miqldap auth to external auth with sssd #15640

Merged
merged 1 commit into from
Sep 18, 2017

Conversation

jvlcek
Copy link
Member

@jvlcek jvlcek commented Jul 24, 2017

MiQ is delivered with the built in LDAP client, MiqLdap. Maintaining an MiQ proprietary LDAP
client is unnecessary as there are multiple mechanisms built into the operating system that provide an interface to an LDAP directory.

This PR provides a tool that will convert an MiQ appliances authentication configuration from
using the MiQ proprietary LDAP client to using SSSD packaged into the operating system.

The user records created in the DB by the MiqLdap client were assigned a userid of either
a Distinguished Name when backed to an LDAP directory or a User Principal Name
when backed to an Active Directory, configured as an LDAP directory. This tool also, optionally,
changes all userids to User Principal Name format.

Related PR, 15535 updates external authentication to always store the user record in the DB with a userid in
User Principal Name format. This tool can be used to convert MiQ authentication
from mode: LDAP(S) to SSSD however until PR 15535 is merged duplicate user records will be created.

Steps for Testing/QA [Optional]

Configure MiQ for both authentication modes of LDAP and LDAPS against both an
LDAP directory and an Active Directory.

Login with a valid user confirm the user record is created in the DB.

Run this tool to convert the authentication mode to external auth with SSSD

Login with the same valid user from step 2 and confirm there is only 1 user record with
a userid in User Principal Name format.

@miq-bot miq-bot added the wip label Jul 24, 2017
@jvlcek jvlcek changed the title [WIP] A tool for converting miqldap auth to external auth with sssd A tool for converting miqldap auth to external auth with sssd Sep 14, 2017
@jvlcek
Copy link
Member Author

jvlcek commented Sep 14, 2017

@miq-bot remove_label wip

@jvlcek
Copy link
Member Author

jvlcek commented Sep 14, 2017

PR 134 is also needed for PR.

@jvlcek
Copy link
Member Author

jvlcek commented Sep 14, 2017

@abellotti, @gtanzillo and @jrafanie I remove WIP.

"Do the MiqLdap to SSSD conversion but skip the normalizing of the userids",
:short => "s",
:default => false,
:type => :flag
Copy link
Member

Choose a reason for hiding this comment

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

In a followup, let's create a separate executable in the bin directory that will invoke this script and allow a user to just run "only_change_userids"

Copy link
Member Author

Choose a reason for hiding this comment

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

@jrafanie Thank you! I've updated PR ManageIQ/manageiq-appliance#134 to include the requested executable script.

class ConfigureSELinux
def configure
LOGGER.debug("Invokded #{self.class}\##{__method__}")
enable_non_standard_ldap_port("10636")
Copy link
Member

Choose a reason for hiding this comment

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

should only enable an ldap port if not 389 and not 636

Copy link
Member

@abellotti abellotti left a comment

Choose a reason for hiding this comment

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

minor change for only enabling selinux for non standard ldap port

@miq-bot
Copy link
Member

miq-bot commented Sep 17, 2017

Some comments on commit jvlcek@26bd19f

spec/tools/miqldap_to_sssd/configure_appliance_settings_spec.rb

  • ⚠️ - 25 - Detected allow_any_instance_of. This RSpec method is highly discouraged, please only use when absolutely necessary.
  • ⚠️ - 26 - Detected allow_any_instance_of. This RSpec method is highly discouraged, please only use when absolutely necessary.

tools/miqldap_to_sssd/configure_apache.rb

  • 💣 💥 🔥 🚒 - 8 - Detected cfme

@miq-bot
Copy link
Member

miq-bot commented Sep 17, 2017

Checked commit jvlcek@26bd19f with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
22 files checked, 1 offense detected

tools/miqldap_to_sssd/configure_database.rb

@jvlcek
Copy link
Member Author

jvlcek commented Sep 17, 2017

@gtanzillo As discussed in the code walkthrough, I've update PR 134 to include a second script, normalize_userid_to_upn, that will allow the userid field of the user records to be normalized to UPN format independently of the conversion from miqldap to sssd.

I've also updated the configuration of selinux to be done only for non-standard ldap ports.

I've performed the following testing for upstream and downstream:

  • Run normalize_userid_to_upn before converting miqldap auth to sssd auth
  • Run normalize_userid_to_upn after converting miqldap auth to sssd auth when the userid's were not updated
  • Run normalize_userid_to_upn after converting miqldap auth to sssd auth when the userid's were updated

I've squashed all of my commits down to 1.

With these changes I believe this PR and PR PR 134 are ready to be merged.

Thank you @abellotti, @gtanzillo and @jrafanie for all the help!

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.

👍 Looks good @jvlcek

@gtanzillo gtanzillo added this to the Sprint 69 Ending Sep 18, 2017 milestone Sep 18, 2017
@gtanzillo gtanzillo merged commit 9dab567 into ManageIQ:master Sep 18, 2017
@abellotti
Copy link
Member

Great work @jvlcek, I KNOW this was tons of research, investigation, coding and testing that you did for all the different configurations. Thank you for this tool !!

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