From 8da18d33dcd1e9c6f10e7f996d63c1a0f352fdc9 Mon Sep 17 00:00:00 2001 From: "oleksii.reshetnik" Date: Mon, 8 Nov 2021 19:02:17 +0200 Subject: [PATCH] feat: added phone number identtifier (ory#137) --- embedx/identity_extension.schema.json | 3 +- identity/address.go | 5 +- identity/extension_verify.go | 45 +- identity/extension_verify_test.go | 434 +++++++++++------- identity/identity_verification.go | 16 +- .../verify/{schema.json => email.schema.json} | 0 .../stub/extension/verify/phone.schema.json | 24 + 7 files changed, 350 insertions(+), 177 deletions(-) rename identity/stub/extension/verify/{schema.json => email.schema.json} (100%) create mode 100644 identity/stub/extension/verify/phone.schema.json diff --git a/embedx/identity_extension.schema.json b/embedx/identity_extension.schema.json index 4d1c67ffcc0e..7205039be9a7 100644 --- a/embedx/identity_extension.schema.json +++ b/embedx/identity_extension.schema.json @@ -39,7 +39,8 @@ "via": { "type": "string", "enum": [ - "email" + "email", + "phone" ] } } diff --git a/identity/address.go b/identity/address.go index 4a8c5d98d68b..d4b4713ecebb 100644 --- a/identity/address.go +++ b/identity/address.go @@ -1,3 +1,6 @@ package identity -const AddressTypeEmail = "email" +const ( + AddressTypeEmail = "email" + AddressTypePhone = "phone" +) diff --git a/identity/extension_verify.go b/identity/extension_verify.go index 7afcc9947d62..cb8ac61228f1 100644 --- a/identity/extension_verify.go +++ b/identity/extension_verify.go @@ -6,7 +6,6 @@ import ( "time" "github.com/ory/jsonschema/v3" - "github.com/ory/kratos/schema" ) @@ -26,25 +25,24 @@ func (r *SchemaExtensionVerification) Run(ctx jsonschema.ValidationContext, s sc defer r.l.Unlock() switch s.Verification.Via { - case "email": + case AddressTypeEmail: if !jsonschema.Formats["email"](value) { return ctx.Error("format", "%q is not valid %q", value, "email") } address := NewVerifiableEmailAddress(fmt.Sprintf("%s", value), r.i.ID) - if has := r.has(r.i.VerifiableAddresses, address); has != nil { - if r.has(r.v, address) == nil { - r.v = append(r.v, *has) - } - return nil - } + r.appendAddress(address) - if has := r.has(r.v, address); has == nil { - r.v = append(r.v, *address) - } + return nil + + case AddressTypePhone: + address := NewVerifiablePhoneAddress(fmt.Sprintf("%s", value), r.i.ID) + + r.appendAddress(address) return nil + case "": return nil } @@ -52,7 +50,25 @@ func (r *SchemaExtensionVerification) Run(ctx jsonschema.ValidationContext, s sc return ctx.Error("", "verification.via has unknown value %q", s.Verification.Via) } -func (r *SchemaExtensionVerification) has(haystack []VerifiableAddress, needle *VerifiableAddress) *VerifiableAddress { +func (r *SchemaExtensionVerification) Finish() error { + r.i.VerifiableAddresses = r.v + return nil +} + +func (r *SchemaExtensionVerification) appendAddress(address *VerifiableAddress) { + if h := has(r.i.VerifiableAddresses, address); h != nil { + if has(r.v, address) == nil { + r.v = append(r.v, *h) + } + return + } + + if has(r.v, address) == nil { + r.v = append(r.v, *address) + } +} + +func has(haystack []VerifiableAddress, needle *VerifiableAddress) *VerifiableAddress { for _, has := range haystack { if has.Value == needle.Value && has.Via == needle.Via { return &has @@ -60,8 +76,3 @@ func (r *SchemaExtensionVerification) has(haystack []VerifiableAddress, needle * } return nil } - -func (r *SchemaExtensionVerification) Finish() error { - r.i.VerifiableAddresses = r.v - return nil -} diff --git a/identity/extension_verify_test.go b/identity/extension_verify_test.go index 3c3aef245156..cecb705cd9c3 100644 --- a/identity/extension_verify_test.go +++ b/identity/extension_verify_test.go @@ -8,196 +8,318 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/ory/jsonschema/v3" _ "github.com/ory/jsonschema/v3/fileloader" - "github.com/ory/kratos/schema" "github.com/ory/kratos/x" +) - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" +const ( + emailSchemaPath = "file://./stub/extension/verify/email.schema.json" + phoneSchemaPath = "file://./stub/extension/verify/phone.schema.json" ) func TestSchemaExtensionVerification(t *testing.T) { - iid := x.NewUUID() - for k, tc := range []struct { - expectErr error - schema string - doc string - expect []VerifiableAddress - existing []VerifiableAddress - }{ - { - doc: `{"username":"foo@ory.sh"}`, - schema: "file://./stub/extension/verify/schema.json", - expect: []VerifiableAddress{ - { - Value: "foo@ory.sh", - Verified: false, - Status: VerifiableAddressStatusPending, - Via: VerifiableAddressTypeEmail, - IdentityID: iid, + t.Run("address verification", func(t *testing.T) { + iid := x.NewUUID() + + for _, tc := range []struct { + name string + schema string + expectErr error + doc string + existing []VerifiableAddress + expect []VerifiableAddress + }{ + { + name: "must create new email address", + schema: emailSchemaPath, + doc: `{"username":"foo@ory.sh"}`, + expect: []VerifiableAddress{ + { + Value: "foo@ory.sh", + Verified: false, + Status: VerifiableAddressStatusPending, + Via: VerifiableAddressTypeEmail, + IdentityID: iid, + }, }, }, - }, - { - doc: `{"username":"foo@ory.sh"}`, - schema: "file://./stub/extension/verify/schema.json", - expect: []VerifiableAddress{ - { - Value: "foo@ory.sh", - Verified: false, - Status: VerifiableAddressStatusPending, - Via: VerifiableAddressTypeEmail, - IdentityID: iid, + { + name: "must create new address because new and existing doesn't match", + schema: emailSchemaPath, + doc: `{"username":"foo@ory.sh"}`, + existing: []VerifiableAddress{ + { + Value: "bar@ory.sh", + Verified: false, + Status: VerifiableAddressStatusPending, + Via: VerifiableAddressTypeEmail, + IdentityID: iid, + }, }, - }, - existing: []VerifiableAddress{ - { - Value: "bar@ory.sh", - Verified: false, - Status: VerifiableAddressStatusPending, - Via: VerifiableAddressTypeEmail, - IdentityID: iid, + expect: []VerifiableAddress{ + { + Value: "foo@ory.sh", + Verified: false, + Status: VerifiableAddressStatusPending, + Via: VerifiableAddressTypeEmail, + IdentityID: iid, + }, }, }, - }, - { - doc: `{"emails":["baz@ory.sh","foo@ory.sh"]}`, - schema: "file://./stub/extension/verify/schema.json", - expect: []VerifiableAddress{ - { - Value: "foo@ory.sh", - Verified: true, - Status: VerifiableAddressStatusCompleted, - Via: VerifiableAddressTypeEmail, - IdentityID: iid, + { + name: "must find existing address in case of match", + schema: emailSchemaPath, + doc: `{"emails":["baz@ory.sh","foo@ory.sh"]}`, + existing: []VerifiableAddress{ + { + Value: "foo@ory.sh", + Verified: true, + Status: VerifiableAddressStatusCompleted, + Via: VerifiableAddressTypeEmail, + IdentityID: iid, + }, + { + Value: "bar@ory.sh", + Verified: true, + Status: VerifiableAddressStatusCompleted, + Via: VerifiableAddressTypeEmail, + IdentityID: iid, + }, }, - { - Value: "baz@ory.sh", - Verified: false, - Status: VerifiableAddressStatusPending, - Via: VerifiableAddressTypeEmail, - IdentityID: iid, + expect: []VerifiableAddress{ + { + Value: "foo@ory.sh", + Verified: true, + Status: VerifiableAddressStatusCompleted, + Via: VerifiableAddressTypeEmail, + IdentityID: iid, + }, + { + Value: "baz@ory.sh", + Verified: false, + Status: VerifiableAddressStatusPending, + Via: VerifiableAddressTypeEmail, + IdentityID: iid, + }, }, }, - existing: []VerifiableAddress{ - { - Value: "foo@ory.sh", - Verified: true, - Status: VerifiableAddressStatusCompleted, - Via: VerifiableAddressTypeEmail, - IdentityID: iid, + { + name: "must return only one address in case of duplication", + schema: emailSchemaPath, + doc: `{"emails":["foo@ory.sh","foo@ory.sh","baz@ory.sh"]}`, + existing: []VerifiableAddress{ + { + Value: "foo@ory.sh", + Verified: true, + Status: VerifiableAddressStatusCompleted, + Via: VerifiableAddressTypeEmail, + IdentityID: iid, + }, + { + Value: "bar@ory.sh", + Verified: true, + Status: VerifiableAddressStatusCompleted, + Via: VerifiableAddressTypeEmail, + IdentityID: iid, + }, }, - { - Value: "bar@ory.sh", - Verified: true, - Status: VerifiableAddressStatusCompleted, - Via: VerifiableAddressTypeEmail, - IdentityID: iid, + expect: []VerifiableAddress{ + { + Value: "foo@ory.sh", + Verified: true, + Status: VerifiableAddressStatusCompleted, + Via: VerifiableAddressTypeEmail, + IdentityID: iid, + }, + { + Value: "baz@ory.sh", + Verified: false, + Status: VerifiableAddressStatusPending, + Via: VerifiableAddressTypeEmail, + IdentityID: iid, + }, }, }, - }, - { - doc: `{"emails":["foo@ory.sh","foo@ory.sh","baz@ory.sh"]}`, - schema: "file://./stub/extension/verify/schema.json", - expect: []VerifiableAddress{ - { - Value: "foo@ory.sh", - Verified: true, - Status: VerifiableAddressStatusCompleted, - Via: VerifiableAddressTypeEmail, - IdentityID: iid, + { + name: "must return error for malformed input", + schema: emailSchemaPath, + doc: `{"emails":["foo@ory.sh","bar@ory.sh"], "username": "foobar"}`, + expectErr: errors.New("I[#/username] S[#/properties/username/format] \"foobar\" is not valid \"email\""), + }, + { + name: "must merge email addresses from multiple attributes", + schema: emailSchemaPath, + doc: `{"emails":["foo@ory.sh","bar@ory.sh","bar@ory.sh"], "username": "foobar@ory.sh"}`, + expect: []VerifiableAddress{ + { + Value: "foo@ory.sh", + Verified: false, + Status: VerifiableAddressStatusPending, + Via: VerifiableAddressTypeEmail, + IdentityID: iid, + }, + { + Value: "bar@ory.sh", + Verified: false, + Status: VerifiableAddressStatusPending, + Via: VerifiableAddressTypeEmail, + IdentityID: iid, + }, + { + Value: "foobar@ory.sh", + Verified: false, + Status: VerifiableAddressStatusPending, + Via: VerifiableAddressTypeEmail, + IdentityID: iid, + }, }, - { - Value: "baz@ory.sh", - Verified: false, - Status: VerifiableAddressStatusPending, - Via: VerifiableAddressTypeEmail, - IdentityID: iid, + }, + { + name: "must create new phone address", + schema: phoneSchemaPath, + doc: `{"username":"+12233344445"}`, + expect: []VerifiableAddress{ + { + Value: "+12233344445", + Verified: false, + Status: VerifiableAddressStatusPending, + Via: VerifiableAddressTypePhone, + IdentityID: iid, + }, }, }, - existing: []VerifiableAddress{ - { - Value: "foo@ory.sh", - Verified: true, - Status: VerifiableAddressStatusCompleted, - Via: VerifiableAddressTypeEmail, - IdentityID: iid, + { + name: "must create new phone address because new and existing doesn't match", + schema: phoneSchemaPath, + doc: `{"username":"+12233344445"}`, + existing: []VerifiableAddress{ + { + Value: "+12112112112", + Verified: false, + Status: VerifiableAddressStatusPending, + Via: VerifiableAddressTypePhone, + IdentityID: iid, + }, }, - { - Value: "bar@ory.sh", - Verified: true, - Status: VerifiableAddressStatusCompleted, - Via: VerifiableAddressTypeEmail, - IdentityID: iid, + expect: []VerifiableAddress{ + { + Value: "+12233344445", + Verified: false, + Status: VerifiableAddressStatusPending, + Via: VerifiableAddressTypePhone, + IdentityID: iid, + }, }, }, - }, - { - doc: `{"emails":["foo@ory.sh","bar@ory.sh"], "username": "foobar"}`, - schema: "file://./stub/extension/verify/schema.json", - expectErr: errors.New("I[#/username] S[#/properties/username/format] \"foobar\" is not valid \"email\""), - }, - { - doc: `{"emails":["foo@ory.sh","bar@ory.sh","bar@ory.sh"], "username": "foobar@ory.sh"}`, - schema: "file://./stub/extension/verify/schema.json", - expect: []VerifiableAddress{ - { - Value: "foo@ory.sh", - Verified: false, - Status: VerifiableAddressStatusPending, - Via: VerifiableAddressTypeEmail, - IdentityID: iid, + { + name: "must find existing phone addresses in case of match", + schema: phoneSchemaPath, + doc: `{"phones":["+12233344445","+12112112112"]}`, + existing: []VerifiableAddress{ + { + Value: "+12112112112", + Verified: true, + Status: VerifiableAddressStatusCompleted, + Via: VerifiableAddressTypePhone, + IdentityID: iid, + }, + { + Value: "+12312312312", + Verified: true, + Status: VerifiableAddressStatusCompleted, + Via: VerifiableAddressTypePhone, + IdentityID: iid, + }, }, - { - Value: "bar@ory.sh", - Verified: false, - Status: VerifiableAddressStatusPending, - Via: VerifiableAddressTypeEmail, - IdentityID: iid, + expect: []VerifiableAddress{ + { + Value: "+12112112112", + Verified: true, + Status: VerifiableAddressStatusCompleted, + Via: VerifiableAddressTypePhone, + IdentityID: iid, + }, + { + Value: "+12233344445", + Verified: false, + Status: VerifiableAddressStatusPending, + Via: VerifiableAddressTypePhone, + IdentityID: iid, + }, }, - { - Value: "foobar@ory.sh", - Verified: false, - Status: VerifiableAddressStatusPending, - Via: VerifiableAddressTypeEmail, - IdentityID: iid, + }, + { + name: "must merge phones from multiple attributes", + schema: phoneSchemaPath, + doc: `{"phones": ["+12233344445","+12233344445","+12112112112"], "username": "+12312312312"}`, + expect: []VerifiableAddress{ + { + Value: "+12233344445", + Verified: false, + Status: VerifiableAddressStatusPending, + Via: VerifiableAddressTypePhone, + IdentityID: iid, + }, + { + Value: "+12112112112", + Verified: false, + Status: VerifiableAddressStatusPending, + Via: VerifiableAddressTypePhone, + IdentityID: iid, + }, + { + Value: "+12312312312", + Verified: false, + Status: VerifiableAddressStatusPending, + Via: VerifiableAddressTypePhone, + IdentityID: iid, + }, }, }, - }, - } { - t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) { - id := &Identity{ID: iid, VerifiableAddresses: tc.existing} - c := jsonschema.NewCompiler() - runner, err := schema.NewExtensionRunner() - require.NoError(t, err) + } { + t.Run(fmt.Sprintf("case=%v", tc.name), func(t *testing.T) { + id := &Identity{ID: iid, VerifiableAddresses: tc.existing} - const expiresAt = time.Minute - e := NewSchemaExtensionVerification(id, time.Minute) - runner.AddRunner(e).Register(c) + c := jsonschema.NewCompiler() - err = c.MustCompile(tc.schema).Validate(bytes.NewBufferString(tc.doc)) - if tc.expectErr != nil { - require.EqualError(t, err, tc.expectErr.Error()) - return - } + runner, err := schema.NewExtensionRunner() + require.NoError(t, err) - require.NoError(t, e.Finish()) + e := NewSchemaExtensionVerification(id, time.Minute) + runner.AddRunner(e).Register(c) - addresses := id.VerifiableAddresses - require.Len(t, addresses, len(tc.expect)) - - for _, actual := range addresses { - var found bool - for _, expect := range tc.expect { - if reflect.DeepEqual(actual, expect) { - found = true - break - } + err = c.MustCompile(tc.schema).Validate(bytes.NewBufferString(tc.doc)) + if tc.expectErr != nil { + require.EqualError(t, err, tc.expectErr.Error()) + return } - assert.True(t, found, "%+v not in %+v", actual, tc.expect) + + require.NoError(t, e.Finish()) + + addresses := id.VerifiableAddresses + require.Len(t, addresses, len(tc.expect)) + + mustContainAddress(t, tc.expect, addresses) + }) + } + }) + +} + +func mustContainAddress(t *testing.T, expected, actual []VerifiableAddress) { + for _, act := range actual { + var found bool + for _, expect := range expected { + if reflect.DeepEqual(act, expect) { + found = true + break } - }) + } + assert.True(t, found, "%+v not in %+v", act, expected) } } diff --git a/identity/identity_verification.go b/identity/identity_verification.go index 5f778930d91d..c0e09fb404ad 100644 --- a/identity/identity_verification.go +++ b/identity/identity_verification.go @@ -4,15 +4,15 @@ import ( "context" "time" - "github.com/ory/kratos/corp" - "github.com/gofrs/uuid" + "github.com/ory/kratos/corp" "github.com/ory/x/sqlxx" ) const ( VerifiableAddressTypeEmail VerifiableAddressType = AddressTypeEmail + VerifiableAddressTypePhone VerifiableAddressType = AddressTypePhone VerifiableAddressStatusPending VerifiableAddressStatus = "pending" VerifiableAddressStatusSent VerifiableAddressStatus = "sent" @@ -90,6 +90,8 @@ func (v VerifiableAddressType) HTMLFormInputType() string { switch v { case VerifiableAddressTypeEmail: return "email" + case VerifiableAddressTypePhone: + return "phone" } return "" } @@ -108,6 +110,16 @@ func NewVerifiableEmailAddress(value string, identity uuid.UUID) *VerifiableAddr } } +func NewVerifiablePhoneAddress(value string, identity uuid.UUID) *VerifiableAddress { + return &VerifiableAddress{ + Value: value, + Verified: false, + Status: VerifiableAddressStatusPending, + Via: VerifiableAddressTypePhone, + IdentityID: identity, + } +} + func (a VerifiableAddress) GetID() uuid.UUID { return a.ID } diff --git a/identity/stub/extension/verify/schema.json b/identity/stub/extension/verify/email.schema.json similarity index 100% rename from identity/stub/extension/verify/schema.json rename to identity/stub/extension/verify/email.schema.json diff --git a/identity/stub/extension/verify/phone.schema.json b/identity/stub/extension/verify/phone.schema.json new file mode 100644 index 000000000000..2bd7d6f4fa35 --- /dev/null +++ b/identity/stub/extension/verify/phone.schema.json @@ -0,0 +1,24 @@ +{ + "type": "object", + "properties": { + "phones": { + "type": "array", + "items": { + "type": "string", + "ory.sh/kratos": { + "verification": { + "via": "phone" + } + } + } + }, + "username": { + "type": "string", + "ory.sh/kratos": { + "verification": { + "via": "phone" + } + } + } + } +}