added some test cases

sourcefuse · Oct 4, 2023 · 7f7c55b · 7f7c55b
1 parent 8138d28
commit 7f7c55b
Show file tree

Hide file tree

Showing 18 changed files with 740 additions and 193 deletions.
diff --git a/...es/authservice/oauth2/auth/request/resolver/CustomOAuth2AuthorizationRequestResolver.java b/...es/authservice/oauth2/auth/request/resolver/CustomOAuth2AuthorizationRequestResolver.java
@@ -2,10 +2,10 @@
 
 import com.sourcefuse.jarc.services.authservice.oauth2.auth.utils.StateUtils;
 import jakarta.servlet.http.HttpServletRequest;
-import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
+import java.text.MessageFormat;
 import java.util.Base64;
 import java.util.HashMap;
 import java.util.Map;
@@ -54,7 +54,8 @@ public final class CustomOAuth2AuthorizationRequestResolver
 
   private Consumer<
     OAuth2AuthorizationRequest.Builder
-  > authorizationRequestCustomizer = customizer -> {};
+  > authorizationRequestCustomizer =
+    (OAuth2AuthorizationRequest.Builder customizer) -> {};
 
   /**
    * Constructs a {@code DefaultOAuth2AuthorizationRequestResolver} using the
@@ -112,9 +113,6 @@ public CustomOAuth2AuthorizationRequestResolver(
   @Override
   public OAuth2AuthorizationRequest resolve(HttpServletRequest request) {
     String registrationId = resolveRegistrationId(request);
-    if (registrationId == null) {
-      return null;
-    }
     String redirectUriAction = getAction(request, "login");
     return resolve(request, registrationId, redirectUriAction);
   }
@@ -124,9 +122,6 @@ public OAuth2AuthorizationRequest resolve(
     HttpServletRequest request,
     String registrationId
   ) {
-    if (registrationId == null) {
-      return null;
-    }
     String redirectUriAction = getAction(request, "authorize");
     return resolve(request, registrationId, redirectUriAction);
   }
@@ -149,7 +144,10 @@ public void setAuthorizationRequestCustomizer(
     this.authorizationRequestCustomizer = authorizationRequestCustomizer;
   }
 
-  private String getAction(HttpServletRequest request, String defaultAction) {
+  private static String getAction(
+    HttpServletRequest request,
+    String defaultAction
+  ) {
     String action = request.getParameter("action");
     if (action == null) {
       return defaultAction;
@@ -162,6 +160,9 @@ private OAuth2AuthorizationRequest resolve(
     String registrationId,
     String redirectUriAction
   ) {
+    if (registrationId == null) {
+      return null;
+    }
     ClientRegistration clientRegistration =
       this.clientRegistrationRepository.findByRegistrationId(registrationId);
     if (clientRegistration == null) {
@@ -170,34 +171,29 @@ private OAuth2AuthorizationRequest resolve(
       );
     }
 
-    String clientId = request.getParameter(StateUtils.clientIdParamKey);
+    String clientId = request.getParameter(StateUtils.CLIENT_ID_PARAM_KEY);
     OAuth2AuthorizationRequest.Builder builder = getBuilder(clientRegistration);
 
     String redirectUriStr = expandRedirectUri(
       request,
       clientRegistration,
       redirectUriAction
     );
-    try {
-      builder
-        .clientId(clientRegistration.getClientId())
-        .authorizationUri(
-          clientRegistration.getProviderDetails().getAuthorizationUri()
-        )
-        .redirectUri(redirectUriStr)
-        .scopes(clientRegistration.getScopes())
-        .state(StateUtils.encode(clientId));
-    } catch (IOException e) {
-      log.info(null, e);
-      throw new IllegalArgumentException(e.getMessage(), e.getCause());
-    }
 
+    builder
+      .clientId(clientRegistration.getClientId())
+      .authorizationUri(
+        clientRegistration.getProviderDetails().getAuthorizationUri()
+      )
+      .redirectUri(redirectUriStr)
+      .scopes(clientRegistration.getScopes())
+      .state(StateUtils.encode(clientId));
     this.authorizationRequestCustomizer.accept(builder);
 
     return builder.build();
   }
 
-  private OAuth2AuthorizationRequest.Builder getBuilder(
+  private static OAuth2AuthorizationRequest.Builder getBuilder(
     ClientRegistration clientRegistration
   ) {
     if (
@@ -264,7 +260,10 @@ private String resolveRegistrationId(HttpServletRequest request) {
    * Null variables are provided as empty strings.
    * <p/>
    * Default redirectUri is:
-   * {@code org.springframework.security.config.oauth2.client.CommonOAuth2Provider#DEFAULT_REDIRECT_URL}
+   * {@code
+   * org.springframework.security.config.oauth2.client.CommonOAuth2Provider
+   * #DEFAULT_REDIRECT_URL
+   * }
    *
    * @return expanded URI
    */
