Skip to content

Commit

Permalink
OZ-689 - Check for privileges and added tests
Browse files Browse the repository at this point in the history
  • Loading branch information
wluyima committed Oct 21, 2024
1 parent b692fd5 commit 81fe373
Show file tree
Hide file tree
Showing 6 changed files with 132 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

import static org.junit.Assert.assertEquals;

import java.util.ArrayList;
import java.util.Arrays;

import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
Expand Down Expand Up @@ -42,6 +45,8 @@ public void started_shouldPassIfTheExternalApiIsEnabledAndConfigIsValid() {
Mockito.when(mockConfig.getBaseUrl()).thenReturn("http://test.test");
Mockito.when(mockConfig.getUsername()).thenReturn("test-user");
Mockito.when(mockConfig.getPassword()).thenReturn("test-password");
Mockito.when(mockConfig.getChargeItemPrivileges()).thenReturn(Arrays.asList("test priv1"));
Mockito.when(mockConfig.getInventoryItemPrivileges()).thenReturn(Arrays.asList("test priv2"));
activator.started();
}

Expand Down Expand Up @@ -74,4 +79,60 @@ public void started_shouldFailIfPasswordIsMissing() {

assertEquals("Fhir Proxy module requires password when external FHIR API is enabled", ex.getMessage());
}

@Test
public void started_shouldFailIfChargeItemPrivilegesIsNull() {
Mockito.when(mockConfig.isExternalApiEnabled()).thenReturn(true);
Mockito.when(mockConfig.getBaseUrl()).thenReturn("http://test.test");
Mockito.when(mockConfig.getUsername()).thenReturn("test-user");
Mockito.when(mockConfig.getPassword()).thenReturn("test-password");

Exception ex = Assert.assertThrows(ModuleException.class, () -> activator.started());

assertEquals("Fhir Proxy module requires privileges for change item definition when external FHIR API is enabled",
ex.getMessage());
}

@Test
public void started_shouldFailIfChargeItemPrivilegesIsEmpty() {
Mockito.when(mockConfig.isExternalApiEnabled()).thenReturn(true);
Mockito.when(mockConfig.getBaseUrl()).thenReturn("http://test.test");
Mockito.when(mockConfig.getUsername()).thenReturn("test-user");
Mockito.when(mockConfig.getPassword()).thenReturn("test-password");
Mockito.when(mockConfig.getChargeItemPrivileges()).thenReturn(new ArrayList<>());

Exception ex = Assert.assertThrows(ModuleException.class, () -> activator.started());

assertEquals("Fhir Proxy module requires privileges for change item definition when external FHIR API is enabled",
ex.getMessage());
}

@Test
public void started_shouldFailIfInventoryPrivilegesIsNull() {
Mockito.when(mockConfig.isExternalApiEnabled()).thenReturn(true);
Mockito.when(mockConfig.getBaseUrl()).thenReturn("http://test.test");
Mockito.when(mockConfig.getUsername()).thenReturn("test-user");
Mockito.when(mockConfig.getPassword()).thenReturn("test-password");
Mockito.when(mockConfig.getChargeItemPrivileges()).thenReturn(Arrays.asList("test priv1"));

Exception ex = Assert.assertThrows(ModuleException.class, () -> activator.started());

assertEquals("Fhir Proxy module requires privileges for inventory item when external FHIR API is enabled",
ex.getMessage());
}

