[v11.2.x] Canvas: Allow API calls to grafana origin (#94129)
Canvas: Allow API calls to grafana origin (#91822)
* allow post URL
* check for config
* allow relative paths
* add allowed internal pattern; add checks for method
* update defaults.ini
* add custom header
* update config comment
* use globbing, switch to older middleware - deprecated call
* add codeowner
* update to use current api, add test
* update fall through logic
* Update pkg/middleware/validate_action_url.go
Co-authored-by: Dan Cech <dcech@grafana.com>
* Update pkg/middleware/validate_action_url.go
Co-authored-by: Dan Cech <dcech@grafana.com>
* add more tests
* Update pkg/middleware/validate_action_url_test.go
Co-authored-by: Dan Cech <dcech@grafana.com>
* fix request headers
* add additional tests for all verbs
* fix request headers++
* throw error when method is unknown
---------
Co-authored-by: Ryan McKinley <ryantxu@gmail.com>
Co-authored-by: Brian Gann <bkgann@gmail.com>
Co-authored-by: Brian Gann <briangann@users.noreply.github.com>
Co-authored-by: Dan Cech <dcech@grafana.com>
(cherry picked from commit f64b121ddb)
Co-authored-by: Adela Almasan <88068998+adela-almasan@users.noreply.github.com>
This commit is contained in:
grafana-delivery-bot[bot]
co-authored by
Adela Almasan
Adela Almasan
parent
35b3075d06
commit
c4b8303799
@@ -244,6 +244,8 @@ func middlewareScenario(t *testing.T, desc string, fn scenarioFunc, cbs ...func(
|
||||
ctxHdlr := getContextHandler(t, cfg, sc.authnService)
|
||||
sc.m.Use(ctxHdlr.Middleware)
|
||||
sc.m.Use(OrgRedirect(sc.cfg, sc.userService))
|
||||
// handle action urls
|
||||
sc.m.Use(ValidateActionUrl(sc.cfg, logger))
|
||||
|
||||
sc.defaultHandler = func(c *contextmodel.ReqContext) {
|
||||
require.NotNil(t, c)
|
||||
|
||||
@@ -0,0 +1,119 @@
|
||||
package middleware
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"net/http"
|
||||
|
||||
"github.com/gobwas/glob"
|
||||
"github.com/grafana/grafana/pkg/cmd/grafana-cli/logger"
|
||||
"github.com/grafana/grafana/pkg/infra/log"
|
||||
"github.com/grafana/grafana/pkg/services/contexthandler"
|
||||
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
|
||||
"github.com/grafana/grafana/pkg/setting"
|
||||
"github.com/grafana/grafana/pkg/util"
|
||||
)
|
||||
|
||||
var (
|
||||
errInvalidAllowedURL = func(url string) error {
|
||||
return fmt.Errorf("action URL '%s' is invalid", url)
|
||||
}
|
||||
)
|
||||
|
||||
type errorWithStatus struct {
|
||||
Underlying error
|
||||
HTTPStatus int
|
||||
}
|
||||
|
||||
func (e errorWithStatus) Error() string {
|
||||
return e.Underlying.Error()
|
||||
}
|
||||
|
||||
func (e errorWithStatus) Unwrap() error {
|
||||
return e.Underlying
|
||||
}
|
||||
|
||||
func ValidateActionUrl(cfg *setting.Cfg, logger log.Logger) func(http.Handler) http.Handler {
|
||||
// get the urls allowed from server config
|
||||
allGlobs, globErr := cacheGlobs(cfg.ActionsAllowPostURL)
|
||||
if globErr != nil {
|
||||
logger.Error("invalid glob settings in config section [security] actions_allow_post_url", "url", cfg.ActionsAllowPostURL)
|
||||
}
|
||||
|
||||
return func(next http.Handler) http.Handler {
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
ctx := contexthandler.FromContext(r.Context())
|
||||
// if no header
|
||||
// return nil
|
||||
// check if action header exists
|
||||
action := ctx.Req.Header.Get("X-Grafana-Action")
|
||||
if action == "" {
|
||||
// header not found, this is not an action request
|
||||
next.ServeHTTP(w, r)
|
||||
return
|
||||
}
|
||||
if globErr != nil {
|
||||
http.Error(w, "check server logs for glob configuration failure", http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
matchErr := check(ctx, allGlobs, logger)
|
||||
if matchErr != nil {
|
||||
http.Error(w, matchErr.Error(), http.StatusMethodNotAllowed)
|
||||
return
|
||||
}
|
||||
// no errors fall through
|
||||
next.ServeHTTP(w, r)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// check
|
||||
// Detects header for action urls and compares to globbed pattern list
|
||||
// returns true if allowed
|
||||
func check(ctx *contextmodel.ReqContext, allGlobs *[]glob.Glob, logger log.Logger) *errorWithStatus {
|
||||
// only process POST and PUT
|
||||
if ctx.Req.Method != http.MethodPost && ctx.Req.Method != http.MethodPut {
|
||||
return &errorWithStatus{
|
||||
Underlying: fmt.Errorf("method not allowed for path %s", ctx.Req.URL),
|
||||
HTTPStatus: http.StatusMethodNotAllowed,
|
||||
}
|
||||
}
|
||||
// for each split config
|
||||
// if matches glob
|
||||
// return nil
|
||||
urlToCheck := ctx.Req.URL
|
||||
if matchesAllowedPath(allGlobs, urlToCheck.Path) {
|
||||
return nil
|
||||
}
|
||||
logger.Warn("POST/PUT to path not allowed", "url", urlToCheck)
|
||||
// return some error
|
||||
return &errorWithStatus{
|
||||
Underlying: fmt.Errorf("method POST/PUT not allowed for path %s", urlToCheck),
|
||||
HTTPStatus: http.StatusMethodNotAllowed,
|
||||
}
|
||||
}
|
||||
|
||||
func matchesAllowedPath(allGlobs *[]glob.Glob, pathToCheck string) bool {
|
||||
logger.Debug("Checking url", "actions", pathToCheck)
|
||||
for _, rule := range *allGlobs {
|
||||
logger.Debug("Checking match", "actions", rule)
|
||||
if rule.Match(pathToCheck) {
|
||||
// allowed
|
||||
logger.Debug("POST/PUT call matches allow configuration settings")
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
func cacheGlobs(actionsAllowPostURL string) (*[]glob.Glob, error) {
|
||||
allowedUrls := util.SplitString(actionsAllowPostURL)
|
||||
allGlobs := make([]glob.Glob, 0)
|
||||
for _, i := range allowedUrls {
|
||||
g, err := glob.Compile(i)
|
||||
if err != nil {
|
||||
return nil, errInvalidAllowedURL(err.Error())
|
||||
}
|
||||
allGlobs = append(allGlobs, g)
|
||||
}
|
||||
return &allGlobs, nil
|
||||
}
|
||||
@@ -0,0 +1,306 @@
|
||||
package middleware
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"net/http"
|
||||
"testing"
|
||||
|
||||
"github.com/grafana/grafana/pkg/setting"
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func TestMiddlewareValidateActionUrl(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
method string
|
||||
path string
|
||||
actionsAllowPostURL string
|
||||
addHeader bool
|
||||
code int
|
||||
}{
|
||||
{
|
||||
name: "DELETE action with valid path",
|
||||
method: "DELETE",
|
||||
path: "/api/plugins/org-generic-app",
|
||||
actionsAllowPostURL: "/api/plugins/*",
|
||||
addHeader: true,
|
||||
code: http.StatusMethodNotAllowed,
|
||||
},
|
||||
{
|
||||
name: "DELETE action with invalid path",
|
||||
method: "DELETE",
|
||||
path: "/api/notplugins/org-generic-app",
|
||||
actionsAllowPostURL: "/api/plugins/*",
|
||||
addHeader: true,
|
||||
code: http.StatusMethodNotAllowed,
|
||||
},
|
||||
{
|
||||
name: "GET action with valid path",
|
||||
method: "GET",
|
||||
path: "/api/plugins/org-generic-app",
|
||||
actionsAllowPostURL: "/api/plugins/*",
|
||||
addHeader: true,
|
||||
code: http.StatusMethodNotAllowed,
|
||||
},
|
||||
{
|
||||
name: "GET action with invalid path",
|
||||
method: "GET",
|
||||
path: "/api/notplugins/org-generic-app",
|
||||
actionsAllowPostURL: "/api/plugins/*",
|
||||
addHeader: true,
|
||||
code: http.StatusMethodNotAllowed,
|
||||
},
|
||||
{
|
||||
name: "GET valid path without header",
|
||||
method: "GET",
|
||||
path: "/", // top-level get
|
||||
actionsAllowPostURL: "",
|
||||
addHeader: false,
|
||||
code: http.StatusOK,
|
||||
},
|
||||
{
|
||||
name: "GET valid path with header",
|
||||
method: "GET",
|
||||
path: "/", // top-level get
|
||||
actionsAllowPostURL: "",
|
||||
addHeader: true,
|
||||
code: http.StatusMethodNotAllowed,
|
||||
},
|
||||
{
|
||||
name: "HEAD request with header",
|
||||
method: "HEAD",
|
||||
path: "/", // top-level
|
||||
actionsAllowPostURL: "",
|
||||
addHeader: true,
|
||||
code: http.StatusMethodNotAllowed,
|
||||
},
|
||||
{
|
||||
name: "OPTIONS request",
|
||||
method: "OPTIONS",
|
||||
path: "/", // top-level
|
||||
actionsAllowPostURL: "",
|
||||
addHeader: false,
|
||||
code: http.StatusOK,
|
||||
},
|
||||
{
|
||||
name: "OPTIONS request with header",
|
||||
method: "OPTIONS",
|
||||
path: "/", // top-level
|
||||
actionsAllowPostURL: "",
|
||||
addHeader: true,
|
||||
code: http.StatusMethodNotAllowed,
|
||||
},
|
||||
{
|
||||
name: "PATCH request with header",
|
||||
method: "PATCH",
|
||||
path: "/", // top-level
|
||||
actionsAllowPostURL: "",
|
||||
addHeader: true,
|
||||
code: http.StatusMethodNotAllowed,
|
||||
},
|
||||
{
|
||||
name: "PATCH request without header",
|
||||
method: "PATCH",
|
||||
path: "/", // top-level
|
||||
actionsAllowPostURL: "",
|
||||
addHeader: false,
|
||||
code: http.StatusOK,
|
||||
},
|
||||
{
|
||||
name: "POST without action header",
|
||||
method: "POST",
|
||||
path: "/api/plugins/org-generic-app",
|
||||
actionsAllowPostURL: "",
|
||||
addHeader: false,
|
||||
code: http.StatusOK,
|
||||
},
|
||||
{
|
||||
name: "POST with action header, no paths defined",
|
||||
method: "POST",
|
||||
path: "/api/plugins/org-generic-app",
|
||||
actionsAllowPostURL: "",
|
||||
addHeader: true,
|
||||
code: http.StatusMethodNotAllowed,
|
||||
},
|
||||
{
|
||||
name: "POST action with allowed path",
|
||||
method: "POST",
|
||||
path: "/api/plugins/org-generic-app",
|
||||
actionsAllowPostURL: "/api/plugins/*",
|
||||
addHeader: true,
|
||||
code: http.StatusOK,
|
||||
},
|
||||
{
|
||||
name: "POST action with invalid path",
|
||||
method: "POST",
|
||||
path: "/api/notplugins/org-generic-app",
|
||||
actionsAllowPostURL: "/api/plugins/*",
|
||||
addHeader: true,
|
||||
code: http.StatusMethodNotAllowed,
|
||||
},
|
||||
{
|
||||
name: "PUT action with valid path with header",
|
||||
method: "PUT",
|
||||
path: "/api/plugins/org-generic-app",
|
||||
actionsAllowPostURL: "/api/plugins/*",
|
||||
addHeader: true,
|
||||
code: http.StatusOK,
|
||||
},
|
||||
{
|
||||
name: "PUT action with invalid path",
|
||||
method: "PUT",
|
||||
path: "/api/notplugins/org-generic-app",
|
||||
actionsAllowPostURL: "/api/plugins/*",
|
||||
addHeader: true,
|
||||
code: http.StatusMethodNotAllowed,
|
||||
},
|
||||
{
|
||||
name: "PUT action with valid path without header",
|
||||
method: "PUT",
|
||||
path: "/api/plugins/org-generic-app",
|
||||
actionsAllowPostURL: "/api/plugins/*",
|
||||
addHeader: false,
|
||||
code: http.StatusOK,
|
||||
},
|
||||
{
|
||||
name: "PUT action with invalid path without header",
|
||||
method: "PUT",
|
||||
path: "/api/notplugins/org-generic-app",
|
||||
actionsAllowPostURL: "/api/plugins/*",
|
||||
addHeader: false,
|
||||
code: http.StatusOK,
|
||||
},
|
||||
{
|
||||
name: "CONNECT unknown verb with header",
|
||||
method: "CONNECT",
|
||||
path: "/api/notplugins/org-generic-app",
|
||||
actionsAllowPostURL: "/api/plugins/*",
|
||||
addHeader: true,
|
||||
code: http.StatusMethodNotAllowed,
|
||||
},
|
||||
{
|
||||
name: "CONNECT unknown verb without header",
|
||||
method: "CONNECT",
|
||||
path: "/api/notplugins/org-generic-app",
|
||||
actionsAllowPostURL: "/api/plugins/*",
|
||||
addHeader: false,
|
||||
code: http.StatusNotFound,
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
middlewareScenario(t, tt.name, func(t *testing.T, sc *scenarioContext) {
|
||||
switch tt.method {
|
||||
case "DELETE":
|
||||
sc.m.Delete(tt.path, sc.defaultHandler)
|
||||
case "GET":
|
||||
sc.m.Get(tt.path, sc.defaultHandler)
|
||||
case "HEAD":
|
||||
sc.m.Head(tt.path, sc.defaultHandler)
|
||||
case "OPTIONS":
|
||||
sc.m.Options(tt.path, sc.defaultHandler)
|
||||
case "PATCH":
|
||||
sc.m.Patch(tt.path, sc.defaultHandler)
|
||||
case "POST":
|
||||
sc.m.Post(tt.path, sc.defaultHandler)
|
||||
case "PUT":
|
||||
sc.m.Put(tt.path, sc.defaultHandler)
|
||||
default:
|
||||
// anything else is an error
|
||||
anError := fmt.Errorf("unknown verb: %s", tt.method)
|
||||
if assert.Errorf(t, anError, "unknown verb: %s", tt.method) {
|
||||
assert.Contains(t, anError.Error(), "unknown verb")
|
||||
}
|
||||
}
|
||||
sc.fakeReq(tt.method, tt.path)
|
||||
if tt.addHeader {
|
||||
sc.req.Header.Add("X-Grafana-Action", "1")
|
||||
}
|
||||
sc.exec()
|
||||
resp := sc.resp.Result()
|
||||
t.Cleanup(func() {
|
||||
err := resp.Body.Close()
|
||||
assert.NoError(t, err)
|
||||
})
|
||||
// nolint:bodyclose
|
||||
assert.Equal(t, tt.code, sc.resp.Result().StatusCode)
|
||||
}, func(cfg *setting.Cfg) {
|
||||
cfg.ActionsAllowPostURL = tt.actionsAllowPostURL
|
||||
})
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestMatchesAllowedPath(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
aPath string
|
||||
allowList string
|
||||
matches bool
|
||||
}{
|
||||
{
|
||||
name: "single url with match",
|
||||
allowList: "/api/plugins/*",
|
||||
aPath: "/api/plugins/my-plugin",
|
||||
matches: true,
|
||||
},
|
||||
{
|
||||
name: "single url no match",
|
||||
allowList: "/api/plugins/*",
|
||||
aPath: "/api/plugin/my-plugin",
|
||||
matches: false,
|
||||
},
|
||||
{
|
||||
name: "multiple urls with match",
|
||||
allowList: "/api/plugins/*, /api/other/**",
|
||||
aPath: "/api/other/my-plugin",
|
||||
matches: true,
|
||||
},
|
||||
{
|
||||
name: "multiple urls no match",
|
||||
allowList: "/api/plugins/*, /api/other/**",
|
||||
aPath: "/api/misc/my-plugin",
|
||||
matches: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range tests {
|
||||
tc := tc
|
||||
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
allGlobs, err := cacheGlobs(tc.allowList)
|
||||
matched := matchesAllowedPath(allGlobs, tc.aPath)
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, matched, tc.matches)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestCacheGlobs(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
allowList string
|
||||
expectedLength int
|
||||
}{
|
||||
{
|
||||
name: "single url",
|
||||
allowList: "/api/plugins",
|
||||
expectedLength: 1,
|
||||
},
|
||||
{
|
||||
name: "multiple urls",
|
||||
allowList: "/api/plugins, /api/other/**",
|
||||
expectedLength: 2,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range tests {
|
||||
tc := tc
|
||||
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
cache, err := cacheGlobs(tc.allowList)
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, len(*cache), tc.expectedLength)
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user