From 4b52a4ddaa46153a1026a971fd217b6e56455420 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Thu, 15 Aug 2019 15:05:08 -0500 Subject: [PATCH 1/7] fix(path-encoding): support punctuation --- .travis.yml | 2 +- Dockerfile | 2 +- DockerfileAlpine | 2 +- Makefile | 4 +- arborist/auth.go | 9 +++-- arborist/policy.go | 2 +- arborist/resource.go | 81 +++++++++++++++++++++++++++---------- arborist/server.go | 5 ++- arborist/server_test.go | 90 ++++++++++++----------------------------- 9 files changed, 100 insertions(+), 97 deletions(-) diff --git a/.travis.yml b/.travis.yml index 31d36b93..89dc9586 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,7 @@ language: go go: - - "1.10" + - "1.12" # Restrict to cloning only 1 commit. git: diff --git a/Dockerfile b/Dockerfile index f6179471..e11d820f 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM golang:1.10-alpine as build +FROM golang:1.12-alpine as build # Install SSL certificates RUN apk update && apk add --no-cache git ca-certificates gcc musl-dev jq curl bash postgresql diff --git a/DockerfileAlpine b/DockerfileAlpine index 5efe9310..df7e8af5 100644 --- a/DockerfileAlpine +++ b/DockerfileAlpine @@ -1,4 +1,4 @@ -FROM golang:1.10-alpine as build +FROM golang:1.12-alpine as build # Install SSL certificates RUN apk update && apk add --no-cache git ca-certificates gcc musl-dev bash jq diff --git a/Makefile b/Makefile index f057da06..b68fb21f 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,5 @@ _default: bin/arborist + @: # if we have a command this silences "nothing to be done" bin/arborist: arborist/*.go # help: run the server go build -o bin/arborist @@ -13,7 +14,8 @@ coverage: test # help: generate test coverage file go test --coverprofile=coverage.out ./arborist/ db-test: $(which psql) # help: set up the database for testing (run automatically by `test`) - createdb || true + @echo 'createdb || true' + @createdb 2>&1 | grep -v 'already exist' || true ./migrations/latest up: upgrade # help: try to migrate the database to the next more recent version diff --git a/arborist/auth.go b/arborist/auth.go index 48a4ad4a..f8d8287c 100644 --- a/arborist/auth.go +++ b/arborist/auth.go @@ -111,7 +111,7 @@ func authorizeAnonymous(request *AuthRequest) (*AuthResponse, error) { resource := request.Resource // See if the resource field is a path or a tag. if strings.HasPrefix(resource, "/") { - resource = formatPathForDb(resource) + resource = FormatPathForDb(resource) } else { tag = resource resource = "" @@ -203,7 +203,7 @@ func authorizeUser(request *AuthRequest) (*AuthResponse, error) { resource := request.Resource // See if the resource field is a path or a tag. if strings.HasPrefix(resource, "/") { - resource = formatPathForDb(resource) + resource = FormatPathForDb(resource) } else { tag = resource resource = "" @@ -299,7 +299,7 @@ func authorizeUser(request *AuthRequest) (*AuthResponse, error) { return &AuthResponse{result}, nil } -// This is similar as authorizeUser, only that this method checks for clientID only +// This is similar to authorizeUser, only that this method checks for clientID only func authorizeClient(request *AuthRequest) (*AuthResponse, error) { var err error var tag string @@ -308,7 +308,7 @@ func authorizeClient(request *AuthRequest) (*AuthResponse, error) { resource := request.Resource // See if the resource field is a path or a tag. if strings.HasPrefix(resource, "/") { - resource = formatPathForDb(resource) + resource = FormatPathForDb(resource) } else { tag = resource resource = "" @@ -424,6 +424,7 @@ func authRequestFromGET(decode func(string, []string) (*TokenInfo, error), r *ht return &authRequest, nil } +// See the FIXME inside. Be careful how this is called, until the implementation is updated. func authorizedResources(db *sqlx.DB, request *AuthRequest) ([]ResourceFromQuery, *ErrorResponse) { // if policies are specified in the request, we can use those (simplest query). if request.Policies != nil && len(request.Policies) > 0 { diff --git a/arborist/policy.go b/arborist/policy.go index 56d5b087..037962c7 100644 --- a/arborist/policy.go +++ b/arborist/policy.go @@ -141,7 +141,7 @@ func (policy *Policy) resources(tx *sqlx.Tx) ([]ResourceFromQuery, error) { resources := []ResourceFromQuery{} queryPaths := make([]string, len(policy.ResourcePaths)) for i, path := range policy.ResourcePaths { - queryPaths[i] = formatPathForDb(path) + queryPaths[i] = FormatPathForDb(path) } resourcesStmt := selectInStmt("resource", "ltree2text(path)", queryPaths) err := tx.Select(&resources, resourcesStmt) diff --git a/arborist/resource.go b/arborist/resource.go index daf35101..a9da6ee1 100644 --- a/arborist/resource.go +++ b/arborist/resource.go @@ -3,6 +3,7 @@ package arborist import ( "encoding/json" "fmt" + "net/url" "regexp" "strings" @@ -30,6 +31,43 @@ type ResourceOut struct { Subresources []string `json:"subresources"` } +var regPercent *regexp.Regexp = regexp.MustCompile(`%`) +var regSlashEncoded *regexp.Regexp = regexp.MustCompile(`%2F`) + +func UnderscoreEncode(decoded string) string { + // Per https://www.ietf.org/rfc/rfc3986.txt, there are the following + // unreserved characters (aside from alphanumeric and underscore) which do + // not urlencode: + // - `-` + // - `.` + // - `~` + // Percent encoding uses hexadecimal, which we mustn't conflict with. So we + // use some made-up codes for these which will not overlap. + encoded := decoded + encoded = strings.ReplaceAll(encoded, "_", "_S0") + encoded = strings.ReplaceAll(encoded, "-", "_S1") + encoded = strings.ReplaceAll(encoded, ".", "_S2") + encoded = strings.ReplaceAll(encoded, "~", "_S3") + encoded = url.QueryEscape(encoded) + // turn the slashes *back*, we only want to "underscore-encode" other stuff + encoded = strings.ReplaceAll(encoded, "%2F", "/") + // finally we turn the % symbols into __ which ltree is OK with + encoded = strings.ReplaceAll(encoded, "%", "__") + return encoded +} + +func UnderscoreDecode(encoded string) string { + // undo the steps from UnderscoreEncode + decoded := encoded + decoded = strings.ReplaceAll(decoded, "_S1", "-") + decoded = strings.ReplaceAll(decoded, "_S2", ".") + decoded = strings.ReplaceAll(decoded, "_S3", "~") + decoded = strings.ReplaceAll(decoded, "__", "%") + decoded = strings.ReplaceAll(decoded, "_S0", "_") + decoded, _ = url.QueryUnescape(decoded) + return decoded +} + // NOTE: the resource unmarshalling, because the resources can be specified // with either the name + endpoint path, or the full path in the JSON input, is // not able to validate all cases precisely. The unmarshalling will pass as @@ -98,8 +136,8 @@ func (resourceFromQuery *ResourceFromQuery) standardize() ResourceOut { subresources = append(subresources, formatDbPath(subresource)) } resource := ResourceOut{ - Name: resourceFromQuery.Name, - Path: formatDbPath(resourceFromQuery.Path), + Name: UnderscoreDecode(resourceFromQuery.Name), + Path: UnderscoreDecode(formatDbPath(resourceFromQuery.Path)), Tag: resourceFromQuery.Tag, Subresources: subresources, } @@ -109,29 +147,30 @@ func (resourceFromQuery *ResourceFromQuery) standardize() ResourceOut { return resource } -// formatPathForDb takes a path from a resource in the database and transforms +// FormatPathForDb takes a path from a resource in the database and transforms // it to the front-end version of the resource path. Inverse of `formatDbPath`. // // formatDbPath("/a/b/c") == "a.b.c" -func formatPathForDb(path string) string { +func FormatPathForDb(path string) string { // -1 means replace everything - return strings.TrimLeft(strings.Replace(path, "/", ".", -1), ".") + result := strings.TrimLeft(strings.Replace(UnderscoreEncode(path), "/", ".", -1), ".") + return result } // formatDbPath takes a path from a resource in the database and transforms it -// to the front-end version of the resource path. Inverse of `formatPathForDb`. +// to the front-end version of the resource path. Inverse of `FormatPathForDb`. // // formatDbPath("a.b.c") == "/a/b/c" func formatDbPath(path string) string { // -1 means replace everything - return "/" + strings.Replace(path, ".", "/", -1) + return UnderscoreDecode("/" + strings.Replace(path, ".", "/", -1)) } // resourceWithPath looks up a resource matching the given path. The database -// schema guarantees such a resource to be unique. Any error returned is -// because of internal database failure. +// schema guarantees such a resource to be unique. Any error returned is because +// of internal database failure. func resourceWithPath(db *sqlx.DB, path string) (*ResourceFromQuery, error) { - path = formatPathForDb(path) + path = FormatPathForDb(path) resources := []ResourceFromQuery{} stmt := ` SELECT @@ -165,9 +204,9 @@ func resourceWithPath(db *sqlx.DB, path string) (*ResourceFromQuery, error) { return &resource, nil } -// resourceWithPath looks up a resource matching the given path. The database -// schema guarantees such a resource to be unique. Any error returned is -// because of internal database failure. +// resourceWithTag looks up a resource matching the given tag. The database +// schema guarantees such a resource to be unique. Any error returned is because +// of internal database failure. func resourceWithTag(db *sqlx.DB, tag string) (*ResourceFromQuery, error) { resources := []ResourceFromQuery{} stmt := ` @@ -228,12 +267,12 @@ func listResourcesFromDb(db *sqlx.DB) ([]ResourceFromQuery, error) { // postgres currently only allows alphanumeric characters and underscores. We // also have to pass through slashes in the path since these will be translated // to the correct format for the database later. -var resourcePathValidRegex = regexp.MustCompile(`^[/a-zA-Z0-9_]*$`) -var resourcePathValidChars = regexp.MustCompile(`[/a-zA-Z0-9_]`) +var resourcePathValidRegex = regexp.MustCompile(`^.*$`) +var resourcePathValidChars = regexp.MustCompile(`.`) // Resource names are the same, except they can't contain slashes at all. -var resourceNameValidRegex = regexp.MustCompile(`^[a-zA-Z0-9_]*$`) -var resourceNameValidChars = regexp.MustCompile(`[a-zA-Z0-9_]`) +var resourceNameValidRegex = regexp.MustCompile(`^[^/]*$`) +var resourceNameValidChars = regexp.MustCompile(`[^/]`) func (resource *ResourceIn) validate() *ErrorResponse { validPath := resourcePathValidRegex.MatchString(resource.Path) @@ -269,7 +308,7 @@ func (resource *ResourceIn) createRecursively(tx *sqlx.Tx) *ErrorResponse { return errResponse } // arborist uses `/` for path separator; ltree in postgres uses `.` - path := formatPathForDb(resource.Path) + path := FormatPathForDb(resource.Path) stmt := "INSERT INTO resource(path, description) VALUES ($1, $2)" _, err := tx.Exec(stmt, path, resource.Description) if err != nil { @@ -302,7 +341,7 @@ func (resource *ResourceIn) deleteInDb(tx *sqlx.Tx) *ErrorResponse { return newErrorResponse(msg, 400, nil) } stmt := "DELETE FROM resource WHERE path = $1" - _, err := tx.Exec(stmt, formatPathForDb(resource.Path)) + _, err := tx.Exec(stmt, FormatPathForDb(resource.Path)) if err != nil { // resource already doesn't exist; this is fine return nil @@ -331,7 +370,7 @@ func (resource *ResourceIn) addPath(parent string) *ErrorResponse { func (resource *ResourceIn) overwriteInDb(tx *sqlx.Tx) *ErrorResponse { // arborist uses `/` for path separator; ltree in postgres uses `.` - path := formatPathForDb(resource.Path) + path := FormatPathForDb(resource.Path) stmt := "INSERT INTO resource(path) VALUES ($1) ON CONFLICT DO NOTHING" _, err := tx.Exec(stmt, path) if err != nil { @@ -354,7 +393,7 @@ func (resource *ResourceIn) overwriteInDb(tx *sqlx.Tx) *ErrorResponse { subPathsKeep := []string{} for _, subresource := range resource.Subresources { subresource.addPath(resource.Path) - subpath := fmt.Sprintf("'%s'", formatPathForDb(subresource.Path)) + subpath := fmt.Sprintf("'%s'", FormatPathForDb(subresource.Path)) subPathsKeep = append(subPathsKeep, subpath) } stmtFormat := ` diff --git a/arborist/server.go b/arborist/server.go index c5d110c8..7d9adce6 100644 --- a/arborist/server.go +++ b/arborist/server.go @@ -67,7 +67,7 @@ func (server *Server) Init() (*Server, error) { // `{resourcePath:/[a-zA-Z0-9_\-\/]+}` // // so we put the slash at the front here and fix it in parseResourcePath. -const resourcePath string = `/{resourcePath:[a-zA-Z0-9_\-\/]+}` +const resourcePath string = `/{resourcePath:.+}` func parseResourcePath(r *http.Request) string { path, exists := mux.Vars(r)["resourcePath"] @@ -352,7 +352,7 @@ func (server *Server) handleAuthRequest(w http.ResponseWriter, r *http.Request, Method: authRequest.Action.Method, stmts: server.stmts, } - server.logger.Info("handling auth request: %v", request) + server.logger.Info("handling auth request: %#v", *request) rv, err := authorizeUser(request) if err != nil { msg := fmt.Sprintf("could not authorize user: %s", err.Error()) @@ -662,6 +662,7 @@ func (server *Server) handleResourceCreate(w http.ResponseWriter, r *http.Reques if errResponse.HTTPError.Code == 500 { errResponse.HTTPError.Code = 400 } + // TODO: patch error message to be intelligible if dumping resource path errResponse.log.write(server.logger) _ = errResponse.write(w, r) return diff --git a/arborist/server_test.go b/arborist/server_test.go index c1c0418c..bfc92495 100644 --- a/arborist/server_test.go +++ b/arborist/server_test.go @@ -161,7 +161,12 @@ func TestServer(t *testing.T) { fmt.Printf("using %s for test database\n", dbUrl) } db, err := sqlx.Open("postgres", dbUrl) + // no error so far, make sure ping returns OK + if err == nil { + err = db.Ping() + } if err != nil { + fmt.Println("couldn't reach db; make sure arborist has correct database configuration!") t.Fatal(err) } server, err := arborist. @@ -176,8 +181,7 @@ func TestServer(t *testing.T) { handler := server.MakeRouter(logDest) // some test data to work with - resourcePath := "/example" - resourceDbPath := "example" + resourcePath := "/example(123)-X.Y*" resourceBody := []byte(fmt.Sprintf(`{"path": "%s"}`, resourcePath)) serviceName := "zxcv" roleName := "hjkl" @@ -315,7 +319,7 @@ func TestServer(t *testing.T) { getTagForResource := func(path string) string { var tags []string - db.Select(&tags, "SELECT tag FROM resource WHERE path = $1", path) + db.Select(&tags, "SELECT tag FROM resource WHERE path = $1", arborist.FormatPathForDb(path)) if len(tags) == 0 { return "" } @@ -592,25 +596,12 @@ func TestServer(t *testing.T) { } }) - t.Run("InvalidPath", func(t *testing.T) { + t.Run("BadJSON", func(t *testing.T) { w := httptest.NewRecorder() - // missing required field - body := []byte(`{"path": "/hyphens-not-allowed"}`) - req := newRequest("POST", "/resource", bytes.NewBuffer(body)) + req := newRequest("POST", "/resource", nil) handler.ServeHTTP(w, req) if w.Code != http.StatusBadRequest { - httpError(t, w, "resource creation didn't fail as expected") - } - }) - - t.Run("InvalidName", func(t *testing.T) { - w := httptest.NewRecorder() - // missing required field - body := []byte(`{"name": "hyphens-not-allowed"}`) - 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") + httpError(t, w, "expected 400 from request missing JSON") } }) }) @@ -619,15 +610,6 @@ func TestServer(t *testing.T) { // so we can test looking it up using the tag. var resourceTag string - t.Run("CreateBadJSON", func(t *testing.T) { - w := httptest.NewRecorder() - req := newRequest("POST", "/resource", nil) - handler.ServeHTTP(w, req) - if w.Code != http.StatusBadRequest { - httpError(t, w, "expected 400 from request missing JSON") - } - }) - t.Run("Create", func(t *testing.T) { w := httptest.NewRecorder() path := "/a" @@ -656,6 +638,17 @@ func TestServer(t *testing.T) { assert.NotEqual(t, "", result.Resource.Tag, msg) resourceTag = result.Resource.Tag + t.Run("Punctuation", func(t *testing.T) { + w := httptest.NewRecorder() + // missing required field + body := []byte(`{"path": "/!@#punctuation$%^-_is_-&*(allowed)-==[].<>{},?\\"}`) + req := newRequest("POST", "/resource", bytes.NewBuffer(body)) + handler.ServeHTTP(w, req) + if w.Code != http.StatusCreated { + httpError(t, w, "couldn't create resource with punctuation") + } + }) + t.Run("AlreadyExists", func(t *testing.T) { w := httptest.NewRecorder() body := []byte(fmt.Sprintf(`{"path": "%s"}`, path)) @@ -666,39 +659,6 @@ func TestServer(t *testing.T) { } }) - // Test that errors are returned if a resource is input with - // invalid characters. (Postgres ltree module only allows - // alphanumeric.) - t.Run("InvalidCharacters", func(t *testing.T) { - t.Run("Path", func(t *testing.T) { - w := httptest.NewRecorder() - body := []byte(`{"path": "/a-b"}`) - req := newRequest("POST", "/resource", bytes.NewBuffer(body)) - handler.ServeHTTP(w, req) - if w.Code != http.StatusBadRequest { - httpError(t, w, "expected error from creating resource with invalid characters") - } - }) - - t.Run("Name", func(t *testing.T) { - w = httptest.NewRecorder() - body = []byte(`{"name": "a-^*#b"}`) - req = newRequest("POST", "/resource", bytes.NewBuffer(body)) - handler.ServeHTTP(w, req) - if w.Code != http.StatusBadRequest { - httpError(t, w, "expected error from creating resource with invalid characters") - } - - w = httptest.NewRecorder() - body = []byte(`{"name": "a/b"}`) - req = newRequest("POST", "/resource", bytes.NewBuffer(body)) - handler.ServeHTTP(w, req) - if w.Code != http.StatusBadRequest { - httpError(t, w, "expected error from creating resource with invalid characters") - } - }) - }) - t.Run("MissingParent", func(t *testing.T) { w := httptest.NewRecorder() body := []byte(`{"path": "/parent/doesnt/exist"}`) @@ -971,9 +931,9 @@ func TestServer(t *testing.T) { // now PUT over the same resource, but keep the subresources w = httptest.NewRecorder() body = []byte(`{ - "name": "Godel", + "name": "Godel,", "subresources": [ - {"name": "Escher", "subresources": [{"name": "Bach"}]}, + {"name": "Escher,", "subresources": [{"name": "Bach"}]}, {"name": "completeness_theorem"} ] }`) @@ -986,7 +946,7 @@ func TestServer(t *testing.T) { newBachTag := getTagForResource("Godel.Escher.Bach") assert.Equal(t, escherTag, newEscherTag, "subresource tag changed after PUT") assert.Equal(t, bachTag, newBachTag, "subresource tag changed after PUT") - getResourceWithPath(t, "/Godel/completeness_theorem") + getResourceWithPath(t, "/Godel,/completeness_theorem") }) t.Run("Delete", func(t *testing.T) { @@ -2284,7 +2244,7 @@ func TestServer(t *testing.T) { t.Run("Tag", func(t *testing.T) { w := httptest.NewRecorder() - tag := getTagForResource(resourceDbPath) + tag := getTagForResource(resourcePath) body := []byte(fmt.Sprintf( `{ "user": {"token": "%s"}, From 1edefdfa464886548bf1f8df22ff47cca4a93006 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Thu, 15 Aug 2019 15:37:26 -0500 Subject: [PATCH 2/7] fix(path-encoding): add test file --- arborist/resource_test.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 arborist/resource_test.go diff --git a/arborist/resource_test.go b/arborist/resource_test.go new file mode 100644 index 00000000..e96eebc1 --- /dev/null +++ b/arborist/resource_test.go @@ -0,0 +1,26 @@ +package arborist + +import ( + "regexp" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestEncodeDecode(t *testing.T) { + inputs := []string{ + "test", + "!@#$%^&*()`\\=+-_'\"<>?,.", + "___ooOO00O___", + "foo-=-bar-=-baz", + "🙃", + } + + var regValidDbPath *regexp.Regexp = regexp.MustCompile(`^[a-zA-Z0-9_]+$`) + + for _, input := range inputs { + encoded := UnderscoreEncode(input) + assert.Equal(t, input, UnderscoreDecode(encoded), "encode/decode broken") + assert.True(t, regValidDbPath.MatchString(encoded), "encoded contains invalid characters") + } +} From 558f563d7de8d8233f15f6c991c1ea5ec18a9973 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Thu, 15 Aug 2019 16:01:56 -0500 Subject: [PATCH 3/7] fix(path-encoding): remove unneeded validation --- arborist/resource.go | 37 +------------------------------------ 1 file changed, 1 insertion(+), 36 deletions(-) diff --git a/arborist/resource.go b/arborist/resource.go index a9da6ee1..73c0a618 100644 --- a/arborist/resource.go +++ b/arborist/resource.go @@ -263,39 +263,8 @@ func listResourcesFromDb(db *sqlx.DB) ([]ResourceFromQuery, error) { return resources, nil } -// Regexes used to validate input resource paths. The `ltree` module for -// postgres currently only allows alphanumeric characters and underscores. We -// also have to pass through slashes in the path since these will be translated -// to the correct format for the database later. -var resourcePathValidRegex = regexp.MustCompile(`^.*$`) -var resourcePathValidChars = regexp.MustCompile(`.`) - -// Resource names are the same, except they can't contain slashes at all. -var resourceNameValidRegex = regexp.MustCompile(`^[^/]*$`) -var resourceNameValidChars = regexp.MustCompile(`[^/]`) - -func (resource *ResourceIn) validate() *ErrorResponse { - validPath := resourcePathValidRegex.MatchString(resource.Path) - if !validPath { - invalidChars := resourcePathValidChars.ReplaceAllLiteralString(resource.Path, "") - msg := fmt.Sprintf("input resource path contains invalid characters: %s", invalidChars) - return newErrorResponse(msg, 400, nil) - } - validName := resourceNameValidRegex.MatchString(resource.Name) - if !validName { - invalidChars := resourceNameValidChars.ReplaceAllLiteralString(resource.Name, "") - msg := fmt.Sprintf("input resource name contains invalid characters: %s", invalidChars) - return newErrorResponse(msg, 400, nil) - } - return nil -} - func (resource *ResourceIn) createInDb(tx *sqlx.Tx) *ErrorResponse { - errResponse := resource.validate() - if errResponse != nil { - return errResponse - } - errResponse = resource.createRecursively(tx) + errResponse := resource.createRecursively(tx) if errResponse != nil { return errResponse } @@ -303,10 +272,6 @@ func (resource *ResourceIn) createInDb(tx *sqlx.Tx) *ErrorResponse { } func (resource *ResourceIn) createRecursively(tx *sqlx.Tx) *ErrorResponse { - errResponse := resource.validate() - if errResponse != nil { - return errResponse - } // arborist uses `/` for path separator; ltree in postgres uses `.` path := FormatPathForDb(resource.Path) stmt := "INSERT INTO resource(path, description) VALUES ($1, $2)" From 5c201812956751d42182dab6f05d3d8326b21d35 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Mon, 19 Aug 2019 09:23:33 -0500 Subject: [PATCH 4/7] fix(path-encoding): add another test case --- arborist/resource_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arborist/resource_test.go b/arborist/resource_test.go index e96eebc1..73b98d4c 100644 --- a/arborist/resource_test.go +++ b/arborist/resource_test.go @@ -11,8 +11,10 @@ func TestEncodeDecode(t *testing.T) { inputs := []string{ "test", "!@#$%^&*()`\\=+-_'\"<>?,.", + "_-.~", "___ooOO00O___", "foo-=-bar-=-baz", + "__S0_S1___S2_S3_S4_S5_S6_S7_2F__2F____AB", "🙃", } From 2bad4d4e640b2c2bf4b748415319af48f89cf61e Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Mon, 19 Aug 2019 09:28:32 -0500 Subject: [PATCH 5/7] fix(path-encoding): remove errant comment --- arborist/server_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/arborist/server_test.go b/arborist/server_test.go index bfc92495..09b90398 100644 --- a/arborist/server_test.go +++ b/arborist/server_test.go @@ -640,7 +640,6 @@ func TestServer(t *testing.T) { t.Run("Punctuation", func(t *testing.T) { w := httptest.NewRecorder() - // missing required field body := []byte(`{"path": "/!@#punctuation$%^-_is_-&*(allowed)-==[].<>{},?\\"}`) req := newRequest("POST", "/resource", bytes.NewBuffer(body)) handler.ServeHTTP(w, req) From a56b15ddf8a3f431598388a7d68f44d23ad85504 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Mon, 19 Aug 2019 09:30:01 -0500 Subject: [PATCH 6/7] fix(path-encoding): update comment --- arborist/server.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arborist/server.go b/arborist/server.go index 7d9adce6..8ff1b768 100644 --- a/arborist/server.go +++ b/arborist/server.go @@ -64,7 +64,7 @@ func (server *Server) Init() (*Server, error) { // For some reason this is not allowed: // -// `{resourcePath:/[a-zA-Z0-9_\-\/]+}` +// `{resourcePath:/.+}` // // so we put the slash at the front here and fix it in parseResourcePath. const resourcePath string = `/{resourcePath:.+}` From 74d26ded9429849f59b841be98c2e20790f2975d Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Tue, 20 Aug 2019 17:26:03 -0500 Subject: [PATCH 7/7] fix(path-encoding): fix queries --- arborist/auth.go | 112 ++++++++++++++++++++++++----------------------- 1 file changed, 58 insertions(+), 54 deletions(-) diff --git a/arborist/auth.go b/arborist/auth.go index f8d8287c..78b3ed27 100644 --- a/arborist/auth.go +++ b/arborist/auth.go @@ -212,34 +212,36 @@ func authorizeUser(request *AuthRequest) (*AuthResponse, error) { if resource != "" { err = request.stmts.Select( ` - SELECT coalesce(text2ltree($6) <@ resource.path, FALSE) FROM ( - SELECT usr_policy.policy_id FROM usr - INNER JOIN usr_policy ON usr_policy.usr_id = usr.id - WHERE usr.name = $1 - UNION - SELECT grp_policy.policy_id FROM usr - INNER JOIN usr_grp ON usr_grp.usr_id = usr.id - INNER JOIN grp_policy ON grp_policy.grp_id = usr_grp.grp_id - WHERE usr.name = $1 - UNION - SELECT grp_policy.policy_id FROM grp - INNER JOIN grp_policy ON grp_policy.grp_id = grp.id - WHERE grp.name = 'anonymous' OR grp.name = 'logged-in' - ) AS policies - JOIN policy_resource ON policy_resource.policy_id = policies.policy_id - JOIN resource ON resource.id = policy_resource.resource_id - WHERE EXISTS ( - SELECT 1 FROM policy_role - JOIN permission ON permission.role_id = policy_role.role_id - WHERE policy_role.policy_id = policies.policy_id - AND (permission.service = $2 OR permission.service = '*') - AND (permission.method = $3 OR permission.method = '*') - ) AND ( - $4 OR policies.policy_id IN ( - SELECT id FROM policy - WHERE policy.name = ANY($5) + SELECT coalesce(text2ltree($6) <@ allowed, FALSE) FROM ( + SELECT array_agg(resource.path) AS allowed FROM ( + SELECT usr_policy.policy_id FROM usr + INNER JOIN usr_policy ON usr_policy.usr_id = usr.id + WHERE usr.name = $1 + UNION + SELECT grp_policy.policy_id FROM usr + INNER JOIN usr_grp ON usr_grp.usr_id = usr.id + INNER JOIN grp_policy ON grp_policy.grp_id = usr_grp.grp_id + WHERE usr.name = $1 + UNION + SELECT grp_policy.policy_id FROM grp + INNER JOIN grp_policy ON grp_policy.grp_id = grp.id + WHERE grp.name = 'anonymous' OR grp.name = 'logged-in' + ) AS policies + JOIN policy_resource ON policy_resource.policy_id = policies.policy_id + JOIN resource ON resource.id = policy_resource.resource_id + WHERE EXISTS ( + SELECT 1 FROM policy_role + JOIN permission ON permission.role_id = policy_role.role_id + WHERE policy_role.policy_id = policies.policy_id + AND (permission.service = $2 OR permission.service = '*') + AND (permission.method = $3 OR permission.method = '*') + ) AND ( + $4 OR policies.policy_id IN ( + SELECT id FROM policy + WHERE policy.name = ANY($5) + ) ) - ) + ) _ `, &authorized, request.Username, // $1 @@ -252,34 +254,36 @@ func authorizeUser(request *AuthRequest) (*AuthResponse, error) { } else if tag != "" { err = request.stmts.Select( ` - SELECT coalesce((SELECT resource.path FROM resource WHERE resource.tag = $6) <@ resource.path, FALSE) FROM ( - SELECT usr_policy.policy_id FROM usr - INNER JOIN usr_policy ON usr_policy.usr_id = usr.id - WHERE usr.name = $1 - UNION - SELECT grp_policy.policy_id FROM usr - INNER JOIN usr_grp ON usr_grp.usr_id = usr.id - INNER JOIN grp_policy ON grp_policy.grp_id = usr_grp.grp_id - WHERE usr.name = $1 - UNION - SELECT grp_policy.policy_id FROM grp - INNER JOIN grp_policy ON grp_policy.grp_id = grp.id - WHERE grp.name = 'anonymous' OR grp.name = 'logged-in' - ) AS policies - JOIN policy_resource ON policy_resource.policy_id = policies.policy_id - JOIN resource ON resource.id = policy_resource.resource_id - WHERE EXISTS ( - SELECT 1 FROM policy_role - JOIN permission ON permission.role_id = policy_role.role_id - WHERE policy_role.policy_id = policies.policy_id - AND (permission.service = $2 OR permission.service = '*') - AND (permission.method = $3 OR permission.method = '*') - ) AND ( - $4 OR policies.policy_id IN ( - SELECT id FROM policy - WHERE policy.name = ANY($5) + SELECT coalesce((SELECT resource.path FROM resource WHERE resource.tag = $6) <@ allowed, FALSE) FROM ( + SELECT array_agg(resource.path) AS allowed FROM ( + SELECT usr_policy.policy_id FROM usr + INNER JOIN usr_policy ON usr_policy.usr_id = usr.id + WHERE usr.name = $1 + UNION + SELECT grp_policy.policy_id FROM usr + INNER JOIN usr_grp ON usr_grp.usr_id = usr.id + INNER JOIN grp_policy ON grp_policy.grp_id = usr_grp.grp_id + WHERE usr.name = $1 + UNION + SELECT grp_policy.policy_id FROM grp + INNER JOIN grp_policy ON grp_policy.grp_id = grp.id + WHERE grp.name = 'anonymous' OR grp.name = 'logged-in' + ) AS policies + JOIN policy_resource ON policy_resource.policy_id = policies.policy_id + JOIN resource ON resource.id = policy_resource.resource_id + WHERE EXISTS ( + SELECT 1 FROM policy_role + JOIN permission ON permission.role_id = policy_role.role_id + WHERE policy_role.policy_id = policies.policy_id + AND (permission.service = $2 OR permission.service = '*') + AND (permission.method = $3 OR permission.method = '*') + ) AND ( + $4 OR policies.policy_id IN ( + SELECT id FROM policy + WHERE policy.name = ANY($5) + ) ) - ) + ) _ `, &authorized, request.Username, // $1