go-pkgz / auth / 25586235045

85%

master: 85%

Build Type

Pull #280

github

Committed by paskal

Commit Message

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.

Pull Request Pull Request #280: fix(apple): validate id_token iss and aud on Sign in with Apple

Coverage Stats

7 of 10 new or added lines in 1 file covered. (70.0%)

42 existing lines in 1 file now uncovered.

2840 of 3343 relevant lines covered (84.95%)

7.98 hits per line