Fixes #252

src/inttest/java/com/faforever/api/user/UsersControllerTest.java

@@ -34,6 +34,7 @@
 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.redirectedUrl;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.redirectedUrlPattern;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 
 public class UsersControllerTest extends AbstractIntegrationTest {
@@ -319,13 +320,13 @@ public void buildSteamLinkUrl() throws Exception {
   public void linkToSteam() throws Exception {
     assertThat(userRepository.getOne(1).getSteamId(), nullValue());
 
-    String steamId = "1234";
-    String callbackUrl = "callbackUrl";
+    String steamId = "12345";
+    String callbackUrl = "http://faforever.com";
     String token = fafTokenService.createToken(
       FafTokenType.LINK_TO_STEAM,
       Duration.ofSeconds(100),
       ImmutableMap.of(
-        UserService.KEY_USER_ID, "2",
+        UserService.KEY_USER_ID, "1",
         UserService.KEY_STEAM_LINK_CALLBACK_URL, callbackUrl
       ));
 
@@ -337,7 +338,36 @@ public void linkToSteam() throws Exception {
       .andExpect(status().isFound())
       .andExpect(redirectedUrl(callbackUrl));
 
-    assertThat(userRepository.getOne(2).getSteamId(), is(steamId));
+    assertThat(userRepository.getOne(1).getSteamId(), is(steamId));
+  }
+
+  @Test
+  @WithAnonymousUser
+  public void linkToSteamAlreadyLinkedAccount() throws Exception {
+    String steamId = "1234";
+    assertThat(userRepository.getOne(1).getSteamId(), nullValue());
+    User userThatOwnsSteamId = userRepository.getOne(2);
+    assertThat(userThatOwnsSteamId.getSteamId(), is(steamId));
+
+    String callbackUrl = "http://faforever.com";
+    String token = fafTokenService.createToken(
+      FafTokenType.LINK_TO_STEAM,
+      Duration.ofSeconds(100),
+      ImmutableMap.of(
+        UserService.KEY_USER_ID, "1",
+        UserService.KEY_STEAM_LINK_CALLBACK_URL, callbackUrl
+      ));
+
+    when(steamService.parseSteamIdFromLoginRedirect(any())).thenReturn(steamId);
+    when(steamService.ownsForgedAlliance(anyString())).thenReturn(true);
+
+    mockMvc.perform(
+      get(String.format("/users/linkToSteam?callbackUrl=%s&token=%s&openid.identity=http://steamcommunity.com/openid/id/%s", callbackUrl, token, steamId)))
+      .andExpect(status().isFound())
+      .andExpect(redirectedUrlPattern(callbackUrl+"?errors=*"+ErrorCode.STEAM_ID_ALREADY_LINKED.getCode()+"*"+userThatOwnsSteamId.getLogin()+"*"));
+    //We expect and error with code STEAM_ID_ALREADY_LINKED and that the error message contains the user that this steam account was linked to already which is MODERATOR with id 2
+
+    assertThat(userRepository.getOne(1).getSteamId(), nullValue());
   }
 
   @Test

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

@@ -88,7 +88,8 @@ public enum ErrorCode {
   VOTED_TWICE_ON_ONE_OPTION(178, "Selected one option twice", "You can not vote twice for one option. Voting Choice with id ''{0}'' was selected twice."),
   CAN_NOT_REVEAL_RESULTS_WHEN_VOTING_IS_NOT_FINISHED(179, "Vote still ongoing", "You can reveal results when voting is ongoing. Please set reveal results only after voting finished."),
   VOTING_SUBJECT_DOES_NOT_EXIST(180, "Voting subject does not exist", "There is no voting subject with the ID ''{0}''."),
-  VOTING_CHOICE_DOES_NOT_EXIST(181, "Invalid choice", "There is no voting choice with the ID ''{0}''.");
+  VOTING_CHOICE_DOES_NOT_EXIST(181, "Invalid choice", "There is no voting choice with the ID ''{0}''."),
+  STEAM_ID_ALREADY_LINKED(182, " Steam account already linked to a FAF account", "You linked this account already to user with name ''{0}''.");
 
   private final int code;
   private final String title;

src/main/java/com/faforever/api/user/UserRepository.java

@@ -12,6 +12,8 @@ public interface UserRepository extends JpaRepository<User, Integer> {
 
   Optional<User> findOneByEmailIgnoreCase(String email);
 
+  Optional<User> findOneBySteamIdIgnoreCase(String steamId);
+
   boolean existsByEmailIgnoreCase(String email);
 
   boolean existsByLoginIgnoreCase(String login);

src/main/java/com/faforever/api/user/UserService.java

@@ -29,7 +29,7 @@
 import java.nio.charset.StandardCharsets;
 import java.time.Duration;
 import java.time.OffsetDateTime;
-import java.util.Collections;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -302,20 +302,28 @@ public String buildSteamLinkUrl(User user, String callbackUrl) {
   @SneakyThrows
   public SteamLinkResult linkToSteam(String token, String steamId) {
     log.debug("linkToSteam requested for steamId '{}' with token: {}", steamId, token);
+    List<Error> errors = new ArrayList<>();
     Map<String, String> attributes = fafTokenService.resolveToken(FafTokenType.LINK_TO_STEAM, token);
 
     User user = userRepository.findById(Integer.parseInt(attributes.get(KEY_USER_ID)))
       .orElseThrow(() -> new ApiException(new Error(TOKEN_INVALID)));
 
-    String callbackUrl = attributes.get(KEY_STEAM_LINK_CALLBACK_URL);
     if (!steamService.ownsForgedAlliance(steamId)) {
-      return new SteamLinkResult(callbackUrl, Collections.singletonList(new Error(ErrorCode.STEAM_LINK_NO_FA_GAME)));
+      errors.add(new Error(ErrorCode.STEAM_LINK_NO_FA_GAME));
     }
 
-    user.setSteamId(steamId);
-    userRepository.save(user);
+    Optional<User> userWithSameSteamIdOptional = userRepository.findOneBySteamIdIgnoreCase(steamId);
+    userWithSameSteamIdOptional.ifPresent(userWithSameId ->
+      errors.add(new Error(ErrorCode.STEAM_ID_ALREADY_LINKED, userWithSameId.getLogin()))
+    );
 
-    return new SteamLinkResult(callbackUrl, Collections.emptyList());
+    if (errors.isEmpty()) {
+      user.setSteamId(steamId);
+      userRepository.save(user);
+    }
+
+    String callbackUrl = attributes.get(KEY_STEAM_LINK_CALLBACK_URL);
+    return new SteamLinkResult(callbackUrl, errors);
   }
 
   @Value

src/test/java/com/faforever/api/user/UserServiceTest.java

@@ -41,6 +41,7 @@
 import static com.faforever.api.user.UserService.KEY_USER_ID;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.Matchers.empty;
+import static org.hamcrest.Matchers.hasItemInArray;
 import static org.hamcrest.Matchers.hasSize;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertThat;
@@ -102,7 +103,7 @@ private static User createUser(int id, String name, String password, String emai
   }
 
   @Before
-  public void setUp() throws Exception {
+  public void setUp() {
     properties = new FafApiProperties();
     properties.getJwt().setSecret(TEST_SECRET);
     properties.getLinkToSteam().setSteamRedirectUrlFormat("%s");
@@ -117,7 +118,7 @@ public void setUp() throws Exception {
 
   @Test
   @SuppressWarnings("unchecked")
-  public void register() throws Exception {
+  public void register() {
     properties.getRegistration().setActivationUrlFormat(ACTIVATION_URL_FORMAT);
 
     instance.register(TEST_USERNAME, TEST_CURRENT_EMAIL, TEST_CURRENT_PASSWORD);
@@ -133,47 +134,47 @@ public void register() throws Exception {
   }
 
   @Test
-  public void registerEmailAlreadyRegistered() throws Exception {
+  public void registerEmailAlreadyRegistered() {
     when(userRepository.existsByEmailIgnoreCase(TEST_CURRENT_EMAIL)).thenReturn(true);
     expectedException.expect(ApiExceptionWithCode.apiExceptionWithCode(ErrorCode.EMAIL_REGISTERED));
 
     instance.register("junit", TEST_CURRENT_EMAIL, TEST_CURRENT_PASSWORD);
   }
 
   @Test
-  public void registerInvalidUsernameWithComma() throws Exception {
+  public void registerInvalidUsernameWithComma() {
     expectedException.expect(ApiExceptionWithCode.apiExceptionWithCode(ErrorCode.USERNAME_INVALID));
     instance.register("junit,", TEST_CURRENT_EMAIL, TEST_CURRENT_PASSWORD);
   }
 
   @Test
-  public void registerInvalidUsernameStartsUnderscore() throws Exception {
+  public void registerInvalidUsernameStartsUnderscore() {
     expectedException.expect(ApiExceptionWithCode.apiExceptionWithCode(ErrorCode.USERNAME_INVALID));
     instance.register("_junit", TEST_CURRENT_EMAIL, TEST_CURRENT_PASSWORD);
   }
 
   @Test
-  public void registerInvalidUsernameTooShort() throws Exception {
+  public void registerInvalidUsernameTooShort() {
     expectedException.expect(ApiExceptionWithCode.apiExceptionWithCode(ErrorCode.USERNAME_INVALID));
     instance.register("ju", TEST_CURRENT_EMAIL, TEST_CURRENT_PASSWORD);
   }
 
   @Test
-  public void registerUsernameTaken() throws Exception {
+  public void registerUsernameTaken() {
     when(userRepository.existsByLoginIgnoreCase("junit")).thenReturn(true);
     expectedException.expect(ApiExceptionWithCode.apiExceptionWithCode(ErrorCode.USERNAME_TAKEN));
     instance.register("junit", TEST_CURRENT_EMAIL, TEST_CURRENT_PASSWORD);
   }
 
   @Test
-  public void registerUsernameReserved() throws Exception {
+  public void registerUsernameReserved() {
     when(nameRecordRepository.getLastUsernameOwnerWithinMonths(any(), anyInt())).thenReturn(Optional.of(1));
     expectedException.expect(ApiExceptionWithCode.apiExceptionWithCode(ErrorCode.USERNAME_RESERVED));
     instance.register("junit", TEST_CURRENT_EMAIL, TEST_CURRENT_PASSWORD);
   }
 
   @Test
-  public void activate() throws Exception {
+  public void activate() {
     when(fafTokenService.resolveToken(FafTokenType.REGISTRATION, TOKEN_VALUE)).thenReturn(ImmutableMap.of(
       UserService.KEY_USERNAME, TEST_USERNAME,
       UserService.KEY_EMAIL, TEST_CURRENT_EMAIL,
@@ -300,7 +301,7 @@ public void changeLoginUsernameReservedBySelf() {
 
   @Test
   @SuppressWarnings("unchecked")
-  public void resetPasswordByLogin() throws Exception {
+  public void resetPasswordByLogin() {
     properties.getPasswordReset().setPasswordResetUrlFormat(PASSWORD_RESET_URL_FORMAT);
 
     User user = createUser(TEST_USERID, TEST_USERNAME, TEST_CURRENT_PASSWORD, TEST_CURRENT_EMAIL);
@@ -324,7 +325,7 @@ public void resetPasswordByLogin() throws Exception {
 
   @Test
   @SuppressWarnings("unchecked")
-  public void resetPasswordByEmail() throws Exception {
+  public void resetPasswordByEmail() {
     properties.getPasswordReset().setPasswordResetUrlFormat(PASSWORD_RESET_URL_FORMAT);
 
     User user = createUser(TEST_USERID, TEST_USERNAME, TEST_CURRENT_PASSWORD, TEST_CURRENT_EMAIL);
@@ -347,7 +348,7 @@ public void resetPasswordByEmail() throws Exception {
   }
 
   @Test
-  public void resetPasswordUnknownUsernameAndEmail() throws Exception {
+  public void resetPasswordUnknownUsernameAndEmail() {
     expectedException.expect(ApiExceptionWithCode.apiExceptionWithCode(ErrorCode.UNKNOWN_IDENTIFIER));
 
     when(userRepository.findOneByEmailIgnoreCase(TEST_CURRENT_EMAIL)).thenReturn(Optional.empty());
@@ -356,7 +357,7 @@ public void resetPasswordUnknownUsernameAndEmail() throws Exception {
   }
 
   @Test
-  public void claimPasswordResetToken() throws Exception {
+  public void claimPasswordResetToken() {
     when(fafTokenService.resolveToken(FafTokenType.PASSWORD_RESET, TOKEN_VALUE))
       .thenReturn(ImmutableMap.of(KEY_USER_ID, "5",
         KEY_PASSWORD, TEST_NEW_PASSWORD));
@@ -375,7 +376,7 @@ public void claimPasswordResetToken() throws Exception {
 
   @Test
   @SuppressWarnings("unchecked")
-  public void buildSteamLinkUrl() throws Exception {
+  public void buildSteamLinkUrl() {
     when(steamService.buildLoginUrl(any())).thenReturn("steamLoginUrl");
 
     User user = createUser(TEST_USERID, TEST_USERNAME, TEST_CURRENT_PASSWORD, TEST_CURRENT_EMAIL);
@@ -401,7 +402,7 @@ public void buildSteamLinkUrl() throws Exception {
   }
 
   @Test
-  public void buildSteamLinkUrlAlreadLinked() throws Exception {
+  public void buildSteamLinkUrlAlreadLinked() {
     expectedException.expect(ApiExceptionWithCode.apiExceptionWithCode(ErrorCode.STEAM_ID_UNCHANGEABLE));
 
     User user = createUser(TEST_USERID, TEST_USERNAME, TEST_CURRENT_PASSWORD, TEST_CURRENT_EMAIL);
@@ -411,7 +412,7 @@ public void buildSteamLinkUrlAlreadLinked() throws Exception {
   }
 
   @Test
-  public void linkToSteam() throws Exception {
+  public void linkToSteam() {
     when(fafTokenService.resolveToken(FafTokenType.LINK_TO_STEAM, TOKEN_VALUE))
       .thenReturn(ImmutableMap.of(
         KEY_USER_ID, "5",
@@ -421,6 +422,7 @@ public void linkToSteam() throws Exception {
 
     User user = createUser(TEST_USERID, TEST_USERNAME, TEST_CURRENT_PASSWORD, TEST_CURRENT_EMAIL);
     when(userRepository.findById(5)).thenReturn(Optional.of(user));
+    when(userRepository.findOneBySteamIdIgnoreCase(STEAM_ID)).thenReturn(Optional.empty());
 
     SteamLinkResult result = instance.linkToSteam(TOKEN_VALUE, STEAM_ID);
 
@@ -431,7 +433,7 @@ public void linkToSteam() throws Exception {
   }
 
   @Test
-  public void linkToSteamUnknownUser() throws Exception {
+  public void linkToSteamUnknownUser() {
     expectedException.expect(ApiExceptionWithCode.apiExceptionWithCode(ErrorCode.TOKEN_INVALID));
 
     when(fafTokenService.resolveToken(FafTokenType.LINK_TO_STEAM, TOKEN_VALUE)).thenReturn(ImmutableMap.of(KEY_USER_ID, "5"));
@@ -442,13 +444,14 @@ public void linkToSteamUnknownUser() throws Exception {
   }
 
   @Test
-  public void linkToSteamNoGame() throws Exception {
+  public void linkToSteamNoGame() {
     when(fafTokenService.resolveToken(FafTokenType.LINK_TO_STEAM, TOKEN_VALUE))
       .thenReturn(ImmutableMap.of(
         KEY_USER_ID, "5",
         KEY_STEAM_LINK_CALLBACK_URL, "callbackUrl"
       ));
     when(steamService.ownsForgedAlliance(any())).thenReturn(false);
+    when(userRepository.findOneBySteamIdIgnoreCase(STEAM_ID)).thenReturn(Optional.empty());
 
     User user = createUser(TEST_USERID, TEST_USERNAME, TEST_CURRENT_PASSWORD, TEST_CURRENT_EMAIL);
     when(userRepository.findById(5)).thenReturn(Optional.of(user));
@@ -463,7 +466,31 @@ public void linkToSteamNoGame() throws Exception {
   }
 
   @Test
-  public void mauticUpdateFailureDoesNotThrowException() throws Exception {
+  public void linkToSteamAlreadyLinked() {
+    when(fafTokenService.resolveToken(FafTokenType.LINK_TO_STEAM, TOKEN_VALUE))
+      .thenReturn(ImmutableMap.of(
+        KEY_USER_ID, "6",
+        KEY_STEAM_LINK_CALLBACK_URL, "callbackUrl"
+      ));
+    when(steamService.ownsForgedAlliance(any())).thenReturn(true);
+    User otherUser = new User();
+    otherUser.setLogin("axel12");
+    when(userRepository.findOneBySteamIdIgnoreCase(STEAM_ID)).thenReturn(Optional.of(otherUser));
+
+    User user = createUser(TEST_USERID, TEST_USERNAME, TEST_CURRENT_PASSWORD, TEST_CURRENT_EMAIL);
+    when(userRepository.findById(6)).thenReturn(Optional.of(user));
+
+    SteamLinkResult result = instance.linkToSteam(TOKEN_VALUE, STEAM_ID);
+
+    assertThat(result.getCallbackUrl(), is("callbackUrl"));
+    assertThat(result.getErrors(), hasSize(1));
+    assertThat(result.getErrors().get(0).getErrorCode(), is(ErrorCode.STEAM_ID_ALREADY_LINKED));
+    assertThat(result.getErrors().get(0).getArgs(), hasItemInArray(otherUser.getLogin()));
+    verifyZeroInteractions(mauticService);
+  }
+
+  @Test
+  public void mauticUpdateFailureDoesNotThrowException() {
     when(mauticService.createOrUpdateContact(any(), any(), any(), any(), any()))
       .thenAnswer(invocation -> {
         CompletableFuture<Object> future = new CompletableFuture<>();