Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement XSRFIgnoreMethods #207

Merged
merged 4 commits into from
Jul 25, 2024
Merged

Implement XSRFIgnoreMethods #207

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
29329bc
Implement XSRFIgnoreMethods
oalexander6 Jul 20, 2024
59d7610
Switch XSRFIgnoreMethods to string slice
oalexander6 Jul 20, 2024
376bdfe
Update README for XSRF protections
oalexander6 Jul 20, 2024
88b74a4
Remove explicit types for Contains method in XSRFIgnoreMethods check
oalexander6 Jul 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions auth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
37 changes: 20 additions & 17 deletions token/jwt.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer not to use a comma-separated string in this context and then apply strings.contains on the string subsequently. It would be more organized to utilize a slice of methods in this scenario and perform an exact match on the request method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, I originally did that but saw the the default values for the related options were constants and wanted to stick to that, which couldn't be done with a slice. I will switch it back to an array of strings.

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
Expand All @@ -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
Expand Down Expand Up @@ -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
}

Expand Down
47 changes: 47 additions & 0 deletions token/jwt_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down