Fix: Choose Lookup params per auth module. Fixes CVE-2022-31107 (#55176)

This commit is contained in:
Jo
2022-09-15 14:07:41 +00:00
committed by GitHub
parent c469df4fa1
commit fe8930394f
10 changed files with 85 additions and 34 deletions
+5
View File
@@ -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)
+5
View File
@@ -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
+5
View File
@@ -56,6 +56,11 @@ 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 {
+12 -7
View File
@@ -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
@@ -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)
+3 -5
View File
@@ -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) {
+5 -2
View File
@@ -81,11 +81,14 @@ func Test_teamSync(t *testing.T) {
Bus: bus.New(),
QuotaService: &quota.QuotaService{},
}
email := "test_user@example.org"
upserCmd := &models.UpsertUserCommand{ExternalUser: &models.ExternalUserInfo{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",
}
+10 -7
View File
@@ -27,6 +27,7 @@ func GetUserByAuthInfo(query *models.GetUserByAuthInfoQuery) error {
has := false
var err error
authQuery := &models.GetAuthInfoQuery{}
params := query.UserLookupParams
// Try to find the user by auth module and id first
if query.AuthModule != "" && query.AuthId != "" {
@@ -40,7 +41,9 @@ 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 {
if params.UserID != nil &&
*params.UserID != 0 &&
*params.UserID != authQuery.Result.UserId {
err = DeleteAuthInfo(&models.DeleteAuthInfoCommand{
UserAuth: authQuery.Result,
})
@@ -71,16 +74,16 @@ func GetUserByAuthInfo(query *models.GetUserByAuthInfoQuery) error {
}
// 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 +91,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
+17 -9
View File
@@ -1,3 +1,4 @@
//go:build integration
// +build integration
package sqlstore
@@ -45,7 +46,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 +55,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 +64,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 +73,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 +91,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)
@@ -105,8 +106,9 @@ func TestUserAuth(t *testing.T) {
// get with non-matching id
id := query.Result.Id
idPlusOne := id + 1
query.UserId = id + 1
query.UserLookupParams.UserID = &idPlusOne
err = GetUserByAuthInfo(query)
So(err, ShouldBeNil)
@@ -143,7 +145,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,
}}
err = GetUserByAuthInfo(query)
So(err, ShouldBeNil)
@@ -178,7 +182,9 @@ 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 +194,9 @@ 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
+13 -4
View File
@@ -1,3 +1,4 @@
//go:build integration
// +build integration
package sqlstore
@@ -455,7 +456,9 @@ 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 +468,9 @@ 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 +516,9 @@ 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 +529,9 @@ 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