inventree / InvenTree

Open Source Inventory Management System
https://docs.inventree.org
MIT License
4.34k stars 786 forks source link

configurable ldap group classes #8475

Closed jacobfelknor closed 6 days ago

jacobfelknor commented 1 week ago

Addresses https://github.com/inventree/InvenTree/issues/6101

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

netlify[bot] commented 1 week ago

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

Name Link
Latest commit 46a9b408ce106f81433f1b9246583c4994181c7b
Latest deploy log https://app.netlify.com/sites/inventree-web-pui-preview/deploys/6737b88ce62650000829282a
wolflu05 commented 1 week ago

This should also address: https://github.com/inventree/InvenTree/discussions/8365#discussioncomment-11072737

jacobfelknor commented 1 week ago

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

SchrodingersGat commented 1 week ago

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

matmair commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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

jacobfelknor commented 6 days ago

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

SchrodingersGat commented 6 days ago

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 commented 6 days ago

Okay, I think that should fix the styling

codecov[bot] commented 6 days ago

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 :warning:
src/backend/InvenTree/machine/machine_type.py 0.00% 1 Missing :warning:
src/backend/InvenTree/part/views.py 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ 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](https://app.codecov.io/gh/inventree/InvenTree/pull/8475/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=inventree) | Coverage Δ | | |---|---|---| | [backend](https://app.codecov.io/gh/inventree/InvenTree/pull/8475/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=inventree) | `85.90% <31.25%> (-0.01%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=inventree#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

SchrodingersGat commented 6 days ago

@jacobfelknor thanks for the contribution :)