From 02b52db240f095301da97e447604095f8a8f819e Mon Sep 17 00:00:00 2001 From: Dragonfire Date: Wed, 3 May 2017 09:15:02 +0200 Subject: [PATCH] Fix #66 Don't manually read JSON values, deserialize into a Java object --- .../com/faforever/api/clan/ClanService.java | 28 ++++++++----------- .../faforever/api/clan/ClansController.java | 4 +-- .../faforever/api/clan/result/ClanResult.java | 5 ++++ .../api/clan/result/InvitationResult.java | 14 ++++++++++ .../api/clan/result/PlayerResult.java | 5 ++++ .../faforever/api/security/JwtService.java | 4 +-- .../faforever/api/clan/ClanServiceTest.java | 23 +++++++-------- 7 files changed, 51 insertions(+), 32 deletions(-) create mode 100644 src/main/java/com/faforever/api/clan/result/InvitationResult.java diff --git a/src/main/java/com/faforever/api/clan/ClanService.java b/src/main/java/com/faforever/api/clan/ClanService.java index 5b88ee351..ae9d64f19 100644 --- a/src/main/java/com/faforever/api/clan/ClanService.java +++ b/src/main/java/com/faforever/api/clan/ClanService.java @@ -1,5 +1,8 @@ package com.faforever.api.clan; +import com.faforever.api.clan.result.ClanResult; +import com.faforever.api.clan.result.InvitationResult; +import com.faforever.api.clan.result.PlayerResult; import com.faforever.api.config.FafApiProperties; import com.faforever.api.data.domain.Clan; import com.faforever.api.data.domain.ClanMembership; @@ -11,9 +14,7 @@ import com.faforever.api.player.PlayerRepository; import com.faforever.api.player.PlayerService; import com.faforever.api.security.JwtService; -import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; -import com.google.common.collect.ImmutableMap; import lombok.SneakyThrows; import org.springframework.security.core.Authentication; import org.springframework.security.jwt.Jwt; @@ -103,36 +104,31 @@ public String generatePlayerInvitationToken(Player requester, int newMemberId, i .plus(fafApiProperties.getClan().getInviteLinkExpireDurationMinutes(), ChronoUnit.MINUTES) .toEpochMilli(); - return jwtService.sign( - ImmutableMap.of(JwtKeys.NEW_MEMBER_ID, newMemberId, - JwtKeys.EXPIRE_IN, expire, - JwtKeys.CLAN, ImmutableMap.of( - JwtKeys.CLAN_ID, clan.getId(), - JwtKeys.CLAN_TAG, clan.getTag(), - JwtKeys.CLAN_NAME, clan.getName()) - )); + InvitationResult result = new InvitationResult(expire, + ClanResult.of(clan), + PlayerResult.of(newMember)); + return jwtService.sign(result); } @SneakyThrows - // TODO @dragonfire don't manually read JSON values, deserialize into a Java object? public void acceptPlayerInvitationToken(String stringToken, Authentication authentication) { Jwt token = jwtService.decodeAndVerify(stringToken); - JsonNode data = objectMapper.readTree(token.getClaims()); + InvitationResult invitation = objectMapper.readValue(token.getClaims(), InvitationResult.class); - if (data.get(JwtKeys.EXPIRE_IN).asLong() < System.currentTimeMillis()) { + if (invitation.isExpired()) { throw new ApiException(new Error(ErrorCode.CLAN_ACCEPT_TOKEN_EXPIRE)); } Player player = playerService.getPlayer(authentication); - Clan clan = clanRepository.findOne(data.get(JwtKeys.CLAN).get(JwtKeys.CLAN_ID).asInt()); + Clan clan = clanRepository.findOne(invitation.getClan().getId()); if (clan == null) { throw new ApiException(new Error(ErrorCode.CLAN_NOT_EXISTS)); } - Player newMember = playerRepository.findOne(data.get(JwtKeys.NEW_MEMBER_ID).asInt()); + Player newMember = playerRepository.findOne(invitation.getNewMember().getId()); if (newMember == null) { - throw new ProgrammingError("ClanMember does not exist: " + data.get(JwtKeys.NEW_MEMBER_ID).asInt()); + throw new ProgrammingError("ClanMember does not exist: " + invitation.getNewMember().getId()); } if (player.getId() != newMember.getId()) { diff --git a/src/main/java/com/faforever/api/clan/ClansController.java b/src/main/java/com/faforever/api/clan/ClansController.java index eb4f71ed9..b3f066e43 100644 --- a/src/main/java/com/faforever/api/clan/ClansController.java +++ b/src/main/java/com/faforever/api/clan/ClansController.java @@ -51,9 +51,9 @@ public MeResult me(Authentication authentication) { : null; ClanResult clanResult = null; if (clan != null) { - clanResult = new ClanResult(clan.getId(), clan.getTag(), clan.getName()); + clanResult = ClanResult.of(clan); } - return new MeResult(new PlayerResult(player.getId(), player.getLogin()), clanResult); + return new MeResult(PlayerResult.of(player), clanResult); } // This request cannot be handled by JSON API because we must simultaneously create two resources (a,b) diff --git a/src/main/java/com/faforever/api/clan/result/ClanResult.java b/src/main/java/com/faforever/api/clan/result/ClanResult.java index d4a0df0d4..693faa8d8 100644 --- a/src/main/java/com/faforever/api/clan/result/ClanResult.java +++ b/src/main/java/com/faforever/api/clan/result/ClanResult.java @@ -1,5 +1,6 @@ package com.faforever.api.clan.result; +import com.faforever.api.data.domain.Clan; import lombok.Data; @Data @@ -7,4 +8,8 @@ public class ClanResult { private final int id; private final String tag; private final String name; + + public static ClanResult of(Clan clan) { + return new ClanResult(clan.getId(), clan.getTag(), clan.getName()); + } } diff --git a/src/main/java/com/faforever/api/clan/result/InvitationResult.java b/src/main/java/com/faforever/api/clan/result/InvitationResult.java new file mode 100644 index 000000000..dcfa4a9fe --- /dev/null +++ b/src/main/java/com/faforever/api/clan/result/InvitationResult.java @@ -0,0 +1,14 @@ +package com.faforever.api.clan.result; + +import lombok.Data; + +@Data +public class InvitationResult { + private final long expire; + private final ClanResult clan; + private final PlayerResult newMember; + + public boolean isExpired() { + return expire < System.currentTimeMillis(); + } +} diff --git a/src/main/java/com/faforever/api/clan/result/PlayerResult.java b/src/main/java/com/faforever/api/clan/result/PlayerResult.java index 4362c610e..fa4d4cfb5 100644 --- a/src/main/java/com/faforever/api/clan/result/PlayerResult.java +++ b/src/main/java/com/faforever/api/clan/result/PlayerResult.java @@ -1,9 +1,14 @@ package com.faforever.api.clan.result; +import com.faforever.api.data.domain.Player; import lombok.Data; @Data public class PlayerResult { private final int id; private final String login; + + public static PlayerResult of(Player newMember) { + return new PlayerResult(newMember.getId(), newMember.getLogin()); + } } diff --git a/src/main/java/com/faforever/api/security/JwtService.java b/src/main/java/com/faforever/api/security/JwtService.java index e2bca9e2d..04025e80f 100644 --- a/src/main/java/com/faforever/api/security/JwtService.java +++ b/src/main/java/com/faforever/api/security/JwtService.java @@ -9,8 +9,6 @@ import javax.inject.Inject; import java.io.IOException; -import java.io.Serializable; -import java.util.Map; @Service public class JwtService { @@ -23,7 +21,7 @@ public JwtService(FafApiProperties fafApiProperties, ObjectMapper objectMapper) this.objectMapper = objectMapper; } - public String sign(Map data) throws IOException { + public String sign(Object data) throws IOException { return JwtHelper.encode(objectMapper.writeValueAsString(data), this.macSigner).getEncoded(); } diff --git a/src/test/java/com/faforever/api/clan/ClanServiceTest.java b/src/test/java/com/faforever/api/clan/ClanServiceTest.java index 04feb2895..df0ba4d8f 100644 --- a/src/test/java/com/faforever/api/clan/ClanServiceTest.java +++ b/src/test/java/com/faforever/api/clan/ClanServiceTest.java @@ -1,6 +1,7 @@ package com.faforever.api.clan; +import com.faforever.api.clan.result.InvitationResult; import com.faforever.api.config.FafApiProperties; import com.faforever.api.data.domain.Clan; import com.faforever.api.data.domain.ClanMembership; @@ -26,7 +27,6 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Collections; -import java.util.Map; import java.util.Optional; import static com.faforever.api.error.ApiExceptionWithCode.apiExceptionWithCode; @@ -236,15 +236,16 @@ public void generatePlayerInvitationToken() throws IOException { when(fafApiProperties.getClan()).thenReturn(props.getClan()); instance.generatePlayerInvitationToken(requester, newMember.getId(), clan.getId()); - ArgumentCaptor captor = ArgumentCaptor.forClass(Map.class); + ArgumentCaptor captor = ArgumentCaptor.forClass(InvitationResult.class); verify(jwtService, Mockito.times(1)).sign(captor.capture()); assertThat("expire", - Long.parseLong(captor.getValue().get("expire").toString()), + captor.getValue().getExpire(), greaterThan(System.currentTimeMillis())); - assertEquals(newMember.getId(), captor.getValue().get("newMemberId")); - assertEquals(clan.getId(), ((Map) captor.getValue().get("clan")).get("id")); - assertEquals(clan.getTag(), ((Map) captor.getValue().get("clan")).get("tag")); - assertEquals(clan.getName(), ((Map) captor.getValue().get("clan")).get("name")); + assertEquals(newMember.getId(), captor.getValue().getNewMember().getId()); + assertEquals(newMember.getLogin(), captor.getValue().getNewMember().getLogin()); + assertEquals(clan.getId(), captor.getValue().getClan().getId()); + assertEquals(clan.getTag(), captor.getValue().getClan().getTag()); + assertEquals(clan.getName(), captor.getValue().getClan().getName()); } @Test @@ -296,7 +297,7 @@ public void acceptPlayerInvitationTokenInvalidPlayer() throws IOException { Jwt jwtToken = Mockito.mock(Jwt.class); when(jwtToken.getClaims()).thenReturn( - String.format("{\"expire\":%s,\"newMemberId\":2,\"clan\":{\"id\":%s}}", + String.format("{\"expire\":%s,\"newMember\":{\"id\":2},\"clan\":{\"id\":%s}}", expire, clan.getId())); when(jwtService.decodeAndVerify(any())).thenReturn(jwtToken); when(clanRepository.findOne(clan.getId())).thenReturn(clan); @@ -326,7 +327,7 @@ public void acceptPlayerInvitationTokenWrongPlayer() throws IOException { Jwt jwtToken = Mockito.mock(Jwt.class); when(jwtToken.getClaims()).thenReturn( - String.format("{\"expire\":%s,\"newMemberId\":%s,\"clan\":{\"id\":%s}}", + String.format("{\"expire\":%s,\"newMember\":{\"id\":%s},\"clan\":{\"id\":%s}}", expire, newMember.getId(), clan.getId())); when(jwtService.decodeAndVerify(any())).thenReturn(jwtToken); when(clanRepository.findOne(clan.getId())).thenReturn(clan); @@ -357,7 +358,7 @@ public void acceptPlayerInvitationTokenPlayerIAlreadyInAClan() throws IOExceptio Jwt jwtToken = Mockito.mock(Jwt.class); when(jwtToken.getClaims()).thenReturn( - String.format("{\"expire\":%s,\"newMemberId\":%s,\"clan\":{\"id\":%s}}", + String.format("{\"expire\":%s,\"newMember\":{\"id\":%s},\"clan\":{\"id\":%s}}", expire, newMember.getId(), clan.getId())); when(jwtService.decodeAndVerify(any())).thenReturn(jwtToken); when(clanRepository.findOne(clan.getId())).thenReturn(clan); @@ -383,7 +384,7 @@ public void acceptPlayerInvitationToken() throws IOException { Jwt jwtToken = Mockito.mock(Jwt.class); when(jwtToken.getClaims()).thenReturn( - String.format("{\"expire\":%s,\"newMemberId\":%s,\"clan\":{\"id\":%s}}", + String.format("{\"expire\":%s,\"newMember\":{\"id\":%s},\"clan\":{\"id\":%s}}", expire, newMember.getId(), clan.getId())); when(jwtService.decodeAndVerify(any())).thenReturn(jwtToken); when(clanRepository.findOne(clan.getId())).thenReturn(clan);