Auth: Add support for forcing authentication in anonymous mode and modify SignIn to use it instead of redirect (#25567)

* Forbid additional redirect urls

* Optionally force login in anonymous mode

* Update LoginCtrl page to ignore redirect parameter

* Modify SignIn to set forceLogin query instead of redirect

* Pass appUrl to frontend and use URL API for updating url query

* Apply suggestions from code review

Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>

* Fix SignIn test

Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>

packages/grafana-runtime/src/config.ts

@@ -15,6 +15,7 @@ export class GrafanaBootConfig implements GrafanaConfig {
   datasources: { [str: string]: DataSourceInstanceSettings } = {};
   panels: { [key: string]: PanelPluginMeta } = {};
   minRefreshInterval = '';
+  appUrl = '';
   appSubUrl = '';
   windowTitlePrefix = '';
   buildInfo: BuildInfo = {} as BuildInfo;
@@ -66,6 +67,7 @@ export class GrafanaBootConfig implements GrafanaConfig {
       newPanelTitle: 'Panel Title',
       playlist_timespan: '1m',
       unsaved_changes_warning: true,
+      appUrl: '',
       appSubUrl: '',
       buildInfo: {
         version: 'v1.0',

pkg/api/frontendsettings.go

@@ -172,6 +172,7 @@ func (hs *HTTPServer) getFrontendSettingsMap(c *models.ReqContext) (map[string]i
 		"datasources":                datasources,
 		"minRefreshInterval":         setting.MinRefreshInterval,
 		"panels":                     panels,
+		"appUrl":                     setting.AppUrl,
 		"appSubUrl":                  setting.AppSubUrl,
 		"allowOrgCreate":             (setting.AllowUserOrgCreate && c.IsSignedIn) || c.IsGrafanaAdmin,
 		"authProxyEnabled":           setting.AuthProxyEnabled,

pkg/api/login.go

@@ -37,11 +37,25 @@ func (hs *HTTPServer) ValidateRedirectTo(redirectTo string) error {
 	if to.IsAbs() {
 		return login.ErrAbsoluteRedirectTo
 	}
+
+	if to.Host != "" {
+		return login.ErrForbiddenRedirectTo
+	}
+
+	// path should have exactly one leading slash
+	if !strings.HasPrefix(to.Path, "/") {
+		return login.ErrForbiddenRedirectTo
+	}
+	if strings.HasPrefix(to.Path, "//") {
+		return login.ErrForbiddenRedirectTo
+	}
+
 	// when using a subUrl, the redirect_to should start with the subUrl (which contains the leading slash), otherwise the redirect
 	// will send the user to the wrong location
 	if hs.Cfg.AppSubUrl != "" && !strings.HasPrefix(to.Path, hs.Cfg.AppSubUrl+"/") {
 		return login.ErrInvalidRedirectTo
 	}
+
 	return nil
 }
 

pkg/api/login_test.go

@@ -207,6 +207,48 @@ func TestLoginViewRedirect(t *testing.T) {
 			appURL:      "http://localhost:3000/",
 			status:      302,
 		},
+		{
+			desc:        "non-Grafana URL without scheme",
+			url:         "example.com",
+			redirectURL: "/",
+			appURL:      "http://localhost:3000/",
+			status:      302,
+		},
+		{
+			desc:        "non-Grafana URL without scheme",
+			url:         "www.example.com",
+			redirectURL: "/",
+			appURL:      "http://localhost:3000/",
+			status:      302,
+		},
+		{
+			desc:        "URL path is a host with two leading slashes",
+			url:         "//example.com",
+			redirectURL: "/",
+			appURL:      "http://localhost:3000/",
+			status:      302,
+		},
+		{
+			desc:        "URL path is a host with three leading slashes",
+			url:         "///example.com",
+			redirectURL: "/",
+			appURL:      "http://localhost:3000/",
+			status:      302,
+		},
+		{
+			desc:        "URL path is an IP address with two leading slashes",
+			url:         "//0.0.0.0",
+			redirectURL: "/",
+			appURL:      "http://localhost:3000/",
+			status:      302,
+		},
+		{
+			desc:        "URL path is an IP address with three leading slashes",
+			url:         "///0.0.0.0",
+			redirectURL: "/",
+			appURL:      "http://localhost:3000/",
+			status:      302,
+		},
 	}
 
 	for _, c := range redirectCases {
@@ -232,7 +274,7 @@ func TestLoginViewRedirect(t *testing.T) {
 			if c.status == 302 {
 				location, ok := sc.resp.Header()["Location"]
 				assert.True(t, ok)
-				assert.Equal(t, location[0], c.redirectURL)
+				assert.Equal(t, c.redirectURL, location[0])
 
 				setCookie, ok := sc.resp.Header()["Set-Cookie"]
 				assert.True(t, ok, "Set-Cookie exists")
@@ -333,6 +375,48 @@ func TestLoginPostRedirect(t *testing.T) {
 			appURL: "https://localhost:3000/",
 			err:    login.ErrAbsoluteRedirectTo,
 		},
+		{
+			desc:   "invalid URL",
+			url:    ":foo",
+			appURL: "http://localhost:3000/",
+			err:    login.ErrInvalidRedirectTo,
+		},
+		{
+			desc:   "non-Grafana URL without scheme",
+			url:    "example.com",
+			appURL: "http://localhost:3000/",
+			err:    login.ErrForbiddenRedirectTo,
+		},
+		{
+			desc:   "non-Grafana URL without scheme",
+			url:    "www.example.com",
+			appURL: "http://localhost:3000/",
+			err:    login.ErrForbiddenRedirectTo,
+		},
+		{
+			desc:   "URL path is a host with two leading slashes",
+			url:    "//example.com",
+			appURL: "http://localhost:3000/",
+			err:    login.ErrForbiddenRedirectTo,
+		},
+		{
+			desc:   "URL path is a host with three leading slashes",
+			url:    "///example.com",
+			appURL: "http://localhost:3000/",
+			err:    login.ErrForbiddenRedirectTo,
+		},
+		{
+			desc:   "URL path is an IP address with two leading slashes",
+			url:    "//0.0.0.0",
+			appURL: "http://localhost:3000/",
+			err:    login.ErrForbiddenRedirectTo,
+		},
+		{
+			desc:   "URL path is an IP address with three leading slashes",
+			url:    "///0.0.0.0",
+			appURL: "http://localhost:3000/",
+			err:    login.ErrForbiddenRedirectTo,
+		},
 	}
 
 	for _, c := range redirectCases {

pkg/login/auth.go

@@ -20,6 +20,7 @@ var (
 	ErrUserDisabled          = errors.New("User is disabled")
 	ErrAbsoluteRedirectTo    = errors.New("Absolute urls are not allowed for redirect_to cookie value")
 	ErrInvalidRedirectTo     = errors.New("Invalid redirect_to cookie value")
+	ErrForbiddenRedirectTo   = errors.New("Forbidden redirect_to cookie value")
 )
 
 var loginLogger = log.New("login")

pkg/middleware/auth.go

@@ -51,8 +51,16 @@ func notAuthorized(c *models.ReqContext) {
 	if setting.AppSubUrl != "" && !strings.HasPrefix(redirectTo, setting.AppSubUrl) {
 		redirectTo = setting.AppSubUrl + c.Req.RequestURI
 	}
-	WriteCookie(c.Resp, "redirect_to", url.QueryEscape(redirectTo), 0, newCookieOptions)
 
+	// remove forceLogin query param if it exists
+	if parsed, err := url.ParseRequestURI(redirectTo); err == nil {
+		params := parsed.Query()
+		params.Del("forceLogin")
+		parsed.RawQuery = params.Encode()
+		WriteCookie(c.Resp, "redirect_to", url.QueryEscape(parsed.String()), 0, newCookieOptions)
+	} else {
+		c.Logger.Debug("Failed parsing request URI; redirect cookie will not be set", "redirectTo", redirectTo, "error", err)
+	}
 	c.Redirect(setting.AppSubUrl + "/login")
 }
 
@@ -79,7 +87,9 @@ func RoleAuth(roles ...models.RoleType) macaron.Handler {
 
 func Auth(options *AuthOptions) macaron.Handler {
 	return func(c *models.ReqContext) {
-		if !c.IsSignedIn && options.ReqSignedIn && !c.AllowAnonymous {
+		forceLogin := c.AllowAnonymous && c.QueryBool("forceLogin")
+		requireLogin := !c.AllowAnonymous || forceLogin
+		if !c.IsSignedIn && options.ReqSignedIn && requireLogin {
 			notAuthorized(c)
 			return
 		}

public/app/core/components/Login/LoginCtrl.tsx

@@ -101,15 +101,8 @@ export class LoginCtrl extends PureComponent<Props, State> {
   };
 
   toGrafana = () => {
-    const params = this.props.routeParams;
     // Use window.location.href to force page reload
-    if (params.redirect && params.redirect[0] === '/') {
-      if (config.appSubUrl !== '' && !params.redirect.startsWith(config.appSubUrl)) {
-        window.location.href = config.appSubUrl + params.redirect;
-      } else {
-        window.location.href = params.redirect;
-      }
-    } else if (this.result.redirectUrl) {
+    if (this.result.redirectUrl) {
       if (config.appSubUrl !== '' && !this.result.redirectUrl.startsWith(config.appSubUrl)) {
         window.location.href = config.appSubUrl + this.result.redirectUrl;
       } else {

public/app/core/components/sidemenu/SignIn.test.tsx

@@ -2,6 +2,10 @@ import React from 'react';
 import { shallow } from 'enzyme';
 import { SignIn } from './SignIn';
 
+jest.mock('../../config', () => ({
+  appUrl: 'http://localhost:3000/',
+}));
+
 describe('Render', () => {
   it('should render component', () => {
     const wrapper = shallow(<SignIn url="/" />);

public/app/core/components/sidemenu/SignIn.tsx

@@ -1,18 +1,26 @@
 import React, { FC } from 'react';
+import config from 'app/core/config';
 import { connectWithStore } from 'app/core/utils/connectWithReduxStore';
 import { StoreState } from 'app/types';
 import { Icon } from '@grafana/ui';
 
+const getForcedLoginUrl = (url: string) => {
+  const urlObj = new URL(url, config.appUrl);
+  let params = urlObj.searchParams;
+  params.set('forceLogin', 'true');
+  return urlObj.toString();
+};
+
 export const SignIn: FC<any> = ({ url }) => {
-  const loginUrl = `login?redirect=${encodeURIComponent(url)}`;
+  const forcedLoginUrl = getForcedLoginUrl(url);
   return (
     <div className="sidemenu-item">
-      <a href={loginUrl} className="sidemenu-link" target="_self">
+      <a href={forcedLoginUrl} className="sidemenu-link" target="_self">
         <span className="icon-circle sidemenu-icon">
           <Icon name="sign-in-alt" size="xl" />
         </span>
       </a>
-      <a href={loginUrl} target="_self">
+      <a href={forcedLoginUrl} target="_self">
         <ul className="dropdown-menu dropdown-menu--sidemenu" role="menu">
           <li className="side-menu-header">
             <span className="sidemenu-item-text">Sign In</span>

public/app/core/components/sidemenu/__snapshots__/SignIn.test.tsx.snap

@@ -6,7 +6,7 @@ exports[`Render should render component 1`] = `
 >
   <a
     className="sidemenu-link"
-    href="login?redirect=%2F"
+    href="http://localhost:3000/?forceLogin=true"
     target="_self"
   >
     <span
@@ -19,7 +19,7 @@ exports[`Render should render component 1`] = `
     </span>
   </a>
   <a
-    href="login?redirect=%2F"
+    href="http://localhost:3000/?forceLogin=true"
     target="_self"
   >
     <ul