From 03de689f71770744f7fee0b7fea80185b3ce4866 Mon Sep 17 00:00:00 2001 From: Hubert Balcerzak Date: Fri, 5 Feb 2021 02:34:01 +0100 Subject: [PATCH] Feature/file owners (#177) * file owners * upload history * but setup-java action version * endpoint descriptions * add migration * fix migration --- spring-app/src/docs/asciidoc/index.adoc | 4 ++ .../up/controller/UploadController.kt | 22 +++++-- .../up/controller/UserController.kt | 40 +++++++++++ .../data/dto/upload/UploadHistoryEntryDTO.kt | 34 ++++++++++ .../pl/starchasers/up/data/model/FileEntry.kt | 5 +- .../pl/starchasers/up/data/model/User.kt | 5 +- .../up/repository/FileEntryRepository.kt | 5 ++ .../pl/starchasers/up/service/FileService.kt | 38 ++++++++--- .../db/migration/V2__file_owners.sql | 8 +++ .../up/controller/UploadControllerTest.kt | 52 ++++++++++++--- .../up/controller/UserControllerTest.kt | 66 +++++++++++++++++++ 11 files changed, 256 insertions(+), 23 deletions(-) create mode 100644 spring-app/src/main/kotlin/pl/starchasers/up/controller/UserController.kt create mode 100644 spring-app/src/main/kotlin/pl/starchasers/up/data/dto/upload/UploadHistoryEntryDTO.kt create mode 100644 spring-app/src/main/resources/db/migration/V2__file_owners.sql create mode 100644 spring-app/src/test/kotlin/pl/starchasers/up/controller/UserControllerTest.kt diff --git a/spring-app/src/docs/asciidoc/index.adoc b/spring-app/src/docs/asciidoc/index.adoc index 98d09b8c..82dd6c69 100644 --- a/spring-app/src/docs/asciidoc/index.adoc +++ b/spring-app/src/docs/asciidoc/index.adoc @@ -11,6 +11,10 @@ include::build/generated-snippets/GetFileDetails/auto-section.adoc[] ''' include::build/generated-snippets/DeleteFile/auto-section.adoc[] +== User Controller +''' +include::build/generated-snippets/ListUserUploadHistory/auto-section.adoc[] + == Configuration Controller include::build/generated-snippets/GetConfiguration/auto-section.adoc[] diff --git a/spring-app/src/main/kotlin/pl/starchasers/up/controller/UploadController.kt b/spring-app/src/main/kotlin/pl/starchasers/up/controller/UploadController.kt index e7587be4..1b6016d8 100644 --- a/spring-app/src/main/kotlin/pl/starchasers/up/controller/UploadController.kt +++ b/spring-app/src/main/kotlin/pl/starchasers/up/controller/UploadController.kt @@ -38,6 +38,7 @@ class UploadController( private val logger = LoggerFactory.getLogger(this::class.java) /** + * Upload new file * @param file Uploaded file content */ @PostMapping("/api/upload") @@ -58,6 +59,7 @@ class UploadController( } /** + * Download a previously uploaded file * @param fileKey File key obtained during upload */ @GetMapping("/u/{fileKey}") @@ -97,29 +99,41 @@ class UploadController( } } + /** + * Verify requesting user's permission to modify this upload + */ @PostMapping("/api/u/{fileKey}/verify") fun verifyFileAccess( @PathVariable fileKey: String, - @Validated @RequestBody operationDto: AuthorizedOperationDTO + @Validated @RequestBody operationDto: AuthorizedOperationDTO?, + principal: Principal? ): BasicResponseDTO { val fileEntry = fileService.findFileEntry(FileKey(fileKey)) ?: throw NotFoundException() - if (!fileService.verifyFileAccess(fileEntry, FileAccessToken(operationDto.accessToken))) throw AccessDeniedException() + val user = userService.fromPrincipal(principal) + if (!fileService.verifyFileAccess(fileEntry, operationDto?.accessToken?.let { FileAccessToken(it) }, user)) + throw AccessDeniedException() return BasicResponseDTO() } @DeleteMapping("/api/u/{fileKey}") fun deleteFile( @PathVariable fileKey: String, - @Validated @RequestBody operationDto: AuthorizedOperationDTO + @Validated @RequestBody operationDto: AuthorizedOperationDTO?, + principal: Principal? ) { val fileEntry = fileService.findFileEntry(FileKey(fileKey)) ?: throw NotFoundException() + val user = userService.fromPrincipal(principal) - if (!fileService.verifyFileAccess(fileEntry, FileAccessToken(operationDto.accessToken))) throw AccessDeniedException() + if (!fileService.verifyFileAccess(fileEntry, operationDto?.accessToken?.let { FileAccessToken(it) }, user)) + throw AccessDeniedException() fileService.deleteFile(fileEntry) } + /** + * @return Uploaded file metadata + */ @GetMapping("/api/u/{fileKey}/details") fun getFileDetails(@PathVariable fileKey: String): FileDetailsDTO = fileService.getFileDetails(FileKey(fileKey)) } diff --git a/spring-app/src/main/kotlin/pl/starchasers/up/controller/UserController.kt b/spring-app/src/main/kotlin/pl/starchasers/up/controller/UserController.kt new file mode 100644 index 00000000..c331756f --- /dev/null +++ b/spring-app/src/main/kotlin/pl/starchasers/up/controller/UserController.kt @@ -0,0 +1,40 @@ +package pl.starchasers.up.controller + +import org.springframework.data.domain.Page +import org.springframework.data.domain.Pageable +import org.springframework.web.bind.annotation.GetMapping +import org.springframework.web.bind.annotation.RequestMapping +import org.springframework.web.bind.annotation.RestController +import pl.starchasers.up.data.dto.upload.UploadHistoryEntryDTO +import pl.starchasers.up.exception.AccessDeniedException +import pl.starchasers.up.security.IsUser +import pl.starchasers.up.service.FileService +import pl.starchasers.up.service.UserService +import java.security.Principal + +@RestController +@RequestMapping("/api/user") +class UserController( + val fileService: FileService, + val userService: UserService +) { + + @GetMapping("/history") + @IsUser + fun listUserUploadHistory(principal: Principal, pageable: Pageable): Page { + return fileService.getUploadHistory( + userService.fromPrincipal(principal) ?: throw AccessDeniedException(), + pageable + ).map { + UploadHistoryEntryDTO( + it.filename.value, + it.createdDate, + it.permanent, + it.toDeleteDate, + it.size.value, + it.contentType.value, + it.key.value + ) + } + } +} diff --git a/spring-app/src/main/kotlin/pl/starchasers/up/data/dto/upload/UploadHistoryEntryDTO.kt b/spring-app/src/main/kotlin/pl/starchasers/up/data/dto/upload/UploadHistoryEntryDTO.kt new file mode 100644 index 00000000..a94bb1df --- /dev/null +++ b/spring-app/src/main/kotlin/pl/starchasers/up/data/dto/upload/UploadHistoryEntryDTO.kt @@ -0,0 +1,34 @@ +package pl.starchasers.up.data.dto.upload + +import java.sql.Timestamp + +data class UploadHistoryEntryDTO( + /** + * Name of the file + */ + val filename: String, + /** + * When was this file uploaded + */ + val uploadDate: Timestamp, + /** + * Whether this file will be automatically deleted + */ + val permanent: Boolean, + /** + * When will this file be automatically deleted. Null if temporary == false + */ + val deleteDate: Timestamp?, + /** + * File size in bytes + */ + val size: Long, + /** + * Mime type of this file. This string is used as content-type header when serving file to clients. + */ + val mimeType: String, + /** + * File key used in file link. + */ + val key: String +) diff --git a/spring-app/src/main/kotlin/pl/starchasers/up/data/model/FileEntry.kt b/spring-app/src/main/kotlin/pl/starchasers/up/data/model/FileEntry.kt index e9e05e02..25d7f096 100644 --- a/spring-app/src/main/kotlin/pl/starchasers/up/data/model/FileEntry.kt +++ b/spring-app/src/main/kotlin/pl/starchasers/up/data/model/FileEntry.kt @@ -38,5 +38,8 @@ class FileEntry( val accessToken: FileAccessToken, @Embedded - val size: FileSize + val size: FileSize, + + @ManyToOne(fetch = FetchType.LAZY) + val owner: User? ) diff --git a/spring-app/src/main/kotlin/pl/starchasers/up/data/model/User.kt b/spring-app/src/main/kotlin/pl/starchasers/up/data/model/User.kt index f5509130..b9d9c205 100644 --- a/spring-app/src/main/kotlin/pl/starchasers/up/data/model/User.kt +++ b/spring-app/src/main/kotlin/pl/starchasers/up/data/model/User.kt @@ -36,5 +36,8 @@ class User( @Embedded @AttributeOverride(name = "value", column = Column(name = "maxFileLifetime")) - var maxFileLifetime: Milliseconds = Milliseconds(0) + var maxFileLifetime: Milliseconds = Milliseconds(0), + + @OneToMany(fetch = FetchType.LAZY, mappedBy = "owner") + val files: MutableSet = mutableSetOf() ) diff --git a/spring-app/src/main/kotlin/pl/starchasers/up/repository/FileEntryRepository.kt b/spring-app/src/main/kotlin/pl/starchasers/up/repository/FileEntryRepository.kt index 0d2d423f..6638778b 100644 --- a/spring-app/src/main/kotlin/pl/starchasers/up/repository/FileEntryRepository.kt +++ b/spring-app/src/main/kotlin/pl/starchasers/up/repository/FileEntryRepository.kt @@ -1,8 +1,11 @@ package pl.starchasers.up.repository +import org.springframework.data.domain.Page +import org.springframework.data.domain.Pageable import org.springframework.data.jpa.repository.JpaRepository import org.springframework.data.jpa.repository.Query import pl.starchasers.up.data.model.FileEntry +import pl.starchasers.up.data.model.User import pl.starchasers.up.data.value.FileKey interface FileEntryRepository : JpaRepository { @@ -22,4 +25,6 @@ interface FileEntryRepository : JpaRepository { """ ) fun findExpiredFiles(): Set + + fun findAllByOwner(owner: User, pageable: Pageable): Page } diff --git a/spring-app/src/main/kotlin/pl/starchasers/up/service/FileService.kt b/spring-app/src/main/kotlin/pl/starchasers/up/service/FileService.kt index 07198406..45bcf29d 100644 --- a/spring-app/src/main/kotlin/pl/starchasers/up/service/FileService.kt +++ b/spring-app/src/main/kotlin/pl/starchasers/up/service/FileService.kt @@ -1,6 +1,8 @@ package pl.starchasers.up.service import org.springframework.beans.factory.annotation.Value +import org.springframework.data.domain.Page +import org.springframework.data.domain.Pageable import org.springframework.stereotype.Service import pl.starchasers.up.data.dto.upload.FileDetailsDTO import pl.starchasers.up.data.dto.upload.UploadCompleteResponseDTO @@ -19,17 +21,25 @@ import javax.transaction.Transactional interface FileService { - fun createFile(tmpFile: InputStream, filename: Filename, contentType: ContentType, size: FileSize, user: User?): UploadCompleteResponseDTO + fun createFile( + tmpFile: InputStream, + filename: Filename, + contentType: ContentType, + size: FileSize, + user: User? + ): UploadCompleteResponseDTO - fun verifyFileAccess(fileEntry: FileEntry, accessToken: FileAccessToken): Boolean + fun verifyFileAccess(fileEntry: FileEntry, accessToken: FileAccessToken?, user: User?): Boolean - fun verifyFileAccess(fileKey: FileKey, accessToken: FileAccessToken): Boolean + fun verifyFileAccess(fileKey: FileKey, accessToken: FileAccessToken?, user: User?): Boolean fun findFileEntry(fileKey: FileKey): FileEntry? fun getFileDetails(fileKey: FileKey): FileDetailsDTO fun deleteFile(fileEntry: FileEntry) + + fun getUploadHistory(user: User, pageable: Pageable): Page } @Service @@ -55,7 +65,11 @@ class FileServiceImpl( ): UploadCompleteResponseDTO { val actualContentType = when { contentType.value.isBlank() -> ContentType("application/octet-stream") - contentType.value == "text/plain" -> ContentType("text/plain; charset=" + charsetDetectionService.detect(tmpFile)) + contentType.value == "text/plain" -> ContentType( + "text/plain; charset=" + charsetDetectionService.detect( + tmpFile + ) + ) else -> contentType } val personalLimit: FileSize = user?.maxTemporaryFileSize @@ -78,7 +92,8 @@ class FileServiceImpl( toDeleteDate, false, accessToken, - size + size, + user ) fileEntryRepository.save(fileEntry) @@ -86,13 +101,14 @@ class FileServiceImpl( return UploadCompleteResponseDTO(key.value, accessToken.value, toDeleteDate) } - override fun verifyFileAccess(fileEntry: FileEntry, accessToken: FileAccessToken): Boolean = - fileEntry.accessToken == accessToken + override fun verifyFileAccess(fileEntry: FileEntry, accessToken: FileAccessToken?, user: User?): Boolean { + return (user != null && fileEntry.owner == user) || fileEntry.accessToken == accessToken + } - override fun verifyFileAccess(fileKey: FileKey, accessToken: FileAccessToken): Boolean = + override fun verifyFileAccess(fileKey: FileKey, accessToken: FileAccessToken?, user: User?): Boolean = fileEntryRepository .findExistingFileByKey(fileKey) - ?.let { it.accessToken == accessToken } ?: false + ?.let { verifyFileAccess(it, accessToken, user) } ?: false override fun findFileEntry(fileKey: FileKey): FileEntry? = fileEntryRepository.findExistingFileByKey(fileKey) @@ -114,5 +130,9 @@ class FileServiceImpl( fileStorageService.deleteFile(fileEntry) } + override fun getUploadHistory(user: User, pageable: Pageable): Page { + return fileEntryRepository.findAllByOwner(user, pageable) + } + private fun generateFileAccessToken(): FileAccessToken = FileAccessToken(util.secureAlphanumericRandomString(128)) } diff --git a/spring-app/src/main/resources/db/migration/V2__file_owners.sql b/spring-app/src/main/resources/db/migration/V2__file_owners.sql new file mode 100644 index 00000000..5904916b --- /dev/null +++ b/spring-app/src/main/resources/db/migration/V2__file_owners.sql @@ -0,0 +1,8 @@ +start transaction; + +alter table file_entry + add column owner_id bigint(20) default null, + add key `ix_file_entry__owner_id` (owner_id), + add constraint `fk_user__file_entry` foreign key (owner_id) references `user` (`id`); + +commit; diff --git a/spring-app/src/test/kotlin/pl/starchasers/up/controller/UploadControllerTest.kt b/spring-app/src/test/kotlin/pl/starchasers/up/controller/UploadControllerTest.kt index 06e20c4d..05719489 100644 --- a/spring-app/src/test/kotlin/pl/starchasers/up/controller/UploadControllerTest.kt +++ b/spring-app/src/test/kotlin/pl/starchasers/up/controller/UploadControllerTest.kt @@ -247,7 +247,8 @@ internal class UploadControllerTest : JpaTestBase() { @TestInstance(TestInstance.Lifecycle.PER_CLASS) inner class VerifyFileAccess( @Autowired val fileService: FileService, - @Autowired val fileEntryRepository: FileEntryRepository + @Autowired val fileEntryRepository: FileEntryRepository, + @Autowired val userService: UserService ) : MockMvcTestBase() { private fun verifyRequestPath(key: String): Path = Path("/api/u/$key/verify") private val content = "example content" @@ -262,7 +263,7 @@ internal class UploadControllerTest : JpaTestBase() { Filename("filename.txt"), ContentType("text/plain"), FileSize(content.byteInputStream().readAllBytes().size.toLong()), - null + userService.getUser(Username("root")) ).key fileAccessToken = fileEntryRepository.findExistingFileByKey(FileKey(fileKey))?.accessToken?.value @@ -285,6 +286,29 @@ internal class UploadControllerTest : JpaTestBase() { } } + @Test + fun `Given valid owner and no token, should return 200`() { + mockMvc.post( + path = verifyRequestPath(fileKey), + headers = HttpHeaders().contentTypeJson().authorization(getAdminAccessToken()) + ) { + isSuccess() + } + } + + @Test + fun `Given invalid owner and valid token, should return 200`() { + mockMvc.post( + path = verifyRequestPath(fileKey), + headers = HttpHeaders().contentTypeJson(), + body = object { + val accessToken = fileAccessToken + } + ) { + isSuccess() + } + } + @Test fun `Given invalid access token, should return 403`() { mockMvc.post( @@ -299,13 +323,12 @@ internal class UploadControllerTest : JpaTestBase() { } @Test - fun `Given missing access token, should return 400`() { + fun `Given missing access token and no user, should return 403`() { mockMvc.post( path = verifyRequestPath(fileKey), headers = HttpHeaders().contentTypeJson(), - body = object {} ) { - isError(HttpStatus.BAD_REQUEST) + isError(HttpStatus.FORBIDDEN) } } @@ -374,7 +397,8 @@ internal class UploadControllerTest : JpaTestBase() { inner class DeleteFile( @Autowired val fileService: FileService, @Autowired val uploadRepository: UploadRepository, - @Autowired val fileEntryRepository: FileEntryRepository + @Autowired val fileEntryRepository: FileEntryRepository, + @Autowired val userService: UserService ) : MockMvcTestBase() { private fun getRequestPath(fileKey: String) = Path("/api/u/$fileKey") @@ -386,13 +410,13 @@ internal class UploadControllerTest : JpaTestBase() { Filename("file"), ContentType("text/plain"), FileSize(fileContent.length.toLong()), - null + userService.getUser(Username("root")) ) } @Test @DocumentResponse - fun `Given valid request, should delete file`() { + fun `Given valid access token, should delete file`() { val response = createTestFile() mockMvc.delete( path = getRequestPath(response.key), @@ -408,6 +432,18 @@ internal class UploadControllerTest : JpaTestBase() { assertNull(fileEntryRepository.findExistingFileByKey(FileKey(response.key))) } + @Test + fun `Given valid owner, should delete file`() { + val response = createTestFile() + + mockMvc.delete( + path = getRequestPath(response.key), + headers = HttpHeaders().contentTypeJson().authorization(getAdminAccessToken()) + ) { + isSuccess() + } + } + @Test fun `Given wrong access token, should return 403`() { val response = createTestFile() diff --git a/spring-app/src/test/kotlin/pl/starchasers/up/controller/UserControllerTest.kt b/spring-app/src/test/kotlin/pl/starchasers/up/controller/UserControllerTest.kt new file mode 100644 index 00000000..94486703 --- /dev/null +++ b/spring-app/src/test/kotlin/pl/starchasers/up/controller/UserControllerTest.kt @@ -0,0 +1,66 @@ +package pl.starchasers.up.controller + +import no.skatteetaten.aurora.mockmvc.extensions.Path +import no.skatteetaten.aurora.mockmvc.extensions.authorization +import no.skatteetaten.aurora.mockmvc.extensions.get +import no.skatteetaten.aurora.mockmvc.extensions.responseJsonPath +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.TestInstance +import org.springframework.beans.factory.annotation.Autowired +import org.springframework.http.HttpHeaders +import org.springframework.http.HttpStatus +import org.springframework.test.annotation.DirtiesContext +import pl.starchasers.up.* +import pl.starchasers.up.data.value.* +import pl.starchasers.up.service.FileService +import pl.starchasers.up.service.UserService +import java.lang.IllegalStateException +import javax.transaction.Transactional + +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +@DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_CLASS) +class UserControllerTest : MockMvcTestBase() { + + @Transactional + @OrderTests + @Nested + inner class ListUserUploadHistory( + @Autowired private val userService: UserService, + @Autowired private val fileService: FileService + ) : MockMvcTestBase() { + + private val listHistoryRequestPath = Path("/api/user/history") + + @DocumentResponse + @Test + fun `Given valid request, should return upload history`() { + val file = fileService.createFile( + "content".byteInputStream(), + Filename("file"), + ContentType("text/html"), + FileSize(7), + userService.getUser(Username("root")) + ) + val fileEntry = fileService.findFileEntry(FileKey(file.key)) ?: throw IllegalStateException() + mockMvc.get( + path = listHistoryRequestPath, + headers = HttpHeaders().authorization(getAdminAccessToken()) + ) { + isSuccess() + responseJsonPath("$.content[0].filename").equalsValue(fileEntry.filename.value) + responseJsonPath("$.content[0].permanent").equalsValue(fileEntry.permanent) + responseJsonPath("$.content[0].size").equalsValue(fileEntry.size.value) + responseJsonPath("$.content[0].mimeType").equalsValue(fileEntry.contentType.value) + responseJsonPath("$.content[0].key").equalsValue(fileEntry.key.value) + } + } + + @Test + fun `Given unauthenticated request, should return 403`() { + mockMvc.get(path = listHistoryRequestPath) { + isError(HttpStatus.FORBIDDEN) + } + } + } +}