Commit 3eee7580 authored by Thomas Bouffard's avatar Thomas Bouffard Committed by Adrien

fix(http): always return non 200 code in case of error (#13)

Returning 200 code prevented the caller to detect that an error occured.
Now return 400 when file argument is empty and 500 for non specific exception.
In addition, as we return text in response on error, set the response content
type to text/plain

More tests have been added to the controller. This implies refactoring
  - inject windows and mack sign collaborator
  - extract new collaborator for all file based operations
Tests now use mockk instead of mockito as mockito cannot mock final class
and mockk is designed for Kotlin

Remove duplicated code in the Rest Controller between 'sign' and
'build and sign Mac installer'
parent c821ca00
......@@ -3,4 +3,5 @@
/bin
/build
/.idea
*.iml
\ No newline at end of file
*.iml
out/
......@@ -3,6 +3,9 @@ import org.apache.tools.ant.filters.ReplaceTokens
buildscript {
ext.kotlin_version = '1.2.40' // Required for Kotlin integration
ext.spring_boot_version = '2.0.1.RELEASE'
ext.spek_version = '1.1.5'
ext.junit_platform_version = '1.0.0' // 1.3.2
ext.junit_jupiter_version = '5.0.0' // 5.3.2 + stay align with platform: 5.x.y with 1.x.y
repositories {
maven { url 'http://repositories.rd.lan/maven/all/' }
jcenter()
......@@ -11,7 +14,7 @@ buildscript {
classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version" // Required for Kotlin integration
classpath "org.jetbrains.kotlin:kotlin-allopen:$kotlin_version" // See https://kotlinlang.org/docs/reference/compiler-plugins.html#kotlin-spring-compiler-plugin
classpath "org.springframework.boot:spring-boot-gradle-plugin:$spring_boot_version"
classpath "org.junit.platform:junit-platform-gradle-plugin:1.0.0"
classpath "org.junit.platform:junit-platform-gradle-plugin:$junit_platform_version"
}
}
......@@ -47,10 +50,12 @@ dependencies {
compile "org.jetbrains.kotlin:kotlin-stdlib-jdk8:$kotlin_version"
compile "org.springframework.boot:spring-boot-starter-web:$spring_boot_version"
compile "org.springframework.boot:spring-boot-starter-test:$spring_boot_version"
testCompile 'org.jetbrains.spek:spek-api:1.1.5'
testCompile 'org.jetbrains.spek:spek-junit-platform-engine:1.1.5'
testCompile 'org.junit.platform:junit-platform-runner:1.0.0'
testCompile "org.jetbrains.spek:spek-api:$spek_version"
testCompile "org.jetbrains.spek:spek-junit-platform-engine:$spek_version"
testCompile "org.junit.platform:junit-platform-runner:$junit_platform_version"
testCompile "org.jetbrains.kotlin:kotlin-test:$kotlin_version"
testCompile 'org.assertj:assertj-core:3.11.1'
testImplementation "io.mockk:mockk:1.8.13"
mypackaging "com.sun.winsw:winsw:2.1.2:bin@exe"
}
......
......@@ -14,5 +14,4 @@
*/
package org.bonitasoft.exception
class BuildDmgException(message : String) : Throwable(message) {
}
\ No newline at end of file
class BuildDmgException(message : String) : CodeSignBaseException(message)
\ No newline at end of file
/**
* Copyright (C) 2018 Bonitasoft S.A.
* Bonitasoft, 32 rue Gustave Eiffel - 38000 Grenoble
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 2.0 of the License, or
* (at your option) any later version.
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
package org.bonitasoft.exception
open class CodeSignBaseException(message : String) : Exception(message)
\ No newline at end of file
......@@ -14,5 +14,4 @@
*/
package org.bonitasoft.exception
class SignException(message : String) : Throwable(message){
}
\ No newline at end of file
class SignException(message : String) : CodeSignBaseException(message)
\ No newline at end of file
......@@ -16,128 +16,76 @@ package org.bonitasoft.releng
import org.apache.commons.logging.Log
import org.apache.commons.logging.LogFactory
import org.bonitasoft.exception.BuildDmgException
import org.bonitasoft.exception.SignException
import org.bonitasoft.exception.CodeSignBaseException
import org.bonitasoft.releng.utils.CodeSignFileUtils
import org.springframework.web.bind.annotation.PostMapping
import org.springframework.web.bind.annotation.RequestParam
import org.springframework.web.bind.annotation.RestController
import org.springframework.web.multipart.MultipartFile
import java.io.File
import java.io.FileInputStream
import java.nio.file.Files
import java.nio.file.Paths
import javax.servlet.http.HttpServletResponse
@RestController
class CodeSigningRestController {
class CodeSigningRestController(var windowsCodeSign: WindowsCodeSign, var macCodeSign: MacCodeSign, var codeSignFileUtils: CodeSignFileUtils) {
val logger: Log = LogFactory.getLog(javaClass)
val macOs: String = "Mac OS X"
var windowsCodeSign = WindowsCodeSign()
var macCodeSign = MacCodeSign()
@PostMapping("/sign")
fun signProduct(@RequestParam("exeFile") file: MultipartFile,
response: HttpServletResponse) {
if (file.isEmpty) {
logger.error("File '%s' is empty".format(file.originalFilename))
response.setStatus(400)
response.outputStream.write("File '%s' is empty".format(file.originalFilename).toByteArray())
response.outputStream.close()
return
}
val tempDirectory: File = Files.createDirectory(Paths.get("codesign" + System.currentTimeMillis())).toFile().absoluteFile
try {
val exeCopy: File = Files.createFile(tempDirectory.toPath().resolve(file.originalFilename)).toFile().absoluteFile
logger.info("Copy of input file as $exeCopy....")
file.transferTo(exeCopy)
val fileSigned: File
try {
if (System.getProperty("os.name") == macOs) {
fileSigned = macCodeSign.signMacProduct(exeCopy)
} else {
fileSigned = windowsCodeSign.signWindowsProduct(exeCopy)
}
response.setContentType("application/octet-stream")
response.setHeader("Content-Disposition", "attachment; filename='${fileSigned.name}'")
FileInputStream(fileSigned.canonicalFile).use { inputStream ->
inputStream.copyTo(response.outputStream)
response.outputStream.close()
}
return
} catch (e: SignException) {
logger.error(e)
response.setStatus(500)
response.outputStream.write("An error occured while signing file.".toByteArray())
response.outputStream.close()
return
} finally {
if (!tempDirectory.deleteRecursively()) {
logger.error("$tempDirectory has not been deleted")
}
}
performJob(file, response, this::signProduct)
}
} catch (e: Throwable) {
logger.error("An error occured", e)
// visible for testing
fun signProduct(exeCopy: File): File {
val fileSigned: File
if (System.getProperty("os.name") == macOs) {
fileSigned = macCodeSign.signMacProduct(exeCopy)
} else {
fileSigned = windowsCodeSign.signWindowsProduct(exeCopy)
}
response.outputStream.write("An error occured server side. Check logs.".toByteArray())
response.outputStream.close()
return fileSigned
}
@PostMapping("/buildAndSignMacInstaller")
fun buildAndSignOsxInstaller(@RequestParam("exeFile") file: MultipartFile,
response: HttpServletResponse) {
performJob(file, response, macCodeSign::buildAndSignMacInstaller)
}
private fun performJob(file: MultipartFile, response: HttpServletResponse, codeSignPerformer: (File) -> (File)) {
if (file.isEmpty) {
logger.error("File '%s' is empty".format(file.originalFilename))
response.setStatus(400)
response.status = 400
response.contentType = "text/plain"
response.outputStream.write("File '%s' is empty".format(file.originalFilename).toByteArray())
response.outputStream.close()
return
}
val tempDirectory: File = Files.createDirectory(Paths.get("codesign" + System.currentTimeMillis())).toFile().absoluteFile
val tempDirectory: File = codeSignFileUtils.createTempDirectory()
try {
val exeCopy: File = Files.createFile(tempDirectory.toPath().resolve(file.originalFilename)).toFile().absoluteFile
val exeCopy: File = codeSignFileUtils.createFileWithOriginalName(tempDirectory, file)
logger.info("Copy of input file as $exeCopy....")
file.transferTo(exeCopy)
try {
val fileSigned: File = macCodeSign.buildAndSignMacInstaller(exeCopy)
val fileSigned: File = codeSignPerformer(exeCopy)
codeSignFileUtils.writeFileContentInResponse(fileSigned, response)
response.setContentType("application/octet-stream");
response.setHeader("Content-Disposition", "attachment; filename='${fileSigned.name}'");
FileInputStream(fileSigned.canonicalFile).use { inputStream ->
inputStream.copyTo(response.outputStream)
response.outputStream.close()
}
return
} catch (e: SignException) {
logger.error(e)
response.setStatus(500);
response.outputStream.write("An error occured while signing file.".toByteArray())
response.outputStream.close()
return
} catch (e: BuildDmgException) {
logger.error(e)
response.setStatus(500);
response.outputStream.write("An error occured while building dmg.".toByteArray())
response.outputStream.close()
return
}
response.contentType = "application/octet-stream"
response.setHeader("Content-Disposition", "attachment; filename='${fileSigned.name}'")
response.outputStream.close()
} catch (e: Throwable) {
logger.error("An error occured", e)
logger.error("An error occurred", e)
response.status = 500
response.contentType = "text/plain"
var message = if (e is CodeSignBaseException) e.message else "An error occurred server side. Check logs."
response.outputStream.write(message?.toByteArray())
response.outputStream.close()
} finally {
if (!tempDirectory.deleteRecursively()) {
logger.error("$tempDirectory has not been deleted")
}
}
response.outputStream.write("An error occured server side. Check logs.".toByteArray())
response.outputStream.close()
}
}
\ No newline at end of file
......@@ -18,16 +18,18 @@ import org.apache.commons.logging.Log
import org.apache.commons.logging.LogFactory
import org.bonitasoft.exception.BuildDmgException
import org.bonitasoft.exception.SignException
import org.springframework.stereotype.Service
import java.io.File
import java.io.FileNotFoundException
import java.io.IOException
@Service
class MacCodeSign {
val logger: Log = LogFactory.getLog(javaClass)
private val logger: Log = LogFactory.getLog(javaClass)
val separator = System.getProperty("file.separator")
fun signMacProduct(zipFile: File): File {
val file: File = unzip(zipFile);
val file: File = unzip(zipFile)
if (!zipFile.delete()) {
throw IOException("Failed to delete temp file $zipFile")
}
......@@ -40,16 +42,16 @@ class MacCodeSign {
.start()
.waitFor()
if (signResult != 0) {
throw SignException("An error occured while signing file.")
throw SignException("An error occurred while signing file. Command result: $signResult")
}
var newZipFile = zip(file);
val newZipFile = zip(file)
logger.info("Osx application has been signed successfully")
return newZipFile
}
fun buildAndSignMacInstaller(zipFile: File): File {
val file: File = unzip(zipFile);
val file: File = unzip(zipFile)
if (!zipFile.delete()) {
throw IOException("Failed to delete temp file $zipFile")
}
......@@ -62,21 +64,21 @@ class MacCodeSign {
.start()
.waitFor()
if (signResult != 0) {
throw SignException("An error occured while signing installer.")
throw SignException("An error occurred while signing installer. Command result: $signResult")
}
logger.info("Building dmg...")
var dmgName = file.nameWithoutExtension
var dmgPath = file.parentFile.canonicalPath + separator + file.nameWithoutExtension + ".dmg"
val dmgName = file.nameWithoutExtension
val dmgPath = file.parentFile.canonicalPath + separator + file.nameWithoutExtension + ".dmg"
val buildDmgResult = ProcessBuilder()
.inheritIO()
.directory(file.parentFile)
.command("hdiutil", "create", "-volname", dmgName, "-srcfolder", file.name, "-ov", "-format", "UDZO", dmgName + ".dmg")
.start()
.waitFor()
var dmg = File(dmgPath)
val dmg = File(dmgPath)
if (buildDmgResult != 0 || !dmg.exists()) {
throw BuildDmgException("An error occured while building dmg")
throw BuildDmgException("An error occurred while building dmg. Command result: $buildDmgResult")
}
logger.info("Signing macOS dmg...")
......@@ -87,7 +89,7 @@ class MacCodeSign {
.start()
.waitFor()
if (signResult != 0) {
throw SignException("An error occured while signing dmg.")
throw SignException("An error occurred while signing dmg. Command result: $signResult")
}
logger.info("dmg has been created and signed successfully")
return dmg
......@@ -104,7 +106,7 @@ class MacCodeSign {
throw IOException("failed to unzip $zipFile. 'unzip' command return code: $result")
}
var file: File = File(zipFile.parentFile.canonicalPath + separator + zipFile.nameWithoutExtension)
val file = File(zipFile.parentFile.canonicalPath + separator + zipFile.nameWithoutExtension)
if (!file.exists()) {
throw FileNotFoundException("${zipFile.nameWithoutExtension} not found in ${zipFile.parentFile.canonicalPath}")
}
......
......@@ -17,12 +17,12 @@ package org.bonitasoft.releng
import org.apache.commons.logging.Log
import org.apache.commons.logging.LogFactory
import org.bonitasoft.exception.SignException
import org.springframework.web.multipart.MultipartFile
import org.springframework.stereotype.Service
import java.io.File
import javax.servlet.http.HttpServletResponse
@Service
class WindowsCodeSign {
val logger: Log = LogFactory.getLog(javaClass)
private val logger: Log = LogFactory.getLog(javaClass)
fun signWindowsProduct(file: File) : File {
val processBuilder = ProcessBuilder("signtool", "sign", "/tr", "http://timestamp.digicert.com",
......@@ -30,9 +30,9 @@ class WindowsCodeSign {
logger.info("Signing windows file...")
val signResult = processBuilder.inheritIO().start().waitFor()
if (signResult == 1) {
throw SignException("An error occured while signing file.")
throw SignException("An error occurred while signing file.")
}
return file;
return file
}
}
\ No newline at end of file
/**
* Copyright (C) 2018 Bonitasoft S.A.
* Bonitasoft, 32 rue Gustave Eiffel - 38000 Grenoble
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 2.0 of the License, or
* (at your option) any later version.
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
package org.bonitasoft.releng.utils
import org.springframework.stereotype.Component
import org.springframework.web.multipart.MultipartFile
import java.io.File
import java.io.FileInputStream
import java.nio.file.Files
import java.nio.file.Paths
import javax.servlet.http.HttpServletResponse
@Component
class CodeSignFileUtils {
fun createTempDirectory(): File =
Files.createDirectory(Paths.get("codesign" + System.currentTimeMillis())).toFile().absoluteFile
fun createFileWithOriginalName(tempDirectory: File, file: MultipartFile): File =
Files.createFile(tempDirectory.toPath().resolve(file.originalFilename)).toFile().absoluteFile
fun writeFileContentInResponse(file: File, response: HttpServletResponse) {
FileInputStream(file.canonicalFile).use { inputStream -> inputStream.copyTo(response.outputStream) }
}
}
\ No newline at end of file
......@@ -14,34 +14,143 @@
*/
package org.bonitasoft.releng
import io.mockk.*
import org.assertj.core.api.Assertions.assertThat
import org.bonitasoft.exception.BuildDmgException
import org.bonitasoft.exception.SignException
import org.bonitasoft.releng.utils.CodeSignFileUtils
import org.jetbrains.spek.api.Spek
import org.jetbrains.spek.api.dsl.given
import org.jetbrains.spek.api.dsl.it
import org.jetbrains.spek.api.dsl.on
import org.junit.platform.runner.JUnitPlatform
import org.junit.runner.RunWith
import org.mockito.Mockito
import org.springframework.mock.web.MockHttpServletResponse
import org.springframework.web.multipart.MultipartFile
import kotlin.test.assertEquals
import java.io.File
@RunWith(JUnitPlatform::class)
class CodeSigningRestControllerTest : Spek({
given("a controller and an empty file") {
val controller: CodeSigningRestController = CodeSigningRestController()
val controller = CodeSigningRestController(mockkClass(WindowsCodeSign::class), mockkClass(MacCodeSign::class), mockkClass(CodeSignFileUtils::class))
val response = MockHttpServletResponse()
val file: MultipartFile = Mockito.mock(MultipartFile::class.java)
Mockito.`when`(file.isEmpty).thenReturn(true)
Mockito.`when`(file.originalFilename).thenReturn("fileName")
val file: MultipartFile = mockkClass(MultipartFile::class)
every { file.isEmpty } returns true
every { file.originalFilename } returns "fileName"
on("sending file to controller") {
on("sending file to controller to sign product") {
controller.signProduct(file, response)
it("should set a code 400 on the response") {
assertEquals(response.status, 400)
it("should fail") {
assertThat(response.status).describedAs("response status").isEqualTo(400)
assertThat(response.contentType).describedAs("response content type").isEqualTo("text/plain")
assertThat(response.contentAsString).describedAs("response content").contains("fileName").contains("empty")
}
}
on("sending file to controller to build and sign osx installer") {
controller.buildAndSignOsxInstaller(file, response)
it("should fail") {
assertThat(response.status).describedAs("response status").isEqualTo(400)
assertThat(response.contentType).describedAs("response content type").isEqualTo("text/plain")
assertThat(response.contentAsString).describedAs("response content").contains("fileName").contains("empty")
}
}
}
given("a controller and a non empty file") {
val codeSignFileUtils: CodeSignFileUtils = mockkClass(CodeSignFileUtils::class)
val macCodeSign = mockkClass(MacCodeSign::class)
val controller = spyk(CodeSigningRestController(mockkClass(WindowsCodeSign::class), macCodeSign, codeSignFileUtils))
val response = MockHttpServletResponse()
val file: MultipartFile = mockkClass(MultipartFile::class)
every { file.isEmpty } returns false
every { file.originalFilename } returns "fileName"
every { file.transferTo(any()) } answers { nothing }
val tempDirectory = mockkClass(type = File::class, relaxed = true)
every { tempDirectory.deleteRecursively() } returns true
every { codeSignFileUtils.createTempDirectory() } returns tempDirectory
every { codeSignFileUtils.createFileWithOriginalName(tempDirectory, file) } returns mockkClass(File::class)
every { codeSignFileUtils.writeFileContentInResponse(any(), response) } just Runs
on("sending file to controller to sign product when signing succeed") {
every { controller.signProduct(any()) } returns mockkClass(type = File::class, relaxed = true)
controller.signProduct(file, response)
it("should succeed") {
assertThat(response.status).describedAs("response status").isEqualTo(200)
assertThat(response.contentType).describedAs("response content type").isEqualTo("application/octet-stream")
assertThat(response.getHeader("Content-Disposition")).describedAs("response content disposition").contains("attachment; filename=")
}
}
on("sending file to controller to sign product when signing fail") {
every { controller.signProduct(any()) } throws SignException("sign fail")
response.reset()
controller.signProduct(file, response)
it("should fail") {
assertThat(response.status).describedAs("response status").isEqualTo(500)
assertThat(response.contentAsString).describedAs("response content").isEqualTo("sign fail")
assertThat(response.contentType).describedAs("response content type").isEqualTo("text/plain")
}
}
on("sending file to controller to sign product when uncovered error occurs") {
every { controller.signProduct(any()) } throws RuntimeException("weird windows error")
response.reset()
controller.signProduct(file, response)
it("should fail") {
assertThat(response.status).describedAs("response status").isEqualTo(500)
assertThat(response.contentAsString).describedAs("response content").isEqualTo("An error occurred server side. Check logs.")
assertThat(response.contentType).describedAs("response content type").isEqualTo("text/plain")
}
}
on("sending file to controller to build and sign osx installer when signing succeed") {
every { macCodeSign.buildAndSignMacInstaller(any()) } returns mockkClass(type = File::class, relaxed = true)
response.reset()
controller.buildAndSignOsxInstaller(file, response)
it("should succeed") {
assertThat(response.status).describedAs("response status").isEqualTo(200)
assertThat(response.contentType).describedAs("response content type").isEqualTo("application/octet-stream")
assertThat(response.getHeader("Content-Disposition")).describedAs("response content disposition").contains("attachment; filename=")
}
}
on("sending file to controller to build and sign osx installer when building fail") {
every { macCodeSign.buildAndSignMacInstaller(any()) } throws BuildDmgException("build fail")
response.reset()
controller.buildAndSignOsxInstaller(file, response)
it("should fail") {
assertThat(response.status).describedAs("response status").isEqualTo(500)
assertThat(response.contentAsString).describedAs("response content").isEqualTo("build fail")
assertThat(response.contentType).describedAs("response content type").isEqualTo("text/plain")
}
}
on("sending file to controller to build and sign osx installer when uncovered error occurs") {
every { macCodeSign.buildAndSignMacInstaller(any()) } throws RuntimeException("weird mac error")
response.reset()
controller.buildAndSignOsxInstaller(file, response)
it("should fail") {
assertThat(response.status).describedAs("response status").isEqualTo(500)
assertThat(response.contentAsString).describedAs("response content").isEqualTo("An error occurred server side. Check logs.")
assertThat(response.contentType).describedAs("response content type").isEqualTo("text/plain")
}
}
}
})
\ No newline at end of file
<configuration>
<appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
<!-- encoders are assigned the type ch.qos.logback.classic.encoder.PatternLayoutEncoder by default -->
<encoder>
<pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{16} %msg%n</pattern>
</encoder>
</appender>
<logger name="org.bonitasoft" level="INFO" />
<!--<logger name="io.mockk" level="DEBUG" />-->
<root level="WARN">
<appender-ref ref="STDOUT" />
</root>
</configuration>
\ No newline at end of file
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment