diff --git a/clouddriver-docker/clouddriver-docker.gradle b/clouddriver-docker/clouddriver-docker.gradle index 6eb53f1f92e..1a2d197764e 100644 --- a/clouddriver-docker/clouddriver-docker.gradle +++ b/clouddriver-docker/clouddriver-docker.gradle @@ -31,4 +31,7 @@ dependencies { testImplementation "org.spockframework:spock-core" testImplementation "org.spockframework:spock-spring" testImplementation "org.springframework:spring-test" + testImplementation "org.springframework.boot:spring-boot-starter-test" + testImplementation "org.springframework.security:spring-security-test" + testImplementation "io.spinnaker.kork:kork-core" } diff --git a/clouddriver-docker/src/main/groovy/com/netflix/spinnaker/clouddriver/docker/registry/config/DockerRegistryConfigurationProperties.groovy b/clouddriver-docker/src/main/groovy/com/netflix/spinnaker/clouddriver/docker/registry/config/DockerRegistryConfigurationProperties.groovy index 02e1d39ef67..0dadf2f862c 100644 --- a/clouddriver-docker/src/main/groovy/com/netflix/spinnaker/clouddriver/docker/registry/config/DockerRegistryConfigurationProperties.groovy +++ b/clouddriver-docker/src/main/groovy/com/netflix/spinnaker/clouddriver/docker/registry/config/DockerRegistryConfigurationProperties.groovy @@ -18,6 +18,7 @@ package com.netflix.spinnaker.clouddriver.docker.registry.config import com.fasterxml.jackson.annotation.JsonTypeName import com.netflix.spinnaker.credentials.definition.CredentialsDefinition +import com.netflix.spinnaker.fiat.model.resources.Permissions import groovy.transform.EqualsAndHashCode import groovy.transform.ToString @@ -68,6 +69,8 @@ class DockerRegistryConfigurationProperties { String catalogFile // Allow filter the repositories by a regular expression String repositoriesRegex + // Permissions for using this account + Permissions.Builder permissions = new Permissions.Builder() } List accounts = [] diff --git a/clouddriver-docker/src/main/groovy/com/netflix/spinnaker/clouddriver/docker/registry/controllers/DockerRegistryImageLookupController.groovy b/clouddriver-docker/src/main/groovy/com/netflix/spinnaker/clouddriver/docker/registry/controllers/DockerRegistryImageLookupController.groovy index a633f523efa..25dcc1021e1 100644 --- a/clouddriver-docker/src/main/groovy/com/netflix/spinnaker/clouddriver/docker/registry/controllers/DockerRegistryImageLookupController.groovy +++ b/clouddriver-docker/src/main/groovy/com/netflix/spinnaker/clouddriver/docker/registry/controllers/DockerRegistryImageLookupController.groovy @@ -24,6 +24,8 @@ import com.netflix.spinnaker.clouddriver.docker.registry.provider.DockerRegistry import com.netflix.spinnaker.clouddriver.docker.registry.security.DockerRegistryNamedAccountCredentials import com.netflix.spinnaker.clouddriver.security.AccountCredentialsProvider import org.springframework.beans.factory.annotation.Autowired +import org.springframework.security.access.prepost.PostFilter +import org.springframework.security.access.prepost.PreAuthorize import org.springframework.web.bind.annotation.RequestMapping import org.springframework.web.bind.annotation.RequestMethod import org.springframework.web.bind.annotation.RequestParam @@ -39,6 +41,7 @@ class DockerRegistryImageLookupController { AccountCredentialsProvider accountCredentialsProvider @RequestMapping(value = "/tags", method = RequestMethod.GET) + @PreAuthorize("hasPermission(#account, 'ACCOUNT', 'READ')") List getTags(@RequestParam('account') String account, @RequestParam('repository') String repository) { def credentials = (DockerRegistryNamedAccountCredentials) accountCredentialsProvider.getCredentials(account) if (!credentials) { @@ -62,6 +65,7 @@ class DockerRegistryImageLookupController { } @RequestMapping(value = '/find', method = RequestMethod.GET) + @PostFilter("hasPermission(filterObject['account'], 'ACCOUNT', 'READ')") List find(LookupOptions lookupOptions) { def account = lookupOptions.account ?: "" diff --git a/clouddriver-docker/src/main/groovy/com/netflix/spinnaker/clouddriver/docker/registry/security/DockerRegistryNamedAccountCredentials.groovy b/clouddriver-docker/src/main/groovy/com/netflix/spinnaker/clouddriver/docker/registry/security/DockerRegistryNamedAccountCredentials.groovy index 8924f77f0db..4bd49c68766 100644 --- a/clouddriver-docker/src/main/groovy/com/netflix/spinnaker/clouddriver/docker/registry/security/DockerRegistryNamedAccountCredentials.groovy +++ b/clouddriver-docker/src/main/groovy/com/netflix/spinnaker/clouddriver/docker/registry/security/DockerRegistryNamedAccountCredentials.groovy @@ -21,6 +21,8 @@ import com.netflix.spinnaker.clouddriver.docker.registry.api.v2.client.DockerOkC import com.netflix.spinnaker.clouddriver.docker.registry.api.v2.client.DockerRegistryClient import com.netflix.spinnaker.clouddriver.docker.registry.exception.DockerRegistryConfigException import com.netflix.spinnaker.clouddriver.security.AbstractAccountCredentials +import com.netflix.spinnaker.fiat.model.Authorization +import com.netflix.spinnaker.fiat.model.resources.Permissions import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException import groovy.transform.CompileStatic import groovy.util.logging.Slf4j @@ -53,6 +55,7 @@ class DockerRegistryNamedAccountCredentials extends AbstractAccountCredentials skip String catalogFile String repositoriesRegex + Permissions permissions DockerOkClientProvider dockerOkClientProvider Builder() {} @@ -177,6 +180,11 @@ class DockerRegistryNamedAccountCredentials extends AbstractAccountCredentials requiredGroupMembership, + Permissions permissions, DockerOkClientProvider dockerOkClientProvider) { if (!accountName) { throw new IllegalArgumentException("Docker Registry account must be provided with a name.") @@ -336,8 +348,8 @@ class DockerRegistryNamedAccountCredentials extends AbstractAccountCredentials repositories, String catalogFile) { + private DockerRegistryCredentials buildCredentials(List repositories, String catalogFile, File dockerconfigFile) { try { DockerRegistryClient client = (new DockerRegistryClient.Builder()) .address(address) @@ -420,6 +432,7 @@ class DockerRegistryNamedAccountCredentials extends AbstractAccountCredentials requiredGroupMembership) { + if (requiredGroupMembership?.empty ?: true) { + return Permissions.EMPTY + } + def builder = new Permissions.Builder() + requiredGroupMembership.forEach { + builder.add(Authorization.READ, it).add(Authorization.WRITE, it) + } + builder.build() + } + private static final String CLOUD_PROVIDER = "dockerRegistry" private final String accountName final String environment @@ -458,7 +482,7 @@ class DockerRegistryNamedAccountCredentials extends AbstractAccountCredentials requiredGroupMembership + final Permissions permissions final List skip final String catalogFile final String repositoriesRegex diff --git a/clouddriver-docker/src/main/java/com/netflix/spinnaker/config/DockerRegistryConfiguration.java b/clouddriver-docker/src/main/java/com/netflix/spinnaker/config/DockerRegistryConfiguration.java index 084586e54e9..5b6a06f73ae 100644 --- a/clouddriver-docker/src/main/java/com/netflix/spinnaker/config/DockerRegistryConfiguration.java +++ b/clouddriver-docker/src/main/java/com/netflix/spinnaker/config/DockerRegistryConfiguration.java @@ -108,6 +108,7 @@ public DockerRegistryHealthIndicator dockerRegistryHealthIndicator( .insecureRegistry(a.getInsecureRegistry()) .repositories(a.getRepositories()) .skip(a.getSkip()) + .permissions(a.getPermissions().build()) .dockerOkClientProvider(dockerOkClientProvider) .build(), dockerRegistryCredentialsRepository); diff --git a/clouddriver-docker/src/test/java/com/netflix/spinnaker/clouddriver/docker/registry/controllers/DockerRegistryImageLookupControllerTest.java b/clouddriver-docker/src/test/java/com/netflix/spinnaker/clouddriver/docker/registry/controllers/DockerRegistryImageLookupControllerTest.java new file mode 100644 index 00000000000..b41db0cdbe0 --- /dev/null +++ b/clouddriver-docker/src/test/java/com/netflix/spinnaker/clouddriver/docker/registry/controllers/DockerRegistryImageLookupControllerTest.java @@ -0,0 +1,211 @@ +/* + * Copyright 2024 Apple, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.netflix.spinnaker.clouddriver.docker.registry.controllers; + +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import com.netflix.spectator.api.NoopRegistry; +import com.netflix.spectator.api.Registry; +import com.netflix.spinnaker.cats.cache.CacheData; +import com.netflix.spinnaker.cats.cache.DefaultCacheData; +import com.netflix.spinnaker.cats.cache.WriteableCache; +import com.netflix.spinnaker.cats.mem.InMemoryCache; +import com.netflix.spinnaker.clouddriver.docker.registry.DockerRegistryCloudProvider; +import com.netflix.spinnaker.clouddriver.docker.registry.cache.Keys; +import com.netflix.spinnaker.clouddriver.docker.registry.controllers.DockerRegistryImageLookupControllerTest.TestConfig; +import com.netflix.spinnaker.clouddriver.docker.registry.security.DockerRegistryNamedAccountCredentials; +import com.netflix.spinnaker.clouddriver.security.AccountCredentialsProvider; +import com.netflix.spinnaker.clouddriver.security.AccountCredentialsRepository; +import com.netflix.spinnaker.clouddriver.security.DefaultAccountCredentialsProvider; +import com.netflix.spinnaker.clouddriver.security.MapBackedAccountCredentialsRepository; +import com.netflix.spinnaker.fiat.model.Authorization; +import com.netflix.spinnaker.fiat.model.UserPermission; +import com.netflix.spinnaker.fiat.model.resources.Account; +import com.netflix.spinnaker.fiat.model.resources.Permissions; +import com.netflix.spinnaker.fiat.model.resources.Role; +import com.netflix.spinnaker.fiat.shared.EnableFiatAutoConfig; +import com.netflix.spinnaker.fiat.shared.FiatService; +import com.netflix.spinnaker.fiat.shared.FiatStatus; +import com.netflix.spinnaker.kork.dynamicconfig.DynamicConfigService; +import com.netflix.spinnaker.kork.dynamicconfig.SpringDynamicConfigService; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.json.AutoConfigureJson; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureWebMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Import; +import org.springframework.security.test.context.support.WithMockUser; +import org.springframework.test.web.servlet.MockMvc; + +@SpringBootTest(classes = TestConfig.class, properties = "services.fiat.cache.max-entries=0") +@AutoConfigureMockMvc +@AutoConfigureWebMvc +@AutoConfigureJson +@EnableFiatAutoConfig +@WithMockUser +class DockerRegistryImageLookupControllerTest { + @Import(DockerRegistryImageLookupController.class) + static class TestConfig { + @Bean + WriteableCache cache() { + return new InMemoryCache(); + } + + @Bean + AccountCredentialsRepository accountCredentialsRepository() { + return new MapBackedAccountCredentialsRepository(); + } + + @Bean + AccountCredentialsProvider accountCredentialsProvider( + AccountCredentialsRepository accountCredentialsRepository) { + return new DefaultAccountCredentialsProvider(accountCredentialsRepository); + } + + @Bean + Registry registry() { + return new NoopRegistry(); + } + + @Bean + DynamicConfigService dynamicConfigService() { + return new SpringDynamicConfigService(); + } + } + + @Autowired MockMvc mockMvc; + @Autowired WriteableCache cache; + @Autowired AccountCredentialsRepository accountCredentialsRepository; + @MockBean FiatStatus fiatStatus; + @MockBean FiatService fiatService; + + @BeforeEach + void setUp() { + given(fiatStatus.isEnabled()).willReturn(true); + } + + @Test + void authorizedToReadTags() throws Exception { + var permissions = createAuthorizedUserPermission(); + given(fiatService.getUserPermission(eq("user"))).willReturn(permissions); + + mockMvc + .perform( + get("/dockerRegistry/images/tags") + .queryParam("account", "test-account") + .queryParam("repository", "test-repository")) + .andExpect(status().isOk()); + } + + @Test + void notAuthorizedToReadTags() throws Exception { + var permissions = createUnauthorizedUserPermission(); + given(fiatService.getUserPermission("user")).willReturn(permissions); + + mockMvc + .perform( + get("/dockerRegistry/images/tags") + .queryParam("account", "test-account") + .queryParam("repository", "test-repository")) + .andExpect(status().isForbidden()); + } + + @Test + void canSearchForAuthorizedItems() throws Exception { + var permissions = createAuthorizedUserPermission(); + given(fiatService.getUserPermission("user")).willReturn(permissions); + cache.merge(Keys.Namespace.TAGGED_IMAGE.getNs(), createTestAccountTaggedImageCacheData()); + var credentials = createTestAccountCredentials(); + accountCredentialsRepository.save(credentials.getName(), credentials); + + mockMvc + .perform(get("/dockerRegistry/images/find")) + .andExpect(jsonPath("$[0].account").value("test-account")); + } + + @Test + void filtersOutUnauthorizedItems() throws Exception { + var permissions = createUnauthorizedUserPermission(); + given(fiatService.getUserPermission("user")).willReturn(permissions); + cache.merge(Keys.Namespace.TAGGED_IMAGE.getNs(), createTestAccountTaggedImageCacheData()); + var credentials = createTestAccountCredentials(); + accountCredentialsRepository.save(credentials.getName(), credentials); + + mockMvc + .perform(get("/dockerRegistry/images/find")) + .andExpectAll(status().isOk(), jsonPath("$.length()").value(0)); + } + + private static UserPermission.View createAuthorizedUserPermission() { + return new UserPermission() + .setId("user") + .addResources( + List.of( + new Account() + .setName("test-account") + .setPermissions( + new Permissions.Builder().add(Authorization.READ, "user").build()), + new Role("user"))) + .getView(); + } + + private static UserPermission.View createUnauthorizedUserPermission() { + return new UserPermission().setId("user").addResources(List.of(new Role("user"))).getView(); + } + + private static CacheData createTestAccountTaggedImageCacheData() { + String imageKey = Keys.getTaggedImageKey("test-account", "test-repository", "1.0"); + return new DefaultCacheData( + imageKey, + Map.of( + "account", + "test-account", + "digest", + "test-digest", + "labels", + Map.of( + "commitId", + "test-commit", + "buildNumber", + "1", + "branch", + "test-branch", + "jobName", + "test-job")), + Map.of()); + } + + private static DockerRegistryNamedAccountCredentials createTestAccountCredentials() { + var credentials = mock(DockerRegistryNamedAccountCredentials.class); + given(credentials.getName()).willReturn("test-account"); + given(credentials.getCloudProvider()) + .willReturn(DockerRegistryCloudProvider.getDOCKER_REGISTRY()); + given(credentials.getRegistry()).willReturn("test-registry"); + return credentials; + } +}