From 111233b71060431bdae65d2ddf4bc5126eb1c40b Mon Sep 17 00:00:00 2001 From: c0llab0rat0r <78339685+c0llab0rat0r@users.noreply.github.com> Date: Mon, 5 Apr 2021 02:36:15 -0500 Subject: [PATCH 1/3] Remove backup api at /database/backup; change method from GET to POST on /api/v3/plugins/database/backup --- README.MD | 18 ++++------- build.sbt | 8 +++++ .../h2/controller/H2BackupController.scala | 24 ++++++++------- .../brouillard/gitbucket/h2/export.scala.html | 2 +- .../controller/H2BackupControllerTests.scala | 30 +++++++++++++++++++ 5 files changed, 57 insertions(+), 25 deletions(-) create mode 100644 src/test/scala/fr/brouillard/gitbucket/h2/controller/H2BackupControllerTests.scala diff --git a/README.MD b/README.MD index 09dd0ed..b3038ca 100644 --- a/README.MD +++ b/README.MD @@ -83,6 +83,11 @@ sbt clean assembly ## Release Notes +### Unreleased +- remove backup api at GET /database/backup +- change method from GET to POST on /api/v3/plugins/database/backup +- backup endpoint is secure by default, and requires an api token for a user with admin rights + ### 1.9.0 - compatibility with GitBucket 4.35.x @@ -102,7 +107,6 @@ sbt clean assembly - compatibility with GitBucket 4.10, scala 2.12 [#20](https://github.com/gitbucket-plugins/gitbucket-h2-backup-plugin/issues/20) - allow to secure `database/backup` endpoint [#1](https://github.com/gitbucket-plugins/gitbucket-h2-backup-plugin/issues/1),[#19](https://github.com/gitbucket-plugins/gitbucket-h2-backup-plugin/issues/19) - see [Securing backup endpoint](#securing-backup-endpoint) paragraph ### 1.3.0 @@ -122,15 +126,3 @@ sbt clean assembly - introduce gitbucket-h2-backup-plugin - allows to backup h2 database via a live dump - -## Securing backup endpoint - -In version 1.4.0, it is possible to secure the `database/backup` endpoint: - -- launch GitBucket with System property _secure.backup_ set to true (for example `-Dsecure.backup=true` on the command line) -- due to actual limitations of GitBucket & plugins security, once the previous setting is activated, -a call to `http://YOUR_GITBUCKET/database/backup` will be temporary redirected `http://YOUR_GITBUCKET/api/v3/plugins/database/backup`. -You have to follow this temporary redirection. - - if you call the endpoint using _httpie_, use the `--follow` parameter -- this secured endpoint route is TEMPORARY you should not call it directly. -If you do think that it will change in the future when GitBucket will support secured routes for plugins. diff --git a/build.sbt b/build.sbt index c7149e2..290e3d6 100644 --- a/build.sbt +++ b/build.sbt @@ -1,6 +1,14 @@ +val ScalatraVersion = "2.7.1" + organization := "fr.brouillard.gitbucket" name := "gitbucket-h2-backup-plugin" version := "1.9.0" scalaVersion := "2.13.3" gitbucketVersion := "4.35.0" scalacOptions += "-deprecation" + +libraryDependencies ++= Seq( + "org.scalatest" %% "scalatest-funspec" % "3.2.3" % "test", + "org.scalatest" %% "scalatest-funsuite" % "3.2.3" % "test", + "org.scalatra" %% "scalatra-scalatest" % ScalatraVersion % "test", +) diff --git a/src/main/scala/fr/brouillard/gitbucket/h2/controller/H2BackupController.scala b/src/main/scala/fr/brouillard/gitbucket/h2/controller/H2BackupController.scala index ab2a63f..eca2823 100644 --- a/src/main/scala/fr/brouillard/gitbucket/h2/controller/H2BackupController.scala +++ b/src/main/scala/fr/brouillard/gitbucket/h2/controller/H2BackupController.scala @@ -2,17 +2,13 @@ package fr.brouillard.gitbucket.h2.controller import java.io.File import java.util.Date - import fr.brouillard.gitbucket.h2._ - import gitbucket.core.controller.ControllerBase import gitbucket.core.util.AdminAuthenticator import gitbucket.core.util.Directory._ import gitbucket.core.servlet.Database - -import org.scalatra.Ok +import org.scalatra.{ActionResult, Ok} import org.slf4j.LoggerFactory - import org.scalatra.forms._ class H2BackupController extends ControllerBase with AdminAuthenticator { @@ -42,26 +38,32 @@ class H2BackupController extends ControllerBase with AdminAuthenticator { }) get("/api/v3/plugins/database/backup") { + doBackupMoved() + } + + post("/api/v3/plugins/database/backup") { context.loginAccount match { case Some(x) if (x.isAdmin) => doExport() case _ => org.scalatra.Unauthorized() } } + // Legacy api that was insecure/open by default get("/database/backup") { - if (sys.props.get("secure.backup") exists (_ equalsIgnoreCase "true")) - org.scalatra.TemporaryRedirect("/api/v3/plugins/database/backup?dest=" + params.getOrElse("dest", defaultBackupFileName())) - else { - doExport() - } + doBackupMoved() + } + + private def doBackupMoved(): ActionResult = { + org.scalatra.MethodNotAllowed("This has moved to POST /api/v3/plugins/database/backup") } - private def doExport(): Unit = { + private def doExport(): ActionResult = { val filePath: String = params.getOrElse("dest", defaultBackupFileName()) exportDatabase(new File(filePath)) Ok("done: " + filePath) } + // Responds to a form post from a web page post("/database/backup", backupForm) { form: BackupForm => exportDatabase(new File(form.destFile)) val msg: String = "H2 Database has been exported to '" + form.destFile + "'." diff --git a/src/main/twirl/fr/brouillard/gitbucket/h2/export.scala.html b/src/main/twirl/fr/brouillard/gitbucket/h2/export.scala.html index 4125da4..0bd500a 100644 --- a/src/main/twirl/fr/brouillard/gitbucket/h2/export.scala.html +++ b/src/main/twirl/fr/brouillard/gitbucket/h2/export.scala.html @@ -15,7 +15,7 @@

