Skip to content

Commit

Permalink
Backport of treat logical.ErrRelativePath as 400 instead of 500 into …
Browse files Browse the repository at this point in the history
…release/1.8.x (#14779)

* no-op commit due to failed cherry-picking

* treat logical.ErrRelativePath as 400 instead of 500

(cherry picked from commit c1e527d)

* add changelog entry

(cherry picked from commit 550d3aa)

* return UserError for logical.ErrRelativePath

(cherry picked from commit 154a3a7)

* add missing import

Co-authored-by: temp <temp@hashicorp.com>
Co-authored-by: Chris Capurso <1036769+ccapurso@users.noreply.github.com>
  • Loading branch information
3 people committed Mar 30, 2022
1 parent c2bd0fd commit 55f8f20
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 0 deletions.
3 changes: 3 additions & 0 deletions changelog/14328.txt
@@ -0,0 +1,3 @@
```release-note:change
core: A request that fails path validation due to relative path check will now be responded to with a 400 rather than 500.
```
74 changes: 74 additions & 0 deletions http/logical_test.go
Expand Up @@ -14,6 +14,8 @@ import (
"testing"
"time"

"github.com/hashicorp/vault/api"
credUserpass "github.com/hashicorp/vault/builtin/credential/userpass"
"github.com/hashicorp/vault/internalshared/configutil"
"github.com/hashicorp/vault/sdk/helper/consts"
"github.com/hashicorp/vault/sdk/helper/logging"
Expand Down Expand Up @@ -499,3 +501,75 @@ func TestLogical_ShouldParseForm(t *testing.T) {
}
}
}

func TestLogical_ErrRelativePath(t *testing.T) {
coreConfig := &vault.CoreConfig{
CredentialBackends: map[string]logical.Factory{
"userpass": credUserpass.Factory,
},
}

cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{
HandlerFunc: Handler,
})

cluster.Start()
defer cluster.Cleanup()

cores := cluster.Cores

core := cores[0].Core
c := cluster.Cores[0].Client
vault.TestWaitActive(t, core)

err := c.Sys().EnableAuthWithOptions("userpass", &api.EnableAuthOptions{
Type: "userpass",
})
if err != nil {
t.Fatalf("failed to enable userpass, err: %v", err)
}

resp, err := c.Logical().Read("auth/userpass/users/user..aaa")

if err == nil || resp != nil {
t.Fatalf("expected read request to fail, resp: %#v, err: %v", resp, err)
}

respErr, ok := err.(*api.ResponseError)

if !ok {
t.Fatalf("unexpected error type, err: %#v", err)
}

if respErr.StatusCode != 400 {
t.Errorf("expected 400 response for read, actual: %d", respErr.StatusCode)
}

if !strings.Contains(respErr.Error(), logical.ErrRelativePath.Error()) {
t.Errorf("expected response for read to include %q", logical.ErrRelativePath.Error())
}

data := map[string]interface{}{
"password": "abc123",
}

resp, err = c.Logical().Write("auth/userpass/users/user..aaa", data)

if err == nil || resp != nil {
t.Fatalf("expected write request to fail, resp: %#v, err: %v", resp, err)
}

respErr, ok = err.(*api.ResponseError)

if !ok {
t.Fatalf("unexpected error type, err: %#v", err)
}

if respErr.StatusCode != 400 {
t.Errorf("expected 400 response for write, actual: %d", respErr.StatusCode)
}

if !strings.Contains(respErr.Error(), logical.ErrRelativePath.Error()) {
t.Errorf("expected response for write to include %q", logical.ErrRelativePath.Error())
}
}
2 changes: 2 additions & 0 deletions sdk/logical/response_util.go
Expand Up @@ -118,6 +118,8 @@ func RespondErrorCommon(req *Request, resp *Response, err error) (int, error) {
statusCode = http.StatusTooManyRequests
case errwrap.Contains(err, ErrMissingRequiredState.Error()):
statusCode = http.StatusPreconditionFailed
case errwrap.Contains(err, ErrRelativePath.Error()):
statusCode = http.StatusBadRequest
}
}

Expand Down
5 changes: 5 additions & 0 deletions vault/request_handling.go
Expand Up @@ -299,6 +299,8 @@ func (c *Core) checkToken(ctx context.Context, req *logical.Request, unauth bool
case logical.ErrUnsupportedPath:
// fail later via bad path to avoid confusing items in the log
checkExists = false
case logical.ErrRelativePath:
return nil, te, errutil.UserError{Err: err.Error()}
case nil:
if existsResp != nil && existsResp.IsError() {
return nil, te, existsResp.Error()
Expand Down Expand Up @@ -752,6 +754,9 @@ func (c *Core) handleRequest(ctx context.Context, req *logical.Request) (retResp

// Validate the token
auth, te, ctErr := c.checkToken(ctx, req, false)
if ctErr == logical.ErrRelativePath {
return logical.ErrorResponse(ctErr.Error()), nil, ctErr
}
if ctErr == logical.ErrPerfStandbyPleaseForward {
return nil, nil, ctErr
}
Expand Down

0 comments on commit 55f8f20

Please sign in to comment.