Force path and domain on CSRF cookie (#70)

Closes #68
ory · Aug 8, 2019 · 4f7eee8 · 4f7eee8
1 parent a4b594c
commit 4f7eee8
Show file tree

Hide file tree

Showing 6 changed files with 36 additions and 6 deletions.
diff --git a/cmd/daemon/serve.go b/cmd/daemon/serve.go
@@ -4,6 +4,7 @@ import (
  "net/http"
  "sync"
 
+ "github.com/ory/x/flagx"
  "github.com/ory/x/healthx"
 
  "github.com/sirupsen/logrus"
@@ -37,7 +38,14 @@ func servePublic(d driver.Driver, wg *sync.WaitGroup, cmd *cobra.Command, args [
  r.ErrorHandler().RegisterPublicRoutes(router)
 
  n.Use(NewNegroniLoggerMiddleware(l.(*logrus.Logger), "public#"+c.SelfPublicURL().String()))
- r.WithCSRFHandler(x.NewCSRFHandler(router, r.Writer()))
+ r.WithCSRFHandler(x.NewCSRFHandler(
+ router,
+ r.Writer(),
+ l,
+ c.SelfPublicURL().Path,
+ c.SelfPublicURL().Hostname(),
+ !flagx.MustGetBool(cmd, "dev"),
+ ))
  n.UseHandler(
  r.CSRFHandler(),
  )

diff --git a/selfservice/handler_test.go b/selfservice/handler_test.go
@@ -10,6 +10,7 @@ import (
  "github.com/google/uuid"
  "github.com/julienschmidt/httprouter"
  "github.com/justinas/nosurf"
+ "github.com/sirupsen/logrus"
  "github.com/stretchr/testify/assert"
  "github.com/stretchr/testify/require"
  "github.com/tidwall/gjson"
@@ -40,7 +41,7 @@ func TestLogoutHandler(t *testing.T) {
 
  router := x.NewRouterPublic()
  handler.RegisterPublicRoutes(router)
- reg.WithCSRFHandler(x.NewCSRFHandler(router, reg.Writer()))
+ reg.WithCSRFHandler(x.NewCSRFHandler(router, reg.Writer(), logrus.New(), "/", "", false))
  ts := httptest.NewServer(reg.CSRFHandler())
  defer ts.Close()
 

diff --git a/selfservice/oidc/strategy.go b/selfservice/oidc/strategy.go
@@ -32,7 +32,7 @@ import (
 )
 
 const (
- BasePath = "/methods/oidc"
+ BasePath = "/auth/browser/methods/oidc"
  AuthPath = BasePath + "/auth/:request"
  CallbackPath = BasePath + "/callback/:provider"
 )

diff --git a/selfservice/password/login.go b/selfservice/password/login.go
@@ -21,7 +21,7 @@ import (
 )
 
 const (
- LoginPath = "/auth/browser/login/methods/password"
+ LoginPath = "/auth/browser/methods/password/login"
 )
 
 func (s *Strategy) setLoginRoutes(r *x.RouterPublic) {

diff --git a/selfservice/password/registration.go b/selfservice/password/registration.go
@@ -21,7 +21,7 @@ import (
 )
 
 const (
- RegistrationPath = "/auth/browser/registration/methods/password"
+ RegistrationPath = "/auth/browser/methods/password/registration"
 )
 
 type RegistrationFormPayload struct {

diff --git a/x/nosurf.go b/x/nosurf.go
@@ -5,6 +5,7 @@ import (
 
  "github.com/justinas/nosurf"
  "github.com/pkg/errors"
+ "github.com/sirupsen/logrus"
 
  "github.com/ory/herodot"
 )
@@ -13,9 +14,29 @@ type CSRFProvider interface {
  CSRFHandler() *nosurf.CSRFHandler
 }
 
-func NewCSRFHandler(router http.Handler, writer herodot.Writer) *nosurf.CSRFHandler {
+func NewCSRFHandler(
+ router http.Handler,
+ writer herodot.Writer,
+ logger logrus.FieldLogger,
+ path string,
+ domain string,
+ secure bool,
+) *nosurf.CSRFHandler {
  n := nosurf.New(router)
+ n.SetBaseCookie(http.Cookie{
+ MaxAge: nosurf.MaxAge,
+ Path: path,
+ Domain: domain,
+ HttpOnly: true,
+ Secure: secure,
+ })
  n.SetFailureHandler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ logger.
+ WithField("expected_token", nosurf.Token(r)).
+ WithField("received_token", r.Form.Get("csrf_token")).
+ WithField("received_token_form", r.PostForm.Get("csrf_token")).
+ Warn("A request failed due to a missing or invalid csrf_token value")
+
  writer.WriteError(w, r, errors.WithStack(herodot.ErrBadRequest.WithReasonf("CSRF token is missing or invalid.")))
  }))
  return n