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

feat: add ldap user/dn/attribute/filter #416

Merged
merged 2 commits into from
Nov 28, 2024

Conversation

d1nuc0m
Copy link
Contributor

@d1nuc0m d1nuc0m commented Nov 26, 2024

Pull Request (PR) description

Allow use of Require ldap-user/ldap-dn/ldap-attribute/ldap-filter in Apache config.
Compatible with pre-existing ldap-group settings.
See mod_authnz_ldap docs

Comment on lines 39 to 42
Optional[String] $ldap_require_user = undef,
Optional[String] $ldap_require_dn = undef,
Optional[String] $ldap_require_attribute = undef,
Optional[String] $ldap_require_filter = undef,
Copy link
Member

Choose a reason for hiding this comment

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

Are empty strings allowed here, or should String be replaced with String[1]? Or are there even more strict datatypes we could use?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, empty strings are not allowed - about more restrictive datatypes I don't think it's possible, it would imply to validate LDAP expressions

@bastelfreak bastelfreak added the enhancement New feature or request label Nov 26, 2024
@d1nuc0m d1nuc0m force-pushed the ldap-require branch 2 times, most recently from 25c228d to bf8b2eb Compare November 26, 2024 13:36
@d1nuc0m
Copy link
Contributor Author

d1nuc0m commented Nov 26, 2024

Sorry for the force pushes, mistakes with commit signing

@d1nuc0m d1nuc0m force-pushed the ldap-require branch 2 times, most recently from 9922a90 to 712c9a8 Compare November 26, 2024 13:37
@d1nuc0m
Copy link
Contributor Author

d1nuc0m commented Nov 27, 2024

Reproduced the test failure on a VM of mine, it looks like the culprit is puppetlabs-puppetdb 8.1.0, see also puppetlabs/puppetlabs-puppetdb/#412

Allow use of Require ldap-user/ldap-dn/ldap-attribute/ldap-filter in
Apache config.
Compatible with pre-existing ldap-group settings.
Unmodified tests pass with puppetdb 8.0.1 module.
See its issue 412 for probable cause.
@d1nuc0m
Copy link
Contributor Author

d1nuc0m commented Nov 27, 2024

@bastelfreak it works, the fix might also allow to unlock modulesync related issues such as #413

@bastelfreak
Copy link
Member

thanks for your work!

@bastelfreak bastelfreak merged commit 5ba0b4a into voxpupuli:master Nov 28, 2024
18 checks passed
@d1nuc0m d1nuc0m deleted the ldap-require branch November 28, 2024 08:09
@d1nuc0m d1nuc0m mentioned this pull request Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants