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
Accessor paths for lookup and revocation of tokens #1188
Changes from 10 commits
38a5d75
a713720
4ed3a85
c7033b1
bb927e3
5dcc6f0
048f3b2
9da2929
edfba16
7b99652
2a35de8
c7c9e0b
928d872
16c4b52
a546823
76900d6
da9ad9c
d1d37d5
64bc542
b8bd534
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ import ( | |
"net/url" | ||
"strings" | ||
|
||
"github.com/hashicorp/errwrap" | ||
"github.com/hashicorp/vault/logical" | ||
"github.com/hashicorp/vault/vault" | ||
) | ||
|
@@ -34,6 +35,7 @@ func Handler(core *vault.Core) http.Handler { | |
mux.Handle("/v1/sys/rekey/update", handleSysRekeyUpdate(core)) | ||
mux.Handle("/v1/sys/capabilities", handleSysCapabilities(core)) | ||
mux.Handle("/v1/sys/capabilities-self", handleSysCapabilities(core)) | ||
mux.Handle("/v1/sys/capabilities-accessor", handleSysCapabilitiesAccessor(core)) | ||
mux.Handle("/v1/sys/", handleLogical(core, true)) | ||
mux.Handle("/v1/", handleLogical(core, false)) | ||
|
||
|
@@ -79,7 +81,7 @@ func request(core *vault.Core, w http.ResponseWriter, rawReq *http.Request, r *l | |
return resp, false | ||
} | ||
if err != nil { | ||
respondError(w, http.StatusInternalServerError, err) | ||
respondErrorStatus(w, err) | ||
return resp, false | ||
} | ||
|
||
|
@@ -139,6 +141,18 @@ func requestAuth(r *http.Request, req *logical.Request) *logical.Request { | |
return req | ||
} | ||
|
||
// Determines the type of the error being returned and sets the HTTP | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens if you have multiple types of errors generated? Which one trumps the other? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Only the latest error type will be considered. In order for this to work as expected, we should follow a practice, i.e. when Since we were only generating internal error till date, I guess it should be fine if we start this practice now. This might not affect any of the existing use-cases. The status code is not being set anywhere after any call crosses core, except In case of multiple errors using multierror, the default case will be Also, since multierror implements error interface, user will get to see the all the errors in a formatted manner, and hopefully not complain for the proper status code. |
||
// status code appropriately | ||
func respondErrorStatus(w http.ResponseWriter, err error) { | ||
status := http.StatusInternalServerError | ||
switch { | ||
// Keep adding more error types here to appropriate the status codes | ||
case errwrap.ContainsType(err, new(vault.StatusBadRequest)): | ||
status = http.StatusBadRequest | ||
} | ||
respondError(w, status, err) | ||
} | ||
|
||
func respondError(w http.ResponseWriter, status int, err error) { | ||
// Adjust status code when sealed | ||
if err == vault.ErrSealed { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this is going to be an annoying code change, but...I don't like "AccessorID". I used "Accessor" in the RFC because it's a noun; calling it an AccessorID means that it's an ID of the accessor, which is not what it is. "AccessID" could work, but I preferred Accessor over AccessID because AccessID could be confused as the actual important value (a value to "access Vault").
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not an annoying change :) I'll do it.