Team LBAC: Add validation/regex of teamheaders (#76905)

* add validation of team header values w. regex

* apply valid headers

* refactor testcases to account for badly formatted json

* refactoring to move validation code close to the validation itself

* removed tes

* Update pkg/api/datasources_test.go

Co-authored-by: Alexander Zobnin <alexanderzobnin@gmail.com>

* Update pkg/api/datasources.go

Co-authored-by: Alexander Zobnin <alexanderzobnin@gmail.com>

* review comments

* review during pairing

---------

Co-authored-by: Alexander Zobnin <alexanderzobnin@gmail.com>
This commit is contained in:
Eric Leijonmarck
2023-10-27 12:37:37 +01:00
committed by GitHub
parent b537bb12f1
commit 57335cb173
3 changed files with 171 additions and 47 deletions

View File

@@ -6,6 +6,7 @@ import (
"errors"
"fmt"
"net/http"
"regexp"
"sort"
"strconv"
"strings"
@@ -351,28 +352,64 @@ func validateJSONData(ctx context.Context, jsonData *simplejson.Json, cfg *setti
// Prevent adding a data source team header with a name that matches the auth proxy header name
if features.IsEnabled(featuremgmt.FlagTeamHttpHeaders) {
teamHTTPHeadersJSON, err := datasources.GetTeamHTTPHeaders(jsonData)
err := validateTeamHTTPHeaderJSON(jsonData)
if err != nil {
datasourcesLogger.Error("Unable to marshal TeamHTTPHeaders")
return errors.New("validation error, invalid format of TeamHTTPHeaders")
}
// whitelisting X-Prom-Label-Policy
for _, headers := range teamHTTPHeadersJSON {
for _, header := range headers {
// TODO: currently we only allow for X-Prom-Label-Policy header to be used by our proxy
for _, name := range []string{"X-Prom-Label-Policy"} {
if http.CanonicalHeaderKey(header.Header) != http.CanonicalHeaderKey(name) {
datasourcesLogger.Error("Cannot add a data source team header that is different than", "headerName", name)
return errors.New("validation error, invalid header name specified")
}
}
}
return err
}
}
return nil
}
// we only allow for now the following headers to be added to a data source team header
var validHeaders = []string{"X-Prom-Label-Policy"}
func validateTeamHTTPHeaderJSON(jsonData *simplejson.Json) error {
teamHTTPHeadersJSON, err := datasources.GetTeamHTTPHeaders(jsonData)
if err != nil {
datasourcesLogger.Error("Unable to marshal TeamHTTPHeaders")
return errors.New("validation error, invalid format of TeamHTTPHeaders")
}
// whitelisting ValidHeaders
// each teams headers
for _, teamheaders := range teamHTTPHeadersJSON {
for _, header := range teamheaders {
if !contains(validHeaders, header.Header) {
datasourcesLogger.Error("Cannot add a data source team header that is different than", "headerName", header.Header)
return errors.New("validation error, invalid header name specified")
}
if !teamHTTPHeaderValueRegexMatch(header.Value) {
datasourcesLogger.Error("Cannot add a data source team header value with invalid value", "headerValue", header.Value)
return errors.New("validation error, invalid header value syntax")
}
}
}
return nil
}
func contains(slice []string, value string) bool {
for _, v := range slice {
if http.CanonicalHeaderKey(v) == http.CanonicalHeaderKey(value) {
return true
}
}
return false
}
// teamHTTPHeaderValueRegexMatch returns true if the header value matches the regex
// words separated by special characters
// namespace!="auth", env="prod", env!~"dev"
func teamHTTPHeaderValueRegexMatch(headervalue string) bool {
// link to regex: https://regex101.com/r/I8KhZz/1
// 1234:{ name!="value",foo!~"bar" }
exp := `^\d+:{(?:\s*\w+\s*(?:=|!=|=~|!~)\s*\"\w+\"\s*,*)+}$`
reg, err := regexp.Compile(exp)
if err != nil {
return false
}
return reg.Match([]byte(strings.TrimSpace(headervalue)))
}
// swagger:route POST /datasources datasources addDataSource
//
// Create a data source.

View File

@@ -3,6 +3,7 @@ package api
import (
"context"
"encoding/json"
"fmt"
"io"
"net/http"
"strings"
@@ -224,44 +225,90 @@ func TestUpdateDataSource_InvalidJSONData(t *testing.T) {
// Using a team HTTP header whose name matches the name specified for auth proxy header should fail
func TestUpdateDataSourceTeamHTTPHeaders_InvalidJSONData(t *testing.T) {
hs := &HTTPServer{
DataSourcesService: &dataSourcesServiceMock{},
Cfg: setting.NewCfg(),
Features: featuremgmt.WithFeatures(featuremgmt.FlagTeamHttpHeaders),
}
sc := setupScenarioContext(t, "/api/datasources/1234")
data := datasources.TeamHTTPHeaders{
"1234": []datasources.TeamHTTPHeader{
// Authorization is used by the auth proxy
// As part of
// contexthandler.AuthHTTPHeaderListFromContext(ctx)
{
Header: "Authorization",
Value: "Could be anything",
tenantID := "1234"
testcases := []struct {
desc string
data datasources.TeamHTTPHeaders
want int
}{
{
desc: "We should only allow for headers being X-Prom-Label-Policy",
data: datasources.TeamHTTPHeaders{tenantID: []datasources.TeamHTTPHeader{
{
Header: "Authorization",
Value: "foo!=bar",
},
},
},
want: 400,
},
{
desc: "Allowed header but no team id",
data: datasources.TeamHTTPHeaders{"": []datasources.TeamHTTPHeader{
{
Header: "X-Prom-Label-Policy",
Value: "foo=bar",
},
},
},
want: 400,
},
{
desc: "Allowed team id and header name with invalid header values ",
data: datasources.TeamHTTPHeaders{tenantID: []datasources.TeamHTTPHeader{
{
Header: "X-Prom-Label-Policy",
Value: "Bad value",
},
},
},
want: 400,
},
// Complete valid case, with team id, header name and header value
{
desc: "Allowed header and header values ",
data: datasources.TeamHTTPHeaders{tenantID: []datasources.TeamHTTPHeader{
{
Header: "X-Prom-Label-Policy",
Value: `1234:{ name!="value",foo!~"bar" }`,
},
},
},
want: 200,
},
}
for _, tc := range testcases {
t.Run(tc.desc, func(t *testing.T) {
hs := &HTTPServer{
DataSourcesService: &dataSourcesServiceMock{
expectedDatasource: &datasources.DataSource{},
},
Cfg: setting.NewCfg(),
Features: featuremgmt.WithFeatures(featuremgmt.FlagTeamHttpHeaders),
accesscontrolService: actest.FakeService{},
}
sc := setupScenarioContext(t, fmt.Sprintf("/api/datasources/%s", tenantID))
hs.Cfg.AuthProxyEnabled = true
hs.Cfg.AuthProxyEnabled = true
jsonData := simplejson.New()
jsonData.Set("teamHttpHeaders", data)
jsonData := simplejson.New()
jsonData.Set("teamHttpHeaders", tc.data)
sc.m.Put(sc.url, routing.Wrap(func(c *contextmodel.ReqContext) response.Response {
c.Req.Body = mockRequestBody(datasources.AddDataSourceCommand{
Name: "Test",
URL: "localhost:5432",
Access: "direct",
Type: "test",
JsonData: jsonData,
})
c.SignedInUser = authedUserWithPermissions(1, 1, []ac.Permission{})
return hs.AddDataSource(c)
}))
sc.m.Put(sc.url, routing.Wrap(func(c *contextmodel.ReqContext) response.Response {
c.Req.Body = mockRequestBody(datasources.AddDataSourceCommand{
Name: "Test",
URL: "localhost:5432",
Access: "direct",
Type: "test",
JsonData: jsonData,
sc.fakeReqWithParams("PUT", sc.url, map[string]string{}).exec()
assert.Equal(t, tc.want, sc.resp.Code)
})
c.SignedInUser = authedUserWithPermissions(1, 1, []ac.Permission{})
return hs.AddDataSource(c)
}))
sc.fakeReqWithParams("PUT", sc.url, map[string]string{}).exec()
assert.Equal(t, 400, sc.resp.Code)
}
}
// Updating data sources with URLs not specifying protocol should work.
@@ -434,6 +481,31 @@ func TestAPI_datasources_AccessControl(t *testing.T) {
}
}
// TeamHTTPHeaderValueRegexMatch returns a regex that can be used to check
func TestTeamHTTPHeaderValueRegexMatch(t *testing.T) {
testcases := []struct {
desc string
teamHeaderValue string
want bool
}{
{
desc: "Should be valid regex match for team headervalue",
teamHeaderValue: `1234:{ name!="value",foo!~"bar" }`,
want: true,
},
{
desc: "Should return false for incorrect header value",
teamHeaderValue: `1234:!="value",foo!~"bar" }`,
want: false,
},
}
for _, tc := range testcases {
t.Run(tc.desc, func(t *testing.T) {
assert.Equal(t, tc.want, teamHTTPHeaderValueRegexMatch(tc.teamHeaderValue))
})
}
}
type dataSourcesServiceMock struct {
datasources.DataSourceService