Skip to content

feat: support direct querying of AD group membership via LDAP #972

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

kriswest
Copy link
Contributor

resolves #909

Adds support for using LDAP directly to confirm AD group membership, which is enabled when the apis.ls config is not set. Also adds documentations for the AD related parts of the config.

Copy link

netlify bot commented Apr 11, 2025

Deploy Preview for endearing-brigadeiros-63f9d0 ready!

Name Link
🔨 Latest commit 189ec0c
🔍 Latest deploy log https://app.netlify.com/sites/endearing-brigadeiros-63f9d0/deploys/67ff941bcaddc90008558d67
😎 Deploy Preview https://deploy-preview-972--endearing-brigadeiros-63f9d0.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

codecov bot commented Apr 11, 2025

Codecov Report

Attention: Patch coverage is 0% with 15 lines in your changes missing coverage. Please review.

Project coverage is 49.42%. Comparing base (3718943) to head (189ec0c).

Files with missing lines Patch % Lines
src/service/passport/activeDirectory.js 0.00% 12 Missing ⚠️
src/config/index.ts 0.00% 2 Missing ⚠️
src/service/passport/index.js 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #972      +/-   ##
==========================================
- Coverage   49.65%   49.42%   -0.24%     
==========================================
  Files          48       48              
  Lines        1718     1726       +8     
  Branches      175      175              
==========================================
  Hits          853      853              
- Misses        841      849       +8     
  Partials       24       24              

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@kriswest
Copy link
Contributor Author

Fixed teh cypress test but am waiting on a review (in git proxy), will update monday with working cypress tests

@kriswest
Copy link
Contributor Author

I'm not sure I can improve the test coverage on this - unless someone has a suggestion on mocking passport. We are however using it successfully this end. Configs were documented.

Otherwise ready for review @JamieSlome

@kriswest
Copy link
Contributor Author

@JamieSlome @grovesy @coopernetes Keen to get a review on this - also it should be tested by someone using a REST API to check group meembership.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LDAP Authentication - Validating Groups of a user
1 participant