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

Feature: Support for multiple AUTH_LDAP_REQUIRE_GROUP from environment variables #1297

Conversation

NeodymiumFerBore
Copy link
Contributor

@NeodymiumFerBore NeodymiumFerBore commented Sep 2, 2024

Related Issue: netbox-community/netbox-chart#210

New Behavior

We can configure multiple AUTH_LDAP_REQUIRE_GROUP via 2 environments variables:

  • AUTH_LDAP_REQUIRE_GROUP_DN (already exists)
  • AUTH_LDAP_REQUIRE_GROUP_DN_SEPARATOR (new)

Contrast to Current Behavior

As of now, we have to use an ldap/extra.py file to configure multiple AUTH_LDAP_REQUIRE_GROUP with LDAPGroupQuery.

Discussion: Benefits and Drawbacks

Why AUTH_LDAP_REQUIRE_GROUP_DN_SEPARATOR?

Building an LDAPGroupQuery with AUTH_LDAP_REQUIRE_GROUP straight from a whitespace ' ' separated list environment variable (like in configuration.py) will break existing configuration if people use a group containing a whitespace in its DN. It may become a nightmare to workaround, if you are not owner of an LDAP parent OU containing a whitespace character. This situation applies to any separator character.

That's why I opted for the AUTH_LDAP_REQUIRE_GROUP_DN_SEPARATOR. By default it is empty, so the AUTH_LDAP_REQUIRE_GROUP_DN environment variable will be used as a string (current behavior is preserved). If AUTH_LDAP_REQUIRE_GROUP_DN_SEPARATOR is not empty, then we use its value to split AUTH_LDAP_REQUIRE_GROUP_DN into a list, and build an OR'ed LDAPGroupQuery from it.

Benefits:

Drawbacks: None?

Changes to the Wiki

Maybe add an example in wiki/LDAP:

### Example configure multiple allowed groups

```yaml
services:
  netbox:
    environment:
      REMOTE_AUTH_ENABLED: "True"
      REMOTE_AUTH_BACKEND: "netbox.authentication.LDAPBackend"
      AUTH_LDAP_SERVER_URI: "ldaps://domain.com"
      AUTH_LDAP_BIND_DN: "CN=Netbox,OU=EmbeddedDevices,OU=MyCompany,DC=domain,dc=com"
      AUTH_LDAP_BIND_PASSWORD: "TopSecretPassword"
      AUTH_LDAP_USER_SEARCH_BASEDN: "OU=MyCompany,DC=domain,dc=com"
      AUTH_LDAP_GROUP_SEARCH_BASEDN: "OU=SubGroups,OU=MyCompany,DC=domain,dc=com"
      AUTH_LDAP_REQUIRE_GROUP_DN: |
        "CN=Netbox-User,OU=SoftwareGroups,OU=SubGroups,OU=MyCompany,DC=domain,dc=com ; CN=Netbox Visitors,OU=SoftwareGroups,OU=SubGroups,OU=MyCompany,DC=domain,dc=com"
      AUTH_LDAP_REQUIRE_GROUP_DN_SEPARATOR: " ; "
      AUTH_LDAP_GROUP_TYPE: "NestedGroupOfNamesType"
      AUTH_LDAP_IS_ADMIN_DN: "CN=Network Configuration Operators,CN=Builtin,DC=domain,dc=com"
      AUTH_LDAP_IS_SUPERUSER_DN: "CN=Domain Admins,CN=Users,DC=domain,dc=com"
      LDAP_IGNORE_CERT_ERRORS: "false"
```

Proposed Release Note Entry

Add support for configuring multiple AUTH_LDAP_REQUIRE_GROUP via environment variables.

Double Check

  • I have read the comments and followed the PR template.
  • I have explained my PR according to the information in the comments.
  • My PR targets the develop branch.

@tobiasge tobiasge merged commit 636495e into netbox-community:develop Sep 3, 2024
9 checks passed
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.

2 participants