IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2026-08-19 01:34:54 -05:00
Code Issues Packages Projects Releases Wiki Activity

Auth: Add option for case insensitive login (#49262)

Browse Source 
* add case insensitive option

* treat id as case insensitive

* Users: Add integration tests for case insensitive querying

* Prefer config struct to global variable

* change key to case_insensitive_login

* impede conflicting users from logging in

* add tests for impeding user retrieval if conflicting

* nits and picks

Co-authored-by: gamab <gabi.mabs@gmail.com>

* Add check in transaction for conflicting user

Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>

* add update tests

* skip on mysql

* add custom messages for user admin view

Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>

* nit: extra else

* linting mistake

Co-authored-by: gamab <gabi.mabs@gmail.com>
Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>
This commit is contained in:
Jguer
2022-06-24 16:59:45 +02:00
committed by GitHub
co-authored by gamab Gabriel MABILLE
parent 620309ced5
commit 0689c5839a
6 changed files with 315 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/sqlstore/user.go
+102 -9
View File
@@ -14,6 +14,27 @@ import (
"github.com/grafana/grafana/pkg/util"
)
type ErrCaseInsensitiveLoginConflict struct {
users []models.User
}
func (e *ErrCaseInsensitiveLoginConflict) Unwrap() error {
return models.ErrCaseInsensitive
}
func (e *ErrCaseInsensitiveLoginConflict) Error() string {
n := len(e.users)
userStrings := make([]string, 0, n)
for _, v := range e.users {
userStrings = append(userStrings, fmt.Sprintf("%s (email:%s, id:%d)", v.Login, v.Email, v.Id))
}
return fmt.Sprintf(
"Found a conflict in user login information. %d users already exist with either the same login or email: [%s].",
n, strings.Join(userStrings, ", "))
}
func (ss *SQLStore) getOrgIDForNewUser(sess *DBSession, args models.CreateUserCommand) (int64, error) {
if ss.Cfg.AutoAssignOrg && args.OrgId != 0 {
if err := verifyExistingOrg(sess, args.OrgId); err != nil {
@@ -30,6 +51,21 @@ func (ss *SQLStore) getOrgIDForNewUser(sess *DBSession, args models.CreateUserCo
return ss.getOrCreateOrg(sess, orgName)
}
func (ss *SQLStore) userCaseInsensitiveLoginConflict(ctx context.Context, sess *DBSession, login, email string) error {
users := make([]models.User, 0)
if err := sess.Where("LOWER(email)=LOWER(?) OR LOWER(login)=LOWER(?)",
email, login).Find(&users); err != nil {
return err
}
if len(users) > 1 {
return &ErrCaseInsensitiveLoginConflict{users}
}
return nil
}
// createUser creates a user in the database
func (ss *SQLStore) createUser(ctx context.Context, sess *DBSession, args models.CreateUserCommand) (models.User, error) {
var user models.User
@@ -46,7 +82,14 @@ func (ss *SQLStore) createUser(ctx context.Context, sess *DBSession, args models
args.Email = args.Login
}
exists, err := sess.Where("email=? OR login=?", args.Email, args.Login).Get(&models.User{})
where := "email=? OR login=?"
if ss.Cfg.CaseInsensitiveLogin {
where = "LOWER(email)=LOWER(?) OR LOWER(login)=LOWER(?)"
args.Login = strings.ToLower(args.Login)
args.Email = strings.ToLower(args.Email)
}
exists, err := sess.Where(where, args.Email, args.Login).Get(&models.User{})
if err != nil {
return user, err
}
@@ -158,6 +201,12 @@ func (ss *SQLStore) GetUserById(ctx context.Context, query *models.GetUserByIdQu
return models.ErrUserNotFound
}
if ss.Cfg.CaseInsensitiveLogin {
if err := ss.userCaseInsensitiveLoginConflict(ctx, sess, user.Login, user.Email); err != nil {
return err
}
}
query.Result = user
return nil
@@ -172,9 +221,13 @@ func (ss *SQLStore) GetUserByLogin(ctx context.Context, query *models.GetUserByL
// Try and find the user by login first.
// It's not sufficient to assume that a LoginOrEmail with an "@" is an email.
user := &models.User{Login: query.LoginOrEmail}
has, err := sess.Where(notServiceAccountFilter(ss)).Get(user)
user := &models.User{}
where := "login=?"
if ss.Cfg.CaseInsensitiveLogin {
where = "LOWER(login)=LOWER(?)"
}
has, err := sess.Where(notServiceAccountFilter(ss)).Where(where, query.LoginOrEmail).Get(user)
if err != nil {
return err
}
@@ -182,8 +235,12 @@ func (ss *SQLStore) GetUserByLogin(ctx context.Context, query *models.GetUserByL
if !has && strings.Contains(query.LoginOrEmail, "@") {
// If the user wasn't found, and it contains an "@" fallback to finding the
// user by email.
user = &models.User{Email: query.LoginOrEmail}
has, err = sess.Get(user)
where = "email=?"
if ss.Cfg.CaseInsensitiveLogin {
where = "LOWER(email)=LOWER(?)"
}
user = &models.User{}
has, err = sess.Where(notServiceAccountFilter(ss)).Where(where, query.LoginOrEmail).Get(user)
}
if err != nil {
@@ -192,6 +249,12 @@ func (ss *SQLStore) GetUserByLogin(ctx context.Context, query *models.GetUserByL
return models.ErrUserNotFound
}
if ss.Cfg.CaseInsensitiveLogin {
if err := ss.userCaseInsensitiveLoginConflict(ctx, sess, user.Login, user.Email); err != nil {
return err
}
}
query.Result = user
return nil
@@ -204,8 +267,13 @@ func (ss *SQLStore) GetUserByEmail(ctx context.Context, query *models.GetUserByE
return models.ErrUserNotFound
}
user := &models.User{Email: query.Email}
has, err := sess.Where(notServiceAccountFilter(ss)).Get(user)
user := &models.User{}
where := "email=?"
if ss.Cfg.CaseInsensitiveLogin {
where = "LOWER(email)=LOWER(?)"
}
has, err := sess.Where(notServiceAccountFilter(ss)).Where(where, query.Email).Get(user)
if err != nil {
return err
@@ -213,6 +281,12 @@ func (ss *SQLStore) GetUserByEmail(ctx context.Context, query *models.GetUserByE
return models.ErrUserNotFound
}
if ss.Cfg.CaseInsensitiveLogin {
if err := ss.userCaseInsensitiveLoginConflict(ctx, sess, user.Login, user.Email); err != nil {
return err
}
}
query.Result = user
return nil
@@ -220,6 +294,11 @@ func (ss *SQLStore) GetUserByEmail(ctx context.Context, query *models.GetUserByE
}
func (ss *SQLStore) UpdateUser(ctx context.Context, cmd *models.UpdateUserCommand) error {
if ss.Cfg.CaseInsensitiveLogin {
cmd.Login = strings.ToLower(cmd.Login)
cmd.Email = strings.ToLower(cmd.Email)
}
return ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error {
user := models.User{
Name: cmd.Name,
@@ -233,6 +312,12 @@ func (ss *SQLStore) UpdateUser(ctx context.Context, cmd *models.UpdateUserComman
return err
}
if ss.Cfg.CaseInsensitiveLogin {
if err := ss.userCaseInsensitiveLoginConflict(ctx, sess, user.Login, user.Email); err != nil {
return err
}
}
sess.publishAfterCommit(&events.UserUpdated{
Timestamp: user.Created,
Id: user.Id,
@@ -430,9 +515,17 @@ func (ss *SQLStore) GetSignedInUser(ctx context.Context, query *models.GetSigned
case query.UserId > 0:
sess.SQL(rawSQL+"WHERE u.id=?", query.UserId)
case query.Login != "":
sess.SQL(rawSQL+"WHERE u.login=?", query.Login)
if ss.Cfg.CaseInsensitiveLogin {
sess.SQL(rawSQL+"WHERE LOWER(u.login)=LOWER(?)", query.Login)
} else {
sess.SQL(rawSQL+"WHERE u.login=?", query.Login)
}
case query.Email != "":
sess.SQL(rawSQL+"WHERE u.email=?", query.Email)
if ss.Cfg.CaseInsensitiveLogin {
sess.SQL(rawSQL+"WHERE LOWER(u.email)=LOWER(?)", query.Email)
} else {
sess.SQL(rawSQL+"WHERE u.email=?", query.Email)
}
}
var user models.SignedInUser
pkg/services/sqlstore/user_test.go
+198
View File
@@ -6,10 +6,78 @@ import (
"testing"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestIntegrationUserUpdate(t *testing.T) {
if testing.Short() {
t.Skip("skipping integration test")
}
ss := InitTestDB(t)
users := createFiveTestUsers(t, ss, func(i int) *models.CreateUserCommand {
return &models.CreateUserCommand{
Email: fmt.Sprint("USER", i, "@test.com"),
Name: fmt.Sprint("USER", i),
Login: fmt.Sprint("loginUSER", i),
IsDisabled: false,
}
})
ss.Cfg.CaseInsensitiveLogin = true
t.Run("Testing DB - update generates duplicate user", func(t *testing.T) {
err := ss.UpdateUser(context.Background(), &models.UpdateUserCommand{
Login: "loginuser2",
UserId: users[0].Id,
})
require.Error(t, err)
})
t.Run("Testing DB - update lowercases existing user", func(t *testing.T) {
err := ss.UpdateUser(context.Background(), &models.UpdateUserCommand{
Login: "loginUSER0",
Email: "USER0@test.com",
UserId: users[0].Id,
})
require.NoError(t, err)
query := models.GetUserByIdQuery{Id: users[0].Id}
err = ss.GetUserById(context.Background(), &query)
require.NoError(t, err)
require.Equal(t, "loginuser0", query.Result.Login)
require.Equal(t, "user0@test.com", query.Result.Email)
})
t.Run("Testing DB - no user info provided", func(t *testing.T) {
err := ss.UpdateUser(context.Background(), &models.UpdateUserCommand{
Login: "",
Email: "",
Name: "Change Name",
UserId: users[3].Id,
})
require.NoError(t, err)
query := models.GetUserByIdQuery{Id: users[3].Id}
err = ss.GetUserById(context.Background(), &query)
require.NoError(t, err)
// Changed
require.Equal(t, "Change Name", query.Result.Name)
// Unchanged
require.Equal(t, "loginUSER3", query.Result.Login)
require.Equal(t, "USER3@test.com", query.Result.Email)
})
ss.Cfg.CaseInsensitiveLogin = false
}
func TestIntegrationUserDataAccess(t *testing.T) {
if testing.Short() {
t.Skip("skipping integration test")
@@ -48,6 +116,52 @@ func TestIntegrationUserDataAccess(t *testing.T) {
require.Len(t, query.Result.Rands, 10)
require.Len(t, query.Result.Salt, 10)
require.False(t, query.Result.IsDisabled)
t.Run("Get User by email case insensitive", func(t *testing.T) {
ss.Cfg.CaseInsensitiveLogin = true
query := models.GetUserByEmailQuery{Email: "USERtest@TEST.COM"}
err = ss.GetUserByEmail(context.Background(), &query)
require.Nil(t, err)
require.Equal(t, query.Result.Email, "usertest@test.com")
require.Equal(t, query.Result.Password, "")
require.Len(t, query.Result.Rands, 10)
require.Len(t, query.Result.Salt, 10)
require.False(t, query.Result.IsDisabled)
ss.Cfg.CaseInsensitiveLogin = false
})
t.Run("Get User by login - case insensitive", func(t *testing.T) {
ss.Cfg.CaseInsensitiveLogin = true
query := models.GetUserByLoginQuery{LoginOrEmail: "USER_test_login"}
err = ss.GetUserByLogin(context.Background(), &query)
require.Nil(t, err)
require.Equal(t, query.Result.Email, "usertest@test.com")
require.Equal(t, query.Result.Password, "")
require.Len(t, query.Result.Rands, 10)
require.Len(t, query.Result.Salt, 10)
require.False(t, query.Result.IsDisabled)
ss.Cfg.CaseInsensitiveLogin = false
})
t.Run("Get User by login - email fallback case insensitive", func(t *testing.T) {
ss.Cfg.CaseInsensitiveLogin = true
query := models.GetUserByLoginQuery{LoginOrEmail: "USERtest@TEST.COM"}
err = ss.GetUserByLogin(context.Background(), &query)
require.Nil(t, err)
require.Equal(t, query.Result.Email, "usertest@test.com")
require.Equal(t, query.Result.Password, "")
require.Len(t, query.Result.Rands, 10)
require.Len(t, query.Result.Salt, 10)
require.False(t, query.Result.IsDisabled)
ss.Cfg.CaseInsensitiveLogin = false
})
})
t.Run("Testing DB - creates and loads disabled user", func(t *testing.T) {
@@ -359,6 +473,90 @@ func TestIntegrationUserDataAccess(t *testing.T) {
assert.Len(t, query.Result.Users, 2)
})
t.Run("Testing DB - error on case insensitive conflict", func(t *testing.T) {
if ss.engine.Dialect().DBType() == migrator.MySQL {
t.Skip("Skipping on MySQL due to case insensitive indexes")
}
cmd := models.CreateUserCommand{
Email: "confusertest@test.com",
Name: "user name",
Login: "user_email_conflict",
}
userEmailConflict, err := ss.CreateUser(context.Background(), cmd)
require.NoError(t, err)
cmd = models.CreateUserCommand{
Email: "confusertest@TEST.COM",
Name: "user name",
Login: "user_email_conflict_two",
}
_, err = ss.CreateUser(context.Background(), cmd)
require.NoError(t, err)
cmd = models.CreateUserCommand{
Email: "user_test_login_conflict@test.com",
Name: "user name",
Login: "user_test_login_conflict",
}
userLoginConflict, err := ss.CreateUser(context.Background(), cmd)
require.NoError(t, err)
cmd = models.CreateUserCommand{
Email: "user_test_login_conflict_two@test.com",
Name: "user name",
Login: "user_test_login_CONFLICT",
}
_, err = ss.CreateUser(context.Background(), cmd)
require.NoError(t, err)
ss.Cfg.CaseInsensitiveLogin = true
t.Run("GetUserByEmail - email conflict", func(t *testing.T) {
query := models.GetUserByEmailQuery{Email: "confusertest@test.com"}
err = ss.GetUserByEmail(context.Background(), &query)
require.Error(t, err)
})
t.Run("GetUserByEmail - login conflict", func(t *testing.T) {
query := models.GetUserByEmailQuery{Email: "user_test_login_conflict@test.com"}
err = ss.GetUserByEmail(context.Background(), &query)
require.Error(t, err)
})
t.Run("GetUserByID - email conflict", func(t *testing.T) {
query := models.GetUserByIdQuery{Id: userEmailConflict.Id}
err = ss.GetUserById(context.Background(), &query)
require.Error(t, err)
})
t.Run("GetUserByID - login conflict", func(t *testing.T) {
query := models.GetUserByIdQuery{Id: userLoginConflict.Id}
err = ss.GetUserById(context.Background(), &query)
require.Error(t, err)
})
t.Run("GetUserByLogin - email conflict", func(t *testing.T) {
query := models.GetUserByLoginQuery{LoginOrEmail: "user_email_conflict_two"}
err = ss.GetUserByLogin(context.Background(), &query)
require.Error(t, err)
})
t.Run("GetUserByLogin - login conflict", func(t *testing.T) {
query := models.GetUserByLoginQuery{LoginOrEmail: "user_test_login_conflict"}
err = ss.GetUserByLogin(context.Background(), &query)
require.Error(t, err)
})
t.Run("GetUserByLogin - login conflict by email", func(t *testing.T) {
query := models.GetUserByLoginQuery{LoginOrEmail: "user_test_login_conflict@test.com"}
err = ss.GetUserByLogin(context.Background(), &query)
require.Error(t, err)
})
ss.Cfg.CaseInsensitiveLogin = false
})
ss = InitTestDB(t)
t.Run("Testing DB - enable all users", func(t *testing.T) {