Skip to content

Commit b4dca67

Browse files
fix: consistently handle string-valued boolean fields from non-compliant OIDC providers (#791)
AWS Cognito (and potentially other providers) return `email_verified` and `phone_number_verified` as strings (`"true"`/`"false"`) instead of proper JSON booleans, violating the [OIDC specification](https://openid.net/specs/openid-connect-basic-1_0.html#StandardClaims). AWS Documentation confirms this: > Currently, Amazon Cognito returns the values for email_verified and phone_number_verified as strings. _Source: https://docs.aws.amazon.com/cognito/latest/developerguide/userinfo-endpoint.html#get-userinfo-response-sample_ ### The Problem The `zitadel/oidc` library currently handles this inconsistently: - ✅ `EmailVerified` uses the custom `Bool` type (added in #139) - ❌ `PhoneNumberVerified` uses Go's standard `bool` This forces developers to handle semantically identical fields differently: ```go // Currently inconsistent code path userInfo.EmailVerified = oidc.Bool(emailValue) // Cast userInfo.PhoneNumberVerified = phoneValue // No cast ``` Additionally, the existing `Bool.UnmarshalJSON` implementation meant that false values couldn't overwrite true. ### Solution Applied `Bool` type consistently to both fields and simplified `Bool.UnmarshalJSON` using a direct switch statement to: - Handle standard JSON booleans (true/false) - Handle AWS Cognito string format ("true"/"false") - Return errors on invalid input instead of silently failing - Allow false to overwrite true Updated tests to match codebase conventions, as well. ### Impact `PhoneNumberVerified` changes from `bool` to `Bool` (type alias of `bool`). Most consumer code should work as-is since `Bool` is just a type alias. Direct type assertions would need updating. ### Definition of Ready - [X] I am happy with the code - [X] Short description of the feature/issue is added in the pr description - [ ] PR is linked to the corresponding user story - [X] Acceptance criteria are met - [ ] All open todos and follow ups are defined in a new ticket and justified - [ ] Deviations from the acceptance criteria and design are agreed with the PO and documented. - [X] No debug or dead code - [X] My code has no repetitions - [X] Critical parts are tested automatically - [x] Where possible E2E tests are implemented - [X] Documentation/examples are up-to-date - [ ] All non-functional requirements are met - [x] Functionality of the acceptance criteria is checked manually on the dev system. Co-authored-by: Wim Van Laer <[email protected]>
1 parent 8138813 commit b4dca67

File tree

3 files changed

+134
-63
lines changed

3 files changed

+134
-63
lines changed

example/server/storage/storage.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -676,7 +676,7 @@ func (s *Storage) setUserinfo(ctx context.Context, userInfo *oidc.UserInfo, user
676676
userInfo.Locale = oidc.NewLocale(user.PreferredLanguage)
677677
case oidc.ScopePhone:
678678
userInfo.PhoneNumber = user.Phone
679-
userInfo.PhoneNumberVerified = user.PhoneVerified
679+
userInfo.PhoneNumberVerified = oidc.Bool(user.PhoneVerified)
680680
case CustomScope:
681681
// you can also have a custom scope and assert public or custom claims based on that
682682
userInfo.AppendClaims(CustomClaim, customClaim(clientID))

pkg/oidc/userinfo.go

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,35 @@
11
package oidc
22

3+
import (
4+
"fmt"
5+
)
6+
7+
type Bool bool
8+
9+
// UnmarshalJSON handles both standard JSON boolean values and string representations.
10+
// This is necessary because some OIDC providers (notably AWS Cognito) incorrectly return
11+
// boolean fields like email_verified and phone_number_verified as strings ("true"/"false")
12+
// instead of proper JSON booleans, violating the OIDC specification.
13+
//
14+
// The method first attempts standard boolean unmarshaling, and falls back to string
15+
// parsing if that fails, making it compatible with both compliant and non-compliant providers.
16+
//
17+
// Ref:
18+
// - https://openid.net/specs/openid-connect-basic-1_0.html#StandardClaims
19+
// - https://docs.aws.amazon.com/cognito/latest/developerguide/userinfo-endpoint.html
20+
func (bs *Bool) UnmarshalJSON(data []byte) error {
21+
s := string(data)
22+
switch s {
23+
case "true", `"true"`:
24+
*bs = true
25+
case "false", `"false"`:
26+
*bs = false
27+
default:
28+
return fmt.Errorf("cannot unmarshal %s into Bool", s)
29+
}
30+
return nil
31+
}
32+
333
// UserInfo implements OpenID Connect Core 1.0, section 5.1.
434
// https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims.
535
type UserInfo struct {
@@ -64,25 +94,12 @@ type UserInfoProfile struct {
6494
type UserInfoEmail struct {
6595
Email string `json:"email,omitempty"`
6696

67-
// Handle providers that return email_verified as a string
68-
// https://forums.aws.amazon.com/thread.jspa?messageID=949441&#949441
69-
// https://discuss.elastic.co/t/openid-error-after-authenticating-against-aws-cognito/206018/11
7097
EmailVerified Bool `json:"email_verified,omitempty"`
7198
}
7299

73-
type Bool bool
74-
75-
func (bs *Bool) UnmarshalJSON(data []byte) error {
76-
if string(data) == "true" || string(data) == `"true"` {
77-
*bs = true
78-
}
79-
80-
return nil
81-
}
82-
83100
type UserInfoPhone struct {
84101
PhoneNumber string `json:"phone_number,omitempty"`
85-
PhoneNumberVerified bool `json:"phone_number_verified,omitempty"`
102+
PhoneNumberVerified Bool `json:"phone_number_verified,omitempty"`
86103
}
87104

88105
type UserInfoAddress struct {

pkg/oidc/userinfo_test.go

Lines changed: 102 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -62,58 +62,112 @@ func TestUserInfoMarshal(t *testing.T) {
6262
assert.Equal(t, out, out2)
6363
}
6464

65-
func TestUserInfoEmailVerifiedUnmarshal(t *testing.T) {
65+
// TestUserInfoVerifiedFieldsUnmarshal ensures email_verified and phone_number_verified
66+
// handle both standard booleans and AWS Cognito's non-compliant strings.
67+
func TestUserInfoVerifiedFieldsUnmarshal(t *testing.T) {
6668
t.Parallel()
6769

68-
t.Run("unmarshal email_verified from json bool true", func(t *testing.T) {
69-
jsonBool := []byte(`{"email": "[email protected]", "email_verified": true}`)
70-
71-
var uie UserInfoEmail
70+
tests := []struct {
71+
name string
72+
json string
73+
wantEmailVerified Bool
74+
wantPhoneVerified Bool
75+
}{
76+
{
77+
name: "booleans true",
78+
json: `{"email_verified": true, "phone_number_verified": true}`,
79+
wantEmailVerified: true,
80+
wantPhoneVerified: true,
81+
},
82+
{
83+
name: "booleans false",
84+
json: `{"email_verified": false, "phone_number_verified": false}`,
85+
wantEmailVerified: false,
86+
wantPhoneVerified: false,
87+
},
88+
{
89+
name: "strings true",
90+
json: `{"email_verified": "true", "phone_number_verified": "true"}`,
91+
wantEmailVerified: true,
92+
wantPhoneVerified: true,
93+
},
94+
{
95+
name: "strings false",
96+
json: `{"email_verified": "false", "phone_number_verified": "false"}`,
97+
wantEmailVerified: false,
98+
wantPhoneVerified: false,
99+
},
100+
{
101+
name: "mixed bool/string",
102+
json: `{"email_verified": true, "phone_number_verified": "false"}`,
103+
wantEmailVerified: true,
104+
wantPhoneVerified: false,
105+
},
106+
}
72107

73-
err := json.Unmarshal(jsonBool, &uie)
74-
assert.NoError(t, err)
75-
assert.Equal(t, UserInfoEmail{
76-
77-
EmailVerified: true,
78-
}, uie)
79-
})
108+
for _, tt := range tests {
109+
t.Run(tt.name, func(t *testing.T) {
110+
var got UserInfo
111+
err := json.Unmarshal([]byte(tt.json), &got)
112+
assert.NoError(t, err)
113+
assert.Equal(t, tt.wantEmailVerified, got.EmailVerified)
114+
assert.Equal(t, tt.wantPhoneVerified, got.PhoneNumberVerified)
115+
})
116+
}
117+
}
80118

81-
t.Run("unmarshal email_verified from json string true", func(t *testing.T) {
82-
jsonBool := []byte(`{"email": "[email protected]", "email_verified": "true"}`)
119+
// TestBoolUnmarshal verifies the Bool type handles various inputs correctly.
120+
func TestBoolUnmarshal(t *testing.T) {
121+
t.Parallel()
83122

84-
var uie UserInfoEmail
123+
tests := []struct {
124+
name string
125+
input string
126+
want Bool
127+
wantErr bool
128+
}{
129+
{
130+
name: "bool true",
131+
input: `true`,
132+
want: true,
133+
},
134+
{
135+
name: "bool false",
136+
input: `false`,
137+
want: false,
138+
},
139+
{
140+
name: "string true",
141+
input: `"true"`,
142+
want: true,
143+
},
144+
{
145+
name: "string false",
146+
input: `"false"`,
147+
want: false,
148+
},
149+
{
150+
name: "invalid string",
151+
input: `"yes"`,
152+
wantErr: true,
153+
},
154+
{
155+
name: "number",
156+
input: `1`,
157+
wantErr: true,
158+
},
159+
}
85160

86-
err := json.Unmarshal(jsonBool, &uie)
87-
assert.NoError(t, err)
88-
assert.Equal(t, UserInfoEmail{
89-
90-
EmailVerified: true,
91-
}, uie)
92-
})
93-
94-
t.Run("unmarshal email_verified from json bool false", func(t *testing.T) {
95-
jsonBool := []byte(`{"email": "[email protected]", "email_verified": false}`)
96-
97-
var uie UserInfoEmail
98-
99-
err := json.Unmarshal(jsonBool, &uie)
100-
assert.NoError(t, err)
101-
assert.Equal(t, UserInfoEmail{
102-
103-
EmailVerified: false,
104-
}, uie)
105-
})
106-
107-
t.Run("unmarshal email_verified from json string false", func(t *testing.T) {
108-
jsonBool := []byte(`{"email": "[email protected]", "email_verified": "false"}`)
109-
110-
var uie UserInfoEmail
111-
112-
err := json.Unmarshal(jsonBool, &uie)
113-
assert.NoError(t, err)
114-
assert.Equal(t, UserInfoEmail{
115-
116-
EmailVerified: false,
117-
}, uie)
118-
})
161+
for _, tt := range tests {
162+
t.Run(tt.name, func(t *testing.T) {
163+
var got Bool
164+
err := json.Unmarshal([]byte(tt.input), &got)
165+
if tt.wantErr {
166+
assert.Error(t, err)
167+
return
168+
}
169+
assert.NoError(t, err)
170+
assert.Equal(t, tt.want, got)
171+
})
172+
}
119173
}

0 commit comments

Comments
 (0)