Skip to content

Conversation

@salbahra
Copy link
Member

@salbahra salbahra commented Apr 1, 2025

Added to support environments that require a service account to perform directory lookups.

@salbahra salbahra requested a review from Copilot April 1, 2025 21:58
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for LDAP authenticated directory searches using a service account binding method while retaining the original direct binding method, and updates related documentation accordingly.

  • Introduces environment variables and instructions for service account binding in Docker documentation.
  • Implements dual LDAP authentication paths (direct and service account binding) in the authentication module.
  • Updates README to document both LDAP authentication methods.

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

File Description
docs/src/install-guide/docker.md Added instructions for configuring LDAP service account binding for secure connections.
api/authentication.py Extended LDAP authentication to support service account binding alongside direct binding.
README.md Updated documentation to describe both LDAP authentication methods.
Files not reviewed (1)
  • Dockerfile: Language not supported
Comments suppressed due to low confidence (1)

api/authentication.py:56

  • Consider adding a more descriptive error message or logging before aborting with a 500 status when the service account credentials are missing to aid debugging.
if not service_account_dn or not service_account_password:

@github-actions
Copy link

github-actions bot commented Apr 1, 2025

Visit the preview URL for this PR (updated for commit d4f19b2):

https://milo-ml--pr12-feature-add-ldap-bin-jub5lg62.web.app

(expires Sat, 24 May 2025 02:13:12 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: e68906831987574d8256ae584091b45408ab3868

…ser-controlled sources

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@salbahra salbahra requested a review from Copilot April 1, 2025 22:05
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for authenticated directory searches by introducing a service account binding option for LDAP authentication, along with associated documentation updates.

  • Added a new documentation section for LDAP service account binding in the Docker installation guide.
  • Enhanced LDAP authentication in the API to support service account binding before user authentication.
  • Updated the README with details on both direct binding and service account binding configurations.

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

File Description
docs/src/install-guide/docker.md Added instructions for configuring LDAP service account binding.
api/authentication.py Extended LDAP authentication logic to support service account binding.
README.md Documented the two LDAP binding approaches and environment variables.
Files not reviewed (1)
  • Dockerfile: Language not supported
Comments suppressed due to low confidence (1)

api/authentication.py:57

  • Consider providing a descriptive error message (e.g., abort(500, description='Missing service account credentials')) to clarify why the service account configuration is incomplete.
if not service_account_dn or not service_account_password:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants