Unsanitize user and org names in DB

.cspell.json

@@ -202,6 +202,7 @@
     "typecheck",
     "Typeflag",
     "unplugin",
+    "unsanitize",
     "Upsert",
     "urfave",
     "usecase",

server/api/login.go

@@ -110,7 +110,7 @@ func HandleAuth(c *gin.Context) {
 
 	_forge, err := server.Config.Services.Manager.ForgeByID(forgeID)
 	if err != nil {
-		log.Error().Err(err).Msgf("Cannot get forge by id %d", forgeID)
+		log.Error().Err(err).Msgf("cannot get forge by id %d", forgeID)
 		c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login?error=internal_error")
 		return
 	}
@@ -183,29 +183,34 @@ func HandleAuth(c *gin.Context) {
 	// create or set the user's organization if it isn't linked yet
 	if user.OrgID == 0 {
 		// check if an org with the same name exists already and assign it to the user if it does
-		if org, err := _store.OrgFindByName(user.Login); err == nil && org != nil {
+		// TODO: find the org by name and forgeID directly
+		org, err := _store.OrgFindByName(user.Login)
+		if err != nil && !errors.Is(err, types.RecordNotExist) {
+			log.Error().Err(err).Msgf("cannot get org %s", user.Login)
+			c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login?error=internal_error")
+			return
+		}
+
+		// if an org with the same name exists and both are from the same forge => assign org to the user
+		if err == nil && org != nil && org.ForgeID == forgeID {
 			org.IsUser = true
 			user.OrgID = org.ID
 
 			if err := _store.OrgUpdate(org); err != nil {
-				log.Error().Err(err).Msgf("on user creation, could not mark org as user")
+				log.Error().Err(err).Msgf("on login, could not assign org to user")
 			}
 		}
-		if err != nil && !errors.Is(err, types.RecordNotExist) {
-			log.Error().Err(err).Msgf("cannot get org %s", user.Login)
-			c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login?error=internal_error")
-			return
-		}
 
-		if user.OrgID == 0 {
+		// if no org with the same name exists => create a new org
+		if user.OrgID == 0 || errors.Is(err, types.RecordNotExist) {
 			org := &model.Org{
 				Name:    user.Login,
 				IsUser:  true,
 				Private: false,
 				ForgeID: user.ForgeID,
 			}
 			if err := _store.OrgCreate(org); err != nil {
-				log.Error().Err(err).Msgf("on user creation, could not create org for user")
+				log.Error().Err(err).Msgf("on login, could not create org for user")
 			}
 			user.OrgID = org.ID
 		}
@@ -217,10 +222,18 @@ func HandleAuth(c *gin.Context) {
 			c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login?error=internal_error")
 			return
 		}
+
+		// this should never happen, but if it does we should make admins aware of it
+		if org != nil && org.ForgeID != forgeID {
+			log.Error().Err(err).Msgf("user org is not from the same forge  %s", user.Login)
+			c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login?error=internal_error")
+			return
+		}
+
 		if org != nil && org.Name != user.Login {
 			org.Name = user.Login
 			if err := _store.OrgUpdate(org); err != nil {
-				log.Error().Err(err).Msgf("on user creation, could not mark org as user")
+				log.Error().Err(err).Msgf("on login, could not mark org as user")
 			}
 		}
 	}

server/api/login_test.go

@@ -38,8 +38,9 @@ func TestHandleAuth(t *testing.T) {
 		Admin:         false,
 	}
 	org := &model.Org{
-		ID:   1,
-		Name: user.Login,
+		ID:      1,
+		ForgeID: 1,
+		Name:    user.Login,
 	}
 
 	server.Config.Server.SessionExpires = time.Hour

server/api/repo.go

@@ -120,6 +120,7 @@ func PostRepo(c *gin.Context) {
 
 	// find org of repo
 	var org *model.Org
+	// TODO: find org by name and forge id
 	org, err = _store.OrgFindByName(repo.Owner)
 	if err != nil && !errors.Is(err, types.RecordNotExist) {
 		c.String(http.StatusInternalServerError, err.Error())

server/store/datastore/helper.go

@@ -31,7 +31,7 @@ func wrapGet(exist bool, err error) error {
 		return types.RecordNotExist
 	}
 	if err != nil {
-		// we only ask for the function's name if needed, as it's not as preformatted as to just execute it
+		// we only ask for the function's name if needed for performance reasons
 		fnName := callerName(2)
 		return fmt.Errorf("%s: %w", fnName, err)
 	}
@@ -44,7 +44,7 @@ func wrapDelete(c int64, err error) error {
 		return types.RecordNotExist
 	}
 	if err != nil {
-		// we only ask for the function's name if needed, as it's not as preformatted as to just execute it
+		// we only ask for the function's name if needed for performance reasons
 		fnName := callerName(2)
 		return fmt.Errorf("%s: %w", fnName, err)
 	}

server/store/datastore/migration/024_unsanitize_org_and_user_names.go

@@ -0,0 +1,66 @@
+// Copyright 2025 Woodpecker Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package migration
+
+import (
+	"fmt"
+
+	"src.techknowlogick.com/xormigrate"
+	"xorm.io/builder"
+	"xorm.io/xorm"
+)
+
+var unSanitizeOrgAndUserNames = xormigrate.Migration{
+	ID: "unsanitize-org-and-user-names",
+	MigrateSession: func(sess *xorm.Session) (err error) {
+		type user struct {
+			ID      int64  `xorm:"pk autoincr 'id'"`
+			Login   string `xorm:"TEXT 'login'"`
+			ForgeID int64  `xorm:"forge_id"`
+		}
+
+		type org struct {
+			ID      int64  `xorm:"pk autoincr 'id'"`
+			Name    string `xorm:"TEXT 'name'"`
+			ForgeID int64  `xorm:"forge_id"`
+		}
+
+		if err := sess.Sync(new(user), new(org)); err != nil {
+			return fmt.Errorf("sync new models failed: %w", err)
+		}
+
+		// get all users
+		var users []*user
+		if err := sess.Find(&users); err != nil {
+			return fmt.Errorf("find all repos failed: %w", err)
+		}
+
+		for _, user := range users {
+			userOrg := &org{}
+			_, err := sess.Where("name = ? AND forge_id = ?", user.Login, user.ForgeID).Get(userOrg)
+			if err != nil {
+				return fmt.Errorf("getting org failed: %w", err)
+			}
+
+			if user.Login != userOrg.Name {
+				userOrg.Name = user.Login
+				if _, err := sess.Where(builder.Eq{"id": userOrg.ID}).Cols("Name").Update(userOrg); err != nil {
+					return fmt.Errorf("updating org name failed: %w", err)
+				}
+			}
+		}
+		return nil
+	},
+}

server/store/datastore/migration/migration.go

@@ -52,6 +52,7 @@ var migrationTasks = []*xormigrate.Migration{
 	&renameTokenFields,
 	&setNewDefaultsForRequireApproval,
 	&removeRepoScm,
+	&unSanitizeOrgAndUserNames,
 }
 
 var allBeans = []any{

server/store/datastore/org.go

@@ -28,11 +28,10 @@ func (s storage) OrgCreate(org *model.Org) error {
 }
 
 func (s storage) orgCreate(org *model.Org, sess *xorm.Session) error {
-	// sanitize
-	org.Name = strings.ToLower(org.Name)
 	if org.Name == "" {
 		return fmt.Errorf("org name is empty")
 	}
+
 	// insert
 	_, err := sess.Insert(org)
 	return err
@@ -48,8 +47,6 @@ func (s storage) OrgUpdate(org *model.Org) error {
 }
 
 func (s storage) orgUpdate(sess *xorm.Session, org *model.Org) error {
-	// sanitize
-	org.Name = strings.ToLower(org.Name)
 	// update
 	_, err := sess.ID(org.ID).AllCols().Update(org)
 	return err
@@ -84,9 +81,8 @@ func (s storage) OrgFindByName(name string) (*model.Org, error) {
 
 func (s storage) orgFindByName(sess *xorm.Session, name string) (*model.Org, error) {
 	// sanitize
-	name = strings.ToLower(name)
 	org := new(model.Org)
-	return org, wrapGet(sess.Where("name = ?", name).Get(org))
+	return org, wrapGet(sess.Where("LOWER(name) = ?", strings.ToLower(name)).Get(org))
 }
 
 func (s storage) OrgRepoList(org *model.Org, p *model.ListOptions) ([]*model.Repo, error) {

server/store/datastore/org_test.go

@@ -34,7 +34,10 @@ func TestOrgCRUD(t *testing.T) {
 
 	// create first org to play with
 	assert.NoError(t, store.OrgCreate(org1))
-	assert.EqualValues(t, "someawesomeorg", org1.Name)
+	assert.EqualValues(t, "someAwesomeOrg", org1.Name)
+
+	// don't allow the same name in different casing
+	assert.Error(t, store.OrgCreate(&model.Org{ID: org1.ID, Name: "someawesomeorg"}))
 
 	// retrieve it
 	orgOne, err := store.OrgGet(org1.ID)
@@ -44,17 +47,14 @@ func TestOrgCRUD(t *testing.T) {
 	// change name
 	assert.NoError(t, store.OrgUpdate(&model.Org{ID: org1.ID, Name: "RenamedOrg"}))
 
-	// force a name duplication and fail
-	assert.Error(t, store.OrgCreate(&model.Org{Name: "reNamedorg"}))
-
 	// find updated org by name
-	orgOne, err = store.OrgFindByName("renamedorG")
+	orgOne, err = store.OrgFindByName("RenamedOrg")
 	assert.NoError(t, err)
 	assert.NotEqualValues(t, org1, orgOne)
 	assert.EqualValues(t, org1.ID, orgOne.ID)
 	assert.EqualValues(t, false, orgOne.IsUser)
 	assert.EqualValues(t, false, orgOne.Private)
-	assert.EqualValues(t, "renamedorg", orgOne.Name)
+	assert.EqualValues(t, "RenamedOrg", orgOne.Name)
 
 	// create two more orgs and repos
 	someUser := &model.Org{Name: "some_other_u", IsUser: true}

server/store/datastore/repo_test.go

@@ -110,14 +110,6 @@ func TestGetRepoName(t *testing.T) {
 	assert.Equal(t, repo.UserID, getrepo.UserID)
 	assert.Equal(t, repo.Owner, getrepo.Owner)
 	assert.Equal(t, repo.Name, getrepo.Name)
-
-	// case-insensitive
-	getrepo, err = store.GetRepoName("Bradrydzewski/test")
-	assert.NoError(t, err)
-	assert.Equal(t, repo.ID, getrepo.ID)
-	assert.Equal(t, repo.UserID, getrepo.UserID)
-	assert.Equal(t, repo.Owner, getrepo.Owner)
-	assert.Equal(t, repo.Name, getrepo.Name)
 }
 
 func TestRepoList(t *testing.T) {

server/store/datastore/user_test.go

@@ -61,8 +61,8 @@ func TestUsers(t *testing.T) {
 
 	// check unique login
 	user2 := model.User{
-		Login:       "joe",
-		Email:       "foo@bar.com",
+		Login:       "Joe",
+		Email:       "foo2@bar.com",
 		AccessToken: "ab20g0ddaf012c744e136da16aa21ad9",
 	}
 	err2 = store.CreateUser(&user2)
@@ -102,13 +102,13 @@ func TestCreateUserWithExistingOrg(t *testing.T) {
 	existingOrg := &model.Org{
 		ForgeID: 1,
 		IsUser:  true,
-		Name:    "existingorg",
+		Name:    "existingOrg",
 		Private: false,
 	}
 
 	err := store.OrgCreate(existingOrg)
 	assert.NoError(t, err)
-	assert.EqualValues(t, "existingorg", existingOrg.Name)
+	assert.EqualValues(t, "existingOrg", existingOrg.Name)
 
 	// Create a new user with the same name as the existing organization
 	newUser := &model.User{
@@ -120,7 +120,7 @@ func TestCreateUserWithExistingOrg(t *testing.T) {
 
 	updatedOrg, err := store.OrgGet(existingOrg.ID)
 	assert.NoError(t, err)
-	assert.Equal(t, "existingorg", updatedOrg.Name)
+	assert.Equal(t, "existingOrg", updatedOrg.Name)
 
 	newUser2 := &model.User{
 		Login: "new-user",