From 602d71687c3191087beab384e0d604cbef9ebc95 Mon Sep 17 00:00:00 2001 From: splaunov Date: Wed, 27 Mar 2024 12:00:49 +0300 Subject: [PATCH 1/3] feat: delegate SMS code generation, sending and verification to an external service (PS-221) ignore: respond 400 when code is invalid (PS-307) ignore: accept all http codes from 200-300 range ignore: add `body` parameter to request config schema (PS-221) feat: delegate SMS code generation, sending and verification to an external service - registration flow (PS-262) feat: delegate SMS code generation, sending and verification to an external service - login flow (PS-263) feat: delegate SMS code generation, sending and verification to an external service - verification flow (PS-221) --- cmd/clidoc/main.go | 2 + .../template/sms/registration_code_valid.go | 54 +++ courier/template/sms/verification_code.go | 1 + driver/config/config.go | 36 +- driver/registry_default.go | 5 +- driver/registry_default_recovery.go | 8 + embedx/config.schema.json | 116 +++++++ identity/address.go | 1 + identity/extension_credentials.go | 43 ++- identity/extension_credentials_test.go | 32 +- identity/extension_verification.go | 10 + identity/extension_verification_test.go | 2 +- identity/identity_verification.go | 1 + .../extension/credentials/code.schema.json | 12 + internal/client-go/go.sum | 1 + internal/testhelpers/fake.go | 4 + persistence/sql/persister_code.go | 11 +- .../sql/persister_registration_code.go | 10 + selfservice/flow/login/handler_test.go | 2 +- selfservice/flow/state.go | 10 +- .../flow/verification/fake_strategy.go | 2 +- selfservice/flow/verification/strategy.go | 2 +- selfservice/flowhelpers/login_test.go | 1 + .../flowhelpers/stub/login.schema.json | 14 +- selfservice/hook/verification.go | 2 +- .../code/.schema/verification.schema.json | 4 + ...all_the_correct_verification_payloads.json | 19 ++ .../strategy/code/code_external_verifier.go | 129 ++++++++ .../strategy/code/code_registration.go | 4 + selfservice/strategy/code/code_sender.go | 141 +++++++- selfservice/strategy/code/persistence.go | 1 + selfservice/strategy/code/strategy.go | 10 +- selfservice/strategy/code/strategy_login.go | 39 ++- .../code/strategy_login_phone_test.go | 262 +++++++++++++++ .../strategy/code/strategy_registration.go | 55 +++- .../code/strategy_registration_phone_test.go | 311 ++++++++++++++++++ selfservice/strategy/code/strategy_test.go | 52 +++ .../strategy/code/strategy_verification.go | 94 +++++- .../code/strategy_verification_phone_test.go | 183 +++++++++++ .../code/strategy_verification_test.go | 2 +- .../code/stub/code.identity.schema.json | 16 + .../code/stub/code.phone.identity.schema.json | 86 +++++ .../strategy/code/stub/default.schema.json | 23 +- .../code/stub/request.config.login.jsonnet | 4 + .../stub/request.config.registration.jsonnet | 4 + .../stub/request.config.verification.jsonnet | 4 + .../strategy/link/strategy_verification.go | 2 +- session/manager_http_test.go | 3 + text/id.go | 32 +- text/message_node.go | 8 + text/message_verification.go | 8 + 51 files changed, 1787 insertions(+), 91 deletions(-) create mode 100644 courier/template/sms/registration_code_valid.go create mode 100644 selfservice/strategy/code/code_external_verifier.go create mode 100644 selfservice/strategy/code/strategy_login_phone_test.go create mode 100644 selfservice/strategy/code/strategy_registration_phone_test.go create mode 100644 selfservice/strategy/code/strategy_verification_phone_test.go create mode 100644 selfservice/strategy/code/stub/code.phone.identity.schema.json create mode 100644 selfservice/strategy/code/stub/request.config.login.jsonnet create mode 100644 selfservice/strategy/code/stub/request.config.registration.jsonnet create mode 100644 selfservice/strategy/code/stub/request.config.verification.jsonnet diff --git a/cmd/clidoc/main.go b/cmd/clidoc/main.go index 6ed8df8d1748..16d2f387613c 100644 --- a/cmd/clidoc/main.go +++ b/cmd/clidoc/main.go @@ -177,6 +177,8 @@ func init() { "NewErrorValidationAddressUnknown": text.NewErrorValidationAddressUnknown(), "NewInfoSelfServiceLoginCodeMFA": text.NewInfoSelfServiceLoginCodeMFA(), "NewInfoSelfServiceLoginCodeMFAHint": text.NewInfoSelfServiceLoginCodeMFAHint("{maskedIdentifier}"), + "NewInfoNodeInputPhone": text.NewInfoNodeInputPhone(), + "NewInfoSelfServicePhoneVerificationSuccessful": text.NewInfoSelfServicePhoneVerificationSuccessful(), } } diff --git a/courier/template/sms/registration_code_valid.go b/courier/template/sms/registration_code_valid.go new file mode 100644 index 000000000000..3235162a12bd --- /dev/null +++ b/courier/template/sms/registration_code_valid.go @@ -0,0 +1,54 @@ +// Copyright © 2023 Ory Corp +// SPDX-License-Identifier: Apache-2.0 + +package sms + +import ( + "context" + "encoding/json" + "os" + + "github.com/ory/kratos/courier/template" +) + +type ( + RegistrationCodeValid struct { + deps template.Dependencies + model *RegistrationCodeValidModel + } + RegistrationCodeValidModel struct { + To string `json:"to"` + RegistrationCode string `json:"registration_code"` + Traits map[string]interface{} `json:"traits"` + RequestURL string `json:"request_url"` + TransientPayload map[string]interface{} `json:"transient_payload"` + } +) + +func NewRegistrationCodeValid(d template.Dependencies, m *RegistrationCodeValidModel) *RegistrationCodeValid { + return &RegistrationCodeValid{deps: d, model: m} +} + +func (t *RegistrationCodeValid) PhoneNumber() (string, error) { + return t.model.To, nil +} + +func (t *RegistrationCodeValid) SMSBody(ctx context.Context) (string, error) { + return template.LoadText( + ctx, + t.deps, + os.DirFS(t.deps.CourierConfig().CourierTemplatesRoot(ctx)), + "registration_code/valid/sms.body.gotmpl", + "registration_code/valid/sms.body*", + t.model, + t.deps.CourierConfig().CourierSMSTemplatesLoginCodeValid(ctx).Body.PlainText, + ) +} + +func (t *RegistrationCodeValid) MarshalJSON() ([]byte, error) { + return json.Marshal(t.model) +} + +func (t *RegistrationCodeValid) TemplateType() template.TemplateType { + return template.TypeLoginCodeValid +} diff --git a/courier/template/sms/verification_code.go b/courier/template/sms/verification_code.go index 4204df0ac4c8..f03185b2a835 100644 --- a/courier/template/sms/verification_code.go +++ b/courier/template/sms/verification_code.go @@ -19,6 +19,7 @@ type ( VerificationCodeValidModel struct { To string `json:"to"` + VerificationURL string `json:"verification_url"` VerificationCode string `json:"verification_code"` Identity map[string]interface{} `json:"identity"` RequestURL string `json:"request_url"` diff --git a/driver/config/config.go b/driver/config/config.go index 0d755a11ba63..59efd8dbd6b5 100644 --- a/driver/config/config.go +++ b/driver/config/config.go @@ -69,6 +69,7 @@ const ( ViperKeyCourierTemplatesVerificationCodeValidEmail = "courier.templates.verification_code.valid.email" ViperKeyCourierTemplatesVerificationCodeValidSMS = "courier.templates.verification_code.valid.sms" ViperKeyCourierTemplatesLoginCodeValidSMS = "courier.templates.login_code.valid.sms" + ViperKeyCourierTemplatesRegistrationCodeValidSMS = "courier.templates.registration_code.valid.sms" ViperKeyCourierDeliveryStrategy = "courier.delivery_strategy" ViperKeyCourierHTTPRequestConfig = "courier.http.request_config" ViperKeyCourierTemplatesLoginCodeValidEmail = "courier.templates.login_code.valid.email" @@ -241,10 +242,17 @@ type ( Enabled bool `json:"enabled"` Config json.RawMessage `json:"config"` } + ExternalSMSVerify struct { + Enabled bool `json:"enabled"` + VerificationStartRequest json.RawMessage `json:"verification_start_request"` + VerificationCheckRequest json.RawMessage `json:"verification_check_request"` + } + SelfServiceStrategyCode struct { *SelfServiceStrategy - PasswordlessEnabled bool `json:"passwordless_enabled"` - MFAEnabled bool `json:"mfa_enabled"` + PasswordlessEnabled bool `json:"passwordless_enabled"` + MFAEnabled bool `json:"mfa_enabled"` + ExternalSMSVerify *ExternalSMSVerify `json:"external_sms_verify"` } Schema struct { ID string `json:"id" koanf:"id"` @@ -313,6 +321,7 @@ type ( CourierTemplatesRegistrationCodeValid(ctx context.Context) *CourierEmailTemplate CourierSMSTemplatesVerificationCodeValid(ctx context.Context) *CourierSMSTemplate CourierSMSTemplatesLoginCodeValid(ctx context.Context) *CourierSMSTemplate + CourierSMSTemplatesRegistrationCodeValid(ctx context.Context) *CourierSMSTemplate CourierMessageRetries(ctx context.Context) int CourierWorkerPullCount(ctx context.Context) int CourierWorkerPullWait(ctx context.Context) time.Duration @@ -794,6 +803,8 @@ func (p *Config) SelfServiceStrategy(ctx context.Context, strategy string) *Self func (p *Config) SelfServiceCodeStrategy(ctx context.Context) *SelfServiceStrategyCode { pp := p.GetProvider(ctx) config := json.RawMessage("{}") + verificationStartRequest := json.RawMessage("{}") + verificationCheckRequest := json.RawMessage("{}") basePath := ViperKeySelfServiceStrategyConfig + ".code" var err error @@ -803,6 +814,18 @@ func (p *Config) SelfServiceCodeStrategy(ctx context.Context) *SelfServiceStrate config = json.RawMessage("{}") } + verificationStartRequest, err = json.Marshal(pp.GetF(basePath+".external_sms_verify.verification_start_request", verificationStartRequest)) + if err != nil { + p.l.WithError(err).Warn("Unable to marshal self service strategy verification_start_request.") + verificationStartRequest = json.RawMessage("{}") + } + + verificationCheckRequest, err = json.Marshal(pp.GetF(basePath+".external_sms_verify.verification_check_request", verificationCheckRequest)) + if err != nil { + p.l.WithError(err).Warn("Unable to marshal self service strategy verification_check_request.") + verificationCheckRequest = json.RawMessage("{}") + } + return &SelfServiceStrategyCode{ SelfServiceStrategy: &SelfServiceStrategy{ Enabled: pp.BoolF(basePath+".enabled", true), @@ -810,6 +833,11 @@ func (p *Config) SelfServiceCodeStrategy(ctx context.Context) *SelfServiceStrate }, PasswordlessEnabled: pp.BoolF(basePath+".passwordless_enabled", false), MFAEnabled: pp.BoolF(basePath+".mfa_enabled", false), + ExternalSMSVerify: &ExternalSMSVerify{ + Enabled: pp.BoolF(basePath+".external_sms_verify.enabled", false), + VerificationStartRequest: verificationStartRequest, + VerificationCheckRequest: verificationCheckRequest, + }, } } @@ -1147,6 +1175,10 @@ func (p *Config) CourierSMSTemplatesLoginCodeValid(ctx context.Context) *Courier return p.CourierSMSTemplatesHelper(ctx, ViperKeyCourierTemplatesLoginCodeValidSMS) } +func (p *Config) CourierSMSTemplatesRegistrationCodeValid(ctx context.Context) *CourierSMSTemplate { + return p.CourierSMSTemplatesHelper(ctx, ViperKeyCourierTemplatesRegistrationCodeValidSMS) +} + 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 eab63a120981..d9cef26b5739 100644 --- a/driver/registry_default.go +++ b/driver/registry_default.go @@ -134,8 +134,9 @@ type RegistryDefault struct { selfserviceVerifyHandler *verification.Handler selfserviceVerificationExecutor *verification.HookExecutor - selfserviceLinkSender *link.Sender - selfserviceCodeSender *code.Sender + selfserviceLinkSender *link.Sender + selfserviceCodeSender *code.Sender + selfserviceExternalVerifier *code.ExternalVerifier selfserviceRecoveryErrorHandler *recovery.ErrorHandler selfserviceRecoveryHandler *recovery.Handler diff --git a/driver/registry_default_recovery.go b/driver/registry_default_recovery.go index 04cf24857eba..f43b083fcd46 100644 --- a/driver/registry_default_recovery.go +++ b/driver/registry_default_recovery.go @@ -95,3 +95,11 @@ func (m *RegistryDefault) CodeSender() *code.Sender { return m.selfserviceCodeSender } + +func (m *RegistryDefault) ExternalVerifier() *code.ExternalVerifier { + if m.selfserviceExternalVerifier == nil { + m.selfserviceExternalVerifier = code.NewExternalVerifier(m) + } + + return m.selfserviceExternalVerifier +} diff --git a/embedx/config.schema.json b/embedx/config.schema.json index 79bc83c88b1d..cf258096a447 100644 --- a/embedx/config.schema.json +++ b/embedx/config.schema.json @@ -1520,6 +1520,122 @@ "examples": ["1h", "1m", "1s"] } } + }, + "external_sms_verify": { + "additionalProperties": false, + "type": "object", + "properties": { + "enabled": { + "type": "boolean", + "title": "Delegate generation, sending and verification of codes sent with SMS to external service", + "description": "If enabled, registration, login and verification flows will use extrnal service to to generate, send and verify one-time codes then fow method is 'code' and identifier is a phone number", + "default": false + }, + "verification_start_request": { + "type": "object", + "properties": { + "url": { + "title": "HTTP address of verifications start API endpoint", + "description": "This URL will be used to generate and send auth or verification code.", + "examples": [ + "https://verify.twilio.com/v2/Services/VAXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX/Verifications" + ], + "type": "string" + }, + "method": { + "type": "string", + "description": "The HTTP method to use (GET, POST, etc)." + }, + "headers": { + "type": "object", + "description": "The HTTP headers that must be applied to request" + }, + "body": { + "type": "string", + "format": "uri", + "pattern": "^(http|https|file|base64)://", + "description": "URI pointing to the jsonnet template used for payload generation. Only used for those HTTP methods, which support HTTP body payloads", + "examples": [ + "file:///path/to/body.jsonnet", + "file://./body.jsonnet", + "base64://ZnVuY3Rpb24oY3R4KSB7CiAgaWRlbnRpdHlfaWQ6IGlmIGN0eFsiaWRlbnRpdHkiXSAhPSBudWxsIHRoZW4gY3R4LmlkZW50aXR5LmlkLAp9=", + "https://oryapis.com/default_body.jsonnet" + ] + }, + "auth": { + "type": "object", + "title": "Auth mechanisms", + "description": "Define which auth mechanism to use", + "oneOf": [ + { + "$ref": "#/definitions/webHookAuthApiKeyProperties" + }, + { + "$ref": "#/definitions/webHookAuthBasicAuthProperties" + } + ] + }, + "additionalProperties": false + }, + "required": [ + "url", + "method" + ], + "additionalProperties": false + }, + "verification_check_request": { + "type": "object", + "properties": { + "url": { + "title": "HTTP address of verifications check API endpoint", + "description": "This URL will be used to check auth or verification code.", + "examples": [ + "https://verify.twilio.com/v2/Services/VAXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX/VerificationCheck" + ], + "type": "string" + }, + "method": { + "type": "string", + "description": "The HTTP method to use (GET, POST, etc)." + }, + "headers": { + "type": "object", + "description": "The HTTP headers that must be applied to request" + }, + "body": { + "type": "string", + "format": "uri", + "pattern": "^(http|https|file|base64)://", + "description": "URI pointing to the jsonnet template used for payload generation. Only used for those HTTP methods, which support HTTP body payloads", + "examples": [ + "file:///path/to/body.jsonnet", + "file://./body.jsonnet", + "base64://ZnVuY3Rpb24oY3R4KSB7CiAgaWRlbnRpdHlfaWQ6IGlmIGN0eFsiaWRlbnRpdHkiXSAhPSBudWxsIHRoZW4gY3R4LmlkZW50aXR5LmlkLAp9=", + "https://oryapis.com/default_body.jsonnet" + ] + }, + "auth": { + "type": "object", + "title": "Auth mechanisms", + "description": "Define which auth mechanism to use", + "oneOf": [ + { + "$ref": "#/definitions/webHookAuthApiKeyProperties" + }, + { + "$ref": "#/definitions/webHookAuthBasicAuthProperties" + } + ] + }, + "additionalProperties": false + }, + "required": [ + "url", + "method" + ], + "additionalProperties": false + } + } } } }, diff --git a/identity/address.go b/identity/address.go index 2e9175642e84..2bb619761cb3 100644 --- a/identity/address.go +++ b/identity/address.go @@ -5,4 +5,5 @@ package identity const ( AddressTypeEmail = "email" + AddressTypePhone = "sms" ) diff --git a/identity/extension_credentials.go b/identity/extension_credentials.go index 3baa826b2e9c..d8b661c8631e 100644 --- a/identity/extension_credentials.go +++ b/identity/extension_credentials.go @@ -8,6 +8,8 @@ import ( "strings" "sync" + "github.com/nyaruka/phonenumbers" + "github.com/ory/jsonschema/v3" "github.com/ory/x/sqlxx" "github.com/ory/x/stringslice" @@ -23,7 +25,7 @@ type SchemaExtensionCredentials struct { } func NewSchemaExtensionCredentials(i *Identity) *SchemaExtensionCredentials { - return &SchemaExtensionCredentials{i: i} + return &SchemaExtensionCredentials{i: i, v: make(map[CredentialsType][]string)} } func (r *SchemaExtensionCredentials) setIdentifier(ct CredentialsType, value interface{}) { @@ -64,12 +66,14 @@ func (r *SchemaExtensionCredentials) Run(ctx jsonschema.ValidationContext, s sch } r.setIdentifier(CredentialsTypeCodeAuth, value) - // case f.AddCase(AddressTypePhone): - // if !jsonschema.Formats["tel"](value) { - // return ctx.Error("format", "%q is not a valid %q", value, s.Credentials.Code.Via) - // } - - // r.setIdentifier(CredentialsTypeCodeAuth, value, CredentialsIdentifierAddressTypePhone) + case f.AddCase(AddressTypePhone): + phoneNumber, err := phonenumbers.Parse(fmt.Sprintf("%s", value), "") + if err != nil { + validationError := ctx.Error("format", "%s", err) + return validationError + } + e164 := fmt.Sprintf("+%d%d", *phoneNumber.CountryCode, *phoneNumber.NationalNumber) + r.setIdentifier(CredentialsTypeCodeAuth, e164) default: return ctx.Error("", "credentials.code.via has unknown value %q", s.Credentials.Code.Via) } @@ -79,5 +83,30 @@ func (r *SchemaExtensionCredentials) Run(ctx jsonschema.ValidationContext, s sch } func (r *SchemaExtensionCredentials) Finish() error { + r.l.Lock() + defer r.l.Unlock() + + for ct := range r.i.Credentials { + _, ok := r.v[ct] + if !ok { + r.v[ct] = []string{} + } + } + for ct, identifiers := range r.v { + cred, ok := r.i.GetCredentials(ct) + if !ok { + cred = &Credentials{ + Type: ct, + Identifiers: []string{}, + Config: sqlxx.JSONRawMessage{}, + } + } + + if ct == CredentialsTypePassword || ct == CredentialsTypeCodeAuth { + cred.Identifiers = identifiers + r.i.SetCredentials(ct, *cred) + } + } + return nil } diff --git a/identity/extension_credentials_test.go b/identity/extension_credentials_test.go index 95cd9d000c6a..bd2d73d64422 100644 --- a/identity/extension_credentials_test.go +++ b/identity/extension_credentials_test.go @@ -6,6 +6,7 @@ package identity_test import ( "bytes" "context" + "errors" "fmt" "testing" @@ -36,6 +37,15 @@ func TestSchemaExtensionCredentials(t *testing.T) { expect: []string{"foo@ory.sh"}, ct: identity.CredentialsTypePassword, }, + { + doc: `{}`, + schema: "file://./stub/extension/credentials/schema.json", + expect: []string{}, + existing: &identity.Credentials{ + Identifiers: []string{"foo@ory.sh"}, + }, + ct: identity.CredentialsTypePassword, + }, { doc: `{"emails":["foo@ory.sh","foo@ory.sh","bar@ory.sh"], "username": "foobar"}`, schema: "file://./stub/extension/credentials/multi.schema.json", @@ -87,6 +97,18 @@ func TestSchemaExtensionCredentials(t *testing.T) { }, ct: identity.CredentialsTypeCodeAuth, }, + { + doc: `{"phone":"not-valid-number"}`, + schema: "file://./stub/extension/credentials/code.schema.json", + ct: identity.CredentialsTypeCodeAuth, + expectErr: errors.New("I[#/phone] S[#/properties/phone] validation failed\n I[#/phone] S[#/properties/phone/format] \"not-valid-number\" is not valid \"tel\"\n I[#/phone] S[#/properties/phone/format] the phone number supplied is not a number"), + }, + { + doc: `{"phone":"+4407376494399"}`, + schema: "file://./stub/extension/credentials/code.schema.json", + expect: []string{"+447376494399"}, + ct: identity.CredentialsTypeCodeAuth, + }, } { t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) { c := jsonschema.NewCompiler() @@ -103,12 +125,16 @@ func TestSchemaExtensionCredentials(t *testing.T) { err = c.MustCompile(ctx, tc.schema).Validate(bytes.NewBufferString(tc.doc)) if tc.expectErr != nil { require.EqualError(t, err, tc.expectErr.Error()) + } else { + require.NoError(t, err) } require.NoError(t, e.Finish()) - credentials, ok := i.GetCredentials(tc.ct) - require.True(t, ok) - assert.ElementsMatch(t, tc.expect, credentials.Identifiers) + if tc.expectErr == nil { + credentials, ok := i.GetCredentials(tc.ct) + require.True(t, ok) + assert.ElementsMatch(t, tc.expect, credentials.Identifiers) + } }) } } diff --git a/identity/extension_verification.go b/identity/extension_verification.go index 3b3f92581c37..372eb6b9364d 100644 --- a/identity/extension_verification.go +++ b/identity/extension_verification.go @@ -5,6 +5,7 @@ package identity import ( "fmt" + "github.com/nyaruka/phonenumbers" "strings" "sync" "time" @@ -76,6 +77,15 @@ func (r *SchemaExtensionVerification) Run(ctx jsonschema.ValidationContext, s sc switch formatString { case "email": normalized = strings.ToLower(strings.TrimSpace(fmt.Sprintf("%s", value))) + case "tel": + fallthrough + case "phone": + phoneNumber, err := phonenumbers.Parse(fmt.Sprintf("%s", value), "") + if err != nil { + validationError := ctx.Error("format", "%s", err) + return validationError + } + normalized = fmt.Sprintf("+%d%d", *phoneNumber.CountryCode, *phoneNumber.NationalNumber) default: normalized = strings.TrimSpace(fmt.Sprintf("%s", value)) } diff --git a/identity/extension_verification_test.go b/identity/extension_verification_test.go index cc7f47b07091..07e23116b5ae 100644 --- a/identity/extension_verification_test.go +++ b/identity/extension_verification_test.go @@ -333,7 +333,7 @@ func TestSchemaExtensionVerification(t *testing.T) { name: "phone:must return error for malformed input", schema: phoneSchemaPath, doc: `{"phones":["+18004444444","+18004444444","12112112"], "username": "+380634872774"}`, - expectErr: errors.New("I[#/phones/2] S[#/properties/phones/items/format] \"12112112\" is not valid \"tel\""), + expectErr: errors.New("I[#/phones/2] S[#/properties/phones/items] validation failed\n I[#/phones/2] S[#/properties/phones/items/format] \"12112112\" is not valid \"tel\"\n I[#/phones/2] S[#/properties/phones/items/format] invalid country code"), }, { name: "missing format returns an error", diff --git a/identity/identity_verification.go b/identity/identity_verification.go index 251fee019d3b..a612fb7bf18e 100644 --- a/identity/identity_verification.go +++ b/identity/identity_verification.go @@ -15,6 +15,7 @@ import ( const ( VerifiableAddressTypeEmail VerifiableAddressType = AddressTypeEmail + VerifiableAddressTypePhone VerifiableAddressType = AddressTypePhone VerifiableAddressStatusPending VerifiableAddressStatus = "pending" VerifiableAddressStatusSent VerifiableAddressStatus = "sent" diff --git a/identity/stub/extension/credentials/code.schema.json b/identity/stub/extension/credentials/code.schema.json index bef244bc9ae5..3741ab0c853f 100644 --- a/identity/stub/extension/credentials/code.schema.json +++ b/identity/stub/extension/credentials/code.schema.json @@ -15,6 +15,18 @@ } } } + }, + "phone": { + "type": "string", + "format": "tel", + "ory.sh/kratos": { + "credentials": { + "code": { + "identifier": true, + "via": "sms" + } + } + } } } } diff --git a/internal/client-go/go.sum b/internal/client-go/go.sum index c966c8ddfd0d..6cc3f5911d11 100644 --- a/internal/client-go/go.sum +++ b/internal/client-go/go.sum @@ -4,6 +4,7 @@ github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5y golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190108225652-1e06a53dbb7e h1:bRhVy7zSSasaqNksaRZiA5EEI+Ei4I1nO5Jh72wfHlg= golang.org/x/net v0.0.0-20190108225652-1e06a53dbb7e/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= +golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4 h1:YUO/7uOKsKeq9UokNS62b8FYywz3ker1l1vDZRCRefw= golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= diff --git a/internal/testhelpers/fake.go b/internal/testhelpers/fake.go index 88f1b22d669a..fa9667681d1c 100644 --- a/internal/testhelpers/fake.go +++ b/internal/testhelpers/fake.go @@ -12,3 +12,7 @@ import ( func RandomEmail() string { return strings.ToLower(randx.MustString(16, randx.Alpha) + "@ory.sh") } + +func RandomPhone() string { + return "+458001" + strings.ToLower(randx.MustString(4, randx.Numeric)) +} diff --git a/persistence/sql/persister_code.go b/persistence/sql/persister_code.go index 31e0b80dc2d2..8c944e34df53 100644 --- a/persistence/sql/persister_code.go +++ b/persistence/sql/persister_code.go @@ -53,9 +53,11 @@ func useOneTimeCode[P any, U interface { var target U nid := p.NetworkID(ctx) if err := p.Transaction(ctx, func(ctx context.Context, tx *pop.Connection) error { - //#nosec G201 -- TableName is static - if err := tx.RawQuery(fmt.Sprintf("UPDATE %s SET submit_count = submit_count + 1 WHERE id = ? AND nid = ?", flowTableName), flowID, nid).Exec(); err != nil { - return err + if userProvidedCode != "external" { + //#nosec G201 -- TableName is static + if err := tx.RawQuery(fmt.Sprintf("UPDATE %s SET submit_count = submit_count + 1 WHERE id = ? AND nid = ?", flowTableName), flowID, nid).Exec(); err != nil { + return err + } } var submitCount int @@ -111,6 +113,9 @@ func useOneTimeCode[P any, U interface { return nil } + if userProvidedCode == "external" { + return nil + } //#nosec G201 -- TableName is static return tx.RawQuery(fmt.Sprintf("UPDATE %s SET used_at = ? WHERE id = ? AND nid = ?", target.TableName(ctx)), time.Now().UTC(), target.GetID(), nid).Exec() }); err != nil { diff --git a/persistence/sql/persister_registration_code.go b/persistence/sql/persister_registration_code.go index 095cb45156ba..4d59db6a2f33 100644 --- a/persistence/sql/persister_registration_code.go +++ b/persistence/sql/persister_registration_code.go @@ -5,6 +5,7 @@ package sql import ( "context" + "github.com/ory/kratos/persistence/sql/update" "time" "github.com/go-faker/faker/v4/pkg/slice" @@ -40,6 +41,15 @@ func (p *Persister) CreateRegistrationCode(ctx context.Context, params *code.Cre return registrationCode, nil } +func (p *Persister) UpdateRegistrationCode(ctx context.Context, code *code.RegistrationCode) (err error) { + ctx, span := p.r.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.UpdateRegistrationCode") + defer otelx.End(span, &err) + + cp := *code + cp.NID = p.NetworkID(ctx) + return update.Generic(ctx, p.GetConnection(ctx), p.r.Tracer(ctx).Tracer(), &cp) +} + func (p *Persister) UseRegistrationCode(ctx context.Context, flowID uuid.UUID, userProvidedCode string, addresses ...string) (_ *code.RegistrationCode, err error) { ctx, span := p.r.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.UseRegistrationCode") defer otelx.End(span, &err) diff --git a/selfservice/flow/login/handler_test.go b/selfservice/flow/login/handler_test.go index c8d5ac97772e..b777286bca9d 100644 --- a/selfservice/flow/login/handler_test.go +++ b/selfservice/flow/login/handler_test.go @@ -444,7 +444,7 @@ func TestFlowLifecycle(t *testing.T) { Config: sqlxx.JSONRawMessage(`{"hashed_password": "$argon2id$v=19$m=32,t=2,p=4$cm94YnRVOW5jZzFzcVE4bQ$MNzk5BtR2vUhrp6qQEjRNw"}`), }, }, - Traits: identity.Traits(fmt.Sprintf(`{"email":"%s"}`, email)), + Traits: identity.Traits(fmt.Sprintf(`{"username":"%s"}`, email)), SchemaID: config.DefaultIdentityTraitsSchemaID, } diff --git a/selfservice/flow/state.go b/selfservice/flow/state.go index 76a0683fc19d..715dd3936983 100644 --- a/selfservice/flow/state.go +++ b/selfservice/flow/state.go @@ -28,12 +28,13 @@ type State string const ( StateChooseMethod State = "choose_method" StateEmailSent State = "sent_email" + StateSMSSent State = "sent_sms" StatePassedChallenge State = "passed_challenge" StateShowForm State = "show_form" StateSuccess State = "success" ) -var states = []State{StateChooseMethod, StateEmailSent, StatePassedChallenge} +var states = []State{StateChooseMethod, StateEmailSent, StateSMSSent, StatePassedChallenge} func indexOf(current State) int { for k, s := range states { @@ -45,10 +46,17 @@ func indexOf(current State) int { } func HasReachedState(expected, actual State) bool { + if expected == StateEmailSent && actual == StateSMSSent || + expected == StateSMSSent && actual == StateEmailSent { + return true + } return indexOf(actual) >= indexOf(expected) } func NextState(current State) State { + if current == StateEmailSent { + return StatePassedChallenge + } if current == StatePassedChallenge { return StatePassedChallenge } diff --git a/selfservice/flow/verification/fake_strategy.go b/selfservice/flow/verification/fake_strategy.go index d497fb5111f3..42b095da65fc 100644 --- a/selfservice/flow/verification/fake_strategy.go +++ b/selfservice/flow/verification/fake_strategy.go @@ -31,7 +31,7 @@ func (f FakeStrategy) Verify(_ http.ResponseWriter, _ *http.Request, _ *Flow) (e return nil } -func (f FakeStrategy) SendVerificationEmail(context.Context, *Flow, *identity.Identity, *identity.VerifiableAddress) error { +func (f FakeStrategy) SendVerification(context.Context, *Flow, *identity.Identity, *identity.VerifiableAddress) error { return nil } diff --git a/selfservice/flow/verification/strategy.go b/selfservice/flow/verification/strategy.go index 3d270bfb8732..96eedd76a652 100644 --- a/selfservice/flow/verification/strategy.go +++ b/selfservice/flow/verification/strategy.go @@ -29,7 +29,7 @@ type ( NodeGroup() node.UiNodeGroup PopulateVerificationMethod(*http.Request, *Flow) error Verify(w http.ResponseWriter, r *http.Request, f *Flow) (err error) - SendVerificationEmail(context.Context, *Flow, *identity.Identity, *identity.VerifiableAddress) error + SendVerification(context.Context, *Flow, *identity.Identity, *identity.VerifiableAddress) error } AdminHandler interface { RegisterAdminVerificationRoutes(admin *x.RouterAdmin) diff --git a/selfservice/flowhelpers/login_test.go b/selfservice/flowhelpers/login_test.go index 3506c73c8513..1c0416059d9a 100644 --- a/selfservice/flowhelpers/login_test.go +++ b/selfservice/flowhelpers/login_test.go @@ -30,6 +30,7 @@ func TestGuessForcedLoginIdentifier(t *testing.T) { Identifiers: []string{"foobar"}, } i.Credentials[identity.CredentialsTypePassword] = ic + i.Traits = []byte(`{"email":"foobar"}`) require.NoError(t, reg.IdentityManager().Create(context.Background(), i)) req := httptest.NewRequest("GET", "/sessions/whoami", nil) diff --git a/selfservice/flowhelpers/stub/login.schema.json b/selfservice/flowhelpers/stub/login.schema.json index 82f85811a16a..a15e45581c08 100644 --- a/selfservice/flowhelpers/stub/login.schema.json +++ b/selfservice/flowhelpers/stub/login.schema.json @@ -5,7 +5,19 @@ "type": "object", "properties": { "traits": { - "type": "object" + "type": "object", + "properties": { + "email": { + "type": "string", + "ory.sh/kratos": { + "credentials": { + "password": { + "identifier": true + } + } + } + } + } } } } diff --git a/selfservice/hook/verification.go b/selfservice/hook/verification.go index 6fdd039c146f..9cbbba2e97eb 100644 --- a/selfservice/hook/verification.go +++ b/selfservice/hook/verification.go @@ -141,7 +141,7 @@ func (e *Verifier) do( return err } - if err := strategy.SendVerificationEmail(ctx, verificationFlow, i, address); err != nil { + if err := strategy.SendVerification(ctx, verificationFlow, i, address); err != nil { return err } diff --git a/selfservice/strategy/code/.schema/verification.schema.json b/selfservice/strategy/code/.schema/verification.schema.json index e60989dcf52e..493ebc7f791f 100644 --- a/selfservice/strategy/code/.schema/verification.schema.json +++ b/selfservice/strategy/code/.schema/verification.schema.json @@ -17,6 +17,10 @@ "type": "string", "format": "email" }, + "phone": { + "type": "string", + "format": "phone" + }, "flow": { "type": "string", "format": "uuid" diff --git a/selfservice/strategy/code/.snapshots/TestVerification-description=should_set_all_the_correct_verification_payloads.json b/selfservice/strategy/code/.snapshots/TestVerification-description=should_set_all_the_correct_verification_payloads.json index 37f61ac9e827..2125fdccd965 100644 --- a/selfservice/strategy/code/.snapshots/TestVerification-description=should_set_all_the_correct_verification_payloads.json +++ b/selfservice/strategy/code/.snapshots/TestVerification-description=should_set_all_the_correct_verification_payloads.json @@ -18,6 +18,25 @@ }, "type": "input" }, + { + "attributes": { + "disabled": false, + "name": "phone", + "node_type": "input", + "required": true, + "type": "tel" + }, + "group": "code", + "messages": [], + "meta": { + "label": { + "id": 1070015, + "text": "Phone", + "type": "info" + } + }, + "type": "input" + }, { "attributes": { "disabled": false, diff --git a/selfservice/strategy/code/code_external_verifier.go b/selfservice/strategy/code/code_external_verifier.go new file mode 100644 index 000000000000..03e588541a50 --- /dev/null +++ b/selfservice/strategy/code/code_external_verifier.go @@ -0,0 +1,129 @@ +package code + +import ( + "context" + "github.com/ory/kratos/courier" + "github.com/ory/kratos/driver/config" + "github.com/ory/kratos/request" + "github.com/ory/kratos/x" + "github.com/ory/x/jsonnetsecure" + "github.com/ory/x/otelx" + "github.com/pkg/errors" + "net/http" +) + +type ( + externalVerifierDependencies interface { + config.Provider + x.LoggingProvider + x.TracingProvider + x.HTTPClientProvider + jsonnetsecure.VMProvider + } + + ExternalVerifierProvider interface { + ExternalVerifier() *ExternalVerifier + } + + ExternalVerifier struct { + deps externalVerifierDependencies + } +) + +func NewExternalVerifier(deps externalVerifierDependencies) *ExternalVerifier { + return &ExternalVerifier{ + deps: deps, + } +} + +func (v *ExternalVerifier) VerificationStart(ctx context.Context, t courier.SMSTemplate) (err error) { + ctx, span := v.deps.Tracer(ctx).Tracer().Start(ctx, "ExternalVerifier.VerificationStart") + defer otelx.End(span, &err) + + builder, err := request.NewBuilder(ctx, v.deps.Config().SelfServiceCodeStrategy(ctx).ExternalSMSVerify.VerificationStartRequest, v.deps, nil) + if err != nil { + return errors.WithStack(err) + } + + req, err := builder.BuildRequest(ctx, t) + if err != nil { + return errors.WithStack(err) + } + req = req.WithContext(ctx) + + res, err := v.deps.HTTPClient(ctx).Do(req) + if err != nil { + return errors.WithStack(err) + } + + if res.StatusCode >= 200 && res.StatusCode < 300 { + phoneNumber, err := t.PhoneNumber() + if err != nil { + return errors.WithStack(err) + } + v.deps.Logger(). + WithField("to", phoneNumber). + WithField("template_type", t.TemplateType). + Debug("ExternalVerifier has started new verification.") + return nil + } + + body := x.MustReadAll(res.Body) + if err := res.Body.Close(); err != nil { + return err + } + + err = errors.Errorf( + "upstream server replied with status code %d and body %s", + res.StatusCode, + body, + ) + return errors.WithStack(err) +} + +func (v *ExternalVerifier) VerificationCheck(ctx context.Context, t courier.SMSTemplate) (err error) { + ctx, span := v.deps.Tracer(ctx).Tracer().Start(ctx, "ExternalVerifier.VerificationCheck") + defer otelx.End(span, &err) + + builder, err := request.NewBuilder(ctx, v.deps.Config().SelfServiceCodeStrategy(ctx).ExternalSMSVerify.VerificationCheckRequest, v.deps, nil) + if err != nil { + return errors.WithStack(err) + } + + req, err := builder.BuildRequest(ctx, t) + if err != nil { + return errors.WithStack(err) + } + req = req.WithContext(ctx) + + res, err := v.deps.HTTPClient(ctx).Do(req) + if err != nil { + return errors.WithStack(err) + } + + if res.StatusCode >= 200 && res.StatusCode < 300 { + phoneNumber, err := t.PhoneNumber() + if err != nil { + return errors.WithStack(err) + } + v.deps.Logger(). + WithField("to", phoneNumber). + WithField("template_type", t.TemplateType). + Debug("ExternalVerifier has checked a code.") + return nil + } else if res.StatusCode == http.StatusBadRequest { + return errors.WithStack(ErrCodeNotFound) + } + + body := x.MustReadAll(res.Body) + if err := res.Body.Close(); err != nil { + return err + } + + err = errors.Errorf( + "upstream server replied with status code %d and body %s", + res.StatusCode, + body, + ) + return errors.WithStack(err) +} diff --git a/selfservice/strategy/code/code_registration.go b/selfservice/strategy/code/code_registration.go index 4093480fb91e..85c9d65abb77 100644 --- a/selfservice/strategy/code/code_registration.go +++ b/selfservice/strategy/code/code_registration.go @@ -81,6 +81,10 @@ func (f *RegistrationCode) GetHMACCode() string { return f.CodeHMAC } +func (f RegistrationCode) GetNID() uuid.UUID { + return f.NID +} + func (f *RegistrationCode) GetID() uuid.UUID { return f.ID } diff --git a/selfservice/strategy/code/code_sender.go b/selfservice/strategy/code/code_sender.go index fdc5e8b38b2d..552f43e26cb6 100644 --- a/selfservice/strategy/code/code_sender.go +++ b/selfservice/strategy/code/code_sender.go @@ -44,6 +44,8 @@ type ( LoginCodePersistenceProvider x.HTTPClientProvider + + ExternalVerifierProvider } SenderProvider interface { CodeSender() *Sender @@ -80,7 +82,12 @@ func (s *Sender) SendCode(ctx context.Context, f flow.Flow, id *identity.Identit // address was used to verify the code. // // See also [this discussion](https://github.com/ory/kratos/pull/3456#discussion_r1307560988). - rawCode := GenerateCode() + var rawCode string + if s.deps.Config().SelfServiceCodeStrategy(ctx).ExternalSMSVerify.Enabled && address.Via == identity.AddressTypePhone { + rawCode = "external" + } else { + rawCode = GenerateCode() + } switch f.GetFlowName() { case flow.RegistrationFlow: @@ -101,21 +108,33 @@ func (s *Sender) SendCode(ctx context.Context, f flow.Flow, id *identity.Identit return err } - emailModel := email.RegistrationCodeValidModel{ - To: address.To, - RegistrationCode: rawCode, - Traits: model, - RequestURL: f.GetRequestURL(), - TransientPayload: transientPayload, + var t courier.Template + switch address.Via { + case identity.ChannelTypeEmail: + t = email.NewRegistrationCodeValid(s.deps, &email.RegistrationCodeValidModel{ + To: address.To, + RegistrationCode: rawCode, + Traits: model, + RequestURL: f.GetRequestURL(), + TransientPayload: transientPayload, + }) + case identity.ChannelTypeSMS: + t = sms.NewRegistrationCodeValid(s.deps, &sms.RegistrationCodeValidModel{ + To: address.To, + RegistrationCode: rawCode, + Traits: model, + RequestURL: f.GetRequestURL(), + TransientPayload: transientPayload, + }) } s.deps.Audit(). WithField("registration_flow_id", code.FlowID). WithField("registration_code_id", code.ID). WithSensitiveField("registration_code", rawCode). - Info("Sending out registration email with code.") + Info("Sending out registration message with code.") - if err := s.send(ctx, string(address.Via), email.NewRegistrationCodeValid(s.deps, &emailModel)); err != nil { + if err := s.send(ctx, string(address.Via), t); err != nil { return errors.WithStack(err) } @@ -138,11 +157,6 @@ func (s *Sender) SendCode(ctx context.Context, f flow.Flow, id *identity.Identit if err != nil { return err } - 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.") var t courier.Template switch address.Via { @@ -164,6 +178,12 @@ func (s *Sender) SendCode(ctx context.Context, f flow.Flow, id *identity.Identit }) } + s.deps.Audit(). + WithField("login_flow_id", code.FlowID). + WithField("login_code_id", code.ID). + WithSensitiveField("login_code", rawCode). + Info("Sending out login message with code.") + if err := s.send(ctx, string(address.Via), t); err != nil { return errors.WithStack(err) } @@ -311,7 +331,12 @@ func (s *Sender) SendVerificationCode(ctx context.Context, f *verification.Flow, return err } - rawCode := GenerateCode() + var rawCode string + if s.deps.Config().SelfServiceCodeStrategy(ctx).ExternalSMSVerify.Enabled && via == identity.AddressTypePhone { + rawCode = "external" + } else { + rawCode = GenerateCode() + } var code *VerificationCode if code, err = s.deps.VerificationCodePersister().CreateVerificationCode(ctx, &CreateVerificationCodeParams{ RawCode: rawCode, @@ -375,6 +400,7 @@ func (s *Sender) SendVerificationCodeTo(ctx context.Context, f *verification.Flo case identity.ChannelTypeSMS: t = sms.NewVerificationCodeValid(s.deps, &sms.VerificationCodeValidModel{ To: code.VerifiableAddress.Value, + VerificationURL: s.constructVerificationLink(ctx, f.ID, codeString), VerificationCode: codeString, Identity: model, RequestURL: f.GetRequestURL(), @@ -417,9 +443,92 @@ func (s *Sender) send(ctx context.Context, via string, t courier.Template) error return errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Expected sms template but got %T", t)) } - _, err = c.QueueSMS(ctx, t) + if s.deps.Config().SelfServiceCodeStrategy(ctx).ExternalSMSVerify.Enabled { + err = s.deps.ExternalVerifier().VerificationStart(ctx, t) + } else { + _, err = c.QueueSMS(ctx, t) + } return err default: return f.ToUnknownCaseErr() } } + +func (s *Sender) VerificationCheckWithExternalVerifier(ctx context.Context, f *verification.Flow, + address *identity.VerifiableAddress, codeString string) error { + + i, err := s.deps.IdentityPool().GetIdentity(ctx, address.IdentityID, identity.ExpandDefault) + if err != nil { + return err + } + + model, err := x.StructToMap(i) + if err != nil { + return err + } + + transientPayload, err := x.ParseRawMessageOrEmpty(f.GetTransientPayload()) + if err != nil { + return errors.WithStack(err) + } + + t := sms.NewVerificationCodeValid(s.deps, &sms.VerificationCodeValidModel{ + To: address.Value, + VerificationCode: codeString, + Identity: model, + RequestURL: f.GetRequestURL(), + TransientPayload: transientPayload, + }) + + if err := s.deps.ExternalVerifier().VerificationCheck(ctx, t); err != nil { + return err + } + + return nil +} + +func (s *Sender) CheckWithExternalVerifier(ctx context.Context, f flow.Flow, id *identity.Identity, + address Address, codeString string) error { + + transientPayload, err := x.ParseRawMessageOrEmpty(f.GetTransientPayload()) + if err != nil { + return errors.WithStack(err) + } + + var t courier.SMSTemplate + + switch c := stringsx.SwitchExact(string(f.GetFlowName())); { + case c.AddCase(string(flow.RegistrationFlow)): + model, err := x.StructToMap(id.Traits) + if err != nil { + return err + } + t = sms.NewRegistrationCodeValid(s.deps, &sms.RegistrationCodeValidModel{ + To: address.To, + RegistrationCode: codeString, + Traits: model, + RequestURL: f.GetRequestURL(), + TransientPayload: transientPayload, + }) + case c.AddCase(string(flow.LoginFlow)): + model, err := x.StructToMap(id) + if err != nil { + return err + } + t = sms.NewLoginCodeValid(s.deps, &sms.LoginCodeValidModel{ + To: address.To, + LoginCode: codeString, + Identity: model, + RequestURL: f.GetRequestURL(), + TransientPayload: transientPayload, + }) + default: + return c.ToUnknownCaseErr() + } + + if err := s.deps.ExternalVerifier().VerificationCheck(ctx, t); err != nil { + return err + } + + return nil +} diff --git a/selfservice/strategy/code/persistence.go b/selfservice/strategy/code/persistence.go index ea5aaff682cc..f248dc763040 100644 --- a/selfservice/strategy/code/persistence.go +++ b/selfservice/strategy/code/persistence.go @@ -36,6 +36,7 @@ type ( RegistrationCodePersister interface { CreateRegistrationCode(context.Context, *CreateRegistrationCodeParams) (*RegistrationCode, error) + UpdateRegistrationCode(context.Context, *RegistrationCode) (err error) UseRegistrationCode(ctx context.Context, flowID uuid.UUID, code string, addresses ...string) (*RegistrationCode, error) DeleteRegistrationCodesOfFlow(ctx context.Context, flowID uuid.UUID) error GetUsedRegistrationCode(ctx context.Context, flowID uuid.UUID) (*RegistrationCode, error) diff --git a/selfservice/strategy/code/strategy.go b/selfservice/strategy/code/strategy.go index ee3ce353e4ae..0aceb7eff980 100644 --- a/selfservice/strategy/code/strategy.go +++ b/selfservice/strategy/code/strategy.go @@ -167,7 +167,9 @@ func (s *Strategy) PopulateMethod(r *http.Request, f flow.Flow) error { return err } case flow.StateEmailSent: - if err := s.populateEmailSentFlow(r.Context(), f); err != nil { + fallthrough + case flow.StateSMSSent: + if err := s.populateEmailOrSMSSentFlow(r.Context(), f); err != nil { return err } case flow.StatePassedChallenge: @@ -192,6 +194,10 @@ func (s *Strategy) populateChooseMethodFlow(r *http.Request, f flow.Flow) error node.NewInputField("email", nil, node.CodeGroup, node.InputAttributeTypeEmail, node.WithRequiredInputAttribute). WithMetaLabel(text.NewInfoNodeInputEmail()), ) + f.GetUI().Nodes.Append( + node.NewInputField("phone", nil, node.CodeGroup, node.InputAttributeTypeTel, node.WithRequiredInputAttribute). + WithMetaLabel(text.NewInfoNodeInputPhone()), + ) codeMetaLabel = text.NewInfoNodeLabelSubmit() case *login.Flow: ds, err := s.deps.Config().DefaultIdentityTraitsSchemaURL(ctx) @@ -267,7 +273,7 @@ func (s *Strategy) populateChooseMethodFlow(r *http.Request, f flow.Flow) error return nil } -func (s *Strategy) populateEmailSentFlow(ctx context.Context, f flow.Flow) error { +func (s *Strategy) populateEmailOrSMSSentFlow(ctx context.Context, f flow.Flow) error { // fresh ui node group freshNodes := node.Nodes{} var route string diff --git a/selfservice/strategy/code/strategy_login.go b/selfservice/strategy/code/strategy_login.go index a9d7459f5c56..391eaefee1ce 100644 --- a/selfservice/strategy/code/strategy_login.go +++ b/selfservice/strategy/code/strategy_login.go @@ -195,6 +195,8 @@ func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow, } return nil, nil case flow.StateEmailSent: + fallthrough + case flow.StateSMSSent: i, err := s.loginVerifyCode(ctx, r, f, &p) if err != nil { return nil, s.HandleLoginError(r, f, &p, err) @@ -237,9 +239,15 @@ func (s *Strategy) loginSendCode(ctx context.Context, w http.ResponseWriter, r * if err != nil { return err } + address, found := lo.Find(i.VerifiableAddresses, func(va identity.VerifiableAddress) bool { + return va.Value == p.Identifier + }) + if !found { + return errors.WithStack(schema.NewUnknownAddressError()) + } addresses = []Address{{ To: p.Identifier, - Via: identity.CodeAddressType(identity.AddressTypeEmail), + Via: address.Via, }} } @@ -319,10 +327,33 @@ func (s *Strategy) loginVerifyCode(ctx context.Context, r *http.Request, f *logi } loginCode, err := s.deps.LoginCodePersister().UseLoginCode(ctx, f.ID, i.ID, p.Code) - if err != nil { - if errors.Is(err, ErrCodeNotFound) { - return nil, schema.NewLoginCodeInvalid() + if errors.Is(err, ErrCodeNotFound) { + loginCode, err = s.deps.LoginCodePersister().UseLoginCode(ctx, f.ID, i.ID, "external") + if err != nil { + if errors.Is(err, ErrCodeNotFound) { + return nil, schema.NewLoginCodeInvalid() + } + return nil, errors.WithStack(err) } + + if !s.deps.Config().SelfServiceCodeStrategy(r.Context()).ExternalSMSVerify.Enabled || + loginCode.AddressType != identity.AddressTypePhone { + return nil, errors.WithStack(errors.New("External SMS verify disabled or unexpected address type: " + loginCode.AddressType)) + } + + id, err := s.deps.IdentityPool().GetIdentity(ctx, loginCode.IdentityID, identity.ExpandDefault) + if err != nil { + return nil, errors.WithStack(err) + } + err = s.deps.CodeSender().CheckWithExternalVerifier(r.Context(), f, id, + Address{To: loginCode.Address, Via: loginCode.AddressType}, p.Code) + if err != nil { + if errors.Is(err, ErrCodeNotFound) { + return nil, schema.NewLoginCodeInvalid() + } + return nil, errors.WithStack(err) + } + } else if err != nil { return nil, errors.WithStack(err) } diff --git a/selfservice/strategy/code/strategy_login_phone_test.go b/selfservice/strategy/code/strategy_login_phone_test.go new file mode 100644 index 000000000000..e6da57dc682f --- /dev/null +++ b/selfservice/strategy/code/strategy_login_phone_test.go @@ -0,0 +1,262 @@ +// Copyright © 2023 Ory Corp +// SPDX-License-Identifier: Apache-2.0 + +package code_test + +import ( + "context" + "encoding/json" + "fmt" + "github.com/ory/kratos/driver/config" + "github.com/ory/kratos/identity" + "github.com/ory/kratos/internal" + oryClient "github.com/ory/kratos/internal/httpclient" + "github.com/ory/kratos/session" + "github.com/ory/x/ioutilx" + "github.com/ory/x/sqlxx" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/tidwall/gjson" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/ory/kratos/internal/testhelpers" +) + +func TestLoginCodeStrategy_SMS(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()), 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"}) + + _ = testhelpers.NewLoginUIFlowEchoServer(t, reg) + _ = testhelpers.NewErrorTestServer(t, reg) + + public, _, _, _ := testhelpers.NewKratosServerWithCSRFAndRouters(t, reg) + + var externalVerifyResult string + var externalVerifyRequestBody string + initExternalSMSVerifier(t, ctx, conf, "file://./stub/request.config.login.jsonnet", + &externalVerifyRequestBody, &externalVerifyResult) + + createIdentity := func(ctx context.Context, t *testing.T) *identity.Identity { + t.Helper() + i := identity.NewIdentity(config.DefaultIdentityTraitsSchemaID) + email := testhelpers.RandomEmail() + phone := testhelpers.RandomPhone() + + i.Traits = identity.Traits(fmt.Sprintf(`{"tos": true, "email": "%s", "phone": "%s"}`, email, phone)) + + i.Credentials[identity.CredentialsTypeCodeAuth] = identity.Credentials{Type: identity.CredentialsTypeCodeAuth, Identifiers: []string{phone}, Config: sqlxx.JSONRawMessage("{\"address_type\": \"phone\", \"used_at\": \"2023-07-26T16:59:06+02:00\"}")} + + var va []identity.VerifiableAddress + va = append(va, identity.VerifiableAddress{ + Value: phone, + Via: identity.AddressTypePhone, + Verified: true, + Status: identity.VerifiableAddressStatusCompleted, + }) + + i.VerifiableAddresses = va + + require.NoError(t, reg.PrivilegedIdentityPool().CreateIdentity(ctx, i)) + return i + } + + type state struct { + flowID string + identity *identity.Identity + client *http.Client + loginCode string + identityPhone string + testServer *httptest.Server + body string + } + + type ApiType string + + const ( + ApiTypeBrowser ApiType = "browser" + ApiTypeSPA ApiType = "spa" + ApiTypeNative ApiType = "api" + ) + + createLoginFlow := func(ctx context.Context, t *testing.T, public *httptest.Server, apiType ApiType) *state { + t.Helper() + + identity := createIdentity(ctx, t) + + var client *http.Client + if apiType == ApiTypeNative { + client = &http.Client{} + } else { + client = testhelpers.NewClientWithCookies(t) + } + + client.Transport = testhelpers.NewTransportWithLogger(http.DefaultTransport, t).RoundTripper + + var clientInit *oryClient.LoginFlow + if apiType == ApiTypeNative { + clientInit = testhelpers.InitializeLoginFlowViaAPI(t, client, public, false) + } else { + clientInit = testhelpers.InitializeLoginFlowViaBrowser(t, client, public, false, apiType == ApiTypeSPA, false, false) + } + + body, err := json.Marshal(clientInit) + require.NoError(t, err) + + csrfToken := gjson.GetBytes(body, "ui.nodes.#(attributes.name==csrf_token).attributes.value").String() + if apiType == ApiTypeNative { + require.Emptyf(t, csrfToken, "csrf_token should be empty in native flows, but was found in: %s", body) + } else { + require.NotEmptyf(t, csrfToken, "could not find csrf_token in: %s", body) + } + + loginPhone := gjson.Get(identity.Traits.String(), "phone").String() + require.NotEmptyf(t, loginPhone, "could not find the phone trait inside the identity: %s", identity.Traits.String()) + + return &state{ + flowID: clientInit.GetId(), + identity: identity, + identityPhone: loginPhone, + client: client, + testServer: public, + } + } + + type onSubmitAssertion func(t *testing.T, s *state, body string, res *http.Response) + + submitLogin := func(ctx context.Context, t *testing.T, s *state, apiType ApiType, vals func(v *url.Values), mustHaveSession bool, submitAssertion onSubmitAssertion) *state { + t.Helper() + + lf, resp, err := testhelpers.NewSDKCustomClient(s.testServer, s.client).FrontendApi.GetLoginFlow(ctx).Id(s.flowID).Execute() + require.NoError(t, err) + require.EqualValues(t, http.StatusOK, resp.StatusCode) + + values := testhelpers.SDKFormFieldsToURLValues(lf.Ui.Nodes) + // we need to remove resend here + // since it is not required for the first request + // subsequent requests might need it later + values.Del("resend") + values.Set("method", "code") + vals(&values) + + body, resp := testhelpers.LoginMakeRequest(t, apiType == ApiTypeNative, apiType == ApiTypeSPA, lf, s.client, testhelpers.EncodeFormAsJSON(t, apiType == ApiTypeNative, values)) + + if submitAssertion != nil { + submitAssertion(t, s, body, resp) + return s + } + s.body = body + + if mustHaveSession { + req, err := http.NewRequest("GET", s.testServer.URL+session.RouteWhoami, nil) + require.NoError(t, err) + + if apiType == ApiTypeNative { + req.Header.Set("Authorization", "Bearer "+gjson.Get(body, "session_token").String()) + } + + resp, err = s.client.Do(req) + require.NoError(t, err) + require.EqualValues(t, http.StatusOK, resp.StatusCode, "%s", string(ioutilx.MustReadAll(resp.Body))) + 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, "%s", body) + } else { + require.EqualValues(t, http.StatusBadRequest, resp.StatusCode, "%s", body) + } + } + + return s + } + + for _, tc := range []struct { + d string + apiType ApiType + }{ + { + d: "SPA client", + apiType: ApiTypeSPA, + }, + { + d: "Browser client", + apiType: ApiTypeBrowser, + }, + { + d: "Native client", + apiType: ApiTypeNative, + }, + } { + + t.Run("test="+tc.d, func(t *testing.T) { + t.Run("case=should be able to log in with code", func(t *testing.T) { + // create login flow + s := createLoginFlow(ctx, t, public, tc.apiType) + + // submit phone + s = submitLogin(ctx, t, s, tc.apiType, func(v *url.Values) { + v.Set("identifier", s.identityPhone) + }, false, nil) + + assert.Contains(t, externalVerifyResult, "code has been sent") + + // 3. Submit OTP + submitLogin(ctx, t, s, tc.apiType, func(v *url.Values) { + v.Set("code", "0000") + }, true, nil) + + assert.Contains(t, externalVerifyResult, "code valid") + }) + + t.Run("case=should not be able to use valid code after 5 attempts", func(t *testing.T) { + s := createLoginFlow(ctx, t, public, tc.apiType) + + // submit email + s = submitLogin(ctx, t, s, tc.apiType, func(v *url.Values) { + v.Set("identifier", s.identityPhone) + }, false, nil) + + assert.Contains(t, externalVerifyResult, "code has been sent") + + for i := 0; i < 5; i++ { + // 3. Submit OTP + s = submitLogin(ctx, t, s, tc.apiType, func(v *url.Values) { + v.Set("code", "111111") + v.Set("identifier", s.identityPhone) + }, false, func(t *testing.T, s *state, body string, resp *http.Response) { + if tc.apiType == ApiTypeBrowser { + // in browser flows we redirect back to the login ui + require.Equal(t, http.StatusOK, resp.StatusCode, "%s", body) + } else { + require.EqualValues(t, http.StatusBadRequest, resp.StatusCode) + } + assert.Contains(t, gjson.Get(body, "ui.messages.0.text").String(), "The login code is invalid or has already been used") + }) + } + + // 3. Submit OTP + s = submitLogin(ctx, t, s, tc.apiType, func(v *url.Values) { + v.Set("code", "0000") + v.Set("identifier", s.identityPhone) + }, false, func(t *testing.T, s *state, body string, resp *http.Response) { + if tc.apiType == ApiTypeBrowser { + // in browser flows we redirect back to the login ui + require.Equal(t, http.StatusOK, resp.StatusCode, "%s", body) + } else { + require.EqualValues(t, http.StatusBadRequest, resp.StatusCode) + } + assert.Contains(t, gjson.Get(body, "ui.messages.0.text").String(), "The request was submitted too often.") + }) + }) + }) + } +} diff --git a/selfservice/strategy/code/strategy_registration.go b/selfservice/strategy/code/strategy_registration.go index dca3054e0c8e..13bae74fdaa5 100644 --- a/selfservice/strategy/code/strategy_registration.go +++ b/selfservice/strategy/code/strategy_registration.go @@ -7,7 +7,9 @@ import ( "context" "database/sql" "encoding/json" + "github.com/samber/lo" "net/http" + "time" "github.com/ory/herodot" "github.com/ory/x/otelx" @@ -163,8 +165,10 @@ func (s *Strategy) Register(w http.ResponseWriter, r *http.Request, f *registrat switch f.GetState() { case flow.StateChooseMethod: - return s.HandleRegistrationError(ctx, r, f, &p, s.registrationSendEmail(ctx, w, r, f, &p, i)) + return s.HandleRegistrationError(ctx, r, f, &p, s.registrationSendMessage(ctx, w, r, f, &p, i)) case flow.StateEmailSent: + fallthrough + case flow.StateSMSSent: return s.HandleRegistrationError(ctx, r, f, &p, s.registrationVerifyCode(ctx, f, &p, i)) case flow.StatePassedChallenge: return s.HandleRegistrationError(ctx, r, f, &p, errors.WithStack(schema.NewNoRegistrationStrategyResponsible())) @@ -173,8 +177,8 @@ func (s *Strategy) Register(w http.ResponseWriter, r *http.Request, f *registrat return s.HandleRegistrationError(ctx, r, f, &p, errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Unexpected flow state: %s", f.GetState()))) } -func (s *Strategy) registrationSendEmail(ctx context.Context, w http.ResponseWriter, r *http.Request, f *registration.Flow, p *updateRegistrationFlowWithCodeMethod, i *identity.Identity) (err error) { - ctx, span := s.deps.Tracer(ctx).Tracer().Start(ctx, "selfservice.strategy.code.strategy.registrationSendEmail") +func (s *Strategy) registrationSendMessage(ctx context.Context, w http.ResponseWriter, r *http.Request, f *registration.Flow, p *updateRegistrationFlowWithCodeMethod, i *identity.Identity) (err error) { + ctx, span := s.deps.Tracer(ctx).Tracer().Start(ctx, "selfservice.strategy.code.strategy.registrationSendMessage") defer otelx.End(span, &err) if len(p.Traits) == 0 { @@ -194,13 +198,18 @@ func (s *Strategy) registrationSendEmail(ctx context.Context, w http.ResponseWri return errors.WithStack(err) } - // Step 3: Get the identity email and send the code + // Step 3: Get the identity address and send the code var addresses []Address for _, identifier := range cred.Identifiers { - addresses = append(addresses, Address{To: identifier, Via: identity.AddressTypeEmail}) + address, found := lo.Find(i.VerifiableAddresses, func(va identity.VerifiableAddress) bool { + return va.Value == cred.Identifiers[0] + }) + if !found { + return errors.WithStack(schema.NewUnknownAddressError()) + } + addresses = append(addresses, Address{To: identifier, Via: address.Via}) } - // kratos only supports `email` identifiers at the moment with the code method - // this is validated in the identity validation step above + if err := s.deps.CodeSender().SendCode(ctx, f, i, addresses...); err != nil { return errors.WithStack(err) } @@ -260,10 +269,36 @@ func (s *Strategy) registrationVerifyCode(ctx context.Context, f *registration.F // Step 3: Attempt to use the code registrationCode, err := s.deps.RegistrationCodePersister().UseRegistrationCode(ctx, f.ID, p.Code, cred.Identifiers...) - if err != nil { - if errors.Is(err, ErrCodeNotFound) { - return errors.WithStack(schema.NewRegistrationCodeInvalid()) + if errors.Is(err, ErrCodeNotFound) { + registrationCode, err = s.deps.RegistrationCodePersister().UseRegistrationCode(ctx, f.ID, "external", cred.Identifiers...) + if err != nil { + if errors.Is(err, ErrCodeNotFound) { + return schema.NewRegistrationCodeInvalid() + } + return errors.WithStack(err) + } + + if !s.deps.Config().SelfServiceCodeStrategy(ctx).ExternalSMSVerify.Enabled || + registrationCode.AddressType != identity.AddressTypePhone { + return errors.WithStack(errors.New("External SMS verify disabled or unexpected address type: " + registrationCode.AddressType)) + } + + err = s.deps.CodeSender().CheckWithExternalVerifier(ctx, f, i, + Address{To: registrationCode.Address, Via: registrationCode.AddressType}, p.Code) + if err != nil { + if errors.Is(err, ErrCodeNotFound) { + return schema.NewRegistrationCodeInvalid() + } + return errors.WithStack(err) + } + registrationCode.UsedAt = sql.NullTime{ + Time: time.Now(), + Valid: true, + } + if err := s.deps.RegistrationCodePersister().UpdateRegistrationCode(ctx, registrationCode); err != nil { + return errors.WithStack(err) } + } else if err != nil { return errors.WithStack(err) } diff --git a/selfservice/strategy/code/strategy_registration_phone_test.go b/selfservice/strategy/code/strategy_registration_phone_test.go new file mode 100644 index 000000000000..4751c71f4e93 --- /dev/null +++ b/selfservice/strategy/code/strategy_registration_phone_test.go @@ -0,0 +1,311 @@ +// Copyright © 2023 Ory Corp +// SPDX-License-Identifier: Apache-2.0 + +package code_test + +import ( + "context" + _ "embed" + "encoding/json" + "fmt" + "github.com/gobuffalo/pop/v6" + "github.com/gofrs/uuid" + "github.com/nyaruka/phonenumbers" + "github.com/ory/kratos/selfservice/strategy/code" + "github.com/ory/x/randx" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/tidwall/gjson" + + "github.com/ory/kratos/driver" + "github.com/ory/kratos/driver/config" + "github.com/ory/kratos/identity" + "github.com/ory/kratos/internal" + oryClient "github.com/ory/kratos/internal/httpclient" + "github.com/ory/kratos/internal/testhelpers" +) + +func TestRegistrationCodeStrategy_SMS(t *testing.T) { + type ApiType string + + const ( + ApiTypeBrowser ApiType = "browser" + ApiTypeSPA ApiType = "spa" + ApiTypeNative ApiType = "api" + ) + + type state struct { + flowID string + client *http.Client + phone string + testServer *httptest.Server + resultIdentity *identity.Identity + } + + setup := func(ctx context.Context, t *testing.T) (*config.Config, *driver.RegistryDefault, *httptest.Server) { + conf, reg := internal.NewFastRegistryWithMocks(t) + testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/code.phone.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()), 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"}) + conf.MustSet(ctx, config.ViperKeySelfServiceRegistrationAfter+".code.hooks", []map[string]interface{}{ + {"hook": "session"}, + }) + conf.MustSet(ctx, config.ViperKeySelfServiceRegistrationEnableLegacyOneStep, true) + + _ = testhelpers.NewRegistrationUIFlowEchoServer(t, reg) + _ = testhelpers.NewErrorTestServer(t, reg) + + public, _, _, _ := testhelpers.NewKratosServerWithCSRFAndRouters(t, reg) + + return conf, reg, public + } + + ctx := context.Background() + conf, reg, public := setup(ctx, t) + + var externalVerifyResult string + var externalVerifyRequestBody string + initExternalSMSVerifier(t, ctx, conf, "file://./stub/request.config.registration.jsonnet", + &externalVerifyRequestBody, &externalVerifyResult) + + createRegistrationFlow := func(ctx context.Context, t *testing.T, public *httptest.Server, apiType ApiType) *state { + t.Helper() + + var client *http.Client + + if apiType == ApiTypeNative { + client = &http.Client{} + } else { + client = testhelpers.NewClientWithCookies(t) + } + + client.Transport = testhelpers.NewTransportWithLogger(http.DefaultTransport, t).RoundTripper + + var clientInit *oryClient.RegistrationFlow + if apiType == ApiTypeNative { + clientInit = testhelpers.InitializeRegistrationFlowViaAPI(t, client, public) + } else { + clientInit = testhelpers.InitializeRegistrationFlowViaBrowser(t, client, public, apiType == ApiTypeSPA, false, false) + } + + body, err := json.Marshal(clientInit) + require.NoError(t, err) + + csrfToken := gjson.GetBytes(body, "ui.nodes.#(attributes.name==csrf_token).attributes.value").String() + if apiType == ApiTypeNative { + require.Emptyf(t, csrfToken, "expected an empty value for csrf_token on native api flows but got %s", body) + } else { + require.NotEmpty(t, csrfToken) + } + + require.Truef(t, gjson.GetBytes(body, "ui.nodes.#(attributes.name==traits.email)").Exists(), "%s", body) + require.Truef(t, gjson.GetBytes(body, "ui.nodes.#(attributes.value==code)").Exists(), "%s", body) + + return &state{ + client: client, + flowID: clientInit.GetId(), + testServer: public, + } + } + + type onSubmitAssertion func(ctx context.Context, t *testing.T, s *state, body string, resp *http.Response) + + registerNewUser := func(ctx context.Context, t *testing.T, s *state, apiType ApiType, submitAssertion onSubmitAssertion) *state { + t.Helper() + + if s.phone == "" { + s.phone = testhelpers.RandomPhone() + } + + rf, resp, err := testhelpers.NewSDKCustomClient(s.testServer, s.client).FrontendApi.GetRegistrationFlow(context.Background()).Id(s.flowID).Execute() + require.NoError(t, err) + require.EqualValues(t, http.StatusOK, resp.StatusCode) + + values := testhelpers.SDKFormFieldsToURLValues(rf.Ui.Nodes) + values.Set("traits.phone", s.phone) + values.Set("traits.tos", "1") + values.Set("method", "code") + + body, resp := testhelpers.RegistrationMakeRequest(t, apiType == ApiTypeNative, apiType == ApiTypeSPA, rf, s.client, testhelpers.EncodeFormAsJSON(t, apiType == ApiTypeNative, values)) + + if submitAssertion != nil { + submitAssertion(ctx, t, s, body, resp) + return s + } + t.Logf("%v", body) + + if apiType == ApiTypeBrowser { + require.EqualValues(t, http.StatusOK, resp.StatusCode, "%s", body) + } else { + require.EqualValues(t, http.StatusBadRequest, resp.StatusCode, "%s", body) + } + + csrfToken := gjson.Get(body, "ui.nodes.#(attributes.name==csrf_token).attributes.value").String() + if apiType == ApiTypeNative { + assert.Emptyf(t, csrfToken, "expected an empty value for csrf_token on native api flows but got %s", body) + } else { + assert.NotEmptyf(t, csrfToken, "%s", body) + } + require.Equal(t, s.phone, gjson.Get(body, "ui.nodes.#(attributes.name==traits.phone).attributes.value").String()) + + return s + } + + submitOTP := func(ctx context.Context, t *testing.T, reg *driver.RegistryDefault, s *state, vals func(v *url.Values), apiType ApiType, submitAssertion onSubmitAssertion) *state { + t.Helper() + + rf, resp, err := testhelpers.NewSDKCustomClient(s.testServer, s.client).FrontendApi.GetRegistrationFlow(context.Background()).Id(s.flowID).Execute() + require.NoError(t, err) + require.EqualValues(t, http.StatusOK, resp.StatusCode) + + values := testhelpers.SDKFormFieldsToURLValues(rf.Ui.Nodes) + // the sdk to values always adds resend which isn't what we always need here. + // so we delete it here. + // the custom vals func can add it again if needed. + values.Del("resend") + values.Set("traits.phone", s.phone) + values.Set("traits.tos", "1") + vals(&values) + + body, resp := testhelpers.RegistrationMakeRequest(t, apiType == ApiTypeNative, apiType == ApiTypeSPA, rf, s.client, testhelpers.EncodeFormAsJSON(t, apiType == ApiTypeNative, values)) + + if submitAssertion != nil { + submitAssertion(ctx, t, s, body, resp) + return s + } + + require.Equal(t, http.StatusOK, resp.StatusCode, "%s", body) + + phoneNumber, err := phonenumbers.Parse(fmt.Sprintf("%s", s.phone), "") + require.NoError(t, err) + e164 := fmt.Sprintf("+%d%d", *phoneNumber.CountryCode, *phoneNumber.NationalNumber) + + verifiableAddress, err := reg.PrivilegedIdentityPool().FindVerifiableAddressByValue(ctx, identity.VerifiableAddressTypePhone, e164) + require.NoError(t, err) + require.Equal(t, strings.ToLower(e164), verifiableAddress.Value) + + id, err := reg.PrivilegedIdentityPool().GetIdentityConfidential(ctx, verifiableAddress.IdentityID) + require.NoError(t, err) + require.NotNil(t, id.ID) + + _, ok := id.GetCredentials(identity.CredentialsTypeCodeAuth) + require.True(t, ok) + + s.resultIdentity = id + return s + } + + for _, tc := range []struct { + d string + apiType ApiType + }{ + { + d: "SPA client", + apiType: ApiTypeSPA, + }, + { + d: "Browser client", + apiType: ApiTypeBrowser, + }, + { + d: "Native client", + apiType: ApiTypeNative, + }, + } { + t.Run("flow="+tc.d, func(t *testing.T) { + t.Run("case=should be able to register with code identity credentials", func(t *testing.T) { + ctx := context.Background() + + // 1. Initiate flow + state := createRegistrationFlow(ctx, t, public, tc.apiType) + + // 2. Submit Identifier (phone) + state = registerNewUser(ctx, t, state, tc.apiType, nil) + + assert.Contains(t, externalVerifyResult, "code has been sent") + + // 3. Submit OTP + state = submitOTP(ctx, t, reg, state, func(v *url.Values) { + v.Set("code", "0000") + }, tc.apiType, nil) + + assert.Contains(t, externalVerifyResult, "code valid") + + }) + + t.Run("case=should normalize phone address on sign up", func(t *testing.T) { + ctx := context.Background() + + // 1. Initiate flow + state := createRegistrationFlow(ctx, t, public, tc.apiType) + random := strings.ToLower(randx.MustString(9, randx.Numeric)) + sourcePhone := "+441" + random + state.phone = "+4401" + random + assert.NotEqual(t, sourcePhone, state.phone) + + // 2. Submit Identifier (email) + state = registerNewUser(ctx, t, state, tc.apiType, nil) + + // 3. Submit OTP + state = submitOTP(ctx, t, reg, state, func(v *url.Values) { + v.Set("code", "0000") + }, tc.apiType, nil) + + creds, ok := state.resultIdentity.GetCredentials(identity.CredentialsTypeCodeAuth) + require.True(t, ok) + require.Len(t, creds.Identifiers, 1) + assert.Equal(t, sourcePhone, creds.Identifiers[0]) + }) + + t.Run("case=code should not be able to use more than 5 times", func(t *testing.T) { + ctx := context.Background() + + // 1. Initiate flow + s := createRegistrationFlow(ctx, t, public, tc.apiType) + + // 2. Submit Identifier (phone) + s = registerNewUser(ctx, t, s, tc.apiType, nil) + + reg.Persister().Transaction(ctx, func(ctx context.Context, connection *pop.Connection) error { + count, err := connection.RawQuery(fmt.Sprintf("SELECT * FROM %s WHERE selfservice_registration_flow_id = ?", new(code.RegistrationCode).TableName(ctx)), uuid.FromStringOrNil(s.flowID)).Count(new(code.RegistrationCode)) + require.NoError(t, err) + require.Equal(t, 1, count) + return nil + }) + + for i := 0; i < 5; i++ { + s = submitOTP(ctx, t, reg, s, func(v *url.Values) { + v.Set("code", "1111") + }, tc.apiType, func(ctx context.Context, t *testing.T, s *state, body string, resp *http.Response) { + if tc.apiType == ApiTypeBrowser { + require.Equal(t, http.StatusOK, resp.StatusCode, "%s", body) + } else { + require.Equal(t, http.StatusBadRequest, resp.StatusCode, "%s", body) + } + require.Contains(t, gjson.Get(body, "ui.messages.0.text").String(), "The registration code is invalid or has already been used") + }) + } + + s = submitOTP(ctx, t, reg, s, func(v *url.Values) { + v.Set("code", "0000") + }, tc.apiType, func(ctx context.Context, t *testing.T, s *state, body string, resp *http.Response) { + if tc.apiType == ApiTypeBrowser { + require.Equal(t, http.StatusOK, resp.StatusCode, "%s", body) + } else { + require.Equal(t, http.StatusBadRequest, resp.StatusCode, "%s", body) + } + require.Contains(t, gjson.Get(body, "ui.messages.0.text").String(), "The request was submitted too often.") + }) + }) + }) + } +} diff --git a/selfservice/strategy/code/strategy_test.go b/selfservice/strategy/code/strategy_test.go index 4561a280fb96..52894f90e5d6 100644 --- a/selfservice/strategy/code/strategy_test.go +++ b/selfservice/strategy/code/strategy_test.go @@ -5,6 +5,11 @@ package code_test import ( "context" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -74,3 +79,50 @@ func TestMaskAddress(t *testing.T) { }) } } + +func initExternalSMSVerifier(t *testing.T, ctx context.Context, conf *config.Config, mapperFile string, + externalVerifyRequestBody *string, externalVerifyResult *string) *httptest.Server { + vs := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + rb, err := io.ReadAll(r.Body) + assert.NoError(t, err) + requestBody := string(rb) + verifyResult := "" + if strings.HasSuffix(r.URL.Path, "start") { + verifyResult = "code has been sent" + } else if strings.HasSuffix(r.URL.Path, "check") { + if strings.Contains(requestBody, "0000") { + verifyResult = "code valid" + } else { + verifyResult = "code invalid" + w.WriteHeader(http.StatusBadRequest) + } + } + *externalVerifyRequestBody = requestBody + *externalVerifyResult = verifyResult + })) + + t.Cleanup(vs.Close) + + requestConfig := `{ + "url": "%s", + "method": "POST", + "body": "%s", + "auth": { + "type": "basic_auth", + "config": { + "user": "me", + "password": "12345" + } + } + }` + verificationStartRequest := fmt.Sprintf(requestConfig, vs.URL+"/start", mapperFile) + verificationCheckRequest := fmt.Sprintf(requestConfig, vs.URL+"/check", mapperFile) + + conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+"."+string(recovery.RecoveryStrategyCode)+".external_sms_verify", fmt.Sprintf(`{ + "enabled": true, + "verification_start_request": %s, + "verification_check_request": %s + }`, verificationStartRequest, verificationCheckRequest)) + + return vs +} diff --git a/selfservice/strategy/code/strategy_verification.go b/selfservice/strategy/code/strategy_verification.go index 5b30e4c2f5ec..c2e32d198838 100644 --- a/selfservice/strategy/code/strategy_verification.go +++ b/selfservice/strategy/code/strategy_verification.go @@ -72,6 +72,9 @@ func (s *Strategy) handleVerificationError(w http.ResponseWriter, r *http.Reques f.UI.GetNodes().Upsert( node.NewInputField("email", body.Email, node.CodeGroup, node.InputAttributeTypeEmail, node.WithRequiredInputAttribute).WithMetaLabel(text.NewInfoNodeInputEmail()), ) + f.UI.GetNodes().Upsert( + node.NewInputField("phone", body.Phone, node.CodeGroup, node.InputAttributeTypeTel, node.WithRequiredInputAttribute).WithMetaLabel(text.NewInfoNodeInputPhone()), + ) } return err @@ -92,6 +95,10 @@ type updateVerificationFlowWithCodeMethod struct { // required: false Email string `form:"email" json:"email"` + // Phone to Verify + // format: tel + Phone string `form:"phone" json:"phone"` + // Sending the anti-csrf token is only required for browser login flows. CSRFToken string `form:"csrf_token" json:"csrf_token"` @@ -154,6 +161,8 @@ func (s *Strategy) Verify(w http.ResponseWriter, r *http.Request, f *verificatio case flow.StateChooseMethod: fallthrough case flow.StateEmailSent: + fallthrough + case flow.StateSMSSent: return s.verificationHandleFormSubmission(w, r, f, body) case flow.StatePassedChallenge: return s.retryVerificationFlowWithMessage(w, r, f.Type, text.NewErrorValidationVerificationRetrySuccess()) @@ -195,27 +204,45 @@ func (s *Strategy) verificationHandleFormSubmission(w http.ResponseWriter, r *ht // If not GET: try to use the submitted code return s.verificationUseCode(w, r, body.Code, f) - } else if len(body.Email) == 0 { - // If no code and no email was provided, fail with a validation error + } else if len(body.Email) == 0 && len(body.Phone) == 0 { + // If no code and no email or phone was provided, fail with a validation error return s.handleVerificationError(w, r, f, body, schema.NewRequiredError("#/email", "email")) } + if len(body.Phone) != 0 && !s.deps.Config().SelfServiceCodeStrategy(r.Context()).ExternalSMSVerify.Enabled { + return s.handleVerificationError(w, r, f, body, errors.New("External SMS verification service is disabled")) + } + if err := flow.EnsureCSRF(s.deps, r, f.Type, s.deps.Config().DisableAPIFlowEnforcement(r.Context()), s.deps.GenerateCSRFToken, body.CSRFToken); err != nil { return s.handleVerificationError(w, r, f, body, err) } + via := identity.VerifiableAddressTypeEmail + to := body.Email + if len(body.Phone) != 0 { + via = identity.VerifiableAddressTypePhone + to = body.Phone + } + if err := s.deps.VerificationCodePersister().DeleteVerificationCodesOfFlow(r.Context(), f.ID); err != nil { return s.handleVerificationError(w, r, f, body, err) } - if err := s.deps.CodeSender().SendVerificationCode(r.Context(), f, identity.VerifiableAddressTypeEmail, body.Email); err != nil { + if err := s.deps.CodeSender().SendVerificationCode(r.Context(), f, via, to); err != nil { if !errors.Is(err, ErrUnknownAddress) { return s.handleVerificationError(w, r, f, body, err) } // Continue execution } - f.State = flow.StateEmailSent + switch via { + case identity.VerifiableAddressTypeEmail: + f.State = flow.StateEmailSent + case identity.VerifiableAddressTypePhone: + f.State = flow.StateSMSSent + default: + return errors.New("Unexpected via: " + via) + } if err := s.PopulateVerificationMethod(r, f); err != nil { return s.handleVerificationError(w, r, f, body, err) @@ -236,25 +263,36 @@ func (s *Strategy) verificationHandleFormSubmission(w http.ResponseWriter, r *ht } func (s *Strategy) verificationUseCode(w http.ResponseWriter, r *http.Request, codeString string, f *verification.Flow) error { + var address *identity.VerifiableAddress code, err := s.deps.VerificationCodePersister().UseVerificationCode(r.Context(), f.ID, codeString) if errors.Is(err, ErrCodeNotFound) { - f.UI.Messages.Clear() - f.UI.Messages.Add(text.NewErrorValidationVerificationCodeInvalidOrAlreadyUsed()) - if err := s.deps.VerificationFlowPersister().UpdateVerificationFlow(r.Context(), f); err != nil { + code, err = s.deps.VerificationCodePersister().UseVerificationCode(r.Context(), f.ID, "external") + + if errors.Is(err, ErrCodeNotFound) { + return s.handleCodeNotFoundError(w, r, f) + } else if err != nil { return s.retryVerificationFlowWithError(w, r, f.Type, err) } - if x.IsBrowserRequest(r) { - http.Redirect(w, r, f.AppendTo(s.deps.Config().SelfServiceFlowVerificationUI(r.Context())).String(), http.StatusSeeOther) - } else { - s.deps.Writer().Write(w, r, f) + if !s.deps.Config().SelfServiceCodeStrategy(r.Context()).ExternalSMSVerify.Enabled || + code.VerifiableAddress.Via != identity.VerifiableAddressTypePhone { + return s.retryVerificationFlowWithError(w, r, f.Type, errors.New("External SMS verify disabled or unexpected via: "+code.VerifiableAddress.Via)) + } + + address = code.VerifiableAddress + + err = s.deps.CodeSender().VerificationCheckWithExternalVerifier(r.Context(), f, address, codeString) + if errors.Is(err, ErrCodeNotFound) { + return s.handleCodeNotFoundError(w, r, f) + } else if err != nil { + return s.retryVerificationFlowWithError(w, r, f.Type, err) } - return errors.WithStack(flow.ErrCompletedByStrategy) } else if err != nil { return s.retryVerificationFlowWithError(w, r, f.Type, err) + } else { + address = code.VerifiableAddress } - address := code.VerifiableAddress address.Verified = true verifiedAt := sqlxx.NullTime(time.Now().UTC()) address.VerifiedAt = &verifiedAt @@ -278,7 +316,11 @@ func (s *Strategy) verificationUseCode(w http.ResponseWriter, r *http.Request, c f.State = flow.StatePassedChallenge // See https://github.com/ory/kratos/issues/1547 f.SetCSRFToken(flow.GetCSRFToken(s.deps, w, r, f.Type)) - f.UI.Messages.Set(text.NewInfoSelfServiceVerificationSuccessful()) + if address.Via == identity.VerifiableAddressTypeEmail { + f.UI.Messages.Set(text.NewInfoSelfServiceVerificationSuccessful()) + } else { + f.UI.Messages.Set(text.NewInfoSelfServicePhoneVerificationSuccessful()) + } f.UI. Nodes. Append(node.NewAnchorField("continue", returnTo.String(), node.CodeGroup, text.NewInfoNodeLabelContinue()). @@ -295,6 +337,21 @@ func (s *Strategy) verificationUseCode(w http.ResponseWriter, r *http.Request, c return nil } +func (s *Strategy) handleCodeNotFoundError(w http.ResponseWriter, r *http.Request, f *verification.Flow) error { + f.UI.Messages.Clear() + f.UI.Messages.Add(text.NewErrorValidationVerificationCodeInvalidOrAlreadyUsed()) + if err := s.deps.VerificationFlowPersister().UpdateVerificationFlow(r.Context(), f); err != nil { + return s.retryVerificationFlowWithError(w, r, f.Type, err) + } + + if x.IsBrowserRequest(r) { + http.Redirect(w, r, f.AppendTo(s.deps.Config().SelfServiceFlowVerificationUI(r.Context())).String(), http.StatusSeeOther) + } else { + s.deps.Writer().Write(w, r, f) + } + return errors.WithStack(flow.ErrCompletedByStrategy) +} + func (s *Strategy) retryVerificationFlowWithMessage(w http.ResponseWriter, r *http.Request, ft flow.Type, message *text.Message) error { s.deps. Logger(). @@ -362,8 +419,13 @@ func (s *Strategy) retryVerificationFlowWithError(w http.ResponseWriter, r *http return errors.WithStack(flow.ErrCompletedByStrategy) } -func (s *Strategy) SendVerificationEmail(ctx context.Context, f *verification.Flow, i *identity.Identity, a *identity.VerifiableAddress) (err error) { - rawCode := GenerateCode() +func (s *Strategy) SendVerification(ctx context.Context, f *verification.Flow, i *identity.Identity, a *identity.VerifiableAddress) (err error) { + var rawCode string + if s.deps.Config().SelfServiceCodeStrategy(ctx).ExternalSMSVerify.Enabled && a.Via == identity.AddressTypePhone { + rawCode = "external" + } else { + rawCode = GenerateCode() + } code, err := s.deps.VerificationCodePersister().CreateVerificationCode(ctx, &CreateVerificationCodeParams{ RawCode: rawCode, diff --git a/selfservice/strategy/code/strategy_verification_phone_test.go b/selfservice/strategy/code/strategy_verification_phone_test.go new file mode 100644 index 000000000000..b9d0082994a3 --- /dev/null +++ b/selfservice/strategy/code/strategy_verification_phone_test.go @@ -0,0 +1,183 @@ +// Copyright © 2022 Ory Corp +// SPDX-License-Identifier: Apache-2.0 + +package code_test + +import ( + "context" + "encoding/json" + "github.com/ory/kratos/driver/config" + "github.com/ory/kratos/identity" + "github.com/ory/kratos/internal" + client "github.com/ory/kratos/internal/httpclient" + "github.com/ory/kratos/internal/testhelpers" + "github.com/ory/kratos/selfservice/flow/recovery" + "github.com/ory/kratos/selfservice/flow/verification" + "github.com/ory/kratos/text" + "github.com/ory/kratos/ui/node" + "github.com/ory/kratos/x" + "github.com/ory/x/assertx" + "github.com/ory/x/ioutilx" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/tidwall/gjson" + "net/http" + "net/url" + "testing" + "time" +) + +func TestPhoneVerification(t *testing.T) { + ctx := context.Background() + conf, reg := internal.NewFastRegistryWithMocks(t) + testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/default.schema.json") + initViper(t, ctx, conf) + + var externalVerifyResult string + var externalVerifyRequestBody string + initExternalSMSVerifier(t, ctx, conf, "file://./stub/request.config.verification.jsonnet", + &externalVerifyRequestBody, &externalVerifyResult) + + var identityToVerify = &identity.Identity{ + ID: x.NewUUID(), + Traits: identity.Traits(`{"phone":"+4580010000"}`), + SchemaID: config.DefaultIdentityTraitsSchemaID, + } + + var verificationPhone = gjson.GetBytes(identityToVerify.Traits, "phone").String() + + _ = testhelpers.NewVerificationUIFlowEchoServer(t, reg) + + public, _ := testhelpers.NewKratosServerWithCSRF(t, reg) + + require.NoError(t, reg.IdentityManager().Create(ctx, identityToVerify, + identity.ManagerAllowWriteProtectedTraits)) + + var expect = func(t *testing.T, hc *http.Client, isAPI, isSPA bool, values func(url.Values), c int) string { + if hc == nil { + hc = testhelpers.NewDebugClient(t) + if !isAPI { + hc = testhelpers.NewClientWithCookies(t) + hc.Transport = testhelpers.NewTransportWithLogger(http.DefaultTransport, t).RoundTripper + } + } + + return testhelpers.SubmitVerificationForm(t, isAPI, isSPA, hc, public, values, c, + testhelpers.ExpectURL(isAPI || isSPA, + public.URL+verification.RouteSubmitFlow, conf.SelfServiceFlowVerificationUI(ctx).String())) + } + + var expectSuccess = func(t *testing.T, hc *http.Client, isAPI, isSPA bool, + values func(url.Values)) string { + return expect(t, hc, isAPI, isSPA, values, http.StatusOK) + } + + submitVerificationCode := func(t *testing.T, body string, c *http.Client, code string) (string, *http.Response) { + action := gjson.Get(body, "ui.action").String() + require.NotEmpty(t, action, "%v", string(body)) + csrfToken := extractCsrfToken([]byte(body)) + + res, err := c.PostForm(action, url.Values{ + "code": {code}, + "csrf_token": {csrfToken}, + }) + require.NoError(t, err) + + return string(ioutilx.MustReadAll(res.Body)), res + } + + t.Run("description=should not be able to verify phone if external verifier is disabled", func(t *testing.T) { + conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+"."+string(recovery.RecoveryStrategyCode)+".external_sms_verify.enabled", false) + t.Cleanup(func() { + conf.MustSet(ctx, config.ViperKeySelfServiceStrategyConfig+"."+string(recovery.RecoveryStrategyCode)+".external_sms_verify.enabled", true) + }) + c := testhelpers.NewClientWithCookies(t) + f := testhelpers.SubmitVerificationForm(t, true, false, c, public, func(v url.Values) { + v.Set("phone", verificationPhone) + }, 400, "") + + assert.Contains(t, f, "External SMS verification service is disabled", "%s", f) + }) + + t.Run("description=should not be able to use an invalid code", func(t *testing.T) { + c := testhelpers.NewClientWithCookies(t) + f := testhelpers.SubmitVerificationForm(t, false, false, c, public, func(v url.Values) { + v.Set("phone", verificationPhone) + }, 200, "") + + body, res := submitVerificationCode(t, f, c, "1111") + assert.Equal(t, http.StatusOK, res.StatusCode) + + assert.Contains(t, externalVerifyResult, "code invalid") + + testhelpers.AssertMessage(t, []byte(body), "The verification code is invalid or has already been used. Please try again.") + }) + + t.Run("description=should verify phone with external verify service", func(t *testing.T) { + check := func(t *testing.T, actual string) { + assert.EqualValues(t, string(node.CodeGroup), gjson.Get(actual, "active").String(), "%s", actual) + assertx.EqualAsJSON(t, text.NewVerificationEmailWithCodeSent(), json.RawMessage(gjson.Get(actual, "ui.messages.0").Raw)) + + assert.Contains(t, externalVerifyResult, "code has been sent") + + cl := testhelpers.NewClientWithCookies(t) + + body, res := submitVerificationCode(t, actual, cl, "0000") + + assert.Equal(t, http.StatusOK, res.StatusCode) + assert.Contains(t, externalVerifyResult, "code valid") + assert.EqualValues(t, "passed_challenge", gjson.Get(body, "state").String(), "%s", body) + assert.EqualValues(t, text.NewInfoSelfServicePhoneVerificationSuccessful().Text, gjson.Get(body, "ui.messages.0.text").String()) + + id, err := reg.PrivilegedIdentityPool().GetIdentityConfidential(context.Background(), identityToVerify.ID) + require.NoError(t, err) + require.Len(t, id.VerifiableAddresses, 1) + + address := id.VerifiableAddresses[0] + assert.EqualValues(t, verificationPhone, address.Value) + assert.True(t, address.Verified) + assert.EqualValues(t, identity.VerifiableAddressStatusCompleted, address.Status) + assert.True(t, time.Time(*address.VerifiedAt).Add(time.Second*5).After(time.Now())) + } + + values := func(v url.Values) { + v.Set("phone", verificationPhone) + } + + t.Run("type=browser", func(t *testing.T) { + check(t, expectSuccess(t, nil, false, false, values)) + }) + + t.Run("type=spa", func(t *testing.T) { + check(t, expectSuccess(t, nil, false, true, values)) + }) + + t.Run("type=api", func(t *testing.T) { + check(t, expectSuccess(t, nil, true, false, values)) + }) + }) + + t.Run("description=should save transient payload to template data", func(t *testing.T) { + var doTest = func(t *testing.T, client *http.Client, isAPI bool, f *client.VerificationFlow) { + externalVerifyRequestBody = "" + expectSuccess(t, client, isAPI, false, + func(v url.Values) { + v.Set("method", "code") + v.Set("phone", verificationPhone) + v.Set("transient_payload", `{"branding": "brand-1"}`) + }) + assert.Equal(t, "code has been sent", externalVerifyResult) + assert.Contains(t, externalVerifyRequestBody, "brand-1", "%s", externalVerifyRequestBody) + } + + t.Run("type=browser", func(t *testing.T) { + c := testhelpers.NewClientWithCookies(t) + f := testhelpers.InitializeVerificationFlowViaBrowser(t, c, false, public) + doTest(t, c, false, f) + }) + t.Run("type=api", func(t *testing.T) { + f := testhelpers.InitializeVerificationFlowViaAPI(t, nil, public) + doTest(t, nil, true, f) + }) + }) +} diff --git a/selfservice/strategy/code/strategy_verification_test.go b/selfservice/strategy/code/strategy_verification_test.go index 322dc56eabd2..72bd0dc70cfa 100644 --- a/selfservice/strategy/code/strategy_verification_test.go +++ b/selfservice/strategy/code/strategy_verification_test.go @@ -118,7 +118,7 @@ func TestVerification(t *testing.T) { c := testhelpers.NewClientWithCookies(t) rs := testhelpers.GetVerificationFlow(t, c, public) - testhelpers.SnapshotTExcept(t, rs.Ui.Nodes, []string{"2.attributes.value"}) + testhelpers.SnapshotTExcept(t, rs.Ui.Nodes, []string{"3.attributes.value"}) assert.EqualValues(t, public.URL+verification.RouteSubmitFlow+"?flow="+rs.Id, rs.Ui.Action) assert.Empty(t, rs.Ui.Messages) }) diff --git a/selfservice/strategy/code/stub/code.identity.schema.json b/selfservice/strategy/code/stub/code.identity.schema.json index a7a4e4448442..b01a8942cba1 100644 --- a/selfservice/strategy/code/stub/code.identity.schema.json +++ b/selfservice/strategy/code/stub/code.identity.schema.json @@ -58,6 +58,22 @@ } } }, + "phone": { + "type": "string", + "format": "phone", + "minLength": 11, + "ory.sh/kratos": { + "credentials": { + "code": { + "identifier": true, + "via": "sms" + } + }, + "verification": { + "via": "sms" + } + } + }, "tos": { "type": "boolean", "title": "Tos", diff --git a/selfservice/strategy/code/stub/code.phone.identity.schema.json b/selfservice/strategy/code/stub/code.phone.identity.schema.json new file mode 100644 index 000000000000..c8b6e6f38823 --- /dev/null +++ b/selfservice/strategy/code/stub/code.phone.identity.schema.json @@ -0,0 +1,86 @@ +{ + "$id": "https://example.com/person.schema.json", + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "Person", + "type": "object", + "properties": { + "traits": { + "type": "object", + "properties": { + "email": { + "type": "string", + "format": "email", + "title": "Email", + "ory.sh/kratos": { + "credentials": { + "code": { + "identifier": true, + "via": "email" + }, + "password": { + "identifier": true + } + }, + "verification": { + "via": "email" + } + } + }, + "email_0": { + "type": "string", + "format": "email", + "title": "Email", + "ory.sh/kratos": { + "credentials": { + "code": { + "identifier": true, + "via": "email" + } + }, + "verification": { + "via": "email" + } + } + }, + "email_1": { + "type": "string", + "format": "email", + "title": "Email", + "ory.sh/kratos": { + "credentials": { + "code": { + "identifier": true, + "via": "email" + } + }, + "verification": { + "via": "email" + } + } + }, + "phone": { + "type": "string", + "format": "phone", + "minLength": 11, + "ory.sh/kratos": { + "credentials": { + "code": { + "identifier": true, + "via": "sms" + } + }, + "verification": { + "via": "sms" + } + } + }, + "tos": { + "type": "boolean", + "title": "Tos", + "description": "Please accept the terms and conditions" + } + }, + "required": ["phone", "tos"] + } + } +} diff --git a/selfservice/strategy/code/stub/default.schema.json b/selfservice/strategy/code/stub/default.schema.json index 8dc923266050..696d0f99f396 100644 --- a/selfservice/strategy/code/stub/default.schema.json +++ b/selfservice/strategy/code/stub/default.schema.json @@ -22,8 +22,27 @@ "via": "email" } } + }, + "phone": { + "type": "string", + "format": "phone", + "minLength": 11, + "ory.sh/kratos": { + "credentials": { + "code": { + "identifier": true, + "via": "sms" + } + }, + "verification": { + "via": "sms" + } + } } - } + }, + "required": [ + ] } - } + }, + "additionalProperties": false } diff --git a/selfservice/strategy/code/stub/request.config.login.jsonnet b/selfservice/strategy/code/stub/request.config.login.jsonnet new file mode 100644 index 000000000000..0560dcf989b0 --- /dev/null +++ b/selfservice/strategy/code/stub/request.config.login.jsonnet @@ -0,0 +1,4 @@ +function(ctx) { + code: ctx.login_code, + [if "transient_payload" in ctx then "transient_payload"]: ctx.transient_payload +} diff --git a/selfservice/strategy/code/stub/request.config.registration.jsonnet b/selfservice/strategy/code/stub/request.config.registration.jsonnet new file mode 100644 index 000000000000..1429123b0148 --- /dev/null +++ b/selfservice/strategy/code/stub/request.config.registration.jsonnet @@ -0,0 +1,4 @@ +function(ctx) { + code: ctx.registration_code, + [if "transient_payload" in ctx then "transient_payload"]: ctx.transient_payload +} diff --git a/selfservice/strategy/code/stub/request.config.verification.jsonnet b/selfservice/strategy/code/stub/request.config.verification.jsonnet new file mode 100644 index 000000000000..74c84d181e5f --- /dev/null +++ b/selfservice/strategy/code/stub/request.config.verification.jsonnet @@ -0,0 +1,4 @@ +function(ctx) { + code: ctx.verification_code, + [if "transient_payload" in ctx then "transient_payload"]: ctx.transient_payload +} diff --git a/selfservice/strategy/link/strategy_verification.go b/selfservice/strategy/link/strategy_verification.go index a2a72ea9a277..def0ace9deec 100644 --- a/selfservice/strategy/link/strategy_verification.go +++ b/selfservice/strategy/link/strategy_verification.go @@ -317,7 +317,7 @@ func (s *Strategy) retryVerificationFlowWithError(w http.ResponseWriter, r *http return errors.WithStack(flow.ErrCompletedByStrategy) } -func (s *Strategy) SendVerificationEmail(ctx context.Context, f *verification.Flow, i *identity.Identity, a *identity.VerifiableAddress) error { +func (s *Strategy) SendVerification(ctx context.Context, f *verification.Flow, i *identity.Identity, a *identity.VerifiableAddress) error { token := NewSelfServiceVerificationToken(a, f, s.d.Config().SelfServiceLinkMethodLifespan(ctx)) if err := s.d.VerificationTokenPersister().CreateVerificationToken(ctx, token); err != nil { return err diff --git a/session/manager_http_test.go b/session/manager_http_test.go index 8a1e166da25c..af33d8f73ed5 100644 --- a/session/manager_http_test.go +++ b/session/manager_http_test.go @@ -587,6 +587,9 @@ func TestDoesSessionSatisfy(t *testing.T) { t.Run(fmt.Sprintf("run=%d/desc=%s", k, tc.d), func(t *testing.T) { id := identity.NewIdentity("") for _, c := range tc.creds { + if c.Type == identity.CredentialsTypePassword { + id.Traits = []byte(`{"email":"` + c.Identifiers[0] + `"}`) + } id.SetCredentials(c.Type, c) } require.NoError(t, reg.IdentityManager().Create(context.Background(), id, identity.ManagerAllowWriteProtectedTraits)) diff --git a/text/id.go b/text/id.go index a466caec0f8c..63ee2cbd8b51 100644 --- a/text/id.go +++ b/text/id.go @@ -86,21 +86,22 @@ const ( ) const ( - InfoNodeLabel ID = 1070000 + iota // 1070000 - InfoNodeLabelInputPassword // 1070001 - InfoNodeLabelGenerated // 1070002 - InfoNodeLabelSave // 1070003 - InfoNodeLabelID // 1070004 - InfoNodeLabelSubmit // 1070005 - InfoNodeLabelVerifyOTP // 1070006 - InfoNodeLabelEmail // 1070007 - InfoNodeLabelResendOTP // 1070008 - InfoNodeLabelContinue // 1070009 - InfoNodeLabelRecoveryCode // 1070010 - InfoNodeLabelVerificationCode // 1070011 - InfoNodeLabelRegistrationCode // 1070012 - InfoNodeLabelLoginCode // 1070013 - InfoNodeLabelLoginAndLinkCredential + InfoNodeLabel ID = 1070000 + iota // 1070000 + InfoNodeLabelInputPassword // 1070001 + InfoNodeLabelGenerated // 1070002 + InfoNodeLabelSave // 1070003 + InfoNodeLabelID // 1070004 + InfoNodeLabelSubmit // 1070005 + InfoNodeLabelVerifyOTP // 1070006 + InfoNodeLabelEmail // 1070007 + InfoNodeLabelResendOTP // 1070008 + InfoNodeLabelContinue // 1070009 + InfoNodeLabelRecoveryCode // 1070010 + InfoNodeLabelVerificationCode // 1070011 + InfoNodeLabelRegistrationCode // 1070012 + InfoNodeLabelLoginCode // 1070013 + InfoNodeLabelLoginAndLinkCredential // 1070014 + InfoNodeLabelPhone // 1070015 ) const ( @@ -108,6 +109,7 @@ const ( InfoSelfServiceVerificationEmailSent // 1080001 InfoSelfServiceVerificationSuccessful // 1080002 InfoSelfServiceVerificationEmailWithCodeSent // 1080003 + InfoSelfServicePhoneVerificationSuccessful // 1080004 ) const ( diff --git a/text/message_node.go b/text/message_node.go index e2dfb7d6dc32..a66d8a0cb49c 100644 --- a/text/message_node.go +++ b/text/message_node.go @@ -117,3 +117,11 @@ func NewInfoNodeLoginAndLinkCredential() *Message { Type: Info, } } + +func NewInfoNodeInputPhone() *Message { + return &Message{ + ID: InfoNodeLabelPhone, + Text: "Phone", + Type: Info, + } +} diff --git a/text/message_verification.go b/text/message_verification.go index 37064e2d0f6c..820a04ececd5 100644 --- a/text/message_verification.go +++ b/text/message_verification.go @@ -28,6 +28,14 @@ func NewInfoSelfServiceVerificationSuccessful() *Message { } } +func NewInfoSelfServicePhoneVerificationSuccessful() *Message { + return &Message{ + ID: InfoSelfServicePhoneVerificationSuccessful, + Type: Info, + Text: "You successfully verified your phone number.", + } +} + func NewVerificationEmailSent() *Message { return &Message{ ID: InfoSelfServiceVerificationEmailSent, From f13a642fc3543454f62fe82b77004848545c24a6 Mon Sep 17 00:00:00 2001 From: maoanran Date: Fri, 6 Sep 2024 10:41:47 +0200 Subject: [PATCH 2/3] feat: do not retry on 429 responses from external verification service (PS-405) --- driver/registry_default.go | 21 +- driver/registry_default_test.go | 28 +++ go.mod | 3 +- .../code/code_external_verifier_test.go | 210 ++++++++++++++++++ .../code/stub/code.verification.check.jsonnet | 4 + .../code/stub/code.verification.start.jsonnet | 22 ++ test/e2e/profiles/code/.kratos.yml | 22 ++ 7 files changed, 308 insertions(+), 2 deletions(-) create mode 100644 selfservice/strategy/code/code_external_verifier_test.go create mode 100644 selfservice/strategy/code/stub/code.verification.check.jsonnet create mode 100644 selfservice/strategy/code/stub/code.verification.start.jsonnet diff --git a/driver/registry_default.go b/driver/registry_default.go index d9cef26b5739..3aae4026330f 100644 --- a/driver/registry_default.go +++ b/driver/registry_default.go @@ -846,7 +846,26 @@ func (m *RegistryDefault) HTTPClient(_ context.Context, opts ...httpx.ResilientO httpx.ResilientClientAllowInternalIPRequestsTo(m.Config().ClientHTTPPrivateIPExceptionURLs(contextx.RootContext)...), ) } - return httpx.NewResilientClient(opts...) + client := httpx.NewResilientClient(opts...) + client.CheckRetry = NoRetryOnRateLimitPolicy + return client +} + +func NoRetryOnRateLimitPolicy(ctx context.Context, resp *http.Response, err error) (bool, error) { + // If there's no response (network error), retry + if resp == nil { + return true, nil + } + + // Do not retry on 4xx errors, except 408 (Request Timeout) + if resp.StatusCode == http.StatusRequestTimeout { + return true, nil + } else if resp.StatusCode >= 400 && resp.StatusCode < 500 { + return false, nil + } + + // Default retry policy will retry on 5xx errors or network errors + return retryablehttp.DefaultRetryPolicy(ctx, resp, err) } func (m *RegistryDefault) WithContextualizer(ctxer contextx.Contextualizer) Registry { diff --git a/driver/registry_default_test.go b/driver/registry_default_test.go index 009dd76173d8..7b99d5ba232b 100644 --- a/driver/registry_default_test.go +++ b/driver/registry_default_test.go @@ -7,6 +7,7 @@ import ( "context" "encoding/json" "fmt" + "net/http" "os" "testing" @@ -975,3 +976,30 @@ func TestGetActiveVerificationStrategy(t *testing.T) { } }) } + +func TestNoRetryOnRateLimitPolicy(t *testing.T) { + t.Run("case=does not retry on 4xx errors except 408", func(t *testing.T) { + resp := &http.Response{StatusCode: 429} + retry, err := driver.NoRetryOnRateLimitPolicy(context.Background(), resp, nil) + assert.False(t, retry) + assert.NoError(t, err) + + resp.StatusCode = 408 + retry, err = driver.NoRetryOnRateLimitPolicy(context.Background(), resp, nil) + assert.True(t, retry) + assert.NoError(t, err) + }) + + t.Run("case=retries on 5xx errors", func(t *testing.T) { + resp := &http.Response{StatusCode: 500} + retry, err := driver.NoRetryOnRateLimitPolicy(context.Background(), resp, nil) + assert.True(t, retry) + assert.NoError(t, err) + }) + + t.Run("case=retries on network errors", func(t *testing.T) { + retry, err := driver.NoRetryOnRateLimitPolicy(context.Background(), nil, assert.AnError) + assert.True(t, retry) + assert.NoError(t, err) + }) +} diff --git a/go.mod b/go.mod index b15b91816bf4..d8f5e8ce5ca7 100644 --- a/go.mod +++ b/go.mod @@ -253,7 +253,7 @@ require ( github.com/mitchellh/mapstructure v1.5.0 // indirect github.com/mitchellh/reflectwalk v1.0.2 // indirect github.com/moby/term v0.0.0-20220808134915-39b0c02b01ae // indirect - github.com/nyaruka/phonenumbers v1.3.6 // indirect + github.com/nyaruka/phonenumbers v1.3.6 github.com/ogier/pflag v0.0.1 // indirect github.com/oklog/ulid v1.3.1 // indirect github.com/opencontainers/go-digest v1.0.0 // indirect @@ -338,5 +338,6 @@ require ( github.com/jackc/puddle/v2 v2.1.2 // indirect github.com/lestrrat-go/httprc v1.0.4 // indirect github.com/segmentio/asm v1.2.0 // indirect + github.com/stretchr/objx v0.5.2 // indirect go.uber.org/atomic v1.10.0 // indirect ) diff --git a/selfservice/strategy/code/code_external_verifier_test.go b/selfservice/strategy/code/code_external_verifier_test.go new file mode 100644 index 000000000000..897828466d96 --- /dev/null +++ b/selfservice/strategy/code/code_external_verifier_test.go @@ -0,0 +1,210 @@ +package code_test + +import ( + "bytes" + "context" + "github.com/hashicorp/go-retryablehttp" + "github.com/ory/kratos/courier/template" + "github.com/ory/kratos/driver" + "github.com/ory/kratos/driver/config" + "github.com/ory/kratos/selfservice/strategy/code" + "github.com/ory/x/configx" + "github.com/ory/x/httpx" + "github.com/ory/x/jsonnetsecure" + "github.com/ory/x/logrusx" + "github.com/ory/x/otelx" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + "io" + "net/http" + "os" + "testing" +) + +type MockDependencies struct { + mock.Mock + t *testing.T + mockHTTPClient *retryablehttp.Client +} + +func NewMockDependencies(t *testing.T, mockHTTPClient *retryablehttp.Client) *MockDependencies { + return &MockDependencies{t: t, mockHTTPClient: mockHTTPClient} +} + +func (m *MockDependencies) Config() *config.Config { + return config.MustNew( + m.t, + logrusx.New("kratos", "test"), + os.Stderr, + configx.WithConfigFiles("../../../test/e2e/profiles/code/.kratos.yml"), + configx.SkipValidation(), + ) +} + +func (m *MockDependencies) Logger() *logrusx.Logger { + return logrusx.New("kratos", "test") +} + +func (m *MockDependencies) Audit() *logrusx.Logger { + return logrusx.New("kratos", "test") +} + +func (m *MockDependencies) Tracer(ctx context.Context) *otelx.Tracer { + return otelx.NewNoop(nil, nil) +} + +func (m *MockDependencies) HTTPClient(ctx context.Context, options ...httpx.ResilientOptions) *retryablehttp.Client { + return m.mockHTTPClient +} + +func (m *MockDependencies) JsonnetVM(ctx context.Context) (jsonnetsecure.VM, error) { + return jsonnetsecure.NewTestProvider(m.t).JsonnetVM(ctx) +} + +type MockSMSTemplate struct { + mock.Mock + marshalJson string +} + +func (m *MockSMSTemplate) MarshalJSON() ([]byte, error) { + return []byte(m.marshalJson), nil +} + +func (m *MockSMSTemplate) SMSBody(ctx context.Context) (string, error) { + return "sms body", nil +} + +func (m *MockSMSTemplate) TemplateType() template.TemplateType { + return "sms" +} + +func (m *MockSMSTemplate) PhoneNumber() (string, error) { + return "1234567890", nil +} + +func TestVerificationStart(t *testing.T) { + ctx := context.Background() + + mockHTTPClient := new(retryablehttp.Client) + mockHTTPClient.CheckRetry = driver.NoRetryOnRateLimitPolicy + mockDeps := NewMockDependencies(t, mockHTTPClient) + + mockSMSTemplate := new(MockSMSTemplate) + mockSMSTemplate.marshalJson = `{"To":"12345678"}` + + externalVerifier := code.NewExternalVerifier(mockDeps) + + t.Run("method=VerificationStart", func(t *testing.T) { + t.Run("case=returns no error for 2xx response", func(t *testing.T) { + mockHTTPClient.HTTPClient = &http.Client{ + Transport: &mockTransport{ + response: &http.Response{ + StatusCode: 200, + Body: io.NopCloser(bytes.NewReader([]byte("OK"))), + }, + }, + } + + err := externalVerifier.VerificationStart(ctx, mockSMSTemplate) + require.NoError(t, err) + }) + + t.Run("case=returns error for 4xx response", func(t *testing.T) { + mockHTTPClient.HTTPClient = &http.Client{ + Transport: &mockTransport{ + response: &http.Response{ + StatusCode: 400, + Body: io.NopCloser(bytes.NewReader([]byte("Bad Request"))), + }, + }, + } + + err := externalVerifier.VerificationStart(ctx, mockSMSTemplate) + require.Error(t, err) + assert.Contains(t, err.Error(), "upstream server replied with status code 400 and body Bad Request") + }) + + t.Run("case=returns error for 5xx response", func(t *testing.T) { + mockHTTPClient.HTTPClient = &http.Client{ + Transport: &mockTransport{ + response: &http.Response{ + StatusCode: 500, + Body: io.NopCloser(bytes.NewReader([]byte("Internal Server Error"))), + }, + }, + } + + err := externalVerifier.VerificationStart(ctx, mockSMSTemplate) + require.Error(t, err) + assert.Contains(t, err.Error(), "giving up after 1 attempt(s)") + }) + }) +} + +func TestVerificationCheck(t *testing.T) { + ctx := context.Background() + + mockHTTPClient := new(retryablehttp.Client) + mockHTTPClient.CheckRetry = driver.NoRetryOnRateLimitPolicy + mockDeps := NewMockDependencies(t, mockHTTPClient) + + mockSMSTemplate := new(MockSMSTemplate) + mockSMSTemplate.marshalJson = `{"To":"12345678", "Code":"1234"}` + + externalVerifier := code.NewExternalVerifier(mockDeps) + + t.Run("method=VerificationCheck", func(t *testing.T) { + t.Run("case=returns no error for 2xx response", func(t *testing.T) { + mockHTTPClient.HTTPClient = &http.Client{ + Transport: &mockTransport{ + response: &http.Response{ + StatusCode: 200, + Body: io.NopCloser(bytes.NewReader([]byte("OK"))), + }, + }, + } + + err := externalVerifier.VerificationCheck(ctx, mockSMSTemplate) + require.NoError(t, err) + }) + + t.Run("case=returns error for 4xx response", func(t *testing.T) { + mockHTTPClient.HTTPClient = &http.Client{ + Transport: &mockTransport{ + response: &http.Response{ + StatusCode: 400, + Body: io.NopCloser(bytes.NewReader([]byte("Bad Request"))), + }, + }, + } + + err := externalVerifier.VerificationCheck(ctx, mockSMSTemplate) + require.Error(t, err) + assert.Contains(t, err.Error(), "The requested resource could not be found") + }) + + t.Run("case=returns error for 5xx response", func(t *testing.T) { + mockHTTPClient.HTTPClient = &http.Client{ + Transport: &mockTransport{ + response: &http.Response{ + StatusCode: 500, + Body: io.NopCloser(bytes.NewReader([]byte("Internal Server Error"))), + }, + }, + } + + err := externalVerifier.VerificationCheck(ctx, mockSMSTemplate) + require.Error(t, err) + assert.Contains(t, err.Error(), "giving up after 1 attempt(s)") + }) + }) +} + +type mockTransport struct { + response *http.Response +} + +func (m *mockTransport) RoundTrip(req *http.Request) (*http.Response, error) { + return m.response, nil +} diff --git a/selfservice/strategy/code/stub/code.verification.check.jsonnet b/selfservice/strategy/code/stub/code.verification.check.jsonnet new file mode 100644 index 000000000000..2be2e9f4e43d --- /dev/null +++ b/selfservice/strategy/code/stub/code.verification.check.jsonnet @@ -0,0 +1,4 @@ +function(ctx) { + to: ctx.To, + code: if std.objectHas(ctx, "VerificationCode") then ctx.VerificationCode else ctx.Code, +} diff --git a/selfservice/strategy/code/stub/code.verification.start.jsonnet b/selfservice/strategy/code/stub/code.verification.start.jsonnet new file mode 100644 index 000000000000..d47fd569b7a8 --- /dev/null +++ b/selfservice/strategy/code/stub/code.verification.start.jsonnet @@ -0,0 +1,22 @@ +local getFlowId(ctx) = + if std.objectHas(ctx, "VerificationURL") then + local start = std.findSubstr("flow=", ctx.VerificationURL); + std.substr(ctx.VerificationURL, start[0]+5, 36) + else + "error_getting_flow_id"; + +local getOperator(ctx) = + if std.objectHas(ctx, "TransientPayload") && std.objectHas(ctx.TransientPayload, "application") then ctx.TransientPayload.application + else "monta"; + +function(ctx) { + to: ctx.To, + [if std.objectHas(ctx, "TransientPayload") && std.objectHas(ctx.TransientPayload, "language") then 'language']: ctx.TransientPayload.language, + [if std.objectHas(ctx, "TransientPayload") && std.objectHas(ctx.TransientPayload, "application") then 'application']: ctx.TransientPayload.application, + template: if std.objectHas(ctx, "VerificationCode") then 'sms_localisation.verification_code_with_link' else 'sms_localisation.account_activation_passcode', + templateParameters: { + "host": "portal.monta.app", + "flow_id": getFlowId(ctx), + "operator": getOperator(ctx), + }, +} diff --git a/test/e2e/profiles/code/.kratos.yml b/test/e2e/profiles/code/.kratos.yml index 3e98857e1628..437f399a5881 100644 --- a/test/e2e/profiles/code/.kratos.yml +++ b/test/e2e/profiles/code/.kratos.yml @@ -42,6 +42,28 @@ selfservice: enabled: true config: lifespan: 1h + external_sms_verify: + enabled: true + verification_start_request: + url: http://notification:8080/api/verifications + method: POST + body: file://stub/code.verification.start.jsonnet + auth: + type: api_key + config: + name: Authorization + value: ... + in: header + verification_check_request: + url: http://notification:8080/api/verifications/check + method: POST + body: file://stub/code.verification.check.jsonnet + auth: + type: api_key + config: + name: Authorization + value: ... + in: header identity: schemas: From 6a60bf9288fb59693cfde35f37de5e5705b00f32 Mon Sep 17 00:00:00 2001 From: splaunov Date: Mon, 25 Nov 2024 19:45:36 +0300 Subject: [PATCH 3/3] feat: in code login flow send identity not found sms (PS-572) --- .../login_code/invalid/email.body.gotmpl | 3 + .../invalid/email.body.plaintext.gotmpl | 3 + .../login_code/invalid/email.subject.gotmpl | 1 + .../login_code/invalid/sms.body.gotmpl | 1 + courier/template/email/login_code_invalid.go | 54 ++++ .../template/email/login_code_invalid_test.go | 23 ++ courier/template/sms/login_code_invalid.go | 52 ++++ .../template/sms/login_code_invalid_test.go | 34 +++ courier/template/testhelpers/testhelpers.go | 9 + courier/template/type.go | 1 + driver/config/config.go | 20 +- embedx/config.schema.json | 6 + selfservice/strategy/code/code_sender.go | 29 ++- selfservice/strategy/code/code_sender_test.go | 26 ++ selfservice/strategy/code/strategy_login.go | 41 ++- .../code/strategy_login_phone_test.go | 233 ++++++++++++------ 16 files changed, 439 insertions(+), 97 deletions(-) create mode 100644 courier/template/courier/builtin/templates/login_code/invalid/email.body.gotmpl create mode 100644 courier/template/courier/builtin/templates/login_code/invalid/email.body.plaintext.gotmpl create mode 100644 courier/template/courier/builtin/templates/login_code/invalid/email.subject.gotmpl create mode 100644 courier/template/courier/builtin/templates/login_code/invalid/sms.body.gotmpl create mode 100644 courier/template/email/login_code_invalid.go create mode 100644 courier/template/email/login_code_invalid_test.go create mode 100644 courier/template/sms/login_code_invalid.go create mode 100644 courier/template/sms/login_code_invalid_test.go diff --git a/courier/template/courier/builtin/templates/login_code/invalid/email.body.gotmpl b/courier/template/courier/builtin/templates/login_code/invalid/email.body.gotmpl new file mode 100644 index 000000000000..2dc41af9c88a --- /dev/null +++ b/courier/template/courier/builtin/templates/login_code/invalid/email.body.gotmpl @@ -0,0 +1,3 @@ +You requested a verification code, but we couldn’t find an account linked to this email address. +Try a different way to log in, or create an account if you don’t have one already. +If you didn’t request a code, you can ignore this message. diff --git a/courier/template/courier/builtin/templates/login_code/invalid/email.body.plaintext.gotmpl b/courier/template/courier/builtin/templates/login_code/invalid/email.body.plaintext.gotmpl new file mode 100644 index 000000000000..2dc41af9c88a --- /dev/null +++ b/courier/template/courier/builtin/templates/login_code/invalid/email.body.plaintext.gotmpl @@ -0,0 +1,3 @@ +You requested a verification code, but we couldn’t find an account linked to this email address. +Try a different way to log in, or create an account if you don’t have one already. +If you didn’t request a code, you can ignore this message. diff --git a/courier/template/courier/builtin/templates/login_code/invalid/email.subject.gotmpl b/courier/template/courier/builtin/templates/login_code/invalid/email.subject.gotmpl new file mode 100644 index 000000000000..991847c068db --- /dev/null +++ b/courier/template/courier/builtin/templates/login_code/invalid/email.subject.gotmpl @@ -0,0 +1 @@ +Someone tried to login using this email address diff --git a/courier/template/courier/builtin/templates/login_code/invalid/sms.body.gotmpl b/courier/template/courier/builtin/templates/login_code/invalid/sms.body.gotmpl new file mode 100644 index 000000000000..cfda76242d15 --- /dev/null +++ b/courier/template/courier/builtin/templates/login_code/invalid/sms.body.gotmpl @@ -0,0 +1 @@ +You requested a verification code, but we couldn’t find an account linked to this phone number. Try a different way to log in, or create an account if you don’t have one already. If you didn’t request a code, you can ignore this message. diff --git a/courier/template/email/login_code_invalid.go b/courier/template/email/login_code_invalid.go new file mode 100644 index 000000000000..13c2647e34c6 --- /dev/null +++ b/courier/template/email/login_code_invalid.go @@ -0,0 +1,54 @@ +// Copyright © 2023 Ory Corp +// SPDX-License-Identifier: Apache-2.0 + +package email + +import ( + "context" + "encoding/json" + "github.com/ory/kratos/courier/template" + "os" + "strings" +) + +type ( + LoginCodeInvalid struct { + d template.Dependencies + m *LoginCodeInvalidModel + } + LoginCodeInvalidModel struct { + To string `json:"to"` + RequestURL string `json:"request_url"` + TransientPayload map[string]interface{} `json:"transient_payload"` + } +) + +func NewLoginCodeInvalid(d template.Dependencies, m *LoginCodeInvalidModel) *LoginCodeInvalid { + return &LoginCodeInvalid{d: d, m: m} +} + +func (t *LoginCodeInvalid) EmailRecipient() (string, error) { + return t.m.To, nil +} + +func (t *LoginCodeInvalid) EmailSubject(ctx context.Context) (string, error) { + subject, err := template.LoadText(ctx, t.d, os.DirFS(t.d.CourierConfig().CourierTemplatesRoot(ctx)), "login_code/invalid/email.subject.gotmpl", "login_code/invalid/email.subject*", t.m, t.d.CourierConfig().CourierTemplatesLoginCodeInvalid(ctx).Subject) + + return strings.TrimSpace(subject), err +} + +func (t *LoginCodeInvalid) EmailBody(ctx context.Context) (string, error) { + return template.LoadHTML(ctx, t.d, os.DirFS(t.d.CourierConfig().CourierTemplatesRoot(ctx)), "login_code/invalid/email.body.gotmpl", "login_code/invalid/email.body*", t.m, t.d.CourierConfig().CourierTemplatesLoginCodeInvalid(ctx).Body.HTML) +} + +func (t *LoginCodeInvalid) EmailBodyPlaintext(ctx context.Context) (string, error) { + return template.LoadText(ctx, t.d, os.DirFS(t.d.CourierConfig().CourierTemplatesRoot(ctx)), "login_code/invalid/email.body.plaintext.gotmpl", "login_code/invalid/email.body.plaintext*", t.m, t.d.CourierConfig().CourierTemplatesLoginCodeInvalid(ctx).Body.PlainText) +} + +func (t *LoginCodeInvalid) MarshalJSON() ([]byte, error) { + return json.Marshal(t.m) +} + +func (t *LoginCodeInvalid) TemplateType() template.TemplateType { + return template.TypeLoginCodeInvalid +} diff --git a/courier/template/email/login_code_invalid_test.go b/courier/template/email/login_code_invalid_test.go new file mode 100644 index 000000000000..80cc955ba566 --- /dev/null +++ b/courier/template/email/login_code_invalid_test.go @@ -0,0 +1,23 @@ +// Copyright © 2023 Ory Corp +// SPDX-License-Identifier: Apache-2.0 + +package email_test + +import ( + "context" + "github.com/ory/kratos/courier/template/email" + "github.com/ory/kratos/courier/template/testhelpers" + "testing" + + "github.com/ory/kratos/internal" +) + +func TestNewLoginCodeInvalid(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + _, reg := internal.NewFastRegistryWithMocks(t) + + tpl := email.NewLoginCodeInvalid(reg, &email.LoginCodeInvalidModel{}) + + testhelpers.TestRendered(t, ctx, tpl) +} diff --git a/courier/template/sms/login_code_invalid.go b/courier/template/sms/login_code_invalid.go new file mode 100644 index 000000000000..c9bc95a80074 --- /dev/null +++ b/courier/template/sms/login_code_invalid.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 ( + LoginCodeInvalid struct { + deps template.Dependencies + model *LoginCodeInvalidModel + } + LoginCodeInvalidModel struct { + To string `json:"to"` + RequestURL string `json:"request_url"` + TransientPayload map[string]interface{} `json:"transient_payload"` + } +) + +func NewLoginCodeInvalid(d template.Dependencies, m *LoginCodeInvalidModel) *LoginCodeInvalid { + return &LoginCodeInvalid{deps: d, model: m} +} + +func (t *LoginCodeInvalid) PhoneNumber() (string, error) { + return t.model.To, nil +} + +func (t *LoginCodeInvalid) SMSBody(ctx context.Context) (string, error) { + return template.LoadText( + ctx, + t.deps, + os.DirFS(t.deps.CourierConfig().CourierTemplatesRoot(ctx)), + "login_code/invalid/sms.body.gotmpl", + "login_code/invalid/sms.body*", + t.model, + t.deps.CourierConfig().CourierSMSTemplatesLoginCodeInvalid(ctx).Body.PlainText, + ) +} + +func (t *LoginCodeInvalid) MarshalJSON() ([]byte, error) { + return json.Marshal(t.model) +} + +func (t *LoginCodeInvalid) TemplateType() template.TemplateType { + return template.TypeLoginCodeInvalid +} diff --git a/courier/template/sms/login_code_invalid_test.go b/courier/template/sms/login_code_invalid_test.go new file mode 100644 index 000000000000..c93c942a1703 --- /dev/null +++ b/courier/template/sms/login_code_invalid_test.go @@ -0,0 +1,34 @@ +// Copyright © 2023 Ory Corp +// SPDX-License-Identifier: Apache-2.0 + +package sms_test + +import ( + "context" + "github.com/ory/kratos/courier/template/testhelpers" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/ory/kratos/courier/template/sms" + "github.com/ory/kratos/internal" +) + +func TestNewLoginCodeInvalid(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + _, reg := internal.NewFastRegistryWithMocks(t) + + const ( + expectedPhone = "+12345678901" + ) + + tpl := sms.NewLoginCodeInvalid(reg, &sms.LoginCodeInvalidModel{To: expectedPhone}) + + testhelpers.TestSMSRendered(t, ctx, tpl) + + actualPhone, err := tpl.PhoneNumber() + require.NoError(t, err) + assert.Equal(t, expectedPhone, actualPhone) +} diff --git a/courier/template/testhelpers/testhelpers.go b/courier/template/testhelpers/testhelpers.go index 66d2f15f6f4b..bbd411408ef7 100644 --- a/courier/template/testhelpers/testhelpers.go +++ b/courier/template/testhelpers/testhelpers.go @@ -36,6 +36,15 @@ func SetupRemoteConfig(t *testing.T, ctx context.Context, plaintext string, html return reg } +func TestSMSRendered(t *testing.T, ctx context.Context, tpl interface { + SMSBody(context.Context) (string, error) +}, +) { + rendered, err := tpl.SMSBody(ctx) + require.NoError(t, err) + assert.NotEmpty(t, rendered) +} + func TestRendered(t *testing.T, ctx context.Context, tpl interface { EmailBody(context.Context) (string, error) EmailSubject(context.Context) (string, error) diff --git a/courier/template/type.go b/courier/template/type.go index 4fc0b9bccca2..ff34101f7905 100644 --- a/courier/template/type.go +++ b/courier/template/type.go @@ -18,6 +18,7 @@ const ( TypeVerificationCodeInvalid TemplateType = "verification_code_invalid" TypeVerificationCodeValid TemplateType = "verification_code_valid" TypeTestStub TemplateType = "stub" + TypeLoginCodeInvalid TemplateType = "login_code_invalid" TypeLoginCodeValid TemplateType = "login_code_valid" TypeRegistrationCodeValid TemplateType = "registration_code_valid" ) diff --git a/driver/config/config.go b/driver/config/config.go index 59efd8dbd6b5..48dd12d532d5 100644 --- a/driver/config/config.go +++ b/driver/config/config.go @@ -68,10 +68,12 @@ const ( ViperKeyCourierTemplatesVerificationCodeInvalidEmail = "courier.templates.verification_code.invalid.email" ViperKeyCourierTemplatesVerificationCodeValidEmail = "courier.templates.verification_code.valid.email" ViperKeyCourierTemplatesVerificationCodeValidSMS = "courier.templates.verification_code.valid.sms" + ViperKeyCourierTemplatesLoginCodeInvalidSMS = "courier.templates.login_code.invalid.sms" ViperKeyCourierTemplatesLoginCodeValidSMS = "courier.templates.login_code.valid.sms" ViperKeyCourierTemplatesRegistrationCodeValidSMS = "courier.templates.registration_code.valid.sms" ViperKeyCourierDeliveryStrategy = "courier.delivery_strategy" ViperKeyCourierHTTPRequestConfig = "courier.http.request_config" + ViperKeyCourierTemplatesLoginCodeInvalidEmail = "courier.templates.login_code.invalid.email" ViperKeyCourierTemplatesLoginCodeValidEmail = "courier.templates.login_code.valid.email" ViperKeyCourierTemplatesRegistrationCodeValidEmail = "courier.templates.registration_code.valid.email" ViperKeyCourierSMTP = "courier.smtp" @@ -250,9 +252,10 @@ type ( SelfServiceStrategyCode struct { *SelfServiceStrategy - PasswordlessEnabled bool `json:"passwordless_enabled"` - MFAEnabled bool `json:"mfa_enabled"` - ExternalSMSVerify *ExternalSMSVerify `json:"external_sms_verify"` + PasswordlessEnabled bool `json:"passwordless_enabled"` + MFAEnabled bool `json:"mfa_enabled"` + ExternalSMSVerify *ExternalSMSVerify `json:"external_sms_verify"` + NotifyUnknownRecipients bool `json:"notify_unknown_recipients"` } Schema struct { ID string `json:"id" koanf:"id"` @@ -317,9 +320,11 @@ type ( CourierTemplatesRecoveryCodeValid(ctx context.Context) *CourierEmailTemplate CourierTemplatesVerificationCodeInvalid(ctx context.Context) *CourierEmailTemplate CourierTemplatesVerificationCodeValid(ctx context.Context) *CourierEmailTemplate + CourierTemplatesLoginCodeInvalid(ctx context.Context) *CourierEmailTemplate CourierTemplatesLoginCodeValid(ctx context.Context) *CourierEmailTemplate CourierTemplatesRegistrationCodeValid(ctx context.Context) *CourierEmailTemplate CourierSMSTemplatesVerificationCodeValid(ctx context.Context) *CourierSMSTemplate + CourierSMSTemplatesLoginCodeInvalid(ctx context.Context) *CourierSMSTemplate CourierSMSTemplatesLoginCodeValid(ctx context.Context) *CourierSMSTemplate CourierSMSTemplatesRegistrationCodeValid(ctx context.Context) *CourierSMSTemplate CourierMessageRetries(ctx context.Context) int @@ -838,6 +843,7 @@ func (p *Config) SelfServiceCodeStrategy(ctx context.Context) *SelfServiceStrate VerificationStartRequest: verificationStartRequest, VerificationCheckRequest: verificationCheckRequest, }, + NotifyUnknownRecipients: pp.BoolF(basePath+".notify_unknown_recipients", false), } } @@ -1171,6 +1177,10 @@ func (p *Config) CourierSMSTemplatesVerificationCodeValid(ctx context.Context) * return p.CourierSMSTemplatesHelper(ctx, ViperKeyCourierTemplatesVerificationCodeValidSMS) } +func (p *Config) CourierSMSTemplatesLoginCodeInvalid(ctx context.Context) *CourierSMSTemplate { + return p.CourierSMSTemplatesHelper(ctx, ViperKeyCourierTemplatesLoginCodeInvalidSMS) +} + func (p *Config) CourierSMSTemplatesLoginCodeValid(ctx context.Context) *CourierSMSTemplate { return p.CourierSMSTemplatesHelper(ctx, ViperKeyCourierTemplatesLoginCodeValidSMS) } @@ -1179,6 +1189,10 @@ func (p *Config) CourierSMSTemplatesRegistrationCodeValid(ctx context.Context) * return p.CourierSMSTemplatesHelper(ctx, ViperKeyCourierTemplatesRegistrationCodeValidSMS) } +func (p *Config) CourierTemplatesLoginCodeInvalid(ctx context.Context) *CourierEmailTemplate { + return p.CourierEmailTemplatesHelper(ctx, ViperKeyCourierTemplatesLoginCodeInvalidEmail) +} + func (p *Config) CourierTemplatesLoginCodeValid(ctx context.Context) *CourierEmailTemplate { return p.CourierEmailTemplatesHelper(ctx, ViperKeyCourierTemplatesLoginCodeValidEmail) } diff --git a/embedx/config.schema.json b/embedx/config.schema.json index cf258096a447..bd0560e479bc 100644 --- a/embedx/config.schema.json +++ b/embedx/config.schema.json @@ -1636,6 +1636,12 @@ "additionalProperties": false } } + }, + "notify_unknown_recipients": { + "title": "Notify unknown recipients", + "description": "Whether to notify recipients, if login code was requested for their address.", + "type": "boolean", + "default": false } } }, diff --git a/selfservice/strategy/code/code_sender.go b/selfservice/strategy/code/code_sender.go index 552f43e26cb6..19d69d13db75 100644 --- a/selfservice/strategy/code/code_sender.go +++ b/selfservice/strategy/code/code_sender.go @@ -5,7 +5,10 @@ package code import ( "context" + "github.com/ory/kratos/courier/template" "net/url" + "regexp" + "slices" "github.com/gofrs/uuid" "github.com/pkg/errors" @@ -443,7 +446,13 @@ func (s *Sender) send(ctx context.Context, via string, t courier.Template) error return errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Expected sms template but got %T", t)) } - if s.deps.Config().SelfServiceCodeStrategy(ctx).ExternalSMSVerify.Enabled { + if s.deps.Config().SelfServiceCodeStrategy(ctx).ExternalSMSVerify.Enabled && + slices.Contains([]template.TemplateType{ + template.TypeRecoveryCodeValid, + template.TypeVerificationCodeValid, + template.TypeLoginCodeValid, + template.TypeRegistrationCodeValid, + }, t.TemplateType()) { err = s.deps.ExternalVerifier().VerificationStart(ctx, t) } else { _, err = c.QueueSMS(ctx, t) @@ -532,3 +541,21 @@ func (s *Sender) CheckWithExternalVerifier(ctx context.Context, f flow.Flow, id return nil } + +func (s *Sender) SendLoginCodeInvalid(ctx context.Context, recipient string) error { + var via string + var t courier.Template + if regexp.MustCompile("^\\+?\\d+$").MatchString(recipient) { + via = identity.ChannelTypeSMS + t = sms.NewLoginCodeInvalid(s.deps, &sms.LoginCodeInvalidModel{ + To: recipient, + }) + } else { + via = identity.ChannelTypeEmail + t = email.NewLoginCodeInvalid(s.deps, &email.LoginCodeInvalidModel{ + To: recipient, + }) + } + + return s.send(ctx, via, t) +} diff --git a/selfservice/strategy/code/code_sender_test.go b/selfservice/strategy/code/code_sender_test.go index e5ba75826eb5..da34be855466 100644 --- a/selfservice/strategy/code/code_sender_test.go +++ b/selfservice/strategy/code/code_sender_test.go @@ -47,6 +47,32 @@ func TestSender(t *testing.T) { i.Traits = identity.Traits(`{"email": "tracked@ory.sh"}`) require.NoError(t, reg.IdentityManager().Create(ctx, i)) + t.Run("method=SendLoginCodeInvalid", func(t *testing.T) { + t.Run("email", func(t *testing.T) { + require.NoError(t, reg.CodeSender().SendLoginCodeInvalid(ctx, "a@foo.bar")) + + messages, err := reg.CourierPersister().NextMessages(ctx, 12) + require.NoError(t, err) + require.Len(t, messages, 1) + + assert.EqualValues(t, "a@foo.bar", messages[0].Recipient) + assert.Contains(t, messages[0].Subject, "Someone tried to login") + assert.Contains(t, messages[0].Body, "we couldn’t find an account") + assert.NotRegexp(t, testhelpers.CodeRegex, messages[0].Body, "Expected message to not contain an 6 digit login code, but it did: ", messages[0].Body) + }) + t.Run("sms", func(t *testing.T) { + require.NoError(t, reg.CodeSender().SendLoginCodeInvalid(ctx, "+1234567890")) + + messages, err := reg.CourierPersister().NextMessages(ctx, 12) + require.NoError(t, err) + require.Len(t, messages, 1) + + assert.EqualValues(t, "+1234567890", messages[0].Recipient) + assert.Contains(t, messages[0].Body, "we couldn’t find an account") + assert.NotRegexp(t, testhelpers.CodeRegex, messages[0].Body, "Expected message to not contain an 6 digit login code, but it did: ", messages[0].Body) + }) + }) + t.Run("method=SendRecoveryCode", func(t *testing.T) { recoveryCode := func(t *testing.T) { t.Helper() diff --git a/selfservice/strategy/code/strategy_login.go b/selfservice/strategy/code/strategy_login.go index 391eaefee1ce..5a62b14c8895 100644 --- a/selfservice/strategy/code/strategy_login.go +++ b/selfservice/strategy/code/strategy_login.go @@ -7,6 +7,7 @@ import ( "context" "database/sql" "encoding/json" + "github.com/ory/kratos/text" "net/http" "strings" @@ -221,6 +222,7 @@ func (s *Strategy) loginSendCode(ctx context.Context, w http.ResponseWriter, r * var addresses []Address var i *identity.Identity + var blockSendingCode = false if f.RequestedAAL > identity.AuthenticatorAssuranceLevel1 { address, found := lo.Find(sess.Identity.VerifiableAddresses, func(va identity.VerifiableAddress) bool { return va.Value == p.Identifier @@ -237,18 +239,31 @@ func (s *Strategy) loginSendCode(ctx context.Context, w http.ResponseWriter, r * // Step 1: Get the identity i, _, err = s.findIdentityByIdentifier(ctx, p.Identifier) if err != nil { - return err + e := new(schema.ValidationError) + if s.deps.Config().SelfServiceCodeStrategy(ctx).NotifyUnknownRecipients && + errors.As(err, &e) && + len(e.Messages) == 1 && + e.Messages[0].ID == text.ErrorValidationNoCodeUser { + if err := s.deps.CodeSender().SendLoginCodeInvalid(ctx, p.Identifier); err != nil { + return err + } + blockSendingCode = true + } else { + return err + } } - address, found := lo.Find(i.VerifiableAddresses, func(va identity.VerifiableAddress) bool { - return va.Value == p.Identifier - }) - if !found { - return errors.WithStack(schema.NewUnknownAddressError()) + if !blockSendingCode { + address, found := lo.Find(i.VerifiableAddresses, func(va identity.VerifiableAddress) bool { + return va.Value == p.Identifier + }) + if !found { + return errors.WithStack(schema.NewUnknownAddressError()) + } + addresses = []Address{{ + To: p.Identifier, + Via: address.Via, + }} } - addresses = []Address{{ - To: p.Identifier, - Via: address.Via, - }} } // Step 2: Delete any previous login codes for this flow ID @@ -258,8 +273,10 @@ func (s *Strategy) loginSendCode(ctx context.Context, w http.ResponseWriter, r * // 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 { - return errors.WithStack(err) + if !blockSendingCode { + if err := s.deps.CodeSender().SendCode(ctx, f, i, addresses...); err != nil { + return errors.WithStack(err) + } } // sets the flow state to code sent diff --git a/selfservice/strategy/code/strategy_login_phone_test.go b/selfservice/strategy/code/strategy_login_phone_test.go index e6da57dc682f..68b408ae7ff9 100644 --- a/selfservice/strategy/code/strategy_login_phone_test.go +++ b/selfservice/strategy/code/strategy_login_phone_test.go @@ -12,6 +12,7 @@ import ( "github.com/ory/kratos/internal" oryClient "github.com/ory/kratos/internal/httpclient" "github.com/ory/kratos/session" + "github.com/ory/kratos/text" "github.com/ory/x/ioutilx" "github.com/ory/x/sqlxx" "github.com/stretchr/testify/assert" @@ -69,13 +70,11 @@ func TestLoginCodeStrategy_SMS(t *testing.T) { } type state struct { - flowID string - identity *identity.Identity - client *http.Client - loginCode string - identityPhone string - testServer *httptest.Server - body string + flowID string + client *http.Client + loginCode string + testServer *httptest.Server + body string } type ApiType string @@ -89,8 +88,6 @@ func TestLoginCodeStrategy_SMS(t *testing.T) { createLoginFlow := func(ctx context.Context, t *testing.T, public *httptest.Server, apiType ApiType) *state { t.Helper() - identity := createIdentity(ctx, t) - var client *http.Client if apiType == ApiTypeNative { client = &http.Client{} @@ -117,15 +114,10 @@ func TestLoginCodeStrategy_SMS(t *testing.T) { require.NotEmptyf(t, csrfToken, "could not find csrf_token in: %s", body) } - loginPhone := gjson.Get(identity.Traits.String(), "phone").String() - require.NotEmptyf(t, loginPhone, "could not find the phone trait inside the identity: %s", identity.Traits.String()) - return &state{ - flowID: clientInit.GetId(), - identity: identity, - identityPhone: loginPhone, - client: client, - testServer: public, + flowID: clientInit.GetId(), + client: client, + testServer: public, } } @@ -179,59 +171,91 @@ func TestLoginCodeStrategy_SMS(t *testing.T) { return s } - for _, tc := range []struct { - d string - apiType ApiType - }{ - { - d: "SPA client", - apiType: ApiTypeSPA, - }, - { - d: "Browser client", - apiType: ApiTypeBrowser, - }, - { - d: "Native client", - apiType: ApiTypeNative, - }, - } { - - t.Run("test="+tc.d, func(t *testing.T) { - t.Run("case=should be able to log in with code", func(t *testing.T) { - // create login flow - s := createLoginFlow(ctx, t, public, tc.apiType) - - // submit phone - s = submitLogin(ctx, t, s, tc.apiType, func(v *url.Values) { - v.Set("identifier", s.identityPhone) - }, false, nil) - - assert.Contains(t, externalVerifyResult, "code has been sent") - - // 3. Submit OTP - submitLogin(ctx, t, s, tc.apiType, func(v *url.Values) { - v.Set("code", "0000") - }, true, nil) - - assert.Contains(t, externalVerifyResult, "code valid") - }) + setNotifyUnknownRecipientsToTrue := func(t *testing.T) { + conf.MustSet(ctx, fmt.Sprintf("%s.%s.notify_unknown_recipients", config.ViperKeySelfServiceStrategyConfig, identity.CredentialsTypeCodeAuth.String()), true) + t.Cleanup(func() { + conf.MustSet(ctx, fmt.Sprintf("%s.%s.notify_unknown_recipients", config.ViperKeySelfServiceStrategyConfig, identity.CredentialsTypeCodeAuth.String()), false) + }) + } - t.Run("case=should not be able to use valid code after 5 attempts", func(t *testing.T) { - s := createLoginFlow(ctx, t, public, tc.apiType) + t.Run("test=notify_unknown_recipients false", func(t *testing.T) { + for _, tc := range []struct { + d string + apiType ApiType + }{ + { + d: "SPA client", + apiType: ApiTypeSPA, + }, + { + d: "Browser client", + apiType: ApiTypeBrowser, + }, + { + d: "Native client", + apiType: ApiTypeNative, + }, + } { + + t.Run("test="+tc.d, func(t *testing.T) { + t.Run("case=should be able to log in with code", func(t *testing.T) { + externalVerifyResult = "" + i := createIdentity(ctx, t) + loginPhone := gjson.Get(i.Traits.String(), "phone").String() + require.NotEmptyf(t, loginPhone, "could not find the phone trait inside the identity: %s", i.Traits.String()) + + // create login flow + s := createLoginFlow(ctx, t, public, tc.apiType) + + // submit phone + s = submitLogin(ctx, t, s, tc.apiType, func(v *url.Values) { + v.Set("identifier", loginPhone) + }, false, nil) - // submit email - s = submitLogin(ctx, t, s, tc.apiType, func(v *url.Values) { - v.Set("identifier", s.identityPhone) - }, false, nil) + assert.Contains(t, externalVerifyResult, "code has been sent") - assert.Contains(t, externalVerifyResult, "code has been sent") + // 3. Submit OTP + submitLogin(ctx, t, s, tc.apiType, func(v *url.Values) { + v.Set("code", "0000") + }, true, nil) + + assert.Contains(t, externalVerifyResult, "code valid") + }) + + t.Run("case=should not be able to use valid code after 5 attempts", func(t *testing.T) { + externalVerifyResult = "" + i := createIdentity(ctx, t) + loginPhone := gjson.Get(i.Traits.String(), "phone").String() + require.NotEmptyf(t, loginPhone, "could not find the phone trait inside the identity: %s", i.Traits.String()) + s := createLoginFlow(ctx, t, public, tc.apiType) + + // submit email + s = submitLogin(ctx, t, s, tc.apiType, func(v *url.Values) { + v.Set("identifier", loginPhone) + }, false, nil) + + assert.Contains(t, externalVerifyResult, "code has been sent") + + for i := 0; i < 5; i++ { + // 3. Submit OTP + s = submitLogin(ctx, t, s, tc.apiType, func(v *url.Values) { + v.Set("code", "111111") + v.Set("identifier", loginPhone) + }, false, func(t *testing.T, s *state, body string, resp *http.Response) { + if tc.apiType == ApiTypeBrowser { + // in browser flows we redirect back to the login ui + require.Equal(t, http.StatusOK, resp.StatusCode, "%s", body) + } else { + require.EqualValues(t, http.StatusBadRequest, resp.StatusCode) + } + assert.Contains(t, gjson.Get(body, "ui.messages.0.text").String(), "The login code is invalid or has already been used") + }) + } - for i := 0; i < 5; i++ { // 3. Submit OTP s = submitLogin(ctx, t, s, tc.apiType, func(v *url.Values) { - v.Set("code", "111111") - v.Set("identifier", s.identityPhone) + v.Set("code", "0000") + v.Set("identifier", loginPhone) }, false, func(t *testing.T, s *state, body string, resp *http.Response) { if tc.apiType == ApiTypeBrowser { // in browser flows we redirect back to the login ui @@ -239,24 +263,71 @@ func TestLoginCodeStrategy_SMS(t *testing.T) { } else { require.EqualValues(t, http.StatusBadRequest, resp.StatusCode) } - assert.Contains(t, gjson.Get(body, "ui.messages.0.text").String(), "The login code is invalid or has already been used") + assert.Contains(t, gjson.Get(body, "ui.messages.0.text").String(), "The request was submitted too often.") }) - } - - // 3. Submit OTP - s = submitLogin(ctx, t, s, tc.apiType, func(v *url.Values) { - v.Set("code", "0000") - v.Set("identifier", s.identityPhone) - }, false, func(t *testing.T, s *state, body string, resp *http.Response) { - if tc.apiType == ApiTypeBrowser { - // in browser flows we redirect back to the login ui - require.Equal(t, http.StatusOK, resp.StatusCode, "%s", body) - } else { - require.EqualValues(t, http.StatusBadRequest, resp.StatusCode) - } - assert.Contains(t, gjson.Get(body, "ui.messages.0.text").String(), "The request was submitted too often.") }) }) - }) - } + } + }) + + t.Run("test=notify_unknown_recipients true", func(t *testing.T) { + setNotifyUnknownRecipientsToTrue(t) + for _, tc := range []struct { + d string + apiType ApiType + }{ + { + d: "SPA client", + apiType: ApiTypeSPA, + }, + { + d: "Browser client", + apiType: ApiTypeBrowser, + }, + { + d: "Native client", + apiType: ApiTypeNative, + }, + } { + t.Run("test="+tc.d, func(t *testing.T) { + t.Run("case=should be able to log in with code", func(t *testing.T) { + externalVerifyResult = "" + i := createIdentity(ctx, t) + loginPhone := gjson.Get(i.Traits.String(), "phone").String() + require.NotEmptyf(t, loginPhone, "could not find the phone trait inside the identity: %s", i.Traits.String()) + // create login flow + s := createLoginFlow(ctx, t, public, tc.apiType) + + // submit phone + s = submitLogin(ctx, t, s, tc.apiType, func(v *url.Values) { + v.Set("identifier", loginPhone) + }, false, nil) + + assert.Contains(t, externalVerifyResult, "code has been sent") + + // 3. Submit OTP + submitLogin(ctx, t, s, tc.apiType, func(v *url.Values) { + v.Set("code", "0000") + }, true, nil) + + assert.Contains(t, externalVerifyResult, "code valid") + }) + t.Run("case=respond with code sent but send info message instead", func(t *testing.T) { + externalVerifyResult = "" + // create login flow + s := createLoginFlow(ctx, t, public, tc.apiType) + + // submit phone + s = submitLogin(ctx, t, s, tc.apiType, func(v *url.Values) { + v.Set("identifier", "+1234567890") + }, false, nil) + + assert.Equal(t, externalVerifyResult, "") + assert.Equal(t, int64(text.InfoSelfServiceLoginEmailWithCodeSent), gjson.GetBytes([]byte(s.body), "ui.messages.0.id").Int(), "%s", s.body) + message := testhelpers.CourierExpectMessage(ctx, t, reg, "+1234567890", "") + assert.Contains(t, message.Body, "we couldn’t find an account linked to this phone") + }) + }) + } + }) }