API Access Configuration, get Artifacts by fling id

service/fling/src/main/java/net/friedl/fling/controller/FlingController.java

@@ -2,6 +2,7 @@ package net.friedl.fling.controller;
 
 import java.io.IOException;
 import java.util.List;
+import java.util.Set;
 import java.util.UUID;
 import javax.validation.Valid;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -56,12 +57,17 @@ public class FlingController {
     return flingService.create(flingDto);
   }
 
-  @PostMapping("/{id}/artifact")
+  @PostMapping("/{id}/artifacts")
   public ArtifactDto postArtifact(@PathVariable UUID id,
       @RequestBody @Valid ArtifactDto artifactDto) {
     return artifactService.create(id, artifactDto);
   }
 
+  @GetMapping("/{id}/artifacts")
+  public Set<ArtifactDto> getArtifacts(@PathVariable UUID id) {
+    return flingService.getArtifacts(id);
+  }
+
   @GetMapping(path = "/{id}")
   public FlingDto getFling(@PathVariable UUID id) {
     return flingService.getById(id);

service/fling/src/main/java/net/friedl/fling/model/mapper/ArtifactMapper.java

@@ -1,6 +1,7 @@
 package net.friedl.fling.model.mapper;
 
 import java.util.List;
+import java.util.Set;
 import org.mapstruct.Mapper;
 import net.friedl.fling.model.dto.ArtifactDto;
 import net.friedl.fling.persistence.entities.ArtifactEntity;
@@ -13,5 +14,7 @@ public interface ArtifactMapper {
 
   List<ArtifactDto> mapEntities(List<ArtifactEntity> artifactEntities);
 
+  Set<ArtifactDto> mapEntities(Set<ArtifactEntity> artifactEntities);
+
   List<ArtifactEntity> mapDtos(List<ArtifactDto> artifactDtos);
 }

service/fling/src/main/java/net/friedl/fling/persistence/repositories/FlingRepository.java

@@ -11,6 +11,6 @@ public interface FlingRepository extends JpaRepository<FlingEntity, UUID> {
 
   FlingEntity findByShareId(String shareId);
 
-  @Query("SELECT COUNT(*) FROM ArtifactEntity a, FlingEntity f where a.fling=f.id and f.id=:flingId")
+  @Query("SELECT fe FROM FlingEntity fe JOIN ArtifactEntity ae ON fe.id=ae.id WHERE ae.id=:artifactId")
-  Long countArtifactsById(Long flingId);
+  FlingEntity findByArtifactId(UUID artifactId);
 }

service/fling/src/main/java/net/friedl/fling/security/FlingWebSecurityConfigurer.java

@@ -1,5 +1,6 @@
 package net.friedl.fling.security;
 
+import static net.friedl.fling.security.FlingAuthorities.FLING_ADMIN;
 import static org.springframework.security.config.Customizer.withDefaults;
 import java.util.List;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -10,6 +11,7 @@ import org.springframework.http.HttpMethod;
 import org.springframework.security.config.annotation.web.builders.HttpSecurity;
 import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
 import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
+import org.springframework.security.config.http.SessionCreationPolicy;
 import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter;
 import org.springframework.web.cors.CorsConfiguration;
 import org.springframework.web.cors.CorsConfigurationSource;
@@ -48,54 +50,76 @@ public class FlingWebSecurityConfigurer extends WebSecurityConfigurerAdapter {
          http
         .csrf().disable()
         .cors(withDefaults())
+        
+        /**********************************************/
+        /** Authentication Interceptor Configuration **/
+        /**********************************************/
         .addFilterBefore(jwtAuthenticationFilter, UsernamePasswordAuthenticationFilter.class)
-        // Everybody can try to authenticate
+        // Do not keep authorization token in session. This would interfere with bearer authentication
+        // in that it is possible to authenticate without a bearer token if the session is kept.
+        // Turn off this confusing and non-obvious behavior.
+        .sessionManagement().sessionCreationPolicy(SessionCreationPolicy.STATELESS).and()
+
+        
+        /*************************************/
+        /** API Authorization Configuration **/
+        /*************************************/
+        //! Go from most specific to more !//
+        //! general, as first hit counts  !//
+
+        /**********************************/
+        /** Authorization for: /api/auth **/
+        /**********************************/
         .authorizeRequests()
             .antMatchers("/api/auth/**")
             .permitAll()
         .and()
-        // We need to go from most specific to more general.
+
-        // Hence, first define user permissions
+
-        .authorizeRequests()
+        /***********************************/
-            // TODO: This is still insecure since URLs are not encrypted
+        /** Authorization for: /api/fling **/
-            // TODO: iframe requests don't send the bearer, use cookie instead
+        /***********************************/
-            .antMatchers(HttpMethod.GET, "/api/fling/{flingId}/download/{downloadId}")
-            .permitAll()
-        .and()
-        .authorizeRequests()
-            .antMatchers(HttpMethod.POST, "/api/artifacts/{flingId}/**")
-            .access("@authorizationService.allowUpload(#flingId, authentication)")
-        .and()
-        .authorizeRequests()
-            .antMatchers(HttpMethod.PATCH, "/api/artifacts/{artifactId}")
-            .access("@authorizationService.allowPatchingArtifact(#artifactId, authentication)")
-        .and()
-        .authorizeRequests()
-            // TODO: This is still insecure since URLs are not encrypted
-            // TODO: iframe requests don't send the bearer, use cookie instead
-            .antMatchers("/api/artifacts/{artifactId}/{downloadId}/download")
-            .permitAll()
-        .and()
-        .authorizeRequests()
-            // TODO: Security by request parameters is just not well supported with spring security
-            // TODO: Change API
-            .regexMatchers(HttpMethod.GET, "\\/api\\/fling\\?(shareId=|flingId=)[a-zA-Z0-9]+")
-            .access("@authorizationService.allowFlingAccess(authentication, request)")
-        .and()
-        .authorizeRequests()
-            // TODO: Security by request parameters is just not well supported with spring security
-            // TODO: Change API
-            .regexMatchers(HttpMethod.GET, "\\/api\\/artifacts\\?(shareId=|flingId=)[a-zA-Z0-9]+")
-            .access("@authorizationService.allowFlingAccess(authentication, request)")
-        .and()
         .authorizeRequests()
           .antMatchers(HttpMethod.GET, "/api/fling/{flingId}/**")
           .access("@authorizationService.allowFlingAccess(#flingId, authentication)")
         .and()
-        // And lastly, the owner is allowed everything
         .authorizeRequests()
-            .antMatchers("/api/**")
+          .antMatchers(HttpMethod.GET, "/api/fling/share/{shareId}")
-            .hasAuthority(FlingAuthorities.FLING_ADMIN.getAuthority());
+          .access("@authorizationService.allowFlingAccessByShareId(#shareId, authentication)")
+        .and()
+        .authorizeRequests()
+          .antMatchers(HttpMethod.POST, "/api/fling/{flingId}/artifact")
+          .access("@authorizationService.allowUpload(#flingId, authentication)")
+        .and()
+        // only admin can create, delete and list flings
+        .authorizeRequests()
+          .antMatchers(HttpMethod.DELETE, "/api/fling/{flingId}")
+          .hasAnyAuthority(FLING_ADMIN.getAuthority())
+        .and()
+        .authorizeRequests()
+          .antMatchers(HttpMethod.POST, "/api/fling")
+          .hasAuthority(FLING_ADMIN.getAuthority())
+        .and()
+        .authorizeRequests()
+          .antMatchers(HttpMethod.GET, "/api/fling")
+          .hasAuthority(FLING_ADMIN.getAuthority())
+        .and()
+
+
+        /***************************************/
+        /** Authorization for: /api/artifacts **/
+        /***************************************/
+        .authorizeRequests()
+          .antMatchers(HttpMethod.GET, "/api/artifacts/{artifactId}/**")
+          .access("@authorizationService.allowArtifactAccess(#artifactId, token)")
+        .and()
+        .authorizeRequests()
+          .antMatchers(HttpMethod.POST, "/api/artifacts/{artifactId}/data")
+          .access("@authorizationService.allowArtifactUpload(#artifactId, token)")
+        .and()
+        .authorizeRequests()
+          .antMatchers(HttpMethod.DELETE, "/api/artifacts/{artifactId}")
+          .access("@authorizationService.allowArtifactUpload(#artifactId, token)");
 
         //@formatter:on
   }

service/fling/src/main/java/net/friedl/fling/service/AuthenticationService.java

@@ -91,7 +91,7 @@ public class AuthenticationService {
     Claims claims = jwtParser.parseClaimsJws(token).getBody();
 
     switch (claims.getSubject()) {
-      case "owner":
+      case "admin":
         return new FlingToken(new FlingAdminAuthority(), token);
       case "user":
         UUID grantedFlingId = UUID.fromString(claims.get("id", String.class));

service/fling/src/main/java/net/friedl/fling/service/AuthorizationService.java

@@ -5,6 +5,7 @@ import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.security.authentication.AbstractAuthenticationToken;
 import org.springframework.stereotype.Service;
 import lombok.extern.slf4j.Slf4j;
+import net.friedl.fling.persistence.entities.FlingEntity;
 import net.friedl.fling.persistence.repositories.FlingRepository;
 import net.friedl.fling.security.FlingAuthorities;
 import net.friedl.fling.security.authentication.FlingToken;
@@ -65,4 +66,20 @@ public class AuthorizationService {
     log.info("User not authorized to access fling[.id={}]", flingId);
     return false;
   }
+
+  public boolean allowFlingAccessByShareId(String shareId, AbstractAuthenticationToken token) {
+    FlingEntity flingEntity = flingRepository.findByShareId(shareId);
+    return allowFlingAccess(flingEntity.getId(), token);
+  }
+
+  public boolean allowArtifactAccess(UUID artifactId, AbstractAuthenticationToken token) {
+    FlingEntity flingEntity = flingRepository.findByArtifactId(artifactId);
+    return allowFlingAccess(flingEntity.getId(), token);
+  }
+
+  public boolean allowArtifactUpload(UUID artifactId, AbstractAuthenticationToken token) {
+    FlingEntity flingEntity = flingRepository.findByArtifactId(artifactId);
+    return allowUpload(flingEntity.getId(), token);
+  }
+
 }

service/fling/src/main/java/net/friedl/fling/service/FlingService.java

@@ -3,6 +3,7 @@ package net.friedl.fling.service;
 import java.io.IOException;
 import java.util.Base64;
 import java.util.List;
+import java.util.Set;
 import java.util.UUID;
 import javax.transaction.Transactional;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -11,7 +12,9 @@ import org.springframework.security.crypto.password.PasswordEncoder;
 import org.springframework.stereotype.Service;
 import org.springframework.util.StringUtils;
 import lombok.extern.slf4j.Slf4j;
+import net.friedl.fling.model.dto.ArtifactDto;
 import net.friedl.fling.model.dto.FlingDto;
+import net.friedl.fling.model.mapper.ArtifactMapper;
 import net.friedl.fling.model.mapper.FlingMapper;
 import net.friedl.fling.persistence.entities.FlingEntity;
 import net.friedl.fling.persistence.repositories.FlingRepository;
@@ -24,15 +27,18 @@ public class FlingService {
 
   private FlingRepository flingRepository;
   private FlingMapper flingMapper;
+  private ArtifactMapper artifactMapper;
   private ArchiveService archiveService;
   private PasswordEncoder passwordEncoder;
 
   @Autowired
   public FlingService(FlingRepository flingRepository, FlingMapper flingMapper,
+      ArtifactMapper artifactMapper,
       ArchiveService archiveService, PasswordEncoder passwordEcoder) {
 
     this.flingRepository = flingRepository;
     this.flingMapper = flingMapper;
+    this.artifactMapper = artifactMapper;
     this.archiveService = archiveService;
     this.passwordEncoder = passwordEcoder;
   }
@@ -92,6 +98,12 @@ public class FlingService {
     log.debug("Deleted fling {}", id);
   }
 
+  public Set<ArtifactDto> getArtifacts(UUID id) {
+    FlingEntity flingEntity = flingRepository.getOne(id);
+    Set<ArtifactDto> artifactDto = artifactMapper.mapEntities(flingEntity.getArtifacts());
+    return artifactDto == null ? Set.of() : artifactDto;
+  }
+
   public boolean validateAuthCode(UUID id, String authCode) {
     FlingEntity flingEntity = flingRepository.getOne(id);
     if (StringUtils.hasText(flingEntity.getAuthCode()) != StringUtils.hasText(authCode)) {

service/fling/src/test/java/net/friedl/fling/controller/FlingControllerTest.java

@@ -1,6 +1,7 @@
 package net.friedl.fling.controller;
 
 import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.Matchers.anyOf;
 import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.not;
 import static org.mockito.ArgumentMatchers.any;
@@ -20,6 +21,7 @@ import java.io.IOException;
 import java.nio.file.Path;
 import java.time.Instant;
 import java.util.List;
+import java.util.Set;
 import java.util.UUID;
 import javax.persistence.EntityNotFoundException;
 import org.hamcrest.Matchers;
@@ -108,7 +110,7 @@ public class FlingControllerTest {
 
   @Test
   public void postArtifact_ok() throws Exception {
-    mockMvc.perform(post("/api/fling/{id}/artifact", flingId)
+    mockMvc.perform(post("/api/fling/{id}/artifacts", flingId)
         .content(mapper.writeValueAsString(artifactDto))
         .contentType(MediaType.APPLICATION_JSON))
         .andExpect(status().isOk());
@@ -118,12 +120,49 @@ public class FlingControllerTest {
   public void postArtifact_validatesBody_notOk() throws Exception {
     ArtifactDto invalidArtifactDto = new ArtifactDto();
 
-    mockMvc.perform(post("/api/fling/{id}/artifact", flingId)
+    mockMvc.perform(post("/api/fling/{id}/artifacts", flingId)
         .content(mapper.writeValueAsString(invalidArtifactDto))
         .contentType(MediaType.APPLICATION_JSON))
         .andExpect(status().isBadRequest());
   }
 
+  @Test
+  public void getArtifact_noFlingWithId_notFound() throws Exception {
+    doThrow(EntityNotFoundException.class).when(flingService).getArtifacts(flingId);
+
+    mockMvc.perform(get("/api/fling/{id}/artifacts", flingId))
+        .andExpect(status().isNotFound());
+  }
+
+  @Test
+  public void getArtifact_flingFound_noArtifacts_emptySet() throws Exception {
+    when(flingService.getArtifacts(flingId)).thenReturn(Set.of());
+
+    mockMvc.perform(get("/api/fling/{id}/artifacts", flingId))
+        .andExpect(status().isOk())
+        .andExpect(content().string(equalTo("[]")));
+  }
+
+  @Test
+  public void getArtifact_flingFound_hasArtifacts_returnArtifacts() throws Exception {
+    ArtifactDto artifactDto1 = ArtifactDto.builder()
+        .id(new UUID(0, 0))
+        .build();
+
+    ArtifactDto artifactDto2 = ArtifactDto.builder()
+        .id(new UUID(0, 1))
+        .build();
+
+    when(flingService.getArtifacts(flingId)).thenReturn(Set.of(artifactDto1, artifactDto2));
+
+    mockMvc.perform(get("/api/fling/{id}/artifacts", flingId))
+        .andExpect(status().isOk())
+        .andExpect(jsonPath("$[0].id",
+            anyOf(equalTo(new UUID(0, 0).toString()), equalTo(new UUID(0, 1).toString()))))
+        .andExpect(jsonPath("$[1].id",
+            anyOf(equalTo(new UUID(0, 0).toString()), equalTo(new UUID(0, 1).toString()))));
+  }
+
   @Test
   public void getFling_noFlingWithId_notFound() throws Exception {
     doThrow(EntityNotFoundException.class).when(flingService).getById(flingId);

service/fling/src/test/java/net/friedl/fling/service/AuthenticationServiceTest.java

@@ -118,7 +118,7 @@ public class AuthenticationServiceTest {
   @Test
   public void parseAuthentication_owner_AdminAuthority() {
     Jws<Claims> jwsClaims = new DefaultJws<>(new DefaultJwsHeader(),
-        new DefaultClaims(Map.of("sub", "owner")), "signature");
+        new DefaultClaims(Map.of("sub", "admin")), "signature");
     when(jwtParser.parseClaimsJws(any(String.class))).thenReturn(jwsClaims);
 
     FlingToken flingToken = authenticationService.parseAuthentication("any");

service/fling/src/test/java/net/friedl/fling/service/FlingServiceTest.java

@@ -4,8 +4,10 @@ import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.hasItems;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.emptyOrNullString;
+import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.not;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.verify;
@@ -13,7 +15,9 @@ import static org.mockito.Mockito.when;
 import java.io.IOException;
 import java.time.Instant;
 import java.util.List;
+import java.util.Set;
 import java.util.UUID;
+import java.util.stream.Collectors;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -25,9 +29,13 @@ import org.springframework.boot.test.mock.mockito.MockBean;
 import org.springframework.context.annotation.Bean;
 import org.springframework.security.crypto.password.PasswordEncoder;
 import org.springframework.test.context.junit.jupiter.SpringExtension;
+import net.friedl.fling.model.dto.ArtifactDto;
 import net.friedl.fling.model.dto.FlingDto;
+import net.friedl.fling.model.mapper.ArtifactMapper;
+import net.friedl.fling.model.mapper.ArtifactMapperImpl;
 import net.friedl.fling.model.mapper.FlingMapper;
 import net.friedl.fling.model.mapper.FlingMapperImpl;
+import net.friedl.fling.persistence.entities.ArtifactEntity;
 import net.friedl.fling.persistence.entities.FlingEntity;
 import net.friedl.fling.persistence.repositories.FlingRepository;
 import net.friedl.fling.service.archive.ArchiveService;
@@ -60,10 +68,17 @@ public class FlingServiceTest {
       return new FlingMapperImpl();
     }
 
+    @Bean
+    public ArtifactMapper ArtifactMapper() {
+      return new ArtifactMapperImpl();
+    }
+
     @Bean
     public FlingService flingService(FlingRepository flingRepository, FlingMapper flingMapper,
+        ArtifactMapper artifactMapper,
         ArchiveService archiveService, PasswordEncoder passwordEncoder) {
-      return new FlingService(flingRepository, flingMapper, archiveService, passwordEncoder);
+      return new FlingService(flingRepository, flingMapper, artifactMapper, archiveService,
+          passwordEncoder);
     }
   }
 
@@ -158,6 +173,37 @@ public class FlingServiceTest {
     verify(flingRepository).deleteById(testId);
   }
 
+  @Test
+  public void getArtifacts_noArtifacts_emptySet() throws IOException {
+    UUID testId = UUID.randomUUID();
+    FlingEntity flingEntity = new FlingEntity();
+    flingEntity.setId(testId);
+    flingEntity.setArtifacts(null);
+
+    when(flingRepository.getOne(testId)).thenReturn(flingEntity);
+
+    assertThat(flingService.getArtifacts(testId), is(empty()));
+  }
+
+  @Test
+  public void getArtifacts_flingWithArtifacts_artifactSet() throws Exception {
+    UUID artifactId = UUID.randomUUID();
+    ArtifactEntity artifactEntity = new ArtifactEntity();
+    artifactEntity.setId(artifactId);
+
+    UUID flingId = UUID.randomUUID();
+    FlingEntity flingEntity = new FlingEntity();
+    flingEntity.setId(flingId);
+    flingEntity.setArtifacts(Set.of(artifactEntity));
+
+    when(flingRepository.getOne(flingId)).thenReturn(flingEntity);
+
+    Set<ArtifactDto> artifacts = flingService.getArtifacts(flingId);
+    assertThat(artifacts, hasSize(1));
+    assertThat(artifacts.stream().map(ArtifactDto::getId).collect(Collectors.toSet()),
+        contains(artifactId));
+  }
+
   @Test
   public void validateAuthCode_codesMatch_true() {
     when(flingRepository.getOne(flingEntity1.getId())).thenReturn(flingEntity1);