Update AUTH_LDAP_MIRROR_GROUPS handling

[ryanmerolle]

Desired Behavior

Per the Django LDAP documentation, the var can be set to True, False, or

This can also be a list or other collection of group names, in which case we’ll only mirror those groups and leave the rest alone. This is ignored if AUTH_LDAP_MIRROR_GROUPS_EXCEPT is set.

Contrast to Current Behavior

The ldap_config.py only assumes the environment variable is true or false.

Changes Required

Update ldap_config.py and docker-compose.yml environment variables, or just document in the wiki.

Discussion: Benefits and Drawbacks

By adding to the python, we can ensure confusion due to this current feature gap with the documented django-auth-ldap.

I can look at tackling this, but I wanted to open up for discussion and approach. We could even look to add support for AUTH_LDAP_MIRROR_GROUPS_EXCEPT.

[cimnine]

This should easily be achieved using dynamic configuration, as introduced in 0.26.0 (PR #343 respectively). Create a second configuration file and mount it where ldap_config.py is and define AUTH_LDAP_MIRROR_GROUPS_EXCEPT in there.

[ryanmerolle]

This approach is very useful, but I do not think defining a list of AUTH_LDAP_MIRROR_GROUPS_EXCEPT is always going to be manageable for individuals who do not control the groups in their LDAP setup. Because AUTH_LDAP_MIRROR_GROUPS is defined in the main ldap_config.py it does not seem like it can be overwritten by a list of groups to mirror.

My thought is to add a conditional into the current ldap_config.py that if the environment variable for AUTH_LDAP_MIRROR_GROUPS is true or false, then set it as such. If not, then set it as nothing, so you can define it in the configuration\ldap\extra.py.

[cimnine]

I was under the impression that 'other' config files (e.g. you_ldap_config.py) overwrite what was previously defined in ldap_config.py. Have you tried it?

[ryanmerolle]

I have tried and it seemed like it did not override for existing vars that were set. It just allowed me to add to the config which is still a good way to extend ldap_config

[tobiasge]

I think the problem that an existing variable is not overridden lies within the scandir loop in configuration.docker.py. There's no filter for main_config, so that it will be loaded twice. When you named your additional configuration extra.py it was loaded before the second copy of ldap_config.py. Try to name your additional file x_extra.py and it should work.

This is just from looking at the code. I have to verify this theory. If this is the case we should fix the loop.

[cimnine]

There's no filter for main_config, so that it will be loaded twice.

This would also explain that loading the main configuration is always shown twice in the logs – because it actually happens twice.

[cimnine]

I can confirm the issue as described by @tobiasge and created a PR to fix it: #450