Skip to content

LDAPImport start_tls#213

Closed
wheldom01 wants to merge 1 commit intobestpractical:stablefrom
wheldom01:4.4/enable-tls-in-ldapimport
Closed

LDAPImport start_tls#213
wheldom01 wants to merge 1 commit intobestpractical:stablefrom
wheldom01:4.4/enable-tls-in-ldapimport

Conversation

@wheldom01
Copy link
Contributor

Patch adds support for the use of Net::LDAP start_tls to the LDAPImport.

@sunnavy
Copy link
Member

sunnavy commented Feb 7, 2018

Sorry for the late response.

I have 2 main concerns:

  • it looks like Net::LDAP::start_tls accepts hash instead of hashref

but in your code:

$msg = $ldap->start_tls( $RT::LDAPTLS );

I'm wondering if it works.

  • $msg returned by start_tls isn't checked

We probably need to check it to see if there are any errors, just like the way we check $ldap->bind.

Thanks!

@wheldom01
Copy link
Contributor Author

wheldom01 commented Feb 8, 2018 via email

@tobiasbp
Copy link

It would be great if this could be completed and merged in. Thanks.

@tobiasbp
Copy link

Is this change compatible with RT 4.4.4? This is the message I receive when running it:
Unable to run rt-ldapimport without dependencies. Rerun configure with the --enable-externalauth option

@wheldom01
Copy link
Contributor Author

wheldom01 commented Sep 13, 2019 via email

@wheldom01
Copy link
Contributor Author

wheldom01 commented Sep 13, 2019 via email

@cbrandtbuffalo
Copy link
Member

In 4c288fc, we ended up modifying LDAPImport to allow passing any arguments supported by Net::LDAP, so this is now covered. We also added TLS options to the example so it's clear how to configure a secure connection. This is available in RT 4.4.4 and newer, including RT 5.

Thanks for opening a PR and suggesting the improvement!

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.

4 participants