Skip to content

Commit 563ab54

Browse files
paskalpaskal
committed
fix(apple): validate id_token iss and aud on Sign in with Apple
After ParseWithClaims succeeded the Apple handler accepted any token Apple had signed, regardless of which Sign-in-with-Apple client it was issued to. The relying party MUST verify iss == https://appleid.apple.com and aud == ClientID per Apple's spec; we did neither, which let an attacker-controlled Sign-in-with-Apple client (or a sibling service in the same Apple developer team) substitute its own id_token and authenticate as the foreign sub. Add validateAppleIDClaims helper, run it after ParseWithClaims, return 403 with "invalid id_token" on rejection. Same fix applied to v1 (github.com/golang-jwt/jwt v4 API: VerifyIssuer/VerifyAudience) and v2 (jwt v5 API: GetIssuer/GetAudience), single PR. Update the test fixture createTestResponseToken to use realistic iss/aud so existing happy-path integration tests keep passing; new TestValidateAppleIDClaims table-driven tests cover wrong-iss, missing-iss, wrong-aud, missing-aud rejection (and audience-as-list match for v2).
1 parent 66189f0 commit 563ab54

4 files changed

Lines changed: 161 additions & 4 deletions

File tree

‎provider/apple.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,12 @@ func (ah AppleHandler) AuthHandler(w http.ResponseWriter, r *http.Request) {
354354
return
355355
}
356356

357+
if err = validateAppleIDClaims(tokenClaims, ah.conf.ClientID); err != nil {
358+
ah.Logf("[WARN] apple id_token rejected: %s", err.Error())
359+
rest.SendErrorJSON(w, r, ah.L, http.StatusForbidden, nil, "invalid id_token")
360+
return
361+
}
362+
357363
u := ah.mapUser(tokenClaims)
358364

359365
u, err = setAvatar(ah.AvatarSaver, u, &http.Client{Timeout: 5 * time.Second})
@@ -544,3 +550,21 @@ func (ah AppleHandler) makeRedirURL(path string) string {
544550

545551
return strings.TrimRight(ah.URL, "/") + strings.TrimSuffix(newPath, "/") + urlCallbackSuffix
546552
}
553+
554+
// appleIDTokenIssuer is the issuer Apple sets on every id_token issued by Sign in with Apple.
555+
// see https://developer.apple.com/documentation/sign_in_with_apple/sign_in_with_apple_rest_api/verifying_a_user
556+
const appleIDTokenIssuer = "https://appleid.apple.com"
557+
558+
// validateAppleIDClaims checks that the id_token claims have the expected Apple
559+
// issuer and that the audience matches the relying party's client ID. The signature
560+
// must already have been verified by the caller; this only catches confused-deputy
561+
// cases where another Apple-signed token is replayed against the wrong audience.
562+
func validateAppleIDClaims(claims jwt.MapClaims, expectedAud string) error {
563+
if !claims.VerifyIssuer(appleIDTokenIssuer, true) {
564+
return fmt.Errorf("invalid id_token issuer")
565+
}
566+
if !claims.VerifyAudience(expectedAud, true) {
567+
return fmt.Errorf("invalid id_token audience")
568+
}
569+
return nil
570+
}

‎provider/apple_test.go‎

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,51 @@ func TestAppleHandlerCreateClientSecret(t *testing.T) {
177177
assert.Equal(t, "auth.example.com", testClaims["sub"])
178178
}
179179

