api7 / api7/lua-resty-ldap

fix: make LDAP operation completion and result states unambiguous

Open
#33 1 comment 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
Terra
Stars
10
Forks
10
PR merge metrics
No merged PRs in 30d

Description

Context

This is a post-merge production-readiness follow-up to #32. It is intentionally kept out of the current refactor PR so that the receive-state and public-result contracts can be designed and tested together.

At head b665ffa91a2671e81158e4c1a445530442b8bc7e, _send_receive treats a non-timeout failure while reading the next response header as end-of-stream. For a multi-response Search, the server can therefore close after zero or more entries but before SearchResultDone, and the accumulated table can be returned as a successful empty or partial result.

The decoder exposes message_id, but the client does not correlate every response with the request in flight or handle messageID=0 as a separate unsolicited notification. RFC 4511 requires responses to carry the corresponding request message ID, reserves zero for unsolicited notifications, and defines a Search result as zero or more entries/references followed by exactly one SearchResultDone:

Goal

Make every successful client operation prove that it received the expected terminal response for the exact request, and expose a stable machine-readable outcome contract that downstream callers can consume without parsing error strings.

Required behavior

  • Track the expected non-zero request messageID and reject any response with a different ID.
  • Handle messageID=0 notifications, including Notice of Disconnection, as connection-level events rather than responses to the current request.
  • Require the expected terminal response for each operation. In particular, Search succeeds only after one matching SearchResultDone; EOF is never a substitute for completion.
  • Treat closed, EOF, RST, timeout, truncated header/body, unexpected operation, duplicate terminal response, and missing terminal response as failures. Invalidate and close the socket so it cannot re-enter a pool or contaminate the next request.
  • Expose enough structured information for callers to distinguish at least invalid credentials, other LDAP result codes, timeout, transport/TLS failure, and protocol failure. Do not require callers to classify failures by matching human-readable strings.
  • Preserve and document the compatibility contract of ldap_authenticate, including its canonical escaped DN return, while adding any structured result needed by downstream integrations.

Acceptance criteria

  • A Search that receives entries and then EOF/RST before SearchResultDone returns an error, never partial success.
  • A Search that receives no entries and closes before SearchResultDone returns an error, never empty success.
  • Wrong-ID responses before or during Bind/Search, an ID-zero Notice of Disconnection, a wrong terminal operation, duplicate SearchResultDone, and truncated headers/bodies all fail closed and close the socket.
  • Both pinned and single-operation clients are covered, including a regression proving that a failed exchange cannot affect a subsequent request or pooled connection.
  • Tests cover valid zero-entry Search, valid multi-entry Search, server LDAP result errors, invalid credentials, timeout, transport/TLS failure, and protocol failure through the public API.
  • Public return/error semantics and compatibility behavior are documented.

Out of scope

APISIX-specific HTTP status mapping, Consumer lookup, Basic credential parsing, log redaction, and product configuration belong in a separate apache/apisix issue after this library contract is agreed.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with lib/resty/ldap/client.lua, especially _send_receive, and review the RFC 4511 sections linked in the issue. Trace pinned and single-operation clients before defining the public result contract. Done means terminal responses are verified, protocol and transport failures close the socket, partial Search results fail, and public tests cover the listed cases and compatibility behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
lua
Domain
authentication, backend, networking
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.