Unsanitize user and org names in DB (#4762)

Co-authored-by: Robert Kaussow <mail@thegeeklab.de>
Co-authored-by: Anbraten <6918444+anbraten@users.noreply.github.com>
Co-authored-by: Anbraten <anton@ju60.de>

.cspell.json

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

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/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

@@ -16,7 +16,6 @@ package datastore
 
 import (
 	"fmt"
-	"strings"
 
 	"xorm.io/xorm"
 
@@ -28,8 +27,6 @@ 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")
 	}
@@ -48,8 +45,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,7 +79,6 @@ 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))
 }

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",