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

configurable ldap group classes #8475

Merged

Conversation

jacobfelknor
Copy link
Contributor

Addresses #6101

Allow more granular LDAP group configuration. If no action is taken, this behaves as it did before due to defaults.

Copy link

netlify bot commented Nov 12, 2024

Deploy Preview for inventree-web-pui-preview canceled.

Name Link
🔨 Latest commit 46a9b40
🔍 Latest deploy log https://app.netlify.com/sites/inventree-web-pui-preview/deploys/6737b88ce62650000829282a

@wolflu05
Copy link
Contributor

This should also address: #8365 (reply in thread)

@jacobfelknor
Copy link
Contributor Author

Force pushed to amend commit author which was incorrect, my apologies

@SchrodingersGat
Copy link
Member

SchrodingersGat commented Nov 12, 2024

@matmair @wolflu05 are either of you in a position to review this properly? I have no experience with LDAP

@matmair
Copy link
Member

matmair commented Nov 12, 2024

I do not have a non-prod setup with LDAPS right now, I would have to build something up in my lab so if Lukas has something I would let him test it.

@jacobfelknor
Copy link
Contributor Author

For reference, these are the new setting values I used to make this work in my environment

INVENTREE_LDAP_GROUP_OBJECT_CLASS: group
INVENTREE_LDAP_GROUP_TYPE_CLASS: NestedMemberDNGroupType
INVENTREE_LDAP_GROUP_TYPE_CLASS_ARGS: member
INVENTREE_LDAP_GROUP_TYPE_CLASS_KWARGS: '{"name_attr": "cn"}'

@wolflu05
Copy link
Contributor

Sorry, I have currently also no testing setup and very limited time, so this will take me some time to get to it. I'd appreciate if someone of you could test this.

@SchrodingersGat
Copy link
Member

@jacobfelknor can you confirm that the new settings are working for your setup?

@SchrodingersGat SchrodingersGat added this to the 1.0.0 milestone Nov 14, 2024
@SchrodingersGat SchrodingersGat added setup Relates to the InvenTree setup / installation process enhancement This is an suggested enhancement or new feature labels Nov 14, 2024
@jacobfelknor
Copy link
Contributor Author

Yes, I'm currently using these settings in my instance and it works with my environment

@SchrodingersGat
Copy link
Member

Ok, can you please fix the style issues and I'm happy to merge. If there are any issues which arise you may get tagged :)

@jacobfelknor
Copy link
Contributor Author

Okay, I think that should fix the styling

Copy link

codecov bot commented Nov 15, 2024

Codecov Report

Attention: Patch coverage is 31.25000% with 11 lines in your changes missing coverage. Please review.

Project coverage is 84.34%. Comparing base (fbe222f) to head (46a9b40).
Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
src/backend/InvenTree/InvenTree/settings.py 0.00% 9 Missing ⚠️
src/backend/InvenTree/machine/machine_type.py 0.00% 1 Missing ⚠️
src/backend/InvenTree/part/views.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8475      +/-   ##
==========================================
- Coverage   84.35%   84.34%   -0.01%     
==========================================
  Files        1178     1178              
  Lines       53746    53751       +5     
  Branches     2029     2029              
==========================================
  Hits        45337    45337              
- Misses       7896     7901       +5     
  Partials      513      513              
Flag Coverage Δ
backend 85.90% <31.25%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@SchrodingersGat SchrodingersGat merged commit f9d3f43 into inventree:master Nov 15, 2024
26 checks passed
@SchrodingersGat
Copy link
Member

@jacobfelknor thanks for the contribution :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This is an suggested enhancement or new feature setup Relates to the InvenTree setup / installation process
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants