Avoid losing token when updating mirror settings (#30429) (#30464)

Backport #30429 by @wolfogre

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
f9fdac9809/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: Jason Song <i@wolfogre.com>
(cherry picked from commit 92f4cd9461949940427f789a345a3a51b2de02bf)
This commit is contained in:
Giteabot 2024-04-14 00:57:53 +08:00 committed by Earl Warren
parent fa1b0d46c7
commit 4fe72284fc
No known key found for this signature in database
GPG key ID: 0579CB2928A78A00
3 changed files with 32 additions and 20 deletions

View file

@ -142,35 +142,39 @@ type remoteAddress struct {
Password string Password string
} }
func mirrorRemoteAddress(ctx context.Context, m *repo_model.Repository, remoteName string, ignoreOriginalURL bool) remoteAddress { func mirrorRemoteAddress(ctx context.Context, m *repo_model.Repository, remoteName string) remoteAddress {
a := remoteAddress{} ret := remoteAddress{}
remoteURL, err := git.GetRemoteAddress(ctx, m.RepoPath(), remoteName)
remoteURL := m.OriginalURL if err != nil {
if ignoreOriginalURL || remoteURL == "" { log.Error("GetRemoteURL %v", err)
var err error return ret
remoteURL, err = git.GetRemoteAddress(ctx, m.RepoPath(), remoteName)
if err != nil {
log.Error("GetRemoteURL %v", err)
return a
}
} }
u, err := giturl.Parse(remoteURL) u, err := giturl.Parse(remoteURL)
if err != nil { if err != nil {
log.Error("giturl.Parse %v", err) log.Error("giturl.Parse %v", err)
return a return ret
} }
if u.Scheme != "ssh" && u.Scheme != "file" { if u.Scheme != "ssh" && u.Scheme != "file" {
if u.User != nil { if u.User != nil {
a.Username = u.User.Username() ret.Username = u.User.Username()
a.Password, _ = u.User.Password() 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 { func FilenameIsImage(filename string) bool {

View file

@ -13,6 +13,7 @@ import (
system_model "code.gitea.io/gitea/models/system" system_model "code.gitea.io/gitea/models/system"
"code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/cache"
"code.gitea.io/gitea/modules/git" "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/gitrepo"
"code.gitea.io/gitea/modules/lfs" "code.gitea.io/gitea/modules/lfs"
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
@ -30,10 +31,15 @@ const gitShortEmptySha = "0000000"
// UpdateAddress writes new address to Git repository and database // UpdateAddress writes new address to Git repository and database
func UpdateAddress(ctx context.Context, m *repo_model.Mirror, addr string) error { 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() remoteName := m.GetRemoteName()
repoPath := m.GetRepository(ctx).RepoPath() repoPath := m.GetRepository(ctx).RepoPath()
// Remove old remote // 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 ") { if err != nil && !strings.HasPrefix(err.Error(), "exit status 128 - fatal: No such remote ") {
return err 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") return repo_model.UpdateRepositoryCols(ctx, m.Repo, "original_url")
} }

View file

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