From dea50c6fbd9eccb09236f0d057eed6f1c4f8368a Mon Sep 17 00:00:00 2001 From: Andreas Gerstmayr Date: Fri, 15 Jul 2022 14:05:14 +0200 Subject: [PATCH] fix CVE-2022-31107 backport 967e17d7ef6bc62a108add33ea699710f0e15870 from v8.4.10 Co-authored-by: Karl Persson Co-authored-by: Jguer diff --git a/pkg/api/ldap_debug.go b/pkg/api/ldap_debug.go index 126e760b67..c9e2b606c5 100644 --- a/pkg/api/ldap_debug.go +++ b/pkg/api/ldap_debug.go @@ -215,6 +215,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 index 1fce9b6f61..611d51444f 100644 --- a/pkg/api/login_oauth.go +++ b/pkg/api/login_oauth.go @@ -250,6 +250,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 index cb5d984e73..82dac2ee9e 100644 --- a/pkg/login/ldap_login.go +++ b/pkg/login/ldap_login.go @@ -56,9 +56,13 @@ var loginUsingLDAP = func(query *models.LoginUserQuery) (bool, error) { ReqContext: query.ReqContext, ExternalUser: externalUser, SignupAllowed: setting.LDAPAllowSignup, + UserLookupParams: models.UserLookupParams{ + Login: &externalUser.Login, + Email: &externalUser.Email, + UserID: nil, + }, } - err = bus.Dispatch(upsert) - if err != nil { + if err = bus.Dispatch(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 2061cf048b..a98efe659e 100644 --- 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,13 +95,18 @@ type LoginUserQuery struct { type GetUserByAuthInfoQuery struct { AuthModule string AuthId string - UserId int64 - Email string - Login string + UserLookupParams Result *User } +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 { LoginOrEmail string diff --git a/pkg/services/contexthandler/authproxy/authproxy.go b/pkg/services/contexthandler/authproxy/authproxy.go index 80e5a5b9e0..0d834748a7 100644 --- a/pkg/services/contexthandler/authproxy/authproxy.go +++ b/pkg/services/contexthandler/authproxy/authproxy.go @@ -246,6 +246,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 @@ -288,6 +293,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/login.go b/pkg/services/login/login.go index 9e08a36b06..b74d1d3e8f 100644 --- a/pkg/services/login/login.go +++ b/pkg/services/login/login.go @@ -37,11 +37,9 @@ func (ls *LoginService) UpsertUser(cmd *models.UpsertUserCommand) error { extUser := cmd.ExternalUser userQuery := &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 := bus.Dispatch(userQuery); err != nil { if !errors.Is(err, models.ErrUserNotFound) { diff --git a/pkg/services/login/login_test.go b/pkg/services/login/login_test.go index 04953b567a..dd84ee29c8 100644 --- a/pkg/services/login/login_test.go +++ b/pkg/services/login/login_test.go @@ -82,10 +82,12 @@ func Test_teamSync(t *testing.T) { QuotaService: "a.QuotaService{}, } - 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/sqlstore/user_auth.go b/pkg/services/sqlstore/user_auth.go index 0bef79e160..5fd744172b 100644 --- a/pkg/services/sqlstore/user_auth.go +++ b/pkg/services/sqlstore/user_auth.go @@ -40,11 +40,12 @@ func GetUserByAuthInfo(query *models.GetUserByAuthInfoQuery) error { } // 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 = DeleteAuthInfo(&models.DeleteAuthInfoCommand{ + if query.UserLookupParams.UserID != nil && + *query.UserLookupParams.UserID != 0 && + *query.UserLookupParams.UserID != authQuery.Result.UserId { + if err := DeleteAuthInfo(&models.DeleteAuthInfoCommand{ UserAuth: authQuery.Result, - }) - if err != nil { + }); err != nil { sqlog.Error("Error removing user_auth entry", "error", err) } @@ -70,17 +71,18 @@ func GetUserByAuthInfo(query *models.GetUserByAuthInfoQuery) error { } } + params := query.UserLookupParams // If not found, try to find the user by id - if !has && query.UserId != 0 { - has, err = x.Id(query.UserId).Get(user) + if !has && params.UserID != nil && *params.UserID != 0 { + has, err = x.Id(*params.UserID).Get(user) if err != nil { return err } } // If not found, try to find the user by email address - if !has && query.Email != "" { - user = &models.User{Email: query.Email} + if !has && params.Email != nil && *params.Email != "" { + user = &models.User{Email: *params.Email} has, err = x.Get(user) if err != nil { return err @@ -88,8 +90,8 @@ func GetUserByAuthInfo(query *models.GetUserByAuthInfoQuery) error { } // If not found, try to find the user by login - if !has && query.Login != "" { - user = &models.User{Login: query.Login} + if !has && params.Login != nil && *params.Login != "" { + user = &models.User{Login: *params.Login} has, err = x.Get(user) if err != nil { return err diff --git a/pkg/services/sqlstore/user_auth_test.go b/pkg/services/sqlstore/user_auth_test.go index e5bb2379e5..d94ce34edb 100644 --- a/pkg/services/sqlstore/user_auth_test.go +++ b/pkg/services/sqlstore/user_auth_test.go @@ -45,7 +45,7 @@ func TestUserAuth(t *testing.T) { // By Login login := "loginuser0" - query := &models.GetUserByAuthInfoQuery{Login: login} + query := &models.GetUserByAuthInfoQuery{UserLookupParams: models.UserLookupParams{Login: &login}} err = GetUserByAuthInfo(query) So(err, ShouldBeNil) @@ -54,7 +54,7 @@ func TestUserAuth(t *testing.T) { // By ID id := query.Result.Id - query = &models.GetUserByAuthInfoQuery{UserId: id} + query = &models.GetUserByAuthInfoQuery{UserLookupParams: models.UserLookupParams{UserID: &id}} err = GetUserByAuthInfo(query) So(err, ShouldBeNil) @@ -63,7 +63,7 @@ func TestUserAuth(t *testing.T) { // By Email email := "user1@test.com" - query = &models.GetUserByAuthInfoQuery{Email: email} + query = &models.GetUserByAuthInfoQuery{UserLookupParams: models.UserLookupParams{Email: &email}} err = GetUserByAuthInfo(query) So(err, ShouldBeNil) @@ -72,7 +72,7 @@ func TestUserAuth(t *testing.T) { // Don't find nonexistent user email = "nonexistent@test.com" - query = &models.GetUserByAuthInfoQuery{Email: email} + query = &models.GetUserByAuthInfoQuery{UserLookupParams: models.UserLookupParams{Email: &email}} err = GetUserByAuthInfo(query) So(err, ShouldEqual, models.ErrUserNotFound) @@ -90,7 +90,7 @@ func TestUserAuth(t *testing.T) { // create user_auth entry login := "loginuser0" - query.Login = login + query.UserLookupParams.Login = &login err = GetUserByAuthInfo(query) So(err, ShouldBeNil) @@ -104,9 +104,9 @@ func TestUserAuth(t *testing.T) { So(query.Result.Login, ShouldEqual, login) // get with non-matching id - id := query.Result.Id + idPlusOne := query.Result.Id + 1 - query.UserId = id + 1 + query.UserLookupParams.UserID = &idPlusOne err = GetUserByAuthInfo(query) So(err, ShouldBeNil) @@ -143,7 +143,7 @@ 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}} err = GetUserByAuthInfo(query) So(err, ShouldBeNil) @@ -178,7 +178,7 @@ func TestUserAuth(t *testing.T) { // Calling GetUserByAuthInfoQuery on an existing user will populate an entry in the user_auth table // Make the first log-in during the past 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}} err = GetUserByAuthInfo(query) getTime = time.Now @@ -188,7 +188,7 @@ func TestUserAuth(t *testing.T) { // Add a second auth module for this user // Have this module's last log-in be more recent 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}} err = GetUserByAuthInfo(query) getTime = time.Now diff --git a/pkg/services/sqlstore/user_test.go b/pkg/services/sqlstore/user_test.go index 7da19f0ef4..aa796ffb02 100644 --- a/pkg/services/sqlstore/user_test.go +++ b/pkg/services/sqlstore/user_test.go @@ -455,7 +455,7 @@ func TestUserDataAccess(t *testing.T) { // Calling GetUserByAuthInfoQuery on an existing user will populate an entry in the user_auth table // Make the first log-in during the past getTime = func() time.Time { return time.Now().AddDate(0, 0, -2) } - query := &models.GetUserByAuthInfoQuery{Login: login, AuthModule: "ldap", AuthId: "ldap0"} + query := &models.GetUserByAuthInfoQuery{AuthModule: "ldap", AuthId: "ldap0", UserLookupParams: models.UserLookupParams{Login: &login}} err := GetUserByAuthInfo(query) getTime = time.Now @@ -465,7 +465,7 @@ func TestUserDataAccess(t *testing.T) { // Add a second auth module for this user // Have this module's last log-in be more recent getTime = func() time.Time { return time.Now().AddDate(0, 0, -1) } - query = &models.GetUserByAuthInfoQuery{Login: login, AuthModule: "oauth", AuthId: "oauth0"} + query = &models.GetUserByAuthInfoQuery{AuthModule: "oauth", AuthId: "oauth0", UserLookupParams: models.UserLookupParams{Login: &login}} err = GetUserByAuthInfo(query) getTime = time.Now @@ -511,7 +511,7 @@ func TestUserDataAccess(t *testing.T) { // Calling GetUserByAuthInfoQuery on an existing user will populate an entry in the user_auth table // Make the first log-in during the past getTime = func() time.Time { return time.Now().AddDate(0, 0, -2) } - query := &models.GetUserByAuthInfoQuery{Login: login, AuthModule: "ldap", AuthId: fmt.Sprint("ldap", i)} + query := &models.GetUserByAuthInfoQuery{AuthModule: "ldap", AuthId: fmt.Sprint("ldap", i), UserLookupParams: models.UserLookupParams{Login: &login}} err := GetUserByAuthInfo(query) getTime = time.Now @@ -522,7 +522,7 @@ func TestUserDataAccess(t *testing.T) { // Log in first user with oauth login := "loginuser0" getTime = func() time.Time { return time.Now().AddDate(0, 0, -1) } - query := &models.GetUserByAuthInfoQuery{Login: login, AuthModule: "oauth", AuthId: "oauth0"} + query := &models.GetUserByAuthInfoQuery{AuthModule: "oauth", AuthId: "oauth0", UserLookupParams: models.UserLookupParams{Login: &login}} err := GetUserByAuthInfo(query) getTime = time.Now