diff --git a/README.md b/README.md index cb8853d..ed2e20f 100644 --- a/README.md +++ b/README.md @@ -75,6 +75,7 @@ The meaning of chosen environment variables: `SPRING_PROFILES_ACTIVE` | (default) `noauth` - authN/Z switched off. `auth` - authN/Z switched on. `TESK_API_AUTHORISATION_*` | A set of env variables configuring authorisation using Elixir group membership `TESK_API_SWAGGER_OAUTH_*` | A set of env variables configuring OAuth2/OIDC client built in Swagger UI + `TESK_API_AUTHORISATION_IGNORE_GROUP_MEMBERSHIP` | If `true` a user can see all the tasks he created, irrespective of group memberships ### Generating new API version stub diff --git a/src/integration-test/java/uk/ac/ebi/tsc/tesk/AuthIT.java b/src/integration-test/java/uk/ac/ebi/tsc/tesk/AuthIT.java index e9c7b13..6a8b88b 100644 --- a/src/integration-test/java/uk/ac/ebi/tsc/tesk/AuthIT.java +++ b/src/integration-test/java/uk/ac/ebi/tsc/tesk/AuthIT.java @@ -38,7 +38,7 @@ @AutoConfigureMockMvc @TestPropertySource(locations = {"classpath:application.properties"}, properties = {"security.oauth2.resource.user-info-uri = http://localhost:8090", - "spring.profiles.active=auth"}) + "spring.profiles.active=auth","tesk.api.authorisation.ignoreGroupMembership=false"}) public class AuthIT { @Autowired diff --git a/src/integration-test/java/uk/ac/ebi/tsc/tesk/AuthIgnoreGroupMembershipIT.java b/src/integration-test/java/uk/ac/ebi/tsc/tesk/AuthIgnoreGroupMembershipIT.java new file mode 100644 index 0000000..bc8b2a8 --- /dev/null +++ b/src/integration-test/java/uk/ac/ebi/tsc/tesk/AuthIgnoreGroupMembershipIT.java @@ -0,0 +1,261 @@ +package uk.ac.ebi.tsc.tesk; + +import com.github.tomakehurst.wiremock.client.WireMock; +import com.github.tomakehurst.wiremock.junit.WireMockRule; +import io.kubernetes.client.ApiClient; +import io.kubernetes.client.util.Config; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.context.TestConfiguration; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Primary; +import org.springframework.http.MediaType; +import org.springframework.test.context.TestPropertySource; +import org.springframework.test.context.junit4.SpringRunner; +import org.springframework.test.web.servlet.MockMvc; + +import static com.github.tomakehurst.wiremock.client.WireMock.*; +import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; +import static uk.ac.ebi.tsc.tesk.TestUtils.getFileContentFromResources; + +/** + * @author Ania Niewielska + *

