Security: Fixes for CVE-2022-31107 and CVE-2022-31097 (#52236)

* Fix XSS in runbook URL (#383) * "Release: Updated versions in package to 8.3.8" (#385) * Fix: Choose Lookup params per auth module v8.3.x (#401) * Fix: Choose Lookup params per auth module Co-authored-by: Karl Persson <kalle.persson@grafana.com> Fix: Prefer pointer to struct in lookup Co-authored-by: Karl Persson <kalle.persson@grafana.com> Fix: user email for ldap Co-authored-by: Karl Persson <kalle.persson@grafana.com> Fix: Use only login for lookup in LDAP Co-authored-by: Karl Persson <kalle.persson@grafana.com> Fix: use user email for ldap Co-authored-by: Karl Persson <kalle.persson@grafana.com> fix remaining test fix nit picks * update lock * fix integration tests * [v8.3.x] Merge 'release-8.3.9` branch (#404) * Fix: Choose Lookup params per auth module Co-authored-by: Karl Persson <kalle.persson@grafana.com> Fix: Prefer pointer to struct in lookup Co-authored-by: Karl Persson <kalle.persson@grafana.com> Fix: user email for ldap Co-authored-by: Karl Persson <kalle.persson@grafana.com> Fix: Use only login for lookup in LDAP Co-authored-by: Karl Persson <kalle.persson@grafana.com> Fix: use user email for ldap Co-authored-by: Karl Persson <kalle.persson@grafana.com> fix remaining test fix nit picks (cherry picked from commit 09ca54d4665583b5648fea5726a19e0b58df4ec0) * fix integration tests (cherry picked from commit 16c3077bb39f41acacbef4b13c65c7c907c54de7) * Release: Bump version to 8.3.9 (#403) * Change bump-version.yml * "Release: Updated versions in package to 8.3.9" Co-authored-by: dsotirakis <dimitrios.sotirakis@grafana.com> * Update yarn.lock Co-authored-by: jguer <joao.guerreiro@grafana.com> Co-authored-by: Grot (@grafanabot) <43478413+grafanabot@users.noreply.github.com> * "Release: Updated versions in package to 8.3.10" (#409) * Update grabpl * update grabpl * CI: Update `grabpl` version - remove `--no-pull-enterprise` flag (#47013) * Update grabpl version * Sign drone * Remove --no-pull-enterprise flag * Sign drone * Update grabpl * update drone, cherry-pick some ci updates * update PR pipeline Co-authored-by: George Robinson <george.robinson@grafana.com> Co-authored-by: Grot (@grafanabot) <43478413+grafanabot@users.noreply.github.com> Co-authored-by: Jguer <joao.guerreiro@grafana.com> Co-authored-by: Dimitris Sotirakis <dimitrios.sotirakis@grafana.com> Co-authored-by: Kevin Minehart <kmineh0151@gmail.com> Co-authored-by: Dimitris Sotirakis <sotirakis.dim@gmail.com>
grafana · Jul 15, 2022 · b8eb142 · b8eb142
1 parent 7c2cb07
commit b8eb142
Show file tree

Hide file tree

Showing 26 changed files with 213 additions and 188 deletions.
diff --git a/.drone.yml b/.drone.yml
diff --git a/.github/workflows/bump-version.yml b/.github/workflows/bump-version.yml
@@ -35,12 +35,6 @@ jobs:
           echo "::set-output name=branch_name::v${{steps.regex-match.outputs.group1}}"
           echo "::set-output name=branch_exist::$(git ls-remote --heads https://github.com/grafana/grafana.git v${{ steps.regex-match.outputs.group1 }}.x | wc -l)"
 
-      - name: Check input version is aligned with branch(not main)
-        if: steps.intermedia.outputs.branch_exist != '0' && !contains(steps.intermedia.outputs.short_ref, steps.intermedia.outputs.branch_name)
-        run: |
-          echo " You need to run the workflow on branch v${{steps.regex-match.outputs.group1}}.x
-          exit 1
-
       - name: Check input version is aligned with branch(main)
         if: steps.intermedia.outputs.branch_exist == '0' && !contains(steps.intermedia.outputs.short_ref, 'main')
         run: |

diff --git a/lerna.json b/lerna.json
@@ -4,5 +4,5 @@
   "packages": [
     "packages/*"
   ],
-  "version": "8.3.7"
+  "version": "8.3.10"
 }
diff --git a/package.json b/package.json
@@ -3,7 +3,7 @@
   "license": "AGPL-3.0-only",
   "private": true,
   "name": "grafana",
-  "version": "8.3.7",
+  "version": "8.3.10",
   "repository": "github:grafana/grafana",
   "scripts": {
     "api-tests": "jest --notify --watch --config=devenv/e2e-api-tests/jest.js",

diff --git a/packages/grafana-data/package.json b/packages/grafana-data/package.json
@@ -2,7 +2,7 @@
   "author": "Grafana Labs",
   "license": "Apache-2.0",
   "name": "@grafana/data",
-  "version": "8.3.7",
+  "version": "8.3.10",
   "description": "Grafana Data Library",
   "keywords": [
     "typescript"
@@ -22,7 +22,7 @@
   },
   "dependencies": {
     "@braintree/sanitize-url": "5.0.2",
-    "@grafana/schema": "8.3.7",
+    "@grafana/schema": "8.3.10",
     "@types/d3-interpolate": "^1.4.0",
     "d3-interpolate": "1.4.0",
     "date-fns": "2.21.3",

diff --git a/packages/grafana-e2e-selectors/package.json b/packages/grafana-e2e-selectors/package.json
@@ -2,7 +2,7 @@
   "author": "Grafana Labs",
   "license": "Apache-2.0",
   "name": "@grafana/e2e-selectors",
-  "version": "8.3.7",
+  "version": "8.3.10",
   "description": "Grafana End-to-End Test Selectors Library",
   "keywords": [
     "cli",

diff --git a/packages/grafana-e2e/package.json b/packages/grafana-e2e/package.json
@@ -2,7 +2,7 @@
   "author": "Grafana Labs",
   "license": "Apache-2.0",
   "name": "@grafana/e2e",
-  "version": "8.3.7",
+  "version": "8.3.10",
   "description": "Grafana End-to-End Test Library",
   "keywords": [
     "cli",
@@ -49,7 +49,7 @@
     "@babel/core": "7.14.6",
     "@babel/preset-env": "7.14.7",
     "@cypress/webpack-preprocessor": "5.9.1",
-    "@grafana/e2e-selectors": "8.3.7",
+    "@grafana/e2e-selectors": "8.3.10",
     "@grafana/tsconfig": "^1.0.0-rc1",
     "@mochajs/json-file-reporter": "^1.2.0",
     "babel-loader": "8.2.2",

diff --git a/packages/grafana-runtime/package.json b/packages/grafana-runtime/package.json
@@ -2,7 +2,7 @@
   "author": "Grafana Labs",
   "license": "Apache-2.0",
   "name": "@grafana/runtime",
-  "version": "8.3.7",
+  "version": "8.3.10",
   "description": "Grafana Runtime Library",
   "keywords": [
     "grafana",
@@ -23,9 +23,9 @@
   },
   "dependencies": {
     "@emotion/css": "11.1.3",
-    "@grafana/data": "8.3.7",
-    "@grafana/e2e-selectors": "8.3.7",
-    "@grafana/ui": "8.3.7",
+    "@grafana/data": "8.3.10",
+    "@grafana/e2e-selectors": "8.3.10",
+    "@grafana/ui": "8.3.10",
     "@sentry/browser": "5.25.0",
     "history": "4.10.1",
     "lodash": "4.17.21",

diff --git a/packages/grafana-schema/package.json b/packages/grafana-schema/package.json
@@ -2,7 +2,7 @@
   "author": "Grafana Labs",
   "license": "Apache-2.0",
   "name": "@grafana/schema",
-  "version": "8.3.7",
+  "version": "8.3.10",
   "description": "Grafana Schema Library",
   "keywords": [
     "typescript"

diff --git a/packages/grafana-toolkit/package.json b/packages/grafana-toolkit/package.json
@@ -2,7 +2,7 @@
   "author": "Grafana Labs",
   "license": "Apache-2.0",
   "name": "@grafana/toolkit",
-  "version": "8.3.7",
+  "version": "8.3.10",
   "description": "Grafana Toolkit",
   "keywords": [
     "grafana",
@@ -28,10 +28,10 @@
   "dependencies": {
     "@babel/core": "7.13.14",
     "@babel/preset-env": "7.13.12",
-    "@grafana/data": "8.3.7",
+    "@grafana/data": "8.3.10",
     "@grafana/eslint-config": "2.5.1",
     "@grafana/tsconfig": "^1.0.0-rc1",
-    "@grafana/ui": "8.3.7",
+    "@grafana/ui": "8.3.10",
     "@jest/core": "26.6.3",
     "@rushstack/eslint-patch": "1.0.6",
     "@types/command-exists": "^1.2.0",

diff --git a/packages/grafana-ui/package.json b/packages/grafana-ui/package.json
@@ -2,7 +2,7 @@
   "author": "Grafana Labs",
   "license": "Apache-2.0",
   "name": "@grafana/ui",
-  "version": "8.3.7",
+  "version": "8.3.10",
   "description": "Grafana Components Library",
   "keywords": [
     "grafana",
@@ -33,9 +33,9 @@
     "@emotion/css": "11.1.3",
     "@emotion/react": "11.1.5",
     "@grafana/aws-sdk": "0.0.3",
-    "@grafana/data": "8.3.7",
-    "@grafana/e2e-selectors": "8.3.7",
-    "@grafana/schema": "8.3.7",
+    "@grafana/data": "8.3.10",
+    "@grafana/e2e-selectors": "8.3.10",
+    "@grafana/schema": "8.3.10",
     "@grafana/slate-react": "0.22.10-grafana",
     "@monaco-editor/react": "4.2.2",
     "@popperjs/core": "2.5.4",

diff --git a/packages/jaeger-ui-components/package.json b/packages/jaeger-ui-components/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@jaegertracing/jaeger-ui-components",
-  "version": "8.3.7",
+  "version": "8.3.10",
   "main": "src/index.ts",
   "types": "src/index.ts",
   "license": "Apache-2.0",
@@ -30,8 +30,8 @@
   "dependencies": {
     "@emotion/css": "11.1.3",
     "@emotion/react": "11.1.5",
-    "@grafana/data": "8.3.7",
-    "@grafana/ui": "8.3.7",
+    "@grafana/data": "8.3.10",
+    "@grafana/ui": "8.3.10",
     "chance": "^1.0.10",
     "classnames": "^2.2.5",
     "combokeys": "^3.0.0",

diff --git a/pkg/api/ldap_debug.go b/pkg/api/ldap_debug.go
@@ -217,6 +217,11 @@ func (hs *HTTPServer) PostSyncUserWithLDAP(c *models.ReqContext) response.Respon
 		ReqContext:    c,
 		ExternalUser:  user,
 		SignupAllowed: hs.Cfg.LDAPAllowSignup,
+		UserLookupParams: models.UserLookupParams{
+			UserID: &query.Result.Id, // Upsert by ID only
+			Email:  nil,
+			Login:  nil,
+		},
 	}
 
 	err = bus.Dispatch(upsertCmd)

diff --git a/pkg/api/login_oauth.go b/pkg/api/login_oauth.go
@@ -310,6 +310,11 @@ func syncUser(
 		ReqContext:    ctx,
 		ExternalUser:  extUser,
 		SignupAllowed: connect.IsSignupAllowed(),
+		UserLookupParams: models.UserLookupParams{
+			Email:  &extUser.Email,
+			UserID: nil,
+			Login:  nil,
+		},
 	}
 	if err := bus.Dispatch(cmd); err != nil {
 		return nil, err

diff --git a/pkg/login/ldap_login.go b/pkg/login/ldap_login.go
@@ -57,9 +57,13 @@ var loginUsingLDAP = func(ctx context.Context, query *models.LoginUserQuery) (bo
 		ReqContext:    query.ReqContext,
 		ExternalUser:  externalUser,
 		SignupAllowed: setting.LDAPAllowSignup,
+		UserLookupParams: models.UserLookupParams{
+			Login:  &externalUser.Login,
+			Email:  &externalUser.Email,
+			UserID: nil,
+		},
 	}
-	err = bus.DispatchCtx(ctx, upsert)
-	if err != nil {
+	if err = bus.DispatchCtx(ctx, upsert); err != nil {
 		return true, err
 	}
 	query.User = upsert.Result

diff --git a/pkg/models/user_auth.go b/pkg/models/user_auth.go
@@ -54,11 +54,11 @@ type RequestURIKey struct{}
 // COMMANDS
 
 type UpsertUserCommand struct {
-	ReqContext    *ReqContext
-	ExternalUser  *ExternalUserInfo
+	ReqContext   *ReqContext
+	ExternalUser *ExternalUserInfo
+	UserLookupParams
+	Result        *User
 	SignupAllowed bool
-
-	Result *User
 }
 
 type SetAuthInfoCommand struct {
@@ -95,9 +95,14 @@ type LoginUserQuery struct {
 type GetUserByAuthInfoQuery struct {
 	AuthModule string
 	AuthId     string
-	UserId     int64
-	Email      string
-	Login      string
+	UserLookupParams
+}
+
+type UserLookupParams struct {
+	// Describes lookup order as well
+	UserID *int64  // if set, will try to find the user by id
+	Email  *string // if set, will try to find the user by email
+	Login  *string // if set, will try to find the user by login
 }
 
 type GetExternalUserInfoByLoginQuery struct {

diff --git a/pkg/services/contexthandler/authproxy/authproxy.go b/pkg/services/contexthandler/authproxy/authproxy.go
@@ -247,6 +247,11 @@ func (auth *AuthProxy) LoginViaLDAP() (int64, error) {
 		ReqContext:    auth.ctx,
 		SignupAllowed: auth.cfg.LDAPAllowSignup,
 		ExternalUser:  extUser,
+		UserLookupParams: models.UserLookupParams{
+			Login:  &extUser.Login,
+			Email:  &extUser.Email,
+			UserID: nil,
+		},
 	}
 	if err := bus.Dispatch(upsert); err != nil {
 		return 0, err
@@ -303,6 +308,11 @@ func (auth *AuthProxy) LoginViaHeader() (int64, error) {
 		ReqContext:    auth.ctx,
 		SignupAllowed: auth.cfg.AuthProxyAutoSignUp,
 		ExternalUser:  extUser,
+		UserLookupParams: models.UserLookupParams{
+			UserID: nil,
+			Login:  &extUser.Login,
+			Email:  &extUser.Email,
+		},
 	}
 
 	err := bus.Dispatch(upsert)

diff --git a/pkg/services/login/authinfoservice/service.go b/pkg/services/login/authinfoservice/service.go
@@ -85,11 +85,12 @@ func (s *Implementation) LookupAndFix(query *models.GetUserByAuthInfoQuery) (boo
 			}
 
 			// if user id was specified and doesn't match the user_auth entry, remove it
-			if query.UserId != 0 && query.UserId != authQuery.Result.UserId {
-				err := s.DeleteAuthInfo(&models.DeleteAuthInfoCommand{
+			if query.UserLookupParams.UserID != nil &&
+				*query.UserLookupParams.UserID != 0 &&
+				*query.UserLookupParams.UserID != authQuery.Result.UserId {
+				if err := s.DeleteAuthInfo(&models.DeleteAuthInfoCommand{
 					UserAuth: authQuery.Result,
-				})
-				if err != nil {
+				}); err != nil {
 					s.logger.Error("Error removing user_auth entry", "error", err)
 				}
 
@@ -120,42 +121,42 @@ func (s *Implementation) LookupAndFix(query *models.GetUserByAuthInfoQuery) (boo
 	return false, nil, nil, models.ErrUserNotFound
 }
 
-func (s *Implementation) LookupByOneOf(userId int64, email string, login string) (bool, *models.User, error) {
-	foundUser := false
+func (s *Implementation) LookupByOneOf(params *models.UserLookupParams) (*models.User, error) {
 	var user *models.User
 	var err error
+	foundUser := false
 
 	// If not found, try to find the user by id
-	if userId != 0 {
-		foundUser, user, err = s.getUserById(userId)
+	if params.UserID != nil && *params.UserID != 0 {
+		foundUser, user, err = s.getUserById(*params.UserID)
 		if err != nil {
-			return false, nil, err
+			return nil, err
 		}
 	}
 
 	// If not found, try to find the user by email address
-	if !foundUser && email != "" {
-		user = &models.User{Email: email}
+	if !foundUser && params.Email != nil && *params.Email != "" {
+		user = &models.User{Email: *params.Email}
 		foundUser, err = s.getUser(user)
 		if err != nil {
-			return false, nil, err
+			return nil, err
 		}
 	}
 
 	// If not found, try to find the user by login
-	if !foundUser && login != "" {
-		user = &models.User{Login: login}
+	if !foundUser && params.Login != nil && *params.Login != "" {
+		user = &models.User{Login: *params.Login}
 		foundUser, err = s.getUser(user)
 		if err != nil {
-			return false, nil, err
+			return nil, err
 		}
 	}
 
 	if !foundUser {
-		return false, nil, models.ErrUserNotFound
+		return nil, models.ErrUserNotFound
 	}
 
-	return foundUser, user, nil
+	return user, nil
 }
 
 func (s *Implementation) GenericOAuthLookup(authModule string, authId string, userID int64) (*models.UserAuth, error) {
@@ -184,7 +185,7 @@ func (s *Implementation) LookupAndUpdate(query *models.GetUserByAuthInfoQuery) (
 
 	// 2. FindByUserDetails
 	if !foundUser {
-		_, user, err = s.LookupByOneOf(query.UserId, query.Email, query.Login)
+		user, err = s.LookupByOneOf(&query.UserLookupParams)
 		if err != nil {
 			return nil, err
 		}