Skip to content

Commit

Permalink
fix: resolve failing test cases
Browse files Browse the repository at this point in the history
  • Loading branch information
aeneasr committed Aug 27, 2020
1 parent be4c7e4 commit f8647b4
Show file tree
Hide file tree
Showing 8 changed files with 83 additions and 50 deletions.
Expand Up @@ -25,6 +25,17 @@
"via": "email"
}
}
},
"name": {
"type": "object",
"properties": {
"first": {
"type": "string"
},
"last": {
"type": "string"
}
}
}
},
"required": [
Expand Down
Expand Up @@ -78,7 +78,9 @@ selfservice:
# provider, client_id, issuer_url, scope, ...
```

## Registration
## Browser Clients

### Registration

Redirecting the browser to the
[Self-Service Login and Registration Endpoint](../user-login-user-registration.mdx#user-login-and-user-registration-process-sequence)
Expand All @@ -87,7 +89,9 @@ is configued, the Registration Request Response Payload will include an `oidc`
method. The method contains different providers, based on your OpenID Connect
Provider configuration:

```json5 title="$ curl http://<kratos-admin>/self-service/browser/flows/requests/registration?request=54a9f113-3a7b-4cb8-a553-f072aa4e1622"
```
curl http://<kratos-admin>/self-service/browser/flows/requests/registration?request=54a9f113-3a7b-4cb8-a553-f072aa4e1622
{
id: '54a9f113-3a7b-4cb8-a553-f072aa4e1622',
expires_at: '2020-05-15T11:04:09.342537Z',
Expand Down Expand Up @@ -219,7 +223,9 @@ filled out. Please note that these claims are just examples, Google doesn't
actually use email addresses in `sub` fields and also does not include the
`website` claim:

```json5 title="$ curl http://<kratos-admin>/self-service/browser/flows/requests/registration?request=54a9f113-3a7b-4cb8-a553-f072aa4e1622"
```shell script
$ curl http://<kratos-admin>/self-service/browser/flows/requests/registration?request=54a9f113-3a7b-4cb8-a553-f072aa4e1622"
{
id: '54a9f113-3a7b-4cb8-a553-f072aa4e1622',
// ...
Expand Down Expand Up @@ -271,7 +277,12 @@ actually use email addresses in `sub` fields and also does not include the
}
```
## Login
If the form is valid, ORY Kratos will create the user and respond with a HTTP 302 redirect
to the configured redirect URL.
Unless the `session` hook is configured, no session cookie will be included in the `Set-Cookie` HTTP header.
### Login
Redirecting the browser to the
[Self-Service Login and Registration Endpoint](../user-login-user-registration.mdx#user-login-and-user-registration-process-sequence)
Expand Down Expand Up @@ -376,7 +387,13 @@ apply for `provider: github`):
}
```
The flow completes otherwise.
If the form is valid (the user exists and the password is correct), ORY Kratos will respond with a HTTP 302 redirect
to the configured redirect URL and set a session cookie using the `Set-Cookie` HTTP Header.
## API Clients
Social Sign In is currently not possible for API Clients. It will be possible in a future version, which is
partially tracked as [kratos#273](https://github.com/ory/kratos/issues/273)
## Security and Defenses
Expand Down
20 changes: 2 additions & 18 deletions docs/docs/self-service/flows/user-settings.mdx
Expand Up @@ -22,23 +22,7 @@ strategy.

## Self-Service User Settings for Browser Applications

ORY Kratos supports browser applications that run on server-side (e.g. Java,
NodeJS, PHP) as well as client-side (e.g. JQuery, ReactJS, AngularJS, ...).

Browser-based user settings management makes use of three core HTTP
technologies:

- HTTP Redirects
- HTTP POST (`application/json`, `application/x-www-urlencoded`) and RESTful GET
requests.
- HTTP Cookies to prevent CSRF and Session Hijaking attack vectors.

The browser flow is the easiest and most secure to set up and integrated with.
ORY Kratos takes care of all required session and CSRF cookies and ensures that
all security requirements are fulfilled.

This flow is not suitable for scenarios where you use purely programmatic
clients that do not work well with HTTP Cookies and HTTP Redirects.
This flow follows the already established [Browser Flow](../../self-service#browser-flows).

### The Settings User Interface

Expand Down Expand Up @@ -451,4 +435,4 @@ Will be addressed in a future release.

ORY Kratos allows you to configure hooks that run before and after a profile
update was successful. For more information about hooks please read the
[Hook Documentation](../hooks/index).
[Hook Documentation](../hooks).
2 changes: 1 addition & 1 deletion persistence/sql/persister_test.go
Expand Up @@ -93,7 +93,7 @@ func TestPersister(t *testing.T) {
}

var l sync.Mutex
if !testing.Short() && false {
if !testing.Short() {
funcs := map[string]func(t *testing.T) string{
"postgres": dockertest.RunTestPostgreSQL,
"mysql": dockertest.RunTestMySQL,
Expand Down
12 changes: 8 additions & 4 deletions selfservice/flow/login/persistence.go
Expand Up @@ -2,7 +2,6 @@ package login

import (
"context"
"encoding/json"
"testing"

"github.com/bxcodec/faker/v3"
Expand Down Expand Up @@ -187,9 +186,14 @@ func TestFlowPersister(p FlowPersister) func(t *testing.T) {
require.Len(t, actual.Methods, 2)
assert.EqualValues(t, identity.CredentialsTypePassword, actual.Active)

js, _ := json.Marshal(actual.Methods)
assert.Equal(t, string(identity.CredentialsTypePassword), actual.Methods[identity.CredentialsTypePassword].Config.FlowMethodConfigurator.(*form.HTMLForm).Action, "%s", js)
assert.Equal(t, string(identity.CredentialsTypeOIDC), actual.Methods[identity.CredentialsTypeOIDC].Config.FlowMethodConfigurator.(*form.HTMLForm).Action)
assert.Equal(t,
expected.Methods[identity.CredentialsTypePassword].Config.FlowMethodConfigurator.(*form.HTMLForm).Action,
actual.Methods[identity.CredentialsTypePassword].Config.FlowMethodConfigurator.(*form.HTMLForm).Action,
)
assert.Equal(t,
expected.Methods[identity.CredentialsTypeOIDC].Config.FlowMethodConfigurator.(*form.HTMLForm).Action,
actual.Methods[identity.CredentialsTypeOIDC].Config.FlowMethodConfigurator.(*form.HTMLForm).Action,
)
})
}
}
11 changes: 8 additions & 3 deletions selfservice/flow/registration/persistence.go
Expand Up @@ -134,9 +134,14 @@ func TestFlowPersister(p FlowPersister) func(t *testing.T) {
require.Len(t, actual.Methods, 2)
assert.EqualValues(t, identity.CredentialsTypePassword, actual.Active)

js, _ := json.Marshal(actual.Methods)
assert.Equal(t, string(identity.CredentialsTypePassword), actual.Methods[identity.CredentialsTypePassword].Config.FlowMethodConfigurator.(*form.HTMLForm).Action, "%s", js)
assert.Equal(t, string(identity.CredentialsTypeOIDC), actual.Methods[identity.CredentialsTypeOIDC].Config.FlowMethodConfigurator.(*form.HTMLForm).Action)
assert.Equal(t,
expected.Methods[identity.CredentialsTypePassword].Config.FlowMethodConfigurator.(*form.HTMLForm).Action,
actual.Methods[identity.CredentialsTypePassword].Config.FlowMethodConfigurator.(*form.HTMLForm).Action,
)
assert.Equal(t,
expected.Methods[identity.CredentialsTypeOIDC].Config.FlowMethodConfigurator.(*form.HTMLForm).Action,
actual.Methods[identity.CredentialsTypeOIDC].Config.FlowMethodConfigurator.(*form.HTMLForm).Action,
)
})
}
}
14 changes: 8 additions & 6 deletions selfservice/hook/session_issuer_test.go
Expand Up @@ -8,6 +8,7 @@ import (
"testing"
"time"

"github.com/ory/x/randx"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/tidwall/gjson"
Expand Down Expand Up @@ -40,7 +41,7 @@ func TestSessionIssuer(t *testing.T) {
i := identity.NewIdentity(configuration.DefaultIdentityTraitsSchemaID)
require.NoError(t, reg.PrivilegedIdentityPool().CreateIdentity(context.Background(), i))
require.NoError(t, h.ExecutePostRegistrationPostPersistHook(w, &r,
&registration.Flow{Type: flow.TypeBrowser}, &session.Session{ID: sid, Identity: i}))
&registration.Flow{Type: flow.TypeBrowser}, &session.Session{ID: sid, Identity: i, Token: randx.MustString(12, randx.AlphaLowerNum)}))

got, err := reg.SessionPersister().GetSession(context.Background(), sid)
require.NoError(t, err)
Expand All @@ -54,21 +55,22 @@ func TestSessionIssuer(t *testing.T) {
w := httptest.NewRecorder()

i := identity.NewIdentity(configuration.DefaultIdentityTraitsSchemaID)
s := &session.Session{ID: x.NewUUID(), Identity: i}
s := &session.Session{ID: x.NewUUID(), Identity: i, Token: randx.MustString(12, randx.AlphaLowerNum)}
f := &registration.Flow{Type: flow.TypeAPI}

require.NoError(t, reg.PrivilegedIdentityPool().CreateIdentity(context.Background(), i))
require.True(t, errors.Is(h.ExecutePostRegistrationPostPersistHook(w, &r, f, s), registration.ErrHookAbortFlow))
err := h.ExecutePostRegistrationPostPersistHook(w, &r, f, s)
require.True(t, errors.Is(err, registration.ErrHookAbortFlow), "%+v", err)

got, err := reg.SessionPersister().GetSession(context.Background(), s.ID)
require.NoError(t, err)
assert.Equal(t, s.ID, got.ID)
assert.Equal(t, s.ID.String(), got.ID.String())
assert.True(t, got.AuthenticatedAt.After(time.Now().Add(-time.Minute)))

assert.Empty(t, w.Header().Get("Set-Cookie"))
body := w.Body.Bytes()
assert.Equal(t, i.ID, gjson.GetBytes(body, "identity.id").String())
assert.Equal(t, s.ID, gjson.GetBytes(body, "session.id").String())
assert.Equal(t, i.ID.String(), gjson.GetBytes(body, "identity.id").String())
assert.Equal(t, s.ID.String(), gjson.GetBytes(body, "session.id").String())
assert.Equal(t, got.Token, gjson.GetBytes(body, "session_token").String())
})
})
Expand Down
36 changes: 23 additions & 13 deletions selfservice/strategy/oidc/strategy_settings_test.go
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/ory/kratos/internal/httpclient/client/common"
"github.com/ory/kratos/internal/httpclient/models"
"github.com/ory/kratos/internal/testhelpers"
"github.com/ory/kratos/selfservice/flow"
"github.com/ory/kratos/selfservice/flow/settings"
"github.com/ory/kratos/selfservice/form"
"github.com/ory/kratos/selfservice/strategy/oidc"
Expand All @@ -37,7 +38,7 @@ func init() {
}

var (
csrfField = &models.FormField{Name: pointerx.String("csrf_token"), Value: "nosurf",
csrfField = &models.FormField{Name: pointerx.String("csrf_token"), Value: x.FakeCSRFToken,
Required: true, Type: pointerx.String("hidden")}
)

Expand Down Expand Up @@ -264,7 +265,7 @@ func TestSettingsStrategy(t *testing.T) {
var unlink = func(t *testing.T, agent, provider string) (body []byte, res *http.Response, req *models.SettingsFlow) {
req = nprSDK(t, agents[agent], "", time.Hour)
body, res = testhelpers.HTTPPostForm(t, agents[agent], action(req),
&url.Values{"csrf_token": {"nosurf"}, "unlink": {provider}})
&url.Values{"csrf_token": {x.FakeCSRFToken}, "unlink": {provider}})
return
}

Expand Down Expand Up @@ -337,7 +338,7 @@ func TestSettingsStrategy(t *testing.T) {
viper.Set(configuration.ViperKeySelfServiceSettingsPrivilegedAuthenticationAfter, time.Minute*5)

body, res := testhelpers.HTTPPostForm(t, agents[agent], action(req),
&url.Values{"csrf_token": {"nosurf"}, "unlink": {provider}})
&url.Values{"csrf_token": {x.FakeCSRFToken}, "unlink": {provider}})
assert.Contains(t, res.Request.URL.String(), uiTS.URL+"/settings?flow="+string(req.ID))

assert.Equal(t, "success", gjson.GetBytes(body, "state").String())
Expand All @@ -351,7 +352,7 @@ func TestSettingsStrategy(t *testing.T) {
var link = func(t *testing.T, agent, provider string) (body []byte, res *http.Response, req *models.SettingsFlow) {
req = nprSDK(t, agents[agent], "", time.Hour)
body, res = testhelpers.HTTPPostForm(t, agents[agent], action(req),
&url.Values{"csrf_token": {"nosurf"}, "link": {provider}})
&url.Values{"csrf_token": {x.FakeCSRFToken}, "link": {provider}})
return
}

Expand Down Expand Up @@ -509,7 +510,7 @@ func TestSettingsStrategy(t *testing.T) {
viper.Set(configuration.ViperKeySelfServiceSettingsPrivilegedAuthenticationAfter, time.Minute*5)

body, res := testhelpers.HTTPPostForm(t, agents[agent], action(req),
&url.Values{"csrf_token": {"nosurf"}, "unlink": {provider}})
&url.Values{"csrf_token": {x.FakeCSRFToken}, "unlink": {provider}})
assert.Contains(t, res.Request.URL.String(), uiTS.URL+"/settings?flow="+string(req.ID))

assert.Equal(t, "success", gjson.GetBytes(body, "state").String())
Expand Down Expand Up @@ -542,7 +543,7 @@ func TestPopulateSettingsMethod(t *testing.T) {
}

nr := func() *settings.Flow {
return &settings.Flow{ID: x.NewUUID(), Methods: map[string]*settings.FlowMethod{}}
return &settings.Flow{Type: flow.TypeBrowser, ID: x.NewUUID(), Methods: map[string]*settings.FlowMethod{}}
}

populate := func(t *testing.T, reg *driver.RegistryDefault, i *identity.Identity, req *settings.Flow) *form.HTMLForm {
Expand All @@ -564,6 +565,15 @@ func TestPopulateSettingsMethod(t *testing.T) {
{Provider: "generic", ID: "github"},
}

t.Run("case=should not populate non-browser flow", func(t *testing.T) {
reg := nreg(t, &oidc.ConfigurationCollection{Providers: []oidc.Configuration{{Provider: "generic", ID: "github"}}})
i := &identity.Identity{Traits: []byte(`{"subject":"foo@bar.com"}`)}
require.NoError(t, reg.PrivilegedIdentityPool().CreateIdentity(context.Background(), i))
req := &settings.Flow{Type: flow.TypeAPI, ID: x.NewUUID(), Methods: map[string]*settings.FlowMethod{}}
require.NoError(t, ns(t, reg).PopulateSettingsMethod(new(http.Request), i, req))
require.Nil(t, req.Methods[identity.CredentialsTypeOIDC.String()])
})

for k, tc := range []struct {
c []oidc.Configuration
i identity.Credentials
Expand All @@ -573,22 +583,22 @@ func TestPopulateSettingsMethod(t *testing.T) {
{
c: []oidc.Configuration{},
e: form.Fields{
{Name: "csrf_token", Type: "hidden", Required: true, Value: "nosurf"},
{Name: "csrf_token", Type: "hidden", Required: true, Value: x.FakeCSRFToken},
},
},
{
c: []oidc.Configuration{
{Provider: "generic", ID: "github"},
},
e: form.Fields{
{Name: "csrf_token", Type: "hidden", Required: true, Value: "nosurf"},
{Name: "csrf_token", Type: "hidden", Required: true, Value: x.FakeCSRFToken},
{Name: "link", Type: "submit", Value: "github"},
},
},
{
c: defaultConfig,
e: form.Fields{
{Name: "csrf_token", Type: "hidden", Required: true, Value: "nosurf"},
{Name: "csrf_token", Type: "hidden", Required: true, Value: x.FakeCSRFToken},
{Name: "link", Type: "submit", Value: "facebook"},
{Name: "link", Type: "submit", Value: "google"},
{Name: "link", Type: "submit", Value: "github"},
Expand All @@ -597,7 +607,7 @@ func TestPopulateSettingsMethod(t *testing.T) {
{
c: defaultConfig,
e: form.Fields{
{Name: "csrf_token", Type: "hidden", Required: true, Value: "nosurf"},
{Name: "csrf_token", Type: "hidden", Required: true, Value: x.FakeCSRFToken},
{Name: "link", Type: "submit", Value: "facebook"},
{Name: "link", Type: "submit", Value: "google"},
{Name: "link", Type: "submit", Value: "github"},
Expand All @@ -607,7 +617,7 @@ func TestPopulateSettingsMethod(t *testing.T) {
{
c: defaultConfig,
e: form.Fields{
{Name: "csrf_token", Type: "hidden", Required: true, Value: "nosurf"},
{Name: "csrf_token", Type: "hidden", Required: true, Value: x.FakeCSRFToken},
{Name: "link", Type: "submit", Value: "facebook"},
{Name: "link", Type: "submit", Value: "github"},
},
Expand All @@ -618,7 +628,7 @@ func TestPopulateSettingsMethod(t *testing.T) {
{
c: defaultConfig,
e: form.Fields{
{Name: "csrf_token", Type: "hidden", Required: true, Value: "nosurf"},
{Name: "csrf_token", Type: "hidden", Required: true, Value: x.FakeCSRFToken},
{Name: "link", Type: "submit", Value: "facebook"},
{Name: "link", Type: "submit", Value: "github"},
{Name: "unlink", Type: "submit", Value: "google"},
Expand All @@ -632,7 +642,7 @@ func TestPopulateSettingsMethod(t *testing.T) {
{
c: defaultConfig,
e: form.Fields{
{Name: "csrf_token", Type: "hidden", Required: true, Value: "nosurf"},
{Name: "csrf_token", Type: "hidden", Required: true, Value: x.FakeCSRFToken},
{Name: "link", Type: "submit", Value: "github"},
{Name: "unlink", Type: "submit", Value: "google"},
{Name: "unlink", Type: "submit", Value: "facebook"},
Expand Down

0 comments on commit f8647b4

Please sign in to comment.