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

Confusing error message when looking up invalid tokens #2020

Closed
xh3b4sd opened this issue Oct 19, 2016 · 6 comments
Closed

Confusing error message when looking up invalid tokens #2020

xh3b4sd opened this issue Oct 19, 2016 · 6 comments

Comments

@xh3b4sd
Copy link

xh3b4sd commented Oct 19, 2016

I create a token.

$ vault token-create
Key             Value
---             -----
token           <token>
token_accessor  <accessor>
token_duration  2592000
token_renewable true
token_policies  [default]

I lookup the token.

$ vault token-lookup <token>
Key                 Value
---                 -----
accessor            <accessor>
creation_time       1476883701
creation_ttl        2592000
display_name        token
explicit_max_ttl    0
id                  <id>
meta                <nil>
num_uses            0
orphan              false
path                auth/token/create
policies            [default]
renewable           true
role
ttl                 2591991

I revoke the token. The message is cool. All good.

$ vault token-revoke <token>
Success! Token revoked if it existed.

When I lookup the token again after it was revoked I get an error message. I as a user am not sure if I did something wrong.

$ vault token-lookup <token>
error looking up token: Error making API request.

URL: GET https://<endpoint>/v1/auth/token/lookup/<token>
Code: 400. Errors:

* bad token

When I lookup foo I get the same error.

$ vault token-lookup foo
error looking up token: Error making API request.

URL: GET https://<endpoint>/v1/auth/token/lookup/foo
Code: 400. Errors:

* bad token

IMO an error should only be shown in case something went wrong with the system. Currently the system gives me the impression I did something wrong. I would rather expect something like this.

Success! Token not found.
@vishalnayak
Copy link
Member

@xh3b4sd I do understand the reasoning behind your suggestion, but Success! Token not found can be more confusing to many as well, since it both says Success! and Token not found.

In comparison, a combination of 400 status code and the message bad token relatively better indicates the problem.

@xh3b4sd
Copy link
Author

xh3b4sd commented Oct 20, 2016

@vishalnayak you say you understand the issue but then you close it. I don't understand this behaviour. Success! Token not found was one suggestion I made. I am not emotionally bound to this specific string in any sense. The issue is that an error is returned when I did nothing wrong. So IMO the issue should be reopened and fixed. Closing issues without having any discussion feels always very rude to me, like you don't care at all about the opinion and concerns other people raise. I spend my time to improve a project. Now I think I could have spent my time better than with improving Vault.

@jefferai
Copy link
Member

@xh3b4sd There is a difference between not caring and disagreeing. It is not that we do not care about your opinion, but we disagree that returning a success error code when an invalid token is provided is the right thing to do. You're looking at it purely from a human perspective, but Vault is API-driven, and when a program thinks it has a valid token and tries to look up its properties, the right behavior if the token is actually invalid is to send a 4XX as this is how errors are normally triggered in client code. It is much harder to reason about seeing success and then having to realize that in fact you need to be looking for and parsing an error message.

This behavior will not be changed.

@xh3b4sd
Copy link
Author

xh3b4sd commented Oct 20, 2016

I did not suggest to return a success status code. The 4xx is reasonable and correct. It is only that the user experience implies that there is a 5xx and you need to carefully read and understand that Code: 400 means that the token does not exist anymore and an actual HTTP status code is talked about. In HTTP there should further 404 be used in case something cannot be found. Vault might be API driven and I am ok with getting insights into all the API details when I want this, but I am dealing with a command line tool that provides a user interface. It is also not consistent to get nice human readable sentences as response on one command and a confusing debug output by default on another. All this makes really no sense to me. This is why I raised concerns.

@jefferai
Copy link
Member

It is only that the user experience implies that there is a 5xx

I don't follow that at all. That would be an internal server error which is very clearly not returned here.

In HTTP there should further 404 be used in case something cannot be found.

Not quite. 404 means that the endpoint can't be found. You're using an old client, but current Vault CLI uses a POST with the token in the body. The endpoint is found; it's the data that's bad. (Current Vault also returns 403 here, not 400, to better reflect that it's not an invalid request per se, it's just that the data supplied does not correspond to an actual authentication token).

It is also not consistent to get nice human readable sentences as response on one command and a confusing debug output by default on another.

All Vault errors respond the same way. It's not debug output, it's giving the user the information needed to figure it out.

@xh3b4sd
Copy link
Author

xh3b4sd commented Oct 20, 2016

Ok, got you. I had a walk and thought about what you said. Your arguments make sense though. It is only that I am as the dumbest user are confused about this. Though I don't really know how to improve this now. Lets move on wth our lives. Thanks for your time @vishalnayak and @jefferai anyway. And thanks for bearing with me.

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

No branches or pull requests

3 participants