fix(apple): validate id_token iss and aud on Sign in with Apple

[paskal]

Summary

After jwt.ParseWithClaims succeeded the Apple handler accepted any token Apple had signed, regardless of which Sign-in-with-Apple client it was issued to. Per Apple's spec the relying party MUST verify:

• iss == "https://appleid.apple.com"
• aud == ClientID

We did neither. This lets an attacker-controlled Sign-in-with-Apple client (or any sibling service in the same Apple developer team) substitute its own valid id_token and authenticate as the foreign sub. The aud check (not the iss check) is what closes the confused-deputy attack: a sibling service holds a token with the real Apple iss but a different aud, and could replay it here if aud is not enforced.

Change

• v1 (provider/apple.go, golang-jwt/jwt v3.2.2): adds validateAppleIDClaims(claims, expectedAud) helper using VerifyIssuer / VerifyAudience, called immediately after ParseWithClaims. Returns 403 invalid id_token on rejection.
• v2 (v2/provider/apple.go, jwt v5): no helper -- the same checks are wired directly into ParseWithClaims via jwt.WithIssuer(appleIDTokenIssuer) and jwt.WithAudience(ah.conf.ClientID) parser options. The errors.Is(err, jwt.ErrTokenInvalidIssuer | ErrTokenInvalidAudience) branch maps a confused-deputy reject to 403 invalid id_token; key/sig parse failures still go to 500 as before.

Both modules in single PR.

Test

• TestValidateAppleIDClaims (v1 only) -- table-driven helper coverage: wrong-iss, missing-iss, wrong-aud, missing-aud, audience-as-list match. v2 dropped the helper alongside its unit table -- the iss/aud cases are now jwt v5 library responsibility, exercised end-to-end by the integration tests below.
• TestAppleHandler_LoginHandler_RejectsWrongIssuer (v1 + v2) -- integration regression test. Drives the full LoginHandler exchange with a token signed by the test JWK but carrying iss = "https://attacker.example.com". With the fix the handler returns 403 invalid id_token; without, the foreign-iss token authenticates and this test fails on the status-code assertion.
• TestAppleHandler_LoginHandler_RejectsWrongAudience (v1 + v2) -- symmetric to the iss test, drives the flow with a token whose iss is the real Apple issuer but whose aud belongs to a different relying party. This is the actual confused-deputy shape; verified to fail when the aud check is removed.

prepareAppleOauthTest gains an explicit testIDTokenOverride parameter so the integration tests can inject iss / aud while existing callers keep defaults.

go test -race ./... green in both modules.

[coveralls]

Coverage Report for CI Build 25600377390

Coverage increased (+0.03%) to 84.997%

Details

• Coverage increased (+0.03%) from the base build.
• Patch coverage: 11 of 11 lines across 1 file are fully covered (100%).
• No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.

---

Coverage Stats

Relevant Lines:	3426
Covered Lines:	2912
Line Coverage:	85.0%
Coverage Strength:	8.02 hits per line

---

💛 - Coveralls

[umputun]

substantive concerns:

1. integration test only exercises wrong-iss, not wrong-aud. The actual confused-deputy shape (sibling Service ID in the same Apple developer team) carries the correct iss = https://appleid.apple.com and a different aud. The existing TestAppleHandler_LoginHandler_RejectsWrongIssuer would still pass even if the helper's aud branch were silently broken or the call site somehow only checked iss. The test infra already supports it: testIDTokenOverride{aud: "other.example.com"}. Worth adding a symmetric TestAppleHandler_LoginHandler_RejectsWrongAudience in both provider/apple_test.go and v2/provider/apple_test.go.

2. v2 unit test has no multi-element-aud-no-match case. The current "audience as string list with match accepted" case only proves the helper finds a match when one exists in the list. If the v2 loop at v2/provider/apple.go:573-582 were broken to aud[0] == expectedAud, the test would still pass. Adding aud: []interface{}{"other.example.com", "third.example.com"} as a wantErr case closes that.

