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

vault login calls the token helper with get and expects that to succeed #23194

Open
ruuda opened this issue Sep 20, 2023 · 2 comments
Open

vault login calls the token helper with get and expects that to succeed #23194

ruuda opened this issue Sep 20, 2023 · 2 comments
Labels

Comments

@ruuda
Copy link

ruuda commented Sep 20, 2023

Describe the bug

When I run vault login, and a token helper is configured, Vault will call the token helper with get. This get might fail, the token helper might not have a token. The reason for running vault login in the first place, is to obtain a token and store it using the token helper. But even if the token helper has no token yet, Vault forces it to exit with exit code 0. If it exits with a nonzero exit code, then vault login fails, and we never get to the store step.

We can of course make the token helper exit with exit code 0, but then if you run e.g. vault kv get, and the token helper has no token, then the token helper no longer has the ability to make the kv get fail. Instead the kv get will fail with a misleading “permission denied” error. Permission is denied, because no X-Vault-Token was included in the request. Vault did not include a token, because the token helper did not return one but signalled success either way.

To Reproduce
Steps to reproduce the behavior:

  1. Configure a token helper in ~/.vault by putting e.g. token_helper = "/tmp/token_helper.sh" there.
  2. In that file, put the following and make the file executable:
    #!/usr/bin/bash
    echo "$@" >&2
    exit 1
  3. Run vault login.
  4. Observe how the script got called with get.
  5. Observe how returning 1 from that get prevented vault login from doing any login.

Expected behavior

I would expect vault login to not call the token helper with get. It should only call the token helper with store.

Once vault login does not call the token helper with get, it becomes possible to return 1 when a get lookup fails (because the token does not exist), and that enables the token helper to print a helpful error message in that case, instead of leaving the user with a misleading permission denied error.

Environment:

  • Vault Server Version (retrieve with vault status): irrelevant, we don’t even get tot he point where vault talks to a server
  • Vault CLI Version (retrieve with vault version): Vault v1.14.3 (cgo)
  • Server Operating System/Architecture: Linux 6.5.3 / x86-64

Vault server configuration file(s): irrelevant

@hghaf099
Copy link
Contributor

Thank you for reporting this issue! So, the concern with vault login seems reasonable as in the purpose of the login is to get a token in the first place. I just want to note that having a VAULT_TOKEN environment variable set to a wrong value does not prevent the login command to fail to retrieve a valid token. Similar to that is an invalid token in the token helper path or even if the token file does not exist, does not prevent the login to fail. So, it is really not the case that an invalid token causes an issue for the login to succeed. All that to say that if the token helper does a right thing to return a token (valid, invalid, empty), the login will go through. Having said that I do see that as an improvement for which a community PR would be greatly appreciated.

@ruuda
Copy link
Author

ruuda commented Sep 21, 2023

So, it is really not the case that an invalid token causes an issue for the login to succeed

Yes, this is true. But the current behavior of vault login means that the token helper is not allowed to signal “I don’t have a token”, it has to exit with exit code 0 even when it doesn’t have one, otherwise vault login is unusable. This is not a big issue, but it leads to a suboptimal user experience for all the commands that are not vault login, which do need a valid token. For those, the token helper already knows that they will fail because it doesn’t have a token, so the token helper could exit with a nonzero exit code and print a helpful message instructing the user to log in first. But instead, the token helper is required to exit with a zero exit code, and the user is presented with a permission error.

I’m not well versed in Go but I’ll see if I can fix this. #22257 is bugging me as well, maybe I can tackle both.

ruuda added a commit to ruuda/openbao that referenced this issue Apr 29, 2024
`vault login` used to call the token helper in `get` mode, because it
constructs an HTTP client, and the client automatically loads the token.
For almost everything that is the right thing to do, but for login, that
is the thing that is supposed to retrieve the token, and login itself
does not require the token. In fact, `vault login` would erase the token
on the client later on.

Calling the token helper in `get` mode is a problem, because if the
token helper fails, that blocks the login. But the token helper might
fail because it doesn't have a token yet.

This fixes hashicorp/vault#23194.

Signed-off-by: Ruud van Asseldonk <ruud@chorus.one>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants