starred/netbox
1
0
Fork 0
mirror of https://github.com/netbox-community/netbox.git synced 2026-01-12 05:20:31 +01:00
Code Issues 273 Packages Projects Releases 10 Wiki Activity

LDAP: Disabled user never re-enabled when doing API calls #7116

New Issue
Closed
opened 2025-12-29 20:19:24 +01:00 by adam · 3 comments
adam commented 2025-12-29 20:19:24 +01:00
Owner
Copy Link

Originally created by @kpetremann on GitHub (Oct 17, 2022).

Originally assigned to: @kpetremann on GitHub.

NetBox version

v3.2.9

Python version

3.9

Steps to Reproduce

  • set FIND_GROUP_PERMS to True
  • set LDAP as the authentication backend
  • use token to do any API requests continuously (either DRF or GraphQL) like this:
    curl -H "Authorization: Token $NETBOX_TOKEN" 127.0.0.1:8001/api/users/users/
  • have the LDAP responding an empty responses. This is difficult to reproduce as it happens during too high load on the LDAP server, which is an issue itself
    • Easier alternative: disable the user manually via the Django admin panel

Expected Behavior

When calling the API, an inactive user should always be synced with the LDAP (using populate_user() method) to recover from such issue.

It should happen whether FIND_GROUP_PERMS is enabled or disabled, to be able to recover from such issue.

If the following behavior is acceptable for you, I have a code ready to be submitted as a PR:

proposed fix

netbox/netbox/api/authentication.py:

class TokenAuthentication(authentication.TokenAuthentication):
    """
    A custom authentication scheme which enforces Token expiration times.
    """
    model = Token

    def authenticate_credentials(self, key):
        model = self.get_model()
        try:
            token = model.objects.prefetch_related('user').get(key=key)
        except model.DoesNotExist:
            raise exceptions.AuthenticationFailed("Invalid token")

        # Enforce the Token's expiration time, if one has been set.
        if token.is_expired:
            raise exceptions.AuthenticationFailed("Token expired")

        user = token.user
        # When LDAP authentication is active try to load user data from LDAP directory
        if settings.REMOTE_AUTH_BACKEND == 'netbox.authentication.LDAPBackend':
            from netbox.authentication import LDAPBackend
            ldap_backend = LDAPBackend()

            # Load from LDAP if FIND_GROUP_PERMS is active
            # Always query LDAP when user is not active, otherwise it is never activated again
            if ldap_backend.settings.FIND_GROUP_PERMS or not token.user.is_active:
                ldap_user = ldap_backend.populate_user(token.user.username)
                # If the user is found in the LDAP directory use it, if not fallback to the local user
                if ldap_user:
                    user = ldap_user

        if not user.is_active:
            raise exceptions.AuthenticationFailed("User inactive")

        return user, token

Observed Behavior

When using LDAP and FIND_GROUP_PERMS, if the LDAP server gives an empty response, the user is disabled.

The issue is that the user is never resynced again with the LDAP when using API calls. populate_user() is never called because the is_active condition exits the function before.

The only current solution is to resync the user by asking him to connect to the UI.

