From dc5d145a3f74a512ec51133ef0b7dcf2c8c70e3f Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Mon, 15 Sep 2025 20:46:38 -0700 Subject: [PATCH 1/3] Improve BasicAuth middleware: use strings.Cut and RFC compliance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace manual for loop with strings.Cut for credential parsing - Simplify realm handling to always quote according to RFC 7617 - Improve code readability and maintainability Fixes #2794 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- middleware/basic_auth.go | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/middleware/basic_auth.go b/middleware/basic_auth.go index 9285f29fd..f9efafc5d 100644 --- a/middleware/basic_auth.go +++ b/middleware/basic_auth.go @@ -84,27 +84,21 @@ func BasicAuthWithConfig(config BasicAuthConfig) echo.MiddlewareFunc { } cred := string(b) - for i := 0; i < len(cred); i++ { - if cred[i] == ':' { - // Verify credentials - valid, err := config.Validator(cred[:i], cred[i+1:], c) - if err != nil { - return err - } else if valid { - return next(c) - } - break + user, pass, ok := strings.Cut(cred, ":") + if ok { + // Verify credentials + valid, err := config.Validator(user, pass, c) + if err != nil { + return err + } else if valid { + return next(c) } } } - realm := defaultRealm - if config.Realm != defaultRealm { - realm = strconv.Quote(config.Realm) - } - // Need to return `401` for browsers to pop-up login box. - c.Response().Header().Set(echo.HeaderWWWAuthenticate, basic+" realm="+realm) + // Realm is case-insensitive, so we can use "basic" directly. See RFC 7617. + c.Response().Header().Set(echo.HeaderWWWAuthenticate, basic+" realm="+strconv.Quote(config.Realm)) return echo.ErrUnauthorized } } From 9144e71b7aa45b7bdd1c301b3d832e3054182e8f Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Mon, 15 Sep 2025 21:53:07 -0700 Subject: [PATCH 2/3] Add comprehensive tests for realm quoting behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tests cover: - Default realm quoting - Custom realm with spaces - Special characters (quotes, backslashes) - Empty realm fallback to default - Unicode realm support Addresses review feedback about testing strconv.Quote behavior in WWW-Authenticate header per RFC 7617 compliance. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- middleware/basic_auth_test.go | 61 +++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/middleware/basic_auth_test.go b/middleware/basic_auth_test.go index b3abfa172..2d3192615 100644 --- a/middleware/basic_auth_test.go +++ b/middleware/basic_auth_test.go @@ -117,3 +117,64 @@ func TestBasicAuth(t *testing.T) { }) } } + +func TestBasicAuthRealm(t *testing.T) { + e := echo.New() + mockValidator := func(u, p string, c echo.Context) (bool, error) { + return false, nil // Always fail to trigger WWW-Authenticate header + } + + tests := []struct { + name string + realm string + expectedAuth string + }{ + { + name: "Default realm", + realm: "Restricted", + expectedAuth: `basic realm="Restricted"`, + }, + { + name: "Custom realm", + realm: "My API", + expectedAuth: `basic realm="My API"`, + }, + { + name: "Realm with special characters", + realm: `Realm with "quotes" and \backslashes`, + expectedAuth: `basic realm="Realm with \"quotes\" and \\backslashes"`, + }, + { + name: "Empty realm (falls back to default)", + realm: "", + expectedAuth: `basic realm="Restricted"`, + }, + { + name: "Realm with unicode", + realm: "测试领域", + expectedAuth: `basic realm="测试领域"`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/", nil) + res := httptest.NewRecorder() + c := e.NewContext(req, res) + + h := BasicAuthWithConfig(BasicAuthConfig{ + Validator: mockValidator, + Realm: tt.realm, + })(func(c echo.Context) error { + return c.String(http.StatusOK, "test") + }) + + err := h(c) + + var he *echo.HTTPError + errors.As(err, &he) + assert.Equal(t, http.StatusUnauthorized, he.Code) + assert.Equal(t, tt.expectedAuth, res.Header().Get(echo.HeaderWWWAuthenticate)) + }) + } +} From cbff9d197169d3479da32ede4e3a6e18ee0da116 Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Mon, 15 Sep 2025 21:54:13 -0700 Subject: [PATCH 3/3] Optimize realm quoting to happen once during middleware creation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move strconv.Quote(config.Realm) from per-request execution to middleware initialization for better performance. - Pre-compute quoted realm at middleware creation time - Avoids repeated string operations on every auth failure - Maintains same behavior with better efficiency Performance improvement suggested during code review. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- middleware/basic_auth.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/middleware/basic_auth.go b/middleware/basic_auth.go index f9efafc5d..4a46098e3 100644 --- a/middleware/basic_auth.go +++ b/middleware/basic_auth.go @@ -66,6 +66,9 @@ func BasicAuthWithConfig(config BasicAuthConfig) echo.MiddlewareFunc { config.Realm = defaultRealm } + // Pre-compute the quoted realm for WWW-Authenticate header (RFC 7617) + quotedRealm := strconv.Quote(config.Realm) + return func(next echo.HandlerFunc) echo.HandlerFunc { return func(c echo.Context) error { if config.Skipper(c) { @@ -98,7 +101,7 @@ func BasicAuthWithConfig(config BasicAuthConfig) echo.MiddlewareFunc { // Need to return `401` for browsers to pop-up login box. // Realm is case-insensitive, so we can use "basic" directly. See RFC 7617. - c.Response().Header().Set(echo.HeaderWWWAuthenticate, basic+" realm="+strconv.Quote(config.Realm)) + c.Response().Header().Set(echo.HeaderWWWAuthenticate, basic+" realm="+quotedRealm) return echo.ErrUnauthorized } }