From 0386b801a9f20ae01091e4de3f3a932b9d1beb99 Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Wed, 18 Dec 2019 22:21:53 -0500 Subject: [PATCH 1/5] Add migration to sanitize repository original_url During a large code move in #6200 the OriginalURL field was accidentially changed to be populated with the CloneAddr field which will contain the username and/or password provided during a migration. This behavior was fixed in previous PR #9097 and this migration will remove any authentication details that were stored in the database between those two. --- models/migrations/migrations.go | 2 ++ models/migrations/v114.go | 60 +++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) create mode 100644 models/migrations/v114.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index cbea5a95dd5f..b05d2a484081 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -282,6 +282,8 @@ var migrations = []Migration{ NewMigration("remove release attachments which repository deleted", removeAttachmentMissedRepo), // v113 -> v114 NewMigration("new feature: change target branch of pull requests", featureChangeTargetBranch), + // v113 -> v114 + NewMigration("Remove authentication credentials from stored URL", sanitizeOriginalURL), } // Migrate database to current version diff --git a/models/migrations/v114.go b/models/migrations/v114.go new file mode 100644 index 000000000000..f4bf2f99da83 --- /dev/null +++ b/models/migrations/v114.go @@ -0,0 +1,60 @@ +// Copyright 2019 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "net/url" + "strings" + + "xorm.io/xorm" +) + +func sanitizeOriginalURL(x *xorm.Engine) error { + + type Repository struct { + ID int64 + OriginalURL string `xorm:"VARCHAR(2048)"` + } + + sess := x.NewSession() + defer sess.Close() + var last int + const batchSize = 50 + for { + var results = make([]Repository, 0, batchSize) + err := x.Where("original_url <> '' AND original_url IS NOT NULL"). + And("original_service_type = 0 OR original_service_type IS NULL"). + OrderBy("id"). + Limit(batchSize, last). + Find(&results) + if err != nil { + return err + } + if len(results) == 0 { + break + } + last += len(results) + + for _, res := range results { + u, err := url.Parse(res.OriginalURL) + if err != nil { + // it is ok to continue here, we only care about fixing URLs that we can read + continue + } + + if len(u.User.Username()) > 0 { + pass, _ := u.User.Password() + userAuth := u.User.Username() + ":" + pass + "@" + OriginalURL := strings.Replace(res.OriginalURL, userAuth, "", -1) + + _, err = x.Exec("UPDATE repository SET original_url = ? WHERE id = ?", OriginalURL, res.ID) + if err != nil { + return err + } + } + } + } + return nil +} From 6f6022c132c9325e374bd1837d670eda654f77e7 Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Wed, 18 Dec 2019 23:32:51 -0500 Subject: [PATCH 2/5] use net/url to rebuild URL instead of strings.Replace --- models/migrations/v114.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/models/migrations/v114.go b/models/migrations/v114.go index f4bf2f99da83..e4877b9a281e 100644 --- a/models/migrations/v114.go +++ b/models/migrations/v114.go @@ -6,7 +6,6 @@ package migrations import ( "net/url" - "strings" "xorm.io/xorm" ) @@ -45,11 +44,8 @@ func sanitizeOriginalURL(x *xorm.Engine) error { } if len(u.User.Username()) > 0 { - pass, _ := u.User.Password() - userAuth := u.User.Username() + ":" + pass + "@" - OriginalURL := strings.Replace(res.OriginalURL, userAuth, "", -1) - - _, err = x.Exec("UPDATE repository SET original_url = ? WHERE id = ?", OriginalURL, res.ID) + originalURL := u.Scheme + "://" + u.Host + u.Path + _, err = x.Exec("UPDATE repository SET original_url = ? WHERE id = ?", originalURL, res.ID) if err != nil { return err } From 96297a33e6cc145578226e0aadda2c2ff081346c Mon Sep 17 00:00:00 2001 From: mrsdizzie Date: Thu, 19 Dec 2019 00:03:29 -0500 Subject: [PATCH 3/5] Update models/migrations/migrations.go --- models/migrations/migrations.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index b05d2a484081..923b5f5759c1 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -282,7 +282,7 @@ var migrations = []Migration{ NewMigration("remove release attachments which repository deleted", removeAttachmentMissedRepo), // v113 -> v114 NewMigration("new feature: change target branch of pull requests", featureChangeTargetBranch), - // v113 -> v114 + // v114 -> v115 NewMigration("Remove authentication credentials from stored URL", sanitizeOriginalURL), } From 47a0c875c4d6b22c5e419a2a89c9815bf35802d7 Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Thu, 19 Dec 2019 00:19:07 -0500 Subject: [PATCH 4/5] changes per lunny --- models/migrations/v114.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/models/migrations/v114.go b/models/migrations/v114.go index e4877b9a281e..7777a36f6c3e 100644 --- a/models/migrations/v114.go +++ b/models/migrations/v114.go @@ -17,8 +17,6 @@ func sanitizeOriginalURL(x *xorm.Engine) error { OriginalURL string `xorm:"VARCHAR(2048)"` } - sess := x.NewSession() - defer sess.Close() var last int const batchSize = 50 for { @@ -42,14 +40,12 @@ func sanitizeOriginalURL(x *xorm.Engine) error { // it is ok to continue here, we only care about fixing URLs that we can read continue } - - if len(u.User.Username()) > 0 { - originalURL := u.Scheme + "://" + u.Host + u.Path + u.User = nil + originalURL := u.String() _, err = x.Exec("UPDATE repository SET original_url = ? WHERE id = ?", originalURL, res.ID) if err != nil { return err } - } } } return nil From 728f8f45ff254c142689e4671cfe57c7e7730de2 Mon Sep 17 00:00:00 2001 From: "j. mccann" Date: Thu, 19 Dec 2019 00:29:02 -0500 Subject: [PATCH 5/5] make fmt --- models/migrations/v114.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/models/migrations/v114.go b/models/migrations/v114.go index 7777a36f6c3e..25a187f6e8ab 100644 --- a/models/migrations/v114.go +++ b/models/migrations/v114.go @@ -42,10 +42,10 @@ func sanitizeOriginalURL(x *xorm.Engine) error { } u.User = nil originalURL := u.String() - _, err = x.Exec("UPDATE repository SET original_url = ? WHERE id = ?", originalURL, res.ID) - if err != nil { - return err - } + _, err = x.Exec("UPDATE repository SET original_url = ? WHERE id = ?", originalURL, res.ID) + if err != nil { + return err + } } } return nil