diff --git a/DEVELOP.md b/DEVELOP.md index 540972e5..eb0bc690 100644 --- a/DEVELOP.md +++ b/DEVELOP.md @@ -160,3 +160,12 @@ if err != nil { } assert.Equal(t, "what we expect", result.A, "result had the wrong value for a") ``` + +### Code Coverage + +Run this to both generate a coverage output file usable by go tools, +`coverage.out`, and open it using the go coverage tool to visualize line-by-line +coverage. +``` +make coverage-viz +``` diff --git a/Makefile b/Makefile index 5e3e6a82..6fdf0659 100644 --- a/Makefile +++ b/Makefile @@ -2,24 +2,45 @@ GOYACC ?= goyacc _default: bin/arborist -test: bin/arborist db-test +bin/arborist: arborist/*.go # help: run the server + go build -o bin/arborist + +test: bin/arborist db-test # help: run the tests go test -v ./arborist/ -bin/arborist: arborist/*.go - go build -o bin/arborist +coverage-viz: coverage # help: generate test coverage file and run coverage visualizer + go tool cover --html=coverage.out -up: upgrade +coverage: test # help: generate test coverage file + go test --coverprofile=coverage.out ./arborist/ + @# Remove auto-generated files from test coverage results + @mv coverage.out tmp + @grep -v "resource_rules.go" tmp > coverage.out + @rm tmp + +db-test: $(which psql) # help: set up the database for testing (run automatically by `test`) + createdb || true + ./migrations/latest + +up: upgrade # help: try to migrate the database to the next more recent version upgrade: ./migrations/up -down: downgrade +down: downgrade # help: try to revert the database to the previous version downgrade: ./migrations/down -db-test: $(which psql) - createdb || true - ./migrations/latest - arborist/resource_rules.go: arborist/resource_rules.y which $(GOYACC) || go get golang.org/x/tools/cmd/goyacc $(GOYACC) -o arborist/resource_rules.go arborist/resource_rules.y + +# You can add a comment following a make target starting with "# help:" to have +# `make help` include that comment in its output. +help: # help: show this help + @echo "Makefile utilities for arborist. Note that most require you to have already" + @echo "exported the necessary postgres variables: \`PGDATABASE\`, \`PGUSER\`, \`PGHOST\`," + @echo "and \`PGPORT\`. Set \`PGSSLMODE=disable\` if not using SSL. See README for details." + @echo "" + @echo "The default command is bin/arborist." + @echo "" + @grep -h "^.*:.*# help" $(MAKEFILE_LIST) | grep -v grep | sed -e "s/:.*# help:/:/" diff --git a/README.md b/README.md index b76f4d85..f348454a 100644 --- a/README.md +++ b/README.md @@ -181,3 +181,9 @@ Run all the tests: ```bash go test ./... ``` + +### Code Coverage + +Use `make coverage` to generate a `coverage.out` file which can be used with go +testing and coverage tools. `make coverage-viz` will additionally open it using +the go coverage tools to visualize line-by-line coverage. diff --git a/arborist/auth.go b/arborist/auth.go index 87075e04..ebaec7d7 100644 --- a/arborist/auth.go +++ b/arborist/auth.go @@ -22,7 +22,7 @@ type AuthRequestJSON_User struct { Audiences []string `json:"aud,omitempty"` } -func (requestJSON *AuthRequestJSON_User) Unmarshal(data []byte) error { +func (requestJSON *AuthRequestJSON_User) UnmarshalJSON(data []byte) error { fields := make(map[string]interface{}) err := json.Unmarshal(data, &fields) if err != nil { @@ -59,7 +59,7 @@ type AuthRequestJSON_Request struct { // UnmarshalJSON defines the deserialization from JSON into an AuthRequestJSON // struct, which includes validating that required fields are present. // (Required fields are anything not in the `optionalFields` variable.) -func (requestJSON *AuthRequestJSON_Request) Unmarshal(data []byte) error { +func (requestJSON *AuthRequestJSON_Request) UnmarshalJSON(data []byte) error { fields := make(map[string]interface{}) err := json.Unmarshal(data, &fields) if err != nil { @@ -105,6 +105,9 @@ func authorize(request *AuthRequest) (*AuthResponse, error) { // parse the resource string exp, args, err := Parse(request.Resource) if err != nil { + // TODO (rudyardrichter, 2019-04-05): this can return some pretty + // unintelligible errors from the yacc code. so far callers are OK to + // validate inputs, but could do better to return more readable errors return nil, err } diff --git a/arborist/errors.go b/arborist/errors.go index a469714d..06c4bbb8 100644 --- a/arborist/errors.go +++ b/arborist/errors.go @@ -15,32 +15,6 @@ func (e *httpError) Error() string { return e.msg } -func nameError(name string, purpose string, reason string) error { - msg := fmt.Sprintf("invalid name %s for %s: %s", name, purpose, reason) - return &httpError{msg, http.StatusBadRequest} -} - -func notExist(entity string, idType string, id string) error { - msg := fmt.Sprintf("%s with %s `%s` does not exist", entity, idType, id) - return &httpError{msg, http.StatusNotFound} -} - -func alreadyExists(entity string, idType string, id string) error { - msg := fmt.Sprintf("%s with %s %s already exists", entity, idType, id) - return &httpError{msg, http.StatusConflict} -} - -func noDelete(entity string, idType string, identifier string, reason string) error { - msg := fmt.Sprintf( - "can't delete %s with %s %s; %s", - entity, - idType, - identifier, - reason, - ) - return &httpError{msg, http.StatusBadRequest} -} - func missingRequiredField(entity string, field string) error { msg := fmt.Sprintf("input %s is missing required field `%s`", entity, field) return &httpError{msg, http.StatusBadRequest} diff --git a/arborist/group.go b/arborist/group.go index 8ecd4c57..c268bcf9 100644 --- a/arborist/group.go +++ b/arborist/group.go @@ -33,12 +33,12 @@ func groupWithName(db *sqlx.DB, name string) (*GroupFromQuery, error) { SELECT grp.name, array_remove(array_agg(usr.name), NULL) AS users, - array_remove(array_agg(policy.name), NULL) AS policies + array_remove(array_agg(DISTINCT policy.name), NULL) AS policies FROM grp - LEFT JOIN usr_grp ON usr_grp.grp_id = grp.id - LEFT JOIN usr ON usr.id = usr_grp.usr_id LEFT JOIN grp_policy ON grp_policy.grp_id = grp.id LEFT JOIN policy ON policy.id = grp_policy.policy_id + LEFT JOIN usr_grp ON usr_grp.grp_id = grp.id + LEFT JOIN usr ON usr.id = usr_grp.usr_id WHERE grp.name = $1 GROUP BY grp.id LIMIT 1 @@ -116,8 +116,8 @@ func (group *Group) deleteInDb(db *sqlx.DB) *ErrorResponse { _, err := db.Exec(stmt, group.Name) if err != nil { // TODO: verify correct error - msg := fmt.Sprintf("failed to delete group: group does not exist: %s", group.Name) - return newErrorResponse(msg, 404, nil) + // group does not exist; that's fine + return nil } return nil } diff --git a/arborist/permission.go b/arborist/permission.go index aeb5b3d9..5a768c2a 100644 --- a/arborist/permission.go +++ b/arborist/permission.go @@ -2,8 +2,6 @@ package arborist import ( "encoding/json" - - "github.com/jmoiron/sqlx" ) type Permission struct { @@ -53,33 +51,3 @@ func (permission *Permission) UnmarshalJSON(data []byte) error { return nil } - -func permissionWithNameExists(db *sqlx.DB, name string) (bool, error) { - stmt := ` - SELECT COUNT(*) - FROM permission - WHERE name = $1 - ` - var count int - err := db.Get(&count, stmt, name) - if err != nil { - return false, err - } - exists := count == 1 - return exists, nil -} - -func permissionsWithNamesExist(db *sqlx.DB, names []string) (bool, error) { - stmt := ` - SELECT COUNT(*) - FROM permission - WHERE name IN ($1) - ` - var count int - err := db.Get(&count, stmt, names) - if err != nil { - return false, err - } - exists := count == len(names) - return exists, nil -} diff --git a/arborist/policy.go b/arborist/policy.go index 8c0ddbea..8149beaa 100644 --- a/arborist/policy.go +++ b/arborist/policy.go @@ -67,12 +67,12 @@ type PolicyFromQuery struct { RoleIDs pq.StringArray `db:"role_ids" json:"role_ids"` } -func (policyFromQuery *PolicyFromQuery) standardize() *Policy { +func (policyFromQuery *PolicyFromQuery) standardize() Policy { paths := make([]string, len(policyFromQuery.ResourcePaths)) for i, queryPath := range policyFromQuery.ResourcePaths { paths[i] = formatDbPath(queryPath) } - policy := &Policy{ + policy := Policy{ Name: policyFromQuery.Name, ResourcePaths: paths, RoleIDs: policyFromQuery.RoleIDs, diff --git a/arborist/resource.go b/arborist/resource.go index a5b00531..9722834c 100644 --- a/arborist/resource.go +++ b/arborist/resource.go @@ -79,12 +79,12 @@ type ResourceFromQuery struct { // standardize takes a resource returned from a query and turns it into the // standard form. -func (resourceFromQuery *ResourceFromQuery) standardize() *Resource { +func (resourceFromQuery *ResourceFromQuery) standardize() Resource { subresources := []string{} for _, subresource := range resourceFromQuery.Subresources { subresources = append(subresources, formatDbPath(subresource)) } - resource := &Resource{ + resource := Resource{ Name: resourceFromQuery.Name, Path: formatDbPath(resourceFromQuery.Path), Subresources: subresources, diff --git a/arborist/server.go b/arborist/server.go index caad757a..a12e1dd9 100644 --- a/arborist/server.go +++ b/arborist/server.go @@ -209,7 +209,7 @@ func (server *Server) handleAuthProxy(w http.ResponseWriter, r *http.Request) { if authHeader == "" { msg := "auth proxy request missing auth header" server.logger.Info(msg) - errResponse := newErrorResponse(msg, 400, nil) + errResponse := newErrorResponse(msg, 401, nil) _ = errResponse.write(w, r) return } @@ -275,6 +275,18 @@ func (server *Server) handleAuthRequest(w http.ResponseWriter, r *http.Request, return } + // check that the request has minimum necessary information + if authRequest.Request.Resource == "" { + msg := "missing resource in auth request" + _ = newErrorResponse(msg, 400, nil).write(w, r) + return + } + if info.username == "" && (info.policies == nil || len(info.policies) == 0) { + msg := "missing both username and policies in request (at least one is required)" + _ = newErrorResponse(msg, 400, nil).write(w, r) + return + } + request := &AuthRequest{ info.username, info.policies, @@ -346,7 +358,7 @@ func (server *Server) handleListAuthResources(w http.ResponseWriter, r *http.Req return } - resources := []*Resource{} + resources := []Resource{} for _, resourceFromQuery := range resourcesFromQuery { resources = append(resources, resourceFromQuery.standardize()) } @@ -366,7 +378,11 @@ func (server *Server) handleListAuthResources(w http.ResponseWriter, r *http.Req } func (server *Server) handlePolicyList(w http.ResponseWriter, r *http.Request) { - policies, err := listPoliciesFromDb(server.db) + policiesFromQuery, err := listPoliciesFromDb(server.db) + policies := []Policy{} + for _, policyFromQuery := range policiesFromQuery { + policies = append(policies, policyFromQuery.standardize()) + } if err != nil { msg := fmt.Sprintf("policies query failed: %s", err.Error()) errResponse := newErrorResponse(msg, 500, nil) @@ -374,7 +390,12 @@ func (server *Server) handlePolicyList(w http.ResponseWriter, r *http.Request) { _ = errResponse.write(w, r) return } - _ = jsonResponseFrom(policies, http.StatusOK).write(w, r) + result := struct { + Policies []Policy `json:"policies"` + }{ + Policies: policies, + } + _ = jsonResponseFrom(result, http.StatusOK).write(w, r) } func (server *Server) handlePolicyCreate(w http.ResponseWriter, r *http.Request, body []byte) { @@ -435,12 +456,12 @@ func (server *Server) handlePolicyDelete(w http.ResponseWriter, r *http.Request) _ = errResponse.write(w, r) return } - _ = jsonResponseFrom(nil, http.StatusCreated).write(w, r) + _ = jsonResponseFrom(nil, http.StatusNoContent).write(w, r) } func (server *Server) handleResourceList(w http.ResponseWriter, r *http.Request) { resourcesFromQuery, err := listResourcesFromDb(server.db) - resources := []*Resource{} + resources := []Resource{} for _, resourceFromQuery := range resourcesFromQuery { resources = append(resources, resourceFromQuery.standardize()) } @@ -451,7 +472,12 @@ func (server *Server) handleResourceList(w http.ResponseWriter, r *http.Request) _ = errResponse.write(w, r) return } - _ = jsonResponseFrom(resources, http.StatusOK).write(w, r) + result := struct { + Resources []Resource `json:"resources"` + }{ + Resources: resources, + } + _ = jsonResponseFrom(result, http.StatusOK).write(w, r) } func (server *Server) handleResourceCreate(w http.ResponseWriter, r *http.Request, body []byte) { @@ -571,7 +597,12 @@ func (server *Server) handleRoleList(w http.ResponseWriter, r *http.Request) { for _, roleFromQuery := range rolesFromQuery { roles = append(roles, roleFromQuery.standardize()) } - _ = jsonResponseFrom(roles, http.StatusOK).write(w, r) + result := struct { + Roles []Role `json:"roles"` + }{ + Roles: roles, + } + _ = jsonResponseFrom(result, http.StatusOK).write(w, r) } func (server *Server) handleRoleCreate(w http.ResponseWriter, r *http.Request, body []byte) { diff --git a/arborist/server_test.go b/arborist/server_test.go index 8ac576bc..b8b21aef 100644 --- a/arborist/server_test.go +++ b/arborist/server_test.go @@ -10,6 +10,7 @@ import ( "log" "net/http" "net/http/httptest" + "net/url" "os" "strings" "testing" @@ -55,6 +56,7 @@ func (jwtApp *mockJWTApp) Decode(token string) (*map[string]interface{}, error) type TestJWT struct { username string policies []string + exp int64 } // Encode takes the information in the TestJWT and creates a string of an @@ -74,6 +76,10 @@ func (testJWT *TestJWT) Encode() string { if err != nil { panic(err) } + exp := testJWT.exp + if exp == 0 { + exp = time.Now().Unix() + 10000 + } var payload []byte if testJWT.policies == nil || len(testJWT.policies) == 0 { payload = []byte(fmt.Sprintf( @@ -87,7 +93,7 @@ func (testJWT *TestJWT) Encode() string { } } }`, - time.Now().Unix()+10000, + exp, testJWT.username, )) } else { @@ -155,6 +161,7 @@ func TestServer(t *testing.T) { serviceName := "zxcv" roleName := "hjkl" permissionName := "qwer" + methodName := permissionName policyName := "asdf" roleBody := []byte(fmt.Sprintf( `{ @@ -166,7 +173,7 @@ func TestServer(t *testing.T) { roleName, permissionName, serviceName, - permissionName, + methodName, )) policyBody := []byte(fmt.Sprintf( `{ @@ -189,7 +196,10 @@ func TestServer(t *testing.T) { // httpError is a utility function which writes some useful output after an error. httpError := func(t *testing.T, w *httptest.ResponseRecorder, msg string) { t.Errorf("%s; got status %d, response: %s", msg, w.Code, w.Body.String()) + fmt.Println("test errored, dumping logs") + fmt.Println("logs start") _, err = logBuffer.WriteTo(os.Stdout) + fmt.Println("logs end") if err != nil { t.Fatal(err) } @@ -205,7 +215,7 @@ func TestServer(t *testing.T) { return req } - createUser := func(t *testing.T, body []byte) { + createUserBytes := func(t *testing.T, body []byte) { w := httptest.NewRecorder() req := newRequest("POST", "/user", bytes.NewBuffer(body)) handler.ServeHTTP(w, req) @@ -223,7 +233,7 @@ func TestServer(t *testing.T) { } } - createResource := func(t *testing.T, body []byte) { + createResourceBytes := func(t *testing.T, body []byte) { w := httptest.NewRecorder() req := newRequest("POST", "/resource", bytes.NewBuffer(body)) handler.ServeHTTP(w, req) @@ -241,7 +251,7 @@ func TestServer(t *testing.T) { } } - createRole := func(t *testing.T, body []byte) { + createRoleBytes := func(t *testing.T, body []byte) { w := httptest.NewRecorder() req := newRequest("POST", "/role", bytes.NewBuffer(body)) handler.ServeHTTP(w, req) @@ -250,7 +260,7 @@ func TestServer(t *testing.T) { } } - createPolicy := func(t *testing.T, body []byte) { + createPolicyBytes := func(t *testing.T, body []byte) { w := httptest.NewRecorder() req := newRequest("POST", "/policy", bytes.NewBuffer(body)) handler.ServeHTTP(w, req) @@ -340,9 +350,71 @@ func TestServer(t *testing.T) { tearDown(t) }) + t.Run("NotFound", func(t *testing.T) { + w := httptest.NewRecorder() + req := newRequest("GET", "/bogus/url", nil) + handler.ServeHTTP(w, req) + if w.Code != http.StatusNotFound { + httpError(t, w, "didn't get 404 for nonexistent URL") + } + result := struct { + Error struct { + Message string `json:"message"` + Code int `json:"code"` + } `json:"error"` + }{} + err = json.Unmarshal(w.Body.Bytes(), &result) + if err != nil { + httpError(t, w, "couldn't read response from 404 handler") + } + assert.Equal(t, 404, result.Error.Code, "unexpected response for 404") + }) + t.Run("Resource", func(t *testing.T) { tearDown := testSetup(t) + t.Run("ListEmpty", func(t *testing.T) { + w := httptest.NewRecorder() + req := newRequest("GET", "/resource", nil) + handler.ServeHTTP(w, req) + if w.Code != http.StatusOK { + httpError(t, w, "can't list resources") + } + result := struct { + Resources []interface{} `json:"resources"` + }{} + err = json.Unmarshal(w.Body.Bytes(), &result) + if err != nil { + httpError(t, w, "couldn't read response from resources list") + } + msg := fmt.Sprintf("got response body: %s", w.Body.String()) + assert.Equal(t, []interface{}{}, result.Resources, msg) + }) + + t.Run("CreateWithError", func(t *testing.T) { + t.Run("MissingPath", func(t *testing.T) { + w := httptest.NewRecorder() + // missing required field + body := []byte(`{"name": "test", "description": "this resource has no path"}`) + req := newRequest("POST", "/resource", bytes.NewBuffer(body)) + handler.ServeHTTP(w, req) + if w.Code != http.StatusBadRequest { + httpError(t, w, "resource creation didn't fail as expected") + } + }) + + t.Run("UnexpectedField", func(t *testing.T) { + w := httptest.NewRecorder() + // missing required field + body := []byte(`{"path": "/a", "barrnt": "unexpected"}`) + req := newRequest("POST", "/resource", bytes.NewBuffer(body)) + handler.ServeHTTP(w, req) + if w.Code != http.StatusBadRequest { + httpError(t, w, "resource creation didn't fail as expected") + } + }) + }) + t.Run("Create", func(t *testing.T) { w := httptest.NewRecorder() body := []byte(`{"path": "/a"}`) @@ -455,6 +527,21 @@ func TestServer(t *testing.T) { if err != nil { httpError(t, w, "couldn't read response from role creation") } + + t.Run("AlreadyExists", func(t *testing.T) { + w := httptest.NewRecorder() + body := []byte(`{ + "id": "foo", + "permissions": [ + {"id": "foo", "action": {"service": "test", "method": "foo"}} + ] + }`) + req := newRequest("POST", "/role", bytes.NewBuffer(body)) + handler.ServeHTTP(w, req) + if w.Code != http.StatusConflict { + httpError(t, w, "expected conflict error from trying to create role again") + } + }) }) t.Run("Read", func(t *testing.T) { @@ -475,6 +562,24 @@ func TestServer(t *testing.T) { assert.Equal(t, "foo", result.Name, msg) }) + t.Run("List", func(t *testing.T) { + w := httptest.NewRecorder() + req := newRequest("GET", "/role", nil) + handler.ServeHTTP(w, req) + if w.Code != http.StatusOK { + httpError(t, w, "can't list roles") + } + result := struct { + Roles []interface{} `json:"roles"` + }{} + err = json.Unmarshal(w.Body.Bytes(), &result) + if err != nil { + httpError(t, w, "couldn't read response from roles list") + } + msg := fmt.Sprintf("got response body: %s", w.Body.String()) + assert.Equal(t, 1, len(result.Roles), msg) + }) + t.Run("Delete", func(t *testing.T) { w := httptest.NewRecorder() req := newRequest("DELETE", "/role/foo", nil) @@ -499,9 +604,13 @@ func TestServer(t *testing.T) { t.Run("Policy", func(t *testing.T) { tearDown := testSetup(t) + roleName := "bazgo-create" + policyName := "bazgo-create-b" + t.Run("Create", func(t *testing.T) { w := httptest.NewRecorder() // set up some resources to work with + // TODO: make more of this setup into "fixtures", not hard-coded body := []byte(`{"path": "/a"}`) req := newRequest("POST", "/resource", bytes.NewBuffer(body)) handler.ServeHTTP(w, req) @@ -524,15 +633,18 @@ func TestServer(t *testing.T) { } // set up roles w = httptest.NewRecorder() - body = []byte(`{ - "id": "bazgo-create", - "permissions": [ - { - "id": "foo", - "action": {"service": "bazgo", "method": "create"} - } - ] - }`) + body = []byte(fmt.Sprintf( + `{ + "id": "%s", + "permissions": [ + { + "id": "foo", + "action": {"service": "bazgo", "method": "create"} + } + ] + }`, + roleName, + )) req = newRequest("POST", "/role", bytes.NewBuffer(body)) handler.ServeHTTP(w, req) if w.Code != http.StatusCreated { @@ -540,11 +652,15 @@ func TestServer(t *testing.T) { } // create the policy w = httptest.NewRecorder() - body = []byte(`{ - "id": "bazgo-create-b", - "resource_paths": ["/a/b"], - "role_ids": ["bazgo-create"] - }`) + body = []byte(fmt.Sprintf( + `{ + "id": "%s", + "resource_paths": ["/a/b"], + "role_ids": ["%s"] + }`, + policyName, + roleName, + )) req = newRequest("POST", "/policy", bytes.NewBuffer(body)) handler.ServeHTTP(w, req) if w.Code != http.StatusCreated { @@ -561,7 +677,8 @@ func TestServer(t *testing.T) { t.Run("Read", func(t *testing.T) { w := httptest.NewRecorder() - req := newRequest("GET", "/policy/bazgo-create-b", nil) + url := fmt.Sprintf("/policy/%s", policyName) + req := newRequest("GET", url, nil) handler.ServeHTTP(w, req) if w.Code != http.StatusOK { httpError(t, w, "policy not found") @@ -576,9 +693,47 @@ func TestServer(t *testing.T) { httpError(t, w, "couldn't read response from GET policy") } msg := fmt.Sprintf("got response body: %s", w.Body.String()) - assert.Equal(t, "bazgo-create-b", result.Name, msg) + assert.Equal(t, policyName, result.Name, msg) assert.Equal(t, []string{"/a/b"}, result.Resources, msg) - assert.Equal(t, []string{"bazgo-create"}, result.Roles, msg) + assert.Equal(t, []string{roleName}, result.Roles, msg) + }) + + t.Run("List", func(t *testing.T) { + w := httptest.NewRecorder() + req := newRequest("GET", "/policy", nil) + handler.ServeHTTP(w, req) + if w.Code != http.StatusOK { + httpError(t, w, "can't list policies") + } + result := struct { + Policies []interface{} `json:"policies"` + }{} + err = json.Unmarshal(w.Body.Bytes(), &result) + fmt.Println(string(w.Body.Bytes())) + if err != nil { + httpError(t, w, "couldn't read response from policies list") + } + msg := fmt.Sprintf("got response body: %s", w.Body.String()) + assert.Equal(t, 1, len(result.Policies), msg) + // TODO (rudyardrichter, 2019-04-15): more checks here on response + }) + + t.Run("Delete", func(t *testing.T) { + w := httptest.NewRecorder() + req := newRequest("DELETE", "/policy/foo", nil) + handler.ServeHTTP(w, req) + if w.Code != http.StatusNoContent { + httpError(t, w, "couldn't delete policy") + } + }) + + t.Run("CheckDeleted", func(t *testing.T) { + w := httptest.NewRecorder() + req := newRequest("GET", "/policy/foo", nil) + handler.ServeHTTP(w, req) + if w.Code != http.StatusNotFound { + httpError(t, w, "policy was not actually deleted") + } }) tearDown(t) @@ -667,9 +822,9 @@ func TestServer(t *testing.T) { }) // do some preliminary setup so we have a policy to work with - createResource(t, resourceBody) - createRole(t, roleBody) - createPolicy(t, policyBody) + createResourceBytes(t, resourceBody) + createRoleBytes(t, roleBody) + createPolicyBytes(t, policyBody) t.Run("GrantPolicy", func(t *testing.T) { w := httptest.NewRecorder() @@ -778,6 +933,16 @@ func TestServer(t *testing.T) { } }) + t.Run("DeleteNotExist", func(t *testing.T) { + w := httptest.NewRecorder() + url := fmt.Sprintf("/user/%s", username) + req := newRequest("DELETE", url, nil) + handler.ServeHTTP(w, req) + if w.Code != http.StatusNoContent { + httpError(t, w, "wrong response from deleting user that doesn't exist") + } + }) + t.Run("CheckDeleted", func(t *testing.T) { w := httptest.NewRecorder() req := newRequest("GET", "/user/foo", nil) @@ -847,6 +1012,34 @@ func TestServer(t *testing.T) { } }) + t.Run("CreateAlreadyExists", func(t *testing.T) { + w := httptest.NewRecorder() + body := []byte(fmt.Sprintf(`{"name": "%s"}`, testGroupName)) + req := newRequest("POST", "/group", bytes.NewBuffer(body)) + handler.ServeHTTP(w, req) + if w.Code != http.StatusConflict { + httpError(t, w, "creating group that already exists didn't error as expected") + } + }) + + t.Run("List", func(t *testing.T) { + w := httptest.NewRecorder() + req := newRequest("GET", "/group", nil) + handler.ServeHTTP(w, req) + if w.Code != http.StatusOK { + httpError(t, w, "can't list groups") + } + result := struct { + Groups []interface{} `json:"groups"` + }{} + err = json.Unmarshal(w.Body.Bytes(), &result) + if err != nil { + httpError(t, w, "couldn't read response from groups list") + } + msg := fmt.Sprintf("got response body: %s", w.Body.String()) + assert.Equal(t, 1, len(result.Groups), msg) + }) + t.Run("Read", func(t *testing.T) { w := httptest.NewRecorder() req := newRequest("GET", fmt.Sprintf("/group/%s", testGroupName), nil) @@ -871,7 +1064,7 @@ func TestServer(t *testing.T) { t.Run("AddUsers", func(t *testing.T) { for _, testUsername := range testGroupUsers { - createUser(t, []byte(fmt.Sprintf(`{"name": "%s"}`, testUsername))) + createUserBytes(t, []byte(fmt.Sprintf(`{"name": "%s"}`, testUsername))) w := httptest.NewRecorder() groupUserURL := fmt.Sprintf("/group/%s/user", testGroupName) body := []byte(fmt.Sprintf(`{"username": "%s"}`, testUsername)) @@ -934,10 +1127,12 @@ func TestServer(t *testing.T) { assert.NotContains(t, result.Users, userToRemove, msg) }) - t.Run("GrantPolicy", func(t *testing.T) { - // TODO: add group policy endpoint, enable test - t.SkipNow() + // do some preliminary setup so we have a policy to work with + createResourceBytes(t, resourceBody) + createRoleBytes(t, roleBody) + createPolicyBytes(t, policyBody) + t.Run("GrantPolicy", func(t *testing.T) { w := httptest.NewRecorder() url := fmt.Sprintf("/group/%s/policy", testGroupName) req := newRequest( @@ -949,7 +1144,7 @@ func TestServer(t *testing.T) { if w.Code != http.StatusNoContent { httpError(t, w, "couldn't grant policy to group") } - // look up user again and check that policy is there + // look up group again and check that policy is there w = httptest.NewRecorder() url = fmt.Sprintf("/group/%s", testGroupName) req = newRequest("GET", url, nil) @@ -974,7 +1169,7 @@ func TestServer(t *testing.T) { t.Run("PolicyNotExist", func(t *testing.T) { w := httptest.NewRecorder() - url := fmt.Sprintf("/group/%s/policy", username) + url := fmt.Sprintf("/group/%s/policy", testGroupName) req := newRequest( "POST", url, @@ -1001,6 +1196,34 @@ func TestServer(t *testing.T) { }) }) + t.Run("RevokePolicy", func(t *testing.T) { + w := httptest.NewRecorder() + url := fmt.Sprintf("/group/%s/policy/%s", testGroupName, policyName) + req := newRequest("DELETE", url, nil) + handler.ServeHTTP(w, req) + if w.Code != http.StatusNoContent { + httpError(t, w, "couldn't revoke policy from group") + } + w = httptest.NewRecorder() + url = fmt.Sprintf("/group/%s", testGroupName) + req = newRequest("GET", url, nil) + handler.ServeHTTP(w, req) + if w.Code != http.StatusOK { + httpError(t, w, "couldn't read group") + } + result := struct { + Name string `json:"name"` + Users []string `json:"users"` + Policies []string `json:"policies"` + }{} + err = json.Unmarshal(w.Body.Bytes(), &result) + if err != nil { + httpError(t, w, "couldn't read response from group read") + } + msg := fmt.Sprintf("got response body: %s", w.Body.String()) + assert.NotContains(t, policyName, result.Policies, msg) + }) + t.Run("Delete", func(t *testing.T) { w := httptest.NewRecorder() url := fmt.Sprintf("/group/%s", testGroupName) @@ -1021,12 +1244,141 @@ func TestServer(t *testing.T) { } }) + t.Run("DeleteNotExist", func(t *testing.T) { + w := httptest.NewRecorder() + url := fmt.Sprintf("/group/%s", testGroupName) + req := newRequest("DELETE", url, nil) + handler.ServeHTTP(w, req) + if w.Code != http.StatusNoContent { + httpError(t, w, "wrong response from deleting group that doesn't exist") + } + }) + tearDown(t) }) t.Run("Auth", func(t *testing.T) { tearDown := testSetup(t) + t.Run("Request", func(t *testing.T) { + createResourceBytes(t, []byte(`{"path": "/example"}`)) + createResourceBytes(t, []byte(`{"path": "/example/a"}`)) + createResourceBytes(t, []byte(`{"path": "/example/b"}`)) + createRoleBytes( + t, + []byte(`{ + "id": "foo", + "permissions": [ + {"id": "test", "action": {"service": "test", "method": "foo"}} + ] + }`), + ) + createRoleBytes( + t, + []byte(`{ + "id": "bar", + "permissions": [ + {"id": "test", "action": {"service": "test", "method": "bar"}} + ] + }`), + ) + createPolicyBytes( + t, + []byte(`{ + "id": "example-policy-foo", + "resource_paths": ["/example/a"], + "role_ids": ["foo"] + }`), + ) + createPolicyBytes( + t, + []byte(`{ + "id": "example-policy-bar", + "resource_paths": ["/example/b"], + "role_ids": ["bar"] + }`), + ) + createUserBytes(t, userBody) + grantUserPolicy(t, username, "example-policy-foo") + + w := httptest.NewRecorder() + token := TestJWT{username: username} + body := []byte(fmt.Sprintf( + `{ + "user": {"token": "%s"}, + "request": { + "resource": "/example/a", + "action": { + "service": "test", + "method": "foo" + } + } + }`, + token.Encode(), + )) + req := newRequest("POST", "/auth/request", bytes.NewBuffer(body)) + handler.ServeHTTP(w, req) + if w.Code != http.StatusOK { + httpError(t, w, "auth request failed") + } + // request should succeed, user has authorization + result := struct { + Auth bool `json:"auth"` + }{} + err = json.Unmarshal(w.Body.Bytes(), &result) + if err != nil { + httpError(t, w, "couldn't read response from auth request") + } + msg := fmt.Sprintf("got response body: %s", w.Body.String()) + assert.Equal(t, true, result.Auth, msg) + + t.Run("Unauthorized", func(t *testing.T) { + w = httptest.NewRecorder() + token = TestJWT{username: username} + body = []byte(fmt.Sprintf( + `{ + "user": {"token": "%s"}, + "request": { + "resource": "/example/b", + "action": { + "service": "test", + "method": "foo" + } + } + }`, + token.Encode(), + )) + req = newRequest("POST", "/auth/request", bytes.NewBuffer(body)) + handler.ServeHTTP(w, req) + if w.Code != http.StatusOK { + httpError(t, w, "auth request failed") + } + // request should fail + result = struct { + Auth bool `json:"auth"` + }{} + err = json.Unmarshal(w.Body.Bytes(), &result) + if err != nil { + httpError(t, w, "couldn't read response from auth request") + } + msg = fmt.Sprintf("got response body: %s", w.Body.String()) + assert.Equal(t, false, result.Auth, msg) + }) + + t.Run("BadRequest", func(t *testing.T) { + w = httptest.NewRecorder() + token = TestJWT{username: username} + body = []byte("not real JSON") + req = newRequest("POST", "/auth/request", bytes.NewBuffer(body)) + handler.ServeHTTP(w, req) + if w.Code != http.StatusBadRequest { + httpError(t, w, "expected error") + } + }) + }) + + deleteEverything() + t.Run("Resources", func(t *testing.T) { t.Run("Empty", func(t *testing.T) { w := httptest.NewRecorder() @@ -1050,10 +1402,10 @@ func TestServer(t *testing.T) { }) t.Run("Granted", func(t *testing.T) { - createResource(t, resourceBody) - createRole(t, roleBody) - createPolicy(t, policyBody) - createUser(t, userBody) + createResourceBytes(t, resourceBody) + createRoleBytes(t, roleBody) + createPolicyBytes(t, policyBody) + createUserBytes(t, userBody) grantUserPolicy(t, username, policyName) w := httptest.NewRecorder() @@ -1074,9 +1426,226 @@ func TestServer(t *testing.T) { } msg := fmt.Sprintf("got response body: %s", w.Body.String()) assert.Equal(t, []string{resourcePath}, result.Resources, msg) + + t.Run("Policies", func(t *testing.T) { + w := httptest.NewRecorder() + body := []byte(fmt.Sprintf( + `{"user": {"token": "%s", "policies": ["%s"]}}`, + token.Encode(), + policyName, + )) + req := newRequest("POST", "/auth/resources", bytes.NewBuffer(body)) + handler.ServeHTTP(w, req) + if w.Code != http.StatusOK { + httpError(t, w, "auth resources request failed") + } + // in this case, since the user has zero access yet, should be empty + result := struct { + Resources []string `json:"resources"` + }{} + err = json.Unmarshal(w.Body.Bytes(), &result) + if err != nil { + httpError(t, w, "couldn't read response from auth resources") + } + msg := fmt.Sprintf("got response body: %s", w.Body.String()) + assert.Equal(t, []string{resourcePath}, result.Resources, msg) + }) + }) + }) + + deleteEverything() + + t.Run("Proxy", func(t *testing.T) { + createResourceBytes(t, resourceBody) + createRoleBytes(t, roleBody) + createPolicyBytes(t, policyBody) + createUserBytes(t, userBody) + grantUserPolicy(t, username, policyName) + token := TestJWT{username: username} + + t.Run("Authorized", func(t *testing.T) { + w := httptest.NewRecorder() + authUrl := fmt.Sprintf( + "/auth/proxy?resource=%s&service=%s&method=%s", + url.QueryEscape(resourcePath), + url.QueryEscape(serviceName), + url.QueryEscape(methodName), + ) + req := newRequest("GET", authUrl, nil) + req.Header.Add("Authorization", fmt.Sprintf("Bearer %s", token.Encode())) + handler.ServeHTTP(w, req) + if w.Code != http.StatusOK { + httpError(t, w, "auth proxy request failed") + } + }) + + t.Run("BadRequest", func(t *testing.T) { + w := httptest.NewRecorder() + authUrl := fmt.Sprintf( + "/auth/proxy?resource=%s&service=%s&method=%s", + url.QueryEscape("not-even-a-resource-path"), + url.QueryEscape(serviceName), + url.QueryEscape(methodName), + ) + req := newRequest("GET", authUrl, nil) + req.Header.Add("Authorization", fmt.Sprintf("Bearer %s", token.Encode())) + handler.ServeHTTP(w, req) + if w.Code != http.StatusBadRequest { + httpError(t, w, "auth proxy request succeeded when it should not have") + } + }) + + t.Run("Unauthorized", func(t *testing.T) { + t.Run("BadHeader", func(t *testing.T) { + w := httptest.NewRecorder() + authUrl := fmt.Sprintf( + "/auth/proxy?resource=%s&service=%s&method=%s", + url.QueryEscape(resourcePath), + url.QueryEscape(serviceName), + url.QueryEscape(methodName), + ) + req := newRequest("GET", authUrl, nil) + req.Header.Add("Authorization", "Bearer garbage") + handler.ServeHTTP(w, req) + if w.Code != http.StatusUnauthorized { + httpError(t, w, "auth proxy request succeeded when it should not have") + } + }) + + t.Run("TokenExpired", func(t *testing.T) { + token := TestJWT{username: username, exp: 1} + w := httptest.NewRecorder() + authUrl := fmt.Sprintf( + "/auth/proxy?resource=%s&service=%s&method=%s", + url.QueryEscape(resourcePath), + url.QueryEscape(serviceName), + url.QueryEscape(methodName), + ) + req := newRequest("GET", authUrl, nil) + req.Header.Add("Authorization", fmt.Sprintf("Bearer %s", token.Encode())) + handler.ServeHTTP(w, req) + if w.Code != http.StatusUnauthorized { + httpError(t, w, "auth proxy request succeeded when it should not have") + } + }) + + t.Run("ResourceNotExist", func(t *testing.T) { + w := httptest.NewRecorder() + authUrl := fmt.Sprintf( + "/auth/proxy?resource=%s&service=%s&method=%s", + url.QueryEscape("/not/authorized"), + url.QueryEscape(serviceName), + url.QueryEscape(methodName), + ) + req := newRequest("GET", authUrl, nil) + req.Header.Add("Authorization", fmt.Sprintf("Bearer %s", token.Encode())) + handler.ServeHTTP(w, req) + if w.Code != http.StatusForbidden { + httpError(t, w, "auth proxy request succeeded when it should not have") + } + }) + + t.Run("WrongMethod", func(t *testing.T) { + w := httptest.NewRecorder() + authUrl := fmt.Sprintf( + "/auth/proxy?resource=%s&service=%s&method=%s", + url.QueryEscape(resourcePath), + url.QueryEscape(serviceName), + url.QueryEscape("bogus_method"), + ) + req := newRequest("GET", authUrl, nil) + req.Header.Add("Authorization", fmt.Sprintf("Bearer %s", token.Encode())) + handler.ServeHTTP(w, req) + if w.Code != http.StatusForbidden { + httpError(t, w, "auth proxy request succeeded when it should not have") + } + }) + + t.Run("WrongService", func(t *testing.T) { + w := httptest.NewRecorder() + authUrl := fmt.Sprintf( + "/auth/proxy?resource=%s&service=%s&method=%s", + url.QueryEscape(resourcePath), + url.QueryEscape("bogus_service"), + url.QueryEscape(methodName), + ) + req := newRequest("GET", authUrl, nil) + req.Header.Add("Authorization", fmt.Sprintf("Bearer %s", token.Encode())) + handler.ServeHTTP(w, req) + if w.Code != http.StatusForbidden { + httpError(t, w, "auth proxy request succeeded when it should not have") + } + }) + }) + + t.Run("MissingAuthHeader", func(t *testing.T) { + w := httptest.NewRecorder() + // request is good + authUrl := fmt.Sprintf( + "/auth/proxy?resource=%s&service=%s&method=%s", + url.QueryEscape(resourcePath), + url.QueryEscape(serviceName), + url.QueryEscape(methodName), + ) + req := newRequest("GET", authUrl, nil) + // but no header added to the request! + handler.ServeHTTP(w, req) + if w.Code != http.StatusUnauthorized { + httpError(t, w, "auth proxy request without auth header didn't fail as expected") + } + }) + + t.Run("MissingMethod", func(t *testing.T) { + w := httptest.NewRecorder() + // omit method + authUrl := fmt.Sprintf( + "/auth/proxy?resource=%s&service=%s", + url.QueryEscape(resourcePath), + url.QueryEscape(serviceName), + ) + req := newRequest("GET", authUrl, nil) + req.Header.Add("Authorization", fmt.Sprintf("Bearer %s", token.Encode())) + handler.ServeHTTP(w, req) + if w.Code != http.StatusBadRequest { + httpError(t, w, "auth proxy request did not error as expected") + } + }) + + t.Run("MissingService", func(t *testing.T) { + w := httptest.NewRecorder() + // omit service + authUrl := fmt.Sprintf( + "/auth/proxy?resource=%s&method=%s", + url.QueryEscape(resourcePath), + url.QueryEscape(methodName), + ) + req := newRequest("GET", authUrl, nil) + req.Header.Add("Authorization", fmt.Sprintf("Bearer %s", token.Encode())) + handler.ServeHTTP(w, req) + if w.Code != http.StatusBadRequest { + httpError(t, w, "auth proxy request did not error as expected") + } + }) + + t.Run("MissingResource", func(t *testing.T) { + w := httptest.NewRecorder() + // omit resource + authUrl := fmt.Sprintf( + "/auth/proxy?&method=%sservice=%s", + url.QueryEscape(methodName), + url.QueryEscape(serviceName), + ) + req := newRequest("GET", authUrl, nil) + req.Header.Add("Authorization", fmt.Sprintf("Bearer %s", token.Encode())) + handler.ServeHTTP(w, req) + if w.Code != http.StatusBadRequest { + httpError(t, w, "auth proxy request did not error as expected") + } }) }) tearDown(t) }) + + deleteEverything() } diff --git a/arborist/user.go b/arborist/user.go index 4a18e955..a6be9c72 100644 --- a/arborist/user.go +++ b/arborist/user.go @@ -127,8 +127,8 @@ func (user *User) deleteInDb(db *sqlx.DB) *ErrorResponse { _, err := db.Exec(stmt, user.Name) if err != nil { // TODO: verify correct error - msg := fmt.Sprintf("failed to delete user: user does not exist: %s", user.Name) - return newErrorResponse(msg, 404, nil) + // user does not exist; that's fine + return nil } return nil }