- Allows to export/backup the H2 database content. The same action can be achieved via an HTTP GET call to @path/database/backup . + Allows to export/backup the H2 database content. The same action can be achieved via an HTTP POST call to @path/api/v3/plugins/database/backup .

backup/
diff --git a/src/test/scala/fr/brouillard/gitbucket/h2/controller/H2BackupControllerTests.scala b/src/test/scala/fr/brouillard/gitbucket/h2/controller/H2BackupControllerTests.scala new file mode 100644 index 0000000..af8e318 --- /dev/null +++ b/src/test/scala/fr/brouillard/gitbucket/h2/controller/H2BackupControllerTests.scala @@ -0,0 +1,30 @@ +package fr.brouillard.gitbucket.h2.controller + +import gitbucket.core.servlet.ApiAuthenticationFilter +import org.scalatra.test.scalatest.ScalatraFunSuite + +class H2BackupControllerTests extends ScalatraFunSuite { + addFilter(classOf[ApiAuthenticationFilter], path="/api/*") + addFilter(classOf[H2BackupController], "/*") + + test("get database backup api") { + get("/api/v3/plugins/database/backup") { + status should equal (405) + body should include ("This has moved") + } + } + + test("get database backup legacy") { + get("/database/backup") { + status should equal (405) + body should include ("This has moved") + } + } + + test("post database backup without credentials is unauthorized") { + post("/api/v3/plugins/database/backup") { + status should equal (401) + } + } + +} From 49c02c16672c49c3c66c59c245555e0e6e757e68 Mon Sep 17 00:00:00 2001 From: c0llab0rat0r <78339685+c0llab0rat0r@users.noreply.github.com> Date: Sat, 17 Apr 2021 13:15:04 -0500 Subject: [PATCH 2/3] Inline H2BackupController::doExport --- .../h2/controller/H2BackupController.scala | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/main/scala/fr/brouillard/gitbucket/h2/controller/H2BackupController.scala b/src/main/scala/fr/brouillard/gitbucket/h2/controller/H2BackupController.scala index eca2823..9c56c0e 100644 --- a/src/main/scala/fr/brouillard/gitbucket/h2/controller/H2BackupController.scala +++ b/src/main/scala/fr/brouillard/gitbucket/h2/controller/H2BackupController.scala @@ -23,7 +23,7 @@ class H2BackupController extends ControllerBase with AdminAuthenticator { // private val defaultBackupFile:String = new File(GitBucketHome, "gitbucket-database-backup.zip").toString; def exportDatabase(exportFile: File): Unit = { - val destFile = if (exportFile.isAbsolute()) exportFile else new File(GitBucketHome + "/backup", exportFile.toString) + val destFile = if (exportFile.isAbsolute) exportFile else new File(GitBucketHome + "/backup", exportFile.toString) val session = Database.getSession(request) val conn = session.conn @@ -43,7 +43,10 @@ class H2BackupController extends ControllerBase with AdminAuthenticator { post("/api/v3/plugins/database/backup") { context.loginAccount match { - case Some(x) if (x.isAdmin) => doExport() + case Some(x) if x.isAdmin => + val filePath: String = params.getOrElse("dest", defaultBackupFileName()) + exportDatabase(new File(filePath)) + Ok("done: " + filePath) case _ => org.scalatra.Unauthorized() } } @@ -57,12 +60,6 @@ class H2BackupController extends ControllerBase with AdminAuthenticator { org.scalatra.MethodNotAllowed("This has moved to POST /api/v3/plugins/database/backup") } - private def doExport(): ActionResult = { - val filePath: String = params.getOrElse("dest", defaultBackupFileName()) - exportDatabase(new File(filePath)) - Ok("done: " + filePath) - } - // Responds to a form post from a web page post("/database/backup", backupForm) { form: BackupForm => exportDatabase(new File(form.destFile)) From fc177816bad89c807d4635cea2a7738458353f6b Mon Sep 17 00:00:00 2001 From: c0llab0rat0r <78339685+c0llab0rat0r@users.noreply.github.com> Date: Sat, 17 Apr 2021 14:33:28 -0500 Subject: [PATCH 3/3] Remove "done: " prefix from backup response so its easier to use the response in scripts; add tests around authorized and unauthorized requests and using a non-default file name --- README.MD | 2 +- .../h2/controller/H2BackupController.scala | 33 ++++--- .../controller/H2BackupControllerTests.scala | 93 +++++++++++++++++++ 3 files changed, 115 insertions(+), 13 deletions(-) diff --git a/README.MD b/README.MD index b3038ca..4d18ac5 100644 --- a/README.MD +++ b/README.MD @@ -40,7 +40,7 @@ You can pass an optional argument `dest` that references the destination file pa For example calling `http://YOUR_GITBUCKET/api/v3/plugins/database/backup?dest=/var/backups/gitbucket.zip` will do an H2 backup of the gitbucket database into the file `/var/backups/gitbucket.zip`. Since `1.3.0`, the _dest_ parameter can denote a relative file path, in this case the file will be created relatively to `GITBUCKET_DATA`. -On success, you will receive a `HTTP 200` answer with a body containing `done: FULL_PATH_OF_BACKUP_FILE`. +On success, you will receive a `HTTP 200` answer with a `text/plain` body containing `FULL_PATH_OF_BACKUP_FILE`. ### HTTP API Authorization diff --git a/src/main/scala/fr/brouillard/gitbucket/h2/controller/H2BackupController.scala b/src/main/scala/fr/brouillard/gitbucket/h2/controller/H2BackupController.scala index 9c56c0e..97b0716 100644 --- a/src/main/scala/fr/brouillard/gitbucket/h2/controller/H2BackupController.scala +++ b/src/main/scala/fr/brouillard/gitbucket/h2/controller/H2BackupController.scala @@ -3,14 +3,33 @@ package fr.brouillard.gitbucket.h2.controller import java.io.File import java.util.Date import fr.brouillard.gitbucket.h2._ +import fr.brouillard.gitbucket.h2.controller.H2BackupController.{defaultBackupFileName, doBackup} import gitbucket.core.controller.ControllerBase +import gitbucket.core.model.Account import gitbucket.core.util.AdminAuthenticator import gitbucket.core.util.Directory._ import gitbucket.core.servlet.Database -import org.scalatra.{ActionResult, Ok} +import org.scalatra.{ActionResult, Ok, Params} import org.slf4j.LoggerFactory import org.scalatra.forms._ +object H2BackupController { + def defaultBackupFileName(): String = { + val format = new java.text.SimpleDateFormat("yyyy-MM-dd_HH-mm") + "gitbucket-db-" + format.format(new Date()) + ".zip" + } + + def doBackup(exportDatabase: File => Unit, loginAccount: Option[Account], params: Params): ActionResult = { + loginAccount match { + case Some(x) if x.isAdmin => + val filePath: String = params.getOrElse("dest", defaultBackupFileName()) + exportDatabase(new File(filePath)) + Ok(filePath, Map("Content-Type" -> "text/plain")) + case _ => org.scalatra.Unauthorized() + } + } +} + class H2BackupController extends ControllerBase with AdminAuthenticator { private val logger = LoggerFactory.getLogger(classOf[H2BackupController]) @@ -42,13 +61,7 @@ class H2BackupController extends ControllerBase with AdminAuthenticator { } post("/api/v3/plugins/database/backup") { - context.loginAccount match { - case Some(x) if x.isAdmin => - val filePath: String = params.getOrElse("dest", defaultBackupFileName()) - exportDatabase(new File(filePath)) - Ok("done: " + filePath) - case _ => org.scalatra.Unauthorized() - } + doBackup(exportDatabase, context.loginAccount, params) } // Legacy api that was insecure/open by default @@ -69,8 +82,4 @@ class H2BackupController extends ControllerBase with AdminAuthenticator { redirect("/admin/h2backup") } - private def defaultBackupFileName(): String = { - val format = new java.text.SimpleDateFormat("yyyy-MM-dd_HH-mm") - "gitbucket-db-" + format.format(new Date()) + ".zip" - } } diff --git a/src/test/scala/fr/brouillard/gitbucket/h2/controller/H2BackupControllerTests.scala b/src/test/scala/fr/brouillard/gitbucket/h2/controller/H2BackupControllerTests.scala index af8e318..37df236 100644 --- a/src/test/scala/fr/brouillard/gitbucket/h2/controller/H2BackupControllerTests.scala +++ b/src/test/scala/fr/brouillard/gitbucket/h2/controller/H2BackupControllerTests.scala @@ -1,8 +1,15 @@ package fr.brouillard.gitbucket.h2.controller +import gitbucket.core.model.Account import gitbucket.core.servlet.ApiAuthenticationFilter +import org.scalatest.funsuite.AnyFunSuite +import org.scalatest.matchers.should.Matchers.{convertToAnyShouldWrapper, equal} +import org.scalatra.{Ok, Params, ScalatraParams} import org.scalatra.test.scalatest.ScalatraFunSuite +import java.io.File +import java.util.Date + class H2BackupControllerTests extends ScalatraFunSuite { addFilter(classOf[ApiAuthenticationFilter], path="/api/*") addFilter(classOf[H2BackupController], "/*") @@ -28,3 +35,89 @@ class H2BackupControllerTests extends ScalatraFunSuite { } } + +class H2BackupControllerObjectTests extends AnyFunSuite { + private def assertDefaultFileName(name: String): Unit = { + assert(name.startsWith("gitbucket-db")) + assert(name.endsWith(".zip")) + } + + private def buildAccount(isAdmin: Boolean) = { + Account( + userName = "a", + fullName = "b", + mailAddress = "c", + password = "d", + isAdmin = isAdmin, + url = None, + registeredDate = new Date(), + updatedDate = new Date(), + lastLoginDate = None, + image = None, + isGroupAccount = false, + isRemoved = false, + description = None) + } + + test("generates default file name") { + assertDefaultFileName(H2BackupController.defaultBackupFileName()) + } + + test("post database backup with admin credentials is executed with default file name") { + val account = buildAccount(true) + val params: Params = new ScalatraParams(Map()) + + var executed = false; + + val exportDatabase = (file: File) => { + assert(!executed) + assertDefaultFileName(file.getName) + + executed = true + } + + val action = H2BackupController.doBackup(exportDatabase, Some(account), params) + + assert(executed) + assert(action.status == 200) + + // Not JSON and not HTML + assert(action.headers.get("Content-Type").contains("text/plain")) + } + + test("post database backup with admin credentials is executed with specific file name") { + val fileName = "foo.zip" + val account = buildAccount(true) + val params: Params = new ScalatraParams(Map("dest" -> Seq(fileName))) + + var executed = false; + + val exportDatabase = (file: File) => { + assert(!executed) + assert(file.getName.equals(fileName)) + + executed = true + } + + val action = H2BackupController.doBackup(exportDatabase, Some(account), params) + + assert(executed) + assert(action.status == 200) + + // Not JSON and not HTML + assert(action.headers.get("Content-Type").contains("text/plain")) + } + + test("post database backup with unprivileged credentials is unauthorized") { + val account = buildAccount(false) + val params: Params = new ScalatraParams(Map()) + + val exportDatabase = (file: File) => { + fail() + } + + val action = H2BackupController.doBackup(exportDatabase, Some(account), params) + assert(action.status == 401) + } + +}