Skip to content

Commit e6bb5c3

Browse files
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 v3.2.2 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. Tests: * TestValidateAppleIDClaims -- table-driven coverage of the helper: wrong-iss, missing-iss, wrong-aud, missing-aud rejection (and audience-as-list match for v2). * TestAppleHandler_LoginHandler_RejectsWrongIssuer -- integration regression test at the handler boundary. Drives the full exchange flow with a token signed by the test JWK but iss = attacker.example.com. With the fix in place the handler returns 403 invalid id_token; if the validateAppleIDClaims call site is reverted the foreign-iss token authenticates (200 with a JWT) and this test fails on the status-code assertion. The unit-level helper test alone wouldn't catch a missing call. prepareAppleOauthTest gains an explicit testIDTokenOverride parameter so the regression test can inject its own iss/aud while existing callers keep their defaults.
1 parent 66189f0 commit e6bb5c3

5 files changed

Lines changed: 275 additions & 14 deletions

File tree

README.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -732,6 +732,15 @@ Then add an Apple provider that accepts the following parameters:
732732
Subsequent logins to your app using Sign In with Apple with the same account do not share any user info and will only return a user identifier in IDToken claims.
733733
This behaves correctly until a user delete sign in for you service with Apple ID in own Apple account profile (security section).
734734
It is recommend that you securely cache the at first login containing the user info for bind it with a user UID at next login.
735+
736+
**Token validation:**
737+
738+
The provider verifies the Apple `id_token` signature against Apple's published JWK, then enforces:
739+
740+
* `iss == "https://appleid.apple.com"` -- per Apple's Sign-in-with-Apple [REST API spec](https://developer.apple.com/documentation/sign_in_with_apple/sign_in_with_apple_rest_api/verifying_a_user)
741+
* `aud == ClientID` (your Service ID or App ID)
742+
743+
A token signed by Apple but issued for a different `aud` is rejected with `403 invalid id_token`. This prevents a confused-deputy attack where a sibling service in the same Apple developer team (or any other Sign-in-with-Apple client) could replay its own valid token against this service.
735744
Provider always get user `UID` (`sub` claim) in `IDToken`.
736745

737746
* Apple doesn't have an API for fetch avatar and user info.

provider/apple.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"crypto/x509"
1414
"encoding/json"
1515
"encoding/pem"
16+
"errors"
1617
"fmt"
1718
"io"
1819
"net/http"
@@ -354,6 +355,12 @@ func (ah AppleHandler) AuthHandler(w http.ResponseWriter, r *http.Request) {
354355
return
355356
}
356357

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

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

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

provider/apple_test.go

Lines changed: 103 additions & 5 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}}
@@ -307,6 +352,32 @@ func TestAppleHandler_LoginHandler(t *testing.T) {
307352

308353
}
309354

355+
// TestAppleHandler_LoginHandler_RejectsWrongIssuer is the regression-style
356+
// integration test: drives the full LoginHandler exchange flow with a token
357+
// signed by the test JWK but carrying iss other than https://appleid.apple.com.
358+
// With the validateAppleIDClaims call in place the handler returns 403; if
359+
// the call site is reverted the foreign-iss token authenticates and the
360+
// handler returns 200 with a JWT cookie -- this test then fails on the
361+
// status-code assertion. Same kind of property check but at the handler
362+
// boundary (the unit-level helper test alone won't catch a missing call).
363+
func TestAppleHandler_LoginHandler_RejectsWrongIssuer(t *testing.T) {
364+
override := testIDTokenOverride{iss: "https://attacker.example.com"}
365+
teardown := prepareAppleOauthTest(t, 8983, 8984, nil, override)
366+
defer teardown()
367+
368+
jar, err := cookiejar.New(nil)
369+
require.NoError(t, err)
370+
client := &http.Client{Jar: jar, Timeout: 5 * time.Second}
371+
372+
resp, err := client.Get("http://localhost:8983/login?site=remark")
373+
require.NoError(t, err)
374+
defer resp.Body.Close()
375+
assert.Equal(t, http.StatusForbidden, resp.StatusCode, "wrong issuer must be rejected at the handler boundary")
376+
body, err := io.ReadAll(resp.Body)
377+
require.NoError(t, err)
378+
assert.Contains(t, string(body), "invalid id_token")
379+
}
380+
310381
func TestAppleHandler_LogoutHandler(t *testing.T) {
311382

312383
teardown := prepareAppleOauthTest(t, 8691, 8692, nil)
@@ -437,7 +508,11 @@ func prepareAppleHandlerTest(responseMode string, scopes []string) (*AppleHandle
437508
return NewApple(p, aCfg, cl)
438509
}
439510

440-
func prepareAppleOauthTest(t *testing.T, loginPort, authPort int, testToken *string) func() {
511+
func prepareAppleOauthTest(t *testing.T, loginPort, authPort int, testToken *string, overrides ...testIDTokenOverride) func() {
512+
var override testIDTokenOverride
513+
if len(overrides) > 0 {
514+
override = overrides[0]
515+
}
441516
signKey, testJWK := createTestSignKeyPairs(t)
442517
provider, err := prepareAppleHandlerTest("", []string{})
443518
assert.NoError(t, err)
@@ -460,7 +535,7 @@ func prepareAppleOauthTest(t *testing.T, loginPort, authPort int, testToken *str
460535
require.NoError(t, err)
461536

462537
// create self-signed JWT
463-
testResponseToken, err := createTestResponseToken(signKey)
538+
testResponseToken, err := createTestResponseTokenWith(signKey, override.issOrDefault(), override.audOrDefault())
464539
require.NoError(t, err)
465540
require.NotEmpty(t, testResponseToken)
466541
if testToken != nil {
@@ -593,12 +668,35 @@ func prepareAppleOauthTest(t *testing.T, loginPort, authPort int, testToken *str
593668
}
594669
}
595670

596-
func createTestResponseToken(privKey interface{}) (string, error) {
671+
// testIDTokenOverride lets a test inject a token with non-default iss/aud
672+
// through prepareAppleOauthTest. The default (zero value) produces a token
673+
// with the canonical Apple issuer and the test ClientID, exercising the
674+
// happy path through validateAppleIDClaims.
675+
type testIDTokenOverride struct {
676+
iss string
677+
aud string
678+
}
679+
680+
func (o testIDTokenOverride) issOrDefault() string {
681+
if o.iss == "" {
682+
return appleIDTokenIssuer
683+
}
684+
return o.iss
685+
}
686+
687+
func (o testIDTokenOverride) audOrDefault() string {
688+
if o.aud == "" {
689+
return "auth.example.com"
690+
}
691+
return o.aud
692+
}
693+
694+
func createTestResponseTokenWith(privKey interface{}, iss, aud string) (string, error) {
597695
claims := &jwt.MapClaims{
598-
"iss": "http://go.localhost.test",
696+
"iss": iss,
599697
"iat": time.Now().Unix(),
600698
"exp": time.Now().Add(time.Second * 30).Unix(),
601-
"aud": "go-pkgz/auth",
699+
"aud": aud,
602700
"sub": "userid1",
603701
"email": "test@example.go",
604702
}

v2/provider/apple.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"crypto/x509"
1414
"encoding/json"
1515
"encoding/pem"
16+
"errors"
1617
"fmt"
1718
"io"
1819
"net/http"
@@ -354,6 +355,12 @@ func (ah AppleHandler) AuthHandler(w http.ResponseWriter, r *http.Request) {
354355
return
355356
}
356357

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

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

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

0 commit comments

Comments
 (0)