From 4665dc253f15aac59643c40428504664ee036d90 Mon Sep 17 00:00:00 2001 From: Kevin Minehart Date: Thu, 14 Jul 2022 14:56:53 -0500 Subject: [PATCH] Security: Fixes for CVE-2022-31107 and CVE-2022-31097 (#52279) * WIP: Fix XSS in runbook URL (#378) (cherry picked from commit f4a8d96a4e1259ea25d9cc702a652f1b819db236) (cherry picked from commit 337c08507b2b1c78ea470192d34cf611fae4b5da) (cherry picked from commit 54b36a07406ed4e26ff8e161e50eda5401f504da) * Update grabpl version (cherry picked from commit b253e87d730f7b8aabdd0b328c5e7a82547c43b3) (cherry picked from commit 080d3e46f3fcd61555795b9fe8fd6ee2492b422a) * Fix: Choose Lookup params per auth module Co-authored-by: Karl Persson Fix: Prefer pointer to struct in lookup Co-authored-by: Karl Persson Fix: user email for ldap Co-authored-by: Karl Persson Fix: Use only login for lookup in LDAP Co-authored-by: Karl Persson Fix: use user email for ldap Co-authored-by: Karl Persson fix remaining test fix nit picks (cherry picked from commit 1eca4aeed878853743cebcf9790b05dd350c4f83) (cherry picked from commit 0777d100e9263d08f51dbac71aee0766c8a85a92) * remove better (broke the pipeline) Co-authored-by: George Robinson Co-authored-by: dsotirakis Co-authored-by: jguer --- .drone.yml | 76 +++++++++---------- lerna.json | 4 +- pkg/api/ldap_debug.go | 5 ++ pkg/api/login_oauth.go | 5 ++ pkg/api/user_test.go | 3 +- pkg/login/ldap_login.go | 8 +- pkg/models/user_auth.go | 19 +++-- pkg/services/contexthandler/auth_jwt.go | 5 ++ .../contexthandler/authproxy/authproxy.go | 10 +++ pkg/services/login/authinfoservice/service.go | 25 +++--- .../login/authinfoservice/user_auth_test.go | 49 ++++++++---- .../login/loginservice/loginservice.go | 8 +- .../login/loginservice/loginservice_test.go | 6 +- pkg/services/login/logintest/logintest.go | 6 +- .../rules/RuleDetailsActionButtons.tsx | 4 +- scripts/drone/pipelines/release.star | 2 +- scripts/drone/steps/lib.star | 2 +- 17 files changed, 148 insertions(+), 89 deletions(-) diff --git a/.drone.yml b/.drone.yml index 95f3fd6c10c..bc8e952744d 100644 --- a/.drone.yml +++ b/.drone.yml @@ -15,7 +15,7 @@ steps: name: identify-runner - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -87,7 +87,7 @@ steps: name: identify-runner - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -175,7 +175,7 @@ steps: name: identify-runner - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -441,7 +441,7 @@ services: steps: - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -511,7 +511,7 @@ services: [] steps: - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -601,7 +601,7 @@ services: [] steps: - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -690,7 +690,7 @@ steps: name: identify-runner - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -758,7 +758,7 @@ steps: name: identify-runner - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -842,7 +842,7 @@ steps: name: identify-runner - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -1275,7 +1275,7 @@ services: steps: - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -1348,7 +1348,7 @@ steps: name: identify-runner - commands: - $$ProgressPreference = "SilentlyContinue" - - Invoke-WebRequest https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/windows/grabpl.exe + - Invoke-WebRequest https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/windows/grabpl.exe -OutFile grabpl.exe image: grafana/ci-wix:0.1.1 name: windows-init @@ -1436,7 +1436,7 @@ services: [] steps: - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -1521,7 +1521,7 @@ steps: name: identify-runner - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -1827,7 +1827,7 @@ steps: name: identify-runner - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -1951,7 +1951,7 @@ services: steps: - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -2028,7 +2028,7 @@ steps: name: identify-runner - commands: - $$ProgressPreference = "SilentlyContinue" - - Invoke-WebRequest https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/windows/grabpl.exe + - Invoke-WebRequest https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/windows/grabpl.exe -OutFile grabpl.exe image: grafana/ci-wix:0.1.1 name: windows-init @@ -2087,7 +2087,7 @@ services: [] steps: - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -2444,7 +2444,7 @@ services: [] steps: - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -2623,7 +2623,7 @@ services: steps: - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -2747,7 +2747,7 @@ steps: name: identify-runner - commands: - $$ProgressPreference = "SilentlyContinue" - - Invoke-WebRequest https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/windows/grabpl.exe + - Invoke-WebRequest https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/windows/grabpl.exe -OutFile grabpl.exe - git clone "https://$$env:GITHUB_TOKEN@github.com/grafana/grafana-enterprise.git" - cd grafana-enterprise @@ -2822,7 +2822,7 @@ services: [] steps: - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -2900,7 +2900,7 @@ services: [] steps: - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -2961,7 +2961,7 @@ services: [] steps: - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -3040,7 +3040,7 @@ services: [] steps: - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -3102,7 +3102,7 @@ services: [] steps: - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -3140,7 +3140,7 @@ services: [] steps: - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -3178,7 +3178,7 @@ services: [] steps: - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -3234,7 +3234,7 @@ services: [] steps: - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -3282,7 +3282,7 @@ services: [] steps: - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -3331,7 +3331,7 @@ steps: name: identify-runner - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -3607,7 +3607,7 @@ steps: name: identify-runner - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -3725,7 +3725,7 @@ services: steps: - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -3796,7 +3796,7 @@ steps: name: identify-runner - commands: - $$ProgressPreference = "SilentlyContinue" - - Invoke-WebRequest https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/windows/grabpl.exe + - Invoke-WebRequest https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/windows/grabpl.exe -OutFile grabpl.exe image: grafana/ci-wix:0.1.1 name: windows-init @@ -3844,7 +3844,7 @@ services: [] steps: - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -4199,7 +4199,7 @@ services: [] steps: - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -4369,7 +4369,7 @@ services: steps: - commands: - mkdir -p bin - - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/grabpl + - curl -fL -o bin/grabpl https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/grabpl - chmod +x bin/grabpl image: byrnedo/alpine-curl:0.1.8 name: grabpl @@ -4484,7 +4484,7 @@ steps: name: identify-runner - commands: - $$ProgressPreference = "SilentlyContinue" - - Invoke-WebRequest https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v2.9.50/windows/grabpl.exe + - Invoke-WebRequest https://grafana-downloads.storage.googleapis.com/grafana-build-pipeline/v9.0.2-security1/windows/grabpl.exe -OutFile grabpl.exe - git clone "https://$$env:GITHUB_TOKEN@github.com/grafana/grafana-enterprise.git" - cd grafana-enterprise @@ -4681,6 +4681,6 @@ kind: secret name: gcp_upload_artifacts_key --- kind: signature -hmac: 3f5060830f3acfffdb22b5565e8270820ebd24f014a851cb35b6d95be0e7977a +hmac: 1921087fa940a78cf26a61a86f784cac7557e71ce09367b1e1e0bb065679cc47 ... diff --git a/lerna.json b/lerna.json index 14a5bc6d5c4..58321a45562 100644 --- a/lerna.json +++ b/lerna.json @@ -1,8 +1,6 @@ { "npmClient": "yarn", "useWorkspaces": true, - "packages": [ - "packages/*" - ], + "packages": ["packages/*"], "version": "9.0.2" } diff --git a/pkg/api/ldap_debug.go b/pkg/api/ldap_debug.go index f62daaa0407..c1184b29d2e 100644 --- a/pkg/api/ldap_debug.go +++ b/pkg/api/ldap_debug.go @@ -220,6 +220,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 = hs.Login.UpsertUser(c.Req.Context(), upsertCmd) diff --git a/pkg/api/login_oauth.go b/pkg/api/login_oauth.go index 4a586c77b6b..b422baf0811 100644 --- a/pkg/api/login_oauth.go +++ b/pkg/api/login_oauth.go @@ -304,6 +304,11 @@ func (hs *HTTPServer) SyncUser( ReqContext: ctx, ExternalUser: extUser, SignupAllowed: connect.IsSignupAllowed(), + UserLookupParams: models.UserLookupParams{ + Email: &extUser.Email, + UserID: nil, + Login: nil, + }, } if err := hs.Login.UpsertUser(ctx.Req.Context(), cmd); err != nil { diff --git a/pkg/api/user_test.go b/pkg/api/user_test.go index 0879f79dd54..3c03574528f 100644 --- a/pkg/api/user_test.go +++ b/pkg/api/user_test.go @@ -75,7 +75,8 @@ func TestUserAPIEndpoint_userLoggedIn(t *testing.T) { } idToken := "testidtoken" token = token.WithExtra(map[string]interface{}{"id_token": idToken}) - query := &models.GetUserByAuthInfoQuery{Login: "loginuser", AuthModule: "test", AuthId: "test"} + login := "loginuser" + query := &models.GetUserByAuthInfoQuery{AuthModule: "test", AuthId: "test", UserLookupParams: models.UserLookupParams{Login: &login}} cmd := &models.UpdateAuthInfoCommand{ UserId: user.Id, AuthId: query.AuthId, diff --git a/pkg/login/ldap_login.go b/pkg/login/ldap_login.go index b52f431cd06..be13d23e166 100644 --- 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, log ReqContext: query.ReqContext, ExternalUser: externalUser, SignupAllowed: setting.LDAPAllowSignup, + UserLookupParams: models.UserLookupParams{ + Login: &externalUser.Login, + Email: &externalUser.Email, + UserID: nil, + }, } - err = loginService.UpsertUser(ctx, upsert) - if err != nil { + if err = loginService.UpsertUser(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 index 221b899c0e1..1dba8ed2705 100644 --- a/pkg/models/user_auth.go +++ b/pkg/models/user_auth.go @@ -55,11 +55,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 { @@ -96,9 +96,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/auth_jwt.go b/pkg/services/contexthandler/auth_jwt.go index cda763f2c37..e61f009d882 100644 --- a/pkg/services/contexthandler/auth_jwt.go +++ b/pkg/services/contexthandler/auth_jwt.go @@ -65,6 +65,11 @@ func (h *ContextHandler) initContextWithJWT(ctx *models.ReqContext, orgId int64) ReqContext: ctx, SignupAllowed: h.Cfg.JWTAuthAutoSignUp, ExternalUser: extUser, + UserLookupParams: models.UserLookupParams{ + UserID: nil, + Login: &query.Login, + Email: &query.Email, + }, } if err := h.loginService.UpsertUser(ctx.Req.Context(), upsert); err != nil { ctx.Logger.Error("Failed to upsert JWT user", "error", err) diff --git a/pkg/services/contexthandler/authproxy/authproxy.go b/pkg/services/contexthandler/authproxy/authproxy.go index ba4a68f3ad6..4587141da7f 100644 --- a/pkg/services/contexthandler/authproxy/authproxy.go +++ b/pkg/services/contexthandler/authproxy/authproxy.go @@ -241,6 +241,11 @@ func (auth *AuthProxy) LoginViaLDAP(reqCtx *models.ReqContext) (int64, error) { ReqContext: reqCtx, SignupAllowed: auth.cfg.LDAPAllowSignup, ExternalUser: extUser, + UserLookupParams: models.UserLookupParams{ + Login: &extUser.Login, + Email: &extUser.Email, + UserID: nil, + }, } if err := auth.loginService.UpsertUser(reqCtx.Req.Context(), upsert); err != nil { return 0, err @@ -298,6 +303,11 @@ func (auth *AuthProxy) loginViaHeader(reqCtx *models.ReqContext) (int64, error) ReqContext: reqCtx, SignupAllowed: auth.cfg.AuthProxyAutoSignUp, ExternalUser: extUser, + UserLookupParams: models.UserLookupParams{ + UserID: nil, + Login: &extUser.Login, + Email: &extUser.Email, + }, } err := auth.loginService.UpsertUser(reqCtx.Req.Context(), upsert) diff --git a/pkg/services/login/authinfoservice/service.go b/pkg/services/login/authinfoservice/service.go index b14b9dfd965..e73832f1083 100644 --- a/pkg/services/login/authinfoservice/service.go +++ b/pkg/services/login/authinfoservice/service.go @@ -43,11 +43,12 @@ func (s *Implementation) LookupAndFix(ctx context.Context, query *models.GetUser } // 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.authInfoStore.DeleteAuthInfo(ctx, &models.DeleteAuthInfoCommand{ + if query.UserLookupParams.UserID != nil && + *query.UserLookupParams.UserID != 0 && + *query.UserLookupParams.UserID != authQuery.Result.UserId { + if err := s.authInfoStore.DeleteAuthInfo(ctx, &models.DeleteAuthInfoCommand{ UserAuth: authQuery.Result, - }) - if err != nil { + }); err != nil { s.logger.Error("Error removing user_auth entry", "error", err) } @@ -77,29 +78,29 @@ func (s *Implementation) LookupAndFix(ctx context.Context, query *models.GetUser return false, nil, nil, models.ErrUserNotFound } -func (s *Implementation) LookupByOneOf(ctx context.Context, userId int64, email string, login string) (*models.User, error) { +func (s *Implementation) LookupByOneOf(ctx context.Context, params *models.UserLookupParams) (*models.User, error) { var user *models.User var err error // If not found, try to find the user by id - if userId != 0 { - user, err = s.authInfoStore.GetUserById(ctx, userId) + if params.UserID != nil && *params.UserID != 0 { + user, err = s.authInfoStore.GetUserById(ctx, *params.UserID) if err != nil && !errors.Is(err, models.ErrUserNotFound) { return nil, err } } // If not found, try to find the user by email address - if user == nil && email != "" { - user, err = s.authInfoStore.GetUserByEmail(ctx, email) + if user == nil && params.Email != nil && *params.Email != "" { + user, err = s.authInfoStore.GetUserByEmail(ctx, *params.Email) if err != nil && !errors.Is(err, models.ErrUserNotFound) { return nil, err } } // If not found, try to find the user by login - if user == nil && login != "" { - user, err = s.authInfoStore.GetUserByLogin(ctx, login) + if user == nil && params.Login != nil && *params.Login != "" { + user, err = s.authInfoStore.GetUserByLogin(ctx, *params.Login) if err != nil && !errors.Is(err, models.ErrUserNotFound) { return nil, err } @@ -138,7 +139,7 @@ func (s *Implementation) LookupAndUpdate(ctx context.Context, query *models.GetU // 2. FindByUserDetails if !foundUser { - user, err = s.LookupByOneOf(ctx, query.UserId, query.Email, query.Login) + user, err = s.LookupByOneOf(ctx, &query.UserLookupParams) if err != nil { return nil, err } diff --git a/pkg/services/login/authinfoservice/user_auth_test.go b/pkg/services/login/authinfoservice/user_auth_test.go index 5189a7d415e..44dd4adfc22 100644 --- a/pkg/services/login/authinfoservice/user_auth_test.go +++ b/pkg/services/login/authinfoservice/user_auth_test.go @@ -42,7 +42,7 @@ func TestUserAuth(t *testing.T) { // By Login login := "loginuser0" - query := &models.GetUserByAuthInfoQuery{Login: login} + query := &models.GetUserByAuthInfoQuery{UserLookupParams: models.UserLookupParams{Login: &login}} user, err := srv.LookupAndUpdate(context.Background(), query) require.Nil(t, err) @@ -51,7 +51,9 @@ func TestUserAuth(t *testing.T) { // By ID id := user.Id - user, err = srv.LookupByOneOf(context.Background(), id, "", "") + user, err = srv.LookupByOneOf(context.Background(), &models.UserLookupParams{ + UserID: &id, + }) require.Nil(t, err) require.Equal(t, user.Id, id) @@ -59,7 +61,9 @@ func TestUserAuth(t *testing.T) { // By Email email := "user1@test.com" - user, err = srv.LookupByOneOf(context.Background(), 0, email, "") + user, err = srv.LookupByOneOf(context.Background(), &models.UserLookupParams{ + Email: &email, + }) require.Nil(t, err) require.Equal(t, user.Email, email) @@ -67,7 +71,9 @@ func TestUserAuth(t *testing.T) { // Don't find nonexistent user email = "nonexistent@test.com" - user, err = srv.LookupByOneOf(context.Background(), 0, email, "") + user, err = srv.LookupByOneOf(context.Background(), &models.UserLookupParams{ + Email: &email, + }) require.Equal(t, models.ErrUserNotFound, err) require.Nil(t, user) @@ -84,7 +90,7 @@ func TestUserAuth(t *testing.T) { // create user_auth entry login := "loginuser0" - query.Login = login + query.UserLookupParams.Login = &login user, err = srv.LookupAndUpdate(context.Background(), query) require.Nil(t, err) @@ -98,9 +104,9 @@ func TestUserAuth(t *testing.T) { require.Equal(t, user.Login, login) // get with non-matching id - id := user.Id + idPlusOne := user.Id + 1 - query.UserId = id + 1 + query.UserLookupParams.UserID = &idPlusOne user, err = srv.LookupAndUpdate(context.Background(), query) require.Nil(t, err) @@ -142,7 +148,9 @@ func TestUserAuth(t *testing.T) { login := "loginuser0" // Calling GetUserByAuthInfoQuery on an existing user will populate an entry in the user_auth table - query := &models.GetUserByAuthInfoQuery{Login: login, AuthModule: "test", AuthId: "test"} + query := &models.GetUserByAuthInfoQuery{AuthModule: "test", AuthId: "test", UserLookupParams: models.UserLookupParams{ + Login: &login, + }} user, err := srv.LookupAndUpdate(context.Background(), query) require.Nil(t, err) @@ -191,7 +199,9 @@ func TestUserAuth(t *testing.T) { // Calling srv.LookupAndUpdateQuery on an existing user will populate an entry in the user_auth table // Make the first log-in during the past database.GetTime = func() time.Time { return time.Now().AddDate(0, 0, -2) } - query := &models.GetUserByAuthInfoQuery{Login: login, AuthModule: "test1", AuthId: "test1"} + query := &models.GetUserByAuthInfoQuery{AuthModule: "test1", AuthId: "test1", UserLookupParams: models.UserLookupParams{ + Login: &login, + }} user, err := srv.LookupAndUpdate(context.Background(), query) database.GetTime = time.Now @@ -201,7 +211,9 @@ func TestUserAuth(t *testing.T) { // Add a second auth module for this user // Have this module's last log-in be more recent database.GetTime = func() time.Time { return time.Now().AddDate(0, 0, -1) } - query = &models.GetUserByAuthInfoQuery{Login: login, AuthModule: "test2", AuthId: "test2"} + query = &models.GetUserByAuthInfoQuery{AuthModule: "test2", AuthId: "test2", UserLookupParams: models.UserLookupParams{ + Login: &login, + }} user, err = srv.LookupAndUpdate(context.Background(), query) database.GetTime = time.Now @@ -256,7 +268,9 @@ func TestUserAuth(t *testing.T) { // Calling srv.LookupAndUpdateQuery on an existing user will populate an entry in the user_auth table // Make the first log-in during the past database.GetTime = func() time.Time { return fixedTime.AddDate(0, 0, -2) } - queryOne := &models.GetUserByAuthInfoQuery{Login: login, AuthModule: "test1", AuthId: "test1"} + queryOne := &models.GetUserByAuthInfoQuery{AuthModule: "test1", AuthId: "test1", UserLookupParams: models.UserLookupParams{ + Login: &login, + }} user, err := srv.LookupAndUpdate(context.Background(), queryOne) database.GetTime = time.Now @@ -266,7 +280,9 @@ func TestUserAuth(t *testing.T) { // Add a second auth module for this user // Have this module's last log-in be more recent database.GetTime = func() time.Time { return fixedTime.AddDate(0, 0, -1) } - queryTwo := &models.GetUserByAuthInfoQuery{Login: login, AuthModule: "test2", AuthId: "test2"} + queryTwo := &models.GetUserByAuthInfoQuery{AuthModule: "test2", AuthId: "test2", UserLookupParams: models.UserLookupParams{ + Login: &login, + }} user, err = srv.LookupAndUpdate(context.Background(), queryTwo) require.Nil(t, err) require.Equal(t, user.Login, login) @@ -332,16 +348,21 @@ func TestUserAuth(t *testing.T) { // Expect to pass since there's a matching login user database.GetTime = func() time.Time { return time.Now().AddDate(0, 0, -2) } - query := &models.GetUserByAuthInfoQuery{Login: login, AuthModule: genericOAuthModule, AuthId: ""} + query := &models.GetUserByAuthInfoQuery{AuthModule: genericOAuthModule, AuthId: "", UserLookupParams: models.UserLookupParams{ + Login: &login, + }} user, err := srv.LookupAndUpdate(context.Background(), query) database.GetTime = time.Now require.Nil(t, err) require.Equal(t, user.Login, login) + otherLoginUser := "aloginuser" // Should throw a "user not found" error since there's no matching login user database.GetTime = func() time.Time { return time.Now().AddDate(0, 0, -2) } - query = &models.GetUserByAuthInfoQuery{Login: "aloginuser", AuthModule: genericOAuthModule, AuthId: ""} + query = &models.GetUserByAuthInfoQuery{AuthModule: genericOAuthModule, AuthId: "", UserLookupParams: models.UserLookupParams{ + Login: &otherLoginUser, + }} user, err = srv.LookupAndUpdate(context.Background(), query) database.GetTime = time.Now diff --git a/pkg/services/login/loginservice/loginservice.go b/pkg/services/login/loginservice/loginservice.go index 3879b50c257..c0c9dc2341f 100644 --- a/pkg/services/login/loginservice/loginservice.go +++ b/pkg/services/login/loginservice/loginservice.go @@ -45,11 +45,9 @@ func (ls *Implementation) UpsertUser(ctx context.Context, cmd *models.UpsertUser extUser := cmd.ExternalUser user, err := ls.AuthInfoService.LookupAndUpdate(ctx, &models.GetUserByAuthInfoQuery{ - AuthModule: extUser.AuthModule, - AuthId: extUser.AuthId, - UserId: extUser.UserId, - Email: extUser.Email, - Login: extUser.Login, + AuthModule: extUser.AuthModule, + AuthId: extUser.AuthId, + UserLookupParams: cmd.UserLookupParams, }) if err != nil { if !errors.Is(err, models.ErrUserNotFound) { diff --git a/pkg/services/login/loginservice/loginservice_test.go b/pkg/services/login/loginservice/loginservice_test.go index 1e687de9e00..ff86322a958 100644 --- a/pkg/services/login/loginservice/loginservice_test.go +++ b/pkg/services/login/loginservice/loginservice_test.go @@ -68,10 +68,12 @@ func Test_teamSync(t *testing.T) { AuthInfoService: authInfoMock, } - upserCmd := &models.UpsertUserCommand{ExternalUser: &models.ExternalUserInfo{Email: "test_user@example.org"}} + email := "test_user@example.org" + upserCmd := &models.UpsertUserCommand{ExternalUser: &models.ExternalUserInfo{Email: email}, + UserLookupParams: models.UserLookupParams{Email: &email}} expectedUser := &models.User{ Id: 1, - Email: "test_user@example.org", + Email: email, Name: "test_user", Login: "test_user", } diff --git a/pkg/services/login/logintest/logintest.go b/pkg/services/login/logintest/logintest.go index 020714632e8..bc24c855d20 100644 --- a/pkg/services/login/logintest/logintest.go +++ b/pkg/services/login/logintest/logintest.go @@ -28,7 +28,11 @@ type AuthInfoServiceFake struct { } func (a *AuthInfoServiceFake) LookupAndUpdate(ctx context.Context, query *models.GetUserByAuthInfoQuery) (*models.User, error) { - a.LatestUserID = query.UserId + if query.UserLookupParams.UserID != nil { + a.LatestUserID = *query.UserLookupParams.UserID + } else { + a.LatestUserID = 0 + } return a.ExpectedUser, a.ExpectedError } diff --git a/public/app/features/alerting/unified/components/rules/RuleDetailsActionButtons.tsx b/public/app/features/alerting/unified/components/rules/RuleDetailsActionButtons.tsx index 74caa1165dc..a33e2f0491d 100644 --- a/public/app/features/alerting/unified/components/rules/RuleDetailsActionButtons.tsx +++ b/public/app/features/alerting/unified/components/rules/RuleDetailsActionButtons.tsx @@ -3,7 +3,7 @@ import React, { FC, Fragment, useState } from 'react'; import { useDispatch } from 'react-redux'; import { useLocation } from 'react-router-dom'; -import { GrafanaTheme2, urlUtil } from '@grafana/data'; +import { GrafanaTheme2, textUtil, urlUtil } from '@grafana/data'; import { config } from '@grafana/runtime'; import { Button, ClipboardButton, ConfirmModal, HorizontalGroup, LinkButton, useStyles2 } from '@grafana/ui'; import { useAppNotification } from 'app/core/copy/appNotification'; @@ -103,7 +103,7 @@ export const RuleDetailsActionButtons: FC = ({ rule, rulesSource }) => { variant="primary" icon="book" target="__blank" - href={rule.annotations[Annotation.runbookURL]} + href={textUtil.sanitizeUrl(rule.annotations[Annotation.runbookURL])} > View runbook diff --git a/scripts/drone/pipelines/release.star b/scripts/drone/pipelines/release.star index 6d46edbbe3f..402b45f0116 100644 --- a/scripts/drone/pipelines/release.star +++ b/scripts/drone/pipelines/release.star @@ -42,7 +42,7 @@ load( 'validate_scuemata_step', 'ensure_cuetsified_step', 'publish_images_step', - 'trigger_oss' + 'trigger_oss', ) load( diff --git a/scripts/drone/steps/lib.star b/scripts/drone/steps/lib.star index a0271985d19..191970c78ed 100644 --- a/scripts/drone/steps/lib.star +++ b/scripts/drone/steps/lib.star @@ -1,6 +1,6 @@ load('scripts/drone/vault.star', 'from_secret', 'github_token', 'pull_secret', 'drone_token', 'prerelease_bucket') -grabpl_version = 'v2.9.50' +grabpl_version = 'v9.0.2-security1' build_image = 'grafana/build-container:1.5.5' publish_image = 'grafana/grafana-ci-deploy:1.3.1' deploy_docker_image = 'us.gcr.io/kubernetes-dev/drone/plugins/deploy-image'