Add support for paged search results to LDAP

[magnetised]

Returns the controls from an LDAP search and includes the
PagedResultsControl ASN so that user code can perform paged searches
according to https://www.rfc-editor.org/rfc/rfc2696.txt

[CLAassistant]

All committers have signed the CLA.

[HansN]

I understand it as only a spontaneous paged result return on a normal search request is covered.
Is it possible to send a search request asking for a paged result or asking for further pages?
Maybe a few lines in the manual or at least a comment section with an example in the code would help. (I am aware of that the manual is far from complete in its present state.)

[HansN]

In RFC 2696 this record is called realSearchControlValue and PagedResultsControl is different.
Why rename it? Changing the naming could make future updates impossible without breaking compatibility.

[HansN]

A simple test case would be good to, if possible.

[magnetised]

@HansN

In RFC 2696 this record is called realSearchControlValue and PagedResultsControl is different.
Why rename it

Good question. I think because the original name realSearchControlValue didn't make much sense to me - no mention of paging and the weird "real" prefix...

I'll revert to the original name if you think it has maintainability advantages.

A simple test case would be good to, if possible.

Sure

[xadhoom]

I understand it as only a spontaneous paged result return on a normal search request is covered.
Is it possible to send a search request asking for a paged result or asking for further pages?

As far as I know, no spontaneous paged result are ever returned if not asked for.

To ask to a paged result the search/3 api must be used in order to pass controls to it.

The flow is to start a search request with the pageControl control and the empty cookie. Then get the cookie from the pagedResultsControl, check if equals zero (no more pages), continue to perform the very same search by just updating the cookie (the cookie is a sort of an opaque cursor), until is zero.

Just to make it clearer, here's some pseudo (elixir, sorry!) code to understand the flow:

defmodule LdapClient do
  @pageControl '1.2.840.113556.1.4.319'

  def run do
    {:ok, handle} = :eldap.open(['192.168.56.10'], [])
  
    search_opts = [
      base: 'dc=192,dc=168,dc=56,dc=10,dc=base',
      filter: :eldap.present('uid'),
      scope: :eldap.wholeSubtree()
    ]
  
    {:done, pages} = do_search(handle, search_opts, [])
  end

  defp do_search(handle, search_opts, pages) do
    {:ok, results} = :eldap.search(handle, search_opts, build_controls())
    {:eldap_search_result, _results, _ref, controls} = results # not ideal, just for quick data extraction from record
    next_cookie = decode_cookie(controls)
    do_search(handle, search_opts, [results | pages], next_cookie)
  end

  defp do_search(_handle, _search_opts, pages, cookie) when cookie == "" or is_nil(cookie) do
    {:done, pages}
  end

  defp do_search(handle, search_opts, pages, cookie) do
    {:ok, results} = :eldap.search(handle, search_opts, build_controls(cookie))
    {:eldap_search_result, _results, _ref, controls} = results
    next_cookie = decode_cookie(controls)
    do_search(handle, search_opts, [results | pages], next_cookie)
  end


  defp decode_cookie(controls) do
    # skipping this fun for clarity, but basically returns a string or nil if no control are present
    # or cookie is set to empty by the server, which means we reached the end of pages
     ...
  end

  def build_controls(cookie \\ '') do
    {:ok, value} = :ELDAPv3.encode(:PagedResultsControl, {:PagedResultsControl, 1, cookie})

    [{:control, @pageControl, true, value}]
  end

end

--- updated comment to reuse PagedResultsControl istead of my local encoder ---

[xadhoom]

additional note about the asn1 definition: while PagedResultsControl is for sure wrong, since is just a Control sequence with a specific type (and the Control sequence is already defined in ELDAPv3.asn1), the realSearchControlValue is not accepted by the ASN1 compiler, because is starts with a lowercase letter.

I'm with @magnetised for the weird naming and suggest to use just SearchControlValue. After all a common dissector like wireshark calls it in this way :)

Example:

Lightweight Directory Access Protocol
    LDAPMessage searchResDone(2) success [1 result]
        messageID: 2
        protocolOp: searchResDone (5)
        [Response To: 8]
        [Time: 0.004485554 seconds]
        controls: 1 item
            Control
                controlType: 1.2.840.113556.1.4.319 (pagedResultsControl)
                SearchControlValue
                    size: 0
                    cookie: 0900000000000000

[magnetised]

I've added a test case and as a result included a few helper functions in the eldap module to make usage much easier.

The usage is now something like:

PageSize = 10,
Control1 = eldap:paged_result_control(PageSize),
{ok, SearchResult1} =  eldap:search(H, #eldap_search{...}, [Control1]), 

{ok, Cookie1} = eldap:paged_result_cookie(SearchResult1),
Control2 = eldap:paged_result_control(PageSize, Cookie1),
{ok, SearchResult2} =  eldap:search(H, #eldap_search{...}, [Control2]), 

%% etc

Happy to rename the control - SearchControlValue makes sense from the RFC, though I don't think the RFC naming is very good - "search control" is very generic which is another reason I went with the "PagedResults" naming - it at least mentions "paging" which is what this does.

With the functions wrapping the encoding-decoding of the actual control it isn't particularly user-facing anymore so being true-ish to the spec is maybe the best way to go.

Regarding the eldap:paged_result_cookie/1 function - would there be value in returning a special response if the cookie is empty [], representing the end of the results? Don't want to add too much api surface - this whole app is pretty low-level.

[magnetised]

@HansN

Maybe a few lines in the manual or at least a comment section with an example in the code would help

Happy to add to the manual once the api is approved/agreed

[xadhoom]

though I don't think the RFC naming is very good
Well, for the sorting control (https://datatracker.ietf.org/doc/html/rfc2891) the value of the Control is called SortKeyList. No proper scheme also there.

Btw, for what it's worth I like those helpers and is exactly the same thing I was doing before discovering this PR.

Also exposing resultControls to the caller make it easier to add stuff without necessarily extending :eldap, like the above mentioned sort Control.

[magnetised]

@HansN I see the move to "stalled" but don't know what's holding this back. Are you waiting for docs, or a rename? I was waiting on some feedback before moving ahead (i.e. are you happy with the name of the control value? is the "extended" api ok?)

[HansN]

@magnetised what is missing is simply my time. Today is first working day after my christmas vacation and there are other tasks with higher priority.

[HansN]

Hi @magnetised,
sorry for the delay, but now I'm back again.
The PR was tested without any problems, and I could now merge it. But first, documentation (in lib/eldap/doc/src/eldap.xml) of the three new functions would be good. And you maybe want to do some squashing before I merge it.

[github-actions]

CT Test Results

No tests were run for this PR. This is either because the build failed, or the PR is based on a branch without GH actions tests configured.

Results for commit 3b7f350

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

• No CT logs found
• HTML Documentation (Download HTML Docs)
• Windows Installer

// Erlang/OTP Github Action Bot

[magnetised]

@HansN documentation added - had to make assumptions about when this would be released which are probably wrong. Feedback on docs appreciated obvs. My types were a bit ad-hoc...

[HansN]

Thanks for the documentation. I'll check it!