Skip to content

Commit

Permalink
Review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
briankassouf committed Jul 5, 2019
1 parent d7df2b2 commit b327f24
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 37 deletions.
22 changes: 15 additions & 7 deletions http/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,32 +22,36 @@ func testHttpGet(t *testing.T, token string, addr string) *http.Response {
loggedToken = "<empty>"
}
t.Logf("Token is %s", loggedToken)
return testHttpData(t, "GET", token, addr, nil, false)
return testHttpData(t, "GET", token, addr, nil, false, 0)
}

func testHttpDelete(t *testing.T, token string, addr string) *http.Response {
return testHttpData(t, "DELETE", token, addr, nil, false)
return testHttpData(t, "DELETE", token, addr, nil, false, 0)
}

// Go 1.8+ clients redirect automatically which breaks our 307 standby testing
func testHttpDeleteDisableRedirect(t *testing.T, token string, addr string) *http.Response {
return testHttpData(t, "DELETE", token, addr, nil, true)
return testHttpData(t, "DELETE", token, addr, nil, true, 0)
}

func testHttpPostWrapped(t *testing.T, token string, addr string, body interface{}, wrapTTL time.Duration) *http.Response {
return testHttpData(t, "POST", token, addr, body, false, wrapTTL)
}

func testHttpPost(t *testing.T, token string, addr string, body interface{}) *http.Response {
return testHttpData(t, "POST", token, addr, body, false)
return testHttpData(t, "POST", token, addr, body, false, 0)
}

func testHttpPut(t *testing.T, token string, addr string, body interface{}) *http.Response {
return testHttpData(t, "PUT", token, addr, body, false)
return testHttpData(t, "PUT", token, addr, body, false, 0)
}

// Go 1.8+ clients redirect automatically which breaks our 307 standby testing
func testHttpPutDisableRedirect(t *testing.T, token string, addr string, body interface{}) *http.Response {
return testHttpData(t, "PUT", token, addr, body, true)
return testHttpData(t, "PUT", token, addr, body, true, 0)
}

func testHttpData(t *testing.T, method string, token string, addr string, body interface{}, disableRedirect bool) *http.Response {
func testHttpData(t *testing.T, method string, token string, addr string, body interface{}, disableRedirect bool, wrapTTL time.Duration) *http.Response {
bodyReader := new(bytes.Buffer)
if body != nil {
enc := json.NewEncoder(bodyReader)
Expand All @@ -68,6 +72,10 @@ func testHttpData(t *testing.T, method string, token string, addr string, body i

req.Header.Set("Content-Type", "application/json")

if wrapTTL > 0 {
req.Header.Set("X-Vault-Wrap-TTL", wrapTTL.String())
}

if len(token) != 0 {
req.Header.Set(consts.AuthHeaderName, token)
}
Expand Down
32 changes: 16 additions & 16 deletions http/logical_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -406,34 +406,34 @@ func TestLogical_Audit_invalidWrappingToken(t *testing.T) {
}

{
resp := testHttpPostWrapped(t, root, addr+"/v1/auth/token/create", nil, 10*time.Second)
testResponseStatus(t, resp, 200)
body := map[string]interface{}{}
testResponseBody(t, resp, &body)

wrapToken := body["wrap_info"].(map[string]interface{})["token"].(string)

// Make a wrapping/unwrap request with an invalid token
resp := testHttpPost(t, root, addr+"/v1/sys/wrapping/unwrap", map[string]interface{}{
"token": "foo",
resp = testHttpPost(t, root, addr+"/v1/sys/wrapping/unwrap", map[string]interface{}{
"token": wrapToken,
})
testResponseStatus(t, resp, 400)
body := map[string][]string{}
testResponseBody(t, resp, &body)
if body["errors"][0] != "wrapping token is not valid or does not exist" {
t.Fatal(body)
}
testResponseStatus(t, resp, 200)

// Check the audit trail on request and response
if len(noop.ReqAuth) != 2 {
if len(noop.ReqAuth) != 3 {
t.Fatalf("bad: %#v", noop)
}
auth := noop.ReqAuth[1]
auth := noop.ReqAuth[2]
if auth.ClientToken != root {
t.Fatalf("bad client token: %#v", auth)
}
if len(noop.Req) != 2 || noop.Req[1].Path != "sys/wrapping/unwrap" {
t.Fatalf("bad:\ngot:\n%#v", noop.Req[1])
if len(noop.Req) != 3 || noop.Req[2].Path != "sys/wrapping/unwrap" {
t.Fatalf("bad:\ngot:\n%#v", noop.Req[2])
}

if len(noop.ReqErrs) != 2 {
// Make sure there is only one error in the logs
if noop.ReqErrs[1] != nil || noop.ReqErrs[2] != nil {
t.Fatalf("bad: %#v", noop.RespErrs)
}
if noop.ReqErrs[1] != consts.ErrInvalidWrappingToken {
t.Fatalf("bad: %#v", noop.ReqErrs)
}
}
}
17 changes: 3 additions & 14 deletions vault/wrapping.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,16 +317,6 @@ func (c *Core) ValidateWrappingToken(ctx context.Context, req *logical.Request)
// Perform audit logging before returning if there's an issue with checking
// the wrapping token
if err != nil || !valid {
// Get non-HMAC'ed request data keys
var nonHMACReqDataKeys []string
entry := c.router.MatchingMountEntry(ctx, req.Path)
if entry != nil {
// Get and set ignored HMAC'd value.
if rawVals, ok := entry.synthesizedConfigCache.Load("audit_non_hmac_request_keys"); ok {
nonHMACReqDataKeys = rawVals.([]string)
}
}

// We log the Auth object like so here since the wrapping token can
// come from the header, which gets set as the ClientToken
auth := &logical.Auth{
Expand All @@ -335,12 +325,11 @@ func (c *Core) ValidateWrappingToken(ctx context.Context, req *logical.Request)
}

logInput := &logical.LogInput{
Auth: auth,
Request: req,
NonHMACReqDataKeys: nonHMACReqDataKeys,
Auth: auth,
Request: req,
}
if err != nil {
logInput.OuterErr = errwrap.Wrapf("error validating wrapping token: {{err}}", err)
logInput.OuterErr = errors.New("error validating wrapping token")
}
if !valid {
logInput.OuterErr = consts.ErrInvalidWrappingToken
Expand Down

0 comments on commit b327f24

Please sign in to comment.