Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v11.0.x] Auth: Force lowercase login/email for users #86985

Merged
merged 1 commit into from May 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 5 additions & 2 deletions pkg/services/sqlstore/migrations/user_mig.go
Expand Up @@ -5,7 +5,7 @@ import (

"xorm.io/xorm"

"github.com/grafana/grafana/pkg/services/sqlstore/migrations/user"
"github.com/grafana/grafana/pkg/services/sqlstore/migrations/usermig"
. "github.com/grafana/grafana/pkg/services/sqlstore/migrator"
"github.com/grafana/grafana/pkg/util"
)
Expand Down Expand Up @@ -157,7 +157,10 @@ func addUserMigrations(mg *Migrator) {

// Service accounts login were not unique per org. this migration is part of making it unique per org
// to be able to create service accounts that are unique per org
mg.AddMigration(user.AllowSameLoginCrossOrgs, &user.ServiceAccountsSameLoginCrossOrgs{})
mg.AddMigration(usermig.AllowSameLoginCrossOrgs, &usermig.ServiceAccountsSameLoginCrossOrgs{})

// Users login and email should be in lower case
mg.AddMigration(usermig.LowerCaseUserLoginAndEmail, &usermig.UsersLowerCaseLoginAndEmail{})
}

const migSQLITEisServiceAccountNullable = `ALTER TABLE user ADD COLUMN tmp_service_account BOOLEAN DEFAULT 0;
Expand Down
@@ -1,4 +1,4 @@
package user
package usermig

import (
"fmt"
Expand Down
Expand Up @@ -8,7 +8,7 @@ import (
"github.com/stretchr/testify/require"

"github.com/grafana/grafana/pkg/infra/log"
usermig "github.com/grafana/grafana/pkg/services/sqlstore/migrations/user"
"github.com/grafana/grafana/pkg/services/sqlstore/migrations/usermig"
"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
Expand Down
@@ -0,0 +1,253 @@
package test

import (
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/services/sqlstore/migrations/usermig"
"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
)

func TestLowerCaseMigration(t *testing.T) {
type migrationTestCase struct {
desc string
users []*user.User
wantUsers []*user.User
}
testCases := []migrationTestCase{
{
desc: "basic case updates login and email to lowercase",
users: []*user.User{
{
ID: 1,
UID: "u1",
Login: "User1",
Email: "USER1@domain.com",
Name: "user1",
OrgID: 1,
Created: now,
Updated: now,
},
{
ID: 2,
UID: "u2",
Login: "User2",
Email: "USER2@domain.com",
Name: "user2",
OrgID: 1,
Created: now,
Updated: now,
},
},
wantUsers: []*user.User{
{
ID: 1,
Login: "user1",
Email: "user1@domain.com",
},
{
ID: 2,
Login: "user2",
Email: "user2@domain.com",
},
},
},
// "2 users - same login, one already lowercase"
{
desc: "2 users with same login one already has lowercase so we keep both",
users: []*user.User{
{
ID: 1,
UID: "u1",
Login: "user1",
Email: "user1@email.com",
Name: "user1",
OrgID: 1,
Created: now,
Updated: now,
},
{
ID: 2,
UID: "u2",
Login: "User1",
Email: "user1-new@email.com",
Name: "user2",
OrgID: 1,
Created: now,
Updated: now,
},
},
wantUsers: []*user.User{
{
ID: 1,
Login: "user1",
Email: "user1@email.com",
},
{
ID: 2,
Login: "User1",
Email: "user1-new@email.com",
},
},
},
// "2 users - same login, one already lowercase"
{
desc: "2 users with same login one already has lowercase so we keep both case for uppercasing comes first in our loop",
users: []*user.User{
{
ID: 1,
UID: "u1",
Login: "User1",
Email: "user1@email.com",
Name: "user1",
OrgID: 1,
Created: now,
Updated: now,
},
{
ID: 2,
UID: "u2",
Login: "user1",
Email: "user1-new@email.com",
Name: "user2",
OrgID: 1,
Created: now,
Updated: now,
},
},
wantUsers: []*user.User{
{
ID: 1,
Login: "User1",
Email: "user1@email.com",
},
{
ID: 2,
Login: "user1",
Email: "user1-new@email.com",
},
},
},
// "2 users - same email, one already lowercase"
{
desc: "2 users with same email one already has lowercase so we keep both",
users: []*user.User{
{
ID: 1,
UID: "u1",
Login: "user1",
Email: "USER1@email.com",
Name: "user1",
OrgID: 1,
Created: now,
Updated: now,
},
{
ID: 2,
UID: "u2",
Login: "user1-new-login",
Email: "user1@email.com",
Name: "user2",
OrgID: 1,
Created: now,
Updated: now,
},
},
wantUsers: []*user.User{
{
ID: 1,
Login: "user1",
Email: "USER1@email.com",
},
{
ID: 2,
Login: "user1-new-login",
Email: "user1@email.com",
},
},
},

// "2 users - same login, none lowercase"
{
desc: "2 users with same login noone is lowercased we pick the most recent user to lowercase",
users: []*user.User{
{
ID: 1,
UID: "u1",
Login: "USER1",
Email: "user1@mail.com",
Name: "user1",
OrgID: 1,
Created: now.Add(-1 * time.Hour),
Updated: now.Add(-1 * time.Hour),
},
{
ID: 2,
UID: "u2",
Login: "User1",
Email: "user1-new@mail.com",
Name: "user2",
OrgID: 1,
Created: now,
Updated: now,
},
},
wantUsers: []*user.User{
{
ID: 1,
Login: "user1",
Email: "user1@mail.com",
},
{
ID: 2,
Login: "User1",
Email: "user1-new@mail.com",
},
},
},
}

for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
// Run initial migration to have a working DB
x := setupTestDB(t)
// Remove migration
_, errDeleteMig := x.Exec(`DELETE FROM migration_log WHERE migration_id = ?`, usermig.LowerCaseUserLoginAndEmail)
require.NoError(t, errDeleteMig)

// insert users
usersCount, err := x.Insert(tc.users)
require.NoError(t, err)
require.Equal(t, int64(len(tc.users)), usersCount)

// run the migration
usermigrator := migrator.NewMigrator(x, &setting.Cfg{Logger: log.New("usermigration.test")})
usermig.AddLowerCaseUserLoginAndEmail(usermigrator)
errRunningMig := usermigrator.Start(false, 0)
require.NoError(t, errRunningMig)

// Check users
resultingUsers := []user.User{}
err = x.Table("user").Find(&resultingUsers)
require.NoError(t, err)

// Check that the users have been updated
require.Equal(t, len(tc.wantUsers), len(resultingUsers))

for i := range tc.wantUsers {
for _, u := range resultingUsers {
if u.ID == tc.wantUsers[i].ID {
assert.Equal(t, tc.wantUsers[i].Login, u.Login)
assert.Equal(t, tc.wantUsers[i].Email, u.Email)
}
}
}
})
}
}
@@ -0,0 +1,97 @@
package usermig

import (
"strings"

"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
"github.com/grafana/grafana/pkg/services/user"
"xorm.io/xorm"
)

const (
LowerCaseUserLoginAndEmail = "update login and email fields to lowercase"
)

// AddLowerCaseUserLoginAndEmail adds a migration that updates the login and email fields of all users to be in lower case.
func AddLowerCaseUserLoginAndEmail(mg *migrator.Migrator) {
mg.AddMigration(LowerCaseUserLoginAndEmail, &UsersLowerCaseLoginAndEmail{})
}

var _ migrator.CodeMigration = new(UsersLowerCaseLoginAndEmail)

type UsersLowerCaseLoginAndEmail struct {
migrator.MigrationBase
}

func (p *UsersLowerCaseLoginAndEmail) SQL(dialect migrator.Dialect) string {
return "code migration"
}

func (p *UsersLowerCaseLoginAndEmail) Exec(sess *xorm.Session, mg *migrator.Migrator) error {
// Get all users
users := make([]*user.User, 0)
err := sess.Table("user").Find(&users)
if err != nil {
return err
}
processedLogins := make(map[string]bool)
processedEmails := make(map[string]bool)

for _, usr := range users {
/*
LOGIN
*/
lowerLogin := strings.ToLower(usr.Login)
// only work through if login is not already in lower case
if usr.Login != lowerLogin && !processedLogins[lowerLogin] {
// Check if lower login exists
existingLowerCasedUserLogin := &user.User{}

// lowercaseexists in database
hasLowerCasedLogin, err := sess.Table("user").Where("login = ?", lowerLogin).Get(existingLowerCasedUserLogin)
if err != nil {
return err
}

// If exact login does not exist and lower case login does not exist, update the user's login to be in lower case
if !hasLowerCasedLogin {
uLogin := user.User{
Name: usr.Name,
Login: lowerLogin,
}
_, err := sess.ID(usr.ID).Update(&uLogin)
if err != nil {
return err
}
}
}
processedLogins[lowerLogin] = true

/*
EMAIL
*/
lowerEmail := strings.ToLower(usr.Email)
// only work through if email is not already in lower case
if usr.Email != lowerEmail && !processedEmails[lowerEmail] {
// Check if lower case email exists
existingUserEmail := &user.User{}
hasLowerCasedEmail, err := sess.Table("user").Where("email = ?", lowerEmail).Get(existingUserEmail)
if err != nil {
return err
}
// If lower case email does not exist, update the user's email to be in lower case
if !hasLowerCasedEmail {
uEmail := user.User{
Name: usr.Name,
Email: lowerEmail,
}
_, err := sess.ID(usr.ID).Update(&uEmail)
if err != nil {
return err
}
}
}
processedEmails[lowerEmail] = true
}
return nil
}