From f77223bfe5792507c61c7ebf7ad416f2dc0fe9e9 Mon Sep 17 00:00:00 2001 From: akshya96 <87045294+akshya96@users.noreply.github.com> Date: Mon, 22 Nov 2021 17:06:59 -0800 Subject: [PATCH] Authenticate to "login" endpoint for non-existent mount path bug (#13162) * changing response from missing client token to permission denied * removing todo comment * fix tests * adding changelog * fixing changelog --- changelog/13162.txt | 3 +++ http/handler_test.go | 4 ++-- http/help_test.go | 4 ++-- http/sys_metrics_test.go | 4 ++-- vault/core_test.go | 4 ++-- vault/request_handling.go | 2 +- vault/router_test.go | 1 + 7 files changed, 13 insertions(+), 9 deletions(-) create mode 100644 changelog/13162.txt diff --git a/changelog/13162.txt b/changelog/13162.txt new file mode 100644 index 000000000..273a7e578 --- /dev/null +++ b/changelog/13162.txt @@ -0,0 +1,3 @@ +```release-note:bug +core: authentication to "login" endpoint for non-existent mount path returns permission denied with status code 403 +``` \ No newline at end of file diff --git a/http/handler_test.go b/http/handler_test.go index b57b49527..c937e7e7b 100644 --- a/http/handler_test.go +++ b/http/handler_test.go @@ -315,8 +315,8 @@ func TestHandler_MissingToken(t *testing.T) { if err != nil { t.Fatal(err) } - if resp.StatusCode != 400 { - t.Fatalf("expected code 400, got: %d", resp.StatusCode) + if resp.StatusCode != 403 { + t.Fatalf("expected code 403, got: %d", resp.StatusCode) } } diff --git a/http/help_test.go b/http/help_test.go index 4d4bc2c54..c3abfc86f 100644 --- a/http/help_test.go +++ b/http/help_test.go @@ -14,8 +14,8 @@ func TestHelp(t *testing.T) { TestServerAuth(t, addr, token) resp := testHttpGet(t, "", addr+"/v1/sys/mounts?help=1") - if resp.StatusCode != http.StatusBadRequest { - t.Fatal("expected bad request with no token") + if resp.StatusCode != http.StatusForbidden { + t.Fatal("expected permission denied with no token") } resp = testHttpGet(t, token, addr+"/v1/sys/mounts?help=1") diff --git a/http/sys_metrics_test.go b/http/sys_metrics_test.go index 6f1a7a62e..241caac2d 100644 --- a/http/sys_metrics_test.go +++ b/http/sys_metrics_test.go @@ -23,7 +23,7 @@ func TestSysMetricsUnauthenticated(t *testing.T) { // Default: Only authenticated access resp := testHttpGet(t, "", addr+"/v1/sys/metrics") - testResponseStatus(t, resp, 400) + testResponseStatus(t, resp, 403) resp = testHttpGet(t, token, addr+"/v1/sys/metrics") testResponseStatus(t, resp, 200) @@ -65,7 +65,7 @@ func TestSysPProfUnauthenticated(t *testing.T) { // Default: Only authenticated access resp := testHttpGet(t, "", addr+"/v1/sys/pprof/cmdline") - testResponseStatus(t, resp, 400) + testResponseStatus(t, resp, 403) resp = testHttpGet(t, token, addr+"/v1/sys/pprof/cmdline") testResponseStatus(t, resp, 200) diff --git a/vault/core_test.go b/vault/core_test.go index c607ffd6c..157d6db1e 100644 --- a/vault/core_test.go +++ b/vault/core_test.go @@ -472,10 +472,10 @@ func TestCore_HandleRequest_MissingToken(t *testing.T) { }, } resp, err := c.HandleRequest(namespace.RootContext(nil), req) - if err == nil || !errwrap.Contains(err, logical.ErrInvalidRequest.Error()) { + if err == nil || !errwrap.Contains(err, logical.ErrPermissionDenied.Error()) { t.Fatalf("err: %v", err) } - if resp.Data["error"] != "missing client token" { + if resp.Data["error"] != logical.ErrPermissionDenied.Error() { t.Fatalf("bad: %#v", resp) } } diff --git a/vault/request_handling.go b/vault/request_handling.go index 4cf9a762d..3a1bdb987 100644 --- a/vault/request_handling.go +++ b/vault/request_handling.go @@ -126,7 +126,7 @@ func (c *Core) fetchACLTokenEntryAndEntity(ctx context.Context, req *logical.Req // Ensure there is a client token if req.ClientToken == "" { - return nil, nil, nil, nil, &logical.StatusBadRequest{Err: "missing client token"} + return nil, nil, nil, nil, logical.ErrPermissionDenied } if c.tokenStore == nil { diff --git a/vault/router_test.go b/vault/router_test.go index 842d75f62..b7ade5ca2 100644 --- a/vault/router_test.go +++ b/vault/router_test.go @@ -373,6 +373,7 @@ func TestRouter_LoginPath(t *testing.T) { {"auth/foo/bar", false}, {"auth/foo/login", true}, {"auth/foo/login/", false}, + {"auth/invalid/login", false}, {"auth/foo/oauth", false}, {"auth/foo/oauth/", true}, {"auth/foo/oauth/redirect", true},