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>

diff --git a/modules/templates/util_misc.go b/modules/templates/util_misc.go
index 6c1b4ab240e34993d1536ee8d30c0f01c473be4c..774385483b4c66693ed46f1591dd16608120d152 100644 (file)
--- a/modules/templates/util_misc.go
+++ b/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 {

diff --git a/services/mirror/mirror_pull.go b/services/mirror/mirror_pull.go
index 21d5f08205dedb100941e44f8d300287b2e6b686..fa23986c54eaed86cc8363d2464a759f2c143c5e 100644 (file)
--- a/services/mirror/mirror_pull.go
+++ b/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")
 }
 

diff --git a/templates/repo/settings/options.tmpl b/templates/repo/settings/options.tmpl
index b8fa4759b1326d0a33bc9af77b658b6e4d09e4c7..df6ccbf6bc2fbd5679ff53553858fa2cdafef029 100644 (file)
--- a/templates/repo/settings/options.tmpl
+++ b/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>