diff --git a/.cspell.json b/.cspell.json index d76b03a6c..be615d7d1 100644 --- a/.cspell.json +++ b/.cspell.json @@ -202,6 +202,7 @@ "typecheck", "Typeflag", "unplugin", + "unsanitize", "Upsert", "urfave", "usecase", diff --git a/server/api/repo.go b/server/api/repo.go index d1a684746..cdb2c6133 100644 --- a/server/api/repo.go +++ b/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()) diff --git a/server/store/datastore/migration/024_unsanitize_org_and_user_names.go b/server/store/datastore/migration/024_unsanitize_org_and_user_names.go new file mode 100644 index 000000000..495580fc3 --- /dev/null +++ b/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 + }, +} diff --git a/server/store/datastore/migration/migration.go b/server/store/datastore/migration/migration.go index d056aad56..a42a5e33e 100644 --- a/server/store/datastore/migration/migration.go +++ b/server/store/datastore/migration/migration.go @@ -52,6 +52,7 @@ var migrationTasks = []*xormigrate.Migration{ &renameTokenFields, &setNewDefaultsForRequireApproval, &removeRepoScm, + &unSanitizeOrgAndUserNames, } var allBeans = []any{ diff --git a/server/store/datastore/org.go b/server/store/datastore/org.go index e9d305d51..2084732a6 100644 --- a/server/store/datastore/org.go +++ b/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)) } diff --git a/server/store/datastore/org_test.go b/server/store/datastore/org_test.go index 5136708f5..d158167aa 100644 --- a/server/store/datastore/org_test.go +++ b/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} diff --git a/server/store/datastore/repo_test.go b/server/store/datastore/repo_test.go index 97ac3723f..03dcb91ae 100644 --- a/server/store/datastore/repo_test.go +++ b/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) { diff --git a/server/store/datastore/user_test.go b/server/store/datastore/user_test.go index 410daa36f..01327b7c7 100644 --- a/server/store/datastore/user_test.go +++ b/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",