Avoid losing token when updating mirror settings (#30429)

Fix #30416. Before (it shows as "Unset" while there's a token): <img width="980" alt="image" src="https://github.com/go-gitea/gitea/assets/9418365/d7148e3e-62c9-4d2e-942d-3d795b79515a"> After: <img width="977" alt="image" src="https://github.com/go-gitea/gitea/assets/9418365/24aaa1db-5baa-4204-9081-470b15ea72b5"> The username shows as "oauth2" because of https://github.com/go-gitea/gitea/blob/f9fdac9809335729b2ac3227b2a5f71a62fc64ad/services/migrations/dump.go#L99 I have checked that all usage of `MirrorRemoteAddress` has been updated. <img width="1806" alt="image" src="https://github.com/go-gitea/gitea/assets/9418365/2f042501-2824-4511-9203-c84a6731a02d"> However, it needs to be checked again when backporting. --------- Co-authored-by: Giteabot <teabot@gitea.io>

Author: wolfogre

modules/templates/util_misc.go

@@ -142,35 +142,39 @@ type remoteAddress struct {
 Password string
 }
 
-func mirrorRemoteAddress(ctx context.Context, m *repo_model.Repository, remoteName string, ignoreOriginalURL bool) remoteAddress {
-a := remoteAddress{}
-
-remoteURL := m.OriginalURL
-if ignoreOriginalURL || remoteURL == "" {
-var err error
-remoteURL, err = git.GetRemoteAddress(ctx, m.RepoPath(), remoteName)
-if err != nil {
-log.Error("GetRemoteURL %v", err)
-return a
-}
+func mirrorRemoteAddress(ctx context.Context, m *repo_model.Repository, remoteName string) remoteAddress {
+ret := remoteAddress{}
+remoteURL, err := git.GetRemoteAddress(ctx, m.RepoPath(), remoteName)
+if err != nil {
+log.Error("GetRemoteURL %v", err)
+return ret
 }
 
 u, err := giturl.Parse(remoteURL)
 if err != nil {
 log.Error("giturl.Parse %v", err)
-return a
+return ret
 }
 
 if u.Scheme != "ssh" && u.Scheme != "file" {
 if u.User != nil {
-a.Username = u.User.Username()
-a.Password, _ = u.User.Password()
+ret.Username = u.User.Username()
+ret.Password, _ = u.User.Password()
 }
-u.User = nil
 }
-a.Address = u.String()
 
-return a
+// The URL stored in the git repo could contain authentication,
+// erase it, or it will be shown in the UI.
+u.User = nil
+ret.Address = u.String()
+// Why not use m.OriginalURL to set ret.Address?
+// It should be OK to use it, since m.OriginalURL should be the same as the authentication-erased URL from the Git repository.
+// However, the old code has already stored authentication in m.OriginalURL when updating mirror settings.
+// That means we need to use "giturl.Parse" for m.OriginalURL again to ensure authentication is erased.
+// Instead of doing this, why not directly use the authentication-erased URL from the Git repository?
+// It should be the same as long as there are no bugs.
+
+return ret
 }
 
 func FilenameIsImage(filename string) bool {

services/mirror/mirror_pull.go

@@ -13,6 +13,7 @@ import (
 system_model "code.gitea.io/gitea/models/system"
 "code.gitea.io/gitea/modules/cache"
 "code.gitea.io/gitea/modules/git"
+giturl "code.gitea.io/gitea/modules/git/url"
 "code.gitea.io/gitea/modules/gitrepo"
 "code.gitea.io/gitea/modules/lfs"
 "code.gitea.io/gitea/modules/log"
@@ -30,10 +31,15 @@ const gitShortEmptySha = "0000000"
 
 // UpdateAddress writes new address to Git repository and database
 func UpdateAddress(ctx context.Context, m *repo_model.Mirror, addr string) error {
+u, err := giturl.Parse(addr)
+if err != nil {
+return fmt.Errorf("invalid addr: %v", err)
+}
+
 remoteName := m.GetRemoteName()
 repoPath := m.GetRepository(ctx).RepoPath()
 // Remove old remote
-_, _, err := git.NewCommand(ctx, "remote", "rm").AddDynamicArguments(remoteName).RunStdString(&git.RunOpts{Dir: repoPath})
+_, _, err = git.NewCommand(ctx, "remote", "rm").AddDynamicArguments(remoteName).RunStdString(&git.RunOpts{Dir: repoPath})
 if err != nil && !strings.HasPrefix(err.Error(), "exit status 128 - fatal: No such remote ") {
 return err
 }
@@ -70,7 +76,9 @@ func UpdateAddress(ctx context.Context, m *repo_model.Mirror, addr string) error
 }
 }
 
-m.Repo.OriginalURL = addr
+// erase authentication before storing in database
+u.User = nil
+m.Repo.OriginalURL = u.String()
 return repo_model.UpdateRepositoryCols(ctx, m.Repo, "original_url")
 }
 

templates/repo/settings/options.tmpl

@@ -156,7 +156,7 @@
 <label for="interval">{{ctx.Locale.Tr "repo.mirror_interval" .MinimumMirrorInterval}}</label>
 <input id="interval" name="interval" value="{{.PullMirror.Interval}}">
 </div>
-{{$address := MirrorRemoteAddress $.Context .Repository .PullMirror.GetRemoteName false}}
+{{$address := MirrorRemoteAddress $.Context .Repository .PullMirror.GetRemoteName}}
 <div class="field {{if .Err_MirrorAddress}}error{{end}}">
 <label for="mirror_address">{{ctx.Locale.Tr "repo.mirror_address"}}</label>
 <input id="mirror_address" name="mirror_address" value="{{$address.Address}}" required>