chore(security)!: refuse tokens without aud

readme.md

@@ -81,8 +81,7 @@ the key set: a key that left the JWKS stops working. `JWT_PUBLIC_KEY` is used on
 The token comes from Auth's `tbp_access` cookie or a `Bearer` header, whichever verifies first. Every token names its kind in `aud`
 (`tbp-login`, `tbp-public-profile`, `tbp-internal`, with `iss=tbp-auth`); a login's `sub` is the user id. Core reads
 `steamId` and the active `role` (missing or unknown = USER) from the claims and grants that role and every lower one
-(`USER < AUDITOR < PLATFORM_ADMIN`). A token without `aud` comes from an Auth from before item 26 (kind in `typ`,
-user id in `userId`) and is still accepted; item 29 removes that path. The claims are pinned in
+(`USER < AUDITOR < PLATFORM_ADMIN`). A token without `aud` is refused. The claims are pinned in
 `src/test/resources/contract/auth-core-contract.json`, the same file as in Auth; the tests build tokens only from it.
 `POST`, `PUT` and `DELETE` need an `X-TBP-Client` or `Authorization` header (CSRF guard), and CORS allows only the
 frontend origin.

src/main/java/core_service/configuration/security/JwtService.java

@@ -10,7 +10,6 @@ import io.jsonwebtoken.Jwts;
 import org.springframework.stereotype.Service;
 
 import java.util.List;
