Skip to content

Conversation

@pboling
Copy link
Member

@pboling pboling commented Nov 6, 2025

Builds on and replaces #47

@pboling pboling self-assigned this Nov 6, 2025
Copilot AI review requested due to automatic review settings November 6, 2025 09:20
@codecov
Copy link

codecov bot commented Nov 6, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.81%. Comparing base (006487f) to head (94b4c93).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #108      +/-   ##
==========================================
+ Coverage   96.79%   96.81%   +0.02%     
==========================================
  Files           4        4              
  Lines         312      314       +2     
  Branches      116      116              
==========================================
+ Hits          302      304       +2     
  Misses         10       10              

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

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 support for JSON-encoded POST bodies in the LDAP authentication strategy, complementing the existing form-encoded support. Rails applications automatically parse JSON request bodies into action_dispatch.request.request_parameters, which the strategy now reads via a new request_data helper method.

  • Introduced a request_data method that checks for Rails-parsed JSON parameters before falling back to standard form parameters
  • Updated all parameter accesses throughout the strategy to use request_data instead of directly accessing request.params
  • Added comprehensive test coverage for JSON authentication flows

Reviewed Changes

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

File Description
lib/omniauth/strategies/ldap.rb Implemented request_data helper and updated parameter access to support JSON bodies
spec/omniauth/strategies/ldap_spec.rb Added test case for JSON POST with Rails environment variables
spec/integration/middleware_spec.rb Added integration tests for JSON authentication flows and missing credentials scenarios
README.md Added documentation on JSON body usage with examples; fixed Discord link reference; added copyright entry

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

@github-actions
Copy link

github-actions bot commented Nov 6, 2025

Code Coverage

Package Line Rate Branch Rate Health
omniauth-ldap 98% 79%
Summary 98% (289 / 296) 79% (100 / 126)

Minimum allowed line rate is 97%

@pboling pboling merged commit 72a5e7e into main Nov 6, 2025
36 of 37 checks passed
@pboling pboling deleted the feat/json-body branch November 6, 2025 09:30
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