Implement XSRFIgnoreMethods

[oalexander6]

XSRFIgnoreMethods

Background

I discovered while using this library that the current XSRF protections do not allow for a GET request sent by the browser on protected routes, due to requiring the presence of the XSRF token in a header. This is a problem for applications that are not SPAs and send GET requests to the server to retrieve a new HTML page. In these situations, the common pattern is to skip XSRF checks on GET requests.

Solution

To resolve this, I implemented a configuration option called XSRFIgnoreMethods, allowing the user to specify a comma-separated list of HTTP methods for which XSRF checks should be skipped when extracting and validating the JWT. I am open to any feedback, modifications, or discussions about this.

[umputun]

isn't this already exist with https://github.com/go-pkgz/auth/blob/master/auth.go#L106 https://github.com/go-pkgz/auth/blob/master/auth.go#L106 ?

[oalexander6]

No, in this situation I still definitely want XSRF protections on POST, PUT, DELETE, etc. but specifically not for GET requests. The DisableXSRF option disables it for all methods.

[oalexander6]

This PR would allow for ignoring just certain HTTP methods, by setting XSRFIgnoreMethods: []string{"GET", "HEAD"} for example, to just skip the XSRF checks for "GET" and "HEAD" only

[umputun]

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.

[oalexander6]

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.

[umputun]

This PR would allow for ignoring just certain HTTP methods, by setting XSRFIgnoreMethods: []string{"GET", "HEAD"} for example, to just skip the XSRF checks for "GET" and "HEAD" only

Unless I missed something, this code does not expect a []string with all the ignored methods but rather a single string (see my comment in the review above).

[umputun]

one more thing - this change should be also documented in readme. Maybe a section about "Options to ignore XSRF protections" or smth like what showing how to do it and providing a basic explanation why this may be needed

[oalexander6]

one more thing - this change should be also documented in readme. Maybe a section about "Options to ignore XSRF protections" or smth like what showing how to do it and providing a basic explanation why this may be needed

Will do

[oalexander6]

The requested changes have been made. Thanks for the feedback.

[coveralls]

Pull Request Test Coverage Report for Build 10101670690

Details

• 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.02%) to 83.371%

---

Totals
Change from base Build 9764450722:	0.02%
Covered Lines:	2582
Relevant Lines:	3097

---

💛 - Coveralls

[umputun]

a minor thing with implicit types for Contains func

[umputun]

LGTM, thx

[paskal]

@oalexander6 could you please copy the very same changes to v2 directory please?

[oalexander6]

@oalexander6 could you please copy the very same changes to v2 directory please?

#210