From 1516cf64e346819dccace1cc25aaccac38b9e47c Mon Sep 17 00:00:00 2001 From: Jonas Hungershausen Date: Fri, 26 Jan 2024 17:01:11 +0100 Subject: [PATCH] feat: support MFA via SMS (#3682) --------- Co-authored-by: zepatrik --- Makefile | 8 +- cmd/clidoc/main.go | 7 +- .../phone-password/identity.schema.json | 24 +- courier/sms_templates.go | 6 + .../login_code/valid/sms.body.gotmpl | 1 + courier/template/sms/login_code_valid.go | 52 +++ courier/template/sms/login_code_valid_test.go | 37 ++ driver/config/config.go | 18 +- driver/registry_default.go | 14 +- driver/registry_default_test.go | 37 +- embedx/config.schema.json | 31 +- embedx/identity_extension.schema.json | 2 +- identity/credentials_code.go | 2 +- 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/registrationhelpers/helpers.go | 16 +- internal/testhelpers/courier.go | 2 +- internal/testhelpers/selfservice_login.go | 25 +- internal/testhelpers/server.go | 5 +- internal/testhelpers/session.go | 14 + schema/errors.go | 11 + .../flow/login/extension_identifier_label.go | 31 +- selfservice/flow/login/handler.go | 25 +- selfservice/flow/login/hook.go | 2 +- selfservice/flow/login/strategy.go | 5 +- selfservice/flow/registration/handler_test.go | 3 +- selfservice/flow/request.go | 15 +- selfservice/flow/request_test.go | 71 ---- ...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/code_sender.go | 28 +- selfservice/strategy/code/strategy.go | 347 +++++++++++------- selfservice/strategy/code/strategy_login.go | 68 +++- .../strategy/code/strategy_login_test.go | 191 ++++++++-- .../code/strategy_registration_test.go | 5 +- selfservice/strategy/code/strategy_test.go | 36 ++ selfservice/strategy/lookup/login.go | 9 +- selfservice/strategy/lookup/strategy.go | 8 +- selfservice/strategy/oidc/strategy.go | 2 +- selfservice/strategy/oidc/strategy_login.go | 4 +- selfservice/strategy/password/login.go | 3 +- selfservice/strategy/password/strategy.go | 2 +- selfservice/strategy/totp/login.go | 6 +- selfservice/strategy/totp/strategy.go | 10 +- selfservice/strategy/webauthn/login.go | 5 +- selfservice/strategy/webauthn/strategy.go | 10 +- .../strategy/webauthn/strategy_test.go | 4 +- spec/api.json | 14 +- spec/swagger.json | 12 +- .../code/registration/success.spec.ts | 20 +- .../integration/profiles/mfa/code.spec.ts | 104 ++++++ .../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 | 190 ++++++---- test/e2e/cypress/support/config.d.ts | 45 ++- test/e2e/cypress/support/configHelpers.ts | 9 + test/e2e/cypress/support/index.d.ts | 18 +- text/id.go | 3 + text/message_login.go | 28 +- 67 files changed, 1473 insertions(+), 509 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 create mode 100644 courier/template/sms/login_code_valid_test.go 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 create mode 100644 test/e2e/cypress/integration/profiles/mfa/code.spec.ts diff --git a/Makefile b/Makefile index cbee2a75fc93..1775d09b25ea 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) @@ -212,3 +212,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/cmd/clidoc/main.go b/cmd/clidoc/main.go index 6a9ff0610017..ef3027a936a3 100644 --- a/cmd/clidoc/main.go +++ b/cmd/clidoc/main.go @@ -168,6 +168,9 @@ func init() { "NewErrorValidationRegistrationRetrySuccessful": text.NewErrorValidationRegistrationRetrySuccessful(), "NewInfoSelfServiceRegistrationRegisterCode": text.NewInfoSelfServiceRegistrationRegisterCode(), "NewErrorValidationLoginLinkedCredentialsDoNotMatch": text.NewErrorValidationLoginLinkedCredentialsDoNotMatch(), + "NewErrorValidationAddressUnknown": text.NewErrorValidationAddressUnknown(), + "NewInfoSelfServiceLoginCodeMFA": text.NewInfoSelfServiceLoginCodeMFA(), + "NewInfoSelfServiceLoginCodeMFAHint": text.NewInfoSelfServiceLoginCodeMFAHint("{maskedIdentifier}"), } } @@ -247,7 +250,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 +269,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/contrib/quickstart/kratos/phone-password/identity.schema.json b/contrib/quickstart/kratos/phone-password/identity.schema.json index 0d986aeb672e..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", @@ -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", @@ -24,7 +44,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..194e113e6c6b --- /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", + "login_code/valid/sms.body*", + t.model, + t.deps.CourierConfig().CourierSMSTemplatesLoginCodeValid(ctx).Body.PlainText, + ) +} + +func (t *LoginCodeValid) MarshalJSON() ([]byte, error) { + return json.Marshal(t.model) +} + +func (t *LoginCodeValid) TemplateType() template.TemplateType { + return template.TypeLoginCodeValid +} 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 35d33ee8316a..6af7e7c17c5c 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" @@ -236,6 +237,7 @@ type ( SelfServiceStrategyCode struct { *SelfServiceStrategy PasswordlessEnabled bool `json:"passwordless_enabled"` + MFAEnabled bool `json:"mfa_enabled"` } Schema struct { ID string `json:"id" koanf:"id"` @@ -303,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 @@ -758,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, } } @@ -782,6 +793,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), } } @@ -1115,6 +1127,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) } diff --git a/driver/registry_default.go b/driver/registry_default.go index 5349c7359d39..890fb7d2f5eb 100644 --- a/driver/registry_default.go +++ b/driver/registry_default.go @@ -327,21 +327,11 @@ 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 { - 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..0dd43e2efc70 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,45 +683,64 @@ 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 }{ { + 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"}, }, + { + 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=%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..e223b4cc1c79 100644 --- a/embedx/config.schema.json +++ b/embedx/config.schema.json @@ -1410,10 +1410,36 @@ "code": { "type": "object", "additionalProperties": false, + "anyOf": [ + { + "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", - "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", "default": false }, "passwordless_login_fallback_enabled": { @@ -1793,6 +1819,9 @@ "properties": { "email": { "$ref": "#/definitions/emailCourierTemplate" + }, + "sms": { + "$ref": "#/definitions/smsCourierTemplate" } }, "required": ["email"] 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/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/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 bedba03fee16..2bd20f81dfd4 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,13 @@ 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 + } +} + 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) @@ -132,8 +143,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,13 +153,14 @@ func InitializeLoginFlowViaBrowser(t *testing.T, client *http.Client, ts *httpte if isSPA { flowID = gjson.GetBytes(body, "id").String() } + require.NotEmpty(t, flowID) - 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) } @@ -163,9 +175,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/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/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/flow/login/extension_identifier_label.go b/selfservice/flow/login/extension_identifier_label.go index 02b725f00b7f..9d28dd8ecf9f 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.ErrInternalServerError.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 096a657c0869..ab7fe85245e2 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 } @@ -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 @@ -781,7 +786,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 @@ -798,7 +803,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 620559f57b06..418e9237df5c 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..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,8 +21,8 @@ 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) - CompletedAuthenticationMethod(ctx context.Context) session.AuthenticationMethod + 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 } 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/.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..14efb22f25a3 --- /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": { + "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.", + "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..49f7d3a37cc1 --- /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": { + "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.", + "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..14efb22f25a3 --- /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": { + "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.", + "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/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..a4b52e980754 100644 --- a/selfservice/strategy/code/strategy.go +++ b/selfservice/strategy/code/strategy.go @@ -4,10 +4,12 @@ package code import ( + "context" "net/http" "strings" "github.com/pkg/errors" + "github.com/tidwall/gjson" "github.com/ory/herodot" "github.com/ory/kratos/continuity" @@ -132,190 +134,243 @@ func (s *Strategy) NodeGroup() node.UiNodeGroup { } func (s *Strategy) PopulateMethod(r *http.Request, f flow.Flow) error { + 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 + } + } + if string(f.GetState()) == "" { f.SetState(flow.StateChooseMethod) } f.GetUI().ResetMessages() - nodes := f.GetUI().Nodes - switch f.GetState() { case flow.StateChooseMethod: + if err := s.populateChooseMethodFlow(r, 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()) + // 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 +} + +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: + f.GetUI().Nodes.Append( + node.NewInputField("email", nil, node.CodeGroup, node.InputAttributeTypeEmail, node.WithRequiredInputAttribute). + WithMetaLabel(text.NewInfoNodeInputEmail()), + ) + codeMetaLabel = text.NewInfoNodeLabelSubmit() + case *login.Flow: + ds, err := s.deps.Config().DefaultIdentityTraitsSchemaURL(ctx) + if err != nil { + return err + } + if f.RequestedAAL == identity.AuthenticatorAssuranceLevel2 { + 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.GetIdentifierLabelFromSchema(r.Context(), ds.String()) + allSchemas, err := s.deps.IdentityTraitsSchemas(ctx) 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()) + iSchema, err := allSchemas.GetByID(sess.Identity.SchemaID) if err != nil { return err } - - // 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) + identifierLabel, err := login.GetIdentifierLabelFromSchemaWithField(ctx, iSchema.RawURL, via) if err != nil { return err } - for _, n := range traitNodes { - nodes.Upsert(n) + 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 } + + 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 + // 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 + } - switch f.GetFlowName() { - case flow.VerificationFlow, flow.RecoveryFlow: - codeMetaLabel = text.NewInfoNodeLabelSubmit() - case flow.LoginFlow: - codeMetaLabel = text.NewInfoSelfServiceLoginCode() - case flow.RegistrationFlow: - codeMetaLabel = text.NewInfoSelfServiceRegistrationRegisterCode() + for _, n := range traitNodes { + f.GetUI().Nodes.Upsert(n) } + } - methodButton := node.NewInputField("method", s.ID(), node.CodeGroup, node.InputAttributeTypeSubmit). - WithMetaLabel(codeMetaLabel) + methodButton := node.NewInputField("method", s.ID(), node.CodeGroup, node.InputAttributeTypeSubmit). + WithMetaLabel(codeMetaLabel) - nodes.Append(methodButton) + f.GetUI().Nodes.Append(methodButton) - f.GetUI().Nodes = nodes + return nil +} - 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 - } +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 + + 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 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()) + resendNode = node.NewInputField("resend", "code", node.CodeGroup, node.InputAttributeTypeSubmit). + WithMetaLabel(text.NewInfoNodeResendOTP()) - case flow.RegistrationFlow: - route = registration.RouteSubmitFlow - codeMetaLabel = text.NewInfoNodeLabelRegistrationCode() - message = text.NewRegistrationEmailWithCodeSent() + 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 - } + // 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 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 - } + 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)) - - // code submit button - freshNodes. - Append(node.NewInputField("method", s.ID(), node.CodeGroup, node.InputAttributeTypeSubmit). - WithMetaLabel(text.NewInfoNodeLabelSubmit())) + resendNode = node.NewInputField("resend", "code", node.CodeGroup, node.InputAttributeTypeSubmit). + WithMetaLabel(text.NewInfoNodeResendOTP()) + default: + return errors.WithStack(herodot.ErrBadRequest.WithReason("received an unexpected flow type")) + } - if resendNode != nil { - freshNodes.Append(resendNode) - } + // 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), + ) - f.GetUI().Nodes = freshNodes + // code input field + freshNodes.Upsert(node.NewInputField("code", nil, node.CodeGroup, node.InputAttributeTypeText, node.WithRequiredInputAttribute). + WithMetaLabel(codeMetaLabel)) - f.GetUI().Method = "POST" - f.GetUI().Action = flow.AppendFlowTo(urlx.AppendPaths(s.deps.Config().SelfPublicURL(r.Context()), route), f.GetID()).String() + // code submit button + freshNodes. + Append(node.NewInputField("method", s.ID(), node.CodeGroup, node.InputAttributeTypeSubmit). + WithMetaLabel(text.NewInfoNodeLabelSubmit())) - // Set the request's CSRF token - if f.GetType() == flow.TypeBrowser { - f.GetUI().SetCSRF(s.deps.GenerateCSRFToken(r)) - } + if resendNode != nil { + freshNodes.Append(resendNode) + } - f.GetUI().Messages.Set(message) + f.GetUI().Nodes = freshNodes - case flow.StatePassedChallenge: - fallthrough - default: - return errors.WithStack(herodot.ErrBadRequest.WithReason("received an unexpected flow state")) - } + f.GetUI().Method = "POST" + f.GetUI().Action = flow.AppendFlowTo(urlx.AppendPaths(s.deps.Config().SelfPublicURL(ctx), route), f.GetID()).String() - // 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().Messages.Set(message) return nil } @@ -361,3 +416,29 @@ const CodeLength = 6 func GenerateCode() string { return randx.MustString(CodeLength, randx.Numeric) } + +// 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 +// - f@bar -> f@bar +// - fo@bar -> fo****@bar +// - +12345678910 -> +12****10 +func MaskAddress(input string) string { + if strings.Contains(input, "@") { + 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)-2:] +} diff --git a/selfservice/strategy/code/strategy_login.go b/selfservice/strategy/code/strategy_login.go index 1a0170d36648..5f7d04e395da 100644 --- a/selfservice/strategy/code/strategy_login.go +++ b/selfservice/strategy/code/strategy_login.go @@ -11,12 +11,13 @@ import ( "github.com/ory/x/sqlcon" - "github.com/gofrs/uuid" "github.com/pkg/errors" "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 +61,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 +107,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) } @@ -136,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) @@ -144,7 +151,15 @@ 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 { + 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 } @@ -167,7 +182,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, sess); err != nil { return nil, s.HandleLoginError(r, f, &p, err) } return nil, nil @@ -184,8 +199,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, sess *session.Session) (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 { @@ -194,10 +209,30 @@ func (s *Strategy) loginSendEmail(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(schema.NewUnknownAddressError()) + } + 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 + } + addresses = []Address{{ + To: p.Identifier, + Via: identity.CodeAddressType(identity.AddressTypeEmail), + }} } // Step 2: Delete any previous login codes for this flow ID @@ -205,11 +240,6 @@ 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), - }} - // 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 cda8ef3c05a9..06d811362978 100644 --- a/selfservice/strategy/code/strategy_login_test.go +++ b/selfservice/strategy/code/strategy_login_test.go @@ -12,6 +12,8 @@ import ( "net/url" "testing" + "github.com/ory/x/ioutilx" + "github.com/ory/x/snapshotx" "github.com/ory/x/sqlcon" "github.com/ory/x/stringsx" @@ -25,6 +27,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" ) @@ -33,7 +36,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"}) @@ -85,6 +88,7 @@ func TestLoginCodeStrategy(t *testing.T) { loginCode string identityEmail string testServer *httptest.Server + body string } type ApiType string @@ -161,6 +165,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) @@ -173,13 +178,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,50 +576,167 @@ func TestLoginCodeStrategy(t *testing.T) { require.True(t, va.Verified) }) - t.Run("case=should not 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("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) + }) - // enable webauthn 2FA method - conf.MustSet(ctx, fmt.Sprintf("%s.%s.enabled", config.ViperKeySelfServiceStrategyConfig, "webauthn"), true) - conf.MustSet(ctx, config.ViperKeySessionWhoAmIAAL, config.HighestAvailableAAL) + 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")) + } - t.Cleanup(func() { - conf.MustSet(ctx, fmt.Sprintf("%s.%s.enabled", config.ViperKeySelfServiceStrategyConfig, "webauthn"), false) - conf.MustSet(ctx, config.ViperKeySessionWhoAmIAAL, "aal1") + 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) + + s = submitLogin(ctx, t, s, tc.apiType, func(v *url.Values) { + v.Set("code", loginCode) + }, true, nil) + + 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")) + } - s := createLoginFlow(ctx, t, public, tc.apiType, 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) - // submit email - s = submitLogin(ctx, t, s, tc.apiType, func(v *url.Values) { - v.Set("identifier", s.identityEmail) - }, false, 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) + }) - 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") + 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")) + } - loginCode := testhelpers.CourierExpectCodeInMessage(t, message, 1) - assert.NotEmpty(t, loginCode) + 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")) + }) - // 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) { - if tc.apiType == ApiTypeSPA { - require.EqualValues(t, http.StatusOK, res.StatusCode) + 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 { - require.EqualValues(t, http.StatusOK, res.StatusCode) + 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) + + 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) }) - 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") + 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/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/code/strategy_test.go b/selfservice/strategy/code/strategy_test.go index 634605cba3f3..4561a280fb96 100644 --- a/selfservice/strategy/code/strategy_test.go +++ b/selfservice/strategy/code/strategy_test.go @@ -38,3 +38,39 @@ 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: "ab@cd", + expected: "ab****@cd", + }, + { + 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)) + }) + } +} 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/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/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/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/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/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/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) { 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) { diff --git a/spec/api.json b/spec/api.json index fcb22f35419e..0ea8c9ef5ba6 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" @@ -5178,6 +5176,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 557a69043a5a..ea6f219adb35 100755 --- a/spec/swagger.json +++ b/spec/swagger.json @@ -1563,6 +1563,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": { @@ -3195,9 +3201,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.", @@ -4090,7 +4093,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/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/mfa/code.spec.ts b/test/e2e/cypress/integration/profiles/mfa/code.spec.ts new file mode 100644 index 000000000000..7961aa850de9 --- /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&via=email") + }) + + it("should be asked to sign in with 2fa if set up", () => { + cy.get("input[name='identifier']").type(email) + cy.contains("Continue 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("Continue 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("Continue 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/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 f8f2b7ad0615..7ef6260f4b78 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( @@ -1103,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) @@ -1147,10 +1156,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 +1172,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 +1191,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 +1254,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 +1280,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 +1404,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 +1516,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 +1532,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 +1553,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/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/test/e2e/cypress/support/index.d.ts b/test/e2e/cypress/support/index.d.ts index 1792db4a51cc..3fef679ec098 100644 --- a/test/e2e/cypress/support/index.d.ts +++ b/test/e2e/cypress/support/index.d.ts @@ -104,10 +104,11 @@ declare global { * * @param opts */ - getMail(opts?: { - removeMail: boolean + getMail(opts: { + 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 */ diff --git a/text/id.go b/text/id.go index fa8184f0ffaf..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 ( @@ -153,6 +155,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..c94db4538704 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,30 @@ 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, + } +} + +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{ + "masked_to": maskedTo, + }), + } +}