Skip to content

Commit 1ae23be

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 e5f47f5 commit 1ae23be

5 files changed

Lines changed: 270 additions & 20 deletions

File tree

README.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -780,6 +780,15 @@ Then add an Apple provider that accepts the following parameters:
780780
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.
781781
This behaves correctly until a user delete sign in for you service with Apple ID in own Apple account profile (security section).
782782
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.
783+
784+
**Token validation:**
785+
786+
The provider verifies the Apple `id_token` signature against Apple's published JWK, then enforces:
787+
788+
* `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)
789+
* `aud == ClientID` (your Service ID or App ID)
790+
791+
A token signed by Apple but issued for a different `aud` is rejected with `403 invalid id_token`. The `aud` check (not the `iss` check) is what closes the confused-deputy attack: a sibling service in the same Apple developer team (or any other Sign-in-with-Apple client) holds tokens with the real Apple `iss` but their own `aud`, and could replay one of those tokens against this service if `aud` were not enforced.
783792
Provider always get user `UID` (`sub` claim) in `IDToken`.
784793

785794
* 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})
@@ -568,3 +575,21 @@ func presence(s string) string {
568575
}
569576
return "present"
570577
}
578+
579+
// appleIDTokenIssuer is the issuer Apple sets on every id_token issued by Sign in with Apple.
580+
// see https://developer.apple.com/documentation/sign_in_with_apple/sign_in_with_apple_rest_api/verifying_a_user
581+
const appleIDTokenIssuer = "https://appleid.apple.com" // #nosec G101 -- public Apple issuer URL, not a credential
582+
583+
// validateAppleIDClaims checks that the id_token claims have the expected Apple
584+
// issuer and that the audience matches the relying party's client ID. The signature
585+
// must already have been verified by the caller; this only catches confused-deputy
586+
// cases where another Apple-signed token is replayed against the wrong audience.
587+
func validateAppleIDClaims(claims jwt.MapClaims, expectedAud string) error {
588+
if !claims.VerifyIssuer(appleIDTokenIssuer, true) {
589+
return errors.New("invalid id_token issuer")
590+
}
591+
if !claims.VerifyAudience(expectedAud, true) {
592+
return errors.New("invalid id_token audience")
593+
}
594+
return nil
595+
}

provider/apple_test.go

Lines changed: 135 additions & 9 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}}
@@ -327,14 +372,54 @@ func TestAppleHandler_LoginHandler(t *testing.T) {
327372

328373
}
329374

375+
// TestAppleHandler_LoginHandler_RejectsWrongIssuer is the regression-style
376+
// integration test: drives the full LoginHandler exchange flow with a token
377+
// signed by the test JWK but carrying iss other than https://appleid.apple.com.
378+
func TestAppleHandler_LoginHandler_RejectsWrongIssuer(t *testing.T) {
379+
override := testIDTokenOverride{iss: "https://attacker.example.com"}
380+
teardown := prepareAppleOauthTest(t, 8983, 8984, nil, override)
381+
defer teardown()
382+
383+
jar, err := cookiejar.New(nil)
384+
require.NoError(t, err)
385+
client := &http.Client{Jar: jar, Timeout: 5 * time.Second}
386+
387+
resp, err := client.Get("http://localhost:8983/login?site=remark")
388+
require.NoError(t, err)
389+
defer resp.Body.Close()
390+
assert.Equal(t, http.StatusForbidden, resp.StatusCode, "wrong issuer must be rejected at the handler boundary")
391+
body, err := io.ReadAll(resp.Body)
392+
require.NoError(t, err)
393+
assert.Contains(t, string(body), "invalid id_token")
394+
}
395+
396+
// TestAppleHandler_LoginHandler_RejectsWrongAudience is the symmetric
397+
// regression test to TestAppleHandler_LoginHandler_RejectsWrongIssuer.
398+
func TestAppleHandler_LoginHandler_RejectsWrongAudience(t *testing.T) {
399+
override := testIDTokenOverride{aud: "other.example.com"}
400+
teardown := prepareAppleOauthTest(t, 8985, 8986, nil, override)
401+
defer teardown()
402+
403+
jar, err := cookiejar.New(nil)
404+
require.NoError(t, err)
405+
client := &http.Client{Jar: jar, Timeout: 5 * time.Second}
406+
407+
resp, err := client.Get("http://localhost:8985/login?site=remark")
408+
require.NoError(t, err)
409+
defer resp.Body.Close()
410+
assert.Equal(t, http.StatusForbidden, resp.StatusCode, "wrong audience must be rejected at the handler boundary")
411+
body, err := io.ReadAll(resp.Body)
412+
require.NoError(t, err)
413+
assert.Contains(t, string(body), "invalid id_token")
414+
}
415+
330416
// TestAppleHandler_LoginHandlerFromRejectsExternalHost is the regression
331-
// test for the redirect validator on the apple login path: with an allowlist
332-
// policy enabled, /login?from=https://evil.example.com must NOT 302 to evil.
417+
// test for the redirect validator on the apple login path.
333418
func TestAppleHandler_LoginHandlerFromRejectsExternalHost(t *testing.T) {
334419
enablePolicy := func(p *Params) {
335420
p.AllowedRedirectHosts = token.AllowedHostsFunc(func() ([]string, error) { return nil, nil })
336421
}
337-
teardown := prepareAppleOauthTest(t, 8987, 8988, nil, enablePolicy)
422+
teardown := prepareAppleOauthTest(t, 8987, 8988, nil, paramOpts(enablePolicy))
338423
defer teardown()
339424

340425
jar, err := cookiejar.New(nil)
@@ -488,7 +573,25 @@ func prepareAppleHandlerTest(responseMode string, scopes []string) (*AppleHandle
488573
return NewApple(p, aCfg, cl)
489574
}
490575

491-
func prepareAppleOauthTest(t *testing.T, loginPort, authPort int, testToken *string, paramOpts ...func(*Params)) func() {
576+
// paramOpts wraps a Params modifier so it can be passed alongside a
577+
// testIDTokenOverride through prepareAppleOauthTest's variadic any list.
578+
type paramOpts func(*Params)
579+
580+
func prepareAppleOauthTest(t *testing.T, loginPort, authPort int, testToken *string, opts ...any) func() {
581+
var override testIDTokenOverride
582+
var paramMods []func(*Params)
583+
for _, o := range opts {
584+
switch v := o.(type) {
585+
case testIDTokenOverride:
586+
override = v
587+
case paramOpts:
588+
paramMods = append(paramMods, v)
589+
case func(*Params):
590+
paramMods = append(paramMods, v)
591+
default:
592+
t.Fatalf("prepareAppleOauthTest: unsupported option type %T", o)
593+
}
594+
}
492595
signKey, testJWK := createTestSignKeyPairs(t)
493596
provider, err := prepareAppleHandlerTest("", []string{})
494597
assert.NoError(t, err)
@@ -511,7 +614,7 @@ func prepareAppleOauthTest(t *testing.T, loginPort, authPort int, testToken *str
511614
require.NoError(t, err)
512615

513616
// create self-signed JWT
514-
testResponseToken, err := createTestResponseToken(signKey)
617+
testResponseToken, err := createTestResponseTokenWith(signKey, override.issOrDefault(), override.audOrDefault())
515618
require.NoError(t, err)
516619
require.NotEmpty(t, testResponseToken)
517620
if testToken != nil {
@@ -535,7 +638,7 @@ func prepareAppleOauthTest(t *testing.T, loginPort, authPort int, testToken *str
535638

536639
params := Params{URL: "url", Cid: "cid", Csecret: "csecret", JwtService: jwtService,
537640
Issuer: "go-pkgz/auth", L: logger.Std}
538-
for _, opt := range paramOpts {
641+
for _, opt := range paramMods {
539642
opt(&params)
540643
}
541644
provider.Params = params
@@ -647,12 +750,35 @@ func prepareAppleOauthTest(t *testing.T, loginPort, authPort int, testToken *str
647750
}
648751
}
649752

650-
func createTestResponseToken(privKey any) (string, error) {
753+
// testIDTokenOverride lets a test inject a token with non-default iss/aud
754+
// through prepareAppleOauthTest. The default (zero value) produces a token
755+
// with the canonical Apple issuer and the test ClientID, exercising the
756+
// happy path through validateAppleIDClaims.
757+
type testIDTokenOverride struct {
758+
iss string
759+
aud string
760+
}
761+
762+
func (o testIDTokenOverride) issOrDefault() string {
763+
if o.iss == "" {
764+
return appleIDTokenIssuer
765+
}
766+
return o.iss
767+
}
768+
769+
func (o testIDTokenOverride) audOrDefault() string {
770+
if o.aud == "" {
771+
return "auth.example.com"
772+
}
773+
return o.aud
774+
}
775+
776+
func createTestResponseTokenWith(privKey any, iss, aud string) (string, error) {
651777
claims := &jwt.MapClaims{
652-
"iss": "http://go.localhost.test",
778+
"iss": iss,
653779
"iat": time.Now().Unix(),
654780
"exp": time.Now().Add(time.Second * 30).Unix(),
655-
"aud": "go-pkgz/auth",
781+
"aud": aud,
656782
"sub": "userid1",
657783
"email": "test@example.go",
658784
}

v2/provider/apple.go

Lines changed: 19 additions & 2 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"
@@ -345,10 +346,22 @@ func (ah AppleHandler) AuthHandler(w http.ResponseWriter, r *http.Request) {
345346
return
346347
}
347348

348-
// get token claims for extract uid (and email or name if they exist in scope)
349+
// get token claims for extract uid (and email or name if they exist in scope).
350+
// jwt v5 parser options enforce iss == https://appleid.apple.com and
351+
// aud == ClientID inline so we don't need a separate validate pass.
349352
tokenClaims := jwt.MapClaims{}
350-
_, err = jwt.ParseWithClaims(resp.IDToken, tokenClaims, keySet.keyFunc)
353+
_, err = jwt.ParseWithClaims(resp.IDToken, tokenClaims, keySet.keyFunc,
354+
jwt.WithIssuer(appleIDTokenIssuer),
355+
jwt.WithAudience(ah.conf.ClientID))
351356
if err != nil {
357+
// distinguish a confused-deputy reject (iss/aud) from a server-side
358+
// parse/sig failure so the handler returns the same 403 + body as
359+
// before for the security-relevant case.
360+
if errors.Is(err, jwt.ErrTokenInvalidIssuer) || errors.Is(err, jwt.ErrTokenInvalidAudience) {
361+
ah.Logf("[WARN] apple id_token rejected: %s", err.Error())
362+
rest.SendErrorJSON(w, r, ah.L, http.StatusForbidden, nil, "invalid id_token")
363+
return
364+
}
352365
ah.Logf("[ERROR] failed to get claims: " + err.Error())
353366
rest.SendErrorJSON(w, r, ah.L, http.StatusInternalServerError, nil, fmt.Sprintf("failed to token validation, key is invalid: %s", resp.Error))
354367
return
@@ -568,3 +581,7 @@ func presence(s string) string {
568581
}
569582
return "present"
570583
}
584+
585+
// appleIDTokenIssuer is the issuer Apple sets on every id_token issued by Sign in with Apple.
586+
// see https://developer.apple.com/documentation/sign_in_with_apple/sign_in_with_apple_rest_api/verifying_a_user
587+
const appleIDTokenIssuer = "https://appleid.apple.com" // #nosec G101 -- public Apple issuer URL, not a credential

0 commit comments

Comments
 (0)