Ban Revoke structure

.gitignore

@@ -3,6 +3,9 @@
 # Generated by gradle
 *.iml
 
+# All log files e.g. by audit
+*.log
+
 # User-specific stuff:
 .idea/workspace.xml
 .idea/tasks.xml

.travis.yml

@@ -19,10 +19,10 @@ before_install:
 install:
   - git clone https://github.com/FAForever/faf-stack.git faf-stack
       && pushd faf-stack
-      && git checkout 78c887c
+    && git checkout a1ef34cb64343225d6793e7af1d8fa0c93a7b3e6
       && cp -r config.template config
+      && ./scripts/init-db.sh
       && popd
-  - docker-compose -f faf-stack/docker-compose.yml up -d faf-db
 
 script:
 - chmod +x gradlew && ./gradlew build -Pversion=${APP_VERSION} --info

gradle.properties

@@ -19,7 +19,7 @@ junitAddonsVersion=1.4
 zohhakVersion=1.1.1
 githubApiVersion=1.84
 jgitVersionn=4.5.0.201609210915-r
-fafCommonsVersion=8dd4a6c991fd607153d8f7cd4bf43180c1f8a912
+fafCommonsVersion=98d878df4c19853d93b2d58269ac83a5897e1145
 h2Version=1.4.193
 jacksonDatatypeJsr310Version=2.9.7
 mockitoVersion=2.19.0

src/inttest/java/com/faforever/api/data/BanInfoTest.java → src/inttest/java/com/faforever/api/data/BanTest.java

@@ -13,10 +13,13 @@
 import org.springframework.test.context.jdbc.Sql;
 import org.springframework.test.context.jdbc.Sql.ExecutionPhase;
 
+import java.time.OffsetDateTime;
+
 import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.core.Is.is;
 import static org.junit.Assert.assertThat;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.patch;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
@@ -27,11 +30,12 @@
 @Sql(executionPhase = ExecutionPhase.BEFORE_TEST_METHOD, scripts = "classpath:sql/prepGameData.sql")
 @Sql(executionPhase = ExecutionPhase.BEFORE_TEST_METHOD, scripts = "classpath:sql/prepModerationReportData.sql")
 @Sql(executionPhase = ExecutionPhase.BEFORE_TEST_METHOD, scripts = "classpath:sql/prepBanData.sql")
+@Sql(executionPhase = ExecutionPhase.AFTER_TEST_METHOD, scripts = "classpath:sql/cleanDefaultUser.sql")
 @Sql(executionPhase = ExecutionPhase.AFTER_TEST_METHOD, scripts = "classpath:sql/cleanBanData.sql")
 @Sql(executionPhase = ExecutionPhase.AFTER_TEST_METHOD, scripts = "classpath:sql/cleanModerationReportData.sql")
 @Sql(executionPhase = ExecutionPhase.AFTER_TEST_METHOD, scripts = "classpath:sql/cleanGameData.sql")
 @Sql(executionPhase = ExecutionPhase.AFTER_TEST_METHOD, scripts = "classpath:sql/cleanMapData.sql")
