From bb9a48ffed4644ce5ff5b4c49ee4ff8ad3811f78 Mon Sep 17 00:00:00 2001 From: Karthik Date: Mon, 7 Aug 2023 14:21:27 -0400 Subject: [PATCH 1/3] Add data access token user role filter feature --- .../cbio/portal/util/GlobalProperties.java | 18 +++++++++++++ .../Authenticating-Users-via-Tokens.md | 6 +++++ portal/src/main/webapp/config_service.jsp | 3 ++- src/main/resources/portal.properties.EXAMPLE | 1 + .../web/DataAccessTokenController.java | 11 +++++++- .../web/OAuth2DataAccessTokenController.java | 19 +++++++++++++ .../web/DataAccessTokenControllerTest.java | 27 +++++++++++++++++++ 7 files changed, 83 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/mskcc/cbio/portal/util/GlobalProperties.java b/core/src/main/java/org/mskcc/cbio/portal/util/GlobalProperties.java index ba311e72ad5..5ad1ddb54e8 100644 --- a/core/src/main/java/org/mskcc/cbio/portal/util/GlobalProperties.java +++ b/core/src/main/java/org/mskcc/cbio/portal/util/GlobalProperties.java @@ -368,6 +368,12 @@ public static String parseUrl(String url) @Value("${download_group:}") // default is empty string public void setDownloadGroup(String property) { downloadGroup = property; } + public static final String DEFAULT_DAT_METHOD = "none"; + + private static String tokenAccessUserRole; + @Value("${dat.filter_user_role:}") // default is empty string + public void setTokenAccessUserRole(String property) { tokenAccessUserRole = property; } + private static Logger LOG = LoggerFactory.getLogger(GlobalProperties.class); private static ConfigPropertyResolver portalProperties = new ConfigPropertyResolver(); private static Properties mavenProperties = initializeProperties(MAVEN_PROPERTIES_FILE_NAME); @@ -1296,4 +1302,16 @@ public static String getDownloadControl() { } } } + + public static String getDataAccessTokenMethod() { + Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); + String dataAccessTokenMethod = getProperty("dat.method"); + + if (authentication != null && + StringUtils.isNotEmpty(tokenAccessUserRole)) { + return authentication.getAuthorities().contains(new SimpleGrantedAuthority(tokenAccessUserRole)) ? dataAccessTokenMethod : DEFAULT_DAT_METHOD; + } else { + return dataAccessTokenMethod; + } + } } diff --git a/docs/deployment/authorization-and-authentication/Authenticating-Users-via-Tokens.md b/docs/deployment/authorization-and-authentication/Authenticating-Users-via-Tokens.md index 9dde6968846..8925135b625 100644 --- a/docs/deployment/authorization-and-authentication/Authenticating-Users-via-Tokens.md +++ b/docs/deployment/authorization-and-authentication/Authenticating-Users-via-Tokens.md @@ -45,6 +45,12 @@ The following properties must be present in portal.properties in order to allow * **Permissible Values**: jwt, uuid, oauth2, none * **Default Value**: none +**Property**: dat.filter\_user\_role (optional) + +* **Description**: This property determines users access in token generation. If present, this role will be checked in user roles before generating a token. +* **Permissible Values**: A string value. +* **Default Value**: none + **Property**: dat.unauth\_users (optional, not used for dat.method = oauth2) * **Description**: A list of users that should not be allowed to download a data access token. diff --git a/portal/src/main/webapp/config_service.jsp b/portal/src/main/webapp/config_service.jsp index ab5bbf48c3e..a34b50522bc 100644 --- a/portal/src/main/webapp/config_service.jsp +++ b/portal/src/main/webapp/config_service.jsp @@ -31,7 +31,6 @@ String[] propNameArray = { "app.version", "app.name", - "dat.method", "oncoprint.custom_driver_annotation.binary.menu_label", "disabled_tabs", "civic.url", @@ -204,6 +203,8 @@ obj.put("sessionServiceEnabled", !StringUtils.isEmpty(GlobalProperties.getSessionServiceUrl())); obj.put("skin_hide_download_controls", GlobalProperties.getDownloadControl()); + + obj.put("dat_method", GlobalProperties.getDataAccessTokenMethod()); out.println(obj.toJSONString()); diff --git a/src/main/resources/portal.properties.EXAMPLE b/src/main/resources/portal.properties.EXAMPLE index efae1b8c794..78b02eb08ac 100644 --- a/src/main/resources/portal.properties.EXAMPLE +++ b/src/main/resources/portal.properties.EXAMPLE @@ -171,6 +171,7 @@ dat.method=none dat.ttl_seconds=2592000 dat.uuid.max_number_per_user=1 dat.jwt.secret_key= +dat.filter_user_role= # OAuth2 token data access settings #dat.oauth2.clientId= diff --git a/web/src/main/java/org/cbioportal/web/DataAccessTokenController.java b/web/src/main/java/org/cbioportal/web/DataAccessTokenController.java index e8938209bdb..06ddc00f27e 100644 --- a/web/src/main/java/org/cbioportal/web/DataAccessTokenController.java +++ b/web/src/main/java/org/cbioportal/web/DataAccessTokenController.java @@ -20,7 +20,7 @@ import io.swagger.annotations.Api; import io.swagger.annotations.ApiOperation; import io.swagger.annotations.ApiParam; -import io.swagger.models.HttpMethod; +import org.apache.commons.lang3.StringUtils; import org.cbioportal.model.DataAccessToken; import org.cbioportal.service.DataAccessTokenService; import org.cbioportal.service.exception.DataAccessTokenNoUserIdentityException; @@ -32,6 +32,7 @@ import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.security.core.Authentication; +import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.validation.annotation.Validated; import org.springframework.web.bind.annotation.*; import org.springframework.web.client.HttpClientErrorException; @@ -54,6 +55,10 @@ public class DataAccessTokenController { @Value("${dat.unauth_users:anonymousUser}") private String[] USERS_WHO_CANNOT_USE_TOKENS; + private static String userRoleToAccessToken; + @Value("${download_group:}") // default is empty string + public void setUserRoleToAccessToken(String property) { userRoleToAccessToken = property; } + @Autowired private DataAccessTokenService tokenService; private Set usersWhoCannotUseTokenSet; @@ -130,6 +135,10 @@ private String getAuthenticatedUser(Authentication authentication) { if (usersWhoCannotUseTokenSet.contains(username)) { throw new DataAccessTokenProhibitedUserException(); } + if(StringUtils.isNotEmpty(userRoleToAccessToken) && + !authentication.getAuthorities().contains(new SimpleGrantedAuthority(userRoleToAccessToken))) { + throw new DataAccessTokenProhibitedUserException(); + } return username; } } diff --git a/web/src/main/java/org/cbioportal/web/OAuth2DataAccessTokenController.java b/web/src/main/java/org/cbioportal/web/OAuth2DataAccessTokenController.java index 528f59ae5a6..1b98a12abbc 100644 --- a/web/src/main/java/org/cbioportal/web/OAuth2DataAccessTokenController.java +++ b/web/src/main/java/org/cbioportal/web/OAuth2DataAccessTokenController.java @@ -35,8 +35,10 @@ import io.swagger.annotations.Api; import io.swagger.annotations.ApiOperation; import io.swagger.annotations.ApiParam; +import org.apache.commons.lang3.StringUtils; import org.cbioportal.model.DataAccessToken; import org.cbioportal.service.DataAccessTokenService; +import org.cbioportal.service.exception.DataAccessTokenNoUserIdentityException; import org.cbioportal.service.exception.DataAccessTokenProhibitedUserException; import org.cbioportal.web.config.annotation.InternalApi; import org.springframework.beans.factory.annotation.Autowired; @@ -46,6 +48,7 @@ import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.security.core.Authentication; +import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.validation.annotation.Validated; import org.springframework.web.bind.annotation.*; import org.springframework.web.client.HttpClientErrorException; @@ -76,6 +79,9 @@ public class OAuth2DataAccessTokenController { @Value("${dat.oauth2.clientId}") private String clientId; + @Value("${dat.filter_user_role:}") // default is empty string + private String userRoleToAccessToken; + @Autowired private DataAccessTokenService tokenService; private String authorizationUrl; @@ -98,6 +104,8 @@ public void postConstruct() throws UnsupportedEncodingException { public ResponseEntity downloadDataAccessToken(Authentication authentication, HttpServletRequest request, HttpServletResponse response) throws IOException { + isUserAuthorizedToAccess(authentication); + // redirect to authentication endpoint HttpHeaders headers = new HttpHeaders(); headers.add("Location", authorizationUrl); @@ -156,4 +164,15 @@ public void revokeDataAccessToken(@ApiParam(required = true, value = "token") @P throw new UnsupportedOperationException("this endpoint is does not apply to OAuth2 data access token method."); } + private void isUserAuthorizedToAccess(Authentication authentication) { + if (authentication == null || !authentication.isAuthenticated()) { + throw new DataAccessTokenNoUserIdentityException(); + } + + if(StringUtils.isNotEmpty(userRoleToAccessToken) && + !authentication.getAuthorities().contains(new SimpleGrantedAuthority(userRoleToAccessToken))) { + throw new DataAccessTokenProhibitedUserException(); + } + } + } diff --git a/web/src/test/java/org/cbioportal/web/DataAccessTokenControllerTest.java b/web/src/test/java/org/cbioportal/web/DataAccessTokenControllerTest.java index 30a5563a2b1..03aaa17d1fc 100644 --- a/web/src/test/java/org/cbioportal/web/DataAccessTokenControllerTest.java +++ b/web/src/test/java/org/cbioportal/web/DataAccessTokenControllerTest.java @@ -37,6 +37,7 @@ import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; import org.springframework.test.context.web.WebAppConfiguration; +import org.springframework.test.util.ReflectionTestUtils; import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.MvcResult; import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; @@ -190,6 +191,32 @@ public void createTokenValidUserTest() throws Exception { .andReturn(); } + @Test + public void createTokenValidUserTestWithUserRole() throws Exception { + ReflectionTestUtils.setField(DataAccessTokenController.class, "userRoleToAccessToken", "PLACEHOLDER_ROLE"); + Mockito.when(tokenService.createDataAccessToken(ArgumentMatchers.anyString())).thenReturn(MOCK_TOKEN_INFO); + HttpSession session = getSession(MOCK_USER, MOCK_PASSWORD); + MvcResult result = mockMvc.perform(MockMvcRequestBuilders.post("/data-access-tokens") + .session((MockHttpSession) session) + .accept(MediaType.APPLICATION_JSON) + .contentType(MediaType.APPLICATION_JSON)) + .andExpect(MockMvcResultMatchers.status().isCreated()) + .andReturn(); + } + + @Test + public void createTokenUnauthorizedUserTestWithUserRole() throws Exception { + ReflectionTestUtils.setField(DataAccessTokenController.class, "userRoleToAccessToken", "TEST"); + Mockito.when(tokenService.createDataAccessToken(ArgumentMatchers.anyString())).thenReturn(MOCK_TOKEN_INFO); + HttpSession session = getSession(MOCK_USER, MOCK_PASSWORD); + MvcResult result = mockMvc.perform(MockMvcRequestBuilders.post("/data-access-tokens") + .session((MockHttpSession) session) + .accept(MediaType.APPLICATION_JSON) + .contentType(MediaType.APPLICATION_JSON)) + .andExpect(MockMvcResultMatchers.status().isUnauthorized()) + .andReturn(); + } + /* Tests mapping for DELETE /data-access-tokens * Checks response status code is 200 success * Checks that correct username argument is passed to service class From 5f8caf7e118286e9260a1d8a8739703c6e24ed40 Mon Sep 17 00:00:00 2001 From: Karthik Date: Fri, 11 Aug 2023 12:42:33 -0400 Subject: [PATCH 2/3] Update and more tests --- .../org/mskcc/cbio/portal/util/GlobalProperties.java | 5 ++++- .../cbioportal/web/DataAccessTokenControllerTest.java | 11 +++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/mskcc/cbio/portal/util/GlobalProperties.java b/core/src/main/java/org/mskcc/cbio/portal/util/GlobalProperties.java index 5ad1ddb54e8..f6905da3aec 100644 --- a/core/src/main/java/org/mskcc/cbio/portal/util/GlobalProperties.java +++ b/core/src/main/java/org/mskcc/cbio/portal/util/GlobalProperties.java @@ -369,6 +369,10 @@ public static String parseUrl(String url) public void setDownloadGroup(String property) { downloadGroup = property; } public static final String DEFAULT_DAT_METHOD = "none"; + + private static String dataAccessTokenMethod; + @Value("${dat.method:none}") // default is empty string + public void setDataAccessTokenMethod(String property) { dataAccessTokenMethod = property; } private static String tokenAccessUserRole; @Value("${dat.filter_user_role:}") // default is empty string @@ -1305,7 +1309,6 @@ public static String getDownloadControl() { public static String getDataAccessTokenMethod() { Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); - String dataAccessTokenMethod = getProperty("dat.method"); if (authentication != null && StringUtils.isNotEmpty(tokenAccessUserRole)) { diff --git a/web/src/test/java/org/cbioportal/web/DataAccessTokenControllerTest.java b/web/src/test/java/org/cbioportal/web/DataAccessTokenControllerTest.java index 03aaa17d1fc..e6cc09e0f37 100644 --- a/web/src/test/java/org/cbioportal/web/DataAccessTokenControllerTest.java +++ b/web/src/test/java/org/cbioportal/web/DataAccessTokenControllerTest.java @@ -268,4 +268,15 @@ public Void answer(InvocationOnMock getAllTokensInvocation) { Assert.fail("Unexpected argument passed to service class. Expected argument: " + MOCK_USER + " Received argument: " + receivedArgument); } } + + @Test + public void createTokenNotLoggedIn() throws Exception { + ReflectionTestUtils.setField(DataAccessTokenController.class, "userRoleToAccessToken", "TEST"); + Mockito.when(tokenService.createDataAccessToken(ArgumentMatchers.anyString())).thenReturn(MOCK_TOKEN_INFO); + MvcResult result = mockMvc.perform(MockMvcRequestBuilders.post("/data-access-tokens") + .accept(MediaType.APPLICATION_JSON) + .contentType(MediaType.APPLICATION_JSON)) + .andExpect(MockMvcResultMatchers.status().isUnauthorized()) + .andReturn(); + } } From bbf94cbc406ef56ff0abc43794a2ee12f861f81a Mon Sep 17 00:00:00 2001 From: Karthik Date: Fri, 11 Aug 2023 13:24:46 -0400 Subject: [PATCH 3/3] Update failing test --- .../java/org/cbioportal/web/DataAccessTokenControllerTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/web/src/test/java/org/cbioportal/web/DataAccessTokenControllerTest.java b/web/src/test/java/org/cbioportal/web/DataAccessTokenControllerTest.java index e6cc09e0f37..b7155e59921 100644 --- a/web/src/test/java/org/cbioportal/web/DataAccessTokenControllerTest.java +++ b/web/src/test/java/org/cbioportal/web/DataAccessTokenControllerTest.java @@ -271,7 +271,6 @@ public Void answer(InvocationOnMock getAllTokensInvocation) { @Test public void createTokenNotLoggedIn() throws Exception { - ReflectionTestUtils.setField(DataAccessTokenController.class, "userRoleToAccessToken", "TEST"); Mockito.when(tokenService.createDataAccessToken(ArgumentMatchers.anyString())).thenReturn(MOCK_TOKEN_INFO); MvcResult result = mockMvc.perform(MockMvcRequestBuilders.post("/data-access-tokens") .accept(MediaType.APPLICATION_JSON)