3. v2 helper is replaceable by jwt v5 parser options. jwt.WithIssuer(appleIDTokenIssuer) + jwt.WithAudience(ah.conf.ClientID) on the existing ParseWithClaims does the same checks against validator.expectedIss / validator.expectedAud, with the library's own list-aware matcher (slices.Contains over ClaimStrings). Drops ~70 lines on the v2 path (helper + unit test + call site). Trade-off is the parse-error branch at v2/provider/apple.go:352-356 would now fire 500 for these failures unless mapped via errors.Is(err, jwt.ErrTokenInvalidIssuer | ErrTokenInvalidAudience | ErrTokenRequiredClaimMissing) to 403. v1 stays as-is, golang-jwt v3.2.2 has no equivalent. Fine to leave the helper if you prefer the v1/v2 symmetry, just flagging the option.

4. minor README nit: the phrase sibling service in the same Apple developer team ... could replay its own valid token reads like the iss check is what stops it, but a sibling shares the same iss. The aud check is what closes it. Could tighten to replay its own valid token (carrying a different aud) or similar.

[paskal]

addressed review feedback in last push:

1. added TestAppleHandler_LoginHandler_RejectsWrongAudience to both v1 (provider/apple_test.go) and v2 (v2/provider/apple_test.go) -- symmetric to the wrong-iss test, drives the LoginHandler with a token whose iss is real Apple but aud is foreign. verified it fails when the aud check is removed.

2. added audience as string list with no match rejected and two malformed-audience cases (malformed audience scalar for aud: 42, malformed audience list element for aud: []any{42, clientID}) to v2 TestValidateAppleIDClaims. that last case lifts v2 validateAppleIDClaims coverage from 88.9% to 100% by hitting the GetAudience err branch.

3. tightened README aud-vs-iss framing -- the paragraph now explicitly says the aud check (not iss) is what closes the confused-deputy attack, with the sibling-service mechanism described in those terms.

skipping the v2 jwt v5 parser-options refactor for now -- can do as a follow-up if useful.

[paskal]

folded the v2 jwt v5 parser-options refactor in:

• replaced the validateAppleIDClaims helper with jwt.WithIssuer(appleIDTokenIssuer) + jwt.WithAudience(ah.conf.ClientID) parser options on the existing jwt.ParseWithClaims call.
• the err return is split via errors.Is(err, jwt.ErrTokenInvalidIssuer|jwt.ErrTokenInvalidAudience) so a confused-deputy reject still returns 403 + "invalid id_token" (security-relevant boundary), while a key/sig parse failure stays 500 (server-side).
• dropped the validateAppleIDClaims function (~17 lines) and the TestValidateAppleIDClaims table (~62 lines) — slices import gone too. v1 untouched (jwt v3.2.2 doesn't have parser options).
• the integration tests TestAppleHandler_LoginHandler_RejectsWrong{Issuer,Audience} still cover the rejection paths end-to-end; the malformed-claim variants are now jwt v5 library responsibility, not ours.

[umputun]

round 1 points all addressed - wrong-aud integration tests in v1+v2, v2 multi-element-no-match (then dropped along with the helper, fine since WithAudience matching is library-tested now), parser-options refactor with errors.Is mapping to 403, README aud-vs-iss framing tightened. Nice that you went the extra mile on the v2 refactor.

one nit - push 2 dropped the validateAppleIDClaims helper from v2 but three test comments still reference it:

• v2/provider/apple_test.go TestAppleHandler_LoginHandler_RejectsWrongIssuer godoc: "With the validateAppleIDClaims call in place..."
• v2/provider/apple_test.go TestAppleHandler_LoginHandler_RejectsWrongAudience godoc: "Without the validateAppleIDClaims aud check..."
• v2/provider/apple_test.go testIDTokenOverride godoc: "exercising the happy path through validateAppleIDClaims"

v1 versions of the same comments are correct since v1 still has the helper, only v2 needs the rewording (something like "the parser-options check at the call site" / "the iss/aud parser options").

coveralls -0.3% is artifactual from deleting the TestValidateAppleIDClaims table in v2 - patch coverage is 13/13. not a blocker.

[paskal]

addressed the v2 godoc nit -- all three references to the now-removed validateAppleIDClaims helper rewritten to mention the parser options instead:

• TestAppleHandler_LoginHandler_RejectsWrongIssuer -> "With the jwt.WithIssuer parser option in place..."
• TestAppleHandler_LoginHandler_RejectsWrongAudience -> "Without the jwt.WithAudience parser option this is the confused-deputy path..."
• testIDTokenOverride -> "...exercising the happy path through the jwt.WithIssuer / jwt.WithAudience parser options."

v1 versions left as-is since v1 still has the helper. force-pushed.