From 180828eb507ab239a9c6589f747a6816b6e50074 Mon Sep 17 00:00:00 2001 From: Patrik Date: Fri, 1 Dec 2023 12:34:28 +0100 Subject: [PATCH 1/2] feat: extract identifier label for login from default identity schema (#3645) --- identity/validator.go | 4 +- schema/extension.go | 50 ++++-- selfservice/flow/login/error_test.go | 2 + .../flow/login/extension_identifier_label.go | 70 +++++++++ .../login/extension_identifier_label_test.go | 143 ++++++++++++++++++ selfservice/flow/login/handler_test.go | 2 + selfservice/strategy/code/strategy.go | 14 +- selfservice/strategy/code/strategy_login.go | 11 +- selfservice/strategy/password/login.go | 10 +- selfservice/strategy/webauthn/login.go | 11 +- .../profiles/email/login/ui.spec.ts | 2 +- 11 files changed, 294 insertions(+), 25 deletions(-) create mode 100644 selfservice/flow/login/extension_identifier_label.go create mode 100644 selfservice/flow/login/extension_identifier_label_test.go diff --git a/identity/validator.go b/identity/validator.go index 878162e0bd7b..3bd8f9476fa5 100644 --- a/identity/validator.go +++ b/identity/validator.go @@ -35,8 +35,8 @@ func NewValidator(d validatorDependencies) *Validator { return &Validator{v: schema.NewValidator(), d: d} } -func (v *Validator) ValidateWithRunner(ctx context.Context, i *Identity, runners ...schema.Extension) error { - runner, err := schema.NewExtensionRunner(ctx, runners...) +func (v *Validator) ValidateWithRunner(ctx context.Context, i *Identity, runners ...schema.ValidateExtension) error { + runner, err := schema.NewExtensionRunner(ctx, schema.WithValidateRunners(runners...)) if err != nil { return err } diff --git a/schema/extension.go b/schema/extension.go index 5b605cfb91d4..4e15aebf2a0a 100644 --- a/schema/extension.go +++ b/schema/extension.go @@ -41,30 +41,41 @@ type ( Recovery struct { Via string `json:"via"` } `json:"recovery"` - Mappings struct { - Identity struct { - Traits []struct { - Path string `json:"path"` - } `json:"traits"` - } `json:"identity"` - } `json:"mappings"` } - Extension interface { + ValidateExtension interface { Run(ctx jsonschema.ValidationContext, config ExtensionConfig, value interface{}) error Finish() error } + CompileExtension interface { + Run(ctx jsonschema.CompilerContext, config ExtensionConfig, rawSchema map[string]interface{}) error + } ExtensionRunner struct { meta *jsonschema.Schema compile func(ctx jsonschema.CompilerContext, m map[string]interface{}) (interface{}, error) validate func(ctx jsonschema.ValidationContext, s interface{}, v interface{}) error - runners []Extension + validateRunners []ValidateExtension + compileRunners []CompileExtension } + + ExtensionRunnerOption func(*ExtensionRunner) ) -func NewExtensionRunner(ctx context.Context, runners ...Extension) (*ExtensionRunner, error) { +func WithValidateRunners(runners ...ValidateExtension) ExtensionRunnerOption { + return func(r *ExtensionRunner) { + r.validateRunners = append(r.validateRunners, runners...) + } +} + +func WithCompileRunners(runners ...CompileExtension) ExtensionRunnerOption { + return func(r *ExtensionRunner) { + r.compileRunners = append(r.compileRunners, runners...) + } +} + +func NewExtensionRunner(ctx context.Context, opts ...ExtensionRunnerOption) (*ExtensionRunner, error) { var err error r := new(ExtensionRunner) c := jsonschema.NewCompiler() @@ -90,6 +101,12 @@ func NewExtensionRunner(ctx context.Context, runners ...Extension) (*ExtensionRu return nil, errors.WithStack(err) } + for _, runner := range r.compileRunners { + if err := runner.Run(ctx, e, m); err != nil { + return nil, err + } + } + return &e, nil } return nil, nil @@ -101,7 +118,7 @@ func NewExtensionRunner(ctx context.Context, runners ...Extension) (*ExtensionRu return nil } - for _, runner := range r.runners { + for _, runner := range r.validateRunners { if err := runner.Run(ctx, *c, v); err != nil { return err } @@ -109,7 +126,10 @@ func NewExtensionRunner(ctx context.Context, runners ...Extension) (*ExtensionRu return nil } - r.runners = runners + for _, opt := range opts { + opt(r) + } + return r, nil } @@ -126,13 +146,13 @@ func (r *ExtensionRunner) Extension() jsonschema.Extension { } } -func (r *ExtensionRunner) AddRunner(run Extension) *ExtensionRunner { - r.runners = append(r.runners, run) +func (r *ExtensionRunner) AddRunner(run ValidateExtension) *ExtensionRunner { + r.validateRunners = append(r.validateRunners, run) return r } func (r *ExtensionRunner) Finish() error { - for _, runner := range r.runners { + for _, runner := range r.validateRunners { if err := runner.Finish(); err != nil { return err } diff --git a/selfservice/flow/login/error_test.go b/selfservice/flow/login/error_test.go index c6b44b75cf3e..5cc78c35bda1 100644 --- a/selfservice/flow/login/error_test.go +++ b/selfservice/flow/login/error_test.go @@ -43,6 +43,8 @@ func TestHandleError(t *testing.T) { conf, reg := internal.NewFastRegistryWithMocks(t) public, _ := testhelpers.NewKratosServer(t, reg) + testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/password.schema.json") + router := httprouter.New() ts := httptest.NewServer(router) t.Cleanup(ts.Close) diff --git a/selfservice/flow/login/extension_identifier_label.go b/selfservice/flow/login/extension_identifier_label.go new file mode 100644 index 000000000000..8eec2b68d877 --- /dev/null +++ b/selfservice/flow/login/extension_identifier_label.go @@ -0,0 +1,70 @@ +// Copyright © 2023 Ory Corp +// SPDX-License-Identifier: Apache-2.0 + +package login + +import ( + "context" + "sort" + + "github.com/ory/kratos/text" + + "github.com/ory/jsonschema/v3" + "github.com/ory/kratos/schema" +) + +type identifierLabelExtension struct { + identifierLabelCandidates []string +} + +var _ schema.CompileExtension = new(identifierLabelExtension) + +func GetIdentifierLabelFromSchema(ctx context.Context, schemaURL string) (*text.Message, error) { + ext := &identifierLabelExtension{} + + runner, err := schema.NewExtensionRunner(ctx, schema.WithCompileRunners(ext)) + if err != nil { + return nil, err + } + c := jsonschema.NewCompiler() + runner.Register(c) + + _, err = c.Compile(ctx, schemaURL) + if err != nil { + return nil, err + } + metaLabel := text.NewInfoNodeLabelID() + if label := ext.getLabel(); label != "" { + metaLabel = text.NewInfoNodeLabelGenerated(label) + } + return metaLabel, nil +} + +func (i *identifierLabelExtension) Run(_ jsonschema.CompilerContext, config schema.ExtensionConfig, rawSchema map[string]interface{}) error { + if config.Credentials.Password.Identifier || + config.Credentials.WebAuthn.Identifier || + config.Credentials.TOTP.AccountName || + config.Credentials.Code.Identifier { + if title, ok := rawSchema["title"]; ok { + // The jsonschema compiler validates the title to be a string, so this should always work. + switch t := title.(type) { + case string: + if t != "" { + i.identifierLabelCandidates = append(i.identifierLabelCandidates, t) + } + } + } + } + return nil +} + +func (i *identifierLabelExtension) getLabel() string { + if len(i.identifierLabelCandidates) == 0 { + // sane default is set elsewhere + return "" + } + // sort the candidates to get a deterministic result + sort.Strings(i.identifierLabelCandidates) + // just take the first, no good way to decide which one is the best + return i.identifierLabelCandidates[0] +} diff --git a/selfservice/flow/login/extension_identifier_label_test.go b/selfservice/flow/login/extension_identifier_label_test.go new file mode 100644 index 000000000000..f504239408e0 --- /dev/null +++ b/selfservice/flow/login/extension_identifier_label_test.go @@ -0,0 +1,143 @@ +// Copyright © 2023 Ory Corp +// SPDX-License-Identifier: Apache-2.0 + +package login + +import ( + "context" + "encoding/base64" + "encoding/json" + "fmt" + "testing" + + "github.com/ory/kratos/text" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/tidwall/sjson" + + "github.com/ory/kratos/schema" +) + +func constructSchema(t *testing.T, ecModifier, ucModifier func(*schema.ExtensionConfig)) string { + var emailConfig, usernameConfig schema.ExtensionConfig + + if ecModifier != nil { + ecModifier(&emailConfig) + } + if ucModifier != nil { + ucModifier(&usernameConfig) + } + + ec, err := json.Marshal(&emailConfig) + require.NoError(t, err) + uc, err := json.Marshal(&usernameConfig) + require.NoError(t, err) + + ec, err = sjson.DeleteBytes(ec, "verification") + require.NoError(t, err) + ec, err = sjson.DeleteBytes(ec, "recovery") + require.NoError(t, err) + ec, err = sjson.DeleteBytes(ec, "credentials.code.via") + require.NoError(t, err) + uc, err = sjson.DeleteBytes(uc, "verification") + require.NoError(t, err) + uc, err = sjson.DeleteBytes(uc, "recovery") + require.NoError(t, err) + uc, err = sjson.DeleteBytes(uc, "credentials.code.via") + require.NoError(t, err) + + return "base64://" + base64.StdEncoding.EncodeToString([]byte(fmt.Sprintf(` +{ + "properties": { + "traits": { + "properties": { + "email": { + "title": "Email", + "ory.sh/kratos": %s + }, + "username": { + "title": "Username", + "ory.sh/kratos": %s + } + } + } + } +}`, ec, uc))) +} + +func TestGetIdentifierLabelFromSchema(t *testing.T) { + ctx := context.Background() + + for _, tc := range []struct { + name string + emailConfig, usernameConfig func(*schema.ExtensionConfig) + expected *text.Message + }{ + { + name: "email for password", + emailConfig: func(c *schema.ExtensionConfig) { + c.Credentials.Password.Identifier = true + }, + expected: text.NewInfoNodeLabelGenerated("Email"), + }, + { + name: "email for webauthn", + emailConfig: func(c *schema.ExtensionConfig) { + c.Credentials.WebAuthn.Identifier = true + }, + expected: text.NewInfoNodeLabelGenerated("Email"), + }, + { + name: "email for totp", + emailConfig: func(c *schema.ExtensionConfig) { + c.Credentials.TOTP.AccountName = true + }, + expected: text.NewInfoNodeLabelGenerated("Email"), + }, + { + name: "email for code", + emailConfig: func(c *schema.ExtensionConfig) { + c.Credentials.Code.Identifier = true + }, + expected: text.NewInfoNodeLabelGenerated("Email"), + }, + { + name: "email for all", + emailConfig: func(c *schema.ExtensionConfig) { + c.Credentials.Password.Identifier = true + c.Credentials.WebAuthn.Identifier = true + c.Credentials.TOTP.AccountName = true + c.Credentials.Code.Identifier = true + }, + expected: text.NewInfoNodeLabelGenerated("Email"), + }, + { + name: "username works as well", + usernameConfig: func(c *schema.ExtensionConfig) { + c.Credentials.Password.Identifier = true + }, + expected: text.NewInfoNodeLabelGenerated("Username"), + }, + { + name: "multiple identifiers", + emailConfig: func(c *schema.ExtensionConfig) { + c.Credentials.Password.Identifier = true + }, + usernameConfig: func(c *schema.ExtensionConfig) { + c.Credentials.Password.Identifier = true + }, + expected: text.NewInfoNodeLabelGenerated("Email"), + }, + { + name: "no identifiers", + expected: text.NewInfoNodeLabelID(), + }, + } { + t.Run(tc.name, func(t *testing.T) { + label, err := GetIdentifierLabelFromSchema(ctx, constructSchema(t, tc.emailConfig, tc.usernameConfig)) + require.NoError(t, err) + assert.Equal(t, tc.expected, label) + }) + } +} diff --git a/selfservice/flow/login/handler_test.go b/selfservice/flow/login/handler_test.go index 85daee4e65a9..9a82e85ccbc7 100644 --- a/selfservice/flow/login/handler_test.go +++ b/selfservice/flow/login/handler_test.go @@ -797,6 +797,8 @@ func TestGetFlow(t *testing.T) { _ = testhelpers.NewErrorTestServer(t, reg) _ = testhelpers.NewRedirTS(t, "", conf) + testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/password.schema.json") + setupLoginUI := func(t *testing.T, c *http.Client) *httptest.Server { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { // It is important that we use a HTTP request to fetch the flow because that will show us if CSRF works or not diff --git a/selfservice/strategy/code/strategy.go b/selfservice/strategy/code/strategy.go index 8b21a89a69e2..a5d0c83703ec 100644 --- a/selfservice/strategy/code/strategy.go +++ b/selfservice/strategy/code/strategy.go @@ -149,9 +149,17 @@ func (s *Strategy) PopulateMethod(r *http.Request, f flow.Flow) error { WithMetaLabel(text.NewInfoNodeInputEmail()), ) } else if f.GetFlowName() == flow.LoginFlow { - // we use the identifier label here since we don't know what - // type of field the identifier is - nodes.Upsert(node.NewInputField("identifier", "", node.DefaultGroup, node.InputAttributeTypeText, node.WithRequiredInputAttribute).WithMetaLabel(text.NewInfoNodeLabelID())) + ds, err := s.deps.Config().DefaultIdentityTraitsSchemaURL(r.Context()) + if err != nil { + return err + } + + identifierLabel, err := login.GetIdentifierLabelFromSchema(r.Context(), ds.String()) + if err != nil { + return err + } + + nodes.Upsert(node.NewInputField("identifier", "", node.DefaultGroup, node.InputAttributeTypeText, node.WithRequiredInputAttribute).WithMetaLabel(identifierLabel)) } else if f.GetFlowName() == flow.RegistrationFlow { ds, err := s.deps.Config().DefaultIdentityTraitsSchemaURL(r.Context()) if err != nil { diff --git a/selfservice/strategy/code/strategy_login.go b/selfservice/strategy/code/strategy_login.go index 64a3ab13a325..1a0170d36648 100644 --- a/selfservice/strategy/code/strategy_login.go +++ b/selfservice/strategy/code/strategy_login.go @@ -22,7 +22,6 @@ import ( "github.com/ory/kratos/selfservice/flow" "github.com/ory/kratos/selfservice/flow/login" "github.com/ory/kratos/session" - "github.com/ory/kratos/text" "github.com/ory/kratos/ui/node" "github.com/ory/kratos/x" "github.com/ory/x/decoderx" @@ -79,10 +78,18 @@ func (s *Strategy) HandleLoginError(r *http.Request, f *login.Flow, body *update email = body.Identifier } + ds, err := s.deps.Config().DefaultIdentityTraitsSchemaURL(r.Context()) + if err != nil { + return err + } + identifierLabel, err := login.GetIdentifierLabelFromSchema(r.Context(), ds.String()) + if err != nil { + return err + } f.UI.SetCSRF(s.deps.GenerateCSRFToken(r)) f.UI.GetNodes().Upsert( node.NewInputField("identifier", email, node.DefaultGroup, node.InputAttributeTypeText, node.WithRequiredInputAttribute). - WithMetaLabel(text.NewInfoNodeLabelID()), + WithMetaLabel(identifierLabel), ) } diff --git a/selfservice/strategy/password/login.go b/selfservice/strategy/password/login.go index 7a56ebaf45d4..5c9ed653872d 100644 --- a/selfservice/strategy/password/login.go +++ b/selfservice/strategy/password/login.go @@ -147,7 +147,15 @@ func (s *Strategy) PopulateLoginMethod(r *http.Request, requestedAAL identity.Au sr.UI.SetCSRF(s.d.GenerateCSRFToken(r)) sr.UI.SetNode(node.NewInputField("identifier", identifier, node.DefaultGroup, node.InputAttributeTypeHidden)) } else { - sr.UI.SetNode(node.NewInputField("identifier", "", node.DefaultGroup, node.InputAttributeTypeText, node.WithRequiredInputAttribute).WithMetaLabel(text.NewInfoNodeLabelID())) + ds, err := s.d.Config().DefaultIdentityTraitsSchemaURL(r.Context()) + if err != nil { + return err + } + identifierLabel, err := login.GetIdentifierLabelFromSchema(r.Context(), ds.String()) + if err != nil { + return err + } + sr.UI.SetNode(node.NewInputField("identifier", "", node.DefaultGroup, node.InputAttributeTypeText, node.WithRequiredInputAttribute).WithMetaLabel(identifierLabel)) } sr.UI.SetCSRF(s.d.GenerateCSRFToken(r)) diff --git a/selfservice/strategy/webauthn/login.go b/selfservice/strategy/webauthn/login.go index 7b968c704151..6b9ee3a154f9 100644 --- a/selfservice/strategy/webauthn/login.go +++ b/selfservice/strategy/webauthn/login.go @@ -90,8 +90,17 @@ func (s *Strategy) populateLoginMethodForPasswordless(r *http.Request, sr *login return nil } + ds, err := s.d.Config().DefaultIdentityTraitsSchemaURL(r.Context()) + if err != nil { + return err + } + identifierLabel, err := login.GetIdentifierLabelFromSchema(r.Context(), ds.String()) + if err != nil { + return err + } + sr.UI.SetCSRF(s.d.GenerateCSRFToken(r)) - sr.UI.SetNode(node.NewInputField("identifier", "", node.DefaultGroup, node.InputAttributeTypeText, node.WithRequiredInputAttribute).WithMetaLabel(text.NewInfoNodeLabelID())) + sr.UI.SetNode(node.NewInputField("identifier", "", node.DefaultGroup, node.InputAttributeTypeText, node.WithRequiredInputAttribute).WithMetaLabel(identifierLabel)) sr.UI.GetNodes().Append(node.NewInputField("method", "webauthn", node.WebAuthnGroup, node.InputAttributeTypeSubmit).WithMetaLabel(text.NewInfoSelfServiceLoginWebAuthn())) return nil } diff --git a/test/e2e/cypress/integration/profiles/email/login/ui.spec.ts b/test/e2e/cypress/integration/profiles/email/login/ui.spec.ts index 8792dd14cb64..ff65ea6ca6b4 100644 --- a/test/e2e/cypress/integration/profiles/email/login/ui.spec.ts +++ b/test/e2e/cypress/integration/profiles/email/login/ui.spec.ts @@ -31,7 +31,7 @@ context("UI tests using the email profile", () => { it("should use the json schema titles", () => { cy.get(`${appPrefix(app)}input[name="identifier"]`) .parent() - .should("contain.text", "ID") + .should("contain.text", "Your E-Mail") cy.get('input[name="password"]') .parentsUntil("label") From bbf874fd7f7c6e5d51b11f39d989a17039a6e955 Mon Sep 17 00:00:00 2001 From: ory-bot <60093411+ory-bot@users.noreply.github.com> Date: Fri, 1 Dec 2023 12:19:30 +0000 Subject: [PATCH 2/2] autogen(docs): regenerate and update changelog [skip ci] --- CHANGELOG.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e44818aab4e..0b1a4f811f75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ **Table of Contents** -- [ (2023-11-29)](#2023-11-29) +- [ (2023-12-01)](#2023-12-01) - [Breaking Changes](#breaking-changes) - [Bug Fixes](#bug-fixes) - [Documentation](#documentation) @@ -314,7 +314,7 @@ -# [](https://github.com/ory/kratos/compare/v1.0.0...v) (2023-11-29) +# [](https://github.com/ory/kratos/compare/v1.0.0...v) (2023-12-01) ## Breaking Changes @@ -844,6 +844,9 @@ https://github.com/ory/kratos/pull/3480 This feature depends on Cockroach functionality and configuration, and is not possible for MySQL or PostgreSQL. +- Extract identifier label for login from default identity schema + ([#3645](https://github.com/ory/kratos/issues/3645)) + ([180828e](https://github.com/ory/kratos/commit/180828eb507ab239a9c6589f747a6816b6e50074)) - Fine-grained hooks for all available flow methods ([#3519](https://github.com/ory/kratos/issues/3519)) ([a37f6bd](https://github.com/ory/kratos/commit/a37f6bddc48443b2fc464699fa5c2922f64d81f6)):