Skip to content

Commit

Permalink
issues/76: dont require csrf for post requests that have no cookies A…
Browse files Browse the repository at this point in the history
…ND no HTTP authentication (#77)

fixes: #76
  • Loading branch information
komuw authored Jul 6, 2022
1 parent 3c4cb68 commit 4922544
Show file tree
Hide file tree
Showing 4 changed files with 127 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,4 @@ Most recent version is listed first.
- use acme for certificates: https://github.com/komuw/ong/pull/69
- issues/73: bind on 0.0.0.0 or localhost conditionally: https://github.com/komuw/ong/pull/74
- redirect IP to domain: https://github.com/komuw/ong/pull/75
- dont require csrf for POST requests that have no cookies and arent http auth: https://github.com/komuw/ong/pull/77
64 changes: 64 additions & 0 deletions example/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package main
import (
"context"
"fmt"
"html/template"
"net/http"
"os"
"sync"
Expand Down Expand Up @@ -40,6 +41,12 @@ func main() {
api.check(200),
middleware.WithOpts("localhost"),
),
server.NewRoute(
"login",
server.MethodAll,
api.login(),
middleware.WithOpts("localhost"),
),
})

_, _ = server.CreateDevCertKey()
Expand Down Expand Up @@ -116,7 +123,64 @@ func (s myAPI) check(code int) http.HandlerFunc {
csrfToken := middleware.GetCsrfToken(r.Context())
s.l.Info(log.F{"msg": "check called", "cspNonce": cspNonce, "csrfToken": csrfToken})

_, _ = fmt.Fprint(w, "hello from check/ endpoint")
// use code, which is a dependency specific to this handler
w.WriteHeader(code)
}
}