@Test
public void started_shouldFailIfInventoryPrivilegesIsEmpty() {
Mockito.when(mockConfig.isExternalApiEnabled()).thenReturn(true);
Mockito.when(mockConfig.getBaseUrl()).thenReturn("http://test.test");
Mockito.when(mockConfig.getUsername()).thenReturn("test-user");
Mockito.when(mockConfig.getPassword()).thenReturn("test-password");
Mockito.when(mockConfig.getChargeItemPrivileges()).thenReturn(Arrays.asList("test priv1"));
Mockito.when(mockConfig.getInventoryItemPrivileges()).thenReturn(new ArrayList<>());

Exception ex = Assert.assertThrows(ModuleException.class, () -> activator.started());

assertEquals("Fhir Proxy module requires privileges for inventory item when external FHIR API is enabled",
ex.getMessage());
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
package org.openmrs.module.fhirproxy;

import static org.mockito.Mockito.when;
import static org.openmrs.module.fhirproxy.Constants.GP_PRIV_CHARGE_ITEM;
import static org.openmrs.module.fhirproxy.Constants.GP_PRIV_INVENTORY;
import static org.openmrs.module.fhirproxy.Constants.MODULE_ID;

import java.io.File;
import java.util.Arrays;

import org.junit.After;
import org.junit.Assert;
Expand All @@ -11,6 +15,11 @@
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.openmrs.Privilege;
import org.openmrs.api.APIException;
import org.openmrs.api.AdministrationService;
import org.openmrs.api.UserService;
import org.openmrs.api.context.Context;
import org.openmrs.util.OpenmrsClassLoader;
import org.openmrs.util.OpenmrsUtil;
import org.powermock.api.mockito.PowerMockito;
Expand All @@ -20,19 +29,28 @@
import org.powermock.reflect.Whitebox;

@RunWith(PowerMockRunner.class)
@PrepareForTest(OpenmrsUtil.class)
@PrepareForTest({ OpenmrsUtil.class, Context.class })
@PowerMockIgnore({ "javax.management.*", "javax.script.*" })
public class FhirProxyUtilsTest {

@Mock
private Config mockConfig;

@Mock
private AdministrationService mockAdminService;

@Mock
private UserService mockUserService;

@Before
public void setup() {
PowerMockito.mockStatic(OpenmrsUtil.class);
PowerMockito.mockStatic(Context.class);
final String testCfgFile = OpenmrsClassLoader.getInstance().getResource(Constants.CONFIG_FILE).getFile();
File testCfgDir = new File(testCfgFile).getParentFile();
Mockito.when(OpenmrsUtil.getDirectoryInApplicationDataDirectory(MODULE_ID)).thenReturn(testCfgDir);
when(OpenmrsUtil.getDirectoryInApplicationDataDirectory(MODULE_ID)).thenReturn(testCfgDir);
when(Context.getAdministrationService()).thenReturn(mockAdminService);
when(Context.getUserService()).thenReturn(mockUserService);
}

@After
Expand All @@ -42,11 +60,29 @@ public void tearDown() {

@Test
public void getConfig_shouldLoadTheConfig() throws Exception {
final String privName1 = "priv-1";
final String privName2 = "priv-2";
final String privName3 = "priv-3";
final String privName4 = "priv-4";
Privilege priv1 = Mockito.mock(Privilege.class);
Privilege priv2 = Mockito.mock(Privilege.class);
Privilege priv3 = Mockito.mock(Privilege.class);
Privilege priv4 = Mockito.mock(Privilege.class);
when(mockAdminService.getGlobalProperty(GP_PRIV_CHARGE_ITEM)).thenReturn(privName1 + ", " + privName2);
when(mockAdminService.getGlobalProperty(GP_PRIV_INVENTORY)).thenReturn(privName3 + ", " + privName4);
when(mockUserService.getPrivilege(privName1)).thenReturn(priv1);
when(mockUserService.getPrivilege(privName2)).thenReturn(priv2);
when(mockUserService.getPrivilege(privName3)).thenReturn(priv3);
when(mockUserService.getPrivilege(privName4)).thenReturn(priv4);

Config cfg = FhirProxyUtils.getConfig();

Assert.assertTrue(cfg.isExternalApiEnabled());
Assert.assertEquals("http://test.test", cfg.getBaseUrl());
Assert.assertEquals("test-user", cfg.getUsername());
Assert.assertEquals("test-password", cfg.getPassword());
Assert.assertEquals(Arrays.asList(privName1, privName2), cfg.getChargeItemPrivileges());
Assert.assertEquals(Arrays.asList(privName3, privName4), cfg.getInventoryItemPrivileges());
}

@Test
Expand All @@ -55,4 +91,12 @@ public void getConfig_shouldGetCachedConfig() throws Exception {
Assert.assertEquals(mockConfig, FhirProxyUtils.getConfig());
}

@Test
public void loadPrivileges_shouldFailIfNoPrivilegeIsFoundMatchingTheName() {
final String privName = "priv";
when(mockAdminService.getGlobalProperty(GP_PRIV_CHARGE_ITEM)).thenReturn(privName);
Exception e = Assert.assertThrows(APIException.class, () -> FhirProxyUtils.getConfig());
Assert.assertEquals("No privilege found with name " + privName, e.getMessage());
}

}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo
}

HttpServletRequest request = (HttpServletRequest) servletRequest;
if (FhirProxyUtils.getConfig().isExternalApiEnabled() && Context.isSessionOpen()) {
if (FhirProxyUtils.getConfig().isExternalApiEnabled() && Context.isAuthenticated()) {
if (LOG.isDebugEnabled()) {
LOG.debug("Delegating to external API to process FHIR request -> {}", request.getRequestURI());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import static org.openmrs.module.fhirproxy.web.ProxyWebConstants.REQ_ROOT_PATH;

import java.io.IOException;
import java.util.Arrays;

import javax.servlet.FilterChain;
import javax.servlet.RequestDispatcher;
Expand All @@ -19,15 +20,17 @@
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.openmrs.api.context.Context;
import org.openmrs.module.fhirproxy.Config;
import org.openmrs.module.fhirproxy.Constants;
import org.openmrs.module.fhirproxy.FhirProxyUtils;
import org.powermock.api.mockito.PowerMockito;
import org.powermock.core.classloader.annotations.PowerMockIgnore;
import org.powermock.core.classloader.annotations.PrepareForTest;
import org.powermock.modules.junit4.PowerMockRunner;

@RunWith(PowerMockRunner.class)
@PrepareForTest(FhirProxyUtils.class)
@PrepareForTest({ FhirProxyUtils.class, Context.class })
@PowerMockIgnore({ "javax.management.*", "javax.script.*" })
public class FhirProxyFilterTest {

Expand All @@ -51,29 +54,36 @@ public class FhirProxyFilterTest {
@Before
public void setup() throws IOException {
PowerMockito.mockStatic(FhirProxyUtils.class);
PowerMockito.mockStatic(Context.class);
when(FhirProxyUtils.getConfig()).thenReturn(mockConfig);
when(Context.isAuthenticated()).thenReturn(true);
filter = new FhirProxyFilter();
}

@Test
public void doFilter_shouldForwardTheRequestForDelegation() throws Exception {
final String resource = "InventoryItem";
final String queryString = "key1=val1&key2=val2";
final String uri = REQ_ROOT_PATH + resource;
final String uri = REQ_ROOT_PATH + Constants.RES_CHARGE_ITEM;
final String privName1 = "priv-1";
final String privName2 = "priv-2";
Mockito.when(mockConfig.isExternalApiEnabled()).thenReturn(true);
Mockito.when(mockRequest.getRequestURI()).thenReturn(uri);
Mockito.when(mockRequest.getQueryString()).thenReturn(queryString);
Mockito.when(mockRequest.getRequestDispatcher(PATH_DELEGATE)).thenReturn(mockDispatcher);
Mockito.when(mockConfig.getChargeItemPrivileges()).thenReturn(Arrays.asList(privName1, privName2));

filter.doFilter(mockRequest, mockResponse, mockChain);

Mockito.verify(mockRequest).setAttribute(ATTRIB_RESOURCE_NAME, resource);
Mockito.verify(mockRequest).setAttribute(ATTRIB_RESOURCE_NAME, Constants.RES_CHARGE_ITEM);
Mockito.verify(mockRequest).setAttribute(ATTRIB_QUERY_STR, queryString);
Mockito.verify(mockDispatcher).forward(mockRequest, mockResponse);
PowerMockito.verifyStatic(Context.class);
Context.requirePrivilege(privName1);
Context.requirePrivilege(privName2);
}

@Test
public void doFilter_shouldForwardTheRequestWithForDelegation() throws Exception {
public void doFilter_shouldForwardTheRequestWithIdForDelegation() throws Exception {
final String resource = "InventoryItem";
final String resId = "abcde";
final String uri = REQ_ROOT_PATH + resource + "/" + resId;
Expand All @@ -94,4 +104,12 @@ public void doFilter_shouldSkipDelegationIfExternalApiIsNotEnabled() throws Exce
Mockito.verifyNoInteractions(mockDispatcher);
}

@Test
public void doFilter_shouldSkipDelegationIfContextIsNotAuthenticated() throws Exception {
when(Context.isAuthenticated()).thenReturn(false);
Mockito.when(mockConfig.isExternalApiEnabled()).thenReturn(true);
filter.doFilter(mockRequest, mockResponse, mockChain);
Mockito.verifyNoInteractions(mockDispatcher);
}

}
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
<artifactId>fhirproxy</artifactId>
<version>1.0.0-SNAPSHOT</version>
<packaging>pom</packaging>
<name>FHIR proxy Module</name>
<name>FHIR Proxy Module</name>
<description>
Intercepts fetch requests for `ChargeItemDefinition` and `InventoryItem` FHIR resources and redirects them to a
configured external API.
Expand Down

0 comments on commit 81fe373

Please sign in to comment.