Skip to content

Commit

Permalink
fix: resolve test issues introduced by new csrf protection
Browse files Browse the repository at this point in the history
  • Loading branch information
@aeneasr
aeneasr committed Aug 26, 2020
1 parent 3646383 commit 625ef5e
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 32 deletions.
11 changes: 2 additions & 9 deletions selfservice/strategy/password/registration.go
View file
Expand Up @@ -16,6 +16,7 @@ import (

"github.com/ory/kratos/driver/configuration"
"github.com/ory/kratos/selfservice/flow"
"github.com/ory/kratos/selfservice/flow/settings"

"github.com/ory/herodot"
"github.com/ory/x/urlx"
Expand All @@ -24,7 +25,6 @@ import (
"github.com/ory/kratos/schema"
"github.com/ory/kratos/selfservice/flow/registration"
"github.com/ory/kratos/selfservice/form"
"github.com/ory/kratos/session"
"github.com/ory/kratos/x"
)

Expand All @@ -40,14 +40,7 @@ type RegistrationFormPayload struct {

func (s *Strategy) RegisterRegistrationRoutes(public *x.RouterPublic) {
s.d.CSRFHandler().ExemptPath(RouteRegistration)
public.POST(RouteRegistration, s.d.SessionHandler().IsNotAuthenticated(s.handleRegistration, func(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
handler := session.RedirectOnAuthenticated(s.c)
if x.IsJSONRequest(r) {
handler = session.RespondWithJSONErrorOnAuthenticated(s.d.Writer(), registration.ErrAlreadyLoggedIn)
}

handler(w, r, ps)
}))
public.POST(RouteRegistration, s.d.SessionHandler().IsNotAuthenticated(s.handleRegistration, settings.OnUnauthenticated(s.c, s.d)))
}

func (s *Strategy) handleRegistrationError(w http.ResponseWriter, r *http.Request, rr *registration.Flow, p *RegistrationFormPayload, err error) {
Expand Down
57 changes: 34 additions & 23 deletions selfservice/strategy/password/registration_test.go
View file
Expand Up @@ -227,6 +227,7 @@ func TestRegistration(t *testing.T) {
"traits.username": {"registration-identifier-4"},
"password": {"password"},
"traits.foobar": {"bar"},
"csrf_token":{x.FakeCSRFToken},
}.Encode())
assert.Contains(t, res.Request.URL.String(), uiTS.URL+"/registration-ts")
})
Expand All @@ -252,11 +253,35 @@ func TestRegistration(t *testing.T) {
res := run(t, false, url.Values{
"traits.username": {"registration-identifier-5"},
"password": {x.NewUUID().String()},
"csrf_token":{x.FakeCSRFToken},
}.Encode())
assert.Contains(t, res.Request.URL.String(), uiTS.URL+"/registration-ts")
})
})

t.Run("case=should fail because of missing CSRF token/type=browser", func(t *testing.T) {
rr := newRegistrationRequest(t, time.Minute, false)
body, _ := makeRequest(t, rr.ID, false, url.Values{
"csrf_token": {"invalid_token"},
"traits.username": {"registration-identifier-csrf-browser"},
"password": {x.NewUUID().String()},
"traits.foobar": {"bar"},
}.Encode(), http.StatusOK)
assertx.EqualAsJSON(t, x.ErrInvalidCSRFToken,
json.RawMessage(gjson.GetBytes(body, "0").Raw), "%s", body)
})

t.Run("case=should pass even without CSRF token/type=api", func(t *testing.T) {
rr := newRegistrationRequest(t, time.Minute, true)
body, _ := makeRequest(t, rr.ID, true, `{
"csrf_token": "invalid_token",
"traits.username": "registration-identifier-csrf-api",
"traits.foobar": "bar",
"password": "5216f2ef-f14b-4c92-bd91-08c2c2fe1448"
}`, http.StatusOK)
assert.NotEmpty(t, gjson.GetBytes(body, "identity.id").Raw, "%s", body) // registration successful
})

t.Run("case=should fail because schema did not specify an identifier", func(t *testing.T) {
viper.Set(configuration.ViperKeyDefaultIdentitySchemaURL, "file://./stub/missing-identifier.schema.json")
run := func(t *testing.T, isAPI bool, payload string) ([]byte, *http.Response) {
Expand All @@ -277,6 +302,7 @@ func TestRegistration(t *testing.T) {
"traits.username": {"registration-identifier-6"},
"password": {x.NewUUID().String()},
"traits.foobar": {"bar"},
"csrf_token":{x.FakeCSRFToken},
}.Encode())
assert.Contains(t, res.Request.URL.String(), errTS.URL)
assert.Equal(t, int64(http.StatusInternalServerError), gjson.GetBytes(body, "0.code").Int(), "%s", body)
Expand All @@ -285,29 +311,6 @@ func TestRegistration(t *testing.T) {
})
})

