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

Mirroring LDAP groups doesn't work with v4.0-beta2 #15831

Closed
Cornelicorn opened this issue Apr 24, 2024 · 9 comments
Closed

Mirroring LDAP groups doesn't work with v4.0-beta2 #15831

Cornelicorn opened this issue Apr 24, 2024 · 9 comments
Assignees
Labels
beta Concerns a bug/feature in a beta release status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application

Comments

@Cornelicorn
Copy link

Deployment Type

NetBox Cloud

NetBox Version

v4.0-beta2

Python Version

3.11

Steps to Reproduce

Setup netbox with ldap authentication and set AUTH_LDAP_MIRROR_GROUPS=True.
This was tested using django-auth-ldap==4.8.0 on Debian 12.

Then try to login with an LDAP user.
This Setup is working on v4.0-beta1

Expected Behavior

The Groups get added to the user and you can login.

Observed Behavior

An error 500 is raised with the following traceback:

Internal Server Error: /login/
Traceback (most recent call last):
  File "/opt/netbox/venv/lib/python3.11/site-packages/django/db/models/fields/__init__.py", line 2117, in get_prep_value
    return int(value)
           ^^^^^^^^^^
TypeError: int() argument must be a string, a bytes-like object or a real number, not 'Group'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/opt/netbox/venv/lib/python3.11/site-packages/django/core/handlers/exception.py", line 55, in inner
    response = get_response(request)
               ^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/venv/lib/python3.11/site-packages/django/core/handlers/base.py", line 197, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/venv/lib/python3.11/site-packages/django/views/generic/base.py", line 104, in view
    return self.dispatch(request, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/venv/lib/python3.11/site-packages/django/utils/decorators.py", line 48, in _wrapper
    return bound_method(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/venv/lib/python3.11/site-packages/django/views/decorators/debug.py", line 143, in sensitive_post_parameters_wrapper
    return view(request, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/netbox/account/views.py", line 44, in dispatch
    return super().dispatch(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/venv/lib/python3.11/site-packages/django/views/generic/base.py", line 143, in dispatch
    return handler(request, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/netbox/account/views.py", line 90, in post
    if form.is_valid():
       ^^^^^^^^^^^^^^^
  File "/opt/netbox/venv/lib/python3.11/site-packages/django/forms/forms.py", line 197, in is_valid
    return self.is_bound and not self.errors
                                 ^^^^^^^^^^^
  File "/opt/netbox/venv/lib/python3.11/site-packages/django/forms/forms.py", line 192, in errors
    self.full_clean()
  File "/opt/netbox/venv/lib/python3.11/site-packages/django/forms/forms.py", line 328, in full_clean
    self._clean_form()
  File "/opt/netbox/venv/lib/python3.11/site-packages/django/forms/forms.py", line 349, in _clean_form
    cleaned_data = self.clean()
                   ^^^^^^^^^^^^
  File "/opt/netbox/venv/lib/python3.11/site-packages/django/contrib/auth/forms.py", line 250, in clean
    self.user_cache = authenticate(
                      ^^^^^^^^^^^^^
  File "/opt/netbox/venv/lib/python3.11/site-packages/django/views/decorators/debug.py", line 75, in sensitive_variables_wrapper
    return func(*func_args, **func_kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/venv/lib/python3.11/site-packages/django/contrib/auth/__init__.py", line 79, in authenticate
    user = backend.authenticate(request, **credentials)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/venv/lib/python3.11/site-packages/django_auth_ldap/backend.py", line 149, in authenticate
    user = self.authenticate_ldap_user(ldap_user, password)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/venv/lib/python3.11/site-packages/django_auth_ldap/backend.py", line 207, in authenticate_ldap_user
    return ldap_user.authenticate(password)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/venv/lib/python3.11/site-packages/django_auth_ldap/backend.py", line 351, in authenticate
    self._get_or_create_user()
  File "/opt/netbox/venv/lib/python3.11/site-packages/django_auth_ldap/backend.py", line 644, in _get_or_create_user
    self._mirror_groups()
  File "/opt/netbox/venv/lib/python3.11/site-packages/django_auth_ldap/backend.py", line 792, in _mirror_groups
    self._user.groups.set(existing_groups + new_groups)
  File "/opt/netbox/venv/lib/python3.11/site-packages/django/db/models/fields/related_descriptors.py", line 1292, in set
    else self.target_field.get_prep_value(obj)
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/venv/lib/python3.11/site-packages/django/db/models/fields/related.py", line 1155, in get_prep_value
    return self.target_field.get_prep_value(value)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/venv/lib/python3.11/site-packages/django/db/models/fields/__init__.py", line 2119, in get_prep_value
    raise e.__class__(
TypeError: Field 'id' expected a number but got <Group: mygroup>.
@Cornelicorn Cornelicorn added status: needs triage This issue is awaiting triage by a maintainer type: bug A confirmed report of unexpected behavior in the application labels Apr 24, 2024
@jeremystretch jeremystretch removed their assignment Apr 24, 2024
@jeremystretch jeremystretch added status: under review Further discussion is needed to determine this issue's scope and/or implementation beta Concerns a bug/feature in a beta release and removed status: needs triage This issue is awaiting triage by a maintainer labels Apr 24, 2024
@jeremystretch
Copy link
Member

This Setup is working on v4.0-beta1

I can't seem to find any changes in beta2 that might impact this. We introduced a new custom Group model for v4.0, but that was present in the first beta release.

@Cornelicorn are you certain this exception is raised only when running beta2, and not for beta1?

@Cornelicorn
Copy link
Author

This Setup is working on v4.0-beta1

I can't seem to find any changes in beta2 that might impact this. We introduced a new custom Group model for v4.0, but that was present in the first beta release.

@Cornelicorn are you certain this exception is raised only when running beta2, and not for beta1?

Yes, also reproduced on a fresh v4.0-beta1. Sorry, the first v4.0-beta1 install was upgraded from v3.7.6 and then this problem didn't occur, as the groups were already set.

@jvalente-salemstate
Copy link

Are you using nested groups, and after upgrading did you change AUTH_LDAP_GROUP_TYPE = NestedGroupOfNamesType() in ldap_config.py?

@arthanson
Copy link
Collaborator

I think this is from the moving of groups and using AUTH_LDAP_MIRROR_GROUPS, the ldap code is trying to use the auth.Groups model (https://github.com/django-auth-ldap/django-auth-ldap/blob/master/django_auth_ldap/backend.py#L782). Will potentially need to make a NetBox specific _mirror_groups function and monkey-patch it in?

@Cornelicorn
Copy link
Author

Are you using nested groups, and after upgrading did you change AUTH_LDAP_GROUP_TYPE = NestedGroupOfNamesType() in ldap_config.py?

I have AUTH_LDAP_GROUP_TYPE = GroupOfNamesType() but we're not using nested groups. Changing the value also doesn't do anything.

I think this is from the moving of groups and using AUTH_LDAP_MIRROR_GROUPS, the ldap code is trying to use the auth.Groups model (https://github.com/django-auth-ldap/django-auth-ldap/blob/master/django_auth_ldap/backend.py#L782). Will potentially need to make a NetBox specific _mirror_groups function and monkey-patch it in?

That's also what I suspect after looking at their code.
Maybe a less invasive approach would be to use the signal they provide and override the groups at that point?

@Cornelicorn
Copy link
Author

Or better override get_or_build_user on LDAPBackend in netbox/netbox/netbox/authentication.py

@arthanson
Copy link
Collaborator

@Cornelicorn is this something you want to work on and submit a PR?

@arthanson arthanson self-assigned this Apr 30, 2024
@arthanson arthanson added status: accepted This issue has been accepted for implementation and removed status: under review Further discussion is needed to determine this issue's scope and/or implementation labels Apr 30, 2024
@arthanson
Copy link
Collaborator

@Cornelicorn could you please try #15902 ? It will hopefully fix the issue.

@Cornelicorn
Copy link
Author

@Cornelicorn is this something you want to work on and submit a PR?

Sorry, overlooked that.

@Cornelicorn could you please try #15902 ? It will hopefully fix the issue.

Commenting on the PR for feedback

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
beta Concerns a bug/feature in a beta release status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application
Projects
None yet
Development

No branches or pull requests

4 participants