-import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
@@ -18,9 +17,8 @@ import java.util.stream.Collectors;
 /**
  * Verifies JWTs issued by tbp-auth-service. Verify-only: {@link JwksKeyLocator} finds the Ed25519 public key.
  * <p>
- * A token names its kind in {@code aud}, and {@code sub} is the user id of a login. A token without {@code aud} comes
- * from an Auth from before item 26: its kind is in {@code typ} and the user id in {@code userId}. Item 29 removes
- * that path.
+ * A token names its kind in {@code aud}, and {@code sub} is the user id of a login. A token without {@code aud} is
+ * refused.
  */
 @Service
 public class JwtService implements ProfileTicketVerifier {
@@ -29,10 +27,6 @@ public class JwtService implements ProfileTicketVerifier {
     static final String LOGIN_AUDIENCE = "tbp-login";
     static final String PROFILE_TICKET_AUDIENCE = "tbp-public-profile";
     static final String SERVICE_AUDIENCE = "tbp-internal";
-    // The kind of a token before item 26; item 29 removes it.
-    private static final String TYPE_CLAIM = "typ";
-    private static final String PROFILE_TICKET_TYPE = "public-profile";
-    private static final String SERVICE_TOKEN_TYPE = "service";
     private static final String ROLE_CLAIM = "role";
 
     private final JwtParser parser;
@@ -45,18 +39,9 @@ public class JwtService implements ProfileTicketVerifier {
         return parser.parseSignedClaims(token).getPayload();
     }
 
-    private static boolean hasAudience(Claims claims) {
-        return claims.getAudience() != null && !claims.getAudience().isEmpty();
-    }
-
-    /**
-     * @param legacyType the {@code typ} of this kind before item 26, null for a login
-     */
-    private static boolean isKind(Claims claims, String audience, String legacyType) {
-        if (hasAudience(claims)) {
-            return claims.getAudience().contains(audience) && ISSUER.equals(claims.getIssuer());
-        }
-        return Objects.equals(legacyType, claims.get(TYPE_CLAIM, String.class));
+    private static boolean isKind(Claims claims, String audience) {
+        return claims.getAudience() != null && claims.getAudience().contains(audience)
+                && ISSUER.equals(claims.getIssuer());
     }
 
     /**
@@ -66,11 +51,10 @@ public class JwtService implements ProfileTicketVerifier {
     public Optional<UserPrincipal> parseLoginToken(String token) {
         try {
             Claims claims = claims(token);
-            if (!isKind(claims, LOGIN_AUDIENCE, null)) {
+            if (!isKind(claims, LOGIN_AUDIENCE)) {
                 return Optional.empty();
             }
-            String userId = hasAudience(claims) ? claims.getSubject()
-                    : claims.getSubject() == null ? null : claims.get("userId", String.class);
+            String userId = claims.getSubject();
             if (userId == null) {
                 return Optional.empty();
             }
@@ -87,7 +71,7 @@ public class JwtService implements ProfileTicketVerifier {
     public boolean isServiceToken(String token) {
         try {
             Claims claims = claims(token);
-            return isKind(claims, SERVICE_AUDIENCE, SERVICE_TOKEN_TYPE) && claims.getExpiration() != null;
+            return isKind(claims, SERVICE_AUDIENCE) && claims.getExpiration() != null;
         } catch (Exception e) {
             return false;
         }
@@ -101,7 +85,7 @@ public class JwtService implements ProfileTicketVerifier {
     public Optional<ProfileTicket> verifyProfileTicket(String ticket) {
         try {
             Claims claims = claims(ticket);
-            if (!isKind(claims, PROFILE_TICKET_AUDIENCE, PROFILE_TICKET_TYPE)) {
+            if (!isKind(claims, PROFILE_TICKET_AUDIENCE)) {
                 return Optional.empty();
             }
             long userId = Long.parseLong(claims.get("profileUserId", String.class));

src/test/java/core_service/configuration/security/JwtServiceTest.java

@@ -5,6 +5,7 @@ import core_service.shared.security.Role;
 import core_service.shared.security.UserPrincipal;
 import core_service.shared.test.AbstractUnitTest;
 import core_service.shared.test.ContractTokens;
+import io.jsonwebtoken.JwtBuilder;
 import io.jsonwebtoken.Jwts;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -48,10 +49,10 @@ class JwtServiceTest extends AbstractUnitTest {
         jwtService = new JwtService(new JwksKeyLocator(jwtConfig, RestClient.create()));
     }
 
-    private String signToken(String userId, String email, Long steamId) {
-        var builder = Jwts.builder()
-                .subject(email)
-                .claim("userId", userId)
+    private String signToken(String userId, Long steamId) {
+        JwtBuilder builder = Jwts.builder()
+                .issuer(JwtService.ISSUER).audience().add(JwtService.LOGIN_AUDIENCE).and()
+                .subject(userId)
                 .issuedAt(new Date())
                 .expiration(new Date(System.currentTimeMillis() + 60_000));
         if (steamId != null) {
@@ -62,7 +63,7 @@ class JwtServiceTest extends AbstractUnitTest {
 
     @Test
     void parseLoginToken_ReturnsTheUser() {
-        String token = signToken("0815", "test@example.com", 123456789L);
+        String token = signToken("0815", 123456789L);
 
         assertEquals(Optional.of(new UserPrincipal("0815", 123456789L)),
                 jwtService.parseLoginToken(token));
@@ -77,8 +78,8 @@ class JwtServiceTest extends AbstractUnitTest {
     void parseLoginToken_RejectsAForeignKey() throws Exception {
         KeyPair other = KeyPairGenerator.getInstance("Ed25519").generateKeyPair();
         String token = Jwts.builder()
-                .subject("test@example.com")
-                .claim("userId", "0815")
+                .issuer(JwtService.ISSUER).audience().add(JwtService.LOGIN_AUDIENCE).and()
+                .subject("0815")
                 .expiration(new Date(System.currentTimeMillis() + 60_000))
                 .signWith(other.getPrivate(), Jwts.SIG.EdDSA)
                 .compact();
@@ -89,7 +90,7 @@ class JwtServiceTest extends AbstractUnitTest {
     @Test
     void parseLoginToken_RejectsATokenWithoutUserId() {
         String token = Jwts.builder()
-                .subject("test@example.com")
+                .issuer(JwtService.ISSUER).audience().add(JwtService.LOGIN_AUDIENCE).and()
                 .expiration(new Date(System.currentTimeMillis() + 60_000))
                 .signWith(signingKey, Jwts.SIG.EdDSA)
                 .compact();
@@ -97,10 +98,10 @@ class JwtServiceTest extends AbstractUnitTest {
         assertEquals(Optional.empty(), jwtService.parseLoginToken(token));
     }
 
-    private String signTicket(String type, String profileUserId, long expiresInMs) {
+    private String signTicket(String audience, String profileUserId, long expiresInMs) {
         return Jwts.builder()
+                .issuer(JwtService.ISSUER).audience().add(audience).and()
                 .subject("public-profile")
-                .claim("typ", type)
                 .claim("profileUserId", profileUserId)
                 .expiration(new Date(System.currentTimeMillis() + expiresInMs))
                 .signWith(signingKey, Jwts.SIG.EdDSA).compact();
@@ -109,12 +110,12 @@ class JwtServiceTest extends AbstractUnitTest {
     @Test
     void verifyProfileTicket_ReturnsTheUser() {
         assertEquals(Optional.of(new ProfileTicket(42L, Set.of(), null)),
-                jwtService.verifyProfileTicket(signTicket("public-profile", "42", 60_000)));
+                jwtService.verifyProfileTicket(signTicket(JwtService.PROFILE_TICKET_AUDIENCE, "42", 60_000)));
     }
 
     @Test
     void verifyProfileTicket_ReadsTheSharedStatGroups() {
-        String ticket = Jwts.builder().subject("public-profile").claim("typ", "public-profile").claim("profileUserId", "42")
+        String ticket = Jwts.builder().subject("public-profile").issuer(JwtService.ISSUER).audience().add(JwtService.PROFILE_TICKET_AUDIENCE).and().claim("profileUserId", "42")
                 .claim("profileStats", List.of("PILE", "TREND"))
                 .expiration(new Date(System.currentTimeMillis() + 60_000)).signWith(signingKey, Jwts.SIG.EdDSA).compact();
 
@@ -123,7 +124,7 @@ class JwtServiceTest extends AbstractUnitTest {
 
     @Test
     void verifyProfileTicket_ReadsTheShownSections() {
-        String ticket = Jwts.builder().subject("public-profile").claim("typ", "public-profile").claim("profileUserId", "42")
+        String ticket = Jwts.builder().subject("public-profile").issuer(JwtService.ISSUER).audience().add(JwtService.PROFILE_TICKET_AUDIENCE).and().claim("profileUserId", "42")
                 .claim("sections", List.of("PLAYING", "STATS"))
                 .expiration(new Date(System.currentTimeMillis() + 60_000)).signWith(signingKey, Jwts.SIG.EdDSA).compact();
 
@@ -132,25 +133,26 @@ class JwtServiceTest extends AbstractUnitTest {
 
     @Test
     void verifyProfileTicket_RejectsLoginTokensOtherTypesExpiredAndForeignTickets() throws Exception {
-        assertEquals(Optional.empty(), jwtService.verifyProfileTicket(signToken("42", "a@b.de", null)));
+        assertEquals(Optional.empty(), jwtService.verifyProfileTicket(signToken("42", null)));
         assertEquals(Optional.empty(), jwtService.verifyProfileTicket(signTicket("something-else", "42", 60_000)));
-        assertEquals(Optional.empty(), jwtService.verifyProfileTicket(signTicket("public-profile", "42", -1_000)));
-        assertEquals(Optional.empty(), jwtService.verifyProfileTicket(signTicket("public-profile", "not-a-number", 60_000)));
+        assertEquals(Optional.empty(), jwtService.verifyProfileTicket(signTicket(JwtService.PROFILE_TICKET_AUDIENCE, "42", -1_000)));
+        assertEquals(Optional.empty(), jwtService.verifyProfileTicket(signTicket(JwtService.PROFILE_TICKET_AUDIENCE, "not-a-number", 60_000)));
         assertEquals(Optional.empty(), jwtService.verifyProfileTicket("garbage"));
 
         PrivateKey otherKey = KeyPairGenerator.getInstance("Ed25519").generateKeyPair().getPrivate();
-        String foreign = Jwts.builder().claim("typ", "public-profile").claim("profileUserId", "42")
+        String foreign = Jwts.builder().issuer(JwtService.ISSUER).audience().add(JwtService.PROFILE_TICKET_AUDIENCE).and().claim("profileUserId", "42")
                 .expiration(new Date(System.currentTimeMillis() + 60_000)).signWith(otherKey, Jwts.SIG.EdDSA).compact();
         assertEquals(Optional.empty(), jwtService.verifyProfileTicket(foreign));
     }
 
     @Test
     void parseLoginToken_ProfileTicketIsNoLogin() {
-        assertEquals(Optional.empty(), jwtService.parseLoginToken(signTicket("public-profile", "42", 60_000)));
+        assertEquals(Optional.empty(), jwtService.parseLoginToken(signTicket(JwtService.PROFILE_TICKET_AUDIENCE, "42", 60_000)));
     }
 
-    private String signServiceToken(PrivateKey key, String type, Long expiresInMs) {
-        io.jsonwebtoken.JwtBuilder builder = Jwts.builder().subject("tbp-auth").claim("typ", type).issuedAt(new Date());
+    private String signServiceToken(PrivateKey key, Long expiresInMs) {
+        JwtBuilder builder = Jwts.builder().issuer(JwtService.ISSUER).audience().add(JwtService.SERVICE_AUDIENCE).and()
+                .subject("tbp-auth").issuedAt(new Date());
         if (expiresInMs != null) {
             builder.expiration(new Date(System.currentTimeMillis() + expiresInMs));
         }
@@ -159,30 +161,30 @@ class JwtServiceTest extends AbstractUnitTest {
 
     @Test
     void isServiceToken_AcceptsAServiceTokenOfAuth() {
-        assertTrue(jwtService.isServiceToken(signServiceToken(signingKey, "service", 60_000L)));
+        assertTrue(jwtService.isServiceToken(signServiceToken(signingKey, 60_000L)));
     }
 
     @Test
     void isServiceToken_RejectsEverythingElse() throws Exception {
         PrivateKey otherKey = KeyPairGenerator.getInstance("Ed25519").generateKeyPair().getPrivate();
 
-        assertFalse(jwtService.isServiceToken(signServiceToken(signingKey, "service", null)), "no exp");
-        assertFalse(jwtService.isServiceToken(signServiceToken(signingKey, "service", -1_000L)), "expired");
-        assertFalse(jwtService.isServiceToken(signServiceToken(otherKey, "service", 60_000L)), "foreign key");
-        assertFalse(jwtService.isServiceToken(signToken("42", "a@b.de", null)), "login token");
-        assertFalse(jwtService.isServiceToken(signTicket("public-profile", "42", 60_000)), "profile ticket");
+        assertFalse(jwtService.isServiceToken(signServiceToken(signingKey, null)), "no exp");
+        assertFalse(jwtService.isServiceToken(signServiceToken(signingKey, -1_000L)), "expired");
+        assertFalse(jwtService.isServiceToken(signServiceToken(otherKey, 60_000L)), "foreign key");
+        assertFalse(jwtService.isServiceToken(signToken("42", null)), "login token");
+        assertFalse(jwtService.isServiceToken(signTicket(JwtService.PROFILE_TICKET_AUDIENCE, "42", 60_000)), "profile ticket");
         assertFalse(jwtService.isServiceToken("garbage"));
     }
 
     @Test
     void parseLoginToken_ServiceTokenIsNoLogin() {
-        assertEquals(Optional.empty(), jwtService.parseLoginToken(signServiceToken(signingKey, "service", 60_000L)));
+        assertEquals(Optional.empty(), jwtService.parseLoginToken(signServiceToken(signingKey, 60_000L)));
     }
 
     private String signTokenWithRole(String role) {
         return Jwts.builder()
-                .subject("test@example.com")
-                .claim("userId", "0815")
+                .issuer(JwtService.ISSUER).audience().add(JwtService.LOGIN_AUDIENCE).and()
+                .subject("0815")
                 .claim("role", role)
                 .expiration(new Date(System.currentTimeMillis() + 60_000))
                 .signWith(signingKey, Jwts.SIG.EdDSA)
@@ -196,13 +198,14 @@ class JwtServiceTest extends AbstractUnitTest {
 
     @Test
     void parseLoginToken_WithoutOrWithAnUnknownRole_IsUser() {
-        assertEquals(Role.USER, jwtService.parseLoginToken(signToken("0815", "test@example.com", null)).orElseThrow().role());
+        assertEquals(Role.USER, jwtService.parseLoginToken(signToken("0815", null)).orElseThrow().role());
         assertEquals(Role.USER, jwtService.parseLoginToken(signTokenWithRole("GOD")).orElseThrow().role());
     }
 
     @Test
     void parseLoginToken_WithARoleThatIsNoString_IsStillALoginAsUser() {
-        String token = Jwts.builder().subject("test@example.com").claim("userId", "0815").claim("role", List.of("AUDITOR"))
+        String token = Jwts.builder().issuer(JwtService.ISSUER).audience().add(JwtService.LOGIN_AUDIENCE).and()
+                .subject("0815").claim("role", List.of("AUDITOR"))
                 .expiration(new Date(System.currentTimeMillis() + 60_000)).signWith(signingKey, Jwts.SIG.EdDSA).compact();
 
         assertEquals(Role.USER, jwtService.parseLoginToken(token).orElseThrow().role());
@@ -212,14 +215,6 @@ class JwtServiceTest extends AbstractUnitTest {
         return ContractTokens.sign(claims, signingKey, 60_000);
     }
 
-    // The shapes of item 29: without the claims an Auth from before item 26 needs.
-    private static Map<String, Object> without(Map<String, Object> claims, String... names) {
-        for (String name : names) {
-            claims.remove(name);
-        }
-        return claims;
-    }
-
     @Test
     void contractTokens_OfEveryKind_AreAccepted() {
         assertEquals("42", jwtService.parseLoginToken(sign(ContractTokens.claims(ContractTokens.LOGIN))).orElseThrow().id());
@@ -229,20 +224,20 @@ class JwtServiceTest extends AbstractUnitTest {
     }
 
     @Test
-    void legacyTokens_OfAnAuthFromBeforeItem26_AreAccepted() {
-        assertEquals("42", jwtService.parseLoginToken(sign(ContractTokens.legacyClaims(ContractTokens.LOGIN))).orElseThrow().id());
-        assertEquals(42L, jwtService.verifyProfileTicket(sign(ContractTokens.legacyClaims(ContractTokens.PUBLIC_PROFILE)))
-                .orElseThrow().userId());
-        assertTrue(jwtService.isServiceToken(sign(ContractTokens.legacyClaims(ContractTokens.SERVICE))));
+    void tokensWithoutAud_AreRefused() {
+        assertTrue(jwtService.parseLoginToken(sign(withoutAudience(ContractTokens.LOGIN))).isEmpty());
+        assertTrue(jwtService.verifyProfileTicket(sign(withoutAudience(ContractTokens.PUBLIC_PROFILE))).isEmpty());
+        assertFalse(jwtService.isServiceToken(sign(withoutAudience(ContractTokens.SERVICE))));
     }
 
-    @Test
-    void tokensWithoutTheOldClaims_AreAccepted() {
-        assertEquals("42", jwtService.parseLoginToken(sign(without(ContractTokens.claims(ContractTokens.LOGIN), "userId")))
-                .orElseThrow().id());
-        assertEquals(42L, jwtService.verifyProfileTicket(sign(without(ContractTokens.claims(ContractTokens.PUBLIC_PROFILE), "typ")))
-                .orElseThrow().userId());
-        assertTrue(jwtService.isServiceToken(sign(without(ContractTokens.claims(ContractTokens.SERVICE), "typ"))));
+    // The shape of an Auth from before item 26, with the old claims it wrote.
+    private static Map<String, Object> withoutAudience(String kind) {
+        Map<String, Object> claims = ContractTokens.claims(kind);
+        claims.remove("aud");
+        claims.remove("iss");
+        claims.put("userId", "42");
+        claims.put("typ", kind.equals(ContractTokens.SERVICE) ? "service" : "public-profile");
+        return claims;
     }
 
     @Test

src/test/java/core_service/modules/account/api/InternalAccountControllerTest.java

@@ -4,8 +4,6 @@ import core_service.shared.test.AbstractIntegrationTest;
 import core_service.shared.test.ContractTokens;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.params.ParameterizedTest;
-import org.junit.jupiter.params.provider.ValueSource;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc;
 import org.springframework.http.HttpMethod;
@@ -117,12 +115,10 @@ class InternalAccountControllerTest extends AbstractIntegrationTest {
         return request(HttpMethod.valueOf(call.path("method").asString()), call.path("path").asString());
     }
 
-    // The calls as Auth's CoreAccountService sends them, with the service tokens of an old and a new Auth.
-    @ParameterizedTest
-    @ValueSource(booleans = {false, true})
-    void contractCalls_AreAnswered(boolean legacyToken) throws Exception {
-        String token = legacyToken ? ContractTokens.sign(ContractTokens.legacyClaims(ContractTokens.SERVICE))
-                : ContractTokens.serviceToken();
+    // The calls as Auth's CoreAccountService sends them.
+    @Test
+    void contractCalls_AreAnswered() throws Exception {
+        String token = ContractTokens.serviceToken();
         // replaceDeleted with [] removes every tombstone, also those of other tests in this database.
         List<Long> tombstones = jdbc.queryForList("select user_id from deleted_user", Long.class);
         try {

src/test/java/core_service/shared/test/ContractTokens.java

@@ -46,14 +46,6 @@ public final class ContractTokens {
         return new LinkedHashMap<>(MAPPER.treeToValue(CONTRACT.path("tokens").required(kind), Map.class));
     }
 
-    /**
-     * The claims of an Auth from before item 26.
-     */
-    @SuppressWarnings("unchecked")
-    public static Map<String, Object> legacyClaims(String kind) {
-        return new LinkedHashMap<>(MAPPER.treeToValue(CONTRACT.path("legacyTokens").required(kind), Map.class));
-    }
-
     public static JsonNode internalCall(String name) {
         return CONTRACT.path("internal").required(name);
     }

src/test/resources/contract/auth-core-contract.json

@@ -1,11 +1,10 @@
 {
-  "_about": "The token and internal-call contract between Auth and Core. The same file is in both repos; ToBePlayed-Deployment/scripts/check-contract.sh compares them. iat and exp are left out. legacyTokens are the shapes of an Auth from before item 26: Core and Auth accept them until item 29.",
+  "_about": "The token and internal-call contract between Auth and Core. The same file is in both repos; ToBePlayed-Deployment/scripts/check-contract.sh compares them. iat and exp are left out.",
   "tokens": {
     "login": {
       "iss": "tbp-auth",
       "aud": ["tbp-login"],
       "sub": "42",
-      "userId": "42",
       "role": "USER",
       "steamId": 76561198000000001
     },
@@ -13,7 +12,6 @@
       "iss": "tbp-auth",
       "aud": ["tbp-public-profile"],
       "sub": "public-profile",
-      "typ": "public-profile",
       "profileUserId": "42",
       "profileStats": ["OVERVIEW"],
       "sections": ["SUMMARY"]
@@ -21,27 +19,7 @@
     "service": {
       "iss": "tbp-auth",
       "aud": ["tbp-internal"],
-      "sub": "tbp-auth",
-      "typ": "service"
-    }
-  },
-  "legacyTokens": {
-    "login": {
-      "sub": "user@example.com",
-      "userId": "42",
-      "role": "USER",
-      "steamId": 76561198000000001
-    },
-    "publicProfile": {
-      "sub": "public-profile",
-      "typ": "public-profile",
-      "profileUserId": "42",
-      "profileStats": ["OVERVIEW"],
-      "sections": ["SUMMARY"]
-    },
-    "service": {
-      "sub": "tbp-auth",
-      "typ": "service"
+      "sub": "tbp-auth"
     }
   },
   "internal": {