Add client certs

[Gisson]

Follow up from PR #7578

[hashicorp-cla]

All committers have signed the CLA.

[Gisson]

The pipeline is failing due to missing attribute AnonymousGroupSearch which was added in this PR (https://github.com/hashicorp/vault/pull/8365/files#diff-e4cb5edff2f9fd14acfe82cfee23f6acR356). What can I do to get the pipeline to pass?

[stephenrjohnson]

Thanks everyone for this work

[Gisson]

I've tested the both the client certificate authentication and the group membership mappings to Vault policies. Everything seems to be ok.

[AeroNotix]

The pipeline is failing due to missing attribute AnonymousGroupSearch which was added in this PR (https://github.com/hashicorp/vault/pull/8365/files#diff-e4cb5edff2f9fd14acfe82cfee23f6acR356). What can I do to get the pipeline to pass?

Odd that this is failing for this reason... clearly the PR adds that field. Might be some odd dependency caching? Can you try running the tests locally to see?

[Gisson]

@AeroNotix I was able to build on my linux machine.
Output:

:~/vault$ make -j6 bootstrap dev
Installing/Updating golang.org/x/tools/cmd/goimports

Installing/Updating github.com/elazarl/go-bindata-assetfs/...
Installing/Updating github.com/hashicorp/go-bindata/...
Installing/Updating github.com/mitchellh/gox
Installing/Updating github.com/kardianos/govendor
Installing/Updating github.com/client9/misspell/cmd/misspell
Installing/Updating github.com/golangci/golangci-lint/cmd/golangci-lint
==> Checking that build is using go version >= 1.13.7...
==> Using go version 1.13.7...
==> Removing old directory...
==> Building...
Number of parallel builds: 1

-->     linux/amd64: github.com/hashicorp/vault

==> Results:
total 117M
-rwxrwxr-x 1 ubuntu ubuntu 117M Feb 17 17:24 vault

Machine details:

:~/vault$ cat /etc/os-release
NAME="Ubuntu"
VERSION="18.04.2 LTS (Bionic Beaver)"
ID=ubuntu
ID_LIKE=debian
PRETTY_NAME="Ubuntu 18.04.2 LTS"
VERSION_ID="18.04"
HOME_URL="https://www.ubuntu.com/"
SUPPORT_URL="https://help.ubuntu.com/"
BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/"
PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy"
VERSION_CODENAME=bionic
UBUNTU_CODENAME=bionic

[AeroNotix]

@Gisson I meant the tests.

[Gisson]

@AeroNotix In the CI the stage that failed was the build one that's why I posted that. However I'm running all the tests now.

[AeroNotix]

Indeed, just wanted to know ahead of time whether the actual tests pass/fail with mine (and your) changes. I remember when I implemented this code there were no issues with CI/tests so maybe something changed with how hashicorp run stuff in CI? Who knows. Probably best to wait for a hashicorper to say what the issue is.

As a quick test - could you try kicking the ci by amending with no changes your previous commit and force pushing?

[Gisson]

@AeroNotix I've make test and they all passed.

I'll try doing what you said and kicking the the ci.

[Gisson]

After a little digging around, I found that the (obvious...) problem lied in GO111MODULE variable set in Circleci config.yml. But this change would affect the project as a whole.

[AeroNotix]

@michelvocks @jefferai @noelledaley any tips for getting the CI to pass?

[michelvocks]

@Gisson Thanks! I forgot to mention that you also have to add anonymous_group_search to the password less map here: https://github.com/hashicorp/vault/blob/master/sdk/helper/ldaputil/config.go#L353-L370

[michelvocks]

@Gisson This looks really good. Thanks for all the work. I think we are ready to get that merged as soon as both comments from @noelledaley have been addressed.

[Gisson]

@michelvocks I've addressed the issues. Are we ready for the merge?

[andaley]

@Gisson thank you for making these updates! everything on the UI side looks pretty good. the one thing i noticed is that after i've uploaded a Client Certificate and Client Key via the UI and make a curl request to http://127.0.0.1:8200/v1/auth/ldap/config, the API doesn't return the client_tls_cert or client_tls_key. i think not returning those keys is intentional, but i'll let @michelvocks confirm and give you a final approval.

again, thanks for your work on this!

[michelvocks]

@Gisson Yes! Thanks a lot for your effort. We are currently in a code-freeze period before the 1.4 Beta release which will delay the merge of this PR for a few days but overall the PR looks good.

@noelledaley Yes, we did that intentionally. In my opinion, the tls cert and tls key should not be returned.

[kalafut]

@Gisson @AeroNotix Thanks for this! One question we had is whether it would be appropriate to require that TLS certs are configured if anonymous_group_search is true?

[AeroNotix]

@kalafut I think that entirely depends on the LDAP service in use. For Google it may make sense, for other LDAP choochers it might not. It needs testing with a bunch out there. I've been far removed from this for a chunk o' time so I would need to revisit it.

Thanks for merging this though, greatly appreciated.

Thanks especially to @Gisson for walking this over the line and putting a big beautiful bow on it. Good job. Great to see open-source in action! I hope our paths cross again.

[kalafut]

LGTM. Thanks!