On Sat, Apr 14, 2018 at 3:48 AM, Julian Markwort
<julian.markw...@uni-muenster.de> wrote:
> [a patch]

Hello Julian,

Could you please post a rebased patch?

I haven't reviewed or tested any code yet, but here's some proof-reading:

+       This behaviour is similar to the cert autentication method

"behavior" (our manual is written in en_US, "cd doc/src/sgml ; git
grep behavior | wc -l" -> 895, "git grep behaviour" -> 0).



+   chain, but it will also check whether the username or it's
+   mapping match the common name (CN) of the provided certificate.



+   Note that certificate chain validation  is always ensured when
+   <literal>cert</literal> authentication method is used

extra space

when *the* ...

+   In this case, the <literal>CN</literal> (nommon name) provided in

"common name"

+   <literal>CN</literal> (Common Name) in the certificate matches

"common"? (why a capital letter here?)

This line isn't modified by your patch, but I saw it while in
proof-reading mode:

  *err_msg = "clientcert can not be set to 0 when using \"cert\"

I think "can not" is usually written "cannot"?

> slightly offtopic opinion:
> While creating the test cases, I stumbled upon the problem of missing
> depencies to run the tests...
> It's complicated enough that the binaries used by these perl tests are not
> named similar to the packages which provide them (the 'prove' binary is
> supplied by 'Test-Harness'), so maybe in the interest of providing a lower
> entry-barrier to running these tests, we could give a more detailed error
> message in the configure script, when using --enable-tap-tests ?

Yeah.  The packages to install depend on your operating system, and in
some cases (macOS, Windows?) which bolt-on package thingamajig you
use, though.  Perhaps the READMEs could be improved with details for
systems we have reports about (like the recently added "Requirements"
section of src/test/ldap/README).

Thomas Munro

Reply via email to