func (s myAPI) login() http.HandlerFunc {
tmpl, err := template.New("myTpl").Parse(`<!DOCTYPE html>
<html>
<body>
<h2>Welcome to awesome website.</h2>
<form method="POST">
<label>Email:</label><br>
<input type="text" id="email" name="email"><br>
<label>First Name:</label><br>
<input type="text" id="firstName" name="firstName"><br>
<input type="hidden" id="{{.CsrfTokenName}}" name="{{.CsrfTokenName}}" value="{{.CsrfTokenValue}}"><br>
<input type="submit">
</form>
<script nonce="{{.CspNonceValue}}">
console.log("hello world");
</script>
</body>
</html>`)
if err != nil {
panic(err)
}

return func(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodPost {
data := struct {
CsrfTokenName string
CsrfTokenValue string
CspNonceValue string
}{
CsrfTokenName: middleware.CsrfTokenFormName,
CsrfTokenValue: middleware.GetCsrfToken(r.Context()),
CspNonceValue: middleware.GetCspNonce(r.Context()),
}
if err = tmpl.Execute(w, data); err != nil {
panic(err)
}
return
}

if err = r.ParseForm(); err != nil {
panic(err)
}

fmt.Println("r.Form: ", r.Form)
for k, v := range r.Form {
fmt.Println("k, v: ", k, v)
}
_, _ = fmt.Fprintf(w, "you have submitted: %s", r.Form)
}
}
44 changes: 33 additions & 11 deletions middleware/csrf.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package middleware
import (
"context"
"errors"
"mime"
"net/http"
"sync"
"time"
Expand All @@ -29,13 +30,19 @@ var (
type csrfContextKey string

const (
csrfCtxKey = csrfContextKey("csrfContextKey")
csrfDefaultToken = ""
csrfCookieName = "csrftoken" // named after what django uses.
csrfHeader = "X-Csrf-Token" // named after what fiber uses.
csrfCookieForm = csrfCookieName
clientCookieHeader = "Cookie"
varyHeader = "Vary"
// CsrfTokenFormName is the name of the html form name attribute for csrf token.
CsrfTokenFormName = "csrftoken" // named after what django uses.
csrfCtxKey = csrfContextKey("csrfContextKey")
csrfDefaultToken = ""
csrfCookieName = CsrfTokenFormName
csrfHeader = "X-Csrf-Token" // named after what fiber uses.
clientCookieHeader = "Cookie"
varyHeader = "Vary"
authorizationHeader = "Authorization"
proxyAuthorizationHeader = "Proxy-Authorization"
ctHeader = "Content-Type"
formUrlEncoded = "application/x-www-form-urlencoded"
multiformData = "multipart/form-data"

// gorilla/csrf; 12hrs
// django: 1yr??
Expand Down Expand Up @@ -76,6 +83,24 @@ func Csrf(wrappedHandler http.HandlerFunc, domain string) http.HandlerFunc {
default:
// For POST requests, we insist on a CSRF cookie, and in this way we can avoid all CSRF attacks, including login CSRF.
actualToken = getToken(r)

ct, _, err := mime.ParseMediaType(r.Header.Get(ctHeader))
if err == nil &&
ct != formUrlEncoded &&
ct != multiformData &&
r.Header.Get(clientCookieHeader) == "" &&
r.Header.Get(authorizationHeader) == "" &&
r.Header.Get(proxyAuthorizationHeader) == "" {
// For POST requests that;
// - are not form data.
// - have no cookies.
// - are not using http authentication.
// then it is okay to not validate csrf for them.
// This is especially useful for REST API endpoints.
// see: https://github.com/komuw/ong/issues/76
break
}

if !csrfStore.exists(actualToken) {
// we should fail the request since it means that the server is not aware of such a token.
cookie.Delete(w, csrfCookieName, domain)
Expand Down Expand Up @@ -179,10 +204,7 @@ func getToken(r *http.Request) (actualToken string) {
}

fromForm := func() string {
if err := r.ParseForm(); err != nil {
return ""
}
return r.Form.Get(csrfCookieForm)
return r.FormValue(CsrfTokenFormName) // calls ParseMultipartForm and ParseForm if necessary
}

fromHeader := func() string {
Expand Down
31 changes: 29 additions & 2 deletions middleware/csrf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"io"
"net/http"
"net/http/httptest"
"strings"
"sync"
"testing"

Expand Down Expand Up @@ -186,7 +187,7 @@ func TestGetToken(t *testing.T) {
req := httptest.NewRequest(http.MethodGet, "/someUri", nil)
err := req.ParseForm()
attest.Ok(t, err)
req.Form.Add(csrfCookieForm, want)
req.Form.Add(CsrfTokenFormName, want)
got := getToken(req)
attest.Equal(t, got, want[csrfStringTokenlength:])
})
Expand All @@ -209,7 +210,7 @@ func TestGetToken(t *testing.T) {
req.Header.Set(csrfHeader, headerToken)
err := req.ParseForm()
attest.Ok(t, err)
req.Form.Add(csrfCookieForm, formToken)
req.Form.Add(CsrfTokenFormName, formToken)

got := getToken(req)
attest.Equal(t, got, cookieToken[csrfStringTokenlength:])
Expand Down Expand Up @@ -477,6 +478,32 @@ func TestCsrf(t *testing.T) {
}
})

t.Run("POST requests with no cookies dont need csrf", func(t *testing.T) {
t.Parallel()

msg := "hello"
domain := "example.com"
wrappedHandler := Csrf(someCsrfHandler(msg), domain)

rec := httptest.NewRecorder()
postMsg := "my name is John"
body := strings.NewReader(postMsg)
req := httptest.NewRequest(http.MethodPost, "/someUri", body)
req.Header.Add(ctHeader, "application/json")
wrappedHandler.ServeHTTP(rec, req)

res := rec.Result()
defer res.Body.Close()

rb, err := io.ReadAll(res.Body)
attest.Ok(t, err)

attest.Equal(t, res.StatusCode, http.StatusOK)
attest.Equal(t, string(rb), msg)
attest.NotZero(t, res.Header.Get(tokenHeader))
attest.Equal(t, len(res.Cookies()), 1)
})

// concurrency safe
t.Run("POST requests with valid token from mutiple tabs", func(t *testing.T) {
t.Parallel()
Expand Down

0 comments on commit 4922544

Please sign in to comment.