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

ldap: Add client certificate support #6668

Merged
merged 8 commits into from
Jun 15, 2023

Conversation

Gnonthgol
Copy link
Contributor

SUMMARY

Add new arguments client_cert and client_key to the ldap modules in order to allow certificate authentication.

Fixes #4066

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

plugins/module_utils/ldap.py

ADDITIONAL INFORMATION

The functional part of the code is copied from work by @andrewshulgin with tests added by myself.

@ansibullbot ansibullbot added WIP Work in progress docs_fragments docs_fragments plugin (shared docs) feature This issue/PR relates to a feature request integration tests/integration module_utils module_utils new_contributor Help guide this first time contributor plugins plugin (any type) tests tests labels Jun 10, 2023
@Gnonthgol Gnonthgol marked this pull request as ready for review June 10, 2023 15:45
@ansibullbot ansibullbot removed the WIP Work in progress label Jun 10, 2023
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!

Please also modify the modules so that they pass required_together=[('client_cert', 'client_key')] to AnsibleModule(). That way the argument spec validation ensures that the user gets a nice error message if they only provide one of the two.

@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-7 labels Jun 10, 2023
@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added the module module label Jun 10, 2023
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

Will merge if nobody objects in ~a week.

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Jun 15, 2023
@felixfontein felixfontein merged commit f3ecf4c into ansible-collections:main Jun 15, 2023
@patchback
Copy link

patchback bot commented Jun 15, 2023

Backport to stable-7: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-7/f3ecf4c7f8041f0653ca9b52b65452bbd899f05a/pr-6668

Backported as #6696

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Jun 15, 2023
* Set up secure ldap server

* ldap: Added client cert options

Shamelessly copied from https://github.com/andrewshulgin/ldap_search

* Added tests for ldap client authentication

* Add changelog fragment

* Make sure the openssl commands work on older versions of openssl

* Apply suggestions from code review

Co-authored-by: Felix Fontein <[email protected]>

* Remove aliases for new arguments

* Add required_together to ldap module declerations

---------

Co-authored-by: Felix Fontein <[email protected]>
(cherry picked from commit f3ecf4c)
@felixfontein
Copy link
Collaborator

@Gnonthgol thanks for implementing this!

felixfontein pushed a commit that referenced this pull request Jun 15, 2023
…upport (#6696)

ldap: Add client certificate support (#6668)

* Set up secure ldap server

* ldap: Added client cert options

Shamelessly copied from https://github.com/andrewshulgin/ldap_search

* Added tests for ldap client authentication

* Add changelog fragment

* Make sure the openssl commands work on older versions of openssl

* Apply suggestions from code review

Co-authored-by: Felix Fontein <[email protected]>

* Remove aliases for new arguments

* Add required_together to ldap module declerations

---------

Co-authored-by: Felix Fontein <[email protected]>
(cherry picked from commit f3ecf4c)

Co-authored-by: Gnonthgol <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs_fragments docs_fragments plugin (shared docs) feature This issue/PR relates to a feature request integration tests/integration module_utils module_utils module module new_contributor Help guide this first time contributor plugins plugin (any type) tests tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ldap_search: Client Certificate Support
3 participants