Skip to content

Commit 2f1c8c6

Browse files
dgellowclaude
dgellow
and
claude
authored
fix: normalize email addresses for admin authentication (#13)
* fix: normalize email addresses for admin authentication - Add centralized email normalization in internal/utils/email.go - Normalize emails to lowercase and trim whitespace - Apply normalization during config loading and auth checks - Add comprehensive tests for email normalization - Fixes issue where admin emails with different cases or whitespace were not recognized 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * style: fix formatting and add missing newlines 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> --------- Co-authored-by: Claude <[email protected]>
1 parent 804763b commit 2f1c8c6

File tree

5 files changed

+271
-3
lines changed

5 files changed

+271
-3
lines changed

‎internal/auth/admin.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55

66
"github.com/dgellow/mcp-front/internal/config"
77
"github.com/dgellow/mcp-front/internal/storage"
8+
"github.com/dgellow/mcp-front/internal/utils"
89
)
910

1011
// IsAdmin checks if a user is admin (either config-based or promoted)
@@ -13,8 +14,11 @@ func IsAdmin(ctx context.Context, email string, adminConfig *config.AdminConfig,
1314
return false
1415
}
1516

17+
// Normalize the input email
18+
normalizedEmail := utils.NormalizeEmail(email)
19+
1620
// Check if user is a config admin (super admin)
17-
if IsConfigAdmin(email, adminConfig) {
21+
if IsConfigAdmin(normalizedEmail, adminConfig) {
1822
return true
1923
}
2024

@@ -23,7 +27,7 @@ func IsAdmin(ctx context.Context, email string, adminConfig *config.AdminConfig,
2327
users, err := store.GetAllUsers(ctx)
2428
if err == nil {
2529
for _, user := range users {
26-
if user.Email == email && user.IsAdmin {
30+
if utils.NormalizeEmail(user.Email) == normalizedEmail && user.IsAdmin {
2731
return true
2832
}
2933
}
@@ -39,8 +43,13 @@ func IsConfigAdmin(email string, adminConfig *config.AdminConfig) bool {
3943
return false
4044
}
4145

46+
// Email should already be normalized by the caller, but normalize anyway for safety
47+
normalizedEmail := utils.NormalizeEmail(email)
48+
4249
for _, adminEmail := range adminConfig.AdminEmails {
43-
if email == adminEmail {
50+
// Admin emails should be normalized during config load, but we normalize here too
51+
// to handle any legacy configs or manual edits
52+
if utils.NormalizeEmail(adminEmail) == normalizedEmail {
4453
return true
4554
}
4655
}

‎internal/auth/admin_test.go

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
package auth
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/dgellow/mcp-front/internal/config"
8+
"github.com/dgellow/mcp-front/internal/storage"
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
)
12+
13+
func TestIsConfigAdmin(t *testing.T) {
14+
adminConfig := &config.AdminConfig{
15+
Enabled: true,
16+
AdminEmails: []string{
17+
"[email protected]",
18+
"[email protected]",
19+
" [email protected] ",
20+
},
21+
}
22+
23+
tests := []struct {
24+
name string
25+
email string
26+
expected bool
27+
}{
28+
{
29+
name: "exact match lowercase",
30+
email: "[email protected]",
31+
expected: true,
32+
},
33+
{
34+
name: "uppercase input for lowercase config",
35+
email: "[email protected]",
36+
expected: true,
37+
},
38+
{
39+
name: "mixed case input",
40+
email: "[email protected]",
41+
expected: true,
42+
},
43+
{
44+
name: "exact match uppercase config",
45+
email: "[email protected]",
46+
expected: true,
47+
},
48+
{
49+
name: "whitespace in input",
50+
email: " [email protected] ",
51+
expected: true,
52+
},
53+
{
54+
name: "match config with whitespace",
55+
email: "[email protected]",
56+
expected: true,
57+
},
58+
{
59+
name: "non-admin email",
60+
email: "[email protected]",
61+
expected: false,
62+
},
63+
{
64+
name: "empty email",
65+
email: "",
66+
expected: false,
67+
},
68+
}
69+
70+
for _, tt := range tests {
71+
t.Run(tt.name, func(t *testing.T) {
72+
result := IsConfigAdmin(tt.email, adminConfig)
73+
assert.Equal(t, tt.expected, result, "IsConfigAdmin(%q, adminConfig)", tt.email)
74+
})
75+
}
76+
}
77+
78+
func TestIsConfigAdmin_NilOrDisabled(t *testing.T) {
79+
tests := []struct {
80+
name string
81+
adminConfig *config.AdminConfig
82+
email string
83+
expected bool
84+
}{
85+
{
86+
name: "nil admin config",
87+
adminConfig: nil,
88+
email: "[email protected]",
89+
expected: false,
90+
},
91+
{
92+
name: "disabled admin config",
93+
adminConfig: &config.AdminConfig{
94+
Enabled: false,
95+
AdminEmails: []string{"[email protected]"},
96+
},
97+
email: "[email protected]",
98+
expected: false,
99+
},
100+
}
101+
102+
for _, tt := range tests {
103+
t.Run(tt.name, func(t *testing.T) {
104+
result := IsConfigAdmin(tt.email, tt.adminConfig)
105+
assert.Equal(t, tt.expected, result, "IsConfigAdmin(%q, %v)", tt.email, tt.adminConfig)
106+
})
107+
}
108+
}
109+
110+
func TestIsAdmin(t *testing.T) {
111+
ctx := context.Background()
112+
113+
// Create a mock storage with a promoted admin
114+
store := storage.NewMemoryStorage()
115+
// Create users and set admin status
116+
require.NoError(t, store.UpsertUser(ctx, "[email protected]"), "Failed to upsert promoted user") // Uppercase to test normalization
117+
require.NoError(t, store.SetUserAdmin(ctx, "[email protected]", true), "Failed to set user as admin")
118+
require.NoError(t, store.UpsertUser(ctx, "[email protected]"), "Failed to upsert regular user")
119+
// [email protected] remains non-admin by default
120+
121+
adminConfig := &config.AdminConfig{
122+
Enabled: true,
123+
AdminEmails: []string{
124+
"[email protected]",
125+
},
126+
}
127+
128+
tests := []struct {
129+
name string
130+
email string
131+
expected bool
132+
}{
133+
{
134+
name: "config admin with exact case",
135+
email: "[email protected]",
136+
expected: true,
137+
},
138+
{
139+
name: "config admin with different case",
140+
email: "[email protected]",
141+
expected: true,
142+
},
143+
{
144+
name: "promoted admin with exact case",
145+
email: "[email protected]",
146+
expected: true,
147+
},
148+
{
149+
name: "promoted admin with different case",
150+
email: "[email protected]",
151+
expected: true,
152+
},
153+
{
154+
name: "regular user",
155+
email: "[email protected]",
156+
expected: false,
157+
},
158+
{
159+
name: "unknown user",
160+
email: "[email protected]",
161+
expected: false,
162+
},
163+
}
164+
165+
for _, tt := range tests {
166+
t.Run(tt.name, func(t *testing.T) {
167+
result := IsAdmin(ctx, tt.email, adminConfig, store)
168+
assert.Equal(t, tt.expected, result, "IsAdmin(ctx, %q, adminConfig, store)", tt.email)
169+
})
170+
}
171+
}

‎internal/config/unmarshal.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66
"regexp"
77
"strings"
88
"time"
9+
10+
"github.com/dgellow/mcp-front/internal/utils"
911
)
1012

1113
// UnmarshalJSON implements custom unmarshaling for MCPClientConfig
@@ -204,6 +206,15 @@ func (p *ProxyConfig) UnmarshalJSON(data []byte) error {
204206
p.Name = raw.Name
205207
p.Admin = raw.Admin
206208

209+
// Normalize admin emails for consistent comparison
210+
if p.Admin != nil && len(p.Admin.AdminEmails) > 0 {
211+
normalizedEmails := make([]string, len(p.Admin.AdminEmails))
212+
for i, email := range p.Admin.AdminEmails {
213+
normalizedEmails[i] = utils.NormalizeEmail(email)
214+
}
215+
p.Admin.AdminEmails = normalizedEmails
216+
}
217+
207218
// Parse BaseURL
208219
if raw.BaseURL != nil {
209220
parsed, err := ParseConfigValue(raw.BaseURL)

‎internal/utils/email.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
package utils
2+
3+
import "strings"
4+
5+
// NormalizeEmail normalizes an email address for consistent comparison
6+
// by converting to lowercase and trimming whitespace
7+
func NormalizeEmail(email string) string {
8+
return strings.ToLower(strings.TrimSpace(email))
9+
}

‎internal/utils/email_test.go

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
package utils
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
)
8+
9+
func TestNormalizeEmail(t *testing.T) {
10+
tests := []struct {
11+
name string
12+
input string
13+
expected string
14+
}{
15+
{
16+
name: "lowercase email",
17+
input: "[email protected]",
18+
expected: "[email protected]",
19+
},
20+
{
21+
name: "uppercase email",
22+
input: "[email protected]",
23+
expected: "[email protected]",
24+
},
25+
{
26+
name: "mixed case email",
27+
input: "[email protected]",
28+
expected: "[email protected]",
29+
},
30+
{
31+
name: "email with leading whitespace",
32+
input: " [email protected]",
33+
expected: "[email protected]",
34+
},
35+
{
36+
name: "email with trailing whitespace",
37+
input: "[email protected] ",
38+
expected: "[email protected]",
39+
},
40+
{
41+
name: "email with surrounding whitespace",
42+
input: " [email protected] ",
43+
expected: "[email protected]",
44+
},
45+
{
46+
name: "email with tabs and newlines",
47+
input: "\t\n[email protected]\n\t",
48+
expected: "[email protected]",
49+
},
50+
{
51+
name: "empty string",
52+
input: "",
53+
expected: "",
54+
},
55+
{
56+
name: "only whitespace",
57+
input: " \t\n ",
58+
expected: "",
59+
},
60+
}
61+
62+
for _, tt := range tests {
63+
t.Run(tt.name, func(t *testing.T) {
64+
result := NormalizeEmail(tt.input)
65+
assert.Equal(t, tt.expected, result, "NormalizeEmail(%q)", tt.input)
66+
})
67+
}
68+
}

0 commit comments

Comments
 (0)