+ * Integration testing of security (authentication and authorisation using OIDC and Elixir groups) + * Kubernetes API and OIDC userInfo endpoint are WireMocked + */ +@RunWith(SpringRunner.class) +@SpringBootTest +@AutoConfigureMockMvc +@TestPropertySource(locations = {"classpath:application.properties"}, + properties = {"security.oauth2.resource.user-info-uri = http://localhost:8090", + "spring.profiles.active=auth", "tesk.api.authorisation.ignoreGroupMembership=true"}) +public class AuthIgnoreGroupMembershipIT { + + @Rule + public WireMockRule mockElixir = new WireMockRule(8090); + + @Rule + public WireMockRule mockKubernetes = new WireMockRule(wireMockConfig().port(9000).usingFilesUnderDirectory("src/integration-test/resources")); + + @Autowired + private MockMvc mvc; + + @TestConfiguration + static class KubernetesClientMock { + @Bean + @Primary + public ApiClient kubernetesApiClient() { + return Config.fromUrl("http://localhost:9000", false); + } + } + + /** + * Any authenticated user should be able to create a task + * + * @throws Exception + */ + @Test + public void author_not_in_group_createTask() throws Exception { + + mockElixir.givenThat( + WireMock.get("/") + .willReturn(okJson("{\"sub\" : \"123\" }"))); + mockKubernetes.givenThat( + WireMock.post("/apis/batch/v1/namespaces/default/jobs") + .willReturn(okJson("{\"metadata\":{\"name\":\"task-fe99716a\"}}"))); + + String path = "fromTesToK8s_minimal/task.json"; + this.mvc.perform(post("/v1/tasks") + .content(getFileContentFromResources(path)) + .header("Authorization", "Bearer BAR") + .contentType(MediaType.APPLICATION_JSON) + .accept(MediaType.APPLICATION_JSON)) + .andExpect(status().isOk()); + } + + @Test + public void author_in_a_group_createTask() throws Exception { + + mockElixir.givenThat( + WireMock.get("/") + .willReturn(okJson("{\"sub\" : \"123\", " + + " \"eduperson_entitlement\" : [\"urn:geant:elixir-europe.org:group:elixir:GA4GH:GA4GH-CAP:EBI#perun.elixir-czech.cz\", \"urn:geant:elixir-europe.org:group:elixir:GA4GH:GA4GH-CAP:EBI:TEST#perun.elixir-czech.cz\"]}"))); + + mockKubernetes.givenThat( + WireMock.post("/apis/batch/v1/namespaces/default/jobs") + .withRequestBody(matchingJsonPath("$.metadata.labels[?(@.creator-group-name == 'TEST')]")) + .willReturn(okJson("{\"metadata\":{\"name\":\"task-fe99716a\"}}"))); + + String path = "fromTesToK8s_minimal/task.json"; + this.mvc.perform(post("/v1/tasks") + .content(getFileContentFromResources(path)) + .header("Authorization", "Bearer BAR") + .contentType(MediaType.APPLICATION_JSON) + .accept(MediaType.APPLICATION_JSON)) + .andExpect(status().isOk()); + } + + + @Test + public void author_in_a_group_getTask() throws Exception { + + mockElixir.givenThat( + WireMock.get("/") + .willReturn(okJson("{\"sub\" : \"123\", \"eduperson_entitlement\" : [\"urn:geant:elixir-europe.org:group:elixir:GA4GH:GA4GH-CAP:EBI#perun.elixir-czech.cz\", \"urn:geant:elixir-europe.org:group:elixir:GA4GH:GA4GH-CAP:EBI:TEST#perun.elixir-czech.cz\"]}"))); + + MockUtil.mockGetTaskKubernetesResponses(this.mockKubernetes); + + + this.mvc.perform(get("/v1/tasks/{id}", "task-123") + .header("Authorization", "Bearer BAR")) + .andExpect(status().isOk()); + this.mvc.perform(get("/v1/tasks/{id}?view=BASIC", "task-123") + .header("Authorization", "Bearer BAR")) + .andExpect(status().isOk()); + this.mvc.perform(get("/v1/tasks/{id}?view=FULL", "task-123") + .header("Authorization", "Bearer BAR")) + .andExpect(status().isOk()); + } + + @Test + public void author_not_in_a_group_getTask() throws Exception { + + mockElixir.givenThat( + WireMock.get("/") + .willReturn(okJson("{\"sub\" : \"123\"}"))); + + MockUtil.mockGetTaskKubernetesResponses(this.mockKubernetes); + + + this.mvc.perform(get("/v1/tasks/{id}", "task-123") + .header("Authorization", "Bearer BAR")) + .andExpect(status().isOk()); + + this.mvc.perform(get("/v1/tasks/{id}?view=BASIC", "task-123") + .header("Authorization", "Bearer BAR")) + .andExpect(status().isOk()); + this.mvc.perform(get("/v1/tasks/{id}?view=FULL", "task-123") + .header("Authorization", "Bearer BAR")) + .andExpect(status().isOk()); + } + + @Test + public void nonAuthor_getTask() throws Exception { + + mockElixir.givenThat( + WireMock.get("/") + .willReturn(okJson("{\"sub\" : \"124\", \"eduperson_entitlement\" : [\"urn:geant:elixir-europe.org:group:elixir:GA4GH:GA4GH-CAP:EBI#perun.elixir-czech.cz\", \"urn:geant:elixir-europe.org:group:elixir:GA4GH:GA4GH-CAP:EBI:TEST#perun.elixir-czech.cz\"]}"))); + + MockUtil.mockGetTaskKubernetesResponses(this.mockKubernetes); + + this.mvc.perform(get("/v1/tasks/{id}", "task-123") + .header("Authorization", "Bearer BAR")) + .andExpect(status().isForbidden()); + this.mvc.perform(get("/v1/tasks/{id}?view=BASIC", "task-123") + .header("Authorization", "Bearer BAR")) + .andExpect(status().isForbidden()); + this.mvc.perform(get("/v1/tasks/{id}?view=FULL", "task-123") + .header("Authorization", "Bearer BAR")) + .andExpect(status().isForbidden()); + } + + @Test + public void author_not_in_group_getList() throws Exception { + + mockElixir.givenThat( + WireMock.get("/") + .willReturn(okJson("{\"sub\" : \"123\"}"))); + + mockKubernetes.givenThat( + WireMock.get("/apis/batch/v1/namespaces/default/jobs?labelSelector=job-type%3Dtaskmaster" + + "%2Ccreator-user-id%3D123") + .willReturn(aResponse().withBodyFile("list/taskmasters_nogroup.json"))); + + MockUtil.mockListTaskKubernetesResponses(this.mockKubernetes); + + perFormListTask(5); + + } + + + + @Test + public void author_in_group_getList() throws Exception { + + mockElixir.givenThat( + WireMock.get("/") + .willReturn(okJson("{\"sub\" : \"123\", \"eduperson_entitlement\" : [\"urn:geant:elixir-europe.org:group:elixir:GA4GH:GA4GH-CAP:EBI#perun.elixir-czech.cz\",\"urn:geant:elixir-europe.org:group:elixir:GA4GH:GA4GH-CAP:EBI:TEST#perun.elixir-czech.cz\"]}"))); + + mockKubernetes.givenThat( + WireMock.get("/apis/batch/v1/namespaces/default/jobs?labelSelector=job-type%3Dtaskmaster%2Ccreator-user-id%3D123") + .willReturn(aResponse().withBodyFile("list/taskmasters_nogroup.json"))); + MockUtil.mockListTaskKubernetesResponses(this.mockKubernetes); + + this.mvc.perform(get("/v1/tasks") + .header("Authorization", "Bearer BAR")) + .andExpect(status().isOk()); + + verify(exactly(0), getRequestedFor(urlEqualTo("/apis/batch/v1/namespaces/default/jobs?labelSelector=job-type%3Dtaskmaster" + + "%2Ccreator-group-name%20in%20%28TEST%29%2Ccreator-user-id%3D123"))); + + perFormListTask(5); + + } + + @Test + public void unauthenicated_createTask() throws Exception { + + String path = "fromTesToK8s_minimal/task.json"; + this.mvc.perform(post("/v1/tasks") + .content(getFileContentFromResources(path)) + .contentType(MediaType.APPLICATION_JSON) + .accept(MediaType.APPLICATION_JSON)).andExpect(status().isUnauthorized()); + } + + @Test + public void unauthenticated_getTask() throws Exception { + this.mvc.perform(get("/v1/tasks/{id}", 123)) + .andExpect(status().isUnauthorized()); + } + + @Test + public void unauthenticated_getList() throws Exception { + + this.mvc.perform(get("/v1/tasks") + .header("Authorization", "different BAR")) + .andExpect(status().isUnauthorized()); + this.mvc.perform(get("/v1/tasks?view=BASIC") + .header("Different", "Bearer BAR")) + .andExpect(status().isUnauthorized()); + this.mvc.perform(get("/v1/tasks?view=FULL")) + .andExpect(status().isUnauthorized()); + + } + + private void perFormListTask(int expectedTasksLength) throws Exception { + + this.mvc.perform(get("/v1/tasks") + .header("Authorization", "Bearer BAR")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.tasks.length()").value(expectedTasksLength)); + this.mvc.perform(get("/v1/tasks?view=BASIC") + .header("Authorization", "Bearer BAR")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.tasks.length()").value(expectedTasksLength)); + this.mvc.perform(get("/v1/tasks?view=FULL") + .header("Authorization", "Bearer BAR")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.tasks.length()").value(expectedTasksLength)); + } + + +} diff --git a/src/integration-test/java/uk/ac/ebi/tsc/tesk/NoAuthIT.java b/src/integration-test/java/uk/ac/ebi/tsc/tesk/NoAuthIT.java index eafc63a..1119f6d 100644 --- a/src/integration-test/java/uk/ac/ebi/tsc/tesk/NoAuthIT.java +++ b/src/integration-test/java/uk/ac/ebi/tsc/tesk/NoAuthIT.java @@ -37,7 +37,7 @@ @AutoConfigureMockMvc @TestPropertySource(locations = {"classpath:application.properties"}, properties = {"security.oauth2.resource.user-info-uri = http://localhost:8090", - "spring.profiles.active=noauth"}) + "spring.profiles.active=noauth","tesk.api.authorisation.ignoreGroupMembership=false"}) public class NoAuthIT { @Autowired diff --git a/src/integration-test/resources/__files/list/taskmasters_nogroup.json b/src/integration-test/resources/__files/list/taskmasters_nogroup.json new file mode 100644 index 0000000..20e56cc --- /dev/null +++ b/src/integration-test/resources/__files/list/taskmasters_nogroup.json @@ -0,0 +1,103 @@ +{ + "items": [ + { + "metadata": { + "labels": { + "creator-group-name": "TEST", + "creator-user-id": "123", + "job-type": "taskmaster" + }, + "name": "task-123" + }, + "spec": { + "selector": { + "matchLabels": { + "controller-uid": "24a0504a-4a2b-11e8-a06f-fa163ecf0042" + } + } + }, + "status": { + "succeeded": 1 + } + }, + { + "metadata": { + "labels": { + "creator-user-id": "123", + "job-type": "taskmaster" + }, + "name": "task-124" + }, + "spec": { + "selector": { + "matchLabels": { + "controller-uid": "24a0504a-4a2b-11e8-a06f-fa163ecf0043" + } + } + }, + "status": { + "succeeded": 1 + } + }, + { + "metadata": { + "labels": { + "creator-group-name": "ABC", + "creator-user-id": "123", + "job-type": "taskmaster" + }, + "name": "task-125" + }, + "spec": { + "selector": { + "matchLabels": { + "controller-uid": "24a0504a-4a2b-11e8-a06f-fa163ecf0044" + } + } + }, + "status": { + "succeeded": 1 + } + }, + { + "metadata": { + "labels": { + "creator-group-name": "ABC", + "creator-user-id": "124", + "job-type": "taskmaster" + }, + "name": "task-126" + }, + "spec": { + "selector": { + "matchLabels": { + "controller-uid": "24a0504a-4a2b-11e8-a06f-fa163ecf0045" + } + } + }, + "status": { + "succeeded": 1 + } + }, + { + "metadata": { + "labels": { + "creator-user-id": "123", + "job-type": "taskmaster" + }, + "name": "task-127" + }, + "spec": { + "selector": { + "matchLabels": { + "controller-uid": "24a0504a-4a2b-11e8-a06f-fa163ecf0045" + } + } + }, + "status": { + "succeeded": 1 + } + } + ], + "metadata": {} +} diff --git a/src/main/java/uk/ac/ebi/tsc/tesk/config/security/AuthorisationProperties.java b/src/main/java/uk/ac/ebi/tsc/tesk/config/security/AuthorisationProperties.java index f98a161..83d1ce2 100644 --- a/src/main/java/uk/ac/ebi/tsc/tesk/config/security/AuthorisationProperties.java +++ b/src/main/java/uk/ac/ebi/tsc/tesk/config/security/AuthorisationProperties.java @@ -43,6 +43,11 @@ public class AuthorisationProperties { */ private String adminGroupFull; + /** + * Ignore user's group membership + */ + private boolean ignoreGroupMembership; + public String getGroupsClaim() { return groupsClaim; } @@ -98,4 +103,12 @@ public String getAdminGroupFull() { public void setAdminGroupFull(String adminGroupFull) { this.adminGroupFull = adminGroupFull; } + + public boolean isIgnoreGroupMembership() { + return ignoreGroupMembership; + } + + public void setIgnoreGroupMembership(boolean ignoreGroupMembership) { + this.ignoreGroupMembership = ignoreGroupMembership; + } } diff --git a/src/main/java/uk/ac/ebi/tsc/tesk/config/security/ElixirPrincipalExtractor.java b/src/main/java/uk/ac/ebi/tsc/tesk/config/security/ElixirPrincipalExtractor.java index 702476c..867a309 100644 --- a/src/main/java/uk/ac/ebi/tsc/tesk/config/security/ElixirPrincipalExtractor.java +++ b/src/main/java/uk/ac/ebi/tsc/tesk/config/security/ElixirPrincipalExtractor.java @@ -55,6 +55,8 @@ public Object extractPrincipal(Map map) { return matcher.group(1); }).collect(Collectors.toSet()); boolean isAdmin = allGroups.stream().anyMatch(name -> authorisationProperties.getAdminGroupFull().equals(name)); - return builder.allGroups(allGroups).teskMemberedGroups(memberedGroups).teskManagedGroups(managedGroups).teskAdmin(isAdmin).build(); + boolean isIgnoreGroupMembership = authorisationProperties.isIgnoreGroupMembership(); + return builder.allGroups(allGroups).teskMemberedGroups(memberedGroups).teskManagedGroups(managedGroups).teskAdmin(isAdmin). + ignoreGroupMembership(isIgnoreGroupMembership).build(); } } diff --git a/src/main/java/uk/ac/ebi/tsc/tesk/config/security/User.java b/src/main/java/uk/ac/ebi/tsc/tesk/config/security/User.java index 03f99f9..2044c98 100644 --- a/src/main/java/uk/ac/ebi/tsc/tesk/config/security/User.java +++ b/src/main/java/uk/ac/ebi/tsc/tesk/config/security/User.java @@ -50,6 +50,12 @@ public class User implements Serializable, UserDetails, Principal { */ private Set teskManagedGroups; + /** + * Ignore group membership, so that task creator can see his tasks, irrespective of group + * membership + */ + private boolean ignoreGroupMembership; + public User() { } @@ -93,30 +99,39 @@ public boolean isMemberInNonManagedGroups() { return false; } + public boolean isIgnoreGroupMembership() { + return ignoreGroupMembership; + } + public String getLabelSelector() { - Set allTeskGroups = new LinkedHashSet<>(); - if (this.teskMemberedGroups != null) { - allTeskGroups.addAll(this.teskMemberedGroups); - } - if (this.teskManagedGroups != null) { - allTeskGroups.addAll(this.teskManagedGroups); - } - if (isTeskAdmin()) { - return null; - } - if (isMember() || isManager()) { - StringBuilder sb = new StringBuilder(); - sb.append(Constants.LABEL_GROUPNAME_KEY).append(" in (").append(StringUtils.collectionToCommaDelimitedString(allTeskGroups)).append(")"); - if (!isManager()) { - sb.append(",").append(Constants.LABEL_USERID_KEY).append("=").append(getUsername()); + if (isIgnoreGroupMembership()) { + return new StringBuilder().append(Constants.LABEL_USERID_KEY).append("=").append(getUsername()).toString(); + } else { + Set allTeskGroups = new LinkedHashSet<>(); + if (this.teskMemberedGroups != null) { + allTeskGroups.addAll(this.teskMemberedGroups); } - return sb.toString(); + if (this.teskManagedGroups != null) { + allTeskGroups.addAll(this.teskManagedGroups); + } + if (isTeskAdmin()) { + return null; + } + if (isMember() || isManager()) { + StringBuilder sb = new StringBuilder(); + sb.append(Constants.LABEL_GROUPNAME_KEY).append(" in (").append(StringUtils.collectionToCommaDelimitedString(allTeskGroups)).append(")"); + if (!isManager()) { + sb.append(",").append(Constants.LABEL_USERID_KEY).append("=").append(getUsername()); + } + return sb.toString(); + } + return null; } - return null; } User(String userId, String preferredUsername, String name, String givenName, String familyName, String email, - Set allGroups, Set teskMemberedGroups, Set teskManagedGroups, boolean teskAdmin) { + Set allGroups, Set teskMemberedGroups, Set teskManagedGroups, boolean teskAdmin, + boolean ignoreGroupMembership) { this.userId = userId; this.preferredUsername = preferredUsername; this.name = name; @@ -127,6 +142,7 @@ public String getLabelSelector() { this.teskMemberedGroups = teskMemberedGroups; this.teskManagedGroups = teskManagedGroups; this.teskAdmin = teskAdmin; + this.ignoreGroupMembership = ignoreGroupMembership; } @Override @@ -181,6 +197,7 @@ public String toString() { ", memberedGroups=(" + StringUtils.collectionToCommaDelimitedString(teskMemberedGroups) + ")" + ", managedGroups=(" + StringUtils.collectionToCommaDelimitedString(teskManagedGroups) + ")" + ", iasAdmin=" + isTeskAdmin() + + ", isIgnoreGroupMembership=" + isIgnoreGroupMembership() + "}"; } @@ -202,6 +219,7 @@ public boolean equals(Object o) { if (allGroups != null ? !allGroups.equals(user.allGroups) : user.allGroups != null) return false; if (teskMemberedGroups != null ? !teskMemberedGroups.equals(user.teskMemberedGroups) : user.teskMemberedGroups != null) return false; + if (ignoreGroupMembership != user.ignoreGroupMembership) return false; return teskManagedGroups != null ? teskManagedGroups.equals(user.teskManagedGroups) : user.teskManagedGroups == null; } @@ -217,6 +235,7 @@ public int hashCode() { result = 31 * result + (teskAdmin ? 1 : 0); result = 31 * result + (teskMemberedGroups != null ? teskMemberedGroups.hashCode() : 0); result = 31 * result + (teskManagedGroups != null ? teskManagedGroups.hashCode() : 0); + result = 31 * result + (ignoreGroupMembership ? 1 : 0); return result; } @@ -231,6 +250,7 @@ public static class UserBuilder { private Set teskMemberedGroups; private Set teskManagedGroups; private boolean teskAdmin = false; + private boolean ignoreGroupMembership = false; UserBuilder(String userId) { this.userId = userId; @@ -286,9 +306,14 @@ public UserBuilder teskAdmin(boolean teskAdmin) { return this; } + public UserBuilder ignoreGroupMembership(boolean ignoreGroupMembership) { + this.ignoreGroupMembership = ignoreGroupMembership; + return this; + } + public User build() { return new User(userId, preferredUsername, name, givenName, familyName, email, - allGroups, teskMemberedGroups, teskManagedGroups, teskAdmin); + allGroups, teskMemberedGroups, teskManagedGroups, teskAdmin, ignoreGroupMembership); } } diff --git a/src/main/java/uk/ac/ebi/tsc/tesk/service/TesService.java b/src/main/java/uk/ac/ebi/tsc/tesk/service/TesService.java index 6d857ed..b91014a 100644 --- a/src/main/java/uk/ac/ebi/tsc/tesk/service/TesService.java +++ b/src/main/java/uk/ac/ebi/tsc/tesk/service/TesService.java @@ -21,9 +21,9 @@ public interface TesService { * Creates new TES task, by converting input and calling create method. * In case of detecting duplicated task ID, retries with new generated ID (up to a limit of retries) */ - @PreAuthorize("hasRole(@authorisationProperties.baseGroupFull) AND" + + @PreAuthorize("(authentication.authenticated AND @authorisationProperties.isIgnoreGroupMembership()) OR (hasRole(@authorisationProperties.baseGroupFull) AND" + "(#user.member OR #user.teskAdmin) AND" + - "(#task.tags?.get('GROUP_NAME') == null OR #user.isGroupMember(#task.tags['GROUP_NAME']))") + "(#task.tags?.get('GROUP_NAME') == null OR #user.isGroupMember(#task.tags['GROUP_NAME'])))") TesCreateTaskResponse createTask(TesTask task, User user); /** @@ -34,7 +34,7 @@ public interface TesService { * @param view - one of {@link TaskView} values, decides on how much detail is put in results * @return - TES task details */ - @PostAuthorize("(#user.username == returnObject.logs[0].metadata['USER_ID'] AND returnObject.logs[0].metadata['GROUP_NAME'] != null AND #user.isGroupMember(returnObject.logs[0].metadata['GROUP_NAME']))" + + @PostAuthorize("(authentication.authenticated AND @authorisationProperties.isIgnoreGroupMembership() AND #user.username == returnObject.logs[0].metadata['USER_ID']) OR (#user.username == returnObject.logs[0].metadata['USER_ID'] AND returnObject.logs[0].metadata['GROUP_NAME'] != null AND #user.isGroupMember(returnObject.logs[0].metadata['GROUP_NAME']))" + " OR (#user.teskAdmin)" + " OR (returnObject.logs[0].metadata['GROUP_NAME'] != null AND #user.isGroupManager(returnObject.logs[0].metadata['GROUP_NAME']))") TesTask getTask(String taskId, TaskView view, User user); @@ -49,7 +49,7 @@ public interface TesService { * @param view - one of {@link TaskView} values, decides on how much detail is put in each resulting task * @return - resulting list of tasks plus paging token (when supported) */ - @PreAuthorize("#user.teskAdmin OR #user.manager OR #user.member") + @PreAuthorize("#user.teskAdmin OR #user.manager OR #user.member OR (authentication.authenticated AND @authorisationProperties.isIgnoreGroupMembership())") TesListTasksResponse listTasks(String namePrefix, Long pageSize, String pageToken, diff --git a/src/main/java/uk/ac/ebi/tsc/tesk/util/component/KubernetesClientWrapper.java b/src/main/java/uk/ac/ebi/tsc/tesk/util/component/KubernetesClientWrapper.java index 19d5e17..2f57262 100644 --- a/src/main/java/uk/ac/ebi/tsc/tesk/util/component/KubernetesClientWrapper.java +++ b/src/main/java/uk/ac/ebi/tsc/tesk/util/component/KubernetesClientWrapper.java @@ -12,6 +12,7 @@ import org.springframework.beans.factory.annotation.Value; import org.springframework.http.HttpStatus; import org.springframework.stereotype.Component; +import uk.ac.ebi.tsc.tesk.config.security.AuthorisationProperties; import uk.ac.ebi.tsc.tesk.config.security.User; import uk.ac.ebi.tsc.tesk.exception.KubernetesException; import uk.ac.ebi.tsc.tesk.exception.TaskNotFoundException; @@ -41,14 +42,17 @@ public class KubernetesClientWrapper { private final String namespace; + private final AuthorisationProperties authorisationProperties; + public KubernetesClientWrapper(BatchV1Api batchApi, @Qualifier("patchBatchApi") BatchV1Api patchBatchApi, CoreV1Api coreApi, @Qualifier("patchCoreApi") CoreV1Api patchCoreApi, - @Value("${tesk.api.k8s.namespace}") String namespace) { + @Value("${tesk.api.k8s.namespace}") String namespace, AuthorisationProperties authorisationProperties) { this.batchApi = batchApi; this.patchBatchApi = patchBatchApi; this.coreApi = coreApi; this.patchCoreApi = patchCoreApi; this.namespace = namespace; + this.authorisationProperties = authorisationProperties; } public V1Job createJob(V1Job job) { @@ -89,11 +93,13 @@ private V1JobList listJobs(String _continue, String labelSelector, Integer limit public V1JobList listAllTaskmasterJobsForUser(String pageToken, Integer itemsPerPage, User user) { //Jobs of taskmaster type String labelSelector = new StringJoiner("=").add(LABEL_JOBTYPE_KEY).add(LABEL_JOBTYPE_VALUE_TASKM).toString(); + if (user.getLabelSelector() != null) { //additional label selectors; limiting results to jobs belonging to chosen groups (where the user is member and/or manager) // and optionally also to only those jobs, which were created bu the user labelSelector += "," + user.getLabelSelector(); } + V1JobList result = this.listJobs(pageToken, labelSelector, itemsPerPage); if (user.isMemberInNonManagedGroups()) { //if there are groups, where user is a manager and other groups, where user is only a member diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 0272d1f..c862000 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -22,17 +22,17 @@ tesk.api.service-info.documentation=https://github.com/EMBL-EBI-TSI/TESK tesk.api.k8s.namespace=default tesk.api.taskmaster.image-name=eu.gcr.io/tes-wes/taskmaster -tesk.api.taskmaster.image-version=v0.4 +tesk.api.taskmaster.image-version=v0.8.5 tesk.api.taskmaster.filer-image-name=eu.gcr.io/tes-wes/filer -tesk.api.taskmaster.filer-image-version=v0.4 +tesk.api.taskmaster.filer-image-version=v0.8.5 tesk.api.taskmaster.ftp.secret-name= -tesk.api.taskmaster.service-account-name=default +tesk.api.taskmaster.service-account-name=taskmaster tesk.api.taskmaster.debug=false tesk.api.taskmaster.executor-secret.name= tesk.api.taskmaster.executor-secret.mount-path=/secret -spring.profiles.active=noauth +spring.profiles.active=auth #group authorisation settings tesk.api.authorisation.groups-claim=eduperson_entitlement tesk.api.authorisation.delimiter=: @@ -48,6 +48,7 @@ tesk.api.authorisation.base-group-full=${tesk.api.authorisation.base-group-nosuf tesk.api.authorisation.admin-group-full=${tesk.api.authorisation.base-group-prefix}${tesk.api.authorisation.admin-subgroup}${tesk.api.authorisation.group-suffix} tesk.api.authorisation.admin-subgroup-suffix=${tesk.api.authorisation.delimiter}${tesk.api.authorisation.admin-subgroup} +tesk.api.authorisation.ignoreGroupMembership=false #server.port=8082 #swagger OAuth2 client settings (we will use custom params - defining OAuth2 standard params may trigger autoconfiguration of things we don't want @@ -56,4 +57,4 @@ tesk.api.swagger-oauth.authorization-endpoint=https://login.elixir-czech.org/oid tesk.api.swagger-oauth.token-endpoint=https://login.elixir-czech.org/oidc/token tesk.api.swagger-oauth.client-id=changeme tesk.api.swagger-oauth.client-secret=changeme -tesk.api.swagger-oauth.scopes=openid:Standard openid,${tesk.api.authorisation.groups-claim}:Access to groups membership,profile:Identity info about user,email:Email \ No newline at end of file +tesk.api.swagger-oauth.scopes=openid:Standard openid,${tesk.api.authorisation.groups-claim}:Access to groups membership,profile:Identity info about user,email:Email diff --git a/src/test/java/uk/ac/ebi/tsc/tesk/util/component/KubernetesClientWrapperTest.java b/src/test/java/uk/ac/ebi/tsc/tesk/util/component/KubernetesClientWrapperTest.java index d443fc4..628a6f2 100644 --- a/src/test/java/uk/ac/ebi/tsc/tesk/util/component/KubernetesClientWrapperTest.java +++ b/src/test/java/uk/ac/ebi/tsc/tesk/util/component/KubernetesClientWrapperTest.java @@ -16,6 +16,7 @@ import org.springframework.test.context.TestPropertySource; import org.springframework.test.context.junit4.SpringRunner; import org.springframework.util.StringUtils; +import uk.ac.ebi.tsc.tesk.config.security.AuthorisationProperties; import uk.ac.ebi.tsc.tesk.config.security.User; import uk.ac.ebi.tsc.tesk.exception.TaskNotFoundException; @@ -42,6 +43,8 @@ public class KubernetesClientWrapperTest { @MockBean private CoreV1Api coreApi; + @MockBean + private AuthorisationProperties authorisationProperties; private KubernetesClientWrapper wrapper; @@ -49,7 +52,7 @@ public class KubernetesClientWrapperTest { public void setUp() { this.wrapper = new KubernetesClientWrapper(batchApi, batchApi, - coreApi, coreApi, namespace); + coreApi, coreApi, namespace, authorisationProperties); } @Test @@ -224,4 +227,4 @@ private V1JobList filteredResultList() { new V1Job().metadata(new V1ObjectMeta().putLabelsItem(LABEL_GROUPNAME_KEY, "ABC").putLabelsItem(LABEL_USERID_KEY, "123")))); return jobList; } -} \ No newline at end of file +} diff --git a/src/test/resources/fromTesToK8s_minimal/job.json b/src/test/resources/fromTesToK8s_minimal/job.json index 46b4f2b..490b0d3 100644 --- a/src/test/resources/fromTesToK8s_minimal/job.json +++ b/src/test/resources/fromTesToK8s_minimal/job.json @@ -18,11 +18,11 @@ "name": "task-98605447" }, "spec": { - "serviceAccountName": "default", + "serviceAccountName": "taskmaster", "containers": [ { "args": [ - "$(JSON_INPUT)", "-n", "default", "-fn", "eu.gcr.io/tes-wes/filer", "-fv", "v0.4" + "$(JSON_INPUT)", "-n", "default", "-fn", "eu.gcr.io/tes-wes/filer", "-fv", "v0.8.5" ], "env": [ { @@ -30,7 +30,7 @@ "value": "{\"outputs\":[{\"url\":\"/path/to/output_file.txt\",\"path\":\"/tes/output.txt\",\"type\":\"FILE\"},{\"url\":\"/path/to/output\",\"path\":\"/outputs/output\",\"type\":\"DIRECTORY\"}],\"inputs\":[{\"name\":\"infile1\",\"description\":\"aa bbb\",\"url\":\"/path1/to/input_file.json\",\"path\":\"/tes/volumes/input.json\",\"type\":\"FILE\"},{\"url\":\"/path2/to/input\",\"path\":\"/tes/volumes/input\",\"type\":\"DIRECTORY\"},{\"path\":\"/container/input/other.txt\",\"type\":\"FILE\",\"content\":\"aaabbbcccddd\"}],\"volumes\":[\"/tmp/tmp1\",\"/tmp/tmp2\"],\"executors\":[{\"apiVersion\":\"batch/v1\",\"kind\":\"Job\",\"metadata\":{\"annotations\":{\"tes-task-name\":\"taskFull\"},\"labels\":{\"job-type\":\"executor\",\"taskmaster-name\":\"task-35605447\",\"executor-no\":\"0\"},\"name\":\"task-35605447-ex-00\"},\"spec\":{\"template\":{\"metadata\":{\"name\":\"task-35605447-ex-00\"},\"spec\":{\"containers\":[{\"args\":[\"echo\",\"hello world\"],\"command\":[],\"image\":\"alpine\",\"name\":\"task-35605447-ex-00\",\"resources\":{\"requests\":{\"cpu\":\"4\"}}}],\"restartPolicy\":\"Never\"}}}},{\"apiVersion\":\"batch/v1\",\"kind\":\"Job\",\"metadata\":{\"annotations\":{\"tes-task-name\":\"taskFull\"},\"labels\":{\"job-type\":\"executor\",\"taskmaster-name\":\"task-35605447\",\"executor-no\":\"1\"},\"name\":\"task-35605447-ex-01\"},\"spec\":{\"template\":{\"metadata\":{\"name\":\"task-35605447-ex-01\"},\"spec\":{\"containers\":[{\"args\":[\"sh\",\"-c\",\"md5sum $src\"],\"command\":[],\"env\":[{\"name\":\"src\",\"value\":\"/container/input/other.txt\"},{\"name\":\"sth\",\"value\":\"sthElse\"}],\"image\":\"alpine\",\"name\":\"task-35605447-ex-01\",\"resources\":{\"requests\":{\"cpu\":\"4\"}},\"workingDir\":\"/starthere\"}],\"restartPolicy\":\"Never\"}}}}],\"resources\":{}}" } ], - "image": "eu.gcr.io/tes-wes/taskmaster:v0.4", + "image": "eu.gcr.io/tes-wes/taskmaster:v0.8.5", "name": "task-98605447", "volumeMounts": [ {