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>

conf/defaults.ini

@@ -406,6 +406,9 @@ csrf_always_check = false
 # Comma-separated list of plugins ids that won't be loaded inside the frontend sandbox
 disable_frontend_sandbox_for_plugins = grafana-incident-app
 
+# Comma-separated list of paths for POST/PUT URL in actions. Empty will allow anything that is not on the same origin
+actions_allow_post_url =
+
 [security.encryption]
 # Defines the time-to-live (TTL) for decrypted data encryption keys stored in memory (cache).
 # Please note that small values may cause performance issues due to a high frequency decryption operations.

conf/sample.ini

@@ -411,6 +411,9 @@
 # Comma-separated list of plugins ids that won't be loaded inside the frontend sandbox
 ;disable_frontend_sandbox_for_plugins =
 
+# Comma-separated list of paths for POST/PUT URL in actions. Empty will allow anything that is not on the same origin
+;actions_allow_post_url =
+
 [security.encryption]
 # Defines the time-to-live (TTL) for decrypted data encryption keys stored in memory (cache).
 # Please note that small values may cause performance issues due to a high frequency decryption operations.

pkg/api/http_server.go

@@ -641,6 +641,8 @@ func (hs *HTTPServer) addMiddlewaresAndStaticRoutes() {
 	if hs.Cfg.EnforceDomain {
 		m.Use(middleware.ValidateHostHeader(hs.Cfg))
 	}
+	// handle action urls
+	m.UseMiddleware(middleware.ValidateActionUrl(hs.Cfg, hs.log))
 
 	m.Use(middleware.HandleNoCacheHeaders)
 

pkg/middleware/middleware_test.go

@@ -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)

pkg/middleware/validate_action_url.go

@@ -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
+}

pkg/middleware/validate_action_url_test.go

@@ -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)
+		})
+	}
+}

pkg/setting/setting.go

