SSO login bug fix

coral/types/api.d.ts

@@ -546,7 +546,7 @@ export type components = {
       errCode?: string;
       message: string;
       debugMessage?: string;
-      data?: Record<string, never>;
+      data?: unknown;
       /** Format: date-time */
       timestamp?: string;
     };
@@ -629,7 +629,7 @@ export type components = {
       sourceEnv?: string;
       selectedTeam?: string;
       typeOfSync?: string;
-      topicDetails?: Record<string, never>[];
+      topicDetails?: unknown[];
       topicSearchFilter?: string;
     };
     SyncConnectorUpdates: {
@@ -2230,25 +2230,25 @@ export type operations = {
       /** @description OK */
       200: {
         content: {
-          "application/json": components["schemas"]["ApiResponse"][];
+          "application/json": unknown;
         };
       };
       /** @description Multi Status */
       207: {
         content: {
-          "application/json": components["schemas"]["ApiResponse"][];
+          "application/json": unknown;
         };
       };
       /** @description Bad Request */
       405: {
         content: {
-          "application/json": components["schemas"]["ApiResponse"][];
+          "application/json": unknown;
         };
       };
       /** @description Internal Server Error */
       500: {
         content: {
-          "application/json": components["schemas"]["ApiResponse"][];
+          "application/json": unknown;
         };
       };
     };
@@ -2267,25 +2267,25 @@ export type operations = {
       /** @description OK */
       200: {
         content: {
-          "application/json": components["schemas"]["ApiResponse"][];
+          "application/json": unknown;
         };
       };
       /** @description Multi Status */
       207: {
         content: {
-          "application/json": components["schemas"]["ApiResponse"][];
+          "application/json": unknown;
         };
       };
       /** @description Bad Request */
       405: {
         content: {
-          "application/json": components["schemas"]["ApiResponse"][];
+          "application/json": unknown;
         };
       };
       /** @description Internal Server Error */
       500: {
         content: {
-          "application/json": components["schemas"]["ApiResponse"][];
+          "application/json": unknown;
         };
       };
     };
@@ -2304,25 +2304,25 @@ export type operations = {
       /** @description OK */
       200: {
         content: {
-          "application/json": components["schemas"]["ApiResponse"][];
+          "application/json": unknown;
         };
       };
       /** @description Multi Status */
       207: {
         content: {
-          "application/json": components["schemas"]["ApiResponse"][];
+          "application/json": unknown;
         };
       };
       /** @description Bad Request */
       405: {
         content: {
-          "application/json": components["schemas"]["ApiResponse"][];
+          "application/json": unknown;
         };
       };
       /** @description Internal Server Error */
       500: {
         content: {
-          "application/json": components["schemas"]["ApiResponse"][];
+          "application/json": unknown;
         };
       };
     };

core/src/main/java/io/aiven/klaw/helpers/db/rdbms/InsertDataJdbc.java

@@ -1,9 +1,10 @@
 package io.aiven.klaw.helpers.db.rdbms;
 
+import static org.springframework.beans.BeanUtils.copyProperties;
+
 import io.aiven.klaw.dao.*;
 import io.aiven.klaw.model.enums.ApiResultStatus;
 import io.aiven.klaw.model.enums.EntityType;
-import io.aiven.klaw.model.enums.NewUserStatus;
 import io.aiven.klaw.model.enums.RequestStatus;
 import io.aiven.klaw.repository.*;
 import java.sql.Timestamp;
@@ -380,13 +381,21 @@ public String insertIntoRegisterUsers(RegisterUserInfo userInfo) {
     Optional<UserInfo> userNameExists = userInfoRepo.findById(userInfo.getUsername());
     if (userNameExists.isPresent()) return "Failure. User already exists";
 
-    // STAGING status comes from AD users
-    RegisterUserInfo userRegistration =
-        registerInfoRepo.findFirstByUsernameAndStatusIn(
-            userInfo.getUsername(),
-            List.of(NewUserStatus.PENDING.value, NewUserStatus.STAGING.value));
-    if (userRegistration != null) {
-      return "Failure. Registration already exists";
+    Optional<RegisterUserInfo> optionalUserRegistration =
+        registerInfoRepo.findByUsername(userInfo.getUsername());
+    if (optionalUserRegistration.isPresent()) {
+      if ("APPROVED".equals(optionalUserRegistration.get().getStatus())) {
+        // do nothing -- user is deleted
+      } else if (!"STAGING".equals(optionalUserRegistration.get().getStatus())
+          && !"PENDING".equals(optionalUserRegistration.get().getStatus())) {
+        return "Failure. Registration already exists";
+      } else {
+        int id = optionalUserRegistration.get().getId();
+        copyProperties(userInfo, optionalUserRegistration.get());
+        optionalUserRegistration.get().setId(id);
+        registerInfoRepo.save(optionalUserRegistration.get());
+        return ApiResultStatus.SUCCESS.value;
+      }
     }
 
     registerInfoRepo.save(userInfo);

core/src/main/java/io/aiven/klaw/helpers/db/rdbms/UpdateDataJdbc.java

@@ -488,7 +488,10 @@ public void updateNewUserRequest(String username, String approver, boolean isApp
     if (isApprove) {
       status = NewUserStatus.APPROVED.value;
     } else {
-      status = NewUserStatus.DECLINED.value;
+      // In case if user registration is declined, delete the record from db, so user can try to
+      // register again with any new data.
+      registerInfoRepo.deleteById("" + registerUser.getId());
+      return;
     }
     if (registerUser != null) {
       if (NewUserStatus.PENDING.value.equals(registerUser.getStatus())) {

core/src/main/java/io/aiven/klaw/service/UsersTeamsControllerService.java

@@ -620,7 +620,8 @@ public ApiResponse addNewUser(UserInfoModel newUser, boolean isExternal) throws
 
       if (isExternal) {
 
-        if ("".equals(newUser.getUserPassword())) {
+        if ("".equals(newUser.getUserPassword())
+            || ACTIVE_DIRECTORY.value.equals(authenticationType)) {
           mailService.sendMail(
               newUser.getUsername(),
               newUser.getUserPassword(),

core/src/test/java/io/aiven/klaw/MockMethods.java

@@ -7,6 +7,7 @@
 import io.aiven.klaw.model.requests.EnvModel;
 import io.aiven.klaw.model.requests.KwClustersModel;
 import io.aiven.klaw.model.requests.KwRolesPermissionsModel;
+import io.aiven.klaw.model.requests.RegisterUserInfoModel;
 import io.aiven.klaw.model.requests.TeamModel;
 import io.aiven.klaw.model.requests.UserInfoModel;
 import io.aiven.klaw.model.response.EnvParams;
@@ -53,6 +54,18 @@ public UserInfoModel getUserInfoModel(String username, String role, String team)
     return userInfoModel;
   }
 
+  public RegisterUserInfoModel getRegisterUserInfoModel(String username, String role) {
+    RegisterUserInfoModel userInfoModel = new RegisterUserInfoModel();
+    userInfoModel.setUsername(username);
+    userInfoModel.setPwd("testpwd");
+    userInfoModel.setRole(role);
+    userInfoModel.setTeamId(1001);
+    userInfoModel.setFullname("New User");
+    userInfoModel.setMailid("[email protected]");
+
+    return userInfoModel;
+  }
+
   public UserInfoModel getUserInfoModelSwitchTeams(
       String username, String role, int teamId, int switchTeamSize) {
     UserInfoModel userInfoModel = new UserInfoModel();

core/src/test/java/io/aiven/klaw/SsoActiveDirectoryAuthenticationIT.java

@@ -1,15 +1,28 @@
 package io.aiven.klaw;
 
+import static io.aiven.klaw.UsersTeamsControllerIT.OBJECT_MAPPER;
+import static io.aiven.klaw.UsersTeamsControllerIT.superAdmin;
+import static io.aiven.klaw.helpers.KwConstants.STAGINGTEAM;
+import static io.aiven.klaw.model.enums.NewUserStatus.PENDING;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.springframework.security.oauth2.core.oidc.endpoint.OidcParameterNames.ID_TOKEN;
 import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.oidcLogin;
 import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.user;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 
+import com.fasterxml.jackson.core.type.TypeReference;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import io.aiven.klaw.model.ApiResponse;
+import io.aiven.klaw.model.requests.RegisterUserInfoModel;
+import io.aiven.klaw.model.response.RegisterUserInfoModelResponse;
+import io.aiven.klaw.model.response.UserInfoModelResponse;
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
+import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.MethodOrderer;
 import org.junit.jupiter.api.Order;
 import org.junit.jupiter.api.Test;
@@ -42,6 +55,13 @@ public class SsoActiveDirectoryAuthenticationIT {
 
   @Autowired private MockMvc mvc;
 
+  private static MockMethods mockMethods;
+
+  @BeforeAll
+  public static void setup() {
+    mockMethods = new MockMethods();
+  }
+
   // Login with Oidc profile with success
   @Test
   @Order(1)
@@ -53,8 +73,9 @@ public void invokeRootPageWithOidcLoginSuccess() {
                       .with(
                           oidcLogin()
                               .oidcUser(
-                                  getOidcUser())) // oidc login with valid preferredName/claims and
-                      // authorities
+                                  getOidcUser(
+                                      superAdmin))) // oidc login with valid preferredName/claims
+                      // and authorities
                       .contentType(MediaType.APPLICATION_JSON)
                       .accept(MediaType.APPLICATION_JSON))
               .andReturn()
@@ -77,9 +98,9 @@ public void invokeRootPageWithBasicLoginFailure() {
           mvc.perform(
                   MockMvcRequestBuilders.get("/")
                       .with(
-                          user("superadmin")
-                              .password("superAdminPwd")) // Invalid login for AD context
-                      // authentication
+                          user(superAdmin)
+                              .password(
+                                  "superAdminPwd")) // Invalid login for AD context authentication
                       .contentType(MediaType.APPLICATION_JSON)
                       .accept(MediaType.APPLICATION_JSON))
               .andReturn()
@@ -95,11 +116,104 @@ public void invokeRootPageWithBasicLoginFailure() {
     ;
   }
 
-  private OidcUser getOidcUser() {
+  /*
+   1. Login with valid AD user, but user doesn't exist in klaw
+   2. A record is created in kwregisterusers table in STAGING state and User is routed to register page to submit all details and signup.
+   3. After user registers, record in kwregisterusers table has an updated status of PENDING
+   4. superadmin now approves and the user should exist in kwusers
+  */
+  @Test
+  @Order(3)
+  public void invokeRootPageWithOidcLoginFailure() {
+    String nonExistingUserInKlaw = "testuser";
+    try {
+      // From browser, this triggers a user to be created in staging users table (kwregisterusers),
+      // if successful login in SSO but user doesn't exist in klaw
+      mvc.perform(
+              MockMvcRequestBuilders.get("/")
+                  .with(
+                      oidcLogin()
+                          .oidcUser(
+                              getOidcUser(
+                                  nonExistingUserInKlaw))) // oidc login with non existing user
+                  .contentType(MediaType.APPLICATION_JSON)
+                  .accept(MediaType.APPLICATION_JSON))
+          .andReturn()
+          .getResponse();
+      RegisterUserInfoModel userInfoModel =
+          mockMethods.getRegisterUserInfoModel(nonExistingUserInKlaw, "USER");
+      String jsonReq = OBJECT_MAPPER.writer().writeValueAsString(userInfoModel);
+
+      // Allow the user to signup
+      MockHttpServletResponse response2 =
+          mvc.perform(
+                  MockMvcRequestBuilders.post("/registerUser")
+                      .with(oidcLogin().oidcUser(getOidcUser(nonExistingUserInKlaw)))
+                      .content(jsonReq)
+                      .contentType(MediaType.APPLICATION_JSON)
+                      .accept(MediaType.APPLICATION_JSON))
+              .andReturn()
+              .getResponse();
+      ApiResponse objectResponse =
+          new ObjectMapper().readValue(response2.getContentAsString(), ApiResponse.class);
+      assertThat(objectResponse.isSuccess()).isTrue();
+
+      // Allow the superadmin to fetch requests to approve
+      MockHttpServletResponse response3 =
+          mvc.perform(
+                  MockMvcRequestBuilders.get("/getNewUserRequests")
+                      .with(oidcLogin().oidcUser(getOidcUser(superAdmin)))
+                      .contentType(MediaType.APPLICATION_JSON)
+                      .accept(MediaType.APPLICATION_JSON))
+              .andReturn()
+              .getResponse();
+
+      List<RegisterUserInfoModelResponse> userInfoModelActualList =
+          new ObjectMapper().readValue(response3.getContentAsString(), new TypeReference<>() {});
+
+      assertThat(userInfoModelActualList.get(0).getUsername()).isEqualTo(nonExistingUserInKlaw);
+      assertThat(userInfoModelActualList.get(0).getStatus()).isEqualTo(PENDING.value);
+
+      // Allow the superadmin to approve the user
+      MockHttpServletResponse response4 =
+          mvc.perform(
+                  MockMvcRequestBuilders.post("/execNewUserRequestApprove")
+                      .with(oidcLogin().oidcUser(getOidcUser(superAdmin)))
+                      .param("username", nonExistingUserInKlaw)
+                      .contentType(MediaType.APPLICATION_JSON)
+                      .accept(MediaType.APPLICATION_JSON))
+              .andReturn()
+              .getResponse();
+      ApiResponse objectResponse1 =
+          new ObjectMapper().readValue(response4.getContentAsString(), ApiResponse.class);
+      assertThat(objectResponse1.isSuccess()).isTrue();
+
+      // Fetch and see if user is now created
+      String userDetailsResponse =
+          mvc.perform(
+                  MockMvcRequestBuilders.get("/getUserDetails")
+                      .with(oidcLogin().oidcUser(getOidcUser(superAdmin)))
+                      .param("userId", nonExistingUserInKlaw)
+                      .contentType(MediaType.APPLICATION_JSON)
+                      .accept(MediaType.APPLICATION_JSON))
+              .andExpect(status().isOk())
+              .andReturn()
+              .getResponse()
+              .getContentAsString();
+      UserInfoModelResponse userInfoModelActual =
+          new ObjectMapper().readValue(userDetailsResponse, new TypeReference<>() {});
+      assertThat(userInfoModelActual.getTeam()).isEqualTo(STAGINGTEAM);
+
+    } catch (Exception e) {
+      throw new RuntimeException(e);
+    }
+  }
+
+  private OidcUser getOidcUser(String username) {
     Map<String, Object> claims = new HashMap<>();
     claims.put("groups", "ROLE_USER");
     claims.put("sub", 123);
-    claims.put("preferred_username", "superadmin"); // existing user with default installation
+    claims.put("preferred_username", username); // existing user with default installation
     OidcIdToken idToken =
         new OidcIdToken(ID_TOKEN, Instant.now(), Instant.now().plusSeconds(60), claims);
     Collection<GrantedAuthority> authorities = getAuthorities();