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

treat logical.ErrRelativePath as 400 instead of 500 #14328

Merged
merged 3 commits into from
Mar 30, 2022
Merged

treat logical.ErrRelativePath as 400 instead of 500 #14328

merged 3 commits into from
Mar 30, 2022

Conversation

ccapurso
Copy link
Contributor

@ccapurso ccapurso commented Mar 1, 2022

The userpass auth backend may return a 500 Internal Server Error to attempts to read or write usernames that contain ... The underlying cause is a check performed in StorageView. SanityCheck. It is theoretically possible for other endpoints to result in a 500 response for a path that contains ... The policies endpoints, for example sys/policies/acl/:path, have their own error handling which results in returning a 400 for various errors:

❯ bin/vault read sys/policies/acl/foo..bar
Error reading sys/policies/acl/foo..bar: Error making API request.

URL: GET http://127.0.0.1:8200/v1/sys/policies/acl/foo..bar
Code: 400. Errors:

* failed to read policy: relative paths not supported

The proposed fix is to treat logical.ErrRelativePath as a 400. Rather than do this directly within the userpass logic, it is done at a higher level in the request handling logic so that it has broad coverage across any backend.

❯ bin/vault read auth/userpass/users/foo..bar
Error reading auth/userpass/users/foo..bar: Error making API request.

URL: GET http://127.0.0.1:8200/v1/auth/userpass/users/foo..bar
Code: 400. Errors:

* 1 error occurred:
        * relative paths not supported

@ccapurso ccapurso requested review from a team and mickael-hc March 1, 2022 21:20
@vercel vercel bot temporarily deployed to Preview – vault March 15, 2022 14:08 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook March 15, 2022 14:08 Inactive
@vercel vercel bot temporarily deployed to Preview – vault March 23, 2022 21:13 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook March 23, 2022 21:13 Inactive
@ccapurso ccapurso requested a review from HridoyRoy March 24, 2022 20:16
Copy link
Contributor

@HridoyRoy HridoyRoy left a comment

Choose a reason for hiding this comment

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

Lgtm!

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