app/vmauth: parse url_prefix only once during config load

This commit is contained in:
Aliaksandr Valialkin 2021-04-21 10:55:29 +03:00
parent 6bc52fe41a
commit 7c4e460513
5 changed files with 103 additions and 68 deletions

View File

@ -33,7 +33,7 @@ type UserInfo struct {
BearerToken string `yaml:"bearer_token"`
Username string `yaml:"username"`
Password string `yaml:"password"`
URLPrefix string `yaml:"url_prefix"`
URLPrefix *yamlURL `yaml:"url_prefix"`
URLMap []URLMap `yaml:"url_map"`
requests *metrics.Counter
@ -42,7 +42,7 @@ type UserInfo struct {
// URLMap is a mapping from source paths to target urls.
type URLMap struct {
SrcPaths []*SrcPath `yaml:"src_paths"`
URLPrefix string `yaml:"url_prefix"`
URLPrefix *yamlURL `yaml:"url_prefix"`
}
// SrcPath represents an src path
@ -51,6 +51,27 @@ type SrcPath struct {
re *regexp.Regexp
}
type yamlURL struct {
u *url.URL
}
func (yu *yamlURL) UnmarshalYAML(f func(interface{}) error) error {
var s string
if err := f(&s); err != nil {
return err
}
u, err := url.Parse(s)
if err != nil {
return fmt.Errorf("cannot unmarshal %q into url: %w", s, err)
}
yu.u = u
return nil
}
func (yu *yamlURL) MarshalYAML() (interface{}, error) {
return yu.u.String(), nil
}
func (sp *SrcPath) match(s string) bool {
prefix, ok := sp.re.LiteralPrefix()
if ok {
@ -173,24 +194,27 @@ func parseAuthConfig(data []byte) (map[string]*UserInfo, error) {
if byAuthToken[authToken] != nil {
return nil, fmt.Errorf("duplicate auth token found for bearer_token=%q, username=%q: %q", authToken, ui.BearerToken, ui.Username)
}
if len(ui.URLPrefix) > 0 {
urlPrefix, err := sanitizeURLPrefix(ui.URLPrefix)
if ui.URLPrefix != nil {
urlPrefix, err := sanitizeURLPrefix(ui.URLPrefix.u)
if err != nil {
return nil, err
}
ui.URLPrefix = urlPrefix
ui.URLPrefix.u = urlPrefix
}
for _, e := range ui.URLMap {
if len(e.SrcPaths) == 0 {
return nil, fmt.Errorf("missing `src_paths`")
return nil, fmt.Errorf("missing `src_paths` in `url_map`")
}
urlPrefix, err := sanitizeURLPrefix(e.URLPrefix)
if e.URLPrefix == nil {
return nil, fmt.Errorf("missing `url_prefix` in `url_map`")
}
urlPrefix, err := sanitizeURLPrefix(e.URLPrefix.u)
if err != nil {
return nil, err
}
e.URLPrefix = urlPrefix
e.URLPrefix.u = urlPrefix
}
if len(ui.URLMap) == 0 && len(ui.URLPrefix) == 0 {
if len(ui.URLMap) == 0 && ui.URLPrefix == nil {
return nil, fmt.Errorf("missing `url_prefix`")
}
if ui.BearerToken != "" {
@ -218,21 +242,17 @@ func getAuthToken(bearerToken, username, password string) string {
return "Basic " + token64
}
func sanitizeURLPrefix(urlPrefix string) (string, error) {
func sanitizeURLPrefix(urlPrefix *url.URL) (*url.URL, error) {
// Remove trailing '/' from urlPrefix
for strings.HasSuffix(urlPrefix, "/") {
urlPrefix = urlPrefix[:len(urlPrefix)-1]
for strings.HasSuffix(urlPrefix.Path, "/") {
urlPrefix.Path = urlPrefix.Path[:len(urlPrefix.Path)-1]
}
// Validate urlPrefix
target, err := url.Parse(urlPrefix)
if err != nil {
return "", fmt.Errorf("invalid `url_prefix: %q`: %w", urlPrefix, err)
if urlPrefix.Scheme != "http" && urlPrefix.Scheme != "https" {
return nil, fmt.Errorf("unsupported scheme for `url_prefix: %q`: %q; must be `http` or `https`", urlPrefix, urlPrefix.Scheme)
}
if target.Scheme != "http" && target.Scheme != "https" {
return "", fmt.Errorf("unsupported scheme for `url_prefix: %q`: %q; must be `http` or `https`", urlPrefix, target.Scheme)
}
if target.Host == "" {
return "", fmt.Errorf("missing hostname in `url_prefix %q`", urlPrefix)
if urlPrefix.Host == "" {
return nil, fmt.Errorf("missing hostname in `url_prefix %q`", urlPrefix.Host)
}
return urlPrefix, nil
}

View File

@ -3,6 +3,7 @@ package main
import (
"bytes"
"fmt"
"net/url"
"regexp"
"testing"
@ -55,6 +56,11 @@ users:
- username: foo
url_prefix: http:///bar
`)
f(`
users:
- username: foo
url_prefix: [bar]
`)
// Username and bearer_token in a single config
f(`
@ -102,6 +108,15 @@ users:
- src_paths: ["/foo/bar"]
`)
// Invalid url_prefix in url_map
f(`
users:
- username: a
url_map:
- src_paths: ["/foo/bar"]
url_prefix: foo.bar
`)
// Missing src_paths in url_map
f(`
users:
@ -143,7 +158,7 @@ users:
getAuthToken("", "foo", "bar"): {
Username: "foo",
Password: "bar",
URLPrefix: "http://aaa:343/bbb",
URLPrefix: mustParseURL("http://aaa:343/bbb"),
},
})
@ -157,11 +172,11 @@ users:
`, map[string]*UserInfo{
getAuthToken("", "foo", ""): {
Username: "foo",
URLPrefix: "http://foo",
URLPrefix: mustParseURL("http://foo"),
},
getAuthToken("", "bar", ""): {
Username: "bar",
URLPrefix: "https://bar/x",
URLPrefix: mustParseURL("https://bar/x"),
},
})
@ -180,11 +195,11 @@ users:
URLMap: []URLMap{
{
SrcPaths: getSrcPaths([]string{"/api/v1/query", "/api/v1/query_range", "/api/v1/label/[^./]+/.+"}),
URLPrefix: "http://vmselect/select/0/prometheus",
URLPrefix: mustParseURL("http://vmselect/select/0/prometheus"),
},
{
SrcPaths: getSrcPaths([]string{"/api/v1/write"}),
URLPrefix: "http://vminsert/insert/0/prometheus",
URLPrefix: mustParseURL("http://vminsert/insert/0/prometheus"),
},
},
},
@ -222,3 +237,13 @@ func areEqualConfigs(a, b map[string]*UserInfo) error {
}
return nil
}
func mustParseURL(u string) *yamlURL {
pu, err := url.Parse(u)
if err != nil {
panic(fmt.Errorf("BUG: cannot parse %q: %w", u, err))
}
return &yamlURL{
u: pu,
}
}

View File

@ -65,11 +65,7 @@ func requestHandler(w http.ResponseWriter, r *http.Request) bool {
httpserver.Errorf(w, r, "cannot determine targetURL: %s", err)
return true
}
if _, err := url.Parse(targetURL); err != nil {
httpserver.Errorf(w, r, "invalid targetURL=%q: %s", targetURL, err)
return true
}
r.Header.Set("vm-target-url", targetURL)
r.Header.Set("vm-target-url", targetURL.String())
reverseProxy.ServeHTTP(w, r)
return true
}

View File

@ -7,37 +7,31 @@ import (
"strings"
)
func mergeURLs(uiURL string, requestURI *url.URL) (string, error) {
prefixURL, err := url.Parse(uiURL)
if err != nil {
return "", fmt.Errorf("BUG - cannot parse userInfo url: %q, err: %w", uiURL, err)
}
prefixURL.Path += requestURI.Path
func mergeURLs(uiURL, requestURI *url.URL) *url.URL {
targetURL := *uiURL
targetURL.Path += requestURI.Path
requestParams := requestURI.Query()
// fast path
if len(requestParams) == 0 {
return prefixURL.String(), nil
return &targetURL
}
// merge query parameters from requests.
userInfoParams := prefixURL.Query()
uiParams := targetURL.Query()
for k, v := range requestParams {
// skip clashed query params from original request
if exist := userInfoParams.Get(k); len(exist) > 0 {
if exist := uiParams.Get(k); len(exist) > 0 {
continue
}
for i := range v {
userInfoParams.Add(k, v[i])
uiParams.Add(k, v[i])
}
}
prefixURL.RawQuery = userInfoParams.Encode()
return prefixURL.String(), nil
targetURL.RawQuery = uiParams.Encode()
return &targetURL
}
func createTargetURL(ui *UserInfo, uOrig *url.URL) (string, error) {
u, err := url.Parse(uOrig.String())
if err != nil {
return "", fmt.Errorf("cannot make a copy of %q: %w", u, err)
}
func createTargetURL(ui *UserInfo, uOrig *url.URL) (*url.URL, error) {
u := *uOrig
// Prevent from attacks with using `..` in r.URL.Path
u.Path = path.Clean(u.Path)
if !strings.HasPrefix(u.Path, "/") {
@ -46,12 +40,12 @@ func createTargetURL(ui *UserInfo, uOrig *url.URL) (string, error) {
for _, e := range ui.URLMap {
for _, sp := range e.SrcPaths {
if sp.match(u.Path) {
return mergeURLs(e.URLPrefix, u)
return mergeURLs(e.URLPrefix.u, &u), nil
}
}
}
if len(ui.URLPrefix) > 0 {
return mergeURLs(ui.URLPrefix, u)
if ui.URLPrefix != nil {
return mergeURLs(ui.URLPrefix.u, &u), nil
}
return "", fmt.Errorf("missing route for %q", u)
return nil, fmt.Errorf("missing route for %q", u.String())
}

View File

@ -16,28 +16,28 @@ func TestCreateTargetURLSuccess(t *testing.T) {
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
if target != expectedTarget {
if target.String() != expectedTarget {
t.Fatalf("unexpected target; got %q; want %q", target, expectedTarget)
}
}
// Simple routing with `url_prefix`
f(&UserInfo{
URLPrefix: "http://foo.bar",
URLPrefix: mustParseURL("http://foo.bar"),
}, "", "http://foo.bar/.")
f(&UserInfo{
URLPrefix: "http://foo.bar",
URLPrefix: mustParseURL("http://foo.bar"),
}, "/", "http://foo.bar/")
f(&UserInfo{
URLPrefix: "http://foo.bar",
URLPrefix: mustParseURL("http://foo.bar"),
}, "a/b?c=d", "http://foo.bar/a/b?c=d")
f(&UserInfo{
URLPrefix: "https://sss:3894/x/y",
URLPrefix: mustParseURL("https://sss:3894/x/y"),
}, "/z", "https://sss:3894/x/y/z")
f(&UserInfo{
URLPrefix: "https://sss:3894/x/y",
URLPrefix: mustParseURL("https://sss:3894/x/y"),
}, "/../../aaa", "https://sss:3894/x/y/aaa")
f(&UserInfo{
URLPrefix: "https://sss:3894/x/y",
URLPrefix: mustParseURL("https://sss:3894/x/y"),
}, "/./asd/../../aaa?a=d&s=s/../d", "https://sss:3894/x/y/aaa?a=d&s=s%2F..%2Fd")
// Complex routing with `url_map`
@ -45,14 +45,14 @@ func TestCreateTargetURLSuccess(t *testing.T) {
URLMap: []URLMap{
{
SrcPaths: getSrcPaths([]string{"/api/v1/query"}),
URLPrefix: "http://vmselect/0/prometheus",
URLPrefix: mustParseURL("http://vmselect/0/prometheus"),
},
{
SrcPaths: getSrcPaths([]string{"/api/v1/write"}),
URLPrefix: "http://vminsert/0/prometheus",
URLPrefix: mustParseURL("http://vminsert/0/prometheus"),
},
},
URLPrefix: "http://default-server",
URLPrefix: mustParseURL("http://default-server"),
}
f(ui, "/api/v1/query?query=up", "http://vmselect/0/prometheus/api/v1/query?query=up")
f(ui, "/api/v1/write", "http://vminsert/0/prometheus/api/v1/write")
@ -63,14 +63,14 @@ func TestCreateTargetURLSuccess(t *testing.T) {
URLMap: []URLMap{
{
SrcPaths: getSrcPaths([]string{"/api/v1/query(_range)?", "/api/v1/label/[^/]+/values"}),
URLPrefix: "http://vmselect/0/prometheus",
URLPrefix: mustParseURL("http://vmselect/0/prometheus"),
},
{
SrcPaths: getSrcPaths([]string{"/api/v1/write"}),
URLPrefix: "http://vminsert/0/prometheus",
URLPrefix: mustParseURL("http://vminsert/0/prometheus"),
},
},
URLPrefix: "http://default-server",
URLPrefix: mustParseURL("http://default-server"),
}
f(ui, "/api/v1/query?query=up", "http://vmselect/0/prometheus/api/v1/query?query=up")
f(ui, "/api/v1/query_range?query=up", "http://vmselect/0/prometheus/api/v1/query_range?query=up")
@ -78,10 +78,10 @@ func TestCreateTargetURLSuccess(t *testing.T) {
f(ui, "/api/v1/write", "http://vminsert/0/prometheus/api/v1/write")
f(ui, "/api/v1/foo/bar", "http://default-server/api/v1/foo/bar")
f(&UserInfo{
URLPrefix: "http://foo.bar?extra_label=team=dev",
URLPrefix: mustParseURL("http://foo.bar?extra_label=team=dev"),
}, "/api/v1/query", "http://foo.bar/api/v1/query?extra_label=team=dev")
f(&UserInfo{
URLPrefix: "http://foo.bar?extra_label=team=mobile",
URLPrefix: mustParseURL("http://foo.bar?extra_label=team=mobile"),
}, "/api/v1/query?extra_label=team=dev", "http://foo.bar/api/v1/query?extra_label=team%3Dmobile")
}
@ -97,7 +97,7 @@ func TestCreateTargetURLFailure(t *testing.T) {
if err == nil {
t.Fatalf("expecting non-nil error")
}
if target != "" {
if target != nil {
t.Fatalf("unexpected target=%q; want empty string", target)
}
}
@ -106,7 +106,7 @@ func TestCreateTargetURLFailure(t *testing.T) {
URLMap: []URLMap{
{
SrcPaths: getSrcPaths([]string{"/api/v1/query"}),
URLPrefix: "http://foobar/baz",
URLPrefix: mustParseURL("http://foobar/baz"),
},
},
}, "/api/v1/write")