Modify ldap init to support multiple ldap hosts #100
Conversation
ktyogurt
force-pushed
the
modyfy_ldap_init
branch
from
86524aa to
2e691be
Compare
ktyogurt
changed the title
ktyogurt
force-pushed
the
modyfy_ldap_init
branch
from
2e691be to
620b768
Compare
There was a problem hiding this comment.
That's a good addition 👍
Looks great. Just a couple of minor additions.
Could you please add it to the Readme? Having a description that host can allow multiple comma-separated entries would be helpful.
Same for the https://docs.stackstorm.com/authentication.html#ldap
Additionally, looks like we don't track the Changelog in this repo.
Could you add the Changelog for this PR to the https://github.com/StackStorm/st2/blob/master/CHANGELOG.rst ?
This way users will know about the new feature.
ktyogurt
force-pushed
the
modyfy_ldap_init
branch
from
83f41ac to
9d8b42d
Compare
Sure. I added a description to the Readme and sent a PR to add changelog. |
| @@ -24,7 +24,7 @@ sudo dnf install python2-devel python3-devel openldap-devel | |||
| | base_ou | yes | | Base OU to search for user and group entries | | |||
| | group_dns | yes | | Which groups user must be member of to be granted access (group names are considered case-insensitive) | | |||
| | group_dns_check | no | `and` | What kind of check to perform when validating user group membership (`and` / `or`). When `and` behavior is used, user needs to be part of all the specified groups and when `or` behavior is used, user needs to be part of at least one or more of the specified groups. | | |||
| | host | yes | | Hostname of the LDAP server | | |||
| | host | yes | | Hostname of the LDAP server. Multiple comma-separated entries are allowed. | | |||
There was a problem hiding this comment.
Nice! 👍
@ktyogurt We'll need the same for the https://docs.stackstorm.com/authentication.html#ldap, a PR against the
https://github.com/StackStorm/st2docs/blob/master/docs/source/authentication.rst
There was a problem hiding this comment.
I sent a PR for document update.
This PR adds the feature to support multiple LDAP hosts. And this PR also add corresponding unit test.
ldap.initialize() can accept the multiple uris as stated in https://www.python-ldap.org/en/python-ldap-3.3.0/reference/ldap.html#ldap.initialize
Then, we can configure for the multiple hosts as
backend_kwargs = {..., "host": "identity1.example.com,identity2.example.com,identity3.example.com,..."}I have confirmed that this works properly with our three LDAP servers and the unit tests are successful.