t.Run("case=should fail because of missing CSRF token/type=browser", func(t *testing.T) {
rr := newRegistrationRequest(t, time.Minute, false)
body, _ := makeRequest(t, rr.ID, false, url.Values{
"csrf_token": {"invalid_token"},
"traits.username": {"registration-identifier-csrf-browser"},
"password": {x.NewUUID().String()},
"traits.foobar": {"bar"},
}.Encode(), http.StatusOK)
assertx.EqualAsJSON(t, x.ErrInvalidCSRFToken,
json.RawMessage(gjson.GetBytes(body, "0").Raw), "%s", body)
})

t.Run("case=should pass even without CSRF token/type=api", func(t *testing.T) {
rr := newRegistrationRequest(t, time.Minute, true)
body, _ := makeRequest(t, rr.ID, true, `{
"csrf_token": "invalid_token",
"traits.username": "registration-identifier-csrf-api",
"traits.foobar": "bar",
"password": "5216f2ef-f14b-4c92-bd91-08c2c2fe1448"
}`, http.StatusOK)
assert.NotEmpty(t, gjson.GetBytes(body, "identity.id").Raw, "%s", body) // registration successful
})

t.Run("case=should fail because schema does not exist", func(t *testing.T) {
viper.Set(configuration.ViperKeyDefaultIdentitySchemaURL, "file://./stub/i-do-not-exist.schema.json")

Expand All @@ -330,6 +333,7 @@ func TestRegistration(t *testing.T) {
"traits.username": {"registration-identifier-7"},
"password": {x.NewUUID().String()},
"traits.foobar": {"bar"},
"csrf_token":{x.FakeCSRFToken},
}.Encode())
assert.Contains(t, res.Request.URL.String(), errTS.URL)
assert.Equal(t, int64(http.StatusInternalServerError), gjson.GetBytes(body, "0.code").Int(), "%s", body)
Expand Down Expand Up @@ -363,6 +367,7 @@ func TestRegistration(t *testing.T) {
"traits.username": {"registration-identifier-8-browser"},
"password": {x.NewUUID().String()},
"traits.foobar": {"bar"},
"csrf_token":{x.FakeCSRFToken},
}.Encode())
assert.Contains(t, res.Request.URL.String(), redirTS.URL+"/registration-return-ts")
assert.Equal(t, `registration-identifier-8-browser`, gjson.GetBytes(body, "identity.traits.username").String(), "%s", body)
Expand Down Expand Up @@ -390,12 +395,14 @@ func TestRegistration(t *testing.T) {
"traits.username": {"registration-identifier-8-browser-duplicate"},
"password": {x.NewUUID().String()},
"traits.foobar": {"bar"},
"csrf_token":{x.FakeCSRFToken},
}.Encode(), http.StatusOK)

body, res := run(t, false, url.Values{
"traits.username": {"registration-identifier-8-browser-duplicate"},
"password": {x.NewUUID().String()},
"traits.foobar": {"bar"},
"csrf_token":{x.FakeCSRFToken},
}.Encode(), http.StatusOK)
assert.Contains(t, res.Request.URL.Path, "registration-ts")
assert.Contains(t, gjson.GetBytes(body, "methods.password.config.messages.0.text").String(), "An account with the same identifier (email, phone, username, ...) exists already.", "%s", body)
Expand All @@ -412,6 +419,7 @@ func TestRegistration(t *testing.T) {
rr := &registration.Flow{
ID: x.NewUUID(),
ExpiresAt: time.Now().Add(time.Minute),
CSRFToken: x.FakeCSRFToken,
Type: ft,
Methods: map[identity.CredentialsType]*registration.FlowMethod{
identity.CredentialsTypePassword: {
Expand Down Expand Up @@ -460,6 +468,7 @@ func TestRegistration(t *testing.T) {
res := run(t, false, url.Values{
"traits.username": {"registration-identifier-9"},
"password": {x.NewUUID().String()},
"csrf_token":{x.FakeCSRFToken},
}.Encode())
assert.Contains(t, res.Request.URL.Path, "registration-ts")
})
Expand Down Expand Up @@ -487,6 +496,7 @@ func TestRegistration(t *testing.T) {
"traits.username": {"registration-identifier-10-browser"},
"password": {"93172388957812344432"},
"traits.foobar": {"bar"},
"csrf_token":{x.FakeCSRFToken},
}.Encode())
assert.Equal(t, res.Request.URL.String(), redirTS.URL+"/registration-return-ts")
assert.Equal(t, `registration-identifier-10-browser`, gjson.GetBytes(body, "identity.traits.username").String(), "%s", body)
Expand Down Expand Up @@ -526,6 +536,7 @@ func TestRegistration(t *testing.T) {
"traits.username": {"registration-identifier-11-browser"},
"password": {"O(lf<ys87LÖ:(h<dsjfl"},
"traits.foobar": {"bar"},
"csrf_token":{x.FakeCSRFToken},
}.Encode()

body1, res1 := makeRequestWithCookieJar(t, newRegistrationRequest(t, time.Minute, false).ID,
Expand Down

0 comments on commit 625ef5e

Please sign in to comment.