From 5d724d619df41d759ab4cc4426a6414eb3a2ef9d Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Thu, 12 Oct 2017 13:51:39 -0700 Subject: [PATCH 1/3] ACL command options --- command/agent/command.go | 15 +++++++++++++++ website/source/docs/commands/agent.html.md.erb | 2 ++ 2 files changed, 17 insertions(+) diff --git a/command/agent/command.go b/command/agent/command.go index d88eb56da..18d620678 100644 --- a/command/agent/command.go +++ b/command/agent/command.go @@ -69,6 +69,7 @@ func (c *Command) readConfig() *Config { Ports: &Ports{}, Server: &ServerConfig{}, Vault: &config.VaultConfig{}, + ACL: &ACLConfig{}, } flags := flag.NewFlagSet("agent", flag.ContinueOnError) @@ -166,6 +167,10 @@ func (c *Command) readConfig() *Config { }), "vault-tls-skip-verify", "") flags.StringVar(&cmdConfig.Vault.TLSServerName, "vault-tls-server-name", "", "") + // ACL options + flags.BoolVar(&cmdConfig.ACL.Enabled, "acl-enabled", false, "") + flags.StringVar(&cmdConfig.ACL.ReplicationToken, "acl-replication-token", "", "") + if err := flags.Parse(c.args); err != nil { return nil } @@ -984,6 +989,16 @@ Client Options: The default speed for network interfaces in MBits if the link speed can not be determined dynamically. +ACL Options: + + -acl-enabled + Specifies whether the agent should enable ACLs. + + -acl-replication-token + The replication token for servers to use when replicating from the + authoratative region. The token must be a valid management token from the + authoratative region. + Consul Options: -consul-address= diff --git a/website/source/docs/commands/agent.html.md.erb b/website/source/docs/commands/agent.html.md.erb index 8552cd07e..9055b68a9 100644 --- a/website/source/docs/commands/agent.html.md.erb +++ b/website/source/docs/commands/agent.html.md.erb @@ -24,6 +24,8 @@ via CLI arguments. The `agent` command accepts the following arguments: * `-alloc-dir=`: Equivalent to the Client [alloc_dir](#alloc_dir) config option. +* `-acl-enabled`: Equivalent to the ACL [enabled](/docs/agent/configuration/acl.html#enabled) config option. +* `-acl-replication-token`: Equivalent to the ACL [replication_token](/docs/agent/configuration/acl.html#replication_token) config option. * `-bind=
`: Equivalent to the [bind_addr](#bind_addr) config option. * `-bootstrap-expect=`: Equivalent to the [bootstrap_expect](#bootstrap_expect) config option. From 21c2ba33e51c4574cfce02644e9bc48ee3d84200 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Thu, 12 Oct 2017 14:07:52 -0700 Subject: [PATCH 2/3] 403 instead of 500 for permission denied --- command/agent/command.go | 6 +++--- command/agent/http.go | 2 ++ command/agent/http_test.go | 18 +++++++++++++++++ command/agent/job_endpoint_test.go | 31 ++++++++++++++++++++++++++++++ 4 files changed, 54 insertions(+), 3 deletions(-) diff --git a/command/agent/command.go b/command/agent/command.go index 18d620678..d72b74612 100644 --- a/command/agent/command.go +++ b/command/agent/command.go @@ -995,9 +995,9 @@ ACL Options: Specifies whether the agent should enable ACLs. -acl-replication-token - The replication token for servers to use when replicating from the - authoratative region. The token must be a valid management token from the - authoratative region. + The replication token for servers to use when replicating from the + authoratative region. The token must be a valid management token from the + authoratative region. Consul Options: diff --git a/command/agent/http.go b/command/agent/http.go index 79641f0ba..11b6e52b0 100644 --- a/command/agent/http.go +++ b/command/agent/http.go @@ -301,6 +301,8 @@ func (s *HTTPServer) wrap(handler func(resp http.ResponseWriter, req *http.Reque code := 500 if http, ok := err.(HTTPCodedError); ok { code = http.Code() + } else if err.Error() == structs.ErrPermissionDenied.Error() { + code = 403 } resp.WriteHeader(code) resp.Write([]byte(err.Error())) diff --git a/command/agent/http_test.go b/command/agent/http_test.go index 1547d51bb..3017046cd 100644 --- a/command/agent/http_test.go +++ b/command/agent/http_test.go @@ -20,6 +20,7 @@ import ( "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/nomad/structs/config" "github.com/hashicorp/nomad/testutil" + "github.com/stretchr/testify/assert" "github.com/ugorji/go/codec" ) @@ -218,6 +219,23 @@ func testPrettyPrint(pretty string, prettyFmt bool, t *testing.T) { } } +func TestPermissionDenied(t *testing.T) { + s := makeHTTPServer(t, func(c *Config) { + c.ACL.Enabled = true + }) + defer s.Shutdown() + + resp := httptest.NewRecorder() + handler := func(resp http.ResponseWriter, req *http.Request) (interface{}, error) { + return nil, structs.ErrPermissionDenied + } + + urlStr := "/v1/job/foo" + req, _ := http.NewRequest("GET", urlStr, nil) + s.Server.wrap(handler)(resp, req) + assert.Equal(t, resp.Code, 403) +} + func TestParseWait(t *testing.T) { t.Parallel() resp := httptest.NewRecorder() diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go index 779463970..835cc7bde 100644 --- a/command/agent/job_endpoint_test.go +++ b/command/agent/job_endpoint_test.go @@ -211,6 +211,37 @@ func TestHTTP_JobsRegister_ACL(t *testing.T) { }) } +// Test that permission denied gets a 403 +func TestHTTP_JobsRegister_ACL_Bad(t *testing.T) { + t.Parallel() + httpACLTest(t, nil, func(s *TestAgent) { + // Create the job + job := api.MockJob() + args := api.JobRegisterRequest{ + Job: job, + WriteRequest: api.WriteRequest{ + Region: "global", + }, + } + buf := encodeReq(args) + + // Make the HTTP request + req, err := http.NewRequest("PUT", "/v1/jobs", buf) + if err != nil { + t.Fatalf("err: %v", err) + } + respW := httptest.NewRecorder() + + // Make the request + obj, err := s.Server.JobsRequest(respW, req) + if err != nil { + t.Fatalf("err: %v", err) + } + assert.Nil(t, obj) + assert.Contains(t, err.Error(), "403") + }) +} + func TestHTTP_JobsRegister_Defaulting(t *testing.T) { t.Parallel() httpTest(t, nil, func(s *TestAgent) { From f1965d63052f673ee57bf9ece6e3d2abaa96d3c7 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Thu, 12 Oct 2017 15:39:05 -0700 Subject: [PATCH 3/3] Handle invalid token as well --- command/agent/http.go | 8 ++++++-- command/agent/http_test.go | 17 +++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/command/agent/http.go b/command/agent/http.go index 11b6e52b0..6a542a5e9 100644 --- a/command/agent/http.go +++ b/command/agent/http.go @@ -301,9 +301,13 @@ func (s *HTTPServer) wrap(handler func(resp http.ResponseWriter, req *http.Reque code := 500 if http, ok := err.(HTTPCodedError); ok { code = http.Code() - } else if err.Error() == structs.ErrPermissionDenied.Error() { - code = 403 + } else { + switch err.Error() { + case structs.ErrPermissionDenied.Error(), structs.ErrTokenNotFound.Error(): + code = 403 + } } + resp.WriteHeader(code) resp.Write([]byte(err.Error())) return diff --git a/command/agent/http_test.go b/command/agent/http_test.go index 3017046cd..be73eba95 100644 --- a/command/agent/http_test.go +++ b/command/agent/http_test.go @@ -236,6 +236,23 @@ func TestPermissionDenied(t *testing.T) { assert.Equal(t, resp.Code, 403) } +func TestTokenNotFound(t *testing.T) { + s := makeHTTPServer(t, func(c *Config) { + c.ACL.Enabled = true + }) + defer s.Shutdown() + + resp := httptest.NewRecorder() + handler := func(resp http.ResponseWriter, req *http.Request) (interface{}, error) { + return nil, structs.ErrTokenNotFound + } + + urlStr := "/v1/job/foo" + req, _ := http.NewRequest("GET", urlStr, nil) + s.Server.wrap(handler)(resp, req) + assert.Equal(t, resp.Code, 403) +} + func TestParseWait(t *testing.T) { t.Parallel() resp := httptest.NewRecorder()