-public class BanInfoTest extends AbstractIntegrationTest {
+public class BanTest extends AbstractIntegrationTest {
   /*
   {
       "data": {
@@ -90,7 +94,7 @@ public void cannotCreateBanInfoAsUser() throws Exception {
   public void canReadBanInfoAsModerator() throws Exception {
     mockMvc.perform(get("/data/banInfo"))
       .andExpect(status().isOk())
-      .andExpect(jsonPath("$.data", hasSize(2)));
+      .andExpect(jsonPath("$.data", hasSize(3)));
   }
 
   @Test
@@ -116,7 +120,6 @@ public void canCreateBanInfoAsModerator() throws Exception {
   @Test
   @WithUserDetails(AUTH_MODERATOR)
   public void canCreateBanInfoBasedOnModerationReportAsModerator() throws Exception {
-
     final BanInfo banInfo = new BanInfo()
       .setLevel(BanLevel.CHAT)
       .setReason("Ban reason")
@@ -128,4 +131,34 @@ public void canCreateBanInfoBasedOnModerationReportAsModerator() throws Exceptio
       .andExpect(status().isCreated())
       .andExpect(jsonPath("$.data.relationships.moderationReport.data.id", is("1")));
   }
+
+  @Test
+  @WithUserDetails(AUTH_MODERATOR)
+  public void moderatorCanRevokeBan() throws Exception {
+    final BanInfo banInfo = new BanInfo();
+    banInfo.setId("3");
+    banInfo.setRevokeTime(OffsetDateTime.now());
+    banInfo.setRevokeReason("revoke reason");
+
+    mockMvc.perform(patch("/data/banInfo/3")
+      .header(HttpHeaders.CONTENT_TYPE, DataController.JSON_API_MEDIA_TYPE)
+      .content(createJsonApiContent(banInfo)))
+      .andExpect(status().isNoContent());
+
+    assertThat(playerRepository.getOne(5).isGlobalBanned(), is(false));
+  }
+
+  @Test
+  @WithUserDetails(AUTH_USER)
+  public void userCanNotRevokeBan() throws Exception {
+    final BanInfo banInfo = new BanInfo();
+    banInfo.setId("3");
+    banInfo.setRevokeTime(OffsetDateTime.now());
+    banInfo.setRevokeReason("new revoke reason");
+
+    mockMvc.perform(patch("/data/banInfo/3")
+      .header(HttpHeaders.CONTENT_TYPE, DataController.JSON_API_MEDIA_TYPE)
+      .content(createJsonApiContent(banInfo)))
+      .andExpect(status().isForbidden());
+  }
 }

src/inttest/resources/sql/cleanBanData.sql

@@ -1,2 +1 @@
-DELETE FROM ban_revoke;
 DELETE FROM ban;

src/inttest/resources/sql/cleanDefaultUser.sql

@@ -0,0 +1,45 @@
+DELETE FROM reported_user;
+DELETE FROM ban;
+DELETE FROM moderation_report;
+DELETE FROM teamkills;
+DELETE FROM unique_id_users;
+DELETE FROM uniqueid;
+DELETE FROM global_rating;
+DELETE FROM ladder1v1_rating;
+DELETE FROM uniqueid_exempt;
+DELETE FROM version_lobby;
+DELETE FROM friends_and_foes;
+DELETE FROM ladder_map;
+DELETE FROM tutorial;
+DELETE FROM map_version_review;
+DELETE FROM map_version_reviews_summary;
+DELETE FROM map_version;
+DELETE FROM `map`;
+DELETE FROM mod_version_review;
+DELETE FROM mod_version_reviews_summary;
+DELETE FROM mod_version;
+DELETE FROM `mod`;
+DELETE FROM mod_stats;
+DELETE FROM oauth_clients;
+DELETE FROM updates_faf;
+DELETE FROM updates_faf_files;
+DELETE FROM avatars;
+DELETE FROM avatars_list;
+DELETE FROM ban;
+DELETE FROM clan_membership;
+DELETE FROM clan;
+DELETE FROM game_player_stats;
+DELETE FROM game_review;
+DELETE FROM game_reviews_summary;
+DELETE FROM game_stats;
+DELETE FROM game_featuredMods;
+DELETE FROM ladder_division_score;
+DELETE FROM ladder_division;
+DELETE FROM lobby_admin;
+DELETE FROM name_history;
+DELETE FROM group_permission_assignment;
+DELETE FROM group_permission;
+DELETE FROM user_group_assignment;
+DELETE FROM user_group;
+DELETE FROM login;
+DELETE FROM email_domain_blacklist;

src/inttest/resources/sql/prepBanData.sql

@@ -1,13 +1,15 @@
-DELETE FROM ban_revoke;
 DELETE FROM ban;
 
 INSERT INTO login (id, login, email, password) VALUES
   (4, 'BANNED', 'banned@faforever.com', 'not relevant');
 
-INSERT INTO ban (id, player_id, author_id, reason, expires_at, level) VALUES
-  (1, 4, 1, 'Test permaban', DATE_ADD(NOW(), INTERVAL 1 DAY), 'GLOBAL'),
-  (2, 2, 1, 'To be revoked ban', DATE_ADD(NOW(), INTERVAL 1 DAY), 'GLOBAL');
-
-INSERT INTO ban_revoke (ban_id, reason, author_id) VALUES
-  (2, 'Test revoke', 1);
-
+INSERT INTO login (id, login, email, password) VALUES
+  (5, 'TO_BE_REVOKED', 'revoke@faforever.com', 'not-relevant');
+
+INSERT INTO ban (id, player_id, author_id, reason, expires_at, level) VALUE
+  (1, 4, 1, 'Test permaban', DATE_ADD(NOW(), INTERVAL 1 DAY), 'GLOBAL');
+INSERT INTO ban (id, player_id, author_id, reason, expires_at, level, revoke_time, revoke_author_id, revoke_reason)
+  VALUE
+  (2, 2, 1, 'To be revoked ban', DATE_ADD(NOW(), INTERVAL 1 DAY), 'GLOBAL', NOW(), 1, 'Test revoke');
+INSERT INTO ban (id, player_id, author_id, reason, expires_at, level) VALUE
+  (3, 5, 1, 'Ban to be revoked', DATE_ADD(NOW(), INTERVAL 30 DAY), 'GLOBAL');

src/inttest/resources/sql/prepDefaultUser.sql

@@ -1,4 +1,5 @@
 DELETE FROM reported_user;
+DELETE FROM ban;
 DELETE FROM moderation_report;
 DELETE FROM teamkills;
 DELETE FROM unique_id_users;
@@ -24,7 +25,6 @@ DELETE FROM updates_faf;
 DELETE FROM updates_faf_files;
 DELETE FROM avatars;
 DELETE FROM avatars_list;
-DELETE FROM ban_revoke;
 DELETE FROM ban;
 DELETE FROM clan_membership;
 DELETE FROM clan;

src/inttest/resources/sql/prepUserNoteData.sql

@@ -1,4 +1,3 @@
-DELETE FROM ban_revoke;
 DELETE FROM ban;
 
 INSERT INTO login (id, login, email, password) VALUES

src/main/java/com/faforever/api/data/domain/BanInfo.java

@@ -9,19 +9,18 @@
 import com.yahoo.elide.annotation.DeletePermission;
 import com.yahoo.elide.annotation.Include;
 import com.yahoo.elide.annotation.OnCreatePreSecurity;
+import com.yahoo.elide.annotation.OnUpdatePreSecurity;
 import com.yahoo.elide.annotation.ReadPermission;
 import com.yahoo.elide.annotation.UpdatePermission;
 import com.yahoo.elide.core.RequestScope;
 import lombok.Setter;
 
-import javax.persistence.CascadeType;
 import javax.persistence.Column;
 import javax.persistence.Entity;
 import javax.persistence.EnumType;
 import javax.persistence.Enumerated;
 import javax.persistence.JoinColumn;
 import javax.persistence.ManyToOne;
-import javax.persistence.OneToOne;
 import javax.persistence.Table;
 import javax.persistence.Transient;
 import javax.validation.constraints.NotNull;
@@ -44,8 +43,10 @@ public class BanInfo extends AbstractEntity {
   private String reason;
   private OffsetDateTime expiresAt;
   private BanLevel level;
-  private BanRevokeData banRevokeData;
   private ModerationReport moderationReport;
+  private String revokeReason;
+  private Player revokeAuthor;
+  private OffsetDateTime revokeTime;
 
   @ManyToOne
   @JoinColumn(name = "player_id")
@@ -55,6 +56,7 @@ public Player getPlayer() {
   }
 
   @ManyToOne
+  @UpdatePermission(expression = "Prefab.Role.None")
   @JoinColumn(name = "author_id")
   @NotNull
   public Player getAuthor() {
@@ -78,13 +80,6 @@ public BanLevel getLevel() {
     return level;
   }
 
-  // Cascading is needed for Create & Delete
-  @OneToOne(cascade = CascadeType.ALL, orphanRemoval = true)
-  @JoinColumn(name = "id")
-  public BanRevokeData getBanRevokeData() {
-    return banRevokeData;
-  }
-
   @ManyToOne
   @JoinColumn(name = "report_id")
   public ModerationReport getModerationReport() {
@@ -96,9 +91,26 @@ public BanDurationType getDuration() {
     return expiresAt == null ? BanDurationType.PERMANENT : BanDurationType.TEMPORARY;
   }
 
+  @Column(name = "revoke_reason")
+  public String getRevokeReason() {
+    return revokeReason;
+  }
+
+  @ManyToOne
+  @UpdatePermission(expression = "Prefab.Role.None")
+  @JoinColumn(name = "revoke_author_id")
+  public Player getRevokeAuthor() {
+    return revokeAuthor;
+  }
+
+  @Column(name = "revoke_time")
+  public OffsetDateTime getRevokeTime() {
+    return revokeTime;
+  }
+
   @Transient
   public BanStatus getBanStatus() {
-    if (banRevokeData != null) {
+    if (revokeTime != null && revokeTime.isBefore(OffsetDateTime.now())) {
       return BanStatus.DISABLED;
     }
     if (getDuration() == BanDurationType.PERMANENT) {
@@ -119,4 +131,24 @@ public void assignReporter(RequestScope scope) {
       this.author = author;
     }
   }
+
+  @OnUpdatePreSecurity("revokeTime")
+  public void revokeTimeUpdated(RequestScope scope) {
+    assignRevokeAuthor(scope);
+  }
+
+  @OnUpdatePreSecurity("revokeReason")
+  public void revokeReasonUpdated(RequestScope scope) {
+    assignRevokeAuthor(scope);
+  }
+
+  private void assignRevokeAuthor(RequestScope scope) {
+    final Object caller = scope.getUser().getOpaqueUser();
+    if (caller instanceof FafUserDetails) {
+      final FafUserDetails fafUser = (FafUserDetails) caller;
+      final Player author = new Player();
+      author.setId(fafUser.getId());
+      this.revokeAuthor = author;
+    }
+  }
 }

src/main/java/com/faforever/api/leaderboard/GlobalLeaderboardRepository.java

@@ -20,15 +20,13 @@ public interface GlobalLeaderboardRepository extends Repository<GlobalLeaderboar
     "  WHERE is_active = 1" +
     "   AND login.id NOT IN (" +
     "     SELECT player_id FROM ban" +
-    "     LEFT JOIN ban_revoke on ban.id = ban_revoke.ban_id" +
-    "     WHERE (expires_at is null or expires_at > NOW()) AND ban_revoke.ban_id IS NULL" +
+    "     WHERE (expires_at is null or expires_at > NOW()) AND (revoke_time IS NULL OR revoke_time > NOW())" +
     "  ) " +
     "  ORDER BY round(mean - 3 * deviation) DESC LIMIT ?#{#pageable.offset},?#{#pageable.pageSize}",
     countQuery = "SELECT count(*) FROM ladder1v1_rating WHERE is_active = 1 AND ladder1v1_rating.numGames > 0" +
       "   AND id NOT IN (" +
       "     SELECT player_id FROM ban" +
-      "     LEFT JOIN ban_revoke on ban.id = ban_revoke.ban_id" +
-      "     WHERE (expires_at is null or expires_at > NOW()) AND ban_revoke.ban_id IS NULL" +
+      "     WHERE (expires_at is null or expires_at > NOW()) AND (revoke_time IS NULL OR revoke_time > NOW())" +
       "  ) -- Dummy placeholder for pageable, prevents 'Unknown parameter position': ?,?,?", nativeQuery = true)
   Page<GlobalLeaderboardEntry> getLeaderboardByPage(Pageable pageable);
 
@@ -44,8 +42,7 @@ public interface GlobalLeaderboardRepository extends Repository<GlobalLeaderboar
     "WHERE is_active = 1\n" +
     "   AND login.id NOT IN (" +
     "     SELECT player_id FROM ban" +
-    "     LEFT JOIN ban_revoke on ban.id = ban_revoke.ban_id" +
-    "     WHERE (expires_at is null or expires_at <= NOW()) AND ban_revoke.ban_id IS NULL" +
+    "     WHERE (expires_at is null or expires_at <= NOW()) AND (revoke_time IS NULL OR revoke_time > NOW())" +
     "  ) " +
     "ORDER BY round(mean - 3 * deviation) DESC) as leaderboard WHERE id = :playerId", nativeQuery = true)
   GlobalLeaderboardEntry findByPlayerId(@Param("playerId") int playerId);