Allow joining public clans w/o invite token

src/inttest/java/com/faforever/api/clan/ClanControllerTest.java

@@ -16,10 +16,10 @@
 
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.core.IsNull.nullValue;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;

src/main/java/com/faforever/api/clan/ClanService.java

@@ -10,15 +10,14 @@
 import com.faforever.api.error.ApiException;
 import com.faforever.api.error.Error;
 import com.faforever.api.error.ErrorCode;
-import com.faforever.api.error.ProgrammingError;
-import com.faforever.api.player.PlayerRepository;
 import com.faforever.api.player.PlayerService;
 import com.faforever.api.security.JwtService;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import lombok.RequiredArgsConstructor;
 import lombok.SneakyThrows;
-import org.springframework.security.core.Authentication;
 import org.springframework.stereotype.Service;
+import org.springframework.transaction.annotation.Transactional;
+import org.springframework.util.Assert;
 
 import java.time.Instant;
 import java.time.temporal.ChronoUnit;
@@ -30,90 +29,103 @@
 public class ClanService {
 
   private final ClanRepository clanRepository;
-  private final PlayerRepository playerRepository;
   private final FafApiProperties fafApiProperties;
   private final JwtService jwtService;
   private final ObjectMapper objectMapper;
   private final PlayerService playerService;
   private final ClanMembershipRepository clanMembershipRepository;
 
-  @SneakyThrows
-  Clan create(String name, String tag, String description, Player creator) {
-    if (creator.getClanMembership() != null) {
-      throw new ApiException(new Error(ErrorCode.CLAN_CREATE_FOUNDER_IS_IN_A_CLAN));
-    }
-    if (clanRepository.findOneByName(name).isPresent()) {
-      throw new ApiException(new Error(ErrorCode.CLAN_NAME_EXISTS, name));
+  @Transactional
+  public void preCreate(Clan clan) {
+    Assert.isNull(clan.getId(), "Clan payload with id can not be used for creation.");
+
+    Player player = playerService.getCurrentPlayer();
+
+    if (player.getClanMembership() != null) {
+      throw ApiException.of(ErrorCode.CLAN_CREATE_FOUNDER_IS_IN_A_CLAN);
     }
-    if (clanRepository.findOneByTag(tag).isPresent()) {
-      throw new ApiException(new Error(ErrorCode.CLAN_TAG_EXISTS, tag));
+
+    if (!player.equals(clan.getFounder())) {
+      throw ApiException.of(ErrorCode.CLAN_INVALID_FOUNDER);
     }
 
+    clanRepository.findOneByName(clan.getName()).ifPresent(c -> {
+      throw ApiException.of(ErrorCode.CLAN_NAME_EXISTS, clan.getName());
+    });
+
+    clanRepository.findOneByTag(clan.getTag()).ifPresent(c -> {
+      throw ApiException.of(ErrorCode.CLAN_TAG_EXISTS, clan.getTag());
+    });
+
+    clan.setLeader(player);
+    final ClanMembership clanMembership = new ClanMembership();
+    clanMembership.setClan(clan);
+    clanMembership.setPlayer(player);
+    clan.setMemberships(Set.of(clanMembership));
+  }
+
+  @SneakyThrows
+  @Transactional
+  @Deprecated
+  // use POST via Elide instead
+  public Clan create(String name, String tag, String description) {
     Clan clan = new Clan();
     clan.setName(name);
     clan.setTag(tag);
     clan.setDescription(description);
     clan.setRequiresInvitation(true);
+    Player currentPlayer = playerService.getCurrentPlayer();
+    clan.setFounder(currentPlayer);
+    clan.setLeader(currentPlayer);
 
-    clan.setFounder(creator);
-    clan.setLeader(creator);
-
-    ClanMembership membership = new ClanMembership();
-    membership.setClan(clan);
-    membership.setPlayer(creator);
-
-    clan.setMemberships(Set.of(membership));
-
-    // clan membership is saved over cascading, otherwise validation will fail
+    // validation is done at preCreate() called by ClanListener
     clanRepository.save(clan);
     return clan;
   }
 
   @SneakyThrows
-  String generatePlayerInvitationToken(Player requester, int newMemberId, int clanId) {
+  @Transactional
+  public String generatePlayerInvitationToken(int newMemberId, int clanId) {
+    Player requester = playerService.getCurrentPlayer();
+
     Clan clan = clanRepository.findById(clanId)
       .orElseThrow(() -> new ApiException(new Error(ErrorCode.CLAN_NOT_EXISTS, clanId)));
 
     if (!requester.getId().equals(clan.getLeader().getId())) {
-      throw new ApiException(new Error(ErrorCode.CLAN_NOT_LEADER, clanId));
+      throw ApiException.of(ErrorCode.CLAN_NOT_LEADER, clanId);
     }
 
-    Player newMember = playerRepository.findById(newMemberId)
-      .orElseThrow(() -> new ApiException(new Error(ErrorCode.CLAN_GENERATE_LINK_PLAYER_NOT_FOUND, newMemberId)));
+    Player newMember = playerService.getById(newMemberId);
 
     long expire = Instant.now()
       .plus(fafApiProperties.getClan().getInviteLinkExpireDurationMinutes(), ChronoUnit.MINUTES)
       .toEpochMilli();
 
-    InvitationResult result = new InvitationResult(expire,
-      ClanResult.of(clan),
-      PlayerResult.of(newMember));
+    InvitationResult result = new InvitationResult(expire, ClanResult.of(clan), PlayerResult.of(newMember));
     return jwtService.sign(result);
   }
 
   @SneakyThrows
-  void acceptPlayerInvitationToken(String stringToken, Authentication authentication) {
+  void acceptPlayerInvitationToken(String stringToken) {
     String decodedToken = jwtService.decodeAndVerify(stringToken);
     InvitationResult invitation = objectMapper.readValue(decodedToken, InvitationResult.class);
 
     if (invitation.isExpired()) {
-      throw new ApiException(new Error(ErrorCode.CLAN_ACCEPT_TOKEN_EXPIRE));
+      throw ApiException.of(ErrorCode.CLAN_ACCEPT_TOKEN_EXPIRE);
     }
 
     final Integer clanId = invitation.clan().id();
-    Player player = playerService.getPlayer(authentication);
+    Player player = playerService.getCurrentPlayer();
     Clan clan = clanRepository.findById(clanId)
       .orElseThrow(() -> new ApiException(new Error(ErrorCode.CLAN_NOT_EXISTS, clanId)));
 
-    Player newMember = playerRepository.findById(invitation.newMember().id())
-      .orElseThrow(() -> new ProgrammingError("ClanMember does not exist: " + invitation.newMember().id()));
-
+    Player newMember = playerService.getById(invitation.newMember().id());
 
     if (!Objects.equals(player.getId(), newMember.getId())) {
-      throw new ApiException(new Error(ErrorCode.CLAN_ACCEPT_WRONG_PLAYER));
+      throw ApiException.of(ErrorCode.CLAN_ACCEPT_WRONG_PLAYER);
     }
     if (newMember.getClan() != null) {
-      throw new ApiException(new Error(ErrorCode.CLAN_ACCEPT_PLAYER_IN_A_CLAN));
+      throw ApiException.of(ErrorCode.CLAN_ACCEPT_PLAYER_IN_A_CLAN);
     }
 
     ClanMembership membership = new ClanMembership();

src/main/java/com/faforever/api/clan/ClansController.java

@@ -11,15 +11,12 @@
 import io.swagger.annotations.ApiResponses;
 import lombok.RequiredArgsConstructor;
 import org.springframework.security.access.prepost.PreAuthorize;
-import org.springframework.security.core.Authentication;
-import org.springframework.transaction.annotation.Transactional;
 import org.springframework.web.bind.annotation.GetMapping;
 import org.springframework.web.bind.annotation.PostMapping;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestParam;
 import org.springframework.web.bind.annotation.RestController;
 
-import java.io.IOException;
 import java.io.Serializable;
 import java.util.Map;
 
@@ -40,8 +37,9 @@ public class ClansController {
     @ApiResponse(code = 200, message = "Success with JSON { player: {id: ?, login: ?}, clan: { id: ?, name: ?, tag: ?}}"),
     @ApiResponse(code = 400, message = "Bad Request")})
   @GetMapping(path = "/me", produces = APPLICATION_JSON_VALUE)
-  public MeResult me(Authentication authentication) {
-    Player player = playerService.getPlayer(authentication);
+  @Deprecated // use regular /me route instead
+  public MeResult me() {
+    Player player = playerService.getCurrentPlayer();
 
     Clan clan = player.getClan();
     ClanResult clanResult = null;
@@ -59,13 +57,11 @@ public MeResult me(Authentication authentication) {
     @ApiResponse(code = 400, message = "Bad Request")})
   @PostMapping(path = "/create", produces = APPLICATION_JSON_VALUE)
   @PreAuthorize("hasRole('ROLE_USER')")
-  @Transactional
+  @Deprecated // use POST /data/clans instead (with a founder in relationships)
   public Map<String, Serializable> createClan(@RequestParam(value = "name") String name,
                                               @RequestParam(value = "tag") String tag,
-                                              @RequestParam(value = "description", required = false) String description,
-                                              Authentication authentication) throws IOException {
-    Player player = playerService.getPlayer(authentication);
-    Clan clan = clanService.create(name, tag, description, player);
+                                              @RequestParam(value = "description", required = false) String description) {
+    Clan clan = clanService.create(name, tag, description);
     return Map.of("id", clan.getId(), "type", "clan");
   }
 
@@ -76,19 +72,14 @@ public Map<String, Serializable> createClan(@RequestParam(value = "name") String
   @GetMapping(path = "/generateInvitationLink", produces = APPLICATION_JSON_VALUE)
   public Map<String, Serializable> generateInvitationLink(
     @RequestParam(value = "clanId") int clanId,
-    @RequestParam(value = "playerId") int newMemberId,
-    Authentication authentication) throws IOException {
-    Player player = playerService.getPlayer(authentication);
-    String jwtToken = clanService.generatePlayerInvitationToken(player, newMemberId, clanId);
+    @RequestParam(value = "playerId") int newMemberId) {
+    String jwtToken = clanService.generatePlayerInvitationToken(newMemberId, clanId);
     return Map.of("jwtToken", jwtToken);
   }
 
-  @ApiOperation("Check invitation link and add Member to Clan")
+  @ApiOperation("Check invitation link and add member to Clan")
   @PostMapping(path = "/joinClan", produces = APPLICATION_JSON_VALUE)
-  @Transactional
-  public void joinClan(
-    @RequestParam(value = "token") String stringToken,
-    Authentication authentication) throws IOException {
-    clanService.acceptPlayerInvitationToken(stringToken, authentication);
+  public void joinClan(@RequestParam(value = "token") String stringToken) {
+    clanService.acceptPlayerInvitationToken(stringToken);
   }
 }

src/main/java/com/faforever/api/config/IgnoreOctetStreamToObjectHttpMessageConverter.java

@@ -6,7 +6,6 @@
 import org.springframework.http.MediaType;
 import org.springframework.http.converter.AbstractHttpMessageConverter;
 import org.springframework.lang.Nullable;
-import org.springframework.security.core.Authentication;
 import org.springframework.util.StreamUtils;
 import org.springframework.web.multipart.MultipartFile;
 
@@ -16,7 +15,7 @@
  * This class is used to support having both @RequestParam and @RequestPart with same multipart name in one request handler.
  * When multipart request contains simple request param octet-stream, this class is used to ignore parsing
  * of byte stream to {@link MapUploadMetadata}.
- * See {@link com.faforever.api.map.MapsController#uploadMap(MultipartFile, String, MapUploadMetadata, Authentication)}
+ * See {@link com.faforever.api.map.MapsController#uploadMap(MultipartFile, String, MapUploadMetadata)}
  */
 public class IgnoreOctetStreamToObjectHttpMessageConverter extends AbstractHttpMessageConverter<byte[]> {
 

src/main/java/com/faforever/api/config/NoopMultipartFileToStringConverter.java

@@ -2,14 +2,13 @@
 
 import com.faforever.api.map.MapUploadMetadata;
 import org.springframework.core.convert.converter.Converter;
-import org.springframework.security.core.Authentication;
 import org.springframework.web.multipart.MultipartFile;
 
 /**
  * This class is used to support having both @RequestParam and @RequestPart with same multipart name in one request handler.
  * When multipart request contains json multipart, this class is used to ignore conversion
  * of MultipartFile to String.
- * See {@link com.faforever.api.map.MapsController#uploadMap(MultipartFile, String, MapUploadMetadata, Authentication)}
+ * See {@link com.faforever.api.map.MapsController#uploadMap(MultipartFile, String, MapUploadMetadata)}
  */
 public class NoopMultipartFileToStringConverter implements Converter<MultipartFile, String> {
 

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

@@ -46,8 +46,8 @@ public class Clan extends AbstractEntity<Clan> implements OwnableEntity {
   private String description;
   private String tagColor;
   private String websiteUrl;
+  private Boolean requiresInvitation = Boolean.TRUE;
   private Set<ClanMembership> memberships;
-  private Boolean requiresInvitation;
 
   @Column(name = "name")
   @NotNull

src/main/java/com/faforever/api/data/listeners/ClanEnricherListener.java

@@ -1,20 +1,31 @@
 package com.faforever.api.data.listeners;
 
+import com.faforever.api.clan.ClanService;
 import com.faforever.api.config.FafApiProperties;
 import com.faforever.api.data.domain.Clan;
 import org.springframework.stereotype.Component;
 
 import jakarta.inject.Inject;
 import jakarta.persistence.PostLoad;
+import jakarta.persistence.PrePersist;
 
 @Component
 public class ClanEnricherListener {
 
   private static FafApiProperties fafApiProperties;
+  private static ClanService clanService;
 
   @Inject
-  public void init(FafApiProperties fafApiProperties) {
+  public void init(FafApiProperties fafApiProperties, ClanService clanService) {
     ClanEnricherListener.fafApiProperties = fafApiProperties;
+    ClanEnricherListener.clanService = clanService;
+  }
+
+  @PrePersist
+  public void prePersist(Clan clan) {
+    if (clan.getId() == null) {
+      clanService.preCreate(clan);
+    }
   }
 
   @PostLoad

src/main/java/com/faforever/api/error/ErrorCode.java

@@ -61,7 +61,7 @@ public enum ErrorCode {
   CLAN_ACCEPT_PLAYER_IN_A_CLAN(152, "Player is in a clan", "You are already in a clan"),
   CLAN_NOT_LEADER(153, "You Permission", "You are not the leader of the clan"),
   CLAN_NOT_EXISTS(154, "Cannot find Clan", "Clan with id {0, number} is not available"),
-  CLAN_GENERATE_LINK_PLAYER_NOT_FOUND(155, "Player not found", "Cannot find player with id {0, number} who should be invited to the clan"),
+  PLAYER_NOT_FOUND(155, "Player not found", "Cannot find player with id {0, number}."),
   CLAN_NAME_EXISTS(156, "Clan Name already in use", "The clan name ''{0}'' is already in use. Please choose a different clan name."),
   CLAN_TAG_EXISTS(157, "Clan Tag already in use", "The clan tag ''{0}'' is already in use. Please choose a different clan tag."),
   VALIDATION_FAILED(158, "Validation failed", "{0}"),
@@ -117,6 +117,7 @@ public enum ErrorCode {
   LESS_PERMISSIVE_LICENSE(207, "Less permissive license", "New license is less permissive than current license."),
   MALFORMED_URL(208, "Malformed URL", "Provided url ''{0}'' is malformed."),
   NOT_ALLOWED_URL_HOST(209, "URL host not allowed", "Provided URL's host is not allowed. URL: ''{0}'', allowed hosts: ''{1}''."),
+  CLAN_INVALID_FOUNDER(210, "Invalid clan founder", "If you create a clan you must be the founder of it."),
   ;