Add X-Requested-With header to prevent CSRF #48
Conversation
…ard into requested-with-header
…ard into requested-with-header
server/api/api.go
Outdated
| } | ||
|
|
||
| func (a *API) RegisterAdminRoutes(r *mux.Router) { | ||
| r.HandleFunc("/api/v1/admin/users/{username}/password", a.adminRequired(a.handleAdminSetPassword)).Methods("POST") | ||
| } | ||
|
|
||
| func (a *API) addHandler(r *mux.Router, path string, method string, f func(http.ResponseWriter, *http.Request)) { |
There was a problem hiding this comment.
This looks a look like a middleware
There was a problem hiding this comment.
There was a problem hiding this comment.
Thanks Jesus! I knew there was a cleaner way. Refactored to use Middleware.
server/api/api.go
Outdated
|
|
||
| r.HandleFunc("/api/v1/login", a.handleLogin).Methods("POST") | ||
| r.HandleFunc("/api/v1/register", a.handleRegister).Methods("POST") | ||
| a.addHandler(r, "/api/v1/login", "POST", a.sessionRequired(a.handleLogin)) |
There was a problem hiding this comment.
You are adding sessionRequired to the login and register ;)
There was a problem hiding this comment.
Ack, fixed that. Much easier to see with the middleware pattern.
| files.Use(a.requireCSRFToken) | ||
|
|
||
| files.HandleFunc("/", a.sessionRequired(a.handleUploadFile)).Methods("POST") | ||
| files.HandleFunc("/{filename}", a.sessionRequired(a.handleServeFile)).Methods("GET") |
There was a problem hiding this comment.
I'm not 100% sure about the need of the CSRFToken check for the static files, actually I think it should be only for the API.
There was a problem hiding this comment.
I think we don't have much control over the image loading and files downloading in the browser to add that header.
There was a problem hiding this comment.
Actually, I had refactored contentBlock to get the images with fetch, passing the session token and CSRF header. However, I removed the CSRF check for GET /files for now, as that (in theory) shouldn't be a CSRF vulnerability as no state is changed. We can think about this one more, and change later if needed.
| r.HandleFunc("/api/v1/blocks", a.sessionRequired(a.handlePostBlocks)).Methods("POST") | ||
| r.HandleFunc("/api/v1/blocks/{blockID}", a.sessionRequired(a.handleDeleteBlock)).Methods("DELETE") | ||
| r.HandleFunc("/api/v1/blocks/{blockID}/subtree", a.attachSession(a.handleGetSubTree, false)).Methods("GET") | ||
| apiv1 := r.PathPrefix("/api/v1").Subrouter() |
There was a problem hiding this comment.
Thanks. I smoke tested this and it looks ok, but please LMK if you see anything off.
There was a problem hiding this comment.
Nope, everything is looking great now
Add X-Requested-With=XMLHttpRequest header to all REST API requests, and check on the server.