Fix parseSteamIdFromLoginRedirect method implementation

FAForever · Jan 15, 2024 · 6d53333 · 6d53333
1 parent bca6325
commit 6d53333
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 32 deletions.
diff --git a/src/main/java/com/faforever/api/user/SteamService.java b/src/main/java/com/faforever/api/user/SteamService.java
@@ -22,6 +22,7 @@
 import java.net.http.HttpRequest;
 import java.net.http.HttpResponse.BodyHandlers;
 import java.util.Map;
+import java.util.Optional;
 
 @Service
 @Slf4j
@@ -43,11 +44,13 @@ String buildLoginUrl(String redirectUrl) {
       .toUriString();
   }
 
+  @SneakyThrows
   String parseSteamIdFromLoginRedirect(HttpServletRequest request) {
     log.trace("Parsing steam id from request: {}", request);
-
-    String identityUrl = request.getParameter("openid.identity");
-    return identityUrl.substring(identityUrl.lastIndexOf("/") + 1);
+    return Optional.ofNullable(request.getParameter("openid.identity"))
+        .map(identityUrl -> identityUrl.substring(identityUrl.lastIndexOf("/") + 1))
+        .orElseThrow(() -> {log.warn("Steam redirect could not be validated! The request does not contain 'openid.identity' parameter. Original OpenID response:\n {}", request);
+          return ApiException.of(ErrorCode.STEAM_LOGIN_VALIDATION_FAILED);});
   }
 
   @SneakyThrows
@@ -93,29 +96,16 @@ void validateSteamRedirect(HttpServletRequest request) {
   }
 
   void handleInvalidOpenIdRedirect(final HttpServletRequest request, final String openIdResponseBody) {
-    boolean containsIdentityParam = request.getParameterMap().containsKey("openid.identity");
-    final String steamId;
-
-    if (containsIdentityParam)
-    {
-       steamId = parseSteamIdFromLoginRedirect(request);
-    } else {
-      log.warn("Steam redirect could not be validated! The request does not contain 'openid.identity' parameter. Original OpenID response:\n {}", openIdResponseBody);
-      throw ApiException.of(ErrorCode.STEAM_LOGIN_VALIDATION_FAILED);
-    }
+    final String steamId = parseSteamIdFromLoginRedirect(request);
 
     if (StringUtils.isNotBlank(steamId)) {
-      accountLinkRepository.findOneByServiceIdAndServiceType(steamId,
-        LinkedServiceType.STEAM).map(AccountLink::getUser).ifPresentOrElse(u ->
-          log.warn(
-              "Steam redirect could not be validated for user with id: ''{}'' and login: ''{}''. Original OpenID response:\n {}",
+      accountLinkRepository.findOneByServiceIdAndServiceType(steamId, LinkedServiceType.STEAM)
+          .map(AccountLink::getUser)
+          .ifPresentOrElse(u -> log.warn("Steam redirect could not be validated for user with id: ''{}'' and login: ''{}''. Original OpenID response:\n {}",
               u.getId(), u.getLogin(), openIdResponseBody),
-        () ->
-          log.warn(
-            "Steam redirect could not be validated! The steam id ''{}'' does not match any account. Original OpenID response:\n {}",
+              () -> log.warn("Steam redirect could not be validated! The steam id ''{}'' does not match any account. Original OpenID response:\n {}",
             StringUtils.deleteWhitespace(steamId).replace("'", ""), // prevent potential log poisoning attack
-            openIdResponseBody)
-      );
+            openIdResponseBody));
     } else {
       log.warn("Steam redirect could not be validated! The steamId from the OpenId redirect is blank. Original OpenID response:\n {}", openIdResponseBody);
     }

diff --git a/src/test/java/com/faforever/api/user/SteamServiceTest.java b/src/test/java/com/faforever/api/user/SteamServiceTest.java
@@ -11,8 +11,6 @@
 import com.faforever.api.error.ApiException;
 import com.faforever.api.error.ErrorCode;
 import jakarta.servlet.http.HttpServletRequest;
-import java.util.Collections;
-import java.util.Map;
 import java.util.Optional;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -36,8 +34,6 @@ class SteamServiceTest {
 
   @Test
   void testHandleInvalidOpenIdRedirect() {
-    when(requestMock.getParameterMap())
-        .thenReturn(Map.of(IDENTITY_NAME_PARAM, new String[]{DUMMY_URL}));
     when(requestMock.getParameter(IDENTITY_NAME_PARAM)).thenReturn(DUMMY_URL);
 
     ApiException thrownException = assertThrows(ApiException.class,
@@ -49,8 +45,6 @@ void testHandleInvalidOpenIdRedirect() {
   @Test
   void testHandleInvalidOpenIdRedirectBlankIdentityParam() {
     final String blankDummyUrl = "";
-    when(requestMock.getParameterMap())
-        .thenReturn(Map.of(IDENTITY_NAME_PARAM, new String[]{blankDummyUrl}));
     when(requestMock.getParameter(IDENTITY_NAME_PARAM)).thenReturn(blankDummyUrl);
 
     ApiException thrownException = assertThrows(ApiException.class,
@@ -61,8 +55,6 @@ void testHandleInvalidOpenIdRedirectBlankIdentityParam() {
 
   @Test
   void testHandleInvalidOpenIdRedirectNoIdentityInRequest() {
-    when(requestMock.getParameterMap()).thenReturn(Collections.emptyMap());
-
     ApiException thrownException = assertThrows(ApiException.class,
         () -> beanUnderTest.handleInvalidOpenIdRedirect(requestMock, DUMMY_RESPONSE));
     assertEquals(ErrorCode.STEAM_LOGIN_VALIDATION_FAILED,
@@ -76,8 +68,6 @@ void testHandleInvalidOpenIdRedirectLinkedAccountExists() {
     when(userMock.getLogin()).thenReturn("dummyLogin");
     AccountLink accountLinkMock = Mockito.mock(AccountLink.class);
     when(accountLinkMock.getUser()).thenReturn(userMock);
-    when(requestMock.getParameterMap())
-        .thenReturn(Map.of(IDENTITY_NAME_PARAM, new String[]{DUMMY_URL}));
     when(requestMock.getParameter(IDENTITY_NAME_PARAM)).thenReturn(DUMMY_URL);
     when(accountLinkRepositoryMock.findOneByServiceIdAndServiceType(anyString(),
         any(LinkedServiceType.class))).thenReturn(