Public Access
chore(security)!: drop the userId and typ claims and tokens without aud
This commit is contained in:
1 parent
9f56742ff6
commit
fd79c63441
5 files changed
+19
-68
No files matched your search
@@ -23,7 +23,6 @@ import java.util.Base64;
|
||||
import java.util.Date;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Objects;
|
||||
import java.util.Optional;
|
||||
|
||||
/**
|
||||
@@ -38,11 +37,8 @@ public class JwtService implements TokenGenerator, ProfileTicketIssuer, ServiceT
|
||||
static final String PROFILE_TICKET_AUDIENCE = "tbp-public-profile";
|
||||
static final String SERVICE_AUDIENCE = "tbp-internal";
|
||||
private static final long PROFILE_TICKET_EXPIRATION_MS = 10 * 60 * 1000;
|
||||
// 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 PROFILE_TICKET_SUBJECT = "public-profile";
|
||||
private static final long SERVICE_TOKEN_EXPIRATION_MS = 60 * 1000;
|
||||
private static final String SERVICE_TOKEN_TYPE = "service";
|
||||
private static final String ED25519 = "Ed25519";
|
||||
private static final String ROLE_CLAIM = "role";
|
||||
|
||||
@@ -75,8 +71,6 @@ public class JwtService implements TokenGenerator, ProfileTicketIssuer, ServiceT
|
||||
.issuer(ISSUER)
|
||||
.audience().add(LOGIN_AUDIENCE).and()
|
||||
.subject(userId)
|
||||
// item 29 removes: a Core from before item 26 reads the user id only from this claim.
|
||||
.claim("userId", userId)
|
||||
.claim(ROLE_CLAIM, role.name())
|
||||
.issuedAt(new Date(System.currentTimeMillis()))
|
||||
.expiration(new Date(System.currentTimeMillis() + sessionProperties.getAccessTokenTtl().toMillis()));
|
||||
@@ -96,8 +90,7 @@ public class JwtService implements TokenGenerator, ProfileTicketIssuer, ServiceT
|
||||
.header().keyId(jwtConfig.getKid()).and()
|
||||
.issuer(ISSUER)
|
||||
.audience().add(PROFILE_TICKET_AUDIENCE).and()
|
||||
.subject(PROFILE_TICKET_TYPE)
|
||||
.claim(TYPE_CLAIM, PROFILE_TICKET_TYPE)
|
||||
.subject(PROFILE_TICKET_SUBJECT)
|
||||
.claim("profileUserId", String.valueOf(userId))
|
||||
.claim("profileStats", profileStats)
|
||||
.claim("sections", sections)
|
||||
@@ -114,7 +107,6 @@ public class JwtService implements TokenGenerator, ProfileTicketIssuer, ServiceT
|
||||
.issuer(ISSUER)
|
||||
.audience().add(SERVICE_AUDIENCE).and()
|
||||
.subject("tbp-auth")
|
||||
.claim(TYPE_CLAIM, SERVICE_TOKEN_TYPE)
|
||||
.issuedAt(new Date(System.currentTimeMillis()))
|
||||
.expiration(new Date(System.currentTimeMillis() + SERVICE_TOKEN_EXPIRATION_MS))
|
||||
.signWith(privateKey, Jwts.SIG.EdDSA)
|
||||
@@ -138,12 +130,10 @@ public class JwtService implements TokenGenerator, ProfileTicketIssuer, ServiceT
|
||||
public Optional<UserPrincipal> parseLoginToken(String token) {
|
||||
try {
|
||||
Claims claims = parser.parseSignedClaims(token).getPayload();
|
||||
if (!isKind(claims, LOGIN_AUDIENCE, null)) {
|
||||
if (!isKind(claims, LOGIN_AUDIENCE)) {
|
||||
return Optional.empty();
|
||||
}
|
||||
// item 29 removes: a token without aud comes from before item 26, with the user id in "userId".
|
||||
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();
|
||||
}
|
||||
@@ -154,18 +144,9 @@ public class JwtService implements TokenGenerator, ProfileTicketIssuer, ServiceT
|
||||
}
|
||||
}
|
||||
|
||||
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());
|
||||
}
|
||||
|
||||
private static PrivateKey decodePrivateKey(String base64Pkcs8Der) {
|
||||
|
||||
@@ -116,7 +116,7 @@ class JwtServiceTest extends AbstractUnitTest {
|
||||
var ticket = jwtService.generateProfileTicket(42L, List.of(), List.of());
|
||||
|
||||
var claims = Jwts.parser().verifyWith(publicKey).build().parseSignedClaims(ticket).getPayload();
|
||||
assertEquals("public-profile", claims.get("typ", String.class));
|
||||
assertEquals(List.of("tbp-public-profile"), List.copyOf(claims.getAudience()));
|
||||
assertEquals("42", claims.get("profileUserId", String.class));
|
||||
assertNull(claims.get("userId"));
|
||||
long secondsLeft = (claims.getExpiration().getTime() - System.currentTimeMillis()) / 1000;
|
||||
@@ -166,7 +166,7 @@ class JwtServiceTest extends AbstractUnitTest {
|
||||
System.arraycopy(raw, 0, der, prefix.length, raw.length);
|
||||
PublicKey fromJwk = KeyFactory.getInstance("Ed25519").generatePublic(new X509EncodedKeySpec(der));
|
||||
String token = jwtService.generateToken("7", null, Role.USER);
|
||||
assertEquals("7", Jwts.parser().verifyWith(fromJwk).build().parseSignedClaims(token).getPayload().get("userId"));
|
||||
assertEquals("7", Jwts.parser().verifyWith(fromJwk).build().parseSignedClaims(token).getPayload().getSubject());
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -175,7 +175,7 @@ class JwtServiceTest extends AbstractUnitTest {
|
||||
|
||||
var parsed = Jwts.parser().verifyWith(publicKey).build().parseSignedClaims(token);
|
||||
assertEquals(KID, parsed.getHeader().getKeyId());
|
||||
assertEquals("service", parsed.getPayload().get("typ", String.class));
|
||||
assertEquals(List.of("tbp-internal"), List.copyOf(parsed.getPayload().getAudience()));
|
||||
assertNull(parsed.getPayload().get("userId"));
|
||||
long secondsLeft = (parsed.getPayload().getExpiration().getTime() - System.currentTimeMillis()) / 1000;
|
||||
assertTrue(secondsLeft > 0 && secondsLeft <= 60, "expires in " + secondsLeft + "s");
|
||||
@@ -191,7 +191,7 @@ class JwtServiceTest extends AbstractUnitTest {
|
||||
|
||||
@Test
|
||||
void parseLoginToken_ATokenFromBeforeRolesIsAUser() {
|
||||
String old = Jwts.builder().header().keyId(KID).and().subject("a@b.de").claim("userId", "7")
|
||||
String old = Jwts.builder().header().keyId(KID).and().issuer("tbp-auth").audience().add("tbp-login").and().subject("7")
|
||||
.expiration(new Date(System.currentTimeMillis() + 60_000))
|
||||
.signWith(privateKey, Jwts.SIG.EdDSA).compact();
|
||||
|
||||
@@ -223,16 +223,13 @@ class JwtServiceTest extends AbstractUnitTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
void parseLoginToken_ALoginOfAnAuthFromBeforeItem26_IsAccepted() {
|
||||
assertEquals("42", jwtService.parseLoginToken(sign(ContractFixture.legacyToken("login"))).orElseThrow().id());
|
||||
}
|
||||
|
||||
@Test
|
||||
void parseLoginToken_ALoginWithoutTheOldUserIdClaim_IsAccepted() {
|
||||
void parseLoginToken_ALoginWithoutAud_IsRefused() {
|
||||
ObjectNode login = (ObjectNode) ContractFixture.token("login").deepCopy();
|
||||
login.remove("userId");
|
||||
login.remove("aud");
|
||||
login.remove("iss");
|
||||
login.put("userId", "42");
|
||||
|
||||
assertEquals("42", jwtService.parseLoginToken(sign(login)).orElseThrow().id());
|
||||
assertTrue(jwtService.parseLoginToken(sign(login)).isEmpty());
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
@@ -31,10 +31,6 @@ public final class ContractFixture {
|
||||
return CONTRACT.path("tokens").required(kind);
|
||||
}
|
||||
|
||||
public static JsonNode legacyToken(String kind) {
|
||||
return CONTRACT.path("legacyTokens").required(kind);
|
||||
}
|
||||
|
||||
public static JsonNode internalCall(String name) {
|
||||
return CONTRACT.path("internal").required(name);
|
||||
}
|
||||
|
||||
@@ -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": {
|
||||
|
||||
Reference in new issue
Block a user