@@ -274,7 +273,10 @@ private static String expandRedirectUri(
     String action
   ) {
     Map<String, String> uriVariables = new HashMap<>();
-    uriVariables.put("registrationId", clientRegistration.getRegistrationId());
+    uriVariables.put(
+      REGISTRATION_ID_URI_VARIABLE_NAME,
+      clientRegistration.getRegistrationId()
+    );
 
     UriComponents uriComponents = UriComponentsBuilder
       .fromHttpUrl(UrlUtils.buildFullRequestUrl(request))
@@ -289,12 +291,13 @@ private static String expandRedirectUri(
     uriVariables.put("baseHost", (host != null) ? host : "");
 
     int port = uriComponents.getPort();
-    uriVariables.put("basePort", (port == -1) ? "" : ":" + port);
+    uriVariables.put(
+      "basePort",
+      (port == -1) ? "" : MessageFormat.format(":{0}", port)
+    );
     String path = uriComponents.getPath();
-    if (StringUtils.hasLength(path)) {
-      if (path.charAt(0) != PATH_DELIMITER) {
-        path = PATH_DELIMITER + path;
-      }
+    if (StringUtils.hasLength(path) && path.charAt(0) != PATH_DELIMITER) {
+      path = PATH_DELIMITER + path;
     }
     uriVariables.put("basePath", (path != null) ? path : "");
     uriVariables.put("baseUrl", uriComponents.toUriString());
@@ -321,7 +324,9 @@ private static void applyNonce(OAuth2AuthorizationRequest.Builder builder) {
       builder.additionalParameters(params ->
         params.put(OidcParameterNames.NONCE, nonceHash)
       );
-    } catch (NoSuchAlgorithmException ex) {}
+    } catch (NoSuchAlgorithmException ex) {
+      log.info(null, ex);
+    }
   }
 
   private static String createHash(String value)

diff --git a/...src/main/java/com/sourcefuse/jarc/services/authservice/oauth2/auth/utils/CommonUtils.java b/...src/main/java/com/sourcefuse/jarc/services/authservice/oauth2/auth/utils/CommonUtils.java
@@ -0,0 +1,15 @@
+package com.sourcefuse.jarc.services.authservice.oauth2.auth.utils;
+
+import com.sourcefuse.jarc.services.authservice.enums.AuthErrorKeys;
+import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
+
+public final class CommonUtils {
+
+  private CommonUtils() {}
+
+  public static OAuth2AuthenticationException throwUserVerificationFailed() {
+    return new OAuth2AuthenticationException(
+      AuthErrorKeys.USER_VERIFICATION_FAILED.toString()
+    );
+  }
+}
diff --git a/.../src/main/java/com/sourcefuse/jarc/services/authservice/oauth2/auth/utils/StateUtils.java b/.../src/main/java/com/sourcefuse/jarc/services/authservice/oauth2/auth/utils/StateUtils.java
@@ -1,6 +1,5 @@
 package com.sourcefuse.jarc.services.authservice.oauth2.auth.utils;
 
-import com.fasterxml.jackson.core.JsonProcessingException;
 import com.google.gson.Gson;
 import java.nio.charset.StandardCharsets;
 import java.util.Base64;
@@ -10,13 +9,13 @@
 
 public final class StateUtils {
 
-  public static final String clientIdParamKey = "clientId";
+  public static final String CLIENT_ID_PARAM_KEY = "clientId";
 
   private StateUtils() {}
 
-  public static String encode(String clientId) throws JsonProcessingException {
+  public static String encode(String clientId) {
     Map<String, String> dataMap = new HashMap<>();
-    dataMap.put(clientIdParamKey, clientId);
+    dataMap.put(CLIENT_ID_PARAM_KEY, clientId);
     dataMap.put("id", UUID.randomUUID().toString());
 
     // Convert the map to a JSON string
@@ -37,6 +36,6 @@ public static String decode(String encodedState) {
     Map<String, Object> dataMap = new Gson().fromJson(decodedString, Map.class);
 
     // Retrieve and return the state from the map
-    return (String) dataMap.get(clientIdParamKey);
+    return (String) dataMap.get(CLIENT_ID_PARAM_KEY);
   }
 }
diff --git a/...va/com/sourcefuse/jarc/services/authservice/oauth2/providers/OAuth2PreVerifyProvider.java b/...va/com/sourcefuse/jarc/services/authservice/oauth2/providers/OAuth2PreVerifyProvider.java
@@ -44,18 +44,15 @@ public User provide(User user, OidcUser oidcUser) {
   public User provide(User user, OAuth2UserInfo oAuth2UserInfo) {
     String fullname = oAuth2UserInfo.getName().replace("( )+", " ");
     List<String> name = Arrays.asList(fullname.split(" "));
-    String firstName = name.size() > 0 ? name.get(0) : null;
-    String lastName = name.size() > 1 ? name.get(name.size() - 1) : null;
-
-    boolean shouldUpdateFirstName =
-      (firstName != null && !firstName.isBlank()) &&
-      !user.getFirstName().equals(firstName);
-    boolean shouldUpdateLastName =
-      (lastName != null && !lastName.isBlank()) &&
-      !user.getLastName().equals(lastName);
+    String firstName = (!name.isEmpty() && name.get(0) != null)
+      ? name.get(0)
+      : user.getFirstName();
+    String lastName = (name.size() > 1 && name.get(name.size() - 1) != null)
+      ? name.get(name.size() - 1)
+      : user.getLastName();
     if (
-      shouldUpdateFirstName ||
-      shouldUpdateLastName ||
+      checkNameIsValidAndNotMatches(firstName, user.getFirstName()) ||
+      checkNameIsValidAndNotMatches(lastName, user.getLastName()) ||
       !user.getUsername().equals(oAuth2UserInfo.getEmail()) ||
       !user.getEmail().equals(oAuth2UserInfo.getEmail())
     ) {
@@ -85,4 +82,11 @@ private void checkUserTenant(User user) {
       this.userTenantRepository.save(userTenant.get());
     }
   }
+
+  private static boolean checkNameIsValidAndNotMatches(
+    String name,
+    String nameToMatch
+  ) {
+    return (name != null && !name.isBlank()) && !nameToMatch.equals(name);
+  }
 }
diff --git a/.../java/com/sourcefuse/jarc/services/authservice/oauth2/providers/OAuth2SignupProvider.java b/.../java/com/sourcefuse/jarc/services/authservice/oauth2/providers/OAuth2SignupProvider.java
@@ -52,7 +52,7 @@ public User provide(OAuth2UserInfo oAuth2UserInfo, AuthProvider provider) {
     }
     String fullname = oAuth2UserInfo.getName().replace("( )+", " ");
     List<String> name = Arrays.asList(fullname.split(" "));
-    String firstName = name.size() > 0 ? name.get(0) : null;
+    String firstName = !name.isEmpty() ? name.get(0) : null;
     String lastName = name.size() > 1 ? name.get(name.size() - 1) : null;
 
     User userToCreate = new User();

diff --git a/...se/jarc/services/authservice/oauth2/request/filters/OAuth2AuthorizationRequestFilter.java b/...se/jarc/services/authservice/oauth2/request/filters/OAuth2AuthorizationRequestFilter.java
@@ -39,7 +39,7 @@ protected void doFilterInternal(
     HttpServletResponse response,
     FilterChain filterChain
   ) throws ServletException, IOException {
-    String clientId = request.getParameter(StateUtils.clientIdParamKey);
+    String clientId = request.getParameter(StateUtils.CLIENT_ID_PARAM_KEY);
     if (
       authorizationRequestMatcher.matches(request) &&
       (clientId == null || clientId.isBlank())

diff --git a/...ava/com/sourcefuse/jarc/services/authservice/oauth2/services/CustomOAuth2UserService.java b/...ava/com/sourcefuse/jarc/services/authservice/oauth2/services/CustomOAuth2UserService.java
@@ -1,11 +1,11 @@
 package com.sourcefuse.jarc.services.authservice.oauth2.services;
 
-import com.sourcefuse.jarc.services.authservice.enums.AuthErrorKeys;
 import com.sourcefuse.jarc.services.authservice.enums.AuthProvider;
 import com.sourcefuse.jarc.services.authservice.models.Role;
 import com.sourcefuse.jarc.services.authservice.models.User;
 import com.sourcefuse.jarc.services.authservice.models.UserCredential;
 import com.sourcefuse.jarc.services.authservice.models.UserTenant;
+import com.sourcefuse.jarc.services.authservice.oauth2.auth.utils.CommonUtils;
 import com.sourcefuse.jarc.services.authservice.oauth2.providers.OAuth2PreVerifyProvider;
 import com.sourcefuse.jarc.services.authservice.oauth2.providers.OAuth2SignupProvider;
 import com.sourcefuse.jarc.services.authservice.oauth2.user.OAuth2UserInfo;
@@ -47,9 +47,8 @@ public class CustomOAuth2UserService extends DefaultOAuth2UserService {
   @Transactional
   public OAuth2User loadUser(OAuth2UserRequest oAuth2UserRequest)
     throws OAuth2AuthenticationException {
-    OAuth2User oAuth2User = super.loadUser(oAuth2UserRequest);
-
     try {
+      OAuth2User oAuth2User = super.loadUser(oAuth2UserRequest);
       return processOAuth2User(oAuth2UserRequest, oAuth2User);
     } catch (AuthenticationException ex) {
       log.error(null, ex);
@@ -65,7 +64,7 @@ public OAuth2User loadUser(OAuth2UserRequest oAuth2UserRequest)
     }
   }
 
-  private OAuth2User processOAuth2User(
+  public OAuth2User processOAuth2User(
     OAuth2UserRequest oAuth2UserRequest,
     OAuth2User oAuth2User
   ) {
@@ -101,29 +100,23 @@ private OAuth2User processOAuth2User(
       this.userCredentialRepository.findOne(
           UserCredentialSpecification.byUserId(user.getId())
         )
-        .orElseThrow(this::throwUserVerificationFailed);
+        .orElseThrow(CommonUtils::throwUserVerificationFailed);
     if (
       !userCredential.getAuthProvider().equalsIgnoreCase(provider) ||
       (!userCredential.getAuthId().equalsIgnoreCase(oAuth2UserInfo.getId()))
     ) {
-      throw throwUserVerificationFailed();
+      throw CommonUtils.throwUserVerificationFailed();
     }
     UserTenant userTenant =
       this.userTenantRepository.findOne(
           UserTenantSpecification.byUserId(user.getId())
         )
-        .orElseThrow(this::throwUserVerificationFailed);
+        .orElseThrow(CommonUtils::throwUserVerificationFailed);
 
     Role role =
       this.roleRepository.findById(userTenant.getRoleId())
-        .orElseThrow(this::throwUserVerificationFailed);
+        .orElseThrow(CommonUtils::throwUserVerificationFailed);
 
     return new OAuth2UserSession(user, userTenant, role, oAuth2User);
   }
-
-  private OAuth2AuthenticationException throwUserVerificationFailed() {
-    return new OAuth2AuthenticationException(
-      AuthErrorKeys.USER_VERIFICATION_FAILED.toString()
-    );
-  }
 }