Skip to content

Commit

Permalink
fix: properly normalize uppercase mail addresses
Browse files Browse the repository at this point in the history
Fixes #3187
Fixes #3289
  • Loading branch information
hperl committed Jun 19, 2023
1 parent 7182eca commit 29d6c31
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 4 deletions.
5 changes: 4 additions & 1 deletion identity/extension_recovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package identity

import (
"fmt"
"strings"
"sync"

"github.com/ory/jsonschema/v3"
Expand Down Expand Up @@ -32,7 +33,9 @@ func (r *SchemaExtensionRecovery) Run(ctx jsonschema.ValidationContext, s schema
return ctx.Error("format", "%q is not valid %q", value, "email")
}

address := NewRecoveryEmailAddress(fmt.Sprintf("%s", value), r.i.ID)
address := NewRecoveryEmailAddress(
strings.ToLower(strings.TrimSpace(
fmt.Sprintf("%s", value))), r.i.ID)

if has := r.has(r.i.RecoveryAddresses, address); has != nil {
if r.has(r.v, address) == nil {
Expand Down
5 changes: 4 additions & 1 deletion identity/extension_verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package identity

import (
"fmt"
"strings"
"sync"
"time"

Expand Down Expand Up @@ -33,7 +34,9 @@ func (r *SchemaExtensionVerification) Run(ctx jsonschema.ValidationContext, s sc
return ctx.Error("format", "%q is not valid %q", value, "email")
}

address := NewVerifiableEmailAddress(fmt.Sprintf("%s", value), r.i.ID)
address := NewVerifiableEmailAddress(
strings.ToLower(strings.TrimSpace(
fmt.Sprintf("%s", value))), r.i.ID)

r.appendAddress(address)

Expand Down
3 changes: 2 additions & 1 deletion identity/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -820,7 +820,8 @@ func (h *Handler) patch(w http.ResponseWriter, r *http.Request, ps httprouter.Pa
return
}

h.r.Writer().Write(w, r, WithCredentialsMetadataAndAdminMetadataInJSON(updatedIdenty))
response := WithCredentialsMetadataAndAdminMetadataInJSON(updatedIdenty)
h.r.Writer().Write(w, r, response)
}

func deletCredentialWebAuthFromIdentity(identity *Identity) (*Identity, error) {
Expand Down
47 changes: 46 additions & 1 deletion identity/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"net/url"
"sort"
"strconv"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -733,7 +734,8 @@ func TestHandler(t *testing.T) {

t.Run("case=PATCH update of state should update state changed at timestamp", func(t *testing.T) {
uuid := x.NewUUID().String()
i := &identity.Identity{Traits: identity.Traits(fmt.Sprintf(`{"subject":"%s"}`, uuid))}
email := "UPPER" + uuid + "@ory.sh"
i := &identity.Identity{Traits: identity.Traits(fmt.Sprintf(`{"subject": %q, "email": %q}`, uuid, email))}
require.NoError(t, reg.PrivilegedIdentityPool().CreateIdentity(context.Background(), i))

for name, ts := range map[string]*httptest.Server{"public": publicTS, "admin": adminTS} {
Expand All @@ -744,6 +746,7 @@ func TestHandler(t *testing.T) {

res := send(t, ts, "PATCH", "/identities/"+i.ID.String(), http.StatusOK, &patch)
assert.EqualValues(t, uuid, res.Get("traits.subject").String(), "%s", res.Raw)
assert.EqualValues(t, email, res.Get("traits.email").String(), "%s", res.Raw)
assert.False(t, res.Get("metadata_admin.admin").Exists(), "%s", res.Raw)
assert.False(t, res.Get("metadata_public.public").Exists(), "%s", res.Raw)
assert.EqualValues(t, identity.StateInactive, res.Get("state").String(), "%s", res.Raw)
Expand All @@ -752,6 +755,7 @@ func TestHandler(t *testing.T) {
res = get(t, ts, "/identities/"+i.ID.String(), http.StatusOK)
assert.EqualValues(t, i.ID.String(), res.Get("id").String(), "%s", res.Raw)
assert.EqualValues(t, uuid, res.Get("traits.subject").String(), "%s", res.Raw)
assert.EqualValues(t, email, res.Get("traits.email").String(), "%s", res.Raw)
assert.False(t, res.Get("metadata_admin.admin").Exists(), "%s", res.Raw)
assert.False(t, res.Get("metadata_public.public").Exists(), "%s", res.Raw)
assert.EqualValues(t, identity.StateInactive, res.Get("state").String(), "%s", res.Raw)
Expand All @@ -760,6 +764,47 @@ func TestHandler(t *testing.T) {
}
})

t.Run("case=PATCH update with uppercase emails should work", func(t *testing.T) {
// Regression test for https://github.com/ory/kratos/issues/3187

for name, ts := range map[string]*httptest.Server{"public": publicTS, "admin": adminTS} {
t.Run("endpoint="+name, func(t *testing.T) {

email := "UPPER" + x.NewUUID().String() + "@ory.sh"
lowercaseEmail := strings.ToLower(email)
var cr identity.CreateIdentityBody
cr.SchemaID = "employee"
cr.Traits = []byte(`{"email":"` + email + `"}`)
res := send(t, ts, "POST", "/identities", http.StatusCreated, &cr)
assert.EqualValues(t, lowercaseEmail, res.Get("recovery_addresses.0.value").String(), "%s", res.Raw)
assert.EqualValues(t, lowercaseEmail, res.Get("verifiable_addresses.0.value").String(), "%s", res.Raw)
identityID := res.Get("id").String()

patch := []patch{
{
"op": "replace",
"path": "/verifiable_addresses/0/verified",
"value": true,
},
}

res = send(t, ts, "PATCH", "/identities/"+identityID, http.StatusOK, &patch)
assert.EqualValues(t, email, res.Get("traits.email").String(), "%s", res.Raw)
assert.False(t, res.Get("metadata_admin.admin").Exists(), "%s", res.Raw)
assert.False(t, res.Get("metadata_public.public").Exists(), "%s", res.Raw)
assert.EqualValues(t, identity.StateActive, res.Get("state").String(), "%s", res.Raw)

res = get(t, ts, "/identities/"+identityID, http.StatusOK)
assert.EqualValues(t, identityID, res.Get("id").String(), "%s", res.Raw)
assert.EqualValues(t, email, res.Get("traits.email").String(), "%s", res.Raw)
assert.False(t, res.Get("metadata_admin.admin").Exists(), "%s", res.Raw)
assert.False(t, res.Get("metadata_public.public").Exists(), "%s", res.Raw)
assert.EqualValues(t, identity.StateActive, res.Get("state").String(), "%s", res.Raw)
})
}

})

t.Run("case=PATCH update should not persist if schema id is invalid", func(t *testing.T) {
uuid := x.NewUUID().String()
i := &identity.Identity{Traits: identity.Traits(fmt.Sprintf(`{"subject":"%s"}`, uuid))}
Expand Down

0 comments on commit 29d6c31

Please sign in to comment.