From 29329bc4fe194d3e373c46768120f8000ac5f8bd Mon Sep 17 00:00:00 2001 From: Owen Alexander Date: Sat, 20 Jul 2024 18:11:50 -0400 Subject: [PATCH 1/4] Implement XSRFIgnoreMethods --- auth.go | 17 +++++++++-------- token/jwt.go | 37 ++++++++++++++++++++----------------- token/jwt_test.go | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 25 deletions(-) diff --git a/auth.go b/auth.go index 40b01d9c..c111bc76 100644 --- a/auth.go +++ b/auth.go @@ -46,14 +46,15 @@ type Opts struct { DisableIAT bool // disable IssuedAt claim // optional (custom) names for cookies and headers - JWTCookieName string // default "JWT" - JWTCookieDomain string // default empty - JWTHeaderKey string // default "X-JWT" - XSRFCookieName string // default "XSRF-TOKEN" - XSRFHeaderKey string // default "X-XSRF-TOKEN" - JWTQuery string // default "token" - SendJWTHeader bool // if enabled send JWT as a header instead of cookie - SameSiteCookie http.SameSite // limit cross-origin requests with SameSite cookie attribute + JWTCookieName string // default "JWT" + JWTCookieDomain string // default empty + JWTHeaderKey string // default "X-JWT" + XSRFCookieName string // default "XSRF-TOKEN" + XSRFHeaderKey string // default "X-XSRF-TOKEN" + XSRFIgnoreMethods string // disable XSRF protection for the specified request methods (ex. "GET,POST"), default empty + JWTQuery string // default "token" + SendJWTHeader bool // if enabled send JWT as a header instead of cookie + SameSiteCookie http.SameSite // limit cross-origin requests with SameSite cookie attribute Issuer string // optional value for iss claim, usually the application name, default "go-pkgz/auth" diff --git a/token/jwt.go b/token/jwt.go index c899b070..cfa8722c 100644 --- a/token/jwt.go +++ b/token/jwt.go @@ -35,11 +35,12 @@ type Handshake struct { const ( // default names for cookies and headers - defaultJWTCookieName = "JWT" - defaultJWTCookieDomain = "" - defaultJWTHeaderKey = "X-JWT" - defaultXSRFCookieName = "XSRF-TOKEN" - defaultXSRFHeaderKey = "X-XSRF-TOKEN" + defaultJWTCookieName = "JWT" + defaultJWTCookieDomain = "" + defaultJWTHeaderKey = "X-JWT" + defaultXSRFCookieName = "XSRF-TOKEN" + defaultXSRFHeaderKey = "X-XSRF-TOKEN" + defaultXSRFIgnoreMethods = "" defaultIssuer = "go-pkgz/auth" @@ -59,17 +60,18 @@ type Opts struct { DisableXSRF bool DisableIAT bool // disable IssuedAt claim // optional (custom) names for cookies and headers - JWTCookieName string - JWTCookieDomain string - JWTHeaderKey string - XSRFCookieName string - XSRFHeaderKey string - JWTQuery string - AudienceReader Audience // allowed aud values - Issuer string // optional value for iss claim, usually application name - AudSecrets bool // uses different secret for differed auds. important: adds pre-parsing of unverified token - SendJWTHeader bool // if enabled send JWT as a header instead of cookie - SameSite http.SameSite // define a cookie attribute making it impossible for the browser to send this cookie cross-site + JWTCookieName string + JWTCookieDomain string + JWTHeaderKey string + XSRFCookieName string + XSRFHeaderKey string + XSRFIgnoreMethods string + JWTQuery string + AudienceReader Audience // allowed aud values + Issuer string // optional value for iss claim, usually application name + AudSecrets bool // uses different secret for differed auds. important: adds pre-parsing of unverified token + SendJWTHeader bool // if enabled send JWT as a header instead of cookie + SameSite http.SameSite // define a cookie attribute making it impossible for the browser to send this cookie cross-site } // NewService makes JWT service @@ -89,6 +91,7 @@ func NewService(opts Opts) *Service { setDefault(&res.JWTQuery, defaultTokenQuery) setDefault(&res.Issuer, defaultIssuer) setDefault(&res.JWTCookieDomain, defaultJWTCookieDomain) + setDefault(&res.XSRFIgnoreMethods, defaultXSRFIgnoreMethods) if opts.TokenDuration == 0 { res.TokenDuration = defaultTokenDuration @@ -293,7 +296,7 @@ func (j *Service) Get(r *http.Request) (Claims, string, error) { return Claims{}, "", fmt.Errorf("token expired") } - if j.DisableXSRF { + if j.DisableXSRF || strings.Contains(j.XSRFIgnoreMethods, r.Method) { return claims, tokenString, nil } diff --git a/token/jwt_test.go b/token/jwt_test.go index e30ee137..d02a2943 100644 --- a/token/jwt_test.go +++ b/token/jwt_test.go @@ -471,6 +471,53 @@ func TestJWT_SetAndGetWithXsrfMismatch(t *testing.T) { assert.Equal(t, claims, c) } +func TestJWT_GetWithXsrfMismatchOnIgnoredMethod(t *testing.T) { + j := NewService(Opts{SecretReader: SecretFunc(mockKeyStore), SecureCookies: false, + TokenDuration: time.Hour, CookieDuration: days31, + JWTCookieName: jwtCustomCookieName, JWTHeaderKey: jwtCustomHeaderKey, + XSRFCookieName: xsrfCustomCookieName, XSRFHeaderKey: xsrfCustomHeaderKey, + ClaimsUpd: ClaimsUpdFunc(func(claims Claims) Claims { + claims.User.SetStrAttr("stra", "stra-val") + claims.User.SetBoolAttr("boola", true) + return claims + }), + Issuer: "remark42", + DisableIAT: true, + }) + + claims := testClaims + claims.SessionOnly = true + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/valid" { + _, e := j.Set(w, claims) + require.NoError(t, e) + w.WriteHeader(200) + } + })) + defer ts.Close() + + resp, err := http.Get(ts.URL + "/valid") + require.Nil(t, err) + assert.Equal(t, 200, resp.StatusCode) + + j.XSRFIgnoreMethods = "GET" + req := httptest.NewRequest("GET", "/valid", nil) + req.AddCookie(resp.Cookies()[0]) + req.Header.Add(xsrfCustomHeaderKey, "random id wrong") + _, _, err = j.Get(req) + require.NoError(t, err, "xsrf mismatch, but ignored") + + j.DisableXSRF = true + j.XSRFIgnoreMethods = "" + req = httptest.NewRequest("GET", "/valid", nil) + req.AddCookie(resp.Cookies()[0]) + req.Header.Add(xsrfCustomHeaderKey, "random id wrong") + c, _, err := j.Get(req) + require.NoError(t, err, "xsrf mismatch, but ignored") + claims.User.Audience = c.Audience // set aud to user because we don't do the normal Get call + assert.Equal(t, claims, c) +} + func TestJWT_SetAndGetWithCookiesExpired(t *testing.T) { j := NewService(Opts{SecretReader: SecretFunc(mockKeyStore), SecureCookies: false, TokenDuration: time.Hour, CookieDuration: days31, From 59d761063a039bd874885b7d107a626b62c0a772 Mon Sep 17 00:00:00 2001 From: Owen Alexander Date: Sat, 20 Jul 2024 19:24:01 -0400 Subject: [PATCH 2/4] Switch XSRFIgnoreMethods to string slice --- auth.go | 2 +- token/jwt.go | 25 ++++++++++++++++--------- token/jwt_test.go | 4 ++-- 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/auth.go b/auth.go index c111bc76..5bd9d879 100644 --- a/auth.go +++ b/auth.go @@ -51,7 +51,7 @@ type Opts struct { JWTHeaderKey string // default "X-JWT" XSRFCookieName string // default "XSRF-TOKEN" XSRFHeaderKey string // default "X-XSRF-TOKEN" - XSRFIgnoreMethods string // disable XSRF protection for the specified request methods (ex. "GET,POST"), default empty + XSRFIgnoreMethods []string // disable XSRF protection for the specified request methods (ex. []string{"GET", "POST")}, default empty JWTQuery string // default "token" SendJWTHeader bool // if enabled send JWT as a header instead of cookie SameSiteCookie http.SameSite // limit cross-origin requests with SameSite cookie attribute diff --git a/token/jwt.go b/token/jwt.go index cfa8722c..b48c3649 100644 --- a/token/jwt.go +++ b/token/jwt.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "net/http" + "slices" "strings" "time" @@ -35,12 +36,11 @@ type Handshake struct { const ( // default names for cookies and headers - defaultJWTCookieName = "JWT" - defaultJWTCookieDomain = "" - defaultJWTHeaderKey = "X-JWT" - defaultXSRFCookieName = "XSRF-TOKEN" - defaultXSRFHeaderKey = "X-XSRF-TOKEN" - defaultXSRFIgnoreMethods = "" + defaultJWTCookieName = "JWT" + defaultJWTCookieDomain = "" + defaultJWTHeaderKey = "X-JWT" + defaultXSRFCookieName = "XSRF-TOKEN" + defaultXSRFHeaderKey = "X-XSRF-TOKEN" defaultIssuer = "go-pkgz/auth" @@ -50,6 +50,10 @@ const ( defaultTokenQuery = "token" ) +var ( + defaultXSRFIgnoreMethods = []string{} +) + // Opts holds constructor params type Opts struct { SecretReader Secret @@ -65,7 +69,7 @@ type Opts struct { JWTHeaderKey string XSRFCookieName string XSRFHeaderKey string - XSRFIgnoreMethods string + XSRFIgnoreMethods []string JWTQuery string AudienceReader Audience // allowed aud values Issuer string // optional value for iss claim, usually application name @@ -91,7 +95,10 @@ func NewService(opts Opts) *Service { setDefault(&res.JWTQuery, defaultTokenQuery) setDefault(&res.Issuer, defaultIssuer) setDefault(&res.JWTCookieDomain, defaultJWTCookieDomain) - setDefault(&res.XSRFIgnoreMethods, defaultXSRFIgnoreMethods) + + if opts.XSRFIgnoreMethods == nil { + opts.XSRFIgnoreMethods = defaultXSRFIgnoreMethods + } if opts.TokenDuration == 0 { res.TokenDuration = defaultTokenDuration @@ -296,7 +303,7 @@ func (j *Service) Get(r *http.Request) (Claims, string, error) { return Claims{}, "", fmt.Errorf("token expired") } - if j.DisableXSRF || strings.Contains(j.XSRFIgnoreMethods, r.Method) { + if j.DisableXSRF || slices.Contains[[]string, string](j.XSRFIgnoreMethods, r.Method) { return claims, tokenString, nil } diff --git a/token/jwt_test.go b/token/jwt_test.go index d02a2943..7b111588 100644 --- a/token/jwt_test.go +++ b/token/jwt_test.go @@ -500,7 +500,7 @@ func TestJWT_GetWithXsrfMismatchOnIgnoredMethod(t *testing.T) { require.Nil(t, err) assert.Equal(t, 200, resp.StatusCode) - j.XSRFIgnoreMethods = "GET" + j.XSRFIgnoreMethods = []string{"GET"} req := httptest.NewRequest("GET", "/valid", nil) req.AddCookie(resp.Cookies()[0]) req.Header.Add(xsrfCustomHeaderKey, "random id wrong") @@ -508,7 +508,7 @@ func TestJWT_GetWithXsrfMismatchOnIgnoredMethod(t *testing.T) { require.NoError(t, err, "xsrf mismatch, but ignored") j.DisableXSRF = true - j.XSRFIgnoreMethods = "" + j.XSRFIgnoreMethods = []string{} req = httptest.NewRequest("GET", "/valid", nil) req.AddCookie(resp.Cookies()[0]) req.Header.Add(xsrfCustomHeaderKey, "random id wrong") From 376bdfe5d7cc354934dce22182c400fcee807e52 Mon Sep 17 00:00:00 2001 From: Owen Alexander Date: Sat, 20 Jul 2024 19:38:25 -0400 Subject: [PATCH 3/4] Update README for XSRF protections --- README.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/README.md b/README.md index 94bcd25e..9d07ff29 100644 --- a/README.md +++ b/README.md @@ -609,6 +609,21 @@ For more details refer to [Complete Guide of Battle.net OAuth API and Login Butt 1. Fill **App name** and **Description** and **URL** of your site 1. In the field **Callback URLs** enter the correct url of your callback handler e.g. https://example.mysite.com/{route}/twitter/callback 1. Under **Key and tokens** take note of the **Consumer API Key** and **Consumer API Secret key**. Those will be used as `cid` and `csecret` + +## XSRF Protections +By default, the XSRF protections will apply to all requests which reach the `middlewares.Auth`, +`middlewares.Admin` or `middlewares.RBAC` middlewares. This will require setting a request header +with a key of `` containing the value of the cookie named ``. + +To disable all XSRF protections, set `DisableXSRF` to `true`. This should probably only be used +during testing or debugging. + +When setting a custom request header is not possible, such as when building a web application which +is not a Single-Page-Application and HTML link tags are used to navigate pages, specific HTTP methods +may be excluded using the `XSRFIgnoreMethods` option. For example, to disable GET requests, set this +option to `XSRFIgnoreMethods: []string{"GET"}`. Adding methods other than GET to this list may result +in XSRF vulnerabilities. + ## Status The library extracted from [remark42](https://github.com/umputun/remark) project. The original code in production use on multiple sites and seems to work fine. From 88b74a45d3ce1364972eeb8956a4777a986b0e79 Mon Sep 17 00:00:00 2001 From: Owen Alexander Date: Thu, 25 Jul 2024 17:20:06 -0400 Subject: [PATCH 4/4] Remove explicit types for Contains method in XSRFIgnoreMethods check --- token/jwt.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/token/jwt.go b/token/jwt.go index b48c3649..73cc1c2c 100644 --- a/token/jwt.go +++ b/token/jwt.go @@ -303,7 +303,7 @@ func (j *Service) Get(r *http.Request) (Claims, string, error) { return Claims{}, "", fmt.Errorf("token expired") } - if j.DisableXSRF || slices.Contains[[]string, string](j.XSRFIgnoreMethods, r.Method) { + if j.DisableXSRF || slices.Contains(j.XSRFIgnoreMethods, r.Method) { return claims, tokenString, nil }