Skip to content

Commit 62f95bd

Browse files
ksarangmathclaude
andauthored
fix(auth): canonicalize email identity keys (#466)
Email identity keys were lower(trim(email)), so the composed, decomposed and Punycode spellings of an internationalized domain were three different identities: logins and invitations missed each other and a second account could be verified for the same mailbox. Derive the key in one place (internal/emailaddr): the domain becomes its lowercase A-label via UTS #46 non-transitional processing with hostname length rules, the comparison form RFC 9598 §5.5 prescribes; the local part is lowercased and otherwise left alone, since RFC 9598 §5.7 forbids transforming it and merging two spellings a mail server might treat as different mailboxes would attach an OIDC login to the wrong account. CreateUserEmail derives the key itself instead of trusting callers. Lookups stay single-key and no queries change. Every pure-ASCII key is a fixed point of the new rule (property-tested), so existing rows keep their keys and no data migration is needed. emailaddr.Equal replaces the EqualFold comparison of OIDC id-token and userinfo emails. The password policy receives the address rather than the key and rejects a password containing either spelling. Supersedes #409. Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
1 parent b427a4b commit 62f95bd

17 files changed

Lines changed: 396 additions & 70 deletions

‎internal/authn/password.go‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@ import (
99
"strconv"
1010
"strings"
1111

12+
"github.com/omnara-ai/omnara/internal/emailaddr"
1213
"golang.org/x/crypto/argon2"
14+
"golang.org/x/text/unicode/norm"
1315
)
1416

1517
const (
@@ -35,7 +37,7 @@ func (e PasswordPolicyError) Error() string {
3537
return e.Reason
3638
}
3739

38-
func ValidateNewPassword(password, normalizedEmail string) error {
40+
func ValidateNewPassword(password, email string) error {
3941
if len(password) > MaxPasswordBytes {
4042
return PasswordPolicyError{Reason: "password is too long"}
4143
}
@@ -63,10 +65,18 @@ func ValidateNewPassword(password, normalizedEmail string) error {
6365
Reason: "password must include at least one lowercase letter, one uppercase letter, one number, and one symbol",
6466
}
6567
}
66-
lower := strings.ToLower(password)
67-
normalizedEmail = strings.ToLower(strings.TrimSpace(normalizedEmail))
68-
if normalizedEmail != "" && strings.Contains(lower, normalizedEmail) {
69-
return PasswordPolicyError{Reason: "password must not include the email address"}
68+
lowered := strings.ToLower(strings.TrimSpace(password))
69+
haystacks := []string{lowered, norm.NFC.String(lowered)}
70+
address := strings.ToLower(strings.TrimSpace(email))
71+
for _, spelling := range []string{address, norm.NFC.String(address), emailaddr.Normalize(email)} {
72+
if spelling == "" {
73+
continue
74+
}
75+
for _, haystack := range haystacks {
76+
if strings.Contains(haystack, spelling) {
77+
return PasswordPolicyError{Reason: "password must not include the email address"}
78+
}
79+
}
7080
}
7181
return nil
7282
}

‎internal/authn/password_test.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,30 @@ func TestValidateNewPassword(t *testing.T) {
147147
password: "Andes Mountain 1!",
148148
email: "an@corp.com",
149149
},
150+
{
151+
name: "contains unicode email as typed",
152+
password: "xÜser@BÜCHER.example1!",
153+
email: "üser@bücher.example",
154+
wantErr: "password must not include the email address",
155+
},
156+
{
157+
name: "contains decomposed spelling of email",
158+
password: "xu\u0308ser@bu\u0308cher.example1A!",
159+
email: "üser@bücher.example",
160+
wantErr: "password must not include the email address",
161+
},
162+
{
163+
name: "contains email followed by a combining mark",
164+
password: "Owner@Example.com\u0301A1!",
165+
email: "owner@example.com",
166+
wantErr: "password must not include the email address",
167+
},
168+
{
169+
name: "contains punycode identity key of email",
170+
password: "xüser@xn--bcher-kva.example1A!",
171+
email: "Üser@BÜCHER.example",
172+
wantErr: "password must not include the email address",
173+
},
150174
}
151175
for _, tt := range tests {
152176
t.Run(tt.name, func(t *testing.T) {

‎internal/emailaddr/emailaddr.go‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
package emailaddr
2+
3+
import (
4+
"strings"
5+
6+
"golang.org/x/net/idna"
7+
)
8+
9+
var domain = idna.New(idna.MapForLookup(), idna.BidiRule(), idna.VerifyDNSLength(true))
10+
11+
func Normalize(email string) string {
12+
trimmed := strings.TrimSpace(email)
13+
at := strings.LastIndexByte(trimmed, '@')
14+
if at <= 0 || at == len(trimmed)-1 {
15+
return strings.ToLower(trimmed)
16+
}
17+
local, host := trimmed[:at], trimmed[at+1:]
18+
ascii, err := domain.ToASCII(host)
19+
if err != nil {
20+
ascii = asciiLower(host)
21+
}
22+
return strings.ToLower(local) + "@" + ascii
23+
}
24+
25+
func Equal(a, b string) bool {
26+
return Normalize(a) == Normalize(b)
27+
}
28+
29+
func asciiLower(text string) string {
30+
return strings.Map(func(r rune) rune {
31+
if 'A' <= r && r <= 'Z' {
32+
return r + 'a' - 'A'
33+
}
34+
return r
35+
}, text)
36+
}
Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
package emailaddr
2+
3+
import (
4+
"math/rand/v2"
5+
"strings"
6+
"testing"
7+
8+
"golang.org/x/net/idna"
9+
)
10+
11+
func TestNormalize(t *testing.T) {
12+
t.Parallel()
13+
tests := []struct {
14+
name string
15+
email string
16+
want string
17+
}{
18+
{name: "ascii", email: " User+Tag@Example.COM ", want: "user+tag@example.com"},
19+
{name: "unicode domain", email: "User@BÜCHER.example", want: "user@xn--bcher-kva.example"},
20+
{name: "decomposed domain", email: "User@BU\u0308CHER.example", want: "user@xn--bcher-kva.example"},
21+
{name: "punycode domain", email: "User@XN--BCHER-KVA.example", want: "user@xn--bcher-kva.example"},
22+
{name: "ideographic dot", email: "User@Example。COM", want: "user@example.com"},
23+
{name: "sharp s is not transitional", email: "user@straße.de", want: "user@xn--strae-oqa.de"},
24+
{name: "capital sharp s maps per UTS 46", email: "user@STRAẞE.de", want: "user@strasse.de"},
25+
{name: "dotted capital I composed", email: "user@İstanbul.com", want: "user@xn--istanbul-o0e.com"},
26+
{name: "dotted capital I decomposed", email: "user@I\u0307stanbul.com", want: "user@xn--istanbul-o0e.com"},
27+
{name: "unicode local is lowercased only", email: "Üser@Example.com", want: "üser@example.com"},
28+
{name: "decomposed local is not recomposed", email: "U\u0308ser@Example.com", want: "u\u0308ser@example.com"},
29+
{name: "invalid hostname keeps legacy key", email: "User@exa_mple.com", want: "user@exa_mple.com"},
30+
{name: "empty label keeps legacy key", email: "user@a..b.com", want: "user@a..b.com"},
31+
{name: "bare ace prefix label keeps legacy key", email: "user@xn--.example", want: "user@xn--.example"},
32+
{name: "host that maps to nothing keeps legacy key", email: "user@\u00ad", want: "user@\u00ad"},
33+
{
34+
name: "over-long label keeps legacy key",
35+
email: "user@" + strings.Repeat("a", 64) + ".com",
36+
want: "user@" + strings.Repeat("a", 64) + ".com",
37+
},
38+
{
39+
name: "invalid unicode domain keeps spelling, ascii case folded",
40+
email: "User@BÜCHER..Example",
41+
want: "user@bÜcher..example",
42+
},
43+
{name: "space before at is preserved", email: "a b @Example.com", want: "a b @example.com"},
44+
{name: "space after at keeps legacy key", email: "User@ Example.com", want: "user@ example.com"},
45+
{name: "missing local part", email: "@Example.com", want: "@example.com"},
46+
{name: "missing domain", email: "User@", want: "user@"},
47+
{name: "not an email", email: " Not-An-Email ", want: "not-an-email"},
48+
{name: "empty", email: "", want: ""},
49+
}
50+
for _, test := range tests {
51+
t.Run(test.name, func(t *testing.T) {
52+
t.Parallel()
53+
if got := Normalize(test.email); got != test.want {
54+
t.Fatalf("Normalize(%q) = %q, want %q", test.email, got, test.want)
55+
}
56+
})
57+
}
58+
}
59+
60+
func TestEqual(t *testing.T) {
61+
t.Parallel()
62+
same := [][2]string{
63+
{"User@BÜCHER.example", "user@bücher.example"},
64+
{"user@bücher.example", "user@bu\u0308cher.example"},
65+
{"user@bücher.example", "user@xn--bcher-kva.example"},
66+
{"Üser@example.com", "üser@example.com"},
67+
{" user@example.com ", "USER@EXAMPLE.COM"},
68+
}
69+
for _, pair := range same {
70+
if !Equal(pair[0], pair[1]) {
71+
t.Errorf("Equal(%q, %q) = false, want true", pair[0], pair[1])
72+
}
73+
}
74+
different := [][2]string{
75+
{"user@example.com", "user@example.org"},
76+
{"user@example.com", "other@example.com"},
77+
{"user@example.com", "user@example.com."},
78+
{"üser@example.com", "u\u0308ser@example.com"},
79+
}
80+
for _, pair := range different {
81+
if Equal(pair[0], pair[1]) {
82+
t.Errorf("Equal(%q, %q) = true, want false", pair[0], pair[1])
83+
}
84+
}
85+
}
86+
87+
func TestNormalizeIsIdempotent(t *testing.T) {
88+
t.Parallel()
89+
alphabet := []rune("aZ09.-_+@ üÜ\u0308ßẞİ。中文\u0301ex")
90+
r := rand.New(rand.NewPCG(7, 0))
91+
for range 50000 {
92+
runes := make([]rune, r.IntN(24))
93+
for j := range runes {
94+
runes[j] = alphabet[r.IntN(len(alphabet))]
95+
}
96+
input := string(runes)
97+
once := Normalize(input)
98+
if twice := Normalize(once); twice != once {
99+
t.Fatalf("Normalize is not idempotent for %q: %q then %q", input, once, twice)
100+
}
101+
}
102+
}
103+
104+
func asciiCorpus() []string {
105+
const ldh = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-"
106+
domains := []string{
107+
"example.com", "EXAMPLE.COM", "example.com.", "-bad.com", "bad-.com", "ab--cd.com",
108+
"xn--bcher-kva.example", "XN--BCHER-KVA.example", "xn--zzzz.example", "xn--.example",
109+
"example.xn--", "a.xn--.b", "xn--", "a_b.com", "localhost", "1.2.3.4", "[1.2.3.4]",
110+
"exa mple.com", "ex!ample.com", "", ".", "a..b.com", ".example.com",
111+
strings.Repeat("a", 63) + ".com", strings.Repeat("a", 64) + ".com",
112+
strings.Repeat("a.", 130) + "com",
113+
}
114+
r := rand.New(rand.NewPCG(1, 0))
115+
for range 50000 {
116+
labels := make([]string, 1+r.IntN(4))
117+
for j := range labels {
118+
label := make([]byte, 1+r.IntN(12))
119+
for k := range label {
120+
label[k] = ldh[r.IntN(len(ldh))]
121+
}
122+
if r.IntN(10) == 0 {
123+
labels[j] = "xn--" + string(label)
124+
} else {
125+
labels[j] = string(label)
126+
}
127+
}
128+
domains = append(domains, strings.Join(labels, "."))
129+
}
130+
return domains
131+
}
132+
133+
func TestNormalizeASCIIIsFixedPoint(t *testing.T) {
134+
t.Parallel()
135+
locals := []string{"User+Tag", "a b", "a ", "\"quoted@part\"", "x.y_z-9", "A"}
136+
for _, domain := range asciiCorpus() {
137+
for _, local := range locals {
138+
for _, input := range []string{" " + local + "@" + domain + " ", local + "@ " + domain} {
139+
if got, want := Normalize(input), strings.ToLower(strings.TrimSpace(input)); got != want {
140+
t.Fatalf("ASCII input %q changed: got %q, want %q", input, got, want)
141+
}
142+
}
143+
}
144+
}
145+
}
146+
147+
func TestDomainProfileOnlyAddsFailuresToLookup(t *testing.T) {
148+
t.Parallel()
149+
inputs := append(asciiCorpus(),
150+
"bücher.example", "BÜCHER.example", "bu\u0308cher.example", "straße.de", "STRAẞE.de",
151+
"İstanbul.com", "I\u0307stanbul.com", "example。com", "\u0308example.com", "中文.中国",
152+
)
153+
for _, input := range inputs {
154+
want, lookupErr := idna.Lookup.ToASCII(input)
155+
got, err := domain.ToASCII(input)
156+
if lookupErr != nil && err == nil {
157+
t.Fatalf("domain profile accepted %q which idna.Lookup rejects: %v", input, lookupErr)
158+
}
159+
if err == nil && got != want {
160+
t.Fatalf("domain profile ToASCII(%q) = %q, idna.Lookup = %q", input, got, want)
161+
}
162+
}
163+
}

‎internal/httpapi/auth/handler.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ type Store interface {
1717
context.Context,
1818
identitystore.PasswordSignupStartInput,
1919
) (identitystore.PasswordSignupStartRecord, error)
20-
ActiveAuthTokenNormalizedEmail(context.Context, string, string) (string, error)
20+
ActiveAuthTokenEmail(context.Context, string, string) (string, error)
2121
CompletePasswordSignup(
2222
context.Context,
2323
identitystore.CompletePasswordSignupInput,

‎internal/httpapi/auth/oauth_routes.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"time"
1414

1515
"github.com/coreos/go-oidc/v3/oidc"
16+
"github.com/omnara-ai/omnara/internal/emailaddr"
1617
"github.com/omnara-ai/omnara/internal/httpapi/apierror"
1718
"github.com/omnara-ai/omnara/internal/httpapi/openapi"
1819
"github.com/omnara-ai/omnara/internal/storage/identitystore"
@@ -284,15 +285,15 @@ func (h *Handler) oidcIdentity(
284285
if userInfoClaims.Subject == "" || userInfoClaims.Subject != claims.Subject {
285286
return externalIdentity{}, errors.New("oidc userinfo subject mismatch")
286287
}
287-
if claims.Email != "" && userInfoClaims.Email != "" && !strings.EqualFold(claims.Email, userInfoClaims.Email) {
288+
if claims.Email != "" && userInfoClaims.Email != "" && !emailaddr.Equal(claims.Email, userInfoClaims.Email) {
288289
return externalIdentity{}, errors.New("oidc userinfo email mismatch")
289290
}
290291
if claims.Email == "" {
291292
claims.Email = userInfoClaims.Email
292293
claims.EmailVerified = userInfoClaims.EmailVerified
293294
} else if claims.EmailVerified == nil &&
294295
userInfoClaims.Email != "" &&
295-
strings.EqualFold(claims.Email, userInfoClaims.Email) {
296+
emailaddr.Equal(claims.Email, userInfoClaims.Email) {
296297
claims.EmailVerified = userInfoClaims.EmailVerified
297298
}
298299
if claims.Name == "" {

‎internal/httpapi/auth/password_routes.go‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"time"
1111

1212
"github.com/omnara-ai/omnara/internal/authn"
13+
"github.com/omnara-ai/omnara/internal/emailaddr"
1314
"github.com/omnara-ai/omnara/internal/httpapi/apierror"
1415
"github.com/omnara-ai/omnara/internal/httpapi/openapi"
1516
"github.com/omnara-ai/omnara/internal/storage"
@@ -161,7 +162,7 @@ func (h *Handler) completeEmailVerificationRoute(w http.ResponseWriter, r *http.
161162
) {
162163
return
163164
}
164-
normalizedEmail, err := h.store.ActiveAuthTokenNormalizedEmail(
165+
email, err := h.store.ActiveAuthTokenEmail(
165166
r.Context(),
166167
body.Token,
167168
identitystore.UserAuthTokenPurposeEmailVerification,
@@ -170,7 +171,7 @@ func (h *Handler) completeEmailVerificationRoute(w http.ResponseWriter, r *http.
170171
h.writeAuthTokenStorageError(w, r, err)
171172
return
172173
}
173-
passwordHash, ok := passwordHashForNewPassword(w, body.Password, normalizedEmail)
174+
passwordHash, ok := passwordHashForNewPassword(w, body.Password, email)
174175
if !ok {
175176
return
176177
}
@@ -332,7 +333,7 @@ func (h *Handler) completePasswordResetRoute(w http.ResponseWriter, r *http.Requ
332333
) {
333334
return
334335
}
335-
normalizedEmail, err := h.store.ActiveAuthTokenNormalizedEmail(
336+
email, err := h.store.ActiveAuthTokenEmail(
336337
r.Context(),
337338
body.Token,
338339
identitystore.UserAuthTokenPurposePasswordReset,
@@ -341,7 +342,7 @@ func (h *Handler) completePasswordResetRoute(w http.ResponseWriter, r *http.Requ
341342
h.writeAuthTokenStorageError(w, r, err)
342343
return
343344
}
344-
passwordHash, ok := passwordHashForNewPassword(w, body.Password, normalizedEmail)
345+
passwordHash, ok := passwordHashForNewPassword(w, body.Password, email)
345346
if !ok {
346347
return
347348
}
@@ -414,7 +415,7 @@ func (h *Handler) changePasswordRoute(w http.ResponseWriter, r *http.Request) {
414415
h.writeAuthServerError(w, r, err)
415416
return
416417
}
417-
passwordHash, ok := passwordHashForNewPassword(w, body.NewPassword, email.NormalizedEmail)
418+
passwordHash, ok := passwordHashForNewPassword(w, body.NewPassword, email.Email)
418419
if !ok {
419420
return
420421
}
@@ -493,8 +494,8 @@ func (h *Handler) requirePublicAuthJSON(w http.ResponseWriter, r *http.Request)
493494
return true
494495
}
495496

496-
func passwordHashForNewPassword(w http.ResponseWriter, password, normalizedEmail string) (string, bool) {
497-
if err := authn.ValidateNewPassword(password, normalizedEmail); err != nil {
497+
func passwordHashForNewPassword(w http.ResponseWriter, password, email string) (string, bool) {
498+
if err := authn.ValidateNewPassword(password, email); err != nil {
498499
apierror.Write(w, openapi.ErrorCodeValidationFailed, err.Error())
499500
return "", false
500501
}
@@ -510,9 +511,9 @@ func normalizeAuthEmail(email string) (string, string, bool) {
510511
trimmed := strings.TrimSpace(email)
511512
parsed, err := mail.ParseAddress(trimmed)
512513
if err != nil || parsed.Address != trimmed {
513-
return "", identitystore.NormalizeEmail(email), false
514+
return "", emailaddr.Normalize(email), false
514515
}
515-
return parsed.Address, identitystore.NormalizeEmail(parsed.Address), true
516+
return parsed.Address, emailaddr.Normalize(parsed.Address), true
516517
}
517518

518519
func padPublicAuthResponse(ctx context.Context, start time.Time) {

‎internal/httpapi/auth/password_routes_test.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,3 +42,11 @@ func TestSafeSignupReturnToAllowsOnlyDeviceApproval(t *testing.T) {
4242
}
4343
}
4444
}
45+
46+
func TestNormalizeAuthEmailCanonicalizesInternationalizedDomain(t *testing.T) {
47+
t.Parallel()
48+
email, normalized, valid := normalizeAuthEmail("User@BÜCHER.example")
49+
if !valid || email != "User@BÜCHER.example" || normalized != "user@xn--bcher-kva.example" {
50+
t.Fatalf("normalize auth email = (%q, %q, %t)", email, normalized, valid)
51+
}
52+
}

0 commit comments

Comments
 (0)