From 0fdfcb52eac1213484af389e03f73bb9e34172cf Mon Sep 17 00:00:00 2001 From: Jonas Hungershausen Date: Sat, 6 Jan 2024 13:59:46 +0100 Subject: [PATCH 01/19] feat: support MFA via SMS --- .../phone-password/identity.schema.json | 25 ++++++- courier/sms_templates.go | 6 ++ .../login_code/valid/sms.body.gotmpl | 1 + courier/template/sms/login_code_valid.go | 52 ++++++++++++++ driver/registry_default.go | 7 +- driver/registry_default_test.go | 12 +++- embedx/identity_extension.schema.json | 2 +- identity/credentials_code.go | 2 +- identity/extension_credentials.go | 2 + internal/testhelpers/selfservice_login.go | 4 +- selfservice/flow/login/handler.go | 2 +- selfservice/flow/login/hook.go | 2 +- selfservice/flow/login/strategy.go | 2 +- selfservice/flow/registration/handler_test.go | 3 +- selfservice/flow/request.go | 15 +--- selfservice/flow/request_test.go | 71 ------------------- selfservice/strategy/code/code_sender.go | 28 +++++--- selfservice/strategy/code/strategy.go | 26 +++++-- selfservice/strategy/code/strategy_login.go | 39 ++++++---- .../strategy/code/strategy_login_test.go | 27 +++---- .../code/strategy_registration_test.go | 5 +- selfservice/strategy/lookup/strategy.go | 8 ++- selfservice/strategy/oidc/strategy.go | 2 +- selfservice/strategy/password/strategy.go | 2 +- selfservice/strategy/totp/strategy.go | 10 +-- selfservice/strategy/webauthn/strategy.go | 10 +-- .../strategy/webauthn/strategy_test.go | 4 +- 27 files changed, 203 insertions(+), 166 deletions(-) create mode 100644 courier/template/courier/builtin/templates/login_code/valid/sms.body.gotmpl create mode 100644 courier/template/sms/login_code_valid.go diff --git a/contrib/quickstart/kratos/phone-password/identity.schema.json b/contrib/quickstart/kratos/phone-password/identity.schema.json index 0d986aeb672e..7f757b0a1690 100644 --- a/contrib/quickstart/kratos/phone-password/identity.schema.json +++ b/contrib/quickstart/kratos/phone-password/identity.schema.json @@ -7,6 +7,26 @@ "traits": { "type": "object", "properties": { + "email": { + "type": "string", + "format": "email", + "title": "E-mail", + "minLength": 3, + "ory.sh/kratos": { + "credentials": { + "password": { + "identifier": true + }, + "code": { + "identifier": true, + "via": "email" + } + }, + "verification": { + "via": "email" + } + } + }, "phone": { "type": "string", "format": "tel", @@ -16,6 +36,9 @@ "credentials": { "password": { "identifier": true + }, + "code": { + "identifier": true } }, "verification": { @@ -24,7 +47,7 @@ } } }, - "required": ["phone"], + "required": ["email", "phone"], "additionalProperties": false } } diff --git a/courier/sms_templates.go b/courier/sms_templates.go index 683ba7d98ca3..b560542d53c9 100644 --- a/courier/sms_templates.go +++ b/courier/sms_templates.go @@ -34,6 +34,12 @@ func NewSMSTemplateFromMessage(d template.Dependencies, m Message) (SMSTemplate, return nil, err } return sms.NewTestStub(d, &t), nil + case template.TypeLoginCodeValid: + var t sms.LoginCodeValidModel + if err := json.Unmarshal(m.TemplateData, &t); err != nil { + return nil, err + } + return sms.NewLoginCodeValid(d, &t), nil default: return nil, errors.Errorf("received unexpected message template type: %s", m.TemplateType) diff --git a/courier/template/courier/builtin/templates/login_code/valid/sms.body.gotmpl b/courier/template/courier/builtin/templates/login_code/valid/sms.body.gotmpl new file mode 100644 index 000000000000..5b88dde9a382 --- /dev/null +++ b/courier/template/courier/builtin/templates/login_code/valid/sms.body.gotmpl @@ -0,0 +1 @@ +Your login code is: {{ .LoginCode }} diff --git a/courier/template/sms/login_code_valid.go b/courier/template/sms/login_code_valid.go new file mode 100644 index 000000000000..c72189bc5f3d --- /dev/null +++ b/courier/template/sms/login_code_valid.go @@ -0,0 +1,52 @@ +// Copyright © 2023 Ory Corp +// SPDX-License-Identifier: Apache-2.0 + +package sms + +import ( + "context" + "encoding/json" + "os" + + "github.com/ory/kratos/courier/template" +) + +type ( + LoginCodeValid struct { + deps template.Dependencies + model *LoginCodeValidModel + } + LoginCodeValidModel struct { + To string + LoginCode string + Identity map[string]interface{} + } +) + +func NewLoginCodeValid(d template.Dependencies, m *LoginCodeValidModel) *LoginCodeValid { + return &LoginCodeValid{deps: d, model: m} +} + +func (t *LoginCodeValid) PhoneNumber() (string, error) { + return t.model.To, nil +} + +func (t *LoginCodeValid) SMSBody(ctx context.Context) (string, error) { + return template.LoadText( + ctx, + t.deps, + os.DirFS(t.deps.CourierConfig().CourierTemplatesRoot(ctx)), + "login_code/valid/sms.body.gotmpl", // TODO:!!! + "login_code/valid/sms.body*", + t.model, + "", + ) +} + +func (t *LoginCodeValid) MarshalJSON() ([]byte, error) { + return json.Marshal(t.model) +} + +func (t *LoginCodeValid) TemplateType() template.TemplateType { + return template.TypeLoginCodeValid +} diff --git a/driver/registry_default.go b/driver/registry_default.go index f9c47bb30739..9e5568b3ee1b 100644 --- a/driver/registry_default.go +++ b/driver/registry_default.go @@ -353,12 +353,7 @@ func (m *RegistryDefault) strategyRegistrationEnabled(ctx context.Context, id st } func (m *RegistryDefault) strategyLoginEnabled(ctx context.Context, id string) bool { - switch id { - case identity.CredentialsTypeCodeAuth.String(): - return m.Config().SelfServiceCodeStrategy(ctx).PasswordlessEnabled - default: - return m.Config().SelfServiceStrategy(ctx, id).Enabled - } + return m.Config().SelfServiceStrategy(ctx, id).Enabled } func (m *RegistryDefault) RegistrationStrategies(ctx context.Context, filters ...registration.StrategyFilter) (registrationStrategies registration.Strategies) { diff --git a/driver/registry_default_test.go b/driver/registry_default_test.go index 533cdd621a9a..729ee41879e1 100644 --- a/driver/registry_default_test.go +++ b/driver/registry_default_test.go @@ -674,39 +674,49 @@ func TestDriverDefault_Strategies(t *testing.T) { t.Run("case=login", func(t *testing.T) { t.Parallel() for k, tc := range []struct { + name string prep func(conf *config.Config) expect []string }{ { + name: "no strategies", prep: func(conf *config.Config) { conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".password.enabled", false) + conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".code.enabled", false) }, }, { + name: "only password", prep: func(conf *config.Config) { conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".password.enabled", true) + conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".code.enabled", false) }, expect: []string{"password"}, }, { + name: "oidc and password", prep: func(conf *config.Config) { conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".oidc.enabled", true) conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".password.enabled", true) + conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".code.enabled", false) }, expect: []string{"password", "oidc"}, }, { + name: "oidc, password and totp", prep: func(conf *config.Config) { conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".oidc.enabled", true) conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".password.enabled", true) conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".totp.enabled", true) + conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".code.enabled", false) }, expect: []string{"password", "oidc", "totp"}, }, { + name: "password and code", prep: func(conf *config.Config) { conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".password.enabled", true) - conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".code.passwordless_enabled", true) + conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".code.enabled", true) }, expect: []string{"password", "code"}, }, diff --git a/embedx/identity_extension.schema.json b/embedx/identity_extension.schema.json index c105cbe42f95..6ad7765ef839 100644 --- a/embedx/identity_extension.schema.json +++ b/embedx/identity_extension.schema.json @@ -48,7 +48,7 @@ }, "via": { "type": "string", - "enum": ["email"] + "enum": ["email", "sms"] } } } diff --git a/identity/credentials_code.go b/identity/credentials_code.go index abc9decbbf46..b7f828ae09a1 100644 --- a/identity/credentials_code.go +++ b/identity/credentials_code.go @@ -7,7 +7,7 @@ import ( "database/sql" ) -type CodeAddressType string +type CodeAddressType = string const ( CodeAddressTypeEmail CodeAddressType = AddressTypeEmail diff --git a/identity/extension_credentials.go b/identity/extension_credentials.go index 3baa826b2e9c..709a47cb769c 100644 --- a/identity/extension_credentials.go +++ b/identity/extension_credentials.go @@ -70,6 +70,8 @@ func (r *SchemaExtensionCredentials) Run(ctx jsonschema.ValidationContext, s sch // } // r.setIdentifier(CredentialsTypeCodeAuth, value, CredentialsIdentifierAddressTypePhone) + case f.AddCase(""): + // continue default: return ctx.Error("", "credentials.code.via has unknown value %q", s.Credentials.Code.Via) } diff --git a/internal/testhelpers/selfservice_login.go b/internal/testhelpers/selfservice_login.go index bedba03fee16..ec8f14a53bf5 100644 --- a/internal/testhelpers/selfservice_login.go +++ b/internal/testhelpers/selfservice_login.go @@ -143,12 +143,12 @@ func InitializeLoginFlowViaBrowser(t *testing.T, client *http.Client, ts *httpte flowID = gjson.GetBytes(body, "id").String() } - rs, _, err := publicClient.FrontendApi.GetLoginFlow(context.Background()).Id(flowID).Execute() + rs, r, err := publicClient.FrontendApi.GetLoginFlow(context.Background()).Id(flowID).Execute() if expectGetError { require.Error(t, err) require.Nil(t, rs) } else { - require.NoError(t, err) + require.NoError(t, err, "%s", ioutilx.MustReadAll(r.Body)) assert.Empty(t, rs.Active) } diff --git a/selfservice/flow/login/handler.go b/selfservice/flow/login/handler.go index 096a657c0869..95fe1e321b30 100644 --- a/selfservice/flow/login/handler.go +++ b/selfservice/flow/login/handler.go @@ -798,7 +798,7 @@ continueLogin: sess = session.NewInactiveSession() } - method := ss.CompletedAuthenticationMethod(r.Context()) + method := ss.CompletedAuthenticationMethod(r.Context(), sess.AMR) sess.CompletedLoginForMethod(method) i = interim break diff --git a/selfservice/flow/login/hook.go b/selfservice/flow/login/hook.go index 2af0c3daf1fc..e34ad32c2433 100644 --- a/selfservice/flow/login/hook.go +++ b/selfservice/flow/login/hook.go @@ -356,7 +356,7 @@ func (e *HookExecutor) maybeLinkCredentials(ctx context.Context, sess *session.S return err } - method := strategy.CompletedAuthenticationMethod(ctx) + method := strategy.CompletedAuthenticationMethod(ctx, sess.AMR) sess.CompletedLoginForMethod(method) return nil diff --git a/selfservice/flow/login/strategy.go b/selfservice/flow/login/strategy.go index c8ad84986a55..4b802a0ae0cf 100644 --- a/selfservice/flow/login/strategy.go +++ b/selfservice/flow/login/strategy.go @@ -23,7 +23,7 @@ type Strategy interface { RegisterLoginRoutes(*x.RouterPublic) PopulateLoginMethod(r *http.Request, requestedAAL identity.AuthenticatorAssuranceLevel, sr *Flow) error Login(w http.ResponseWriter, r *http.Request, f *Flow, identityID uuid.UUID) (i *identity.Identity, err error) - CompletedAuthenticationMethod(ctx context.Context) session.AuthenticationMethod + CompletedAuthenticationMethod(ctx context.Context, methods session.AuthenticationMethods) session.AuthenticationMethod } type Strategies []Strategy diff --git a/selfservice/flow/registration/handler_test.go b/selfservice/flow/registration/handler_test.go index eae66fb720f8..a9e7b842718c 100644 --- a/selfservice/flow/registration/handler_test.go +++ b/selfservice/flow/registration/handler_test.go @@ -21,6 +21,7 @@ import ( "github.com/ory/kratos/corpx" "github.com/ory/kratos/hydra" + "github.com/ory/x/ioutilx" "github.com/ory/x/urlx" "github.com/stretchr/testify/assert" @@ -479,7 +480,7 @@ func TestOIDCStrategyOrder(t *testing.T) { resp, err := client.Do(req) require.NoError(t, err) - require.Equal(t, http.StatusOK, resp.StatusCode) + require.Equal(t, http.StatusOK, resp.StatusCode, "%s", ioutilx.MustReadAll(resp.Body)) verifiableAddress, err := reg.PrivilegedIdentityPool().FindVerifiableAddressByValue(ctx, identity.VerifiableAddressTypeEmail, email) require.NoError(t, err) diff --git a/selfservice/flow/request.go b/selfservice/flow/request.go index 6db872f43199..bb140eae24a2 100644 --- a/selfservice/flow/request.go +++ b/selfservice/flow/request.go @@ -10,7 +10,6 @@ import ( "strings" "github.com/ory/kratos/driver/config" - "github.com/ory/kratos/identity" "github.com/ory/kratos/selfservice/strategy" "github.com/ory/x/decoderx" @@ -106,19 +105,7 @@ func MethodEnabledAndAllowed(ctx context.Context, flowName FlowName, expected, a return errors.WithStack(ErrStrategyNotResponsible) } - var ok bool - if strings.EqualFold(actual, identity.CredentialsTypeCodeAuth.String()) { - switch flowName { - case RegistrationFlow, LoginFlow: - ok = d.Config().SelfServiceCodeStrategy(ctx).PasswordlessEnabled - case VerificationFlow, RecoveryFlow: - ok = d.Config().SelfServiceCodeStrategy(ctx).Enabled - } - } else { - ok = d.Config().SelfServiceStrategy(ctx, expected).Enabled - } - - if !ok { + if !d.Config().SelfServiceStrategy(ctx, expected).Enabled { return errors.WithStack(herodot.ErrNotFound.WithReason(strategy.EndpointDisabledMessage)) } diff --git a/selfservice/flow/request_test.go b/selfservice/flow/request_test.go index adc47f6149c9..3728748f8044 100644 --- a/selfservice/flow/request_test.go +++ b/selfservice/flow/request_test.go @@ -101,74 +101,3 @@ func TestMethodEnabledAndAllowed(t *testing.T) { assert.Contains(t, string(body), "The requested resource could not be found") }) } - -func TestMethodCodeEnabledAndAllowed(t *testing.T) { - ctx := context.Background() - conf, d := internal.NewFastRegistryWithMocks(t) - - currentFlow := make(chan flow.FlowName, 1) - - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - f := <-currentFlow - if err := flow.MethodEnabledAndAllowedFromRequest(r, f, "code", d); err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) - return - } - w.WriteHeader(http.StatusNoContent) - })) - - t.Run("login code allowed", func(t *testing.T) { - conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".code.passwordless_enabled", true) - currentFlow <- flow.LoginFlow - res, err := ts.Client().PostForm(ts.URL, url.Values{"method": {"code"}}) - require.NoError(t, err) - require.NoError(t, res.Body.Close()) - assert.Equal(t, http.StatusNoContent, res.StatusCode) - }) - - t.Run("login code not allowed", func(t *testing.T) { - conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".code.passwordless_enabled", false) - currentFlow <- flow.LoginFlow - res, err := ts.Client().PostForm(ts.URL, url.Values{"method": {"code"}}) - require.NoError(t, err) - body, err := io.ReadAll(res.Body) - require.NoError(t, err) - require.NoError(t, res.Body.Close()) - assert.Equal(t, http.StatusInternalServerError, res.StatusCode) - assert.Contains(t, string(body), "The requested resource could not be found") - }) - - t.Run("registration code allowed", func(t *testing.T) { - conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".code.passwordless_enabled", true) - currentFlow <- flow.RegistrationFlow - res, err := ts.Client().PostForm(ts.URL, url.Values{"method": {"code"}}) - require.NoError(t, err) - require.NoError(t, res.Body.Close()) - assert.Equal(t, http.StatusNoContent, res.StatusCode) - }) - - t.Run("registration code not allowed", func(t *testing.T) { - conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".code.passwordless_enabled", false) - currentFlow <- flow.RegistrationFlow - res, err := ts.Client().PostForm(ts.URL, url.Values{"method": {"code"}}) - require.NoError(t, err) - body, err := io.ReadAll(res.Body) - require.NoError(t, err) - require.NoError(t, res.Body.Close()) - assert.Equal(t, http.StatusInternalServerError, res.StatusCode) - assert.Contains(t, string(body), "The requested resource could not be found") - }) - - t.Run("recovery and verification should still be allowed if registration and login is disabled", func(t *testing.T) { - conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".code.passwordless_enabled", false) - conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".code.enabled", true) - - for _, f := range []flow.FlowName{flow.RecoveryFlow, flow.VerificationFlow} { - currentFlow <- f - res, err := ts.Client().PostForm(ts.URL, url.Values{"method": {"code"}}) - require.NoError(t, err) - require.NoError(t, res.Body.Close()) - assert.Equal(t, http.StatusNoContent, res.StatusCode) - } - }) -} diff --git a/selfservice/strategy/code/code_sender.go b/selfservice/strategy/code/code_sender.go index d41beef463ed..ccc3330f9ba8 100644 --- a/selfservice/strategy/code/code_sender.go +++ b/selfservice/strategy/code/code_sender.go @@ -84,7 +84,7 @@ func (s *Sender) SendCode(ctx context.Context, f flow.Flow, id *identity.Identit code, err := s.deps. RegistrationCodePersister(). CreateRegistrationCode(ctx, &CreateRegistrationCodeParams{ - AddressType: address.Via, + AddressType: identity.CodeAddressType(address.Via), RawCode: rawCode, ExpiresIn: s.deps.Config().SelfServiceCodeMethodLifespan(ctx), FlowID: f.GetID(), @@ -118,7 +118,7 @@ func (s *Sender) SendCode(ctx context.Context, f flow.Flow, id *identity.Identit code, err := s.deps. LoginCodePersister(). CreateLoginCode(ctx, &CreateLoginCodeParams{ - AddressType: address.Via, + AddressType: identity.CodeAddressType(address.Via), Address: address.To, RawCode: rawCode, ExpiresIn: s.deps.Config().SelfServiceCodeMethodLifespan(ctx), @@ -133,19 +133,29 @@ func (s *Sender) SendCode(ctx context.Context, f flow.Flow, id *identity.Identit if err != nil { return err } - - emailModel := email.LoginCodeValidModel{ - To: address.To, - LoginCode: rawCode, - Identity: model, - } s.deps.Audit(). WithField("login_flow_id", code.FlowID). WithField("login_code_id", code.ID). WithSensitiveField("login_code", rawCode). Info("Sending out login email with code.") - if err := s.send(ctx, string(address.Via), email.NewLoginCodeValid(s.deps, &emailModel)); err != nil { + var t courier.Template + switch address.Via { + case identity.ChannelTypeEmail: + t = email.NewLoginCodeValid(s.deps, &email.LoginCodeValidModel{ + To: address.To, + LoginCode: rawCode, + Identity: model, + }) + case identity.ChannelTypeSMS: + t = sms.NewLoginCodeValid(s.deps, &sms.LoginCodeValidModel{ + To: address.To, + LoginCode: rawCode, + Identity: model, + }) + } + + if err := s.send(ctx, string(address.Via), t); err != nil { return errors.WithStack(err) } diff --git a/selfservice/strategy/code/strategy.go b/selfservice/strategy/code/strategy.go index a5d0c83703ec..de6cf64fb290 100644 --- a/selfservice/strategy/code/strategy.go +++ b/selfservice/strategy/code/strategy.go @@ -132,6 +132,15 @@ func (s *Strategy) NodeGroup() node.UiNodeGroup { } func (s *Strategy) PopulateMethod(r *http.Request, f flow.Flow) error { + switch f.GetFlowName() { + case flow.LoginFlow: + fallthrough + case flow.RegistrationFlow: + if !s.deps.Config().SelfServiceCodeStrategy(r.Context()).PasswordlessEnabled { + return nil + } + } + if string(f.GetState()) == "" { f.SetState(flow.StateChooseMethod) } @@ -153,13 +162,20 @@ func (s *Strategy) PopulateMethod(r *http.Request, f flow.Flow) error { if err != nil { return err } - - identifierLabel, err := login.GetIdentifierLabelFromSchema(r.Context(), ds.String()) - if err != nil { - return err + lf, ok := f.(*login.Flow) + if !ok { + return errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Expected login.Flow but got: %T", f)) } + if lf.RequestedAAL == identity.AuthenticatorAssuranceLevel2 { + nodes.Upsert(node.NewInputField("identifier", "", node.DefaultGroup, node.InputAttributeTypeText, node.WithRequiredInputAttribute).WithMetaLabel(text.NewInfoNodeLabelID())) + } else { + 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)) + 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 1a0170d36648..3bf5f1890c37 100644 --- a/selfservice/strategy/code/strategy_login.go +++ b/selfservice/strategy/code/strategy_login.go @@ -17,6 +17,8 @@ import ( "github.com/ory/herodot" "github.com/ory/x/otelx" + "github.com/samber/lo" + "github.com/ory/kratos/identity" "github.com/ory/kratos/schema" "github.com/ory/kratos/selfservice/flow" @@ -60,7 +62,16 @@ type updateLoginFlowWithCodeMethod struct { func (s *Strategy) RegisterLoginRoutes(*x.RouterPublic) {} -func (s *Strategy) CompletedAuthenticationMethod(ctx context.Context) session.AuthenticationMethod { +func (s *Strategy) CompletedAuthenticationMethod(ctx context.Context, amr session.AuthenticationMethods) session.AuthenticationMethod { + aal1Satisfied := lo.ContainsBy(amr, func(am session.AuthenticationMethod) bool { + return am.Method != identity.CredentialsTypeCodeAuth && am.AAL == identity.AuthenticatorAssuranceLevel1 + }) + if aal1Satisfied { + return session.AuthenticationMethod{ + Method: identity.CredentialsTypeCodeAuth, + AAL: identity.AuthenticatorAssuranceLevel2, + } + } return session.AuthenticationMethod{ Method: identity.CredentialsTypeCodeAuth, AAL: identity.AuthenticatorAssuranceLevel1, @@ -97,9 +108,6 @@ func (s *Strategy) HandleLoginError(r *http.Request, f *login.Flow, body *update } func (s *Strategy) PopulateLoginMethod(r *http.Request, requestedAAL identity.AuthenticatorAssuranceLevel, lf *login.Flow) error { - if requestedAAL > identity.AuthenticatorAssuranceLevel1 { - return nil - } return s.PopulateMethod(r, lf) } @@ -144,10 +152,6 @@ func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow, return nil, err } - if err := login.CheckAAL(f, identity.AuthenticatorAssuranceLevel1); err != nil { - return nil, err - } - var p updateLoginFlowWithCodeMethod if err := s.dx.Decode(r, &p, decoderx.HTTPDecoderSetValidatePayloads(true), @@ -167,7 +171,7 @@ func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow, switch f.GetState() { case flow.StateChooseMethod: - if err := s.loginSendEmail(ctx, w, r, f, &p); err != nil { + if err := s.loginSendCode(ctx, w, r, f, &p); err != nil { return nil, s.HandleLoginError(r, f, &p, err) } return nil, nil @@ -184,8 +188,8 @@ func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow, return nil, s.HandleLoginError(r, f, &p, errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Unexpected flow state: %s", f.GetState()))) } -func (s *Strategy) loginSendEmail(ctx context.Context, w http.ResponseWriter, r *http.Request, f *login.Flow, p *updateLoginFlowWithCodeMethod) (err error) { - ctx, span := s.deps.Tracer(ctx).Tracer().Start(ctx, "selfservice.strategy.code.strategy.loginSendEmail") +func (s *Strategy) loginSendCode(ctx context.Context, w http.ResponseWriter, r *http.Request, f *login.Flow, p *updateLoginFlowWithCodeMethod) (err error) { + ctx, span := s.deps.Tracer(ctx).Tracer().Start(ctx, "selfservice.strategy.code.strategy.loginSendCode") defer otelx.End(span, &err) if len(p.Identifier) == 0 { @@ -205,10 +209,15 @@ func (s *Strategy) loginSendEmail(ctx context.Context, w http.ResponseWriter, r return errors.WithStack(err) } - addresses := []Address{{ - To: p.Identifier, - Via: identity.CodeAddressType(identity.AddressTypeEmail), - }} + matches := lo.Filter(i.VerifiableAddresses, func(va identity.VerifiableAddress, _ int) bool { + return va.Value == maybeNormalizeEmail(p.Identifier) + }) + addresses := lo.Map(matches, func(va identity.VerifiableAddress, _ int) Address { + return Address{ + To: va.Value, + Via: va.Via, + } + }) // kratos only supports `email` identifiers at the moment with the code method // this is validated in the identity validation step above diff --git a/selfservice/strategy/code/strategy_login_test.go b/selfservice/strategy/code/strategy_login_test.go index cda8ef3c05a9..093f12f1271c 100644 --- a/selfservice/strategy/code/strategy_login_test.go +++ b/selfservice/strategy/code/strategy_login_test.go @@ -12,6 +12,7 @@ import ( "net/url" "testing" + "github.com/ory/x/ioutilx" "github.com/ory/x/sqlcon" "github.com/ory/x/stringsx" @@ -33,7 +34,7 @@ func TestLoginCodeStrategy(t *testing.T) { ctx := context.Background() conf, reg := internal.NewFastRegistryWithMocks(t) testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/code.identity.schema.json") - conf.MustSet(ctx, fmt.Sprintf("%s.%s.enabled", config.ViperKeySelfServiceStrategyConfig, identity.CredentialsTypeCodeAuth.String()), false) + conf.MustSet(ctx, fmt.Sprintf("%s.%s.enabled", config.ViperKeySelfServiceStrategyConfig, identity.CredentialsTypeCodeAuth.String()), true) conf.MustSet(ctx, fmt.Sprintf("%s.%s.passwordless_enabled", config.ViperKeySelfServiceStrategyConfig, identity.CredentialsTypeCodeAuth.String()), true) conf.MustSet(ctx, config.ViperKeySelfServiceBrowserDefaultReturnTo, "https://www.ory.sh") conf.MustSet(ctx, config.ViperKeyURLsAllowedReturnToDomains, []string{"https://www.ory.sh"}) @@ -173,13 +174,14 @@ func TestLoginCodeStrategy(t *testing.T) { resp, err = s.client.Do(req) require.NoError(t, err) require.EqualValues(t, http.StatusOK, resp.StatusCode) + body = string(ioutilx.MustReadAll(resp.Body)) } else { // SPAs need to be informed that the login has not yet completed using status 400. // Browser clients will redirect back to the login URL. if apiType == ApiTypeBrowser { - require.EqualValues(t, http.StatusOK, resp.StatusCode) + require.EqualValues(t, http.StatusOK, resp.StatusCode, "%s", body) } else { - require.EqualValues(t, http.StatusBadRequest, resp.StatusCode) + require.EqualValues(t, http.StatusBadRequest, resp.StatusCode, "%s", body) } } @@ -570,22 +572,13 @@ func TestLoginCodeStrategy(t *testing.T) { require.True(t, va.Verified) }) - t.Run("case=should not populate on 2FA request", func(t *testing.T) { + t.Run("case=should populate on 2FA request", func(t *testing.T) { if tc.apiType == ApiTypeNative { t.Skip("skipping test since it is not applicable to native clients") } ctx := context.Background() - // enable webauthn 2FA method - conf.MustSet(ctx, fmt.Sprintf("%s.%s.enabled", config.ViperKeySelfServiceStrategyConfig, "webauthn"), true) - conf.MustSet(ctx, config.ViperKeySessionWhoAmIAAL, config.HighestAvailableAAL) - - t.Cleanup(func() { - conf.MustSet(ctx, fmt.Sprintf("%s.%s.enabled", config.ViperKeySelfServiceStrategyConfig, "webauthn"), false) - conf.MustSet(ctx, config.ViperKeySessionWhoAmIAAL, "aal1") - }) - s := createLoginFlow(ctx, t, public, tc.apiType, false) // submit email @@ -603,17 +596,13 @@ func TestLoginCodeStrategy(t *testing.T) { s = submitLogin(ctx, t, s, tc.apiType, func(v *url.Values) { v.Set("code", loginCode) }, false, func(t *testing.T, s *state, body string, res *http.Response) { - if tc.apiType == ApiTypeSPA { - require.EqualValues(t, http.StatusOK, res.StatusCode) - } else { - require.EqualValues(t, http.StatusOK, res.StatusCode) - } + require.EqualValues(t, http.StatusOK, res.StatusCode) }) clientInit := testhelpers.InitializeLoginFlowViaBrowser(t, s.client, public, false, tc.apiType == ApiTypeSPA, false, false, testhelpers.InitFlowWithAAL("aal2")) body, err := json.Marshal(clientInit) require.NoError(t, err) - require.Len(t, gjson.GetBytes(body, "ui.nodes.#(group==code)").Array(), 0, "should not populate code field on 2fa request") + require.Len(t, gjson.GetBytes(body, "ui.nodes.#(group==code)").Array(), 1) }) }) } diff --git a/selfservice/strategy/code/strategy_registration_test.go b/selfservice/strategy/code/strategy_registration_test.go index e591e627f5ef..1ca150b5f0ba 100644 --- a/selfservice/strategy/code/strategy_registration_test.go +++ b/selfservice/strategy/code/strategy_registration_test.go @@ -96,7 +96,7 @@ func TestRegistrationCodeStrategy(t *testing.T) { conf, reg := internal.NewFastRegistryWithMocks(t) testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/code.identity.schema.json") conf.MustSet(ctx, fmt.Sprintf("%s.%s.enabled", config.ViperKeySelfServiceStrategyConfig, identity.CredentialsTypePassword.String()), false) - conf.MustSet(ctx, fmt.Sprintf("%s.%s.enabled", config.ViperKeySelfServiceStrategyConfig, identity.CredentialsTypeCodeAuth.String()), false) + conf.MustSet(ctx, fmt.Sprintf("%s.%s.enabled", config.ViperKeySelfServiceStrategyConfig, identity.CredentialsTypeCodeAuth.String()), true) conf.MustSet(ctx, fmt.Sprintf("%s.%s.passwordless_enabled", config.ViperKeySelfServiceStrategyConfig, identity.CredentialsTypeCodeAuth), true) conf.MustSet(ctx, config.ViperKeySelfServiceBrowserDefaultReturnTo, "https://www.ory.sh") conf.MustSet(ctx, config.ViperKeyURLsAllowedReturnToDomains, []string{"https://www.ory.sh"}) @@ -176,6 +176,7 @@ func TestRegistrationCodeStrategy(t *testing.T) { submitAssertion(ctx, t, s, body, resp) return s } + t.Logf("%v", body) if apiType == ApiTypeBrowser { require.EqualValues(t, http.StatusOK, resp.StatusCode) @@ -532,7 +533,7 @@ func TestRegistrationCodeStrategy(t *testing.T) { require.Contains(t, gjson.GetBytes(body, "ui.messages").String(), "Could not find any login identifiers") } else { - require.Equal(t, http.StatusBadRequest, resp.StatusCode) + require.Equal(t, http.StatusBadRequest, resp.StatusCode, "%v", body) require.Contains(t, gjson.Get(body, "ui.messages").String(), "Could not find any login identifiers") } }) diff --git a/selfservice/strategy/lookup/strategy.go b/selfservice/strategy/lookup/strategy.go index 13c233913f18..e8f12cac9948 100644 --- a/selfservice/strategy/lookup/strategy.go +++ b/selfservice/strategy/lookup/strategy.go @@ -24,8 +24,10 @@ import ( ) // var _ login.Strategy = new(Strategy) -var _ settings.Strategy = new(Strategy) -var _ identity.ActiveCredentialsCounter = new(Strategy) +var ( + _ settings.Strategy = new(Strategy) + _ identity.ActiveCredentialsCounter = new(Strategy) +) type lookupStrategyDependencies interface { x.LoggingProvider @@ -104,7 +106,7 @@ func (s *Strategy) NodeGroup() node.UiNodeGroup { return node.LookupGroup } -func (s *Strategy) CompletedAuthenticationMethod(ctx context.Context) session.AuthenticationMethod { +func (s *Strategy) CompletedAuthenticationMethod(ctx context.Context, _ session.AuthenticationMethods) session.AuthenticationMethod { return session.AuthenticationMethod{ Method: s.ID(), AAL: identity.AuthenticatorAssuranceLevel2, diff --git a/selfservice/strategy/oidc/strategy.go b/selfservice/strategy/oidc/strategy.go index e67a5826b57e..4f5848e8a431 100644 --- a/selfservice/strategy/oidc/strategy.go +++ b/selfservice/strategy/oidc/strategy.go @@ -663,7 +663,7 @@ func (s *Strategy) NodeGroup() node.UiNodeGroup { return node.OpenIDConnectGroup } -func (s *Strategy) CompletedAuthenticationMethod(ctx context.Context) session.AuthenticationMethod { +func (s *Strategy) CompletedAuthenticationMethod(ctx context.Context, _ session.AuthenticationMethods) session.AuthenticationMethod { return session.AuthenticationMethod{ Method: s.ID(), AAL: identity.AuthenticatorAssuranceLevel1, diff --git a/selfservice/strategy/password/strategy.go b/selfservice/strategy/password/strategy.go index bb750bc9ef80..911ad619cd15 100644 --- a/selfservice/strategy/password/strategy.go +++ b/selfservice/strategy/password/strategy.go @@ -109,7 +109,7 @@ func (s *Strategy) ID() identity.CredentialsType { return identity.CredentialsTypePassword } -func (s *Strategy) CompletedAuthenticationMethod(ctx context.Context) session.AuthenticationMethod { +func (s *Strategy) CompletedAuthenticationMethod(ctx context.Context, _ session.AuthenticationMethods) session.AuthenticationMethod { return session.AuthenticationMethod{ Method: s.ID(), AAL: identity.AuthenticatorAssuranceLevel1, diff --git a/selfservice/strategy/totp/strategy.go b/selfservice/strategy/totp/strategy.go index bcaf09069053..6c3205abd9ac 100644 --- a/selfservice/strategy/totp/strategy.go +++ b/selfservice/strategy/totp/strategy.go @@ -24,9 +24,11 @@ import ( "github.com/ory/x/decoderx" ) -var _ login.Strategy = new(Strategy) -var _ settings.Strategy = new(Strategy) -var _ identity.ActiveCredentialsCounter = new(Strategy) +var ( + _ login.Strategy = new(Strategy) + _ settings.Strategy = new(Strategy) + _ identity.ActiveCredentialsCounter = new(Strategy) +) type totpStrategyDependencies interface { x.LoggingProvider @@ -107,7 +109,7 @@ func (s *Strategy) NodeGroup() node.UiNodeGroup { return node.TOTPGroup } -func (s *Strategy) CompletedAuthenticationMethod(ctx context.Context) session.AuthenticationMethod { +func (s *Strategy) CompletedAuthenticationMethod(ctx context.Context, _ session.AuthenticationMethods) session.AuthenticationMethod { return session.AuthenticationMethod{ Method: s.ID(), AAL: identity.AuthenticatorAssuranceLevel2, diff --git a/selfservice/strategy/webauthn/strategy.go b/selfservice/strategy/webauthn/strategy.go index aa4529cda620..998490055996 100644 --- a/selfservice/strategy/webauthn/strategy.go +++ b/selfservice/strategy/webauthn/strategy.go @@ -23,9 +23,11 @@ import ( "github.com/ory/x/decoderx" ) -var _ login.Strategy = new(Strategy) -var _ settings.Strategy = new(Strategy) -var _ identity.ActiveCredentialsCounter = new(Strategy) +var ( + _ login.Strategy = new(Strategy) + _ settings.Strategy = new(Strategy) + _ identity.ActiveCredentialsCounter = new(Strategy) +) type webauthnStrategyDependencies interface { x.LoggingProvider @@ -112,7 +114,7 @@ func (s *Strategy) NodeGroup() node.UiNodeGroup { return node.WebAuthnGroup } -func (s *Strategy) CompletedAuthenticationMethod(ctx context.Context) session.AuthenticationMethod { +func (s *Strategy) CompletedAuthenticationMethod(ctx context.Context, _ session.AuthenticationMethods) session.AuthenticationMethod { aal := identity.AuthenticatorAssuranceLevel1 if !s.d.Config().WebAuthnForPasswordless(ctx) { aal = identity.AuthenticatorAssuranceLevel2 diff --git a/selfservice/strategy/webauthn/strategy_test.go b/selfservice/strategy/webauthn/strategy_test.go index 699a36ab6cbb..cc5f6fafb475 100644 --- a/selfservice/strategy/webauthn/strategy_test.go +++ b/selfservice/strategy/webauthn/strategy_test.go @@ -26,13 +26,13 @@ func TestCompletedAuthenticationMethod(t *testing.T) { assert.Equal(t, session.AuthenticationMethod{ Method: strategy.ID(), AAL: identity.AuthenticatorAssuranceLevel2, - }, strategy.CompletedAuthenticationMethod(context.Background())) + }, strategy.CompletedAuthenticationMethod(context.Background(), session.AuthenticationMethods{})) conf.MustSet(ctx, config.ViperKeyWebAuthnPasswordless, true) assert.Equal(t, session.AuthenticationMethod{ Method: strategy.ID(), AAL: identity.AuthenticatorAssuranceLevel1, - }, strategy.CompletedAuthenticationMethod(context.Background())) + }, strategy.CompletedAuthenticationMethod(context.Background(), session.AuthenticationMethods{})) } func TestCountActiveFirstFactorCredentials(t *testing.T) { From 434d08cacf7fc42e19690bc0fcb099b63b92df4f Mon Sep 17 00:00:00 2001 From: Jonas Hungershausen Date: Thu, 11 Jan 2024 15:12:26 +0100 Subject: [PATCH 02/19] chore: cleanup --- driver/config/config.go | 2 + driver/registry_default.go | 7 +- driver/registry_default_test.go | 16 +- embedx/config.schema.json | 5 + internal/registrationhelpers/helpers.go | 16 +- internal/testhelpers/courier.go | 2 +- internal/testhelpers/selfservice_login.go | 3 +- internal/testhelpers/session.go | 14 + selfservice/flow/login/handler.go | 18 +- selfservice/flow/login/strategy.go | 3 +- selfservice/strategy/code/strategy.go | 318 +++++++++--------- selfservice/strategy/code/strategy_login.go | 50 ++- .../strategy/code/strategy_login_test.go | 84 ++++- selfservice/strategy/lookup/login.go | 9 +- selfservice/strategy/oidc/strategy_login.go | 4 +- selfservice/strategy/password/login.go | 3 +- selfservice/strategy/totp/login.go | 6 +- selfservice/strategy/webauthn/login.go | 5 +- 18 files changed, 333 insertions(+), 232 deletions(-) diff --git a/driver/config/config.go b/driver/config/config.go index 35d33ee8316a..399098cebe5d 100644 --- a/driver/config/config.go +++ b/driver/config/config.go @@ -236,6 +236,7 @@ type ( SelfServiceStrategyCode struct { *SelfServiceStrategy PasswordlessEnabled bool `json:"passwordless_enabled"` + MFAEnabled bool `json:"mfa_enabled"` } Schema struct { ID string `json:"id" koanf:"id"` @@ -782,6 +783,7 @@ func (p *Config) SelfServiceCodeStrategy(ctx context.Context) *SelfServiceStrate Config: config, }, PasswordlessEnabled: pp.BoolF(basePath+".passwordless_enabled", false), + MFAEnabled: pp.BoolF(basePath+".mfa_enabled", false), } } diff --git a/driver/registry_default.go b/driver/registry_default.go index 9e5568b3ee1b..4b256a7de1b2 100644 --- a/driver/registry_default.go +++ b/driver/registry_default.go @@ -344,12 +344,7 @@ func (m *RegistryDefault) selfServiceStrategies() []any { } func (m *RegistryDefault) strategyRegistrationEnabled(ctx context.Context, id string) bool { - switch id { - case identity.CredentialsTypeCodeAuth.String(): - return m.Config().SelfServiceCodeStrategy(ctx).PasswordlessEnabled - default: - return m.Config().SelfServiceStrategy(ctx, id).Enabled - } + return m.Config().SelfServiceStrategy(ctx, id).Enabled } func (m *RegistryDefault) strategyLoginEnabled(ctx context.Context, id string) bool { diff --git a/driver/registry_default_test.go b/driver/registry_default_test.go index 729ee41879e1..0c7d4d6e70e6 100644 --- a/driver/registry_default_test.go +++ b/driver/registry_default_test.go @@ -621,39 +621,49 @@ func TestDriverDefault_Strategies(t *testing.T) { t.Run("case=registration", func(t *testing.T) { t.Parallel() for k, tc := range []struct { + name string prep func(conf *config.Config) expect []string }{ { + name: "no strategies", prep: func(conf *config.Config) { conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".password.enabled", false) + conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".code.enabled", false) }, }, { + name: "only password", prep: func(conf *config.Config) { conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".password.enabled", true) + conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".code.enabled", false) }, expect: []string{"password"}, }, { + name: "oidc and password", prep: func(conf *config.Config) { conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".oidc.enabled", true) conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".password.enabled", true) + conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".code.enabled", false) }, expect: []string{"password", "oidc"}, }, { + name: "oidc, password and totp", prep: func(conf *config.Config) { conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".oidc.enabled", true) conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".password.enabled", true) conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".totp.enabled", true) + conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".code.enabled", false) }, expect: []string{"password", "oidc"}, }, { + name: "password and code", prep: func(conf *config.Config) { conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".password.enabled", true) - conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".code.passwordless_enabled", true) + conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".code.enabled", true) }, expect: []string{"password", "code"}, }, @@ -673,7 +683,7 @@ func TestDriverDefault_Strategies(t *testing.T) { t.Run("case=login", func(t *testing.T) { t.Parallel() - for k, tc := range []struct { + for _, tc := range []struct { name string prep func(conf *config.Config) expect []string @@ -721,7 +731,7 @@ func TestDriverDefault_Strategies(t *testing.T) { expect: []string{"password", "code"}, }, } { - t.Run(fmt.Sprintf("run=%d", k), func(t *testing.T) { + t.Run(fmt.Sprintf("run=%s", tc.name), func(t *testing.T) { conf, reg := internal.NewVeryFastRegistryWithoutDB(t) tc.prep(conf) diff --git a/embedx/config.schema.json b/embedx/config.schema.json index a42568389fe7..7c6dc570e398 100644 --- a/embedx/config.schema.json +++ b/embedx/config.schema.json @@ -1416,6 +1416,11 @@ "title": "Enables Login and Registration with the Code Method", "default": false }, + "mfa_enabled": { + "type": "boolean", + "title": "Enables Login flows Code Method to fulfil MFA requests", + "default": false + }, "passwordless_login_fallback_enabled": { "type": "boolean", "title": "Passwordless Login Fallback Enabled", diff --git a/internal/registrationhelpers/helpers.go b/internal/registrationhelpers/helpers.go index e9dbe7598797..67a636b567fb 100644 --- a/internal/registrationhelpers/helpers.go +++ b/internal/registrationhelpers/helpers.go @@ -106,7 +106,7 @@ func AssertSchemDoesNotExist(t *testing.T, reg *driver.RegistryDefault, flows [] reset() t.Run("case=should fail because schema does not exist", func(t *testing.T) { - var check = func(t *testing.T, actual string) { + check := func(t *testing.T, actual string) { assert.Equal(t, int64(http.StatusInternalServerError), gjson.Get(actual, "code").Int(), "%s", actual) assert.Equal(t, "Internal Server Error", gjson.Get(actual, "status").String(), "%s", actual) assert.Contains(t, gjson.Get(actual, "reason").String(), "no such file or directory", "%s", actual) @@ -164,7 +164,7 @@ func AssertCSRFFailures(t *testing.T, reg *driver.RegistryDefault, flows []strin apiClient := testhelpers.NewDebugClient(t) _ = testhelpers.NewErrorTestServer(t, reg) - var values = url.Values{ + values := url.Values{ "csrf_token": {"invalid_token"}, "traits.username": {testhelpers.RandomEmail()}, "traits.foobar": {"bar"}, @@ -253,7 +253,7 @@ func AssertRegistrationRespectsValidation(t *testing.T, reg *driver.RegistryDefa t.Run("case=should return an error because not passing validation", func(t *testing.T) { email := testhelpers.RandomEmail() - var check = func(t *testing.T, actual string) { + check := func(t *testing.T, actual string) { assert.NotEmpty(t, gjson.Get(actual, "id").String(), "%s", actual) assert.Contains(t, gjson.Get(actual, "ui.action").String(), publicTS.URL+registration.RouteSubmitFlow, "%s", actual) CheckFormContent(t, []byte(actual), "password", "csrf_token", "traits.username", "traits.foobar") @@ -261,7 +261,7 @@ func AssertRegistrationRespectsValidation(t *testing.T, reg *driver.RegistryDefa assert.Equal(t, email, gjson.Get(actual, "ui.nodes.#(attributes.name==traits.username).attributes.value").String(), "%s", actual) } - var values = func(v url.Values) { + values := func(v url.Values) { v.Set("traits.username", email) v.Del("traits.foobar") payload(v) @@ -411,7 +411,7 @@ func AssertCommonErrorCases(t *testing.T, flows []string) { }) t.Run("case=should show the error ui because the request id is missing", func(t *testing.T) { - var check = func(t *testing.T, actual string) { + check := func(t *testing.T, actual string) { assert.Equal(t, int64(http.StatusNotFound), gjson.Get(actual, "code").Int(), "%s", actual) assert.Equal(t, "Not Found", gjson.Get(actual, "status").String(), "%s", actual) assert.Contains(t, gjson.Get(actual, "message").String(), "Unable to locate the resource", "%s", actual) @@ -481,14 +481,14 @@ func AssertCommonErrorCases(t *testing.T, flows []string) { }) }) - t.Run("case=should fail because the return_to url is not allowed", func(t *testing.T) { + t.Run("case=should fail because the password was used in databreaches", func(t *testing.T) { testhelpers.SetDefaultIdentitySchemaFromRaw(conf, multifieldSchema) t.Cleanup(func() { testhelpers.SetDefaultIdentitySchemaFromRaw(conf, basicSchema) }) email := testhelpers.RandomEmail() - var check = func(t *testing.T, actual string) { + check := func(t *testing.T, actual string) { assert.NotEmpty(t, gjson.Get(actual, "id").String(), "%s", actual) assert.Contains(t, gjson.Get(actual, "ui.action").String(), publicTS.URL+registration.RouteSubmitFlow, "%s", actual) CheckFormContent(t, []byte(actual), "password", "csrf_token", "traits.username", "traits.foobar") @@ -498,7 +498,7 @@ func AssertCommonErrorCases(t *testing.T, flows []string) { assert.Equal(t, "password", gjson.Get(actual, "ui.nodes.#(attributes.name==method).attributes.value").String(), "%s", actual) } - var values = func(v url.Values) { + values := func(v url.Values) { v.Set("traits.username", email) v.Set("password", "password") v.Set("traits.foobar", "bar") diff --git a/internal/testhelpers/courier.go b/internal/testhelpers/courier.go index fd9aa63f45d2..796dac29989d 100644 --- a/internal/testhelpers/courier.go +++ b/internal/testhelpers/courier.go @@ -36,7 +36,7 @@ func CourierExpectMessage(ctx context.Context, t *testing.T, reg interface { } } - require.Failf(t, "could not find courier messages with recipient %s and subject %s", recipient, subject) + require.Failf(t, "could not find courier messages", "could not find courier messages with recipient %s and subject %s", recipient, subject) return nil } diff --git a/internal/testhelpers/selfservice_login.go b/internal/testhelpers/selfservice_login.go index ec8f14a53bf5..e92771c969fc 100644 --- a/internal/testhelpers/selfservice_login.go +++ b/internal/testhelpers/selfservice_login.go @@ -132,8 +132,8 @@ func InitializeLoginFlowViaBrowser(t *testing.T, client *http.Client, ts *httpte require.NoError(t, err) body := x.MustReadAll(res.Body) require.NoError(t, res.Body.Close()) + require.Equal(t, 200, res.StatusCode, "%s", body) if expectInitError { - require.Equal(t, 200, res.StatusCode) require.NotNil(t, res.Request.URL) require.Contains(t, res.Request.URL.String(), "error-ts") } @@ -142,6 +142,7 @@ func InitializeLoginFlowViaBrowser(t *testing.T, client *http.Client, ts *httpte if isSPA { flowID = gjson.GetBytes(body, "id").String() } + require.NotEmpty(t, flowID) rs, r, err := publicClient.FrontendApi.GetLoginFlow(context.Background()).Id(flowID).Execute() if expectGetError { diff --git a/internal/testhelpers/session.go b/internal/testhelpers/session.go index e6e9da316ffd..adbc4f81a392 100644 --- a/internal/testhelpers/session.go +++ b/internal/testhelpers/session.go @@ -196,6 +196,20 @@ func NewHTTPClientWithIdentitySessionCookie(t *testing.T, reg *driver.RegistryDe return NewHTTPClientWithSessionCookie(t, reg, s) } +func NewHTTPClientWithIdentitySessionCookieLocalhost(t *testing.T, reg *driver.RegistryDefault, id *identity.Identity) *http.Client { + req := NewTestHTTPRequest(t, "GET", "/sessions/whoami", nil) + s, err := session.NewActiveSession(req, + id, + NewSessionLifespanProvider(time.Hour), + time.Now(), + identity.CredentialsTypePassword, + identity.AuthenticatorAssuranceLevel1, + ) + require.NoError(t, err, "Could not initialize session from identity.") + + return NewHTTPClientWithSessionCookieLocalhost(t, reg, s) +} + func NewHTTPClientWithIdentitySessionToken(t *testing.T, reg *driver.RegistryDefault, id *identity.Identity) *http.Client { req := NewTestHTTPRequest(t, "GET", "/sessions/whoami", nil) s, err := session.NewActiveSession(req, diff --git a/selfservice/flow/login/handler.go b/selfservice/flow/login/handler.go index 95fe1e321b30..553aa469ccb3 100644 --- a/selfservice/flow/login/handler.go +++ b/selfservice/flow/login/handler.go @@ -186,14 +186,6 @@ func (h *Handler) NewLoginFlow(w http.ResponseWriter, r *http.Request, ft flow.T } preLoginHook: - if f.Refresh { - f.UI.Messages.Set(text.NewInfoLoginReAuth()) - } - - if sess != nil && f.RequestedAAL > sess.AuthenticatorAssuranceLevel && f.RequestedAAL > identity.AuthenticatorAssuranceLevel1 { - f.UI.Messages.Add(text.NewInfoLoginMFA()) - } - var strategyFilters []StrategyFilter orgID := uuid.NullUUID{ Valid: false, @@ -222,6 +214,14 @@ preLoginHook: } } + if f.Refresh { + f.UI.Messages.Set(text.NewInfoLoginReAuth()) + } + + if sess != nil && f.RequestedAAL > sess.AuthenticatorAssuranceLevel && f.RequestedAAL > identity.AuthenticatorAssuranceLevel1 { + f.UI.Messages.Add(text.NewInfoLoginMFA()) + } + if err := sortNodes(r.Context(), f.UI.Nodes); err != nil { return nil, nil, err } @@ -781,7 +781,7 @@ continueLogin: var i *identity.Identity var group node.UiNodeGroup for _, ss := range h.d.AllLoginStrategies() { - interim, err := ss.Login(w, r, f, sess.IdentityID) + interim, err := ss.Login(w, r, f, sess) group = ss.NodeGroup() if errors.Is(err, flow.ErrStrategyNotResponsible) { continue diff --git a/selfservice/flow/login/strategy.go b/selfservice/flow/login/strategy.go index 4b802a0ae0cf..c70ad9cc8684 100644 --- a/selfservice/flow/login/strategy.go +++ b/selfservice/flow/login/strategy.go @@ -7,7 +7,6 @@ import ( "context" "net/http" - "github.com/gofrs/uuid" "github.com/pkg/errors" "github.com/ory/kratos/identity" @@ -22,7 +21,7 @@ type Strategy interface { NodeGroup() node.UiNodeGroup RegisterLoginRoutes(*x.RouterPublic) PopulateLoginMethod(r *http.Request, requestedAAL identity.AuthenticatorAssuranceLevel, sr *Flow) error - Login(w http.ResponseWriter, r *http.Request, f *Flow, identityID uuid.UUID) (i *identity.Identity, err error) + Login(w http.ResponseWriter, r *http.Request, f *Flow, sess *session.Session) (i *identity.Identity, err error) CompletedAuthenticationMethod(ctx context.Context, methods session.AuthenticationMethods) session.AuthenticationMethod } diff --git a/selfservice/strategy/code/strategy.go b/selfservice/strategy/code/strategy.go index de6cf64fb290..ba834780b1b9 100644 --- a/selfservice/strategy/code/strategy.go +++ b/selfservice/strategy/code/strategy.go @@ -4,6 +4,7 @@ package code import ( + "context" "net/http" "strings" @@ -132,11 +133,23 @@ func (s *Strategy) NodeGroup() node.UiNodeGroup { } func (s *Strategy) PopulateMethod(r *http.Request, f flow.Flow) error { - switch f.GetFlowName() { - case flow.LoginFlow: - fallthrough - case flow.RegistrationFlow: - if !s.deps.Config().SelfServiceCodeStrategy(r.Context()).PasswordlessEnabled { + codeConfig := s.deps.Config().SelfServiceCodeStrategy(r.Context()) + switch f := f.(type) { + case *login.Flow: + if f.RequestedAAL == identity.AuthenticatorAssuranceLevel2 { + if !codeConfig.MFAEnabled { + // if the flow is requesting AAL2 but MFA is not enabled for the code strategy, we return nil so that + // other strategies can fulfil the request + return nil + } + } else if !codeConfig.PasswordlessEnabled { + // if the flow is a normal login flow but passwordless is not enabled for the code strategy, + // we return nil so that other strategies can fulfil the request + return nil + } + case *registration.Flow: + // for registration flows, we don't have AAL requirements, so we just check that passwordless is enabled. + if !codeConfig.PasswordlessEnabled { return nil } } @@ -147,191 +160,184 @@ func (s *Strategy) PopulateMethod(r *http.Request, f flow.Flow) error { f.GetUI().ResetMessages() - nodes := f.GetUI().Nodes - switch f.GetState() { case flow.StateChooseMethod: + if err := s.populateChooseMethodFlow(r.Context(), f); err != nil { + return err + } + case flow.StateEmailSent: + if err := s.populateEmailSentFlow(r.Context(), f); err != nil { + return err + } + case flow.StatePassedChallenge: + fallthrough + default: + return errors.WithStack(herodot.ErrBadRequest.WithReason("received an unexpected flow state")) + } - if f.GetFlowName() == flow.VerificationFlow || f.GetFlowName() == flow.RecoveryFlow { - nodes.Append( - node.NewInputField("email", nil, node.CodeGroup, node.InputAttributeTypeEmail, node.WithRequiredInputAttribute). - WithMetaLabel(text.NewInfoNodeInputEmail()), - ) - } else if f.GetFlowName() == flow.LoginFlow { - ds, err := s.deps.Config().DefaultIdentityTraitsSchemaURL(r.Context()) - if err != nil { - return err - } - lf, ok := f.(*login.Flow) - if !ok { - return errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Expected login.Flow but got: %T", f)) - } - if lf.RequestedAAL == identity.AuthenticatorAssuranceLevel2 { - nodes.Upsert(node.NewInputField("identifier", "", node.DefaultGroup, node.InputAttributeTypeText, node.WithRequiredInputAttribute).WithMetaLabel(text.NewInfoNodeLabelID())) - } else { - 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 { - return err - } + // no matter the flow type or state we need to set the CSRF token + if f.GetType() == flow.TypeBrowser { + f.GetUI().SetCSRF(s.deps.GenerateCSRFToken(r)) + } + return nil +} - // set the traits on the default group so that the ui can render them - // this prevents having multiple of the same ui fields on the same ui form - traitNodes, err := container.NodesFromJSONSchema(r.Context(), node.DefaultGroup, ds.String(), "", nil) +func (s *Strategy) populateChooseMethodFlow(ctx context.Context, f flow.Flow) error { + var codeMetaLabel *text.Message + switch f := f.(type) { + case *recovery.Flow, *verification.Flow: + f.GetUI().Nodes.Append( + node.NewInputField("email", nil, node.CodeGroup, node.InputAttributeTypeEmail, node.WithRequiredInputAttribute). + WithMetaLabel(text.NewInfoNodeInputEmail()), + ) + codeMetaLabel = text.NewInfoNodeLabelSubmit() + case *login.Flow: + codeMetaLabel = text.NewInfoSelfServiceLoginCode() + ds, err := s.deps.Config().DefaultIdentityTraitsSchemaURL(ctx) + if err != nil { + return err + } + if f.RequestedAAL == identity.AuthenticatorAssuranceLevel2 { + f.GetUI().Nodes.Upsert(node.NewInputField("identifier", "", node.DefaultGroup, node.InputAttributeTypeText, node.WithRequiredInputAttribute).WithMetaLabel(text.NewInfoNodeLabelID())) + } else { + identifierLabel, err := login.GetIdentifierLabelFromSchema(ctx, ds.String()) if err != nil { return err } - for _, n := range traitNodes { - nodes.Upsert(n) - } + f.GetUI().Nodes.Upsert(node.NewInputField("identifier", "", node.DefaultGroup, node.InputAttributeTypeText, node.WithRequiredInputAttribute).WithMetaLabel(identifierLabel)) + } + case *registration.Flow: + codeMetaLabel = text.NewInfoSelfServiceRegistrationRegisterCode() + ds, err := s.deps.Config().DefaultIdentityTraitsSchemaURL(ctx) + if err != nil { + return err } - var codeMetaLabel *text.Message - - switch f.GetFlowName() { - case flow.VerificationFlow, flow.RecoveryFlow: - codeMetaLabel = text.NewInfoNodeLabelSubmit() - case flow.LoginFlow: - codeMetaLabel = text.NewInfoSelfServiceLoginCode() - case flow.RegistrationFlow: - codeMetaLabel = text.NewInfoSelfServiceRegistrationRegisterCode() + // set the traits on the default group so that the ui can render them + // this prevents having multiple of the same ui fields on the same ui form + traitNodes, err := container.NodesFromJSONSchema(ctx, node.DefaultGroup, ds.String(), "", nil) + if err != nil { + return err } - methodButton := node.NewInputField("method", s.ID(), node.CodeGroup, node.InputAttributeTypeSubmit). - WithMetaLabel(codeMetaLabel) + for _, n := range traitNodes { + f.GetUI().Nodes.Upsert(n) + } + } - nodes.Append(methodButton) + methodButton := node.NewInputField("method", s.ID(), node.CodeGroup, node.InputAttributeTypeSubmit). + WithMetaLabel(codeMetaLabel) - f.GetUI().Nodes = nodes + f.GetUI().Nodes.Append(methodButton) - case flow.StateEmailSent: - // fresh ui node group - freshNodes := node.Nodes{} - var route string - var codeMetaLabel *text.Message - var message *text.Message - - var resendNode *node.Node - - switch f.GetFlowName() { - case flow.RecoveryFlow: - route = recovery.RouteSubmitFlow - codeMetaLabel = text.NewInfoNodeLabelRecoveryCode() - message = text.NewRecoveryEmailWithCodeSent() - - resendNode = node.NewInputField("email", nil, node.CodeGroup, node.InputAttributeTypeEmail, node.WithRequiredInputAttribute). - WithMetaLabel(text.NewInfoNodeResendOTP()) - case flow.VerificationFlow: - route = verification.RouteSubmitFlow - codeMetaLabel = text.NewInfoNodeLabelVerificationCode() - message = text.NewVerificationEmailWithCodeSent() - - case flow.LoginFlow: - route = login.RouteSubmitFlow - codeMetaLabel = text.NewInfoNodeLabelLoginCode() - message = text.NewLoginEmailWithCodeSent() - - // preserve the login identifier that were submitted - // so we can retry the code flow with the same data - for _, n := range f.GetUI().Nodes { - if n.Group == node.DefaultGroup { - // we don't need the user to change the values here - // for better UX let's make them disabled - // when there are errors we won't hide the fields - if len(n.Messages) == 0 { - if input, ok := n.Attributes.(*node.InputAttributes); ok { - input.Type = "hidden" - n.Attributes = input - } - } - freshNodes = append(freshNodes, n) - } - } + return nil +} - resendNode = node.NewInputField("resend", "code", node.CodeGroup, node.InputAttributeTypeSubmit). - WithMetaLabel(text.NewInfoNodeResendOTP()) +func (s *Strategy) populateEmailSentFlow(ctx context.Context, f flow.Flow) error { + // fresh ui node group + freshNodes := node.Nodes{} + var route string + var codeMetaLabel *text.Message + var message *text.Message - case flow.RegistrationFlow: - route = registration.RouteSubmitFlow - codeMetaLabel = text.NewInfoNodeLabelRegistrationCode() - message = text.NewRegistrationEmailWithCodeSent() + var resendNode *node.Node - // in the registration flow we need to preserve the trait fields that were submitted - // so we can retry the code flow with the same data - for _, n := range f.GetUI().Nodes { - if t, ok := n.Attributes.(*node.InputAttributes); ok && t.Type == node.InputAttributeTypeSubmit { - continue - } + switch f.GetFlowName() { + case flow.RecoveryFlow: + route = recovery.RouteSubmitFlow + codeMetaLabel = text.NewInfoNodeLabelRecoveryCode() + message = text.NewRecoveryEmailWithCodeSent() + + resendNode = node.NewInputField("email", nil, node.CodeGroup, node.InputAttributeTypeEmail, node.WithRequiredInputAttribute). + WithMetaLabel(text.NewInfoNodeResendOTP()) + case flow.VerificationFlow: + route = verification.RouteSubmitFlow + codeMetaLabel = text.NewInfoNodeLabelVerificationCode() + message = text.NewVerificationEmailWithCodeSent() - if n.Group == node.DefaultGroup { - // we don't need the user to change the values here - // for better UX let's make them disabled - // when there are errors we won't hide the fields - if len(n.Messages) == 0 { - if input, ok := n.Attributes.(*node.InputAttributes); ok { - input.Type = "hidden" - n.Attributes = input - } + case flow.LoginFlow: + route = login.RouteSubmitFlow + codeMetaLabel = text.NewInfoNodeLabelLoginCode() + message = text.NewLoginEmailWithCodeSent() + + // preserve the login identifier that was submitted + // so we can retry the code flow with the same data + for _, n := range f.GetUI().Nodes { + if n.Group == node.DefaultGroup { + // we don't need the user to change the values here + // for better UX let's make them disabled + // when there are errors we won't hide the fields + if len(n.Messages) == 0 { + if input, ok := n.Attributes.(*node.InputAttributes); ok { + input.Type = "hidden" + n.Attributes = input } - freshNodes = append(freshNodes, n) } + freshNodes = append(freshNodes, n) } - - resendNode = node.NewInputField("resend", "code", node.CodeGroup, node.InputAttributeTypeSubmit). - WithMetaLabel(text.NewInfoNodeResendOTP()) - default: - return errors.WithStack(herodot.ErrBadRequest.WithReason("received an unexpected flow type")) } - // Hidden field Required for the re-send code button - // !!important!!: this field must be appended before the code submit button since upsert will replace the first node with the same name - freshNodes.Upsert( - node.NewInputField("method", s.NodeGroup(), node.CodeGroup, node.InputAttributeTypeHidden), - ) - - // code input field - freshNodes.Upsert(node.NewInputField("code", nil, node.CodeGroup, node.InputAttributeTypeText, node.WithRequiredInputAttribute). - WithMetaLabel(codeMetaLabel)) + resendNode = node.NewInputField("resend", "code", node.CodeGroup, node.InputAttributeTypeSubmit). + WithMetaLabel(text.NewInfoNodeResendOTP()) - // code submit button - freshNodes. - Append(node.NewInputField("method", s.ID(), node.CodeGroup, node.InputAttributeTypeSubmit). - WithMetaLabel(text.NewInfoNodeLabelSubmit())) + case flow.RegistrationFlow: + route = registration.RouteSubmitFlow + codeMetaLabel = text.NewInfoNodeLabelRegistrationCode() + message = text.NewRegistrationEmailWithCodeSent() + + // in the registration flow we need to preserve the trait fields that were submitted + // so we can retry the code flow with the same data + for _, n := range f.GetUI().Nodes { + if t, ok := n.Attributes.(*node.InputAttributes); ok && t.Type == node.InputAttributeTypeSubmit { + continue + } - if resendNode != nil { - freshNodes.Append(resendNode) + if n.Group == node.DefaultGroup { + // we don't need the user to change the values here + // for better UX let's make them disabled + // when there are errors we won't hide the fields + if len(n.Messages) == 0 { + if input, ok := n.Attributes.(*node.InputAttributes); ok { + input.Type = "hidden" + n.Attributes = input + } + } + freshNodes = append(freshNodes, n) + } } - f.GetUI().Nodes = freshNodes + resendNode = node.NewInputField("resend", "code", node.CodeGroup, node.InputAttributeTypeSubmit). + WithMetaLabel(text.NewInfoNodeResendOTP()) + default: + return errors.WithStack(herodot.ErrBadRequest.WithReason("received an unexpected flow type")) + } - f.GetUI().Method = "POST" - f.GetUI().Action = flow.AppendFlowTo(urlx.AppendPaths(s.deps.Config().SelfPublicURL(r.Context()), route), f.GetID()).String() + // Hidden field Required for the re-send code button + // !!important!!: this field must be appended before the code submit button since upsert will replace the first node with the same name + freshNodes.Upsert( + node.NewInputField("method", s.NodeGroup(), node.CodeGroup, node.InputAttributeTypeHidden), + ) - // Set the request's CSRF token - if f.GetType() == flow.TypeBrowser { - f.GetUI().SetCSRF(s.deps.GenerateCSRFToken(r)) - } + // code input field + freshNodes.Upsert(node.NewInputField("code", nil, node.CodeGroup, node.InputAttributeTypeText, node.WithRequiredInputAttribute). + WithMetaLabel(codeMetaLabel)) - f.GetUI().Messages.Set(message) + // code submit button + freshNodes. + Append(node.NewInputField("method", s.ID(), node.CodeGroup, node.InputAttributeTypeSubmit). + WithMetaLabel(text.NewInfoNodeLabelSubmit())) - case flow.StatePassedChallenge: - fallthrough - default: - return errors.WithStack(herodot.ErrBadRequest.WithReason("received an unexpected flow state")) + if resendNode != nil { + freshNodes.Append(resendNode) } - // no matter the flow type or state we need to set the CSRF token - if f.GetType() == flow.TypeBrowser { - f.GetUI().SetCSRF(s.deps.GenerateCSRFToken(r)) - } + f.GetUI().Nodes = freshNodes + + f.GetUI().Method = "POST" + f.GetUI().Action = flow.AppendFlowTo(urlx.AppendPaths(s.deps.Config().SelfPublicURL(ctx), route), f.GetID()).String() + + f.GetUI().Messages.Set(message) return nil } diff --git a/selfservice/strategy/code/strategy_login.go b/selfservice/strategy/code/strategy_login.go index 3bf5f1890c37..df4352bf1344 100644 --- a/selfservice/strategy/code/strategy_login.go +++ b/selfservice/strategy/code/strategy_login.go @@ -11,7 +11,6 @@ import ( "github.com/ory/x/sqlcon" - "github.com/gofrs/uuid" "github.com/pkg/errors" "github.com/ory/herodot" @@ -144,7 +143,7 @@ func (s *Strategy) findIdentityByIdentifier(ctx context.Context, identifier stri return id, false, nil } -func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow, _ uuid.UUID) (_ *identity.Identity, err error) { +func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow, sess *session.Session) (_ *identity.Identity, err error) { ctx, span := s.deps.Tracer(r.Context()).Tracer().Start(r.Context(), "selfservice.strategy.code.strategy.Login") defer otelx.End(span, &err) @@ -171,7 +170,7 @@ func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow, switch f.GetState() { case flow.StateChooseMethod: - if err := s.loginSendCode(ctx, w, r, f, &p); err != nil { + if err := s.loginSendCode(ctx, w, r, f, &p, sess); err != nil { return nil, s.HandleLoginError(r, f, &p, err) } return nil, nil @@ -188,7 +187,7 @@ func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow, return nil, s.HandleLoginError(r, f, &p, errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Unexpected flow state: %s", f.GetState()))) } -func (s *Strategy) loginSendCode(ctx context.Context, w http.ResponseWriter, r *http.Request, f *login.Flow, p *updateLoginFlowWithCodeMethod) (err error) { +func (s *Strategy) loginSendCode(ctx context.Context, w http.ResponseWriter, r *http.Request, f *login.Flow, p *updateLoginFlowWithCodeMethod, sess *session.Session) (err error) { ctx, span := s.deps.Tracer(ctx).Tracer().Start(ctx, "selfservice.strategy.code.strategy.loginSendCode") defer otelx.End(span, &err) @@ -198,10 +197,35 @@ func (s *Strategy) loginSendCode(ctx context.Context, w http.ResponseWriter, r * p.Identifier = maybeNormalizeEmail(p.Identifier) - // Step 1: Get the identity - i, _, err := s.findIdentityByIdentifier(ctx, p.Identifier) - if err != nil { - return err + var addresses []Address + var i *identity.Identity + if f.RequestedAAL > identity.AuthenticatorAssuranceLevel1 { + address, found := lo.Find(sess.Identity.VerifiableAddresses, func(va identity.VerifiableAddress) bool { + return va.Value == p.Identifier + }) + if !found { + return errors.WithStack(herodot.ErrBadRequest.WithReasonf("The supplied address does not match any known addresses.")) + } + i = sess.Identity + addresses = []Address{{ + To: address.Value, + Via: address.Via, + }} + } else { + // Step 1: Get the identity + i, _, err = s.findIdentityByIdentifier(ctx, p.Identifier) + if err != nil { + return err + } + matches := lo.Filter(i.VerifiableAddresses, func(va identity.VerifiableAddress, _ int) bool { + return va.Value == maybeNormalizeEmail(p.Identifier) + }) + addresses = lo.Map(matches, func(va identity.VerifiableAddress, _ int) Address { + return Address{ + To: va.Value, + Via: va.Via, + } + }) } // Step 2: Delete any previous login codes for this flow ID @@ -209,16 +233,6 @@ func (s *Strategy) loginSendCode(ctx context.Context, w http.ResponseWriter, r * return errors.WithStack(err) } - matches := lo.Filter(i.VerifiableAddresses, func(va identity.VerifiableAddress, _ int) bool { - return va.Value == maybeNormalizeEmail(p.Identifier) - }) - addresses := lo.Map(matches, func(va identity.VerifiableAddress, _ int) Address { - return Address{ - To: va.Value, - Via: va.Via, - } - }) - // kratos only supports `email` identifiers at the moment with the code method // this is validated in the identity validation step above if err := s.deps.CodeSender().SendCode(ctx, f, i, addresses...); err != nil { diff --git a/selfservice/strategy/code/strategy_login_test.go b/selfservice/strategy/code/strategy_login_test.go index 093f12f1271c..14281bfd4654 100644 --- a/selfservice/strategy/code/strategy_login_test.go +++ b/selfservice/strategy/code/strategy_login_test.go @@ -26,6 +26,7 @@ import ( oryClient "github.com/ory/kratos/internal/httpclient" "github.com/ory/kratos/internal/testhelpers" "github.com/ory/kratos/session" + "github.com/ory/kratos/text" "github.com/ory/kratos/x" "github.com/ory/x/sqlxx" ) @@ -86,6 +87,7 @@ func TestLoginCodeStrategy(t *testing.T) { loginCode string identityEmail string testServer *httptest.Server + body string } type ApiType string @@ -162,6 +164,7 @@ func TestLoginCodeStrategy(t *testing.T) { submitAssertion(t, s, body, resp) return s } + s.body = body if mustHaveSession { req, err := http.NewRequest("GET", s.testServer.URL+session.RouteWhoami, nil) @@ -572,37 +575,90 @@ func TestLoginCodeStrategy(t *testing.T) { require.True(t, va.Verified) }) - t.Run("case=should populate on 2FA request", func(t *testing.T) { - if tc.apiType == ApiTypeNative { - t.Skip("skipping test since it is not applicable to native clients") - } - + t.Run("case=should be able to get AAL2 session", func(t *testing.T) { ctx := context.Background() + conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".code.mfa_enabled", true) + t.Cleanup(func() { + conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".code.mfa_enabled", false) + }) - s := createLoginFlow(ctx, t, public, tc.apiType, false) + identity := createIdentity(ctx, t, false) + var cl *http.Client + var f *oryClient.LoginFlow + if tc.apiType == ApiTypeNative { + cl = testhelpers.NewHTTPClientWithIdentitySessionToken(t, reg, identity) + f = testhelpers.InitializeLoginFlowViaAPI(t, cl, public, false, testhelpers.InitFlowWithAAL("aal2")) + } else { + cl = testhelpers.NewHTTPClientWithIdentitySessionCookieLocalhost(t, reg, identity) + f = testhelpers.InitializeLoginFlowViaBrowser(t, cl, public, false, tc.apiType == ApiTypeSPA, false, false, testhelpers.InitFlowWithAAL("aal2")) + } - // submit email + body, err := json.Marshal(f) + require.NoError(t, err) + require.Len(t, gjson.GetBytes(body, "ui.nodes.#(group==code)").Array(), 1) + require.Len(t, gjson.GetBytes(body, "ui.messages").Array(), 1, "%s", body) + require.EqualValues(t, gjson.GetBytes(body, "ui.messages.0.id").Int(), text.InfoSelfServiceLoginMFA, "%s", body) + + s := &state{ + flowID: f.GetId(), + identity: identity, + client: cl, + testServer: public, + identityEmail: gjson.Get(identity.Traits.String(), "email").String(), + } s = submitLogin(ctx, t, s, tc.apiType, func(v *url.Values) { v.Set("identifier", s.identityEmail) - }, false, nil) + }, true, nil) message := testhelpers.CourierExpectMessage(ctx, t, reg, s.identityEmail, "Login to your account") assert.Contains(t, message.Body, "please login to your account by entering the following code") - loginCode := testhelpers.CourierExpectCodeInMessage(t, message, 1) assert.NotEmpty(t, loginCode) - // 3. Submit OTP s = submitLogin(ctx, t, s, tc.apiType, func(v *url.Values) { v.Set("code", loginCode) - }, false, func(t *testing.T, s *state, body string, res *http.Response) { - require.EqualValues(t, http.StatusOK, res.StatusCode) + }, true, nil) + + testhelpers.EnsureAAL(t, cl, public, "aal2", "code") + }) + + t.Run("case=cannot use different identifier", func(t *testing.T) { + ctx := context.Background() + conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".code.mfa_enabled", true) + t.Cleanup(func() { + conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".code.mfa_enabled", false) }) - clientInit := testhelpers.InitializeLoginFlowViaBrowser(t, s.client, public, false, tc.apiType == ApiTypeSPA, false, false, testhelpers.InitFlowWithAAL("aal2")) - body, err := json.Marshal(clientInit) + identity := createIdentity(ctx, t, false) + var cl *http.Client + var f *oryClient.LoginFlow + if tc.apiType == ApiTypeNative { + cl = testhelpers.NewHTTPClientWithIdentitySessionToken(t, reg, identity) + f = testhelpers.InitializeLoginFlowViaAPI(t, cl, public, false, testhelpers.InitFlowWithAAL("aal2")) + } else { + cl = testhelpers.NewHTTPClientWithIdentitySessionCookieLocalhost(t, reg, identity) + f = testhelpers.InitializeLoginFlowViaBrowser(t, cl, public, false, tc.apiType == ApiTypeSPA, false, false, testhelpers.InitFlowWithAAL("aal2")) + } + + body, err := json.Marshal(f) require.NoError(t, err) require.Len(t, gjson.GetBytes(body, "ui.nodes.#(group==code)").Array(), 1) + require.Len(t, gjson.GetBytes(body, "ui.messages").Array(), 1, "%s", body) + require.EqualValues(t, gjson.GetBytes(body, "ui.messages.0.id").Int(), text.InfoSelfServiceLoginMFA, "%s", body) + + s := &state{ + flowID: f.GetId(), + identity: identity, + client: cl, + testServer: public, + identityEmail: gjson.Get(identity.Traits.String(), "email").String(), + } + email := testhelpers.RandomEmail() + s = submitLogin(ctx, t, s, tc.apiType, func(v *url.Values) { + v.Set("identifier", email) + }, true, nil) + + require.Equal(t, "The supplied address does not match any known addresses.", gjson.Get(s.body, "ui.messages.0.text").String(), "%s", body) }) }) } diff --git a/selfservice/strategy/lookup/login.go b/selfservice/strategy/lookup/login.go index 75edc2181059..2eda9b5796c8 100644 --- a/selfservice/strategy/lookup/login.go +++ b/selfservice/strategy/lookup/login.go @@ -8,8 +8,6 @@ import ( "net/http" "time" - "github.com/gofrs/uuid" - "github.com/ory/x/sqlcon" "github.com/ory/x/sqlxx" @@ -21,6 +19,7 @@ import ( "github.com/ory/kratos/schema" "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" @@ -89,7 +88,7 @@ type updateLoginFlowWithLookupSecretMethod struct { Code string `json:"lookup_secret"` } -func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow, identityID uuid.UUID) (i *identity.Identity, err error) { +func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow, sess *session.Session) (i *identity.Identity, err error) { if err := login.CheckAAL(f, identity.AuthenticatorAssuranceLevel2); err != nil { return nil, err } @@ -110,7 +109,7 @@ func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow, return nil, s.handleLoginError(r, f, err) } - i, c, err := s.d.PrivilegedIdentityPool().FindByCredentialsIdentifier(r.Context(), s.ID(), identityID.String()) + i, c, err := s.d.PrivilegedIdentityPool().FindByCredentialsIdentifier(r.Context(), s.ID(), sess.IdentityID.String()) if errors.Is(err, sqlcon.ErrNoRows) { return nil, s.handleLoginError(r, f, errors.WithStack(schema.NewNoLookupDefined())) } else if err != nil { @@ -138,7 +137,7 @@ func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow, return nil, s.handleLoginError(r, f, errors.WithStack(schema.NewErrorValidationLookupInvalid())) } - toUpdate, err := s.d.PrivilegedIdentityPool().GetIdentityConfidential(r.Context(), identityID) + toUpdate, err := s.d.PrivilegedIdentityPool().GetIdentityConfidential(r.Context(), sess.IdentityID) if err != nil { return nil, err } diff --git a/selfservice/strategy/oidc/strategy_login.go b/selfservice/strategy/oidc/strategy_login.go index a3048df1e15b..641b3d42b977 100644 --- a/selfservice/strategy/oidc/strategy_login.go +++ b/selfservice/strategy/oidc/strategy_login.go @@ -13,8 +13,6 @@ import ( "github.com/julienschmidt/httprouter" "golang.org/x/oauth2" - "github.com/gofrs/uuid" - "github.com/ory/kratos/session" "github.com/ory/kratos/ui/node" @@ -181,7 +179,7 @@ func (s *Strategy) processLogin(w http.ResponseWriter, r *http.Request, loginFlo return nil, s.handleError(w, r, loginFlow, provider.Config().ID, nil, errors.WithStack(herodot.ErrInternalServerError.WithReason("Unable to find matching OpenID Connect Credentials.").WithDebugf(`Unable to find credentials that match the given provider "%s" and subject "%s".`, provider.Config().ID, claims.Subject))) } -func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow, _ uuid.UUID) (i *identity.Identity, err error) { +func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow, _ *session.Session) (i *identity.Identity, err error) { ctx, span := s.d.Tracer(r.Context()).Tracer().Start(r.Context(), "selfservice.strategy.oidc.strategy.Login") defer span.End() diff --git a/selfservice/strategy/password/login.go b/selfservice/strategy/password/login.go index 5c9ed653872d..706a4fcab0f0 100644 --- a/selfservice/strategy/password/login.go +++ b/selfservice/strategy/password/login.go @@ -11,6 +11,7 @@ import ( "time" "github.com/ory/kratos/selfservice/flowhelpers" + "github.com/ory/kratos/session" "github.com/ory/x/stringsx" @@ -46,7 +47,7 @@ func (s *Strategy) handleLoginError(w http.ResponseWriter, r *http.Request, f *l return err } -func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow, identityID uuid.UUID) (i *identity.Identity, err error) { +func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow, _ *session.Session) (i *identity.Identity, err error) { if err := login.CheckAAL(f, identity.AuthenticatorAssuranceLevel1); err != nil { return nil, err } diff --git a/selfservice/strategy/totp/login.go b/selfservice/strategy/totp/login.go index bc9816d265f3..1eaa5aeedac7 100644 --- a/selfservice/strategy/totp/login.go +++ b/selfservice/strategy/totp/login.go @@ -7,7 +7,6 @@ import ( "encoding/json" "net/http" - "github.com/gofrs/uuid" "github.com/pkg/errors" "github.com/pquerna/otp" "github.com/pquerna/otp/totp" @@ -17,6 +16,7 @@ import ( "github.com/ory/kratos/schema" "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" @@ -85,7 +85,7 @@ type updateLoginFlowWithTotpMethod struct { TOTPCode string `json:"totp_code"` } -func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow, identityID uuid.UUID) (i *identity.Identity, err error) { +func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow, sess *session.Session) (i *identity.Identity, err error) { if err := login.CheckAAL(f, identity.AuthenticatorAssuranceLevel2); err != nil { return nil, err } @@ -106,7 +106,7 @@ func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow, return nil, s.handleLoginError(r, f, err) } - i, c, err := s.d.PrivilegedIdentityPool().FindByCredentialsIdentifier(r.Context(), s.ID(), identityID.String()) + i, c, err := s.d.PrivilegedIdentityPool().FindByCredentialsIdentifier(r.Context(), s.ID(), sess.IdentityID.String()) if err != nil { return nil, s.handleLoginError(r, f, errors.WithStack(schema.NewNoTOTPDeviceRegistered())) } diff --git a/selfservice/strategy/webauthn/login.go b/selfservice/strategy/webauthn/login.go index 6b9ee3a154f9..38cdf283b9f1 100644 --- a/selfservice/strategy/webauthn/login.go +++ b/selfservice/strategy/webauthn/login.go @@ -10,6 +10,7 @@ import ( "time" "github.com/ory/kratos/selfservice/flowhelpers" + "github.com/ory/kratos/session" "github.com/gofrs/uuid" @@ -200,7 +201,7 @@ type updateLoginFlowWithWebAuthnMethod struct { Login string `json:"webauthn_login"` } -func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow, identityID uuid.UUID) (i *identity.Identity, err error) { +func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow, sess *session.Session) (i *identity.Identity, err error) { if f.Type != flow.TypeBrowser { return nil, flow.ErrStrategyNotResponsible } @@ -232,7 +233,7 @@ func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow, return s.loginPasswordless(w, r, f, &p) } - return s.loginMultiFactor(w, r, f, identityID, &p) + return s.loginMultiFactor(w, r, f, sess.IdentityID, &p) } func (s *Strategy) loginPasswordless(w http.ResponseWriter, r *http.Request, f *login.Flow, p *updateLoginFlowWithWebAuthnMethod) (i *identity.Identity, err error) { From 9492acf11a1ee454572e6837b126a9914371879e Mon Sep 17 00:00:00 2001 From: Jonas Hungershausen Date: Fri, 12 Jan 2024 13:46:34 +0100 Subject: [PATCH 03/19] chore: add e2e tests --- Makefile | 4 + schema/errors.go | 11 ++ selfservice/strategy/code/strategy_login.go | 2 +- .../integration/profiles/mfa/code.spec.ts | 104 ++++++++++++++++++ test/e2e/cypress/support/config.d.ts | 45 +++++++- test/e2e/cypress/support/configHelpers.ts | 9 ++ text/id.go | 1 + text/message_login.go | 9 +- 8 files changed, 181 insertions(+), 4 deletions(-) create mode 100644 test/e2e/cypress/integration/profiles/mfa/code.spec.ts diff --git a/Makefile b/Makefile index 59fd30158bdb..11b67090b43a 100644 --- a/Makefile +++ b/Makefile @@ -210,3 +210,7 @@ licenses: .bin/licenses node_modules # checks open-source licenses node_modules: package-lock.json npm ci touch node_modules + +.PHONY: kratos-config-e2e +kratos-config-e2e: + sh ./test/e2e/render-kratos-config.sh diff --git a/schema/errors.go b/schema/errors.go index d72e02af7d11..30c1f72976f3 100644 --- a/schema/errors.go +++ b/schema/errors.go @@ -359,3 +359,14 @@ func NewLinkedCredentialsDoNotMatch() error { Messages: new(text.Messages).Add(text.NewErrorValidationLoginLinkedCredentialsDoNotMatch()), }) } + +func NewUnknownAddressError() error { + return errors.WithStack(&ValidationError{ + ValidationError: &jsonschema.ValidationError{ + Message: `the supplied address does not match any known addresses.`, + InstancePtr: "#/", + }, + Messages: new(text.Messages).Add(text.NewErrorValidationAddressUnknown()), + }, + ) +} diff --git a/selfservice/strategy/code/strategy_login.go b/selfservice/strategy/code/strategy_login.go index df4352bf1344..76e6f99b0561 100644 --- a/selfservice/strategy/code/strategy_login.go +++ b/selfservice/strategy/code/strategy_login.go @@ -204,7 +204,7 @@ func (s *Strategy) loginSendCode(ctx context.Context, w http.ResponseWriter, r * return va.Value == p.Identifier }) if !found { - return errors.WithStack(herodot.ErrBadRequest.WithReasonf("The supplied address does not match any known addresses.")) + return errors.WithStack(schema.NewUnknownAddressError()) } i = sess.Identity addresses = []Address{{ diff --git a/test/e2e/cypress/integration/profiles/mfa/code.spec.ts b/test/e2e/cypress/integration/profiles/mfa/code.spec.ts new file mode 100644 index 000000000000..74019d018596 --- /dev/null +++ b/test/e2e/cypress/integration/profiles/mfa/code.spec.ts @@ -0,0 +1,104 @@ +// Copyright © 2023 Ory Corp +// SPDX-License-Identifier: Apache-2.0 + +import { gen, website } from "../../../helpers" +import { routes as express } from "../../../helpers/express" +import { routes as react } from "../../../helpers/react" + +context("2FA code", () => { + ;[ + { + login: react.login, + settings: react.settings, + base: react.base, + app: "react" as "react", + profile: "spa", + }, + { + login: express.login, + settings: express.settings, + base: express.base, + app: "express" as "express", + profile: "mfa", + }, + ].forEach(({ settings, login, profile, app }) => { + describe(`for app ${app}`, () => { + before(() => { + cy.useConfigProfile(profile) + cy.proxy(app) + }) + + let email: string + let password: string + + beforeEach(() => { + email = gen.email() + password = gen.password() + cy.useConfig((builder) => + builder + .longPrivilegedSessionTime() + .useLaxAal() + .enableCode() + .enableCodeMFA(), + ) + + cy.register({ + email, + password, + fields: { "traits.website": website }, + }) + cy.deleteMail() + cy.visit(login + "?aal=aal2") + }) + + it("should be asked to sign in with 2fa if set up", () => { + cy.get("input[name='identifier']").type(email) + cy.contains("Sign in with code").click() + + cy.get("input[name='code']").should("be.visible") + cy.getLoginCodeFromEmail(email).then((code) => { + cy.get("input[name='code']").type(code) + cy.contains("Submit").click() + }) + + cy.getSession({ + expectAal: "aal2", + expectMethods: ["password", "code"], + }) + }) + + it("can't use different email in 2fa request", () => { + cy.get("input[name='identifier']").type(gen.email()) + cy.contains("Sign in with code").click() + + cy.get("*[data-testid='ui/message/4010010']").should("be.visible") + cy.get("input[name='code']").should("not.exist") + cy.get("input[name='identifier']").should("be.visible") + + // The current session should be unchanged + cy.getSession({ + expectAal: "aal1", + expectMethods: ["password"], + }) + }) + + it("entering wrong code should not invalidate correct codes", () => { + cy.get("input[name='identifier']").type(email) + cy.contains("Sign in with code").click() + + cy.get("input[name='code']").should("be.visible") + cy.get("input[name='code']").type("123456") + cy.contains("Submit").click() + cy.getLoginCodeFromEmail(email).then((code) => { + cy.get("input[name='code']").type(code) + cy.contains("Submit").click() + }) + + cy.getSession({ + expectAal: "aal2", + expectMethods: ["password", "code"], + }) + }) + }) + }) +}) diff --git a/test/e2e/cypress/support/config.d.ts b/test/e2e/cypress/support/config.d.ts index 060cb12822bf..c7e9742aed39 100644 --- a/test/e2e/cypress/support/config.d.ts +++ b/test/e2e/cypress/support/config.d.ts @@ -1,4 +1,4 @@ -// Copyright © 2023 Ory Corp +// Copyright © 2024 Ory Corp // SPDX-License-Identifier: Apache-2.0 /* eslint-disable */ @@ -111,6 +111,7 @@ export type OverrideTheBaseURLWhichShouldBeUsedAsTheBaseForRecoveryAndVerificati string export type HowLongALinkIsValidFor = string export type EnablesLoginAndRegistrationWithTheCodeMethod = boolean +export type EnablesLoginFlowsCodeMethodToFulfilMFARequests = boolean /** * This setting allows the code method to always login a user with code if they have registered with another authentication method such as password or social sign in. */ @@ -200,6 +201,7 @@ export type SelfServiceOIDCProvider = SelfServiceOIDCProvider1 & { apple_private_key?: ApplePrivateKey requested_claims?: OpenIDConnectClaims organization_id?: OrganizationID + additional_id_token_audiences?: AdditionalClientIdsAllowedWhenUsingIDTokenSubmission } export type SelfServiceOIDCProvider1 = { [k: string]: unknown | undefined @@ -259,6 +261,7 @@ export type ApplePrivateKey = string * The ID of the organization that this provider belongs to. Only effective in the Ory Network. */ export type OrganizationID = string +export type AdditionalClientIdsAllowedWhenUsingIDTokenSubmission = string[] /** * A list and configuration of OAuth2 and OpenID Connect providers Ory Kratos should integrate with. */ @@ -335,6 +338,14 @@ export type HTTPAddressOfAPIEndpoint1 = string export type AuthMechanisms1 = | WebHookAuthApiKeyProperties | WebHookAuthBasicAuthProperties +/** + * The channel id. Corresponds to the .via property of the identity schema for recovery, verification, etc. Currently only phone is supported. + */ +export type ChannelId = "sms" +/** + * The channel type. Currently only http is supported. + */ +export type ChannelType = "http" /** * If set, the login and registration flows will handle the Ory OAuth 2.0 & OpenID `login_challenge` query parameter to serve as an OpenID Connect Provider. This URL should point to Ory Hydra when you are not running on the Ory Network and be left untouched otherwise. */ @@ -563,6 +574,7 @@ export interface OryKratosConfiguration2 { } code?: { passwordless_enabled?: EnablesLoginAndRegistrationWithTheCodeMethod + mfa_enabled?: EnablesLoginFlowsCodeMethodToFulfilMFARequests passwordless_login_fallback_enabled?: PasswordlessLoginFallbackEnabled enabled?: EnablesCodeMethod config?: CodeConfiguration @@ -959,10 +971,25 @@ export interface CourierConfiguration { * Defines the maximum number of times the sending of a message is retried after it failed before it is marked as abandoned */ message_retries?: number + /** + * Configures the dispatch worker. + */ + worker?: { + /** + * Defines how many messages are pulled from the queue at once. + */ + pull_count?: number + /** + * Defines how long the worker waits before pulling messages from the queue again. + */ + pull_wait?: string + [k: string]: unknown | undefined + } delivery_strategy?: DeliveryStrategy http?: HTTPConfiguration - smtp: SMTPConfiguration + smtp?: SMTPConfiguration sms?: SMSSenderConfiguration + channels?: CourierChannelConfiguration[] } export interface CourierTemplates { invalid?: { @@ -970,6 +997,7 @@ export interface CourierTemplates { } valid?: { email: EmailCourierTemplate + sms?: SmsCourierTemplate } } export interface EmailCourierTemplate { @@ -985,6 +1013,14 @@ export interface EmailCourierTemplate { } subject?: string } +export interface SmsCourierTemplate { + body?: { + /** + * A template send to the SMS provider. + */ + plaintext?: string + } +} /** * Configures outgoing emails using HTTP. */ @@ -1087,6 +1123,11 @@ export interface SMSSenderConfiguration { additionalProperties?: false } } +export interface CourierChannelConfiguration { + id: ChannelId + type?: ChannelType + request_config: HttpRequestConfig +} export interface OAuth2ProviderConfiguration { url?: OAuth20ProviderURL headers?: HTTPRequestHeaders diff --git a/test/e2e/cypress/support/configHelpers.ts b/test/e2e/cypress/support/configHelpers.ts index 545bff2449c3..c8ccf05b70d2 100644 --- a/test/e2e/cypress/support/configHelpers.ts +++ b/test/e2e/cypress/support/configHelpers.ts @@ -132,4 +132,13 @@ export class ConfigBuilder { this.config.session.whoami.required_aal = "aal1" return this } + public enableCode() { + this.config.selfservice.methods.code.enabled = true + return this + } + + public enableCodeMFA() { + this.config.selfservice.methods.code.mfa_enabled = true + return this + } } diff --git a/text/id.go b/text/id.go index fa8184f0ffaf..a4c57761e53a 100644 --- a/text/id.go +++ b/text/id.go @@ -153,6 +153,7 @@ const ( ErrorValidationLoginRetrySuccess // 4010007 ErrorValidationLoginCodeInvalidOrAlreadyUsed // 4010008 ErrorValidationLoginLinkedCredentialsDoNotMatch // 4010009 + ErrorValidationLoginAddressUnknown // 4010010 ) const ( diff --git a/text/message_login.go b/text/message_login.go index 4918cb893701..ab5be6d71806 100644 --- a/text/message_login.go +++ b/text/message_login.go @@ -117,7 +117,6 @@ func NewInfoLoginWith(provider string) *Message { } func NewInfoLoginWithAndLink(provider string) *Message { - return &Message{ ID: InfoSelfServiceLoginWithAndLink, Text: fmt.Sprintf("Sign in with %s and link credential", provider), @@ -243,3 +242,11 @@ func NewErrorValidationLoginLinkedCredentialsDoNotMatch() *Message { Type: Error, } } + +func NewErrorValidationAddressUnknown() *Message { + return &Message{ + ID: ErrorValidationLoginAddressUnknown, + Text: "The address you entered does not match any known addresses in the current account.", + Type: Error, + } +} From 1f8517b9c8df3b56f50e4e5410cfc1d08ce346fb Mon Sep 17 00:00:00 2001 From: Jonas Hungershausen Date: Fri, 12 Jan 2024 18:55:01 +0100 Subject: [PATCH 04/19] chore: clean up mail assertions --- cmd/clidoc/main.go | 5 +- test/e2e/cypress/support/commands.ts | 185 +++++++++++++++------------ test/e2e/cypress/support/index.d.ts | 14 +- 3 files changed, 110 insertions(+), 94 deletions(-) diff --git a/cmd/clidoc/main.go b/cmd/clidoc/main.go index 6a9ff0610017..6265157c9ca2 100644 --- a/cmd/clidoc/main.go +++ b/cmd/clidoc/main.go @@ -168,6 +168,7 @@ func init() { "NewErrorValidationRegistrationRetrySuccessful": text.NewErrorValidationRegistrationRetrySuccessful(), "NewInfoSelfServiceRegistrationRegisterCode": text.NewInfoSelfServiceRegistrationRegisterCode(), "NewErrorValidationLoginLinkedCredentialsDoNotMatch": text.NewErrorValidationLoginLinkedCredentialsDoNotMatch(), + "NewErrorValidationAddressUnknown": text.NewErrorValidationAddressUnknown(), } } @@ -247,7 +248,7 @@ func writeMessages(path string, sortedMessages []*text.Message) error { r := regexp.MustCompile(`(?s)(.*?)`) result := r.ReplaceAllString(string(content), "\n"+w.String()+"\n") - f, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0755) + f, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0o755) if err != nil { return err } @@ -266,7 +267,7 @@ func writeMessages(path string, sortedMessages []*text.Message) error { func writeMessagesJson(path string, sortedMessages []*text.Message) error { result := codeEncode(sortedMessages) - f, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0755) + f, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0o755) if err != nil { return err } diff --git a/test/e2e/cypress/support/commands.ts b/test/e2e/cypress/support/commands.ts index f8f2b7ad0615..d047c94387da 100644 --- a/test/e2e/cypress/support/commands.ts +++ b/test/e2e/cypress/support/commands.ts @@ -16,9 +16,9 @@ import { import dayjs from "dayjs" import YAML from "yamljs" -import { Strategy } from "." +import { MailMessage, Strategy } from "." import { OryKratosConfiguration } from "./config" -import { UiNode, UiNodeAttributes } from "@ory/kratos-client" +import { UiNode } from "@ory/kratos-client" import { ConfigBuilder } from "./configHelpers" const configFile = "kratos.generated.yml" @@ -1023,7 +1023,13 @@ Cypress.Commands.add("deleteMail", ({ atLeast = 0 } = {}) => { return Promise.resolve() }) - return req() + try { + return req() + } catch (e) { + cy.log(e) + // just retry, since retry logic is already implemented in req() and will abort after enough tries + return req() + } }) Cypress.Commands.add( @@ -1147,10 +1153,11 @@ Cypress.Commands.add( // Uses the verification email but waits so that it expires Cypress.Commands.add("recoverEmailButExpired", ({ expect: { email } }) => { - cy.getMail().should((message) => { - expect(message.subject).to.equal("Recover access to your account") - expect(message.toAddresses[0].trim()).to.equal(email) - + cy.getMail({ + removeMail: true, + email, + subject: "Recover access to your account", + }).should((message) => { const link = parseHtml(message.body).querySelector("a") expect(link).to.not.be.null expect(link.href).to.contain(APP_URL) @@ -1162,10 +1169,11 @@ Cypress.Commands.add("recoverEmailButExpired", ({ expect: { email } }) => { Cypress.Commands.add( "recoveryEmailWithCode", ({ expect: { email, enterCode = true } }) => { - cy.getMail({ removeMail: true }).should((message) => { - expect(message.subject).to.equal("Recover access to your account") - expect(message.toAddresses[0].trim()).to.equal(email) - + cy.getMail({ + removeMail: true, + email, + subject: "Recover access to your account", + }).should((message) => { const code = extractRecoveryCode(message.body) expect(code).to.not.be.undefined expect(code.length).to.equal(6) @@ -1180,30 +1188,37 @@ Cypress.Commands.add( Cypress.Commands.add( "recoverEmail", ({ expect: { email }, shouldVisit = true }) => - cy.getMail().should((message) => { - expect(message.subject).to.equal("Recover access to your account") - expect(message.fromAddress.trim()).to.equal("no-reply@ory.kratos.sh") - expect(message.toAddresses).to.have.length(1) - expect(message.toAddresses[0].trim()).to.equal(email) + cy + .getMail({ + removeMail: true, + email, + subject: "Recover access to your account", + }) + .should((message) => { + expect(message.fromAddress.trim()).to.equal("no-reply@ory.kratos.sh") + expect(message.toAddresses).to.have.length(1) + expect(message.toAddresses[0].trim()).to.equal(email) - const link = parseHtml(message.body).querySelector("a") - expect(link).to.not.be.null - expect(link.href).to.contain(APP_URL) + const link = parseHtml(message.body).querySelector("a") + expect(link).to.not.be.null + expect(link.href).to.contain(APP_URL) - if (shouldVisit) { - cy.visit(link.href) - } - return link.href - }), + if (shouldVisit) { + cy.visit(link.href) + } + return link.href + }), ) // Uses the verification email but waits so that it expires Cypress.Commands.add( "verifyEmailButExpired", ({ expect: { email }, strategy = "code" }) => { - cy.getMail().should((message) => { - expect(message.subject).to.equal("Please verify your email address") - + cy.getMail({ + removeMail: true, + email, + subject: "Please verify your email address", + }).should((message) => { expect(message.fromAddress.trim()).to.equal("no-reply@ory.kratos.sh") expect(message.toAddresses).to.have.length(1) expect(message.toAddresses[0].trim()).to.equal(email) @@ -1236,13 +1251,6 @@ Cypress.Commands.add( }, ) -Cypress.Commands.add("useVerificationStrategy", (strategy: Strategy) => { - cy.updateConfigFile((config) => { - config.selfservice.flows.verification.use = strategy - return config - }) -}) - Cypress.Commands.add("useLookupSecrets", (value: boolean) => { cy.updateConfigFile((config) => { config.selfservice.methods = { @@ -1269,45 +1277,60 @@ Cypress.Commands.add("expectSettingsSaved", () => { Cypress.Commands.add( "getMail", - ({ removeMail = true, expectedCount = 1, email = undefined } = {}) => { + ({ + removeMail = true, + expectedCount = 1, + email = undefined, + subject = undefined, + } = {}) => { let tries = 0 const req = () => - cy.request(`${MAIL_API}/mail`).then((response) => { - expect(response.body).to.have.property("mailItems") - let count = response.body.mailItems.length - if (count === 0 && tries < 100) { - tries++ - cy.wait(pollInterval) - return req() - } - - let mailItem: any - if (email) { - const filtered = response.body.mailItems.filter((m: any) => - m.toAddresses.includes(email), - ) - - if (filtered.length === 0) { + cy + .request(`${MAIL_API}/mail`) + .then((response: Cypress.Response<{ mailItems: MailMessage[] }>) => { + expect(response.body).to.have.property("mailItems") + let count = response.body.mailItems.length + if (count === 0 && tries < 100) { tries++ cy.wait(pollInterval) return req() } - expect(filtered.length).to.equal(expectedCount) - mailItem = filtered[0] - } else { - expect(count).to.equal(expectedCount) - mailItem = response.body.mailItems[0] - } + let mailItem: MailMessage - if (removeMail) { - return cy.deleteMail({ atLeast: count }).then(() => { - return Promise.resolve(mailItem) - }) - } + if (!subject && !email) { + expect(count).to.equal(expectedCount) + mailItem = response.body.mailItems[0] + } else { + const filters: ((m: MailMessage) => boolean)[] = [] + if (email) { + filters.push((m: MailMessage) => m.toAddresses.includes(email)) + } + if (subject) { + filters.push((m: MailMessage) => m.subject.includes(subject)) + } + const filtered = response.body.mailItems.filter((m) => { + return filters.every((f) => f(m)) + }) + + if (filtered.length === 0) { + tries++ + cy.wait(pollInterval) + return req() + } + + expect(filtered.length).to.equal(expectedCount) + mailItem = filtered[0] + + if (removeMail) { + return cy.deleteMail({ atLeast: count }).then(() => { + return Promise.resolve(mailItem) + }) + } + } - return Promise.resolve(mailItem) - }) + return Promise.resolve(mailItem) + }) return req() }, @@ -1378,8 +1401,8 @@ Cypress.Commands.add( Cypress.Commands.add( "shouldHaveCsrfError", ({ app }: { app: "express" | "react" }) => { - let initial - let pathname + let initial: string + let pathname: string cy.location().should((location) => { initial = location.search pathname = location.pathname @@ -1490,9 +1513,12 @@ Cypress.Commands.add( Cypress.Commands.add("getVerificationCodeFromEmail", (email) => { return cy - .getMail({ removeMail: true }) + .getMail({ + removeMail: true, + email, + subject: "Please verify your email address", + }) .should((message) => { - expect(message.subject).to.equal("Please verify your email address") expect(message.toAddresses[0].trim()).to.equal(email) }) .then((message) => { @@ -1503,18 +1529,15 @@ Cypress.Commands.add("getVerificationCodeFromEmail", (email) => { }) }) -Cypress.Commands.add("enableRegistrationViaCode", (enable: boolean = true) => { - cy.updateConfigFile((config) => { - config.selfservice.methods.code.passwordless_enabled = enable - return config - }) -}) - Cypress.Commands.add("getRegistrationCodeFromEmail", (email, opts) => { return cy - .getMail({ removeMail: true, email, ...opts }) + .getMail({ + removeMail: true, + email, + subject: "Complete your account registration", + ...opts, + }) .should((message) => { - expect(message.subject).to.equal("Complete your account registration") expect(message.toAddresses[0].trim()).to.equal(email) }) .then((message) => { @@ -1527,9 +1550,13 @@ Cypress.Commands.add("getRegistrationCodeFromEmail", (email, opts) => { Cypress.Commands.add("getLoginCodeFromEmail", (email, opts) => { return cy - .getMail({ removeMail: true, email, ...opts }) + .getMail({ + removeMail: true, + email, + subject: "Login to your account", + ...opts, + }) .should((message) => { - expect(message.subject).to.equal("Login to your account") expect(message.toAddresses[0].trim()).to.equal(email) }) .then((message) => { diff --git a/test/e2e/cypress/support/index.d.ts b/test/e2e/cypress/support/index.d.ts index 1792db4a51cc..bd618ceedf8f 100644 --- a/test/e2e/cypress/support/index.d.ts +++ b/test/e2e/cypress/support/index.d.ts @@ -108,6 +108,7 @@ declare global { removeMail: boolean expectedCount?: number email?: string + subject?: string }): Chainable performEmailVerification(opts?: { @@ -560,13 +561,6 @@ declare global { strategy?: Strategy }): Chainable - /** - * Sets the strategy to use for verification - * - * @param strategy the Strategy - */ - useVerificationStrategy(strategy: Strategy): Chainable - /** * Disables verification */ @@ -723,12 +717,6 @@ declare global { */ getVerificationCodeFromEmail(email: string): Chainable - /** - * Enables the registration code method - * @param enable - */ - enableRegistrationViaCode(enable: boolean): Chainable - /** * Extracts a registration code from the received email */ From f1e15fa9f9b2587de787ce4af45fa10f24a65659 Mon Sep 17 00:00:00 2001 From: Jonas Hungershausen Date: Mon, 15 Jan 2024 11:50:47 +0100 Subject: [PATCH 05/19] chore: fix e2e tests --- .../strategy/code/strategy_login_test.go | 2 +- .../code/registration/success.spec.ts | 20 +++------------ .../profiles/oidc-provider/login.spec.ts | 4 ++- .../profiles/recovery/code/errors.spec.ts | 18 ++++++++++--- .../profiles/recovery/link/errors.spec.ts | 25 +++++++++++++++---- .../verification/registration/errors.spec.ts | 5 +++- .../verification/settings/error.spec.ts | 5 +++- .../verification/verify/errors.spec.ts | 19 +++++++++++--- .../verification/verify/success.spec.ts | 5 +++- test/e2e/cypress/support/commands.ts | 7 ++++-- test/e2e/cypress/support/index.d.ts | 4 +-- 11 files changed, 76 insertions(+), 38 deletions(-) diff --git a/selfservice/strategy/code/strategy_login_test.go b/selfservice/strategy/code/strategy_login_test.go index 14281bfd4654..26348761e01d 100644 --- a/selfservice/strategy/code/strategy_login_test.go +++ b/selfservice/strategy/code/strategy_login_test.go @@ -658,7 +658,7 @@ func TestLoginCodeStrategy(t *testing.T) { v.Set("identifier", email) }, true, nil) - require.Equal(t, "The supplied address does not match any known addresses.", gjson.Get(s.body, "ui.messages.0.text").String(), "%s", body) + require.Equal(t, "The address you entered does not match any known addresses in the current account.", gjson.Get(s.body, "ui.messages.0.text").String(), "%s", body) }) }) } diff --git a/test/e2e/cypress/integration/profiles/code/registration/success.spec.ts b/test/e2e/cypress/integration/profiles/code/registration/success.spec.ts index cdc5e8019219..53fd71ac0f66 100644 --- a/test/e2e/cypress/integration/profiles/code/registration/success.spec.ts +++ b/test/e2e/cypress/integration/profiles/code/registration/success.spec.ts @@ -289,23 +289,9 @@ context("Registration success with code method", () => { cy.get(Selectors[app]["submitRecovery"]).click() - if (app === "mobile") { - cy.get('[data-testid="session-token"]').then((token) => { - cy.getSession({ - expectAal: "aal1", - token: token.text(), - }).then((session) => { - cy.wrap(session).as("session") - }) - }) - - cy.get('[data-testid="session-content"]').should("contain", email) - cy.get('[data-testid="session-token"]').should("not.be.empty") - } else { - cy.getSession({ expectAal: "aal1" }).then((session) => { - cy.wrap(session).as("session") - }) - } + cy.getSession({ expectAal: "aal1" }).then((session) => { + cy.wrap(session).as("session") + }) cy.get("@session").then(({ identity }) => { expect(identity.id).to.not.be.empty diff --git a/test/e2e/cypress/integration/profiles/oidc-provider/login.spec.ts b/test/e2e/cypress/integration/profiles/oidc-provider/login.spec.ts index 8264047a8f44..861cfd0d2a79 100644 --- a/test/e2e/cypress/integration/profiles/oidc-provider/login.spec.ts +++ b/test/e2e/cypress/integration/profiles/oidc-provider/login.spec.ts @@ -273,7 +273,9 @@ context("OpenID Provider - change between flows", () => { cy.notifyUnknownRecipients("recovery", false) cy.longPrivilegedSessionTime() - const fakeOidcFlow = (identity: identityWithWebsite) => { + const fakeOidcFlow = ( + identity: ReturnType, + ) => { cy.get("input[name='username']").type(identity.email) cy.get("button[name='action'][value='accept']").click() // consent screen for the 'fake' oidc provider diff --git a/test/e2e/cypress/integration/profiles/recovery/code/errors.spec.ts b/test/e2e/cypress/integration/profiles/recovery/code/errors.spec.ts index f0e28989aed2..23d877049e6c 100644 --- a/test/e2e/cypress/integration/profiles/recovery/code/errors.spec.ts +++ b/test/e2e/cypress/integration/profiles/recovery/code/errors.spec.ts @@ -114,7 +114,10 @@ context("Account Recovery Errors", () => { ) cy.get('input[name="code"]').should("be.visible") - cy.getMail().should((message) => { + cy.getMail({ + subject: "Account access attempted", + email, + }).should((message) => { expect(message.subject).to.equal("Account access attempted") expect(message.fromAddress.trim()).to.equal("no-reply@ory.kratos.sh") expect(message.toAddresses).to.have.length(1) @@ -182,7 +185,10 @@ context("Account Recovery Errors", () => { "An email containing a recovery code has been sent to the email address you provided. If you have not received an email, check the spelling of the address and make sure to use the address you registered with.", ) - cy.getMail().then((mail) => { + cy.getMail({ + subject: "recovery_code_valid REMOTE TEMPLATE SUBJECT", + email: identity.email, + }).then((mail) => { expect(mail.body).to.include("recovery_code_valid REMOTE TEMPLATE") }) }) @@ -191,14 +197,18 @@ context("Account Recovery Errors", () => { cy.notifyUnknownRecipients("recovery") cy.remoteCourierRecoveryCodeTemplates() cy.visit(recovery) - cy.get(appPrefix(app) + "input[name='email']").type(email()) + const email = gen.email() + cy.get(appPrefix(app) + "input[name='email']").type(email) cy.get("button[value='code']").click() cy.get('[data-testid="ui/message/1060003"]').should( "have.text", "An email containing a recovery code has been sent to the email address you provided. If you have not received an email, check the spelling of the address and make sure to use the address you registered with.", ) - cy.getMail().then((mail) => { + cy.getMail({ + subject: "recovery_code_invalid REMOTE TEMPLATE SUBJECT", + email: email, + }).then((mail) => { expect(mail.body).to.include("recovery_code_invalid REMOTE TEMPLATE") }) }) diff --git a/test/e2e/cypress/integration/profiles/recovery/link/errors.spec.ts b/test/e2e/cypress/integration/profiles/recovery/link/errors.spec.ts index 97d72d3221de..4b01db22a70c 100644 --- a/test/e2e/cypress/integration/profiles/recovery/link/errors.spec.ts +++ b/test/e2e/cypress/integration/profiles/recovery/link/errors.spec.ts @@ -72,7 +72,10 @@ context("Account Recovery Errors", () => { cy.recoverApi({ email: identity.email }) cy.wait(1000) - cy.getMail().should((message) => { + cy.getMail({ + subject: "Recover access to your account", + email: identity.email, + }).should((message) => { expect(message.subject).to.equal("Recover access to your account") expect(message.toAddresses[0].trim()).to.equal(identity.email) @@ -104,7 +107,10 @@ context("Account Recovery Errors", () => { ) cy.get('input[name="email"]').should("have.value", email) - cy.getMail().should((message) => { + cy.getMail({ + subject: "Account access attempted", + email, + }).should((message) => { expect(message.subject).to.equal("Account access attempted") expect(message.fromAddress.trim()).to.equal("no-reply@ory.kratos.sh") expect(message.toAddresses).to.have.length(1) @@ -172,7 +178,10 @@ context("Account Recovery Errors", () => { cy.registerApi(identity) cy.recoverApi({ email: identity.email }) - cy.getMail().then((mail) => { + cy.getMail({ + subject: "Recover access to your account", + email: identity.email, + }).then((mail) => { console.log(mail) const link = parseHtml(mail.body).querySelector("a") cy.visit(link.href + "-not") // add random stuff to the confirm challenge @@ -189,7 +198,10 @@ context("Account Recovery Errors", () => { cy.registerApi(identity) cy.recoverApi({ email: identity.email }) - cy.getMail().then((mail) => { + cy.getMail({ + subject: "Recover access to your account", + email: identity.email, + }).then((mail) => { const link = parseHtml(mail.body).querySelector("a") // Workaround for cypress cy.visit limitation. @@ -215,7 +227,10 @@ context("Account Recovery Errors", () => { const identity = gen.identityWithWebsite() cy.recoverApi({ email: identity.email }) - cy.getMail().then((mail) => { + cy.getMail({ + subject: "Account Access Attempted", + email: identity.email, + }).then((mail) => { expect(mail.body).to.include( "this is a remote invalid recovery template", ) diff --git a/test/e2e/cypress/integration/profiles/verification/registration/errors.spec.ts b/test/e2e/cypress/integration/profiles/verification/registration/errors.spec.ts index 74b0ad72231c..06a68f4af237 100644 --- a/test/e2e/cypress/integration/profiles/verification/registration/errors.spec.ts +++ b/test/e2e/cypress/integration/profiles/verification/registration/errors.spec.ts @@ -65,7 +65,10 @@ context("Account Verification Registration Errors", () => { }) it("is unable to verify the email address if the code is incorrect", () => { - cy.getMail().then((mail) => { + cy.getMail({ + subject: "Please verify your email address", + email: identity.email, + }).then((mail) => { const link = parseHtml(mail.body).querySelector("a") expect(verifyHrefPattern.test(link.href)).to.be.true diff --git a/test/e2e/cypress/integration/profiles/verification/settings/error.spec.ts b/test/e2e/cypress/integration/profiles/verification/settings/error.spec.ts index 128234a2d570..287f63a931e0 100644 --- a/test/e2e/cypress/integration/profiles/verification/settings/error.spec.ts +++ b/test/e2e/cypress/integration/profiles/verification/settings/error.spec.ts @@ -71,7 +71,10 @@ context("Account Verification Settings Error", () => { cy.get('input[name="traits.email"]').clear().type(email) cy.get('button[value="profile"]').click() - cy.getMail().then((mail) => { + cy.getMail({ + subject: "Please verify your email address", + email, + }).then((mail) => { const link = parseHtml(mail.body).querySelector("a") expect(verifyHrefPattern.test(link.href)).to.be.true diff --git a/test/e2e/cypress/integration/profiles/verification/verify/errors.spec.ts b/test/e2e/cypress/integration/profiles/verification/verify/errors.spec.ts index 0b47b0838f92..97fd908a2e22 100644 --- a/test/e2e/cypress/integration/profiles/verification/verify/errors.spec.ts +++ b/test/e2e/cypress/integration/profiles/verification/verify/errors.spec.ts @@ -68,7 +68,11 @@ context("Account Verification Error", () => { cy.wait(1000) cy.shortVerificationLifespan() - cy.getMail().then((message) => { + cy.getMail({ + removeMail: true, + subject: "Please verify your email address", + email: identity.email, + }).then((message) => { expect(message.subject).to.equal( "Please verify your email address", ) @@ -101,6 +105,8 @@ context("Account Verification Error", () => { strategy: s, }) + cy.wait(1000) + cy.verifyEmailButExpired({ expect: { email: identity.email }, strategy: s, @@ -128,7 +134,10 @@ context("Account Verification Error", () => { cy.contains("An email containing a verification") - cy.getMail().then((mail) => { + cy.getMail({ + email: identity.email, + subject: "Please verify your email address", + }).then((mail) => { const link = parseHtml(mail.body).querySelector("a") expect(verifyHrefPattern.test(link.href)).to.be.true @@ -148,7 +157,11 @@ context("Account Verification Error", () => { const email = gen.identity().email cy.get('input[name="email"]').type(email) cy.get(`button[value="${s}"]`).click() - cy.getMail().then((mail) => { + cy.getMail({ + subject: "Someone tried to verify this email address", + email, + removeMail: true, + }).then((mail) => { expect(mail.toAddresses).includes(email) expect(mail.subject).eq( "Someone tried to verify this email address", diff --git a/test/e2e/cypress/integration/profiles/verification/verify/success.spec.ts b/test/e2e/cypress/integration/profiles/verification/verify/success.spec.ts index ae39cdbc0d83..0938e80f186d 100644 --- a/test/e2e/cypress/integration/profiles/verification/verify/success.spec.ts +++ b/test/e2e/cypress/integration/profiles/verification/verify/success.spec.ts @@ -61,7 +61,10 @@ context("Account Verification Settings Success", () => { cy.contains("An email containing a verification") - cy.getMail().should((message) => { + cy.getMail({ + subject: "Someone tried to verify this email address", + email, + }).should((message) => { expect(message.subject.trim()).to.equal( "Someone tried to verify this email address", ) diff --git a/test/e2e/cypress/support/commands.ts b/test/e2e/cypress/support/commands.ts index d047c94387da..7ef6260f4b78 100644 --- a/test/e2e/cypress/support/commands.ts +++ b/test/e2e/cypress/support/commands.ts @@ -1109,7 +1109,10 @@ Cypress.Commands.add("noSession", () => Cypress.Commands.add( "performEmailVerification", ({ expect: { email, redirectTo }, strategy = "code" }) => { - cy.getMail().then((message) => { + cy.getMail({ + email, + subject: "Please verify your email address", + }).then((message) => { expect(message.subject).to.equal("Please verify your email address") expect(message.fromAddress.trim()).to.equal("no-reply@ory.kratos.sh") expect(message.toAddresses).to.have.length(1) @@ -1282,7 +1285,7 @@ Cypress.Commands.add( expectedCount = 1, email = undefined, subject = undefined, - } = {}) => { + }) => { let tries = 0 const req = () => cy diff --git a/test/e2e/cypress/support/index.d.ts b/test/e2e/cypress/support/index.d.ts index bd618ceedf8f..3fef679ec098 100644 --- a/test/e2e/cypress/support/index.d.ts +++ b/test/e2e/cypress/support/index.d.ts @@ -104,8 +104,8 @@ declare global { * * @param opts */ - getMail(opts?: { - removeMail: boolean + getMail(opts: { + removeMail?: boolean expectedCount?: number email?: string subject?: string From 686a1027240be2b199d1dbbee08840b00706334e Mon Sep 17 00:00:00 2001 From: Jonas Hungershausen Date: Mon, 15 Jan 2024 16:55:00 +0100 Subject: [PATCH 06/19] chore: revert --- selfservice/strategy/code/strategy_login.go | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/selfservice/strategy/code/strategy_login.go b/selfservice/strategy/code/strategy_login.go index 76e6f99b0561..962866c5529b 100644 --- a/selfservice/strategy/code/strategy_login.go +++ b/selfservice/strategy/code/strategy_login.go @@ -217,15 +217,10 @@ func (s *Strategy) loginSendCode(ctx context.Context, w http.ResponseWriter, r * if err != nil { return err } - matches := lo.Filter(i.VerifiableAddresses, func(va identity.VerifiableAddress, _ int) bool { - return va.Value == maybeNormalizeEmail(p.Identifier) - }) - addresses = lo.Map(matches, func(va identity.VerifiableAddress, _ int) Address { - return Address{ - To: va.Value, - Via: va.Via, - } - }) + addresses = []Address{{ + To: p.Identifier, + Via: identity.CodeAddressType(identity.AddressTypeEmail), + }} } // Step 2: Delete any previous login codes for this flow ID From 26a1df34ba226632ad17b9feeda7cb65ec708204 Mon Sep 17 00:00:00 2001 From: Jonas Hungershausen Date: Wed, 17 Jan 2024 14:58:54 +0100 Subject: [PATCH 07/19] chore: cr --- Makefile | 4 +- courier/template/sms/login_code_valid.go | 4 +- courier/template/sms/login_code_valid_test.go | 37 +++++++++++++++++++ driver/config/config.go | 6 +++ 4 files changed, 47 insertions(+), 4 deletions(-) create mode 100644 courier/template/sms/login_code_valid_test.go diff --git a/Makefile b/Makefile index 361e25e897bf..e90c4c3ea7dc 100644 --- a/Makefile +++ b/Makefile @@ -175,7 +175,7 @@ docker: DOCKER_BUILDKIT=1 DOCKER_CONTENT_TRUST=1 docker build -f .docker/Dockerfile-build --build-arg=COMMIT=$(VCS_REF) --build-arg=BUILD_DATE=$(BUILD_DATE) -t oryd/kratos:${IMAGE_TAG} . .PHONY: test-e2e -test-e2e: node_modules test-resetdb +test-e2e: node_modules test-resetdb kratos-config-e2e source script/test-envs.sh test/e2e/run.sh sqlite test/e2e/run.sh postgres @@ -183,7 +183,7 @@ test-e2e: node_modules test-resetdb test/e2e/run.sh mysql .PHONY: test-e2e-playwright -test-e2e-playwright: node_modules test-resetdb +test-e2e-playwright: node_modules test-resetdb kratos-config-e2e source script/test-envs.sh test/e2e/run.sh --only-setup (cd test/e2e; DB=memory npm run playwright) diff --git a/courier/template/sms/login_code_valid.go b/courier/template/sms/login_code_valid.go index c72189bc5f3d..194e113e6c6b 100644 --- a/courier/template/sms/login_code_valid.go +++ b/courier/template/sms/login_code_valid.go @@ -36,10 +36,10 @@ func (t *LoginCodeValid) SMSBody(ctx context.Context) (string, error) { ctx, t.deps, os.DirFS(t.deps.CourierConfig().CourierTemplatesRoot(ctx)), - "login_code/valid/sms.body.gotmpl", // TODO:!!! + "login_code/valid/sms.body.gotmpl", "login_code/valid/sms.body*", t.model, - "", + t.deps.CourierConfig().CourierSMSTemplatesLoginCodeValid(ctx).Body.PlainText, ) } diff --git a/courier/template/sms/login_code_valid_test.go b/courier/template/sms/login_code_valid_test.go new file mode 100644 index 000000000000..a2915c750c89 --- /dev/null +++ b/courier/template/sms/login_code_valid_test.go @@ -0,0 +1,37 @@ +// Copyright © 2023 Ory Corp +// SPDX-License-Identifier: Apache-2.0 + +package sms_test + +import ( + "context" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/ory/kratos/courier/template/sms" + "github.com/ory/kratos/internal" +) + +func TestNewLoginCodeValid(t *testing.T) { + _, reg := internal.NewFastRegistryWithMocks(t) + + const ( + expectedPhone = "+12345678901" + otp = "012345" + ) + + tpl := sms.NewLoginCodeValid(reg, &sms.LoginCodeValidModel{To: expectedPhone, LoginCode: otp}) + + expectedBody := fmt.Sprintf("Your login code is: %s\n", otp) + + actualBody, err := tpl.SMSBody(context.Background()) + require.NoError(t, err) + assert.Equal(t, expectedBody, actualBody) + + actualPhone, err := tpl.PhoneNumber() + require.NoError(t, err) + assert.Equal(t, expectedPhone, actualPhone) +} diff --git a/driver/config/config.go b/driver/config/config.go index 399098cebe5d..ffea63704525 100644 --- a/driver/config/config.go +++ b/driver/config/config.go @@ -67,6 +67,7 @@ const ( ViperKeyCourierTemplatesVerificationCodeInvalidEmail = "courier.templates.verification_code.invalid.email" ViperKeyCourierTemplatesVerificationCodeValidEmail = "courier.templates.verification_code.valid.email" ViperKeyCourierTemplatesVerificationCodeValidSMS = "courier.templates.verification_code.valid.sms" + ViperKeyCourierTemplatesLoginCodeValidSMS = "courier.templates.login_code.valid.sms" ViperKeyCourierDeliveryStrategy = "courier.delivery_strategy" ViperKeyCourierHTTPRequestConfig = "courier.http.request_config" ViperKeyCourierTemplatesLoginCodeValidEmail = "courier.templates.login_code.valid.email" @@ -304,6 +305,7 @@ type ( CourierTemplatesLoginCodeValid(ctx context.Context) *CourierEmailTemplate CourierTemplatesRegistrationCodeValid(ctx context.Context) *CourierEmailTemplate CourierSMSTemplatesVerificationCodeValid(ctx context.Context) *CourierSMSTemplate + CourierSMSTemplatesLoginCodeValid(ctx context.Context) *CourierSMSTemplate CourierMessageRetries(ctx context.Context) int CourierWorkerPullCount(ctx context.Context) int CourierWorkerPullWait(ctx context.Context) time.Duration @@ -1117,6 +1119,10 @@ func (p *Config) CourierSMSTemplatesVerificationCodeValid(ctx context.Context) * return p.CourierSMSTemplatesHelper(ctx, ViperKeyCourierTemplatesVerificationCodeValidSMS) } +func (p *Config) CourierSMSTemplatesLoginCodeValid(ctx context.Context) *CourierSMSTemplate { + return p.CourierSMSTemplatesHelper(ctx, ViperKeyCourierTemplatesLoginCodeValidSMS) +} + func (p *Config) CourierTemplatesLoginCodeValid(ctx context.Context) *CourierEmailTemplate { return p.CourierEmailTemplatesHelper(ctx, ViperKeyCourierTemplatesLoginCodeValidEmail) } From 523811886eef62cbf699d06c85eb83be2d5d14db Mon Sep 17 00:00:00 2001 From: Jonas Hungershausen Date: Thu, 18 Jan 2024 10:52:09 +0100 Subject: [PATCH 08/19] chore: make passwordless_enabled config change bw compat --- driver/config/config.go | 10 +++++++++- driver/registry_default_test.go | 9 +++++++++ embedx/config.schema.json | 5 +++-- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/driver/config/config.go b/driver/config/config.go index ffea63704525..6af7e7c17c5c 100644 --- a/driver/config/config.go +++ b/driver/config/config.go @@ -761,8 +761,16 @@ func (p *Config) SelfServiceStrategy(ctx context.Context, strategy string) *Self case "code", "password", "profile": defaultEnabled = true } + + // Backwards compatibility for the old "passwordless_enabled" key + // This force-enables the code strategy, if passwordless is enabled, because in earlier versions it was possible to + // disable the code strategy, but enable passwordless + enabled := pp.BoolF(basePath+".enabled", defaultEnabled) + if strategy == "code" { + enabled = enabled || pp.Bool(basePath+".passwordless_enabled") + } return &SelfServiceStrategy{ - Enabled: pp.BoolF(basePath+".enabled", defaultEnabled), + Enabled: enabled, Config: config, } } diff --git a/driver/registry_default_test.go b/driver/registry_default_test.go index 0c7d4d6e70e6..0dd43e2efc70 100644 --- a/driver/registry_default_test.go +++ b/driver/registry_default_test.go @@ -730,6 +730,15 @@ func TestDriverDefault_Strategies(t *testing.T) { }, expect: []string{"password", "code"}, }, + { + name: "code is enabled if passwordless_enabled is true", + prep: func(conf *config.Config) { + conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".password.enabled", false) + conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".code.enabled", false) + conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".code.passwordless_enabled", true) + }, + expect: []string{"code"}, + }, } { t.Run(fmt.Sprintf("run=%s", tc.name), func(t *testing.T) { conf, reg := internal.NewVeryFastRegistryWithoutDB(t) diff --git a/embedx/config.schema.json b/embedx/config.schema.json index 7c6dc570e398..3bdedc9a8111 100644 --- a/embedx/config.schema.json +++ b/embedx/config.schema.json @@ -1413,12 +1413,13 @@ "properties": { "passwordless_enabled": { "type": "boolean", - "title": "Enables Login and Registration with the Code Method", + "title": "Enables login and registration with the code method.", + "description": "If set to true, code.enabled will be set to true as well.", "default": false }, "mfa_enabled": { "type": "boolean", - "title": "Enables Login flows Code Method to fulfil MFA requests", + "title": "Enables login flows code method to fulfil MFA requests", "default": false }, "passwordless_login_fallback_enabled": { From f47c55c8142a087cf92f33ea451429e790da1967 Mon Sep 17 00:00:00 2001 From: Jonas Hungershausen Date: Fri, 19 Jan 2024 14:51:02 +0100 Subject: [PATCH 09/19] fix: config schema --- embedx/config.schema.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/embedx/config.schema.json b/embedx/config.schema.json index 3bdedc9a8111..c25414eddefc 100644 --- a/embedx/config.schema.json +++ b/embedx/config.schema.json @@ -1799,6 +1799,9 @@ "properties": { "email": { "$ref": "#/definitions/emailCourierTemplate" + }, + "sms": { + "$ref": "#/definitions/smsCourierTemplate" } }, "required": ["email"] From 8821e6b0c51f7fbec05e05712ec4d7b53647aba4 Mon Sep 17 00:00:00 2001 From: Jonas Hungershausen Date: Mon, 22 Jan 2024 10:14:20 +0100 Subject: [PATCH 10/19] chore: remove extra case --- identity/extension_credentials.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/identity/extension_credentials.go b/identity/extension_credentials.go index 709a47cb769c..3baa826b2e9c 100644 --- a/identity/extension_credentials.go +++ b/identity/extension_credentials.go @@ -70,8 +70,6 @@ func (r *SchemaExtensionCredentials) Run(ctx jsonschema.ValidationContext, s sch // } // r.setIdentifier(CredentialsTypeCodeAuth, value, CredentialsIdentifierAddressTypePhone) - case f.AddCase(""): - // continue default: return ctx.Error("", "credentials.code.via has unknown value %q", s.Credentials.Code.Via) } From bef975ef0c93c8c27cf5f422234310b3fd1603fc Mon Sep 17 00:00:00 2001 From: Jonas Hungershausen Date: Wed, 24 Jan 2024 12:30:16 +0100 Subject: [PATCH 11/19] chore: u --- cmd/clidoc/main.go | 2 + .../phone-password/identity.schema.json | 5 +- embedx/config.schema.json | 20 ++ internal/client-go/api_frontend.go | 8 + .../model_identity_credentials_code.go | 1 + internal/httpclient/api_frontend.go | 8 + .../model_identity_credentials_code.go | 1 + internal/testhelpers/selfservice_login.go | 17 +- internal/testhelpers/server.go | 5 +- .../flow/login/extension_identifier_label.go | 31 ++- selfservice/flow/login/handler.go | 5 + ...suite=mfa-case=verify_initial_payload.json | 84 +++++++ ...suite=mfa-case=verify_initial_payload.json | 70 ++++++ ...suite=mfa-case=verify_initial_payload.json | 84 +++++++ selfservice/strategy/code/strategy.go | 41 +++- selfservice/strategy/code/strategy_login.go | 12 + .../strategy/code/strategy_login_test.go | 216 ++++++++++++------ spec/api.json | 14 +- spec/swagger.json | 12 +- .../integration/profiles/mfa/code.spec.ts | 22 +- test/e2e/run.sh | 2 +- text/id.go | 2 + text/message_login.go | 19 ++ 23 files changed, 578 insertions(+), 103 deletions(-) create mode 100644 selfservice/strategy/code/.snapshots/TestLoginCodeStrategy-test=Browser_client-suite=mfa-case=verify_initial_payload.json create mode 100644 selfservice/strategy/code/.snapshots/TestLoginCodeStrategy-test=Native_client-suite=mfa-case=verify_initial_payload.json create mode 100644 selfservice/strategy/code/.snapshots/TestLoginCodeStrategy-test=SPA_client-suite=mfa-case=verify_initial_payload.json diff --git a/cmd/clidoc/main.go b/cmd/clidoc/main.go index 6265157c9ca2..ef3027a936a3 100644 --- a/cmd/clidoc/main.go +++ b/cmd/clidoc/main.go @@ -169,6 +169,8 @@ func init() { "NewInfoSelfServiceRegistrationRegisterCode": text.NewInfoSelfServiceRegistrationRegisterCode(), "NewErrorValidationLoginLinkedCredentialsDoNotMatch": text.NewErrorValidationLoginLinkedCredentialsDoNotMatch(), "NewErrorValidationAddressUnknown": text.NewErrorValidationAddressUnknown(), + "NewInfoSelfServiceLoginCodeMFA": text.NewInfoSelfServiceLoginCodeMFA(), + "NewInfoSelfServiceLoginCodeMFAHint": text.NewInfoSelfServiceLoginCodeMFAHint("{maskedIdentifier}"), } } diff --git a/contrib/quickstart/kratos/phone-password/identity.schema.json b/contrib/quickstart/kratos/phone-password/identity.schema.json index 7f757b0a1690..e3a013ffc282 100644 --- a/contrib/quickstart/kratos/phone-password/identity.schema.json +++ b/contrib/quickstart/kratos/phone-password/identity.schema.json @@ -1,5 +1,5 @@ { - "$id": "https://schemas.ory.sh/presets/kratos/quickstart/email-password/identity.schema.json", + "$id": "https://schemas.ory.sh/presets/kratos/quickstart/phone-password/identity.schema.json", "$schema": "http://json-schema.org/draft-07/schema#", "title": "Person", "type": "object", @@ -36,9 +36,6 @@ "credentials": { "password": { "identifier": true - }, - "code": { - "identifier": true } }, "verification": { diff --git a/embedx/config.schema.json b/embedx/config.schema.json index c25414eddefc..e13623c343f6 100644 --- a/embedx/config.schema.json +++ b/embedx/config.schema.json @@ -1410,6 +1410,26 @@ "code": { "type": "object", "additionalProperties": false, + "oneOf": [ + { + "properties": { + "passwordless_enabled": { "const": true }, + "mfa_enabled": { "const": false } + } + }, + { + "properties": { + "mfa_enabled": { "const": true }, + "passwordless_enabled": { "const": false } + } + }, + { + "properties": { + "mfa_enabled": { "const": false }, + "passwordless_enabled": { "const": false } + } + } + ], "properties": { "passwordless_enabled": { "type": "boolean", diff --git a/internal/client-go/api_frontend.go b/internal/client-go/api_frontend.go index ab8c45632935..94c0d05a6dba 100644 --- a/internal/client-go/api_frontend.go +++ b/internal/client-go/api_frontend.go @@ -1890,6 +1890,7 @@ type FrontendApiApiCreateNativeLoginFlowRequest struct { xSessionToken *string returnSessionTokenExchangeCode *bool returnTo *string + via *string } func (r FrontendApiApiCreateNativeLoginFlowRequest) Refresh(refresh bool) FrontendApiApiCreateNativeLoginFlowRequest { @@ -1912,6 +1913,10 @@ func (r FrontendApiApiCreateNativeLoginFlowRequest) ReturnTo(returnTo string) Fr r.returnTo = &returnTo return r } +func (r FrontendApiApiCreateNativeLoginFlowRequest) Via(via string) FrontendApiApiCreateNativeLoginFlowRequest { + r.via = &via + return r +} func (r FrontendApiApiCreateNativeLoginFlowRequest) Execute() (*LoginFlow, *http.Response, error) { return r.ApiService.CreateNativeLoginFlowExecute(r) @@ -1986,6 +1991,9 @@ func (a *FrontendApiService) CreateNativeLoginFlowExecute(r FrontendApiApiCreate if r.returnTo != nil { localVarQueryParams.Add("return_to", parameterToString(*r.returnTo, "")) } + if r.via != nil { + localVarQueryParams.Add("via", parameterToString(*r.via, "")) + } // to determine the Content-Type header localVarHTTPContentTypes := []string{} diff --git a/internal/client-go/model_identity_credentials_code.go b/internal/client-go/model_identity_credentials_code.go index f542b359639a..75857f31c272 100644 --- a/internal/client-go/model_identity_credentials_code.go +++ b/internal/client-go/model_identity_credentials_code.go @@ -18,6 +18,7 @@ import ( // IdentityCredentialsCode CredentialsCode represents a one time login/registration code type IdentityCredentialsCode struct { + // The type of the address for this code AddressType *string `json:"address_type,omitempty"` UsedAt NullableTime `json:"used_at,omitempty"` } diff --git a/internal/httpclient/api_frontend.go b/internal/httpclient/api_frontend.go index ab8c45632935..94c0d05a6dba 100644 --- a/internal/httpclient/api_frontend.go +++ b/internal/httpclient/api_frontend.go @@ -1890,6 +1890,7 @@ type FrontendApiApiCreateNativeLoginFlowRequest struct { xSessionToken *string returnSessionTokenExchangeCode *bool returnTo *string + via *string } func (r FrontendApiApiCreateNativeLoginFlowRequest) Refresh(refresh bool) FrontendApiApiCreateNativeLoginFlowRequest { @@ -1912,6 +1913,10 @@ func (r FrontendApiApiCreateNativeLoginFlowRequest) ReturnTo(returnTo string) Fr r.returnTo = &returnTo return r } +func (r FrontendApiApiCreateNativeLoginFlowRequest) Via(via string) FrontendApiApiCreateNativeLoginFlowRequest { + r.via = &via + return r +} func (r FrontendApiApiCreateNativeLoginFlowRequest) Execute() (*LoginFlow, *http.Response, error) { return r.ApiService.CreateNativeLoginFlowExecute(r) @@ -1986,6 +1991,9 @@ func (a *FrontendApiService) CreateNativeLoginFlowExecute(r FrontendApiApiCreate if r.returnTo != nil { localVarQueryParams.Add("return_to", parameterToString(*r.returnTo, "")) } + if r.via != nil { + localVarQueryParams.Add("via", parameterToString(*r.via, "")) + } // to determine the Content-Type header localVarHTTPContentTypes := []string{} diff --git a/internal/httpclient/model_identity_credentials_code.go b/internal/httpclient/model_identity_credentials_code.go index f542b359639a..75857f31c272 100644 --- a/internal/httpclient/model_identity_credentials_code.go +++ b/internal/httpclient/model_identity_credentials_code.go @@ -18,6 +18,7 @@ import ( // IdentityCredentialsCode CredentialsCode represents a one time login/registration code type IdentityCredentialsCode struct { + // The type of the address for this code AddressType *string `json:"address_type,omitempty"` UsedAt NullableTime `json:"used_at,omitempty"` } diff --git a/internal/testhelpers/selfservice_login.go b/internal/testhelpers/selfservice_login.go index e92771c969fc..6469aec52ac3 100644 --- a/internal/testhelpers/selfservice_login.go +++ b/internal/testhelpers/selfservice_login.go @@ -58,6 +58,7 @@ type initFlowOptions struct { returnTo string refresh bool oauth2LoginChallenge string + via string } func (o *initFlowOptions) apply(opts []InitFlowWithOption) *initFlowOptions { @@ -86,6 +87,9 @@ func getURLFromInitOptions(ts *httptest.Server, path string, forced bool, opts . if o.oauth2LoginChallenge != "" { q.Set("login_challenge", o.oauth2LoginChallenge) } + if o.via != "" { + q.Set("via", o.via) + } u := urlx.ParseOrPanic(ts.URL + path) u.RawQuery = q.Encode() @@ -118,6 +122,12 @@ func InitFlowWithOAuth2LoginChallenge(hlc string) InitFlowWithOption { } } +func InitFlowWithVia(via string) InitFlowWithOption { + return func(o *initFlowOptions) { + o.via = via + } +} + func InitializeLoginFlowViaBrowser(t *testing.T, client *http.Client, ts *httptest.Server, forced bool, isSPA bool, expectInitError bool, expectGetError bool, opts ...InitFlowWithOption) *kratos.LoginFlow { publicClient := NewSDKCustomClient(ts, client) @@ -164,9 +174,12 @@ func InitializeLoginFlowViaAPI(t *testing.T, client *http.Client, ts *httptest.S if o.aal != "" { req = req.Aal(string(o.aal)) } + if o.via != "" { + req = req.Via(o.via) + } - rs, _, err := req.Execute() - require.NoError(t, err) + rs, res, err := req.Execute() + require.NoError(t, err, "%s", ioutilx.MustReadAll(res.Body)) assert.Empty(t, rs.Active) return rs diff --git a/internal/testhelpers/server.go b/internal/testhelpers/server.go index c3dffb9ab9d8..2608b04f2d1f 100644 --- a/internal/testhelpers/server.go +++ b/internal/testhelpers/server.go @@ -33,7 +33,10 @@ func NewKratosServerWithCSRFAndRouters(t *testing.T, reg driver.Registry) (publi ran := negroni.New() ran.UseFunc(x.RedirectAdminMiddleware) ran.UseHandler(ra) - public = httptest.NewServer(x.NewTestCSRFHandler(rp, reg)) + rpn := negroni.New() + rpn.UseFunc(x.HTTPLoaderContextMiddleware(reg)) + rpn.UseHandler(rp) + public = httptest.NewServer(x.NewTestCSRFHandler(rpn, reg)) admin = httptest.NewServer(ran) ctx := context.Background() diff --git a/selfservice/flow/login/extension_identifier_label.go b/selfservice/flow/login/extension_identifier_label.go index 02b725f00b7f..1774f62a9c3c 100644 --- a/selfservice/flow/login/extension_identifier_label.go +++ b/selfservice/flow/login/extension_identifier_label.go @@ -6,6 +6,10 @@ package login import ( "context" + "github.com/pkg/errors" + "github.com/samber/lo" + + "github.com/ory/herodot" "github.com/ory/kratos/text" "github.com/ory/jsonschema/v3" @@ -13,25 +17,46 @@ import ( ) type identifierLabelExtension struct { + field string identifierLabelCandidates []string } -var _ schema.CompileExtension = new(identifierLabelExtension) +var ( + _ schema.CompileExtension = new(identifierLabelExtension) + ErrUnknownTrait = herodot.ErrBadRequest.WithReasonf("Trait does not exist in identity schema") +) func GetIdentifierLabelFromSchema(ctx context.Context, schemaURL string) (*text.Message, error) { - ext := &identifierLabelExtension{} + return GetIdentifierLabelFromSchemaWithField(ctx, schemaURL, "") +} + +func GetIdentifierLabelFromSchemaWithField(ctx context.Context, schemaURL string, trait string) (*text.Message, error) { + ext := &identifierLabelExtension{ + field: trait, + } runner, err := schema.NewExtensionRunner(ctx, schema.WithCompileRunners(ext)) if err != nil { return nil, err } c := jsonschema.NewCompiler() + c.ExtractAnnotations = true runner.Register(c) - _, err = c.Compile(ctx, schemaURL) + s, err := c.Compile(ctx, schemaURL) if err != nil { return nil, err } + + if trait != "" { + f, ok := s.Properties["traits"].Properties[trait] + if !ok { + knownTraits := lo.Keys(s.Properties["traits"].Properties) + return nil, errors.WithStack(ErrUnknownTrait.WithDetail("trait", trait).WithDetail("known_traits", knownTraits)) + } + return text.NewInfoNodeLabelGenerated(f.Title), nil + } + metaLabel := text.NewInfoNodeLabelID() if label := ext.getLabel(); label != "" { metaLabel = text.NewInfoNodeLabelGenerated(label) diff --git a/selfservice/flow/login/handler.go b/selfservice/flow/login/handler.go index 553aa469ccb3..ab7fe85245e2 100644 --- a/selfservice/flow/login/handler.go +++ b/selfservice/flow/login/handler.go @@ -293,6 +293,11 @@ type createNativeLoginFlow struct { // // in: query ReturnTo string `json:"return_to"` + + // Via should contain the identity's credential the code should be sent to. Only relevant in aal2 flows. + // + // in: query + Via string `json:"via"` } // swagger:route GET /self-service/login/api frontend createNativeLoginFlow diff --git a/selfservice/strategy/code/.snapshots/TestLoginCodeStrategy-test=Browser_client-suite=mfa-case=verify_initial_payload.json b/selfservice/strategy/code/.snapshots/TestLoginCodeStrategy-test=Browser_client-suite=mfa-case=verify_initial_payload.json new file mode 100644 index 000000000000..300c9f3ad955 --- /dev/null +++ b/selfservice/strategy/code/.snapshots/TestLoginCodeStrategy-test=Browser_client-suite=mfa-case=verify_initial_payload.json @@ -0,0 +1,84 @@ +{ + "organization_id": null, + "refresh": false, + "requested_aal": "aal2", + "state": "choose_method", + "type": "browser", + "ui": { + "messages": [ + { + "id": 1010004, + "text": "Please complete the second authentication challenge.", + "type": "info" + } + ], + "method": "POST", + "nodes": [ + { + "attributes": { + "disabled": false, + "name": "csrf_token", + "node_type": "input", + "required": true, + "type": "hidden" + }, + "group": "default", + "messages": [], + "meta": {}, + "type": "input" + }, + { + "attributes": { + "disabled": false, + "name": "identifier", + "node_type": "input", + "required": true, + "type": "text", + "value": "" + }, + "group": "default", + "messages": [ + { + "context": { + "maskedTo": "fi****@ory.sh" + }, + "id": 1010020, + "text": "We will send a code to fi****@ory.sh. To verify that this is your address please enter it here.", + "type": "info" + } + ], + "meta": { + "label": { + "context": { + "title": "Email" + }, + "id": 1070002, + "text": "Email", + "type": "info" + } + }, + "type": "input" + }, + { + "attributes": { + "disabled": false, + "name": "method", + "node_type": "input", + "type": "submit", + "value": "code" + }, + "group": "code", + "messages": [], + "meta": { + "label": { + "id": 1010019, + "text": "Continue with code", + "type": "info" + } + }, + "type": "input" + } + ] + } +} + diff --git a/selfservice/strategy/code/.snapshots/TestLoginCodeStrategy-test=Native_client-suite=mfa-case=verify_initial_payload.json b/selfservice/strategy/code/.snapshots/TestLoginCodeStrategy-test=Native_client-suite=mfa-case=verify_initial_payload.json new file mode 100644 index 000000000000..458af9498d62 --- /dev/null +++ b/selfservice/strategy/code/.snapshots/TestLoginCodeStrategy-test=Native_client-suite=mfa-case=verify_initial_payload.json @@ -0,0 +1,70 @@ +{ + "organization_id": null, + "refresh": false, + "requested_aal": "aal2", + "state": "choose_method", + "type": "api", + "ui": { + "messages": [ + { + "id": 1010004, + "text": "Please complete the second authentication challenge.", + "type": "info" + } + ], + "method": "POST", + "nodes": [ + { + "attributes": { + "disabled": false, + "name": "identifier", + "node_type": "input", + "required": true, + "type": "text" + }, + "group": "default", + "messages": [ + { + "context": { + "maskedTo": "fi****@ory.sh" + }, + "id": 1010020, + "text": "We will send a code to fi****@ory.sh. To verify that this is your address please enter it here.", + "type": "info" + } + ], + "meta": { + "label": { + "context": { + "title": "Email" + }, + "id": 1070002, + "text": "Email", + "type": "info" + } + }, + "type": "input" + }, + { + "attributes": { + "disabled": false, + "name": "method", + "node_type": "input", + "type": "submit", + "value": "code" + }, + "group": "code", + "messages": [], + "meta": { + "label": { + "id": 1010019, + "text": "Continue with code", + "type": "info" + } + }, + "type": "input" + } + ] + } +} + diff --git a/selfservice/strategy/code/.snapshots/TestLoginCodeStrategy-test=SPA_client-suite=mfa-case=verify_initial_payload.json b/selfservice/strategy/code/.snapshots/TestLoginCodeStrategy-test=SPA_client-suite=mfa-case=verify_initial_payload.json new file mode 100644 index 000000000000..300c9f3ad955 --- /dev/null +++ b/selfservice/strategy/code/.snapshots/TestLoginCodeStrategy-test=SPA_client-suite=mfa-case=verify_initial_payload.json @@ -0,0 +1,84 @@ +{ + "organization_id": null, + "refresh": false, + "requested_aal": "aal2", + "state": "choose_method", + "type": "browser", + "ui": { + "messages": [ + { + "id": 1010004, + "text": "Please complete the second authentication challenge.", + "type": "info" + } + ], + "method": "POST", + "nodes": [ + { + "attributes": { + "disabled": false, + "name": "csrf_token", + "node_type": "input", + "required": true, + "type": "hidden" + }, + "group": "default", + "messages": [], + "meta": {}, + "type": "input" + }, + { + "attributes": { + "disabled": false, + "name": "identifier", + "node_type": "input", + "required": true, + "type": "text", + "value": "" + }, + "group": "default", + "messages": [ + { + "context": { + "maskedTo": "fi****@ory.sh" + }, + "id": 1010020, + "text": "We will send a code to fi****@ory.sh. To verify that this is your address please enter it here.", + "type": "info" + } + ], + "meta": { + "label": { + "context": { + "title": "Email" + }, + "id": 1070002, + "text": "Email", + "type": "info" + } + }, + "type": "input" + }, + { + "attributes": { + "disabled": false, + "name": "method", + "node_type": "input", + "type": "submit", + "value": "code" + }, + "group": "code", + "messages": [], + "meta": { + "label": { + "id": 1010019, + "text": "Continue with code", + "type": "info" + } + }, + "type": "input" + } + ] + } +} + diff --git a/selfservice/strategy/code/strategy.go b/selfservice/strategy/code/strategy.go index ba834780b1b9..d33c5f34c388 100644 --- a/selfservice/strategy/code/strategy.go +++ b/selfservice/strategy/code/strategy.go @@ -9,6 +9,7 @@ import ( "strings" "github.com/pkg/errors" + "github.com/tidwall/gjson" "github.com/ory/herodot" "github.com/ory/kratos/continuity" @@ -162,7 +163,7 @@ func (s *Strategy) PopulateMethod(r *http.Request, f flow.Flow) error { switch f.GetState() { case flow.StateChooseMethod: - if err := s.populateChooseMethodFlow(r.Context(), f); err != nil { + if err := s.populateChooseMethodFlow(r, f); err != nil { return err } case flow.StateEmailSent: @@ -182,7 +183,8 @@ func (s *Strategy) PopulateMethod(r *http.Request, f flow.Flow) error { return nil } -func (s *Strategy) populateChooseMethodFlow(ctx context.Context, f flow.Flow) error { +func (s *Strategy) populateChooseMethodFlow(r *http.Request, f flow.Flow) error { + ctx := r.Context() var codeMetaLabel *text.Message switch f := f.(type) { case *recovery.Flow, *verification.Flow: @@ -192,14 +194,37 @@ func (s *Strategy) populateChooseMethodFlow(ctx context.Context, f flow.Flow) er ) codeMetaLabel = text.NewInfoNodeLabelSubmit() case *login.Flow: - codeMetaLabel = text.NewInfoSelfServiceLoginCode() ds, err := s.deps.Config().DefaultIdentityTraitsSchemaURL(ctx) if err != nil { return err } if f.RequestedAAL == identity.AuthenticatorAssuranceLevel2 { - f.GetUI().Nodes.Upsert(node.NewInputField("identifier", "", node.DefaultGroup, node.InputAttributeTypeText, node.WithRequiredInputAttribute).WithMetaLabel(text.NewInfoNodeLabelID())) + via := r.URL.Query().Get("via") + if via == "" { + return errors.WithStack(herodot.ErrBadRequest.WithReason("AAL2 login via code requires the `via` query parameter")) + } + + sess, err := s.deps.SessionManager().FetchFromRequest(r.Context(), r) + if err != nil { + return err + } + + identifierLabel, err := login.GetIdentifierLabelFromSchemaWithField(ctx, sess.Identity.SchemaURL, via) + if err != nil { + return err + } + + value := gjson.GetBytes(sess.Identity.Traits, via).String() + if value == "" { + return errors.WithStack(herodot.ErrBadRequest.WithReasonf("No value found for trait %s in the current identity", via)) + } + + codeMetaLabel = text.NewInfoSelfServiceLoginCodeMFA() + idNode := node.NewInputField("identifier", "", node.DefaultGroup, node.InputAttributeTypeText, node.WithRequiredInputAttribute).WithMetaLabel(identifierLabel) + idNode.Messages.Add(text.NewInfoSelfServiceLoginCodeMFAHint(maskAddress(value))) + f.GetUI().Nodes.Upsert(idNode) } else { + codeMetaLabel = text.NewInfoSelfServiceLoginCode() identifierLabel, err := login.GetIdentifierLabelFromSchema(ctx, ds.String()) if err != nil { return err @@ -383,3 +408,11 @@ const CodeLength = 6 func GenerateCode() string { return randx.MustString(CodeLength, randx.Numeric) } + +func maskAddress(input string) string { + if strings.Contains(input, "@") { + parts := strings.Split(input, "@") + return parts[0][:2] + strings.Repeat("*", 4) + "@" + parts[1] + } + return input[:3] + strings.Repeat("*", 4) + input[len(input)-3:] +} diff --git a/selfservice/strategy/code/strategy_login.go b/selfservice/strategy/code/strategy_login.go index 962866c5529b..5f7d04e395da 100644 --- a/selfservice/strategy/code/strategy_login.go +++ b/selfservice/strategy/code/strategy_login.go @@ -151,6 +151,18 @@ func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow, return nil, err } + var aal identity.AuthenticatorAssuranceLevel + + if s.deps.Config().SelfServiceCodeStrategy(ctx).PasswordlessEnabled { + aal = identity.AuthenticatorAssuranceLevel1 + } else if s.deps.Config().SelfServiceCodeStrategy(ctx).MFAEnabled { + aal = identity.AuthenticatorAssuranceLevel2 + } + + if err := login.CheckAAL(f, aal); err != nil { + return nil, err + } + var p updateLoginFlowWithCodeMethod if err := s.dx.Decode(r, &p, decoderx.HTTPDecoderSetValidatePayloads(true), diff --git a/selfservice/strategy/code/strategy_login_test.go b/selfservice/strategy/code/strategy_login_test.go index 26348761e01d..06d811362978 100644 --- a/selfservice/strategy/code/strategy_login_test.go +++ b/selfservice/strategy/code/strategy_login_test.go @@ -13,6 +13,7 @@ import ( "testing" "github.com/ory/x/ioutilx" + "github.com/ory/x/snapshotx" "github.com/ory/x/sqlcon" "github.com/ory/x/stringsx" @@ -575,90 +576,167 @@ func TestLoginCodeStrategy(t *testing.T) { require.True(t, va.Verified) }) - t.Run("case=should be able to get AAL2 session", func(t *testing.T) { + t.Run("suite=mfa", func(t *testing.T) { ctx := context.Background() + conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".code.passwordless_enabled", false) conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".code.mfa_enabled", true) t.Cleanup(func() { + conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".code.passwordless_enabled", true) conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".code.mfa_enabled", false) }) - identity := createIdentity(ctx, t, false) - var cl *http.Client - var f *oryClient.LoginFlow - if tc.apiType == ApiTypeNative { - cl = testhelpers.NewHTTPClientWithIdentitySessionToken(t, reg, identity) - f = testhelpers.InitializeLoginFlowViaAPI(t, cl, public, false, testhelpers.InitFlowWithAAL("aal2")) - } else { - cl = testhelpers.NewHTTPClientWithIdentitySessionCookieLocalhost(t, reg, identity) - f = testhelpers.InitializeLoginFlowViaBrowser(t, cl, public, false, tc.apiType == ApiTypeSPA, false, false, testhelpers.InitFlowWithAAL("aal2")) - } + t.Run("case=should be able to get AAL2 session", func(t *testing.T) { + identity := createIdentity(ctx, t, false) + var cl *http.Client + var f *oryClient.LoginFlow + if tc.apiType == ApiTypeNative { + cl = testhelpers.NewHTTPClientWithIdentitySessionToken(t, reg, identity) + f = testhelpers.InitializeLoginFlowViaAPI(t, cl, public, false, testhelpers.InitFlowWithAAL("aal2"), testhelpers.InitFlowWithVia("email")) + } else { + cl = testhelpers.NewHTTPClientWithIdentitySessionCookieLocalhost(t, reg, identity) + f = testhelpers.InitializeLoginFlowViaBrowser(t, cl, public, false, tc.apiType == ApiTypeSPA, false, false, testhelpers.InitFlowWithAAL("aal2"), testhelpers.InitFlowWithVia("email")) + } - body, err := json.Marshal(f) - require.NoError(t, err) - require.Len(t, gjson.GetBytes(body, "ui.nodes.#(group==code)").Array(), 1) - require.Len(t, gjson.GetBytes(body, "ui.messages").Array(), 1, "%s", body) - require.EqualValues(t, gjson.GetBytes(body, "ui.messages.0.id").Int(), text.InfoSelfServiceLoginMFA, "%s", body) - - s := &state{ - flowID: f.GetId(), - identity: identity, - client: cl, - testServer: public, - identityEmail: gjson.Get(identity.Traits.String(), "email").String(), - } - s = submitLogin(ctx, t, s, tc.apiType, func(v *url.Values) { - v.Set("identifier", s.identityEmail) - }, true, nil) + body, err := json.Marshal(f) + require.NoError(t, err) + require.Len(t, gjson.GetBytes(body, "ui.nodes.#(group==code)").Array(), 1) + require.Len(t, gjson.GetBytes(body, "ui.messages").Array(), 1, "%s", body) + require.EqualValues(t, gjson.GetBytes(body, "ui.messages.0.id").Int(), text.InfoSelfServiceLoginMFA, "%s", body) + + s := &state{ + flowID: f.GetId(), + identity: identity, + client: cl, + testServer: public, + identityEmail: gjson.Get(identity.Traits.String(), "email").String(), + } + s = submitLogin(ctx, t, s, tc.apiType, func(v *url.Values) { + v.Set("identifier", s.identityEmail) + }, true, nil) - message := testhelpers.CourierExpectMessage(ctx, t, reg, s.identityEmail, "Login to your account") - assert.Contains(t, message.Body, "please login to your account by entering the following code") - loginCode := testhelpers.CourierExpectCodeInMessage(t, message, 1) - assert.NotEmpty(t, loginCode) + message := testhelpers.CourierExpectMessage(ctx, t, reg, s.identityEmail, "Login to your account") + assert.Contains(t, message.Body, "please login to your account by entering the following code") + loginCode := testhelpers.CourierExpectCodeInMessage(t, message, 1) + assert.NotEmpty(t, loginCode) - s = submitLogin(ctx, t, s, tc.apiType, func(v *url.Values) { - v.Set("code", loginCode) - }, true, nil) + s = submitLogin(ctx, t, s, tc.apiType, func(v *url.Values) { + v.Set("code", loginCode) + }, true, nil) - testhelpers.EnsureAAL(t, cl, public, "aal2", "code") - }) + testhelpers.EnsureAAL(t, cl, public, "aal2", "code") + }) + t.Run("case=cannot use different identifier", func(t *testing.T) { + identity := createIdentity(ctx, t, false) + var cl *http.Client + var f *oryClient.LoginFlow + if tc.apiType == ApiTypeNative { + cl = testhelpers.NewHTTPClientWithIdentitySessionToken(t, reg, identity) + f = testhelpers.InitializeLoginFlowViaAPI(t, cl, public, false, testhelpers.InitFlowWithAAL("aal2"), testhelpers.InitFlowWithVia("email")) + } else { + cl = testhelpers.NewHTTPClientWithIdentitySessionCookieLocalhost(t, reg, identity) + f = testhelpers.InitializeLoginFlowViaBrowser(t, cl, public, false, tc.apiType == ApiTypeSPA, false, false, testhelpers.InitFlowWithAAL("aal2"), testhelpers.InitFlowWithVia("email")) + } - t.Run("case=cannot use different identifier", func(t *testing.T) { - ctx := context.Background() - conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".code.mfa_enabled", true) - t.Cleanup(func() { - conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+".code.mfa_enabled", false) + body, err := json.Marshal(f) + require.NoError(t, err) + require.Len(t, gjson.GetBytes(body, "ui.nodes.#(group==code)").Array(), 1) + require.Len(t, gjson.GetBytes(body, "ui.messages").Array(), 1, "%s", body) + require.EqualValues(t, gjson.GetBytes(body, "ui.messages.0.id").Int(), text.InfoSelfServiceLoginMFA, "%s", body) + + s := &state{ + flowID: f.GetId(), + identity: identity, + client: cl, + testServer: public, + identityEmail: gjson.Get(identity.Traits.String(), "email").String(), + } + email := testhelpers.RandomEmail() + s = submitLogin(ctx, t, s, tc.apiType, func(v *url.Values) { + v.Set("identifier", email) + }, true, nil) + + require.Equal(t, "The address you entered does not match any known addresses in the current account.", gjson.Get(s.body, "ui.messages.0.text").String(), "%s", body) }) - identity := createIdentity(ctx, t, false) - var cl *http.Client - var f *oryClient.LoginFlow - if tc.apiType == ApiTypeNative { - cl = testhelpers.NewHTTPClientWithIdentitySessionToken(t, reg, identity) - f = testhelpers.InitializeLoginFlowViaAPI(t, cl, public, false, testhelpers.InitFlowWithAAL("aal2")) - } else { - cl = testhelpers.NewHTTPClientWithIdentitySessionCookieLocalhost(t, reg, identity) - f = testhelpers.InitializeLoginFlowViaBrowser(t, cl, public, false, tc.apiType == ApiTypeSPA, false, false, testhelpers.InitFlowWithAAL("aal2")) - } + t.Run("case=verify initial payload", func(t *testing.T) { + fixedEmail := fmt.Sprintf("fixed_mfa_test_%s@ory.sh", tc.apiType) + identity := createIdentity(ctx, t, false, fixedEmail) + var cl *http.Client + var f *oryClient.LoginFlow + if tc.apiType == ApiTypeNative { + cl = testhelpers.NewHTTPClientWithIdentitySessionToken(t, reg, identity) + f = testhelpers.InitializeLoginFlowViaAPI(t, cl, public, false, testhelpers.InitFlowWithAAL("aal2"), testhelpers.InitFlowWithVia("email_1")) + } else { + cl = testhelpers.NewHTTPClientWithIdentitySessionCookieLocalhost(t, reg, identity) + f = testhelpers.InitializeLoginFlowViaBrowser(t, cl, public, false, tc.apiType == ApiTypeSPA, false, false, testhelpers.InitFlowWithAAL("aal2"), testhelpers.InitFlowWithVia("email_1")) + } - body, err := json.Marshal(f) - require.NoError(t, err) - require.Len(t, gjson.GetBytes(body, "ui.nodes.#(group==code)").Array(), 1) - require.Len(t, gjson.GetBytes(body, "ui.messages").Array(), 1, "%s", body) - require.EqualValues(t, gjson.GetBytes(body, "ui.messages.0.id").Int(), text.InfoSelfServiceLoginMFA, "%s", body) - - s := &state{ - flowID: f.GetId(), - identity: identity, - client: cl, - testServer: public, - identityEmail: gjson.Get(identity.Traits.String(), "email").String(), - } - email := testhelpers.RandomEmail() - s = submitLogin(ctx, t, s, tc.apiType, func(v *url.Values) { - v.Set("identifier", email) - }, true, nil) + body, err := json.Marshal(f) + require.NoError(t, err) + snapshotx.SnapshotTJSON(t, body, snapshotx.ExceptPaths("ui.nodes.0.attributes.value", "id", "created_at", "expires_at", "updated_at", "issued_at", "request_url", "ui.action")) + }) + + t.Run("case=using a non existing identity trait results in an error", func(t *testing.T) { + identity := createIdentity(ctx, t, false) + var cl *http.Client + var res *http.Response + var err error + if tc.apiType == ApiTypeNative { + cl = testhelpers.NewHTTPClientWithIdentitySessionToken(t, reg, identity) + res, err = cl.Get(public.URL + "/self-service/login/api?aal=aal2&via=doesnt_exist") + } else { + cl = testhelpers.NewHTTPClientWithIdentitySessionCookieLocalhost(t, reg, identity) + res, err = cl.Get(public.URL + "/self-service/login/browser?aal=aal2&via=doesnt_exist") + } + require.NoError(t, err) - require.Equal(t, "The address you entered does not match any known addresses in the current account.", gjson.Get(s.body, "ui.messages.0.text").String(), "%s", body) + body := ioutilx.MustReadAll(res.Body) + if tc.apiType == ApiTypeNative { + body = []byte(gjson.GetBytes(body, "error").Raw) + } + require.Equal(t, "Trait does not exist in identity schema", gjson.GetBytes(body, "reason").String(), "%s", body) + }) + + t.Run("case=missing via parameter results results in an error", func(t *testing.T) { + identity := createIdentity(ctx, t, false) + var cl *http.Client + var res *http.Response + var err error + if tc.apiType == ApiTypeNative { + cl = testhelpers.NewHTTPClientWithIdentitySessionToken(t, reg, identity) + res, err = cl.Get(public.URL + "/self-service/login/api?aal=aal2") + } else { + cl = testhelpers.NewHTTPClientWithIdentitySessionCookieLocalhost(t, reg, identity) + res, err = cl.Get(public.URL + "/self-service/login/browser?aal=aal2") + } + require.NoError(t, err) + + body := ioutilx.MustReadAll(res.Body) + if tc.apiType == ApiTypeNative { + body = []byte(gjson.GetBytes(body, "error").Raw) + } + require.Equal(t, "AAL2 login via code requires the `via` query parameter", gjson.GetBytes(body, "reason").String(), "%s", body) + }) + t.Run("case=unset trait in identity should lead to an error", func(t *testing.T) { + identity := createIdentity(ctx, t, false) + var cl *http.Client + var res *http.Response + var err error + if tc.apiType == ApiTypeNative { + cl = testhelpers.NewHTTPClientWithIdentitySessionToken(t, reg, identity) + res, err = cl.Get(public.URL + "/self-service/login/api?aal=aal2&via=email_1") + } else { + cl = testhelpers.NewHTTPClientWithIdentitySessionCookieLocalhost(t, reg, identity) + res, err = cl.Get(public.URL + "/self-service/login/browser?aal=aal2&via=email_1") + } + require.NoError(t, err) + + body := ioutilx.MustReadAll(res.Body) + if tc.apiType == ApiTypeNative { + body = []byte(gjson.GetBytes(body, "error").Raw) + } + require.Equal(t, "No value found for trait email_1 in the current identity", gjson.GetBytes(body, "reason").String(), "%s", body) + }) }) }) } diff --git a/spec/api.json b/spec/api.json index 8ba46ef3790d..0b2c06bb6df4 100644 --- a/spec/api.json +++ b/spec/api.json @@ -81,9 +81,6 @@ } }, "schemas": { - "CodeAddressType": { - "type": "string" - }, "DefaultError": {}, "Duration": { "description": "A Duration represents the elapsed time between two instants\nas an int64 nanosecond count. The representation limits the\nlargest representable duration to approximately 290 years.", @@ -1010,7 +1007,8 @@ "description": "CredentialsCode represents a one time login/registration code", "properties": { "address_type": { - "$ref": "#/components/schemas/CodeAddressType" + "description": "The type of the address for this code", + "type": "string" }, "used_at": { "$ref": "#/components/schemas/NullTime" @@ -5169,6 +5167,14 @@ "schema": { "type": "string" } + }, + { + "description": "Via should contain the identity's credential the code should be sent to. Only relevant in aal2 flows.", + "in": "query", + "name": "via", + "schema": { + "type": "string" + } } ], "responses": { diff --git a/spec/swagger.json b/spec/swagger.json index 0efbf7716e9c..7072a049cc76 100755 --- a/spec/swagger.json +++ b/spec/swagger.json @@ -1558,6 +1558,12 @@ "description": "The URL to return the browser to after the flow was completed.", "name": "return_to", "in": "query" + }, + { + "type": "string", + "description": "Via should contain the identity's credential the code should be sent to. Only relevant in aal2 flows.", + "name": "via", + "in": "query" } ], "responses": { @@ -3190,9 +3196,6 @@ } }, "definitions": { - "CodeAddressType": { - "type": "string" - }, "DefaultError": {}, "Duration": { "description": "A Duration represents the elapsed time between two instants\nas an int64 nanosecond count. The representation limits the\nlargest representable duration to approximately 290 years.", @@ -4085,7 +4088,8 @@ "type": "object", "properties": { "address_type": { - "$ref": "#/definitions/CodeAddressType" + "description": "The type of the address for this code", + "type": "string" }, "used_at": { "$ref": "#/definitions/NullTime" diff --git a/test/e2e/cypress/integration/profiles/mfa/code.spec.ts b/test/e2e/cypress/integration/profiles/mfa/code.spec.ts index 74019d018596..7961aa850de9 100644 --- a/test/e2e/cypress/integration/profiles/mfa/code.spec.ts +++ b/test/e2e/cypress/integration/profiles/mfa/code.spec.ts @@ -7,13 +7,13 @@ import { routes as react } from "../../../helpers/react" context("2FA code", () => { ;[ - { - login: react.login, - settings: react.settings, - base: react.base, - app: "react" as "react", - profile: "spa", - }, + // { + // login: react.login, + // settings: react.settings, + // base: react.base, + // app: "react" as "react", + // profile: "spa", + // }, { login: express.login, settings: express.settings, @@ -48,12 +48,12 @@ context("2FA code", () => { fields: { "traits.website": website }, }) cy.deleteMail() - cy.visit(login + "?aal=aal2") + cy.visit(login + "?aal=aal2&via=email") }) it("should be asked to sign in with 2fa if set up", () => { cy.get("input[name='identifier']").type(email) - cy.contains("Sign in with code").click() + cy.contains("Continue with code").click() cy.get("input[name='code']").should("be.visible") cy.getLoginCodeFromEmail(email).then((code) => { @@ -69,7 +69,7 @@ context("2FA code", () => { it("can't use different email in 2fa request", () => { cy.get("input[name='identifier']").type(gen.email()) - cy.contains("Sign in with code").click() + cy.contains("Continue with code").click() cy.get("*[data-testid='ui/message/4010010']").should("be.visible") cy.get("input[name='code']").should("not.exist") @@ -84,7 +84,7 @@ context("2FA code", () => { it("entering wrong code should not invalidate correct codes", () => { cy.get("input[name='identifier']").type(email) - cy.contains("Sign in with code").click() + cy.contains("Continue with code").click() cy.get("input[name='code']").should("be.visible") cy.get("input[name='code']").type("123456") diff --git a/test/e2e/run.sh b/test/e2e/run.sh index 863a70bb910b..e2d0c0d261a4 100755 --- a/test/e2e/run.sh +++ b/test/e2e/run.sh @@ -83,7 +83,7 @@ prepare() { if [ -z ${NODE_UI_PATH+x} ]; then node_ui_dir="$(mktemp -d -t ci-XXXXXXXXXX)/kratos-selfservice-ui-node" - git clone --depth 1 --branch master https://github.com/ory/kratos-selfservice-ui-node.git "$node_ui_dir" + git clone --depth 1 --branch jonas-jonas/addViaLoginParameter https://github.com/ory/kratos-selfservice-ui-node.git "$node_ui_dir" (cd "$node_ui_dir" && npm i --legacy-peer-deps && npm run build) else node_ui_dir="${NODE_UI_PATH}" diff --git a/text/id.go b/text/id.go index a4c57761e53a..562a179740ef 100644 --- a/text/id.go +++ b/text/id.go @@ -28,6 +28,8 @@ const ( InfoSelfServiceLoginLink // 1010016 InfoSelfServiceLoginAndLink // 1010017 InfoSelfServiceLoginWithAndLink // 1010018 + InfoSelfServiceLoginCodeMFA // 1010019 + InfoSelfServiceLoginCodeMFAHint // 1010020 ) const ( diff --git a/text/message_login.go b/text/message_login.go index ab5be6d71806..85c7219fb8f7 100644 --- a/text/message_login.go +++ b/text/message_login.go @@ -250,3 +250,22 @@ func NewErrorValidationAddressUnknown() *Message { Type: Error, } } + +func NewInfoSelfServiceLoginCodeMFA() *Message { + return &Message{ + ID: InfoSelfServiceLoginCodeMFA, + Type: Info, + Text: "Continue with code", + } +} + +func NewInfoSelfServiceLoginCodeMFAHint(maskedTo string) *Message { + return &Message{ + ID: InfoSelfServiceLoginCodeMFAHint, + Type: Info, + Text: fmt.Sprintf("We will send a code to %s. To verify that this is your address please enter it here.", maskedTo), + Context: context(map[string]any{ + "maskedTo": maskedTo, + }), + } +} From 1f8fefb76ea5ffadabe927093788d10c9c14650b Mon Sep 17 00:00:00 2001 From: Jonas Hungershausen Date: Wed, 24 Jan 2024 16:48:38 +0100 Subject: [PATCH 12/19] chore: fix ci --- .github/workflows/ci.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 43129b0991ce..113b9ded7196 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -184,6 +184,7 @@ jobs: uses: actions/checkout@v3 with: repository: ory/kratos-selfservice-ui-node + ref: jonas-jonas/addViaLoginParameter path: node-ui - run: | cd node-ui From 9cc90063aeac9ca2e13e973bcb732fc82514b3a4 Mon Sep 17 00:00:00 2001 From: Jonas Hungershausen Date: Thu, 25 Jan 2024 10:25:22 +0100 Subject: [PATCH 13/19] chore: cr --- internal/testhelpers/selfservice_login.go | 1 + .../flow/login/extension_identifier_label.go | 2 +- selfservice/strategy/code/strategy.go | 27 +++++++++++++--- selfservice/strategy/code/strategy_test.go | 32 +++++++++++++++++++ 4 files changed, 56 insertions(+), 6 deletions(-) diff --git a/internal/testhelpers/selfservice_login.go b/internal/testhelpers/selfservice_login.go index 6469aec52ac3..2bd20f81dfd4 100644 --- a/internal/testhelpers/selfservice_login.go +++ b/internal/testhelpers/selfservice_login.go @@ -122,6 +122,7 @@ func InitFlowWithOAuth2LoginChallenge(hlc string) InitFlowWithOption { } } +// InitFlowWithVia sets the `via` query parameter which is used by the code MFA flows to determine the trait to use to send the code to the user func InitFlowWithVia(via string) InitFlowWithOption { return func(o *initFlowOptions) { o.via = via diff --git a/selfservice/flow/login/extension_identifier_label.go b/selfservice/flow/login/extension_identifier_label.go index 1774f62a9c3c..9d28dd8ecf9f 100644 --- a/selfservice/flow/login/extension_identifier_label.go +++ b/selfservice/flow/login/extension_identifier_label.go @@ -23,7 +23,7 @@ type identifierLabelExtension struct { var ( _ schema.CompileExtension = new(identifierLabelExtension) - ErrUnknownTrait = herodot.ErrBadRequest.WithReasonf("Trait does not exist in identity schema") + ErrUnknownTrait = herodot.ErrInternalServerError.WithReasonf("Trait does not exist in identity schema") ) func GetIdentifierLabelFromSchema(ctx context.Context, schemaURL string) (*text.Message, error) { diff --git a/selfservice/strategy/code/strategy.go b/selfservice/strategy/code/strategy.go index d33c5f34c388..61bca5915638 100644 --- a/selfservice/strategy/code/strategy.go +++ b/selfservice/strategy/code/strategy.go @@ -221,7 +221,7 @@ func (s *Strategy) populateChooseMethodFlow(r *http.Request, f flow.Flow) error codeMetaLabel = text.NewInfoSelfServiceLoginCodeMFA() idNode := node.NewInputField("identifier", "", node.DefaultGroup, node.InputAttributeTypeText, node.WithRequiredInputAttribute).WithMetaLabel(identifierLabel) - idNode.Messages.Add(text.NewInfoSelfServiceLoginCodeMFAHint(maskAddress(value))) + idNode.Messages.Add(text.NewInfoSelfServiceLoginCodeMFAHint(MaskAddress(value))) f.GetUI().Nodes.Upsert(idNode) } else { codeMetaLabel = text.NewInfoSelfServiceLoginCode() @@ -409,10 +409,27 @@ func GenerateCode() string { return randx.MustString(CodeLength, randx.Numeric) } -func maskAddress(input string) string { +// MaskAddress masks an address by replacing the middle part with asterisks. +// +// If the address contains an @, the part before the @ is masked by taking the first 2 characters and adding 4 * +// (if the part before the @ is less than 2 characters the full value is used). +// Otherwise, the first 3 characters and last two characters are taken and 4 * are added in between. +// +// Examples: +// - foo@bar -> fo****@bar +// - foobar -> fo****ar +// - fo@bar -> fo@bar +// - +12345678910 -> +12****10 +func MaskAddress(input string) string { if strings.Contains(input, "@") { - parts := strings.Split(input, "@") - return parts[0][:2] + strings.Repeat("*", 4) + "@" + parts[1] + pre, post, found := strings.Cut(input, "@") + if !found || len(pre) < 2 { + return input + } + return pre[:2] + strings.Repeat("*", 4) + "@" + post + } + if len(input) < 6 { + return input } - return input[:3] + strings.Repeat("*", 4) + input[len(input)-3:] + return input[:3] + strings.Repeat("*", 4) + input[len(input)-2:] } diff --git a/selfservice/strategy/code/strategy_test.go b/selfservice/strategy/code/strategy_test.go index 634605cba3f3..5a6234531d9d 100644 --- a/selfservice/strategy/code/strategy_test.go +++ b/selfservice/strategy/code/strategy_test.go @@ -38,3 +38,35 @@ func TestGenerateCode(t *testing.T) { assert.Len(t, stringslice.Unique(codes), len(codes)) } + +func TestMaskAddress(t *testing.T) { + for _, tc := range []struct { + address string + expected string + }{ + { + address: "a", + expected: "a", + }, + { + address: "fixed@ory.sh", + expected: "fi****@ory.sh", + }, + { + address: "f@ory.sh", + expected: "f@ory.sh", + }, + { + address: "+12345678910", + expected: "+12****10", + }, + { + address: "+123456", + expected: "+12****56", + }, + } { + t.Run("case="+tc.address, func(t *testing.T) { + assert.Equal(t, tc.expected, code.MaskAddress(tc.address)) + }) + } +} From 2b88f3a6d676f3f8d329464699316eb0c6a218a9 Mon Sep 17 00:00:00 2001 From: Jonas Hungershausen Date: Thu, 25 Jan 2024 10:26:20 +0100 Subject: [PATCH 14/19] chore: use master ui node example --- .github/workflows/ci.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 113b9ded7196..43129b0991ce 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -184,7 +184,6 @@ jobs: uses: actions/checkout@v3 with: repository: ory/kratos-selfservice-ui-node - ref: jonas-jonas/addViaLoginParameter path: node-ui - run: | cd node-ui From 944eada1b6c6a9d03d625e7d25aa18e3a0b9334a Mon Sep 17 00:00:00 2001 From: Jonas Hungershausen Date: Thu, 25 Jan 2024 13:22:37 +0100 Subject: [PATCH 15/19] chore: adjust config schema --- embedx/config.schema.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/embedx/config.schema.json b/embedx/config.schema.json index e13623c343f6..e223b4cc1c79 100644 --- a/embedx/config.schema.json +++ b/embedx/config.schema.json @@ -1410,7 +1410,7 @@ "code": { "type": "object", "additionalProperties": false, - "oneOf": [ + "anyOf": [ { "properties": { "passwordless_enabled": { "const": true }, From b421df22556ff105e9e5d545702694713d0d8319 Mon Sep 17 00:00:00 2001 From: zepatrik Date: Thu, 25 Jan 2024 15:36:47 +0100 Subject: [PATCH 16/19] fix: use schema URL directly --- selfservice/strategy/code/strategy.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/selfservice/strategy/code/strategy.go b/selfservice/strategy/code/strategy.go index 61bca5915638..c4a094d16471 100644 --- a/selfservice/strategy/code/strategy.go +++ b/selfservice/strategy/code/strategy.go @@ -209,7 +209,15 @@ func (s *Strategy) populateChooseMethodFlow(r *http.Request, f flow.Flow) error return err } - identifierLabel, err := login.GetIdentifierLabelFromSchemaWithField(ctx, sess.Identity.SchemaURL, via) + allSchemas, err := s.deps.IdentityTraitsSchemas(ctx) + if err != nil { + return err + } + iSchema, err := allSchemas.GetByID(sess.Identity.SchemaID) + if err != nil { + return err + } + identifierLabel, err := login.GetIdentifierLabelFromSchemaWithField(ctx, iSchema.RawURL, via) if err != nil { return err } From 0942043063cac6730e498238bcea7f6a017510c4 Mon Sep 17 00:00:00 2001 From: zepatrik Date: Fri, 26 Jan 2024 12:22:18 +0100 Subject: [PATCH 17/19] chore: minor details --- selfservice/strategy/code/strategy.go | 3 ++- selfservice/strategy/code/strategy_test.go | 4 ++++ test/e2e/run.sh | 2 +- text/message_login.go | 2 +- 4 files changed, 8 insertions(+), 3 deletions(-) diff --git a/selfservice/strategy/code/strategy.go b/selfservice/strategy/code/strategy.go index c4a094d16471..a4b52e980754 100644 --- a/selfservice/strategy/code/strategy.go +++ b/selfservice/strategy/code/strategy.go @@ -426,7 +426,8 @@ func GenerateCode() string { // Examples: // - foo@bar -> fo****@bar // - foobar -> fo****ar -// - fo@bar -> fo@bar +// - f@bar -> f@bar +// - fo@bar -> fo****@bar // - +12345678910 -> +12****10 func MaskAddress(input string) string { if strings.Contains(input, "@") { diff --git a/selfservice/strategy/code/strategy_test.go b/selfservice/strategy/code/strategy_test.go index 5a6234531d9d..4561a280fb96 100644 --- a/selfservice/strategy/code/strategy_test.go +++ b/selfservice/strategy/code/strategy_test.go @@ -48,6 +48,10 @@ func TestMaskAddress(t *testing.T) { address: "a", expected: "a", }, + { + address: "ab@cd", + expected: "ab****@cd", + }, { address: "fixed@ory.sh", expected: "fi****@ory.sh", diff --git a/test/e2e/run.sh b/test/e2e/run.sh index e2d0c0d261a4..863a70bb910b 100755 --- a/test/e2e/run.sh +++ b/test/e2e/run.sh @@ -83,7 +83,7 @@ prepare() { if [ -z ${NODE_UI_PATH+x} ]; then node_ui_dir="$(mktemp -d -t ci-XXXXXXXXXX)/kratos-selfservice-ui-node" - git clone --depth 1 --branch jonas-jonas/addViaLoginParameter https://github.com/ory/kratos-selfservice-ui-node.git "$node_ui_dir" + git clone --depth 1 --branch master https://github.com/ory/kratos-selfservice-ui-node.git "$node_ui_dir" (cd "$node_ui_dir" && npm i --legacy-peer-deps && npm run build) else node_ui_dir="${NODE_UI_PATH}" diff --git a/text/message_login.go b/text/message_login.go index 85c7219fb8f7..6432cf66d7b7 100644 --- a/text/message_login.go +++ b/text/message_login.go @@ -265,7 +265,7 @@ func NewInfoSelfServiceLoginCodeMFAHint(maskedTo string) *Message { Type: Info, Text: fmt.Sprintf("We will send a code to %s. To verify that this is your address please enter it here.", maskedTo), Context: context(map[string]any{ - "maskedTo": maskedTo, + "masked_address": maskedTo, }), } } From 276ff67225b93dd039fe267645c69e6544aec931 Mon Sep 17 00:00:00 2001 From: zepatrik Date: Fri, 26 Jan 2024 12:23:12 +0100 Subject: [PATCH 18/19] chore: minor details --- text/message_login.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/message_login.go b/text/message_login.go index 6432cf66d7b7..c94db4538704 100644 --- a/text/message_login.go +++ b/text/message_login.go @@ -265,7 +265,7 @@ func NewInfoSelfServiceLoginCodeMFAHint(maskedTo string) *Message { Type: Info, Text: fmt.Sprintf("We will send a code to %s. To verify that this is your address please enter it here.", maskedTo), Context: context(map[string]any{ - "masked_address": maskedTo, + "masked_to": maskedTo, }), } } From 1e9e44eed70864f19446de331d91e6ec232eff97 Mon Sep 17 00:00:00 2001 From: Jonas Hungershausen Date: Fri, 26 Jan 2024 13:24:52 +0100 Subject: [PATCH 19/19] chore: snapshots --- ...st=Browser_client-suite=mfa-case=verify_initial_payload.json | 2 +- ...est=Native_client-suite=mfa-case=verify_initial_payload.json | 2 +- ...y-test=SPA_client-suite=mfa-case=verify_initial_payload.json | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/selfservice/strategy/code/.snapshots/TestLoginCodeStrategy-test=Browser_client-suite=mfa-case=verify_initial_payload.json b/selfservice/strategy/code/.snapshots/TestLoginCodeStrategy-test=Browser_client-suite=mfa-case=verify_initial_payload.json index 300c9f3ad955..14efb22f25a3 100644 --- a/selfservice/strategy/code/.snapshots/TestLoginCodeStrategy-test=Browser_client-suite=mfa-case=verify_initial_payload.json +++ b/selfservice/strategy/code/.snapshots/TestLoginCodeStrategy-test=Browser_client-suite=mfa-case=verify_initial_payload.json @@ -40,7 +40,7 @@ "messages": [ { "context": { - "maskedTo": "fi****@ory.sh" + "masked_to": "fi****@ory.sh" }, "id": 1010020, "text": "We will send a code to fi****@ory.sh. To verify that this is your address please enter it here.", diff --git a/selfservice/strategy/code/.snapshots/TestLoginCodeStrategy-test=Native_client-suite=mfa-case=verify_initial_payload.json b/selfservice/strategy/code/.snapshots/TestLoginCodeStrategy-test=Native_client-suite=mfa-case=verify_initial_payload.json index 458af9498d62..49f7d3a37cc1 100644 --- a/selfservice/strategy/code/.snapshots/TestLoginCodeStrategy-test=Native_client-suite=mfa-case=verify_initial_payload.json +++ b/selfservice/strategy/code/.snapshots/TestLoginCodeStrategy-test=Native_client-suite=mfa-case=verify_initial_payload.json @@ -26,7 +26,7 @@ "messages": [ { "context": { - "maskedTo": "fi****@ory.sh" + "masked_to": "fi****@ory.sh" }, "id": 1010020, "text": "We will send a code to fi****@ory.sh. To verify that this is your address please enter it here.", diff --git a/selfservice/strategy/code/.snapshots/TestLoginCodeStrategy-test=SPA_client-suite=mfa-case=verify_initial_payload.json b/selfservice/strategy/code/.snapshots/TestLoginCodeStrategy-test=SPA_client-suite=mfa-case=verify_initial_payload.json index 300c9f3ad955..14efb22f25a3 100644 --- a/selfservice/strategy/code/.snapshots/TestLoginCodeStrategy-test=SPA_client-suite=mfa-case=verify_initial_payload.json +++ b/selfservice/strategy/code/.snapshots/TestLoginCodeStrategy-test=SPA_client-suite=mfa-case=verify_initial_payload.json @@ -40,7 +40,7 @@ "messages": [ { "context": { - "maskedTo": "fi****@ory.sh" + "masked_to": "fi****@ory.sh" }, "id": 1010020, "text": "We will send a code to fi****@ory.sh. To verify that this is your address please enter it here.",