From 31e995eb60894062cbfcac136ae6a70ecba16860 Mon Sep 17 00:00:00 2001 From: gitCarrot Date: Fri, 30 Aug 2024 14:05:40 -1000 Subject: [PATCH] Improve ootb user experience and fix minor errors Change OotbActiveUserProfileService in the test from autowired to MockBean and add mocking in setup function for better unit test Ignore post construct in ootb controller in OotbSecurityConfigTest to fucus on the functions of OotbSecurityConfig test test for v4 git actions Change post construct function to update admin user at first to EventListener --- .github/workflows/coverage.yml | 2 +- .../api/controller/OotbRestController.java | 18 + .../groupings/controller/HomeController.java | 11 +- .../resources/ootb.active.user.profiles.json | 329 +++++++++--------- .../configuration/OotbSecurityConfigTest.java | 16 +- .../OotbActiveUserProfileServiceTest.java | 36 +- 6 files changed, 238 insertions(+), 174 deletions(-) diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 7ee66fc89..a51950ab8 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -52,7 +52,7 @@ jobs: git push fi - name: Upload Jacoco coverage report - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v4 with: name: jacoco-report path: target/site/jacoco/ diff --git a/src/main/java/edu/hawaii/its/api/controller/OotbRestController.java b/src/main/java/edu/hawaii/its/api/controller/OotbRestController.java index 6c6f82b3b..4b67979c6 100644 --- a/src/main/java/edu/hawaii/its/api/controller/OotbRestController.java +++ b/src/main/java/edu/hawaii/its/api/controller/OotbRestController.java @@ -2,11 +2,15 @@ import java.util.List; +import jakarta.annotation.PostConstruct; + import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.owasp.html.PolicyFactory; import org.owasp.html.Sanitizers; import org.springframework.beans.factory.annotation.Value; +import org.springframework.context.event.ContextRefreshedEvent; +import org.springframework.context.event.EventListener; import org.springframework.http.HttpMethod; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.GetMapping; @@ -51,6 +55,20 @@ public OotbRestController(UserContextService userContextService, this.ootbHttpRequestService = ootbHttpRequestService; } + @EventListener + public void onApplicationEvent(ContextRefreshedEvent event) { + init(); + } + + public void init() { + String adminProfile = ootbActiveUserProfileService.findGivenNameForAdminRole(); + if (adminProfile != null) { + updateActiveDefaultUser(adminProfile); + } else { + logger.error("Admin profile not found, unable to initiate default API request."); + } + } + /* * Checks to make sure that the API is running and that there are no issues with * the overrides file. Gets the active profiles and only runs the tests the diff --git a/src/main/java/edu/hawaii/its/groupings/controller/HomeController.java b/src/main/java/edu/hawaii/its/groupings/controller/HomeController.java index a17e42702..38946dce4 100755 --- a/src/main/java/edu/hawaii/its/groupings/controller/HomeController.java +++ b/src/main/java/edu/hawaii/its/groupings/controller/HomeController.java @@ -1,5 +1,6 @@ package edu.hawaii.its.groupings.controller; +import java.util.Arrays; import java.util.Locale; import java.util.Map; @@ -9,6 +10,7 @@ import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.annotation.Value; +import org.springframework.core.env.Environment; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.stereotype.Controller; import org.springframework.ui.Model; @@ -20,6 +22,7 @@ import org.springframework.web.servlet.mvc.support.RedirectAttributes; import edu.hawaii.its.groupings.access.UserContextService; +import edu.hawaii.its.groupings.configuration.Realm; import edu.hawaii.its.groupings.service.EmailService; import edu.hawaii.its.groupings.type.Feedback; @@ -38,15 +41,21 @@ public class HomeController { private final UserContextService userContextService; - public HomeController(EmailService emailService, UserContextService userContextService) { + private final Realm realmService; + + public HomeController(EmailService emailService, UserContextService userContextService, Realm realmService) { this.emailService = emailService; this.userContextService = userContextService; + this.realmService = realmService; } // Mapping to home. @GetMapping(value = { "/", "/home" }) public String home(Map locale) { logger.info("User at home. The client locale is " + locale); + if (realmService.isOotb()) { + return "redirect:/login"; + } return "home"; } diff --git a/src/main/resources/ootb.active.user.profiles.json b/src/main/resources/ootb.active.user.profiles.json index fac7f359f..9f084ad75 100644 --- a/src/main/resources/ootb.active.user.profiles.json +++ b/src/main/resources/ootb.active.user.profiles.json @@ -10,58 +10,62 @@ }, "groupings": [ { - "name": "group1:include", - "displayName": "group1:include", - "extension": "include", - "displayExtension": "include", - "description": "This is the first group", + "name": "shared-group-in-each-profile:owners", + "displayName": "shared-group-in-each-profile:owners", + "extension": "owners", + "displayExtension": "owners", + "description": "This is a shared group in each profile", "members": [ { - "name": "s", - "uhUuid": "11111112", - "uid": "member2" + "name": "shared-owner-1", + "uhUuid": "31255231", + "uid": "sowner1" } ] }, { - "name": "group2:exclude", - "displayName": "group2:exclude", + "name": "admin-include:exclude", + "displayName": "admin-include:exclude", "extension": "exclude", "displayExtension": "exclude", - "description": "This is the second group", + "description": "Admin owned include group", "members": [ { - "name": "s", - "uhUuid": "11111112", - "uid": "member2" + "name": "MemberUser", + "uid": "member0123", + "uhUuid": "11111111" } ] }, { - "name": "shared-groupPath:owners", - "displayName": "shared-groupPath:owners", + "name": "member-group:owners", + "displayName": "member-group:owners", "extension": "owners", "displayExtension": "owners", - "description": "This is the shared-groupPath", - "members": [ - { - "name": "s", - "uhUuid": "11111112", - "uid": "member2" - } - ] + "description": "Member owned group", + "members": [] }, { - "name": "memberGroup:owners", - "displayName": "memberGroup:owners", - "extension": "owners", - "displayExtension": "owners", - "description": "This is the memberGroup", + "name": "member-group:include", + "displayName": "member-group:include", + "extension": "include", + "displayExtension": "include", + "description": "Member owned group", "members": [ { - "name": "s", - "uhUuid": "11111112", - "uid": "member2" + "name": "complex-member7", + "uhUuid": "19283746", + "uid": "cmember7" + }, + { + "name": "complex-member10", + "uhUuid": "01234567", + "uid": "cmember10" + }, + { + "name": "AdminUser", + "uid": "admin0123", + "uhUuid": "33333333" } ] } @@ -78,86 +82,59 @@ }, "groupings": [ { - "name": "group1:include", - "displayName": "group1:include", + "name": "shared-group-in-each-profile:owners", + "displayName": "shared-group-in-each-profile:owners", + "extension": "owners", + "displayExtension": "owners", + "description": "This is a shared group in each profile", + "members": [ + { + "name": "shared-owner-2", + "uhUuid": "29325231", + "uid": "sowner2" + } + ] + }, + { + "name": "admin-include:include", + "displayName": "admin-include:include", "extension": "include", "displayExtension": "include", - "description": "This is the first group", + "description": "Admin owned include group", "members": [ { - "name": "s", - "uhUuid": "11111112", - "uid": "member2" + "name": "OwnerUser", + "uid": "owner0123", + "uhUuid": "22222222" + }, + { + "name": "AdminUser", + "uid": "admin0123", + "uhUuid": "33333333" } ] }, - { - "name": "group2:exclude", - "displayName": "group2:exclude", + { + "name": "owner-group:exclude", + "displayName": "owner-group:exclude", "extension": "exclude", "displayExtension": "exclude", - "description": "This is the second group", - "members": [ - { - "name": "s", - "uhUuid": "11111112", - "uid": "member2" - } - ] - }, - { - "name": "shared-groupPath:owners", - "displayName": "shared-groupPath:owners", - "extension": "owners", - "displayExtension": "owners", - "description": "This is the shared-groupPath", - "members": [ - { - "name": "s", - "uhUuid": "11111112", - "uid": "member2" - } - ] - }, - { - "name": "shared-groupPath:include", - "displayName": "shared-groupPath:include", - "extension": "include", - "displayExtension": "include", - "description": "This is the shared-groupPath", + "description": "Owner owned group", "members": [ { - "name": "s", - "uhUuid": "11111112", - "uid": "member2" - } - ] - }, - { - "name": "ownerGroup:owners", - "displayName": "ownerGroup:owners", - "extension": "owners", - "displayExtension": "owners", - "description": "This is the fourth group", - "members": [ + "name": "complex-member3", + "uhUuid": "56473829", + "uid": "cmember3" + }, { - "name": "s", - "uhUuid": "11111112", - "uid": "member2" - } - ] - }, - { - "name": "zoommeeting:owners", - "displayName": "zoommeeting:owners", - "extension": "owners", - "displayExtension": "owners", - "description": "This is the fourth group", - "members": [ + "name": "complex-member4", + "uhUuid": "45261378", + "uid": "cmember4" + }, { - "name": "s", - "uhUuid": "11111112", - "uid": "member2" + "name": "OwnerUser", + "uid": "owner0123", + "uhUuid": "22222222" } ] } @@ -174,59 +151,58 @@ }, "groupings": [ { - "name": "group1:include", - "displayName": "group1:include", - "extension": "include", - "displayExtension": "include", - "description": "This is the first group", + "name": "shared-group-in-groupings:owners", + "displayName": "shared-group-in-groupings:owners", + "extension": "owners", + "displayExtension": "owners", + "description": "This is a shared group in admin user groupings", "members": [ { - "name": "s", - "uhUuid": "11111112", - "uid": "member2" - } - ] - }, - { - "name": "group2:exclude", - "displayName": "group2:exclude", - "extension": "exclude", - "displayExtension": "exclude", - "description": "This is the second group", - "members": [ + "name": "AdminUser", + "uid": "admin0123", + "uhUuid": "33333333" + }, + { + "name": "OwnerUser", + "uid": "owner0123", + "uhUuid": "22222222" + }, { - "name": "s", - "uhUuid": "11111112", - "uid": "member2" + "name": "MemberUser", + "uid": "member0123", + "uhUuid": "11111111" } ] }, { - "name": "shared-groupPath:owners", - "displayName": "shared-groupPath:owners", + "name": "shared-group-in-each-profile:owners", + "displayName": "shared-group-in-each-profile:owners", "extension": "owners", "displayExtension": "owners", - "description": "This is the shared-groupPath", + "description": "This is a shared group in each profile", "members": [ { - "name": "s", - "uhUuid": "11111112", - "uid": "member2" + "name": "shared-owner-3", + "uhUuid": "29382734", + "uid": "sowner3" } ] }, { - "name": "adminGroup:owners", - "displayName": "adminGroup:owners", + "name": "admin-include:owners", + "displayName": "admin-include:owners", "extension": "owners", "displayExtension": "owners", - "description": "This is the fourth group", + "description": "Admin owned include group", + "members": [] + }, + { + "name": "admin-include:include", + "displayName": "admin-include:include", + "extension": "include", + "displayExtension": "include", + "description": "Admin owned include group", "members": [ - { - "name": "s", - "uhUuid": "11111112", - "uid": "member2" - }, { "name": "AdminUser", "uid": "admin0123", @@ -235,50 +211,69 @@ ] }, { - "name": "adminGroup:include", - "displayName": "adminGroup:include", - "extension": "include", - "displayExtension": "include", - "description": "This is the fourth group", + "name": "admin-group:owners", + "displayName": "admin-group:owners", + "extension": "owners", + "displayExtension": "owners", + "description": "Admin owned group", + "members": [] + }, + { + "name": "owner-complex:owners", + "displayName": "Owner-Complex: Owners", + "extension": "owners", + "displayExtension": "Owners", + "description": "Owner's owned complex group", "members": [ { - "name": "s", - "uhUuid": "11111112", - "uid": "member2" + "name": "complex-member1", + "uhUuid": "32532314", + "uid": "cmember1" }, { - "name": "AdminUser", - "uid": "admin0123", - "uhUuid": "33333333" + "name": "complex-member2", + "uhUuid": "87453218", + "uid": "cmember2" }, { - "name": "OwnerUser", - "uid": "owner0123", - "uhUuid": "22222222" - } - ] - }, - { - "name": "adminGroup:basis", - "displayName": "adminGroup:basis", - "extension": "basis", - "displayExtension": "basis", - "description": "This is the fourth group", - "members": [ + "name": "complex-member3", + "uhUuid": "56473829", + "uid": "cmember3" + }, { - "name": "s", - "uhUuid": "11111112", - "uid": "member2" + "name": "complex-member4", + "uhUuid": "45261378", + "uid": "cmember4" }, { - "name": "AdminUser", - "uid": "admin0123", - "uhUuid": "33333333" + "name": "complex-member5", + "uhUuid": "98765432", + "uid": "cmember5" }, { - "name": "OwnerUser", - "uid": "owner0123", - "uhUuid": "22222222" + "name": "complex-member6", + "uhUuid": "12345678", + "uid": "cmember6" + }, + { + "name": "complex-member7", + "uhUuid": "19283746", + "uid": "cmember7" + }, + { + "name": "complex-member8", + "uhUuid": "72635489", + "uid": "cmember8" + }, + { + "name": "complex-member9", + "uhUuid": "65432109", + "uid": "cmember9" + }, + { + "name": "complex-member10", + "uhUuid": "01234567", + "uid": "cmember10" } ] } diff --git a/src/test/java/edu/hawaii/its/groupings/configuration/OotbSecurityConfigTest.java b/src/test/java/edu/hawaii/its/groupings/configuration/OotbSecurityConfigTest.java index 384eef742..f58e07ba4 100644 --- a/src/test/java/edu/hawaii/its/groupings/configuration/OotbSecurityConfigTest.java +++ b/src/test/java/edu/hawaii/its/groupings/configuration/OotbSecurityConfigTest.java @@ -2,14 +2,14 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.InjectMocks; import org.mockito.Mock; -import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.security.authentication.AuthenticationManager; import org.springframework.security.config.annotation.authentication.configuration.AuthenticationConfiguration; import org.springframework.security.config.annotation.web.builders.HttpSecurity; @@ -18,17 +18,25 @@ import org.springframework.security.web.SecurityFilterChain; import org.springframework.test.context.ActiveProfiles; +import edu.hawaii.its.api.controller.OotbRestController; + @SpringBootTest(classes = { SpringBootWebApplication.class }) @ActiveProfiles("ootb") -@ExtendWith(MockitoExtension.class) class OotbSecurityConfigTest { @Autowired private OotbSecurityConfig ootbSecurityConfig; + @MockBean + private OotbRestController ootbRestController; @Mock private AuthenticationConfiguration authenticationConfiguration; + @BeforeEach + public void setUp() { + // Ignore post constructor to update ootb active profile in ootbRestController to focus on testing functions in OotbSecurityConfig + when(ootbRestController.updateActiveDefaultUser("admin0123")).thenReturn(null); + } @Test public void testAuthenticationManager() throws Exception { diff --git a/src/test/java/edu/hawaii/its/groupings/service/OotbActiveUserProfileServiceTest.java b/src/test/java/edu/hawaii/its/groupings/service/OotbActiveUserProfileServiceTest.java index f741a6334..88f5e8e11 100644 --- a/src/test/java/edu/hawaii/its/groupings/service/OotbActiveUserProfileServiceTest.java +++ b/src/test/java/edu/hawaii/its/groupings/service/OotbActiveUserProfileServiceTest.java @@ -5,29 +5,63 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import java.util.HashMap; import java.util.HashSet; +import java.util.List; +import java.util.Map; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.security.core.userdetails.UserDetails; import org.springframework.security.core.userdetails.UsernameNotFoundException; import org.springframework.test.context.ActiveProfiles; import edu.hawaii.its.groupings.access.Role; +import edu.hawaii.its.groupings.access.UhAttributes; import edu.hawaii.its.groupings.access.User; import edu.hawaii.its.groupings.configuration.SpringBootWebApplication; +import edu.hawaii.its.groupings.type.OotbActiveProfile; @SpringBootTest(classes = { SpringBootWebApplication.class }) @ActiveProfiles("ootb") class OotbActiveUserProfileServiceTest { + @MockBean private OotbActiveUserProfileService ootbActiveUserProfileService; @BeforeEach public void setUp() { - ootbActiveUserProfileService = new OotbActiveUserProfileService("ootb.active.user.profiles.json"); + + List availableProfiles = List.of("admin0123"); + + UhAttributes mockAttributes = mock(UhAttributes.class); + when(mockAttributes.getValue("givenName")).thenReturn("admin0123"); + when(mockAttributes.getValue("cn")).thenReturn("AdminUser"); + + User user = new User.Builder("admin0123") + .uhUuid("33333333") + .addAuthorities(List.of("ROLE_UH", "ROLE_OWNER", "ROLE_ADMIN")) + .addAttribute("givenName", "admin0123") + .build(); + + user.setAttributes(mockAttributes); + + Map activeProfiles = new HashMap<>(); + OotbActiveProfile adminProfile = mock(OotbActiveProfile.class); + activeProfiles.put("admin0123", adminProfile); + + when(ootbActiveUserProfileService.findGivenNameForAdminRole()).thenReturn("admin0123"); + when(ootbActiveUserProfileService.loadUserByUsername("admin0123")).thenReturn(user); + when(ootbActiveUserProfileService.loadUserByUsername("NON_EXISTENT")) + .thenThrow(new UsernameNotFoundException("User not found")); + when(ootbActiveUserProfileService.getAvailableProfiles()).thenReturn(availableProfiles); + when(ootbActiveUserProfileService.getUsers()).thenReturn(Map.of("admin0123", user)); + when(ootbActiveUserProfileService.getActiveProfiles()).thenReturn(activeProfiles); } @Test