180+
func TestValidateAppleIDClaims(t *testing.T) {
181+
const clientID = "auth.example.com"
182+
tests := []struct {
183+
name string
184+
claims jwt.MapClaims
185+
wantErr string
186+
}{
187+
{
188+
name: "valid iss and aud accepted",
189+
claims: jwt.MapClaims{"iss": "https://appleid.apple.com", "aud": clientID, "sub": "u1"},
190+
},
191+
{
192+
name: "wrong issuer rejected",
193+
claims: jwt.MapClaims{"iss": "https://attacker.example.com", "aud": clientID, "sub": "u1"},
194+
wantErr: "invalid id_token issuer",
195+
},
196+
{
197+
name: "missing issuer rejected",
198+
claims: jwt.MapClaims{"aud": clientID, "sub": "u1"},
199+
wantErr: "invalid id_token issuer",
200+
},
201+
{
202+
name: "wrong audience rejected",
203+
claims: jwt.MapClaims{"iss": "https://appleid.apple.com", "aud": "other.example.com", "sub": "u1"},
204+
wantErr: "invalid id_token audience",
205+
},
206+
{
207+
name: "missing audience rejected",
208+
claims: jwt.MapClaims{"iss": "https://appleid.apple.com", "sub": "u1"},
209+
wantErr: "invalid id_token audience",
210+
},
211+
}
212+
for _, tt := range tests {
213+
t.Run(tt.name, func(t *testing.T) {
214+
err := validateAppleIDClaims(tt.claims, clientID)
215+
if tt.wantErr == "" {
216+
require.NoError(t, err)
217+
return
218+
}
219+
require.Error(t, err)
220+
assert.Contains(t, err.Error(), tt.wantErr)
221+
})
222+
}
223+
}
224+
180225
func TestAppleParseUserData(t *testing.T) {
181226

182227
ah := AppleHandler{Params: Params{L: logger.NoOp}}
@@ -594,11 +639,15 @@ func prepareAppleOauthTest(t *testing.T, loginPort, authPort int, testToken *str
594639
}
595640

596641
func createTestResponseToken(privKey interface{}) (string, error) {
642+
return createTestResponseTokenWith(privKey, appleIDTokenIssuer, "auth.example.com")
643+
}
644+
645+
func createTestResponseTokenWith(privKey interface{}, iss, aud string) (string, error) {
597646
claims := &jwt.MapClaims{
598-
"iss": "http://go.localhost.test",
647+
"iss": iss,
599648
"iat": time.Now().Unix(),
600649
"exp": time.Now().Add(time.Second * 30).Unix(),
601-
"aud": "go-pkgz/auth",
650+
"aud": aud,
602651
"sub": "userid1",
603652
"email": "test@example.go",
604653
}

‎v2/provider/apple.go‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,12 @@ func (ah AppleHandler) AuthHandler(w http.ResponseWriter, r *http.Request) {
354354
return
355355
}
356356

357+
if err = validateAppleIDClaims(tokenClaims, ah.conf.ClientID); err != nil {
358+
ah.Logf("[WARN] apple id_token rejected: %s", err.Error())
359+
rest.SendErrorJSON(w, r, ah.L, http.StatusForbidden, nil, "invalid id_token")
360+
return
361+
}
362+
357363
u := ah.mapUser(tokenClaims)
358364

359365
u, err = setAvatar(ah.AvatarSaver, u, &http.Client{Timeout: 5 * time.Second})
@@ -549,3 +555,28 @@ func (ah AppleHandler) makeRedirURL(path string) string {
549555

550556
return strings.TrimRight(ah.URL, "/") + strings.TrimSuffix(newPath, "/") + urlCallbackSuffix
551557
}
558+
559+
// appleIDTokenIssuer is the issuer Apple sets on every id_token issued by Sign in with Apple.
560+
// see https://developer.apple.com/documentation/sign_in_with_apple/sign_in_with_apple_rest_api/verifying_a_user
561+
const appleIDTokenIssuer = "https://appleid.apple.com"
562+
563+
// validateAppleIDClaims checks that the id_token claims have the expected Apple
564+
// issuer and that the audience matches the relying party's client ID. The signature
565+
// must already have been verified by the caller; this only catches confused-deputy
566+
// cases where another Apple-signed token is replayed against the wrong audience.
567+
func validateAppleIDClaims(claims jwt.MapClaims, expectedAud string) error {
568+
iss, err := claims.GetIssuer()
569+
if err != nil || iss != appleIDTokenIssuer {
570+
return fmt.Errorf("invalid id_token issuer")
571+
}
572+
aud, err := claims.GetAudience()
573+
if err != nil {
574+
return fmt.Errorf("invalid id_token audience")
575+
}
576+
for _, a := range aud {
577+
if a == expectedAud {
578+
return nil
579+
}
580+
}
581+
return fmt.Errorf("invalid id_token audience")
582+
}

‎v2/provider/apple_test.go‎

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,55 @@ func TestAppleHandlerCreateClientSecret(t *testing.T) {
177177
assert.Equal(t, "auth.example.com", testClaims["sub"])
178178
}
179179

180+
func TestValidateAppleIDClaims(t *testing.T) {
181+
const clientID = "auth.example.com"
182+
tests := []struct {
183+
name string
184+
claims jwt.MapClaims
185+
wantErr string
186+
}{
187+
{
188+
name: "valid iss and aud accepted",
189+
claims: jwt.MapClaims{"iss": "https://appleid.apple.com", "aud": clientID, "sub": "u1"},
190+
},
191+
{
192+
name: "wrong issuer rejected",
193+
claims: jwt.MapClaims{"iss": "https://attacker.example.com", "aud": clientID, "sub": "u1"},
194+
wantErr: "invalid id_token issuer",
195+
},
196+
{
197+
name: "missing issuer rejected",
198+
claims: jwt.MapClaims{"aud": clientID, "sub": "u1"},
199+
wantErr: "invalid id_token issuer",
200+
},
201+
{
202+
name: "wrong audience rejected",
203+
claims: jwt.MapClaims{"iss": "https://appleid.apple.com", "aud": "other.example.com", "sub": "u1"},
204+
wantErr: "invalid id_token audience",
205+
},
206+
{
207+
name: "missing audience rejected",
208+
claims: jwt.MapClaims{"iss": "https://appleid.apple.com", "sub": "u1"},
209+
wantErr: "invalid id_token audience",
210+
},
211+
{
212+
name: "audience as string list with match accepted",
213+
claims: jwt.MapClaims{"iss": "https://appleid.apple.com", "aud": []interface{}{"other", clientID}, "sub": "u1"},
214+
},
215+
}
216+
for _, tt := range tests {
217+
t.Run(tt.name, func(t *testing.T) {
218+
err := validateAppleIDClaims(tt.claims, clientID)
219+
if tt.wantErr == "" {
220+
require.NoError(t, err)
221+
return
222+
}
223+
require.Error(t, err)
224+
assert.Contains(t, err.Error(), tt.wantErr)
225+
})
226+
}
227+
}
228+
180229
func TestAppleParseUserData(t *testing.T) {
181230

182231
ah := AppleHandler{Params: Params{L: logger.NoOp}}
@@ -625,11 +674,15 @@ func prepareAppleOauthTest(t *testing.T, loginPort, authPort int, testToken *str
625674
}
626675

627676
func createTestResponseToken(privKey interface{}) (string, error) {
677+
return createTestResponseTokenWith(privKey, appleIDTokenIssuer, "auth.example.com")
678+
}
679+
680+
func createTestResponseTokenWith(privKey interface{}, iss, aud string) (string, error) {
628681
claims := &jwt.MapClaims{
629-
"iss": "http://go.localhost.test",
682+
"iss": iss,
630683
"iat": time.Now().Unix(),
631684
"exp": time.Now().Add(time.Second * 30).Unix(),
632-
"aud": "go-pkgz/auth",
685+
"aud": aud,
633686
"sub": "userid1",
634687
"email": "test@example.go",
635688
}

0 commit comments

Comments
 (0)