public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Auth: Enable case insensitive logins/emails by default (#84840)

Browse Source 
* wip

* wip

* wip

* wip postgres tests
This commit is contained in:
Eric Leijonmarck
2024-03-22 16:45:18 +01:00
committed by GitHub
parent d7fa99e2df
commit 2f7fd729ef
10 changed files with 294 additions and 252 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/user/userimpl/store.go
+21 -53
View File
@@ -23,7 +23,7 @@ type store interface {
GetByID(context.Context, int64) (*user.User, error)
GetNotServiceAccount(context.Context, int64) (*user.User, error)
Delete(context.Context, int64) error
LoginConflict(ctx context.Context, login, email string, caseInsensitive bool) error
LoginConflict(ctx context.Context, login, email string) error
CaseInsensitiveLoginConflict(context.Context, string, string) error
GetByLogin(context.Context, *user.GetUserByLoginQuery) (*user.User, error)
GetByEmail(context.Context, *user.GetUserByEmailQuery) (*user.User, error)
@@ -94,12 +94,9 @@ func (ss *sqlStore) Insert(ctx context.Context, cmd *user.User) (int64, error) {
func (ss *sqlStore) Get(ctx context.Context, usr *user.User) (*user.User, error) {
ret := &user.User{}
err := ss.db.WithDbSession(ctx, func(sess *db.Session) error {
where := "email=? OR login=?"
login := usr.Login
email := usr.Email
if ss.cfg.CaseInsensitiveLogin {
where = "LOWER(email)=LOWER(?) OR LOWER(login)=LOWER(?)"
}
where := "LOWER(email)=LOWER(?) OR LOWER(login)=LOWER(?)"
exists, err := sess.Where(where, email, login).Get(ret)
if !exists {
@@ -198,10 +195,7 @@ func (ss *sqlStore) GetByLogin(ctx context.Context, query *user.GetUserByLoginQu
// Since username can be an email address, attempt login with email address
// first if the login field has the "@" symbol.
if strings.Contains(query.LoginOrEmail, "@") {
where = "email=?"
if ss.cfg.CaseInsensitiveLogin {
where = "LOWER(email)=LOWER(?)"
}
where = "LOWER(email)=LOWER(?)"
has, err = sess.Where(ss.notServiceAccountFilter()).Where(where, query.LoginOrEmail).Get(usr)
if err != nil {
return err
@@ -210,10 +204,7 @@ func (ss *sqlStore) GetByLogin(ctx context.Context, query *user.GetUserByLoginQu
// Look for the login field instead of email
if !has {
where = "login=?"
if ss.cfg.CaseInsensitiveLogin {
where = "LOWER(login)=LOWER(?)"
}
where = "LOWER(login)=LOWER(?)"
has, err = sess.Where(ss.notServiceAccountFilter()).Where(where, query.LoginOrEmail).Get(usr)
}
@@ -222,10 +213,8 @@ func (ss *sqlStore) GetByLogin(ctx context.Context, query *user.GetUserByLoginQu
} else if !has {
return user.ErrUserNotFound
}
if ss.cfg.CaseInsensitiveLogin {
if err := ss.userCaseInsensitiveLoginConflict(ctx, sess, usr.Login, usr.Email); err != nil {
return err
}
if err := ss.userCaseInsensitiveLoginConflict(ctx, sess, usr.Login, usr.Email); err != nil {
return err
}
return nil
})
@@ -244,11 +233,7 @@ func (ss *sqlStore) GetByEmail(ctx context.Context, query *user.GetUserByEmailQu
return user.ErrUserNotFound
}
where := "email=?"
if ss.cfg.CaseInsensitiveLogin {
where = "LOWER(email)=LOWER(?)"
}
where := "LOWER(email)=LOWER(?)"
has, err := sess.Where(ss.notServiceAccountFilter()).Where(where, query.Email).Get(usr)
if err != nil {
@@ -257,10 +242,8 @@ func (ss *sqlStore) GetByEmail(ctx context.Context, query *user.GetUserByEmailQu
return user.ErrUserNotFound
}
if ss.cfg.CaseInsensitiveLogin {
if err := ss.userCaseInsensitiveLoginConflict(ctx, sess, usr.Login, usr.Email); err != nil {
return err
}
if err := ss.userCaseInsensitiveLoginConflict(ctx, sess, usr.Login, usr.Email); err != nil {
return err
}
return nil
})
@@ -288,21 +271,18 @@ func (ss *sqlStore) userCaseInsensitiveLoginConflict(ctx context.Context, sess *
// LoginConflict returns an error if the provided email or login are already
// associated with a user. If caseInsensitive is true the search is not case
// sensitive.
func (ss *sqlStore) LoginConflict(ctx context.Context, login, email string, caseInsensitive bool) error {
func (ss *sqlStore) LoginConflict(ctx context.Context, login, email string) error {
err := ss.db.WithDbSession(ctx, func(sess *db.Session) error {
return ss.loginConflict(ctx, sess, login, email, caseInsensitive)
return ss.loginConflict(ctx, sess, login, email)
})
return err
}
func (ss *sqlStore) loginConflict(ctx context.Context, sess *db.Session, login, email string, caseInsensitive bool) error {
func (ss *sqlStore) loginConflict(ctx context.Context, sess *db.Session, login, email string) error {
users := make([]user.User, 0)
where := "email=? OR login=?"
if caseInsensitive {
where = "LOWER(email)=LOWER(?) OR LOWER(login)=LOWER(?)"
login = strings.ToLower(login)
email = strings.ToLower(email)
}
where := "LOWER(email)=LOWER(?) OR LOWER(login)=LOWER(?)"
login = strings.ToLower(login)
email = strings.ToLower(email)
exists, err := sess.Where(where, email, login).Get(&user.User{})
if err != nil {
@@ -323,10 +303,8 @@ func (ss *sqlStore) loginConflict(ctx context.Context, sess *db.Session, login,
}
func (ss *sqlStore) Update(ctx context.Context, cmd *user.UpdateUserCommand) error {
if ss.cfg.CaseInsensitiveLogin {
cmd.Login = strings.ToLower(cmd.Login)
cmd.Email = strings.ToLower(cmd.Email)
}
cmd.Login = strings.ToLower(cmd.Login)
cmd.Email = strings.ToLower(cmd.Email)
return ss.db.WithTransactionalDbSession(ctx, func(sess *db.Session) error {
user := user.User{
@@ -341,10 +319,8 @@ func (ss *sqlStore) Update(ctx context.Context, cmd *user.UpdateUserCommand) err
return err
}
if ss.cfg.CaseInsensitiveLogin {
if err := ss.userCaseInsensitiveLoginConflict(ctx, sess, user.Login, user.Email); err != nil {
return err
}
if err := ss.userCaseInsensitiveLoginConflict(ctx, sess, user.Login, user.Email); err != nil {
return err
}
sess.PublishAfterCommit(&events.UserUpdated{
@@ -418,17 +394,9 @@ func (ss *sqlStore) GetSignedInUser(ctx context.Context, query *user.GetSignedIn
case query.UserID > 0:
sess.SQL(rawSQL+"WHERE u.id=?", query.UserID)
case 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)
}
sess.SQL(rawSQL+"WHERE LOWER(u.login)=LOWER(?)", query.Login)
case 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)
}
sess.SQL(rawSQL+"WHERE LOWER(u.email)=LOWER(?)", query.Email)
default:
return user.ErrNoUniqueID
}
pkg/services/user/userimpl/store_test.go
+114 -75
View File
@@ -15,6 +15,7 @@ import (
"github.com/grafana/grafana/pkg/services/org/orgimpl"
"github.com/grafana/grafana/pkg/services/quota/quotaimpl"
"github.com/grafana/grafana/pkg/services/searchusers/sortopts"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
"github.com/grafana/grafana/pkg/services/supportbundles/supportbundlestest"
"github.com/grafana/grafana/pkg/services/user"
@@ -28,46 +29,34 @@ func TestMain(m *testing.M) {
func TestIntegrationUserGet(t *testing.T) {
testCases := []struct {
name string
wantErr error
searchLogin string
searchEmail string
caseInsensitive bool
name string
wantErr error
searchLogin string
searchEmail string
}{
{
name: "user not found non exact - not case insensitive",
wantErr: user.ErrUserNotFound,
searchLogin: "Test",
searchEmail: "Test@email.com",
caseInsensitive: false,
name: "user found non exact",
wantErr: nil,
searchLogin: "test",
searchEmail: "Test@email.com",
},
{
name: "user found exact - not case insensitive",
wantErr: nil,
searchLogin: "test",
searchEmail: "test@email.com",
caseInsensitive: false,
name: "user found exact",
wantErr: nil,
searchLogin: "test",
searchEmail: "test@email.com",
},
{
name: "user found non exact - case insensitive",
wantErr: nil,
searchLogin: "Test",
searchEmail: "Test@email.com",
caseInsensitive: true,
name: "user found exact - case insensitive",
wantErr: nil,
searchLogin: "Test",
searchEmail: "Test@email.com",
},
{
name: "user found exact - case insensitive",
wantErr: nil,
searchLogin: "Test",
searchEmail: "Test@email.com",
caseInsensitive: true,
},
{
name: "user not found - case insensitive",
wantErr: user.ErrUserNotFound,
searchLogin: "Test_login",
searchEmail: "Test*@email.com",
caseInsensitive: true,
name: "user not found - case insensitive",
wantErr: user.ErrUserNotFound,
searchLogin: "Test_login",
searchEmail: "Test*@email.com",
},
}
@@ -92,10 +81,9 @@ func TestIntegrationUserGet(t *testing.T) {
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
if !tc.caseInsensitive && db.IsTestDbMySQL() {
if db.IsTestDbMySQL() {
t.Skip("mysql is always case insensitive")
}
cfg.CaseInsensitiveLogin = tc.caseInsensitive
usr, err := userStore.Get(context.Background(),
&user.User{
Email: tc.searchEmail,
@@ -224,7 +212,6 @@ func TestIntegrationUserDataAccess(t *testing.T) {
require.False(t, result.IsDisabled)
t.Run("Get User by email case insensitive", func(t *testing.T) {
userStore.cfg.CaseInsensitiveLogin = true
query := user.GetUserByEmailQuery{Email: "USERtest@TEST.COM"}
result, err := userStore.GetByEmail(context.Background(), &query)
require.Nil(t, err)
@@ -234,8 +221,6 @@ func TestIntegrationUserDataAccess(t *testing.T) {
require.Len(t, result.Rands, 10)
require.Len(t, result.Salt, 10)
require.False(t, result.IsDisabled)
userStore.cfg.CaseInsensitiveLogin = false
})
t.Run("Testing DB - creates and loads user", func(t *testing.T) {
@@ -256,7 +241,6 @@ func TestIntegrationUserDataAccess(t *testing.T) {
require.Len(t, result.Rands, 10)
require.Len(t, result.Salt, 10)
require.False(t, result.IsDisabled)
ss.Cfg.CaseInsensitiveLogin = false
})
})
@@ -264,43 +248,104 @@ func TestIntegrationUserDataAccess(t *testing.T) {
if ss.GetDBType() == migrator.MySQL {
t.Skip("Skipping on MySQL due to case insensitive indexes")
}
userStore.cfg.CaseInsensitiveLogin = true
cmd := user.CreateUserCommand{
Email: "confusertest@test.com",
Name: "user name",
Login: "user_email_conflict",
}
// userEmailConflict
_, err = usrSvc.Create(context.Background(), &cmd)
testOrgID := int64(1)
err := ss.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
// create a user
// add additional user with conflicting login where DOMAIN is upper case
cmd := user.User{
Email: "confusertest@test.com",
Name: "user name",
Login: "user_email_conflict",
OrgID: testOrgID,
Created: time.Now(),
Updated: time.Now(),
}
rawSQL := fmt.Sprintf(
"INSERT INTO %s (email, login, org_id, version, is_admin, created, updated) VALUES (?,?,?,0,%s,?,?)",
ss.Quote("user"),
ss.GetDialect().BooleanStr(false),
)
_, err := sess.Exec(rawSQL, cmd.Email, cmd.Login, cmd.OrgID, cmd.Created, cmd.Updated)
if err != nil {
return err
}
return nil
})
require.NoError(t, err)
err = ss.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
// create a user
// add additional user with conflicting login where DOMAIN is upper case
cmd := user.User{
Email: "confusertest@TEST.COM",
Name: "user name",
Login: "user_email_conflict_two",
OrgID: testOrgID,
Created: time.Now(),
Updated: time.Now(),
}
rawSQL := fmt.Sprintf(
"INSERT INTO %s (email, login, org_id, version, is_admin, created, updated) VALUES (?,?,?,0,%s,?,?)",
ss.Quote("user"),
ss.GetDialect().BooleanStr(false),
)
_, err := sess.Exec(rawSQL, cmd.Email, cmd.Login, cmd.OrgID, cmd.Created, cmd.Updated)
if err != nil {
return err
}
return nil
})
require.NoError(t, err)
cmd = user.CreateUserCommand{
Email: "confusertest@TEST.COM",
Name: "user name",
Login: "user_email_conflict_two",
}
_, err := usrSvc.Create(context.Background(), &cmd)
err = ss.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
// create a user
// add additional user with conflicting login where DOMAIN is upper case
// userLoginConflict
cmd := user.User{
Email: "user_test_login_conflict@test.com",
Name: "user name",
Login: "user_test_login_conflict",
OrgID: testOrgID,
Created: time.Now(),
Updated: time.Now(),
}
rawSQL := fmt.Sprintf(
"INSERT INTO %s (email, login, org_id, version, is_admin, created, updated) VALUES (?,?,?,0,%s,?,?)",
ss.Quote("user"),
ss.GetDialect().BooleanStr(false),
)
_, err := sess.Exec(rawSQL, cmd.Email, cmd.Login, cmd.OrgID, cmd.Created, cmd.Updated)
if err != nil {
return err
}
return nil
})
require.NoError(t, err)
cmd = user.CreateUserCommand{
Email: "user_test_login_conflict@test.com",
Name: "user name",
Login: "user_test_login_conflict",
}
// userLoginConflict
_, err = usrSvc.Create(context.Background(), &cmd)
err = ss.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
// create a user
// add additional user with conflicting login where DOMAIN is upper case
// userLoginConflict
cmd := user.User{
Email: "user_test_login_conflict_two@test.com",
Name: "user name",
Login: "user_test_login_CONFLICT",
OrgID: testOrgID,
Created: time.Now(),
Updated: time.Now(),
}
rawSQL := fmt.Sprintf(
"INSERT INTO %s (email, login, org_id, version, is_admin, created, updated) VALUES (?,?,?,0,%s,?,?)",
ss.Quote("user"),
ss.GetDialect().BooleanStr(false),
)
_, err := sess.Exec(rawSQL, cmd.Email, cmd.Login, cmd.OrgID, cmd.Created, cmd.Updated)
if err != nil {
return err
}
return nil
})
require.NoError(t, err)
cmd = user.CreateUserCommand{
Email: "user_test_login_conflict_two@test.com",
Name: "user name",
Login: "user_test_login_CONFLICT",
}
_, err = usrSvc.Create(context.Background(), &cmd)
require.NoError(t, err)
ss.Cfg.CaseInsensitiveLogin = true
t.Run("GetByEmail - email conflict", func(t *testing.T) {
query := user.GetUserByEmailQuery{Email: "confusertest@test.com"}
_, err = userStore.GetByEmail(context.Background(), &query)
@@ -371,8 +416,6 @@ func TestIntegrationUserDataAccess(t *testing.T) {
require.NotEqual(t, user2.Login, result.Login)
require.NotEqual(t, user2.Name, result.Name)
})
ss.Cfg.CaseInsensitiveLogin = false
})
t.Run("Change user password", func(t *testing.T) {
@@ -880,8 +923,6 @@ func TestIntegrationUserUpdate(t *testing.T) {
}
})
userStore.cfg.CaseInsensitiveLogin = true
t.Run("Testing DB - update generates duplicate user", func(t *testing.T) {
err := userStore.Update(context.Background(), &user.UpdateUserCommand{
Login: "loginuser2",
@@ -926,8 +967,6 @@ func TestIntegrationUserUpdate(t *testing.T) {
require.Equal(t, "loginUSER3", result.Login)
require.Equal(t, "USER3@test.com", result.Email)
})
ss.Cfg.CaseInsensitiveLogin = false
}
func createFiveTestUsers(t *testing.T, svc user.Service, fn func(i int) *user.CreateUserCommand) []user.User {
pkg/services/user/userimpl/user.go
+6 -15
View File
@@ -71,16 +71,11 @@ func ProvideService(
func (s *Service) GetUsageStats(ctx context.Context) map[string]any {
stats := map[string]any{}
caseInsensitiveLoginVal := 0
basicAuthStrongPasswordPolicyVal := 0
if s.cfg.CaseInsensitiveLogin {
caseInsensitiveLoginVal = 1
}
if s.cfg.BasicAuthStrongPasswordPolicy {
basicAuthStrongPasswordPolicyVal = 1
}
stats["stats.case_insensitive_login.count"] = caseInsensitiveLoginVal
stats["stats.password_policy.count"] = basicAuthStrongPasswordPolicyVal
count, err := s.store.CountUserAccountsWithEmptyRole(ctx)
@@ -132,7 +127,7 @@ func (s *Service) Create(ctx context.Context, cmd *user.CreateUserCommand) (*use
cmd.Email = cmd.Login
}
err = s.store.LoginConflict(ctx, cmd.Login, cmd.Email, s.cfg.CaseInsensitiveLogin)
err = s.store.LoginConflict(ctx, cmd.Login, cmd.Email)
if err != nil {
return nil, user.ErrUserAlreadyExists
}
@@ -218,10 +213,8 @@ func (s *Service) GetByID(ctx context.Context, query *user.GetUserByIDQuery) (*u
if err != nil {
return nil, err
}
if s.cfg.CaseInsensitiveLogin {
if err := s.store.CaseInsensitiveLoginConflict(ctx, user.Login, user.Email); err != nil {
return nil, err
}
if err := s.store.CaseInsensitiveLoginConflict(ctx, user.Login, user.Email); err != nil {
return nil, err
}
return user, nil
}
@@ -235,10 +228,8 @@ func (s *Service) GetByEmail(ctx context.Context, query *user.GetUserByEmailQuer
}
func (s *Service) Update(ctx context.Context, cmd *user.UpdateUserCommand) error {
if s.cfg.CaseInsensitiveLogin {
cmd.Login = strings.ToLower(cmd.Login)
cmd.Email = strings.ToLower(cmd.Email)
}
cmd.Login = strings.ToLower(cmd.Login)
cmd.Email = strings.ToLower(cmd.Email)
return s.store.Update(ctx, cmd)
}
@@ -404,7 +395,7 @@ func readQuotaConfig(cfg *setting.Cfg) (*quota.Map, error) {
// CreateServiceAccount creates a service account in the user table and adds service account to an organisation in the org_user table
func (s *Service) CreateServiceAccount(ctx context.Context, cmd *user.CreateUserCommand) (*user.User, error) {
cmd.Email = cmd.Login
err := s.store.LoginConflict(ctx, cmd.Login, cmd.Email, s.cfg.CaseInsensitiveLogin)
err := s.store.LoginConflict(ctx, cmd.Login, cmd.Email)
if err != nil {
return nil, serviceaccounts.ErrServiceAccountAlreadyExists.Errorf("service account with login %s already exists", cmd.Login)
}
pkg/services/user/userimpl/user_test.go
+2 -17
View File
@@ -50,18 +50,6 @@ func TestUserService(t *testing.T) {
t.Run("get user by ID", func(t *testing.T) {
userService.cfg = setting.NewCfg()
userService.cfg.CaseInsensitiveLogin = false
userStore.ExpectedUser = &user.User{ID: 1, Email: "email", Login: "login", Name: "name"}
u, err := userService.GetByID(context.Background(), &user.GetUserByIDQuery{ID: 1})
require.NoError(t, err)
require.Equal(t, "login", u.Login)
require.Equal(t, "name", u.Name)
require.Equal(t, "email", u.Email)
})
t.Run("get user by ID with case insensitive login", func(t *testing.T) {
userService.cfg = setting.NewCfg()
userService.cfg.CaseInsensitiveLogin = true
userStore.ExpectedUser = &user.User{ID: 1, Email: "email", Login: "login", Name: "name"}
u, err := userService.GetByID(context.Background(), &user.GetUserByIDQuery{ID: 1})
require.NoError(t, err)
@@ -99,7 +87,6 @@ func TestUserService(t *testing.T) {
})
t.Run("GetByID - email conflict", func(t *testing.T) {
userService.cfg.CaseInsensitiveLogin = true
userStore.ExpectedError = errors.New("email conflict")
query := user.GetUserByIDQuery{}
_, err := userService.GetByID(context.Background(), &query)
@@ -218,14 +205,12 @@ func TestMetrics(t *testing.T) {
userStore.ExpectedCountUserAccountsWithEmptyRoles = int64(1)
userService.cfg = setting.NewCfg()
userService.cfg.CaseInsensitiveLogin = true
userService.cfg.BasicAuthStrongPasswordPolicy = true
stats := userService.GetUsageStats(context.Background())
assert.NotEmpty(t, stats)
assert.Len(t, stats, 3, stats)
assert.Equal(t, 1, stats["stats.case_insensitive_login.count"])
assert.Len(t, stats, 2, stats)
assert.Equal(t, int64(1), stats["stats.user.role_none.count"])
assert.Equal(t, 1, stats["stats.password_policy.count"])
})
@@ -269,7 +254,7 @@ func (f *FakeUserStore) CaseInsensitiveLoginConflict(context.Context, string, st
return f.ExpectedError
}
func (f *FakeUserStore) LoginConflict(context.Context, string, string, bool) error {
func (f *FakeUserStore) LoginConflict(context.Context, string, string) error {
return f.ExpectedError
}