Originally created by @kpetremann on GitHub (Oct 17, 2022). Originally assigned to: @kpetremann on GitHub. ### NetBox version v3.2.9 ### Python version 3.9 ### Steps to Reproduce * set FIND_GROUP_PERMS to True * set LDAP as the authentication backend * use token to do any API requests continuously (either DRF or GraphQL) like this: `curl -H "Authorization: Token $NETBOX_TOKEN" 127.0.0.1:8001/api/users/users/` * have the LDAP responding an empty responses. This is difficult to reproduce as it happens during too high load on the LDAP server, which is an issue itself * Easier alternative: disable the user manually via the Django admin panel ### Expected Behavior When calling the API, an inactive user should always be synced with the LDAP (using `populate_user()` method) to recover from such issue. It should happen whether FIND_GROUP_PERMS is enabled or disabled, to be able to recover from such issue. If the following behavior is acceptable for you, I have a code ready to be submitted as a PR: <details><summary>proposed fix</summary> _**netbox/netbox/api/authentication.py:**_ class TokenAuthentication(authentication.TokenAuthentication): """ A custom authentication scheme which enforces Token expiration times. """ model = Token def authenticate_credentials(self, key): model = self.get_model() try: token = model.objects.prefetch_related('user').get(key=key) except model.DoesNotExist: raise exceptions.AuthenticationFailed("Invalid token") # Enforce the Token's expiration time, if one has been set. if token.is_expired: raise exceptions.AuthenticationFailed("Token expired") user = token.user # When LDAP authentication is active try to load user data from LDAP directory if settings.REMOTE_AUTH_BACKEND == 'netbox.authentication.LDAPBackend': from netbox.authentication import LDAPBackend ldap_backend = LDAPBackend() # Load from LDAP if FIND_GROUP_PERMS is active # Always query LDAP when user is not active, otherwise it is never activated again if ldap_backend.settings.FIND_GROUP_PERMS or not token.user.is_active: ldap_user = ldap_backend.populate_user(token.user.username) # If the user is found in the LDAP directory use it, if not fallback to the local user if ldap_user: user = ldap_user if not user.is_active: raise exceptions.AuthenticationFailed("User inactive") return user, token </details> ### Observed Behavior When using LDAP and FIND_GROUP_PERMS, if the LDAP server gives an empty response, the user is disabled. The issue is that the user is never resynced again with the LDAP when using API calls. `populate_user()` is never called because the is_active condition exits the function before. The only current solution is to resync the user by asking him to connect to the UI.
adam added the type: bugstatus: accepted labels 2025-12-29 20:19:24 +01:00
adam closed this issue 2025-12-29 20:19:25 +01:00
adam commented 2025-12-29 20:19:25 +01:00
Author
Owner
Copy Link

@kkthxbye-code commented on GitHub (Oct 18, 2022):

This issue is following https://github.com/netbox-community/netbox/pull/7928

I'm not sure I'm seeing how they are related at all, the user.is_active check was not changed and was still done before populating the user.

If the following behavior is acceptable for you, I have a code ready to be submitted as a PR:

Just a heads-up, your proposed fix is based on an older version, so it doesn't match develop.

From a quick glance, moving the is_active check makes sense. I personally don't see a reason to force the update from the AD when FIND_GROUP_PERMS is disabled though. The LDAP implementation is not great when it comes to the API and not having to deal with it when FIND_GROUP_PERMS is disabled is a plus imo.

@kkthxbye-code commented on GitHub (Oct 18, 2022): >This issue is following https://github.com/netbox-community/netbox/pull/7928 I'm not sure I'm seeing how they are related at all, the `user.is_active` check was not changed and was still done before populating the user. > If the following behavior is acceptable for you, I have a code ready to be submitted as a PR: Just a heads-up, your proposed fix is based on an older version, so it doesn't match develop. From a quick glance, moving the `is_active` check makes sense. I personally don't see a reason to force the update from the AD when FIND_GROUP_PERMS is disabled though. The LDAP implementation is not great when it comes to the API and not having to deal with it when FIND_GROUP_PERMS is disabled is a plus imo.
adam commented 2025-12-29 20:19:25 +01:00
Author
Owner
Copy Link

@kpetremann commented on GitHub (Oct 18, 2022):

Hello @kkthxbye-code

I'm not sure I'm seeing how they are related at all, the user.is_active check was not changed and was still done before populating the user.

My apologies, it was a mistake. I'll update the issue.

Actually, the bug is here since the introduction of the feature: a3d40e3521

Just a heads-up, your proposed fix is based on an older version, so it doesn't match develop.

indeed, it is based on the 3.2.9 we have in production. But still my fix is compatible, is just that the code changed before the lines I am editing. My code in Github is based on the develop branch, it may be easier to read: https://github.com/netbox-community/netbox/compare/develop...kpetremann:netbox:fix_ldap_user_never_reenabled

From a quick glance, moving the is_active check makes sense

👍

I personally don't see a reason to force the update from the AD when FIND_GROUP_PERMS is disabled though.

There are two reasons here from my point of view.

The configuration tells how to map the is_active status with LDAP via:

