Remove sub-path from container registry realm (#31293) (#31300)

Backport #31293 by wxiaoguang

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

diff --git a/modules/setting/packages.go b/modules/setting/packages.go
index b225615a24012c0de92bf8d0be3ffd9799af986a..00fba67b393ac80dba89593d96f7c1f7e65a6343 100644 (file)
--- a/modules/setting/packages.go
+++ b/modules/setting/packages.go
@@ -6,7 +6,6 @@ package setting
 import (
        "fmt"
        "math"
-       "net/url"
        "os"
        "path/filepath"
 
@@ -19,7 +18,6 @@ var (
                Storage           *Storage
                Enabled           bool
                ChunkedUploadPath string
-               RegistryHost      string
 
                LimitTotalOwnerCount int64
                LimitTotalOwnerSize  int64
@@ -66,9 +64,6 @@ func loadPackagesFrom(rootCfg ConfigProvider) (err error) {
                return err
        }
 
-       appURL, _ := url.Parse(AppURL)
-       Packages.RegistryHost = appURL.Host
-
        Packages.ChunkedUploadPath = filepath.ToSlash(sec.Key("CHUNKED_UPLOAD_PATH").MustString("tmp/package-upload"))
        if !filepath.IsAbs(Packages.ChunkedUploadPath) {
                Packages.ChunkedUploadPath = filepath.ToSlash(filepath.Join(AppDataPath, Packages.ChunkedUploadPath))

diff --git a/modules/test/utils.go b/modules/test/utils.go
index 4a0c2f1b3b298e051986c59be2c5d7d952bd8c96..8dee92fbce49c86be7f2f8ffdd49c4ad77e811c3 100644 (file)
--- a/modules/test/utils.go
+++ b/modules/test/utils.go
@@ -34,8 +34,10 @@ func IsNormalPageCompleted(s string) bool {
        return strings.Contains(s, `<footer class="page-footer"`) && strings.Contains(s, `</html>`)
 }
 
-func MockVariableValue[T any](p *T, v T) (reset func()) {
+func MockVariableValue[T any](p *T, v ...T) (reset func()) {
        old := *p
-       *p = v
+       if len(v) > 0 {
+               *p = v[0]
+       }
        return func() { *p = old }
 }

diff --git a/routers/api/packages/container/container.go b/routers/api/packages/container/container.go
index 2a6d44ba081249e6582b0156e00663b06ff87d5e..b0c4458d5114ee9ae1fe1983556ca54ef1d0e1f4 100644 (file)
--- a/routers/api/packages/container/container.go
+++ b/routers/api/packages/container/container.go
@@ -116,9 +116,9 @@ func apiErrorDefined(ctx *context.Context, err *namedError) {
 }
 
 func apiUnauthorizedError(ctx *context.Context) {
-       // TODO: it doesn't seem quite right but it doesn't really cause problem at the moment.
-       // container registry requires that the "/v2" must be in the root, so the sub-path in AppURL should be removed, ideally.
-       ctx.Resp.Header().Add("WWW-Authenticate", `Bearer realm="`+httplib.GuessCurrentAppURL(ctx)+`v2/token",service="container_registry",scope="*"`)
+       // container registry requires that the "/v2" must be in the root, so the sub-path in AppURL should be removed
+       realmURL := strings.TrimSuffix(httplib.GuessCurrentAppURL(ctx), setting.AppSubURL+"/") + "/v2/token"
+       ctx.Resp.Header().Add("WWW-Authenticate", `Bearer realm="`+realmURL+`",service="container_registry",scope="*"`)
        apiErrorDefined(ctx, errUnauthorized)
 }
 

diff --git a/routers/web/user/package.go b/routers/web/user/package.go
index 2a18796687363268ac3114f5977d90a12d6c70db..dad4c8f602a57b04ea00babf74b3f97326afa346 100644 (file)
--- a/routers/web/user/package.go
+++ b/routers/web/user/package.go
@@ -5,6 +5,7 @@ package user
 
 import (
        "net/http"
+       "net/url"
 
        "code.gitea.io/gitea/models/db"
        org_model "code.gitea.io/gitea/models/organization"
@@ -15,6 +16,7 @@ import (
        repo_model "code.gitea.io/gitea/models/repo"
        "code.gitea.io/gitea/modules/base"
        "code.gitea.io/gitea/modules/container"
+       "code.gitea.io/gitea/modules/httplib"
        "code.gitea.io/gitea/modules/log"
        "code.gitea.io/gitea/modules/optional"
        alpine_module "code.gitea.io/gitea/modules/packages/alpine"
@@ -178,7 +180,11 @@ func ViewPackageVersion(ctx *context.Context) {
 
        switch pd.Package.Type {
        case packages_model.TypeContainer:
-               ctx.Data["RegistryHost"] = setting.Packages.RegistryHost
+               registryAppURL, err := url.Parse(httplib.GuessCurrentAppURL(ctx))
+               if err != nil {
+                       registryAppURL, _ = url.Parse(setting.AppURL)
+               }
+               ctx.Data["RegistryHost"] = registryAppURL.Host
        case packages_model.TypeAlpine:
                branches := make(container.Set[string])
                repositories := make(container.Set[string])

diff --git a/tests/integration/api_packages_container_test.go b/tests/integration/api_packages_container_test.go
index 9ac6e5256b14ccc1aea241662221df6718dc2a1c..fcd1cc529f214d83ab4d40d2f8ed7f01fccce91d 100644 (file)
--- a/tests/integration/api_packages_container_test.go
+++ b/tests/integration/api_packages_container_test.go
@@ -84,7 +84,7 @@ func TestPackageContainer(t *testing.T) {
                        Token string `json:"token"`
                }
 
-               authenticate := []string{`Bearer realm="` + setting.AppURL + `v2/token",service="container_registry",scope="*"`}
+               defaultAuthenticateValues := []string{`Bearer realm="` + setting.AppURL + `v2/token",service="container_registry",scope="*"`}
 
                t.Run("Anonymous", func(t *testing.T) {
                        defer tests.PrintCurrentTest(t)()
@@ -92,7 +92,7 @@ func TestPackageContainer(t *testing.T) {
                        req := NewRequest(t, "GET", fmt.Sprintf("%sv2", setting.AppURL))
                        resp := MakeRequest(t, req, http.StatusUnauthorized)
 
-                       assert.ElementsMatch(t, authenticate, resp.Header().Values("WWW-Authenticate"))
+                       assert.ElementsMatch(t, defaultAuthenticateValues, resp.Header().Values("WWW-Authenticate"))
 
                        req = NewRequest(t, "GET", fmt.Sprintf("%sv2/token", setting.AppURL))
                        resp = MakeRequest(t, req, http.StatusOK)
@@ -115,6 +115,12 @@ func TestPackageContainer(t *testing.T) {
 
                        req = NewRequest(t, "GET", fmt.Sprintf("%sv2/token", setting.AppURL))
                        MakeRequest(t, req, http.StatusUnauthorized)
+
+                       defer test.MockVariableValue(&setting.AppURL, "https://domain:8443/sub-path/")()
+                       defer test.MockVariableValue(&setting.AppSubURL, "/sub-path")()
+                       req = NewRequest(t, "GET", "/v2")
+                       resp = MakeRequest(t, req, http.StatusUnauthorized)
+                       assert.Equal(t, `Bearer realm="https://domain:8443/v2/token",service="container_registry",scope="*"`, resp.Header().Get("WWW-Authenticate"))
                })
 
                t.Run("User", func(t *testing.T) {
@@ -123,7 +129,7 @@ func TestPackageContainer(t *testing.T) {
                        req := NewRequest(t, "GET", fmt.Sprintf("%sv2", setting.AppURL))
                        resp := MakeRequest(t, req, http.StatusUnauthorized)
 
-                       assert.ElementsMatch(t, authenticate, resp.Header().Values("WWW-Authenticate"))
+                       assert.ElementsMatch(t, defaultAuthenticateValues, resp.Header().Values("WWW-Authenticate"))
 
                        req = NewRequest(t, "GET", fmt.Sprintf("%sv2/token", setting.AppURL)).
                                AddBasicAuth(user.Name)