From 63eb22d74b42df1ab96624dc3a1e78e5d3915cd8 Mon Sep 17 00:00:00 2001 From: icefed Date: Tue, 5 Mar 2024 20:50:09 +0800 Subject: [PATCH] =?UTF-8?q?Fix:=20=E2=80=98autoRedirect=E2=80=99=20hardcod?= =?UTF-8?q?e=20=E2=80=98https=E2=80=99=20scheme?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: icefed --- docs/content/about/configuration.md | 3 +- registry/auth/token/accesscontroller.go | 94 ++++++++++++++------ registry/auth/token/accesscontroller_test.go | 89 ++++++++++++++++++ 3 files changed, 157 insertions(+), 29 deletions(-) create mode 100644 registry/auth/token/accesscontroller_test.go diff --git a/docs/content/about/configuration.md b/docs/content/about/configuration.md index 482a40ca7..75fb79c56 100644 --- a/docs/content/about/configuration.md +++ b/docs/content/about/configuration.md @@ -591,7 +591,8 @@ security. | `service` | yes | The service being authenticated. | | `issuer` | yes | The name of the token issuer. The issuer inserts this into the token so it must match the value configured for the issuer. | | `rootcertbundle` | yes | The absolute path to the root certificate bundle. This bundle contains the public part of the certificates used to sign authentication tokens. | -| `autoredirect` | no | When set to `true`, `realm` will automatically be set using the Host header of the request as the domain and a path of `/auth/token/`| +| `autoredirect` | no | When set to `true`, `realm` will automatically be set using the Host header of the request as the domain and a path of `/auth/token/`(or specified by `autoredirectpath`), the `realm` URL Scheme will use `X-Forwarded-Proto` header if set, otherwise it will be set to `https`. | +| `autoredirectpath` | no | The path to redirect to if `autoredirect` is set to `true`, default: `/auth/token/`. | For more information about Token based authentication configuration, see the diff --git a/registry/auth/token/accesscontroller.go b/registry/auth/token/accesscontroller.go index 8f610c46d..4d54248b1 100644 --- a/registry/auth/token/accesscontroller.go +++ b/registry/auth/token/accesscontroller.go @@ -9,6 +9,7 @@ import ( "fmt" "io" "net/http" + "net/url" "os" "strings" @@ -83,11 +84,12 @@ var ( // authChallenge implements the auth.Challenge interface. type authChallenge struct { - err error - realm string - autoRedirect bool - service string - accessSet accessSet + err error + realm string + autoRedirect bool + autoRedirectPath string + service string + accessSet accessSet } var _ auth.Challenge = authChallenge{} @@ -102,13 +104,28 @@ func (ac authChallenge) Status() int { return http.StatusUnauthorized } +func buildAutoRedirectURL(r *http.Request, autoRedirectPath string) string { + scheme := "https" + + if forwardedProto := r.Header.Get("X-Forwarded-Proto"); len(forwardedProto) > 0 { + scheme = forwardedProto + } + + u := &url.URL{ + Scheme: scheme, + Host: r.Host, + Path: autoRedirectPath, + } + return u.String() +} + // challengeParams constructs the value to be used in // the WWW-Authenticate response challenge header. // See https://tools.ietf.org/html/rfc6750#section-3 func (ac authChallenge) challengeParams(r *http.Request) string { var realm string if ac.autoRedirect { - realm = fmt.Sprintf("https://%s/auth/token", r.Host) + realm = buildAutoRedirectURL(r, ac.autoRedirectPath) } else { realm = ac.realm } @@ -134,23 +151,29 @@ func (ac authChallenge) SetHeaders(r *http.Request, w http.ResponseWriter) { // accessController implements the auth.AccessController interface. type accessController struct { - realm string - autoRedirect bool - issuer string - service string - rootCerts *x509.CertPool - trustedKeys map[string]crypto.PublicKey + realm string + autoRedirect bool + autoRedirectPath string + issuer string + service string + rootCerts *x509.CertPool + trustedKeys map[string]crypto.PublicKey } +const ( + defaultAutoRedirectPath = "/auth/token" +) + // tokenAccessOptions is a convenience type for handling // options to the contstructor of an accessController. type tokenAccessOptions struct { - realm string - autoRedirect bool - issuer string - service string - rootCertBundle string - jwks string + realm string + autoRedirect bool + autoRedirectPath string + issuer string + service string + rootCertBundle string + jwks string } // checkOptions gathers the necessary options @@ -187,6 +210,19 @@ func checkOptions(options map[string]interface{}) (tokenAccessOptions, error) { } opts.autoRedirect = autoRedirect } + if opts.autoRedirect { + autoRedirectPathVal, ok := options["autoredirectpath"] + if ok { + autoRedirectPath, ok := autoRedirectPathVal.(string) + if !ok { + return opts, fmt.Errorf("token auth requires a valid option string: autoredirectpath") + } + opts.autoRedirectPath = autoRedirectPath + } + if opts.autoRedirectPath == "" { + opts.autoRedirectPath = defaultAutoRedirectPath + } + } return opts, nil } @@ -287,12 +323,13 @@ func newAccessController(options map[string]interface{}) (auth.AccessController, } return &accessController{ - realm: config.realm, - autoRedirect: config.autoRedirect, - issuer: config.issuer, - service: config.service, - rootCerts: rootPool, - trustedKeys: trustedKeys, + realm: config.realm, + autoRedirect: config.autoRedirect, + autoRedirectPath: config.autoRedirectPath, + issuer: config.issuer, + service: config.service, + rootCerts: rootPool, + trustedKeys: trustedKeys, }, nil } @@ -300,10 +337,11 @@ func newAccessController(options map[string]interface{}) (auth.AccessController, // for actions on resources described by the given access items. func (ac *accessController) Authorized(req *http.Request, accessItems ...auth.Access) (*auth.Grant, error) { challenge := &authChallenge{ - realm: ac.realm, - autoRedirect: ac.autoRedirect, - service: ac.service, - accessSet: newAccessSet(accessItems...), + realm: ac.realm, + autoRedirect: ac.autoRedirect, + autoRedirectPath: ac.autoRedirectPath, + service: ac.service, + accessSet: newAccessSet(accessItems...), } prefix, rawToken, ok := strings.Cut(req.Header.Get("Authorization"), " ") diff --git a/registry/auth/token/accesscontroller_test.go b/registry/auth/token/accesscontroller_test.go new file mode 100644 index 000000000..fde203114 --- /dev/null +++ b/registry/auth/token/accesscontroller_test.go @@ -0,0 +1,89 @@ +package token + +import ( + "net/http" + "net/http/httptest" + "testing" +) + +func TestBuildAutoRedirectURL(t *testing.T) { + cases := []struct { + name string + reqGetter func() *http.Request + autoRedirectPath string + expectedURL string + }{{ + name: "http", + reqGetter: func() *http.Request { + req := httptest.NewRequest("GET", "http://example.com/", nil) + return req + }, + autoRedirectPath: "/auth", + expectedURL: "https://example.com/auth", + }, { + name: "x-forwarded", + reqGetter: func() *http.Request { + req := httptest.NewRequest("GET", "http://example.com/", nil) + req.Header.Set("X-Forwarded-Proto", "http") + return req + }, + autoRedirectPath: "/auth/token", + expectedURL: "http://example.com/auth/token", + }} + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + req := tc.reqGetter() + result := buildAutoRedirectURL(req, tc.autoRedirectPath) + if result != tc.expectedURL { + t.Errorf("expected %s, got %s", tc.expectedURL, result) + } + }) + } +} + +func TestCheckOptions(t *testing.T) { + realm := "https://auth.example.com/token/" + issuer := "test-issuer.example.com" + service := "test-service.example.com" + + options := map[string]interface{}{ + "realm": realm, + "issuer": issuer, + "service": service, + "rootcertbundle": "", + "autoredirect": true, + "autoredirectpath": "/auth", + } + + ta, err := checkOptions(options) + if err != nil { + t.Fatal(err) + } + if ta.autoRedirect != true { + t.Fatal("autoredirect should be true") + } + if ta.autoRedirectPath != "/auth" { + t.Fatal("autoredirectpath should be /auth") + } + + options = map[string]interface{}{ + "realm": realm, + "issuer": issuer, + "service": service, + "rootcertbundle": "", + "autoredirect": true, + "autoredirectforcetlsdisabled": true, + } + + ta, err = checkOptions(options) + if err != nil { + t.Fatal(err) + } + if ta.autoRedirect != true { + t.Fatal("autoredirect should be true") + } + if ta.autoRedirectPath != "/auth/token" { + t.Fatal("autoredirectpath should be /auth/token") + } +}