From 09d58d1858b650ae055f0e8b1896906ffe167b94 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov <84287187+averche@users.noreply.github.com> Date: Mon, 20 Mar 2023 13:25:09 -0400 Subject: [PATCH] openapi: Fix logic for labeling unauthenticated/sudo paths (#19600) --- changelog/19600.txt | 3 + sdk/framework/openapi.go | 47 ++++++++++- sdk/framework/openapi_test.go | 149 ++++++++++++++++++++++++++-------- 3 files changed, 162 insertions(+), 37 deletions(-) create mode 100644 changelog/19600.txt diff --git a/changelog/19600.txt b/changelog/19600.txt new file mode 100644 index 000000000000..f2c1f71fa027 --- /dev/null +++ b/changelog/19600.txt @@ -0,0 +1,3 @@ +```release-note:bug +openapi: Fix logic for labeling unauthenticated/sudo paths. +``` diff --git a/sdk/framework/openapi.go b/sdk/framework/openapi.go index 57906672827a..d69e0b83e69d 100644 --- a/sdk/framework/openapi.go +++ b/sdk/framework/openapi.go @@ -547,14 +547,55 @@ func constructRequestResponseName(path, prefix, suffix string) string { return b.String() } +// specialPathMatch checks whether the given path matches one of the special +// paths, taking into account * and + wildcards (e.g. foo/+/bar/*) func specialPathMatch(path string, specialPaths []string) bool { - // Test for exact or prefix match of special paths. + // pathMatchesByParts determines if the path matches the special path's + // pattern, accounting for the '+' and '*' wildcards + pathMatchesByParts := func(pathParts []string, specialPathParts []string) bool { + if len(pathParts) < len(specialPathParts) { + return false + } + for i := 0; i < len(specialPathParts); i++ { + var ( + part = pathParts[i] + pattern = specialPathParts[i] + ) + if pattern == "+" { + continue + } + if pattern == "*" { + return true + } + if strings.HasSuffix(pattern, "*") && strings.HasPrefix(part, pattern[0:len(pattern)-1]) { + return true + } + if pattern != part { + return false + } + } + return len(pathParts) == len(specialPathParts) + } + + pathParts := strings.Split(path, "/") + for _, sp := range specialPaths { - if sp == path || - (strings.HasSuffix(sp, "*") && strings.HasPrefix(path, sp[0:len(sp)-1])) { + // exact match + if sp == path { + return true + } + + // match * + if strings.HasSuffix(sp, "*") && strings.HasPrefix(path, sp[0:len(sp)-1]) { + return true + } + + // match + + if strings.Contains(sp, "+") && pathMatchesByParts(pathParts, strings.Split(sp, "/")) { return true } } + return false } diff --git a/sdk/framework/openapi_test.go b/sdk/framework/openapi_test.go index 648a785fa969..bb1338b4e0a3 100644 --- a/sdk/framework/openapi_test.go +++ b/sdk/framework/openapi_test.go @@ -226,42 +226,123 @@ func TestOpenAPI_SplitFields(t *testing.T) { } func TestOpenAPI_SpecialPaths(t *testing.T) { - tests := []struct { - pattern string - rootPaths []string - root bool - unauthPaths []string - unauth bool + tests := map[string]struct { + pattern string + rootPaths []string + rootExpected bool + unauthenticatedPaths []string + unauthenticatedExpected bool }{ - {"foo", []string{}, false, []string{"foo"}, true}, - {"foo", []string{"foo"}, true, []string{"bar"}, false}, - {"foo/bar", []string{"foo"}, false, []string{"foo/*"}, true}, - {"foo/bar", []string{"foo/*"}, true, []string{"foo"}, false}, - {"foo/", []string{"foo/*"}, true, []string{"a", "b", "foo/"}, true}, - {"foo", []string{"foo*"}, true, []string{"a", "fo*"}, true}, - {"foo/bar", []string{"a", "b", "foo/*"}, true, []string{"foo/baz/*"}, false}, + "empty": { + pattern: "foo", + rootPaths: []string{}, + rootExpected: false, + unauthenticatedPaths: []string{}, + unauthenticatedExpected: false, + }, + "exact-match-unauthenticated": { + pattern: "foo", + rootPaths: []string{}, + rootExpected: false, + unauthenticatedPaths: []string{"foo"}, + unauthenticatedExpected: true, + }, + "exact-match-root": { + pattern: "foo", + rootPaths: []string{"foo"}, + rootExpected: true, + unauthenticatedPaths: []string{"bar"}, + unauthenticatedExpected: false, + }, + "asterisk-match-unauthenticated": { + pattern: "foo/bar", + rootPaths: []string{"foo"}, + rootExpected: false, + unauthenticatedPaths: []string{"foo/*"}, + unauthenticatedExpected: true, + }, + "asterisk-match-root": { + pattern: "foo/bar", + rootPaths: []string{"foo/*"}, + rootExpected: true, + unauthenticatedPaths: []string{"foo"}, + unauthenticatedExpected: false, + }, + "path-ends-with-slash": { + pattern: "foo/", + rootPaths: []string{"foo/*"}, + rootExpected: true, + unauthenticatedPaths: []string{"a", "b", "foo*"}, + unauthenticatedExpected: true, + }, + "asterisk-match-no-slash": { + pattern: "foo", + rootPaths: []string{"foo*"}, + rootExpected: true, + unauthenticatedPaths: []string{"a", "fo*"}, + unauthenticatedExpected: true, + }, + "multiple-root-paths": { + pattern: "foo/bar", + rootPaths: []string{"a", "b", "foo/*"}, + rootExpected: true, + unauthenticatedPaths: []string{"foo/baz/*"}, + unauthenticatedExpected: false, + }, + "plus-match-unauthenticated": { + pattern: "foo/bar/baz", + rootPaths: []string{"foo/bar"}, + rootExpected: false, + unauthenticatedPaths: []string{"foo/+/baz"}, + unauthenticatedExpected: true, + }, + "plus-match-root": { + pattern: "foo/bar/baz", + rootPaths: []string{"foo/+/baz"}, + rootExpected: true, + unauthenticatedPaths: []string{"foo/bar"}, + unauthenticatedExpected: false, + }, + "plus-and-asterisk": { + pattern: "foo/bar/baz/something", + rootPaths: []string{"foo/+/baz/*"}, + rootExpected: true, + unauthenticatedPaths: []string{"foo/+/baz*"}, + unauthenticatedExpected: true, + }, + "double-plus-good": { + pattern: "foo/bar/baz", + rootPaths: []string{"foo/+/+"}, + rootExpected: true, + unauthenticatedPaths: []string{"foo/bar"}, + unauthenticatedExpected: false, + }, } - for i, test := range tests { - doc := NewOASDocument("version") - path := Path{ - Pattern: test.pattern, - } - sp := &logical.Paths{ - Root: test.rootPaths, - Unauthenticated: test.unauthPaths, - } - err := documentPath(&path, sp, "kv", logical.TypeLogical, doc) - if err != nil { - t.Fatal(err) - } - result := test.root - if doc.Paths["/"+test.pattern].Sudo != result { - t.Fatalf("Test (root) %d: Expected %v got %v", i, test.root, result) - } - result = test.unauth - if doc.Paths["/"+test.pattern].Unauthenticated != result { - t.Fatalf("Test (unauth) %d: Expected %v got %v", i, test.unauth, result) - } + for name, test := range tests { + t.Run(name, func(t *testing.T) { + doc := NewOASDocument("version") + path := Path{ + Pattern: test.pattern, + } + specialPaths := &logical.Paths{ + Root: test.rootPaths, + Unauthenticated: test.unauthenticatedPaths, + } + + if err := documentPath(&path, specialPaths, "kv", logical.TypeLogical, doc); err != nil { + t.Fatal(err) + } + + actual := doc.Paths["/"+test.pattern].Sudo + if actual != test.rootExpected { + t.Fatalf("Test (root): expected: %v; got: %v", test.rootExpected, actual) + } + + actual = doc.Paths["/"+test.pattern].Unauthenticated + if actual != test.unauthenticatedExpected { + t.Fatalf("Test (unauth): expected: %v; got: %v", test.unauthenticatedExpected, actual) + } + }) } }