AUTH_LDAP_USER_FLAGS_BY_GROUP = {
    "is_active": "cn=active,ou=groups,dc=example,dc=com",

Meaning, the authority of the is_active status is the LDAP, and if someone disables it in the admin, it should be restored automatically. But I agree this point could be argued.

But, the real reason is that we disabled FIND_GROUP_PERMS to reduce load on the LDAP. It means we completely loose the LDAP group sync, unless the user are connecting themselves to the UI. Asking the user is not a good solution to us as our users are mainly API and apps completely industrialized and automated.

To avoid that, we planned to schedule a job to run popupate_user() for each user in our netbox instance to keep groups and user up to date. However, doing that exposes us to the deactivated user issue which never recover. This is why I also added the or not token.user.is_active in the condition. If necessary I can remove it of course, and we will try to find another solution to this.

I don't think it would add much queries to the LDAP, as it would force queries only for disabled users.

@kpetremann commented on GitHub (Oct 18, 2022): Hello @kkthxbye-code > I'm not sure I'm seeing how they are related at all, the user.is_active check was not changed and was still done before populating the user. My apologies, it was a mistake. I'll update the issue. Actually, the bug is here since the introduction of the feature: https://github.com/netbox-community/netbox/commit/a3d40e35212841bef7366953a39ff4c04e826bdd > Just a heads-up, your proposed fix is based on an older version, so it doesn't match develop. indeed, it is based on the 3.2.9 we have in production. But still my fix is compatible, is just that the code changed before the lines I am editing. My code in Github is based on the develop branch, it may be easier to read: https://github.com/netbox-community/netbox/compare/develop...kpetremann:netbox:fix_ldap_user_never_reenabled > From a quick glance, moving the is_active check makes sense :+1: > I personally don't see a reason to force the update from the AD when FIND_GROUP_PERMS is disabled though. There are two reasons here from my point of view. The configuration tells how to map the `is_active` status with LDAP via: ``` AUTH_LDAP_USER_FLAGS_BY_GROUP = { "is_active": "cn=active,ou=groups,dc=example,dc=com", ``` Meaning, the authority of the `is_active` status is the LDAP, and if someone disables it in the admin, it should be restored automatically. But I agree this point could be argued. But, the real reason is that we disabled FIND_GROUP_PERMS to reduce load on the LDAP. It means we completely loose the LDAP group sync, unless the user are connecting themselves to the UI. Asking the user is not a good solution to us as our users are mainly API and apps completely industrialized and automated. To avoid that, we planned to schedule a job to run popupate_user() for each user in our netbox instance to keep groups and user up to date. However, doing that exposes us to the deactivated user issue which never recover. This is why I also added the `or not token.user.is_active` in the condition. If necessary I can remove it of course, and we will try to find another solution to this. I don't think it would add much queries to the LDAP, as it would force queries only for disabled users.
adam commented 2025-12-29 20:19:25 +01:00
Author
Owner
Copy Link

@kkthxbye-code commented on GitHub (Oct 18, 2022):

Seems resonable, I've assigned you to the issue. Feel free to create a PR targeting the develop branch. Let me know if you have any questions. I'll help test when the PR is up.

@kkthxbye-code commented on GitHub (Oct 18, 2022): Seems resonable, I've assigned you to the issue. Feel free to create a PR targeting the develop branch. Let me know if you have any questions. I'll help test when the PR is up.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
beta
breaking change
complexity: high
complexity: low
complexity: medium
needs milestone
netbox
pending closure
plugin candidate
pull-request
Mirrored from GitHub Pull Request
 severity: high
severity: low
severity: medium
status: accepted
status: backlog
status: blocked
status: duplicate
status: needs owner
status: needs triage
status: revisions needed
status: under review
topic: GraphQL
topic: Internationalization
topic: OpenAPI
topic: UI/UX
topic: cabling
topic: event rules
topic: htmx navigation
topic: industrialization
topic: migrations
topic: plugins
topic: scripts
topic: templating
topic: testing
type: bug
type: deprecation
type: documentation
type: feature
type: housekeeping
type: translation
No Label status: accepted type: bug
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
adam
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/netbox#7116
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?