refactor: improve CSRF infrastructure

ory · Aug 25, 2020 · 7e367e7 · 7e367e7
1 parent cd84e51
commit 7e367e7
Show file tree

Hide file tree

Showing 13 changed files with 108 additions and 88 deletions.
diff --git a/cmd/daemon/serve.go b/cmd/daemon/serve.go
@@ -57,7 +57,6 @@ func servePublic(d driver.Driver, wg *sync.WaitGroup, cmd *cobra.Command, args [
 		c.SelfPublicURL().Hostname(),
 		!flagx.MustGetBool(cmd, "dev"),
 	)
-	csrf.ExemptPath(session.RouteWhoami)
 	r.WithCSRFHandler(csrf)
 	n.UseHandler(r.CSRFHandler())
 	server := graceful.WithDefaults(&http.Server{

diff --git a/driver/configuration/provider_viper_test.go b/driver/configuration/provider_viper_test.go
@@ -8,8 +8,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/markbates/pkger"
-
 	"github.com/ory/x/logrusx"
 
 	_ "github.com/ory/jsonschema/v3/fileloader"
@@ -28,15 +26,11 @@ import (
 var schema []byte
 
 func init() {
-	file, err := pkger.Open("/.schema/config.schema.json")
+	var err error
+	schema, err = ioutil.ReadFile("../../.schema/config.schema.json")
 	if err != nil {
 		panic("Unable to open configuration JSON Schema.")
 	}
-	defer file.Close()
-	schema, err = ioutil.ReadAll(file)
-	if err != nil {
-		panic("Unable to read configuration JSON Schema.")
-	}
 }
 
 func TestViperProvider(t *testing.T) {

diff --git a/persistence/sql/migratest/migration_test.go b/persistence/sql/migratest/migration_test.go
@@ -51,7 +51,7 @@ func TestMigrations(t *testing.T) {
 	l := logrusx.New("", "", logrusx.ForceLevel(logrus.TraceLevel))
 	plog.Logger = gobuffalologger.Logrus{FieldLogger: l.Entry}
 
-	if !testing.Short() && false {
+	if !testing.Short() {
 		dockertest.Parallel([]func(){
 			func() {
 				connections["postgres"] = dockertest.ConnectToTestPostgreSQLPop(t)

diff --git a/selfservice/strategy/password/.schema/login.schema.json b/selfservice/strategy/password/.schema/login.schema.json
@@ -11,6 +11,9 @@
       "type": "string",
       "minLength": 1
     },
+    "csrf_token": {
+      "type": "string"
+    },
     "identifier": {
       "type": "string",
       "minLength": 1

diff --git a/selfservice/strategy/password/.schema/registration.schema.json b/selfservice/strategy/password/.schema/registration.schema.json
@@ -11,6 +11,9 @@
       "type": "string",
       "minLength": 1
     },
+    "csrf_token": {
+      "type": "string"
+    },
     "traits": {}
   }
 }
diff --git a/selfservice/strategy/password/login.go b/selfservice/strategy/password/login.go
@@ -6,6 +6,7 @@ import (
 	"net/http"
 
 	"github.com/julienschmidt/httprouter"
+	"github.com/justinas/nosurf"
 	"github.com/pkg/errors"
 
 	"github.com/ory/x/decoderx"
@@ -26,6 +27,7 @@ const (
 )
 
 func (s *Strategy) RegisterLoginRoutes(r *x.RouterPublic) {
+	s.d.CSRFHandler().ExemptPath(RouteLogin)
 	r.POST(RouteLogin, s.handleLogin)
 }
 
@@ -103,6 +105,17 @@ func (s *Strategy) handleLogin(w http.ResponseWriter, r *http.Request, _ httprou
 		return
 	}
 
+	var p LoginFormPayload
+	if err := s.hd.Decode(r, &p, decoderx.MustHTTPRawJSONSchemaCompiler(loginSchema)); err != nil {
+		s.handleLoginError(w, r, ar, &p, err)
+		return
+	}
+
+	if ar.Type == flow.TypeBrowser && !nosurf.VerifyToken(s.d.GenerateCSRFToken(r), p.CSRFToken) {
+		s.handleLoginError(w, r, ar, &p, x.ErrInvalidCSRFToken)
+		return
+	}
+
 	if _, err := s.d.SessionManager().FetchFromRequest(r.Context(), r); err == nil && !ar.Forced {
 		if ar.Type == flow.TypeBrowser {
 			http.Redirect(w, r, s.c.SelfServiceBrowserDefaultReturnTo().String(), http.StatusFound)
@@ -113,12 +126,6 @@ func (s *Strategy) handleLogin(w http.ResponseWriter, r *http.Request, _ httprou
 		return
 	}
 
-	var p LoginFormPayload
-	if err := s.hd.Decode(r, &p, decoderx.MustHTTPRawJSONSchemaCompiler(loginSchema)); err != nil {
-		s.handleLoginError(w, r, ar, &p, err)
-		return
-	}
-
 	if err := ar.Valid(); err != nil {
 		s.handleLoginError(w, r, ar, &p, err)
 		return