Skip to content

Conversation

@pboling
Copy link
Member

@pboling pboling commented Nov 5, 2025

Documents and closes #50

@pboling pboling self-assigned this Nov 5, 2025
Copilot AI review requested due to automatic review settings November 5, 2025 09:43
Copy link
Contributor

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 documentation and test coverage to clarify the distinction between the LDAP search attribute (:uid config option) and the OmniAuth auth hash's auth.uid field, which always returns the Distinguished Name (DN).

  • Adds a new test case demonstrating that auth.uid returns the DN (not the sAMAccountName attribute value) when using sAMAccountName as the search attribute
  • Adds comprehensive README documentation explaining the difference between :uid config option and auth.uid result
  • Documents where to find username-style values (auth.info.nickname or auth.extra.raw_info)

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
spec/omniauth/strategies/ldap_spec.rb Adds test case verifying that auth.uid returns DN when using sAMAccountName as the search attribute, and that nickname correctly maps from sAMAccountName
README.md Adds new documentation section explaining the distinction between :uid config option (search attribute) and auth.uid result (DN), with examples and RFC references

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@pboling pboling force-pushed the feat/spec-auth-uid-as-dn branch from fcf4f64 to a97b52d Compare November 5, 2025 10:00
@codecov
Copy link

codecov bot commented Nov 5, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.71%. Comparing base (6efd08e) to head (25968b2).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #101   +/-   ##
=======================================
  Coverage   97.71%   97.71%           
=======================================
  Files           4        4           
  Lines         219      219           
  Branches       59       59           
=======================================
  Hits          214      214           
  Misses          5        5           

☔ 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.

@pboling pboling force-pushed the feat/spec-auth-uid-as-dn branch from a97b52d to 0eef3ad Compare November 5, 2025 10:16
@pboling pboling force-pushed the feat/spec-auth-uid-as-dn branch from 0eef3ad to 25968b2 Compare November 5, 2025 10:18
@github-actions
Copy link

github-actions bot commented Nov 5, 2025

Code Coverage

Package Line Rate Branch Rate Health
omniauth-ldap 98% 80%
Summary 98% (204 / 208) 80% (51 / 64)

Minimum allowed line rate is 98%

@pboling pboling merged commit 5d67e0d into main Nov 5, 2025
29 of 30 checks passed
@pboling pboling deleted the feat/spec-auth-uid-as-dn branch November 5, 2025 10:21
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.

UID configuration is ignored

2 participants