@@ -180,6 +180,7 @@ type Cfg struct {
 	DisableFrontendSandboxForPlugins []string
 	DisableGravatar                  bool
 	DataProxyWhiteList               map[string]bool
+	ActionsAllowPostURL              string
 
 	TempDataLifetime time.Duration
 
@@ -1538,6 +1539,7 @@ func readSecuritySettings(iniFile *ini.File, cfg *Cfg) error {
 
 	cfg.ContentTypeProtectionHeader = security.Key("x_content_type_options").MustBool(true)
 	cfg.XSSProtectionHeader = security.Key("x_xss_protection").MustBool(true)
+	cfg.ActionsAllowPostURL = security.Key("actions_allow_post_url").MustString("")
 	cfg.StrictTransportSecurity = security.Key("strict_transport_security").MustBool(false)
 	cfg.StrictTransportSecurityMaxAge = security.Key("strict_transport_security_max_age_seconds").MustInt(86400)
 	cfg.StrictTransportSecurityPreload = security.Key("strict_transport_security_preload").MustBool(false)

public/app/plugins/panel/canvas/editor/element/utils.ts

@@ -1,6 +1,7 @@
-import { AppEvents } from '@grafana/data';
+import { AppEvents, textUtil } from '@grafana/data';
 import { BackendSrvRequest, getBackendSrv, getTemplateSrv } from '@grafana/runtime';
 import { appEvents } from 'app/core/core';
+import { createAbsoluteUrl, RelativeUrl } from 'app/features/alerting/unified/utils/url';
 import { getDashboardSrv } from 'app/features/dashboard/services/DashboardSrv';
 
 import { HttpRequestMethod } from '../../panelcfg.gen';
@@ -10,30 +11,26 @@ import { APIEditorConfig } from './APIEditor';
 type IsLoadingCallback = (loading: boolean) => void;
 
 export const callApi = (api: APIEditorConfig, updateLoadingStateCallback?: IsLoadingCallback) => {
-  if (api && api.endpoint) {
-    // If API endpoint origin matches Grafana origin, don't call it.
-    const interpolatedUrl = interpolateVariables(api.endpoint);
-    if (requestMatchesGrafanaOrigin(interpolatedUrl)) {
-      appEvents.emit(AppEvents.alertError, ['Cannot call API at Grafana origin.']);
-      updateLoadingStateCallback && updateLoadingStateCallback(false);
-      return;
-    }
-    const request = getRequest(api);
-
-    getBackendSrv()
-      .fetch(request)
-      .subscribe({
-        error: (error) => {
-          appEvents.emit(AppEvents.alertError, ['An error has occurred. Check console output for more details.']);
-          console.error('API call error: ', error);
-          updateLoadingStateCallback && updateLoadingStateCallback(false);
-        },
-        complete: () => {
-          appEvents.emit(AppEvents.alertSuccess, ['API call was successful']);
-          updateLoadingStateCallback && updateLoadingStateCallback(false);
-        },
-      });
+  if (!api.endpoint) {
+    appEvents.emit(AppEvents.alertError, ['API endpoint is not defined.']);
+    return;
   }
+
+  const request = getRequest(api);
+
+  getBackendSrv()
+    .fetch(request)
+    .subscribe({
+      error: (error) => {
+        appEvents.emit(AppEvents.alertError, ['An error has occurred. Check console output for more details.']);
+        console.error('API call error: ', error);
+        updateLoadingStateCallback && updateLoadingStateCallback(false);
+      },
+      complete: () => {
+        appEvents.emit(AppEvents.alertSuccess, ['API call was successful']);
+        updateLoadingStateCallback && updateLoadingStateCallback(false);
+      },
+    });
 };
 
 export const interpolateVariables = (text: string) => {
@@ -42,9 +39,10 @@ export const interpolateVariables = (text: string) => {
 };
 
 export const getRequest = (api: APIEditorConfig) => {
-  const requestHeaders: HeadersInit = [];
+  const endpoint = getEndpoint(interpolateVariables(api.endpoint));
+  const url = new URL(endpoint);
 
-  const url = new URL(interpolateVariables(api.endpoint!));
+  const requestHeaders: Record<string, string> = {};
 
   let request: BackendSrvRequest = {
     url: url.toString(),
@@ -54,23 +52,24 @@ export const getRequest = (api: APIEditorConfig) => {
   };
 
   if (api.headerParams) {
-    api.headerParams.forEach((param) => {
-      requestHeaders.push([interpolateVariables(param[0]), interpolateVariables(param[1])]);
+    api.headerParams.forEach(([name, value]) => {
+      requestHeaders[interpolateVariables(name)] = interpolateVariables(value);
     });
   }
 
   if (api.queryParams) {
-    api.queryParams?.forEach((param) => {
-      url.searchParams.append(interpolateVariables(param[0]), interpolateVariables(param[1]));
+    api.queryParams?.forEach(([name, value]) => {
+      url.searchParams.append(interpolateVariables(name), interpolateVariables(value));
     });
 
     request.url = url.toString();
   }
 
   if (api.method === HttpRequestMethod.POST) {
-    requestHeaders.push(['Content-Type', api.contentType!]);
+    requestHeaders['Content-Type'] = api.contentType!;
   }
 
+  requestHeaders['X-Grafana-Action'] = '1';
   request.headers = requestHeaders;
 
   return request;
@@ -85,8 +84,13 @@ const getData = (api: APIEditorConfig) => {
   return data;
 };
 
-const requestMatchesGrafanaOrigin = (requestEndpoint: string) => {
-  const requestURL = new URL(requestEndpoint);
-  const grafanaURL = new URL(window.location.origin);
-  return requestURL.origin === grafanaURL.origin;
+const getEndpoint = (endpoint: string) => {
+  const isRelativeUrl = endpoint.startsWith('/');
+  if (isRelativeUrl) {
+    // eslint-disable-next-line @typescript-eslint/consistent-type-assertions
+    const sanitizedRelativeURL = textUtil.sanitizeUrl(endpoint) as RelativeUrl;
+    endpoint = createAbsoluteUrl(sanitizedRelativeURL, []);
+  }
+
+  return endpoint;
 };