Add support for Angular XSRF protection #2153
Conversation
|
Hey nwmac! Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA. |
|
Note that the e2e tests will need updating, as we won't be able to call the API without the XSRF header. |
| if c.Request().Method() == "GET" || c.Request().Method() == "HEAD" { | ||
| return h(c) | ||
| } | ||
| errMsg := "Failed to get stored XSRF token from user session" |
There was a problem hiding this comment.
It's recommended to check for cross-origin requests at this point using request headers: https://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF)_Prevention_Cheat_Sheet#Verifying_Same_Origin_with_Standard_Headers.
|
Just curious--why not use an existing csrf library like https://github.com/gorilla/csrf here? The approach is similar, but the library is already battle-tested and mitigates a few more issues than this implementation (afaict). |
|
@jmcarp Thank you for the comments... I will take a look at that library and will update this PR to use it in place of the custom middleware. Thanks for the pointer. |
…er stronger checking as per other libraries
|
@jmcarp I tried to switch over to the gorilla library, but we use echo server and its written for the golang http handler mechanism. There is a wrapper that lets you use this with echo - which I tried. Could get the middleware working, but could not get the token in order to send to the front-end. Spent a long time debugging this and the gorilla csrf lib stores the token on the a context keyed by the request object - it appears that the request object that the middleware sees is different to that later on in the chain, so when I request the token it always comes back empty. Couldn't find a way around this. For now, I've updated the PR slightly. We are already using the echo cors middleware - https://echo.labstack.com/middleware/cors - for CORS, so that should be covered. They do have a csrf middleware, but there isn't much to it over and above what was already implemented. |
…port # Conflicts: # package.json # src/backend/app-core/main.go # src/frontend/app/app.module.ts # src/test-e2e/endpoints/endpoints-connect.e2e-spec.ts # src/test-e2e/endpoints/endpoints-register.e2e-spec.ts # src/test-e2e/helpers/e2e-helpers.ts
Codecov Report
@@ Coverage Diff @@
## v2-master #2153 +/- ##
=============================================
- Coverage 71.05% 71.02% -0.04%
=============================================
Files 581 581
Lines 24255 24257 +2
Branches 5441 5442 +1
=============================================
- Hits 17235 17229 -6
- Misses 7020 7028 +8 |
…port # Conflicts: # src/frontend/app/store/effects/auth.effects.ts
…port # Conflicts: # src/frontend/app/app.module.ts
|
LGTM |
No description provided.