Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ldap] auth method fix request_timeout #11975

Merged
merged 6 commits into from
Jul 1, 2021
Merged

Conversation

fairclothjm
Copy link
Contributor

Description

This PR fixes a bug where the LDAP auth method does not implement the request_timeout configuration parameter, neither storing it, nor enforcing it for connections made to an LDAP server.

@vercel vercel bot temporarily deployed to Preview – vault July 1, 2021 15:38 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook July 1, 2021 15:38 Inactive
@fairclothjm
Copy link
Contributor Author

Manual Tests

Setup

First, navigate to the vault directory and build this branch for docker:

git checkout ldap-request-timeout
make docker-dev

Next, from the vault-tools directory on your machine checkout the ldap-local-docker-dev branch:

git checkout ldap-local-docker-dev
cd users/jodonnell/ldap-auth-example

Next, run the startup script with the dev argument which will use the docker image built in the first step:

./run.sh dev

Export env vars:

export VAULT_ADDR="http://localhost:8200"
export VAULT_TOKEN="root"

Test

vault auth enable ldap
vault read auth/ldap/config # request_timeout is 90, the default
vault write auth/ldap/config timeout_request=29
vault read auth/ldap/config # request_timeout is 29

Copy link
Member

@calvn calvn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of small comments, but looking good! PasswordlessMap() is called on config read, so I think that we were always storing request_timeout, just not returning it back.

sdk/helper/ldaputil/config_test.go Outdated Show resolved Hide resolved
sdk/helper/ldaputil/config_test.go Outdated Show resolved Hide resolved
Co-authored-by: Calvin Leung Huang <1883212+calvn@users.noreply.github.com>
@vercel vercel bot temporarily deployed to Preview – vault-storybook July 1, 2021 16:54 Inactive
Co-authored-by: Calvin Leung Huang <1883212+calvn@users.noreply.github.com>
@vercel vercel bot temporarily deployed to Preview – vault July 1, 2021 16:54 Inactive
@vercel vercel bot temporarily deployed to Preview – vault July 1, 2021 16:54 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook July 1, 2021 16:54 Inactive
changelog/11975.txt Outdated Show resolved Hide resolved
Co-authored-by: Calvin Leung Huang <1883212+calvn@users.noreply.github.com>
@vercel vercel bot temporarily deployed to Preview – vault-storybook July 1, 2021 17:47 Inactive
@vercel vercel bot temporarily deployed to Preview – vault July 1, 2021 17:47 Inactive
@fairclothjm fairclothjm merged commit de13b64 into main Jul 1, 2021
@calvn calvn deleted the ldap-request-timeout branch July 1, 2021 19:29
@calvn calvn added this to the 1.8 milestone Jul 1, 2021
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