Fix repository creation permission check (#2725)

2025-11-04 20:45:58 +01:00 · 2021-04-04 10:22:34 +09:00
parent 0d2c156579
commit 43c629b3e5
4 changed files with 55 additions and 42 deletions
--- a/src/main/scala/gitbucket/core/controller/AccountController.scala
+++ b/src/main/scala/gitbucket/core/controller/AccountController.scala
@@ -725,7 +725,14 @@ trait AccountControllerBase extends AccountManagementControllerBase {
  post("/new", newRepositoryForm)(usersOnly { form =>
    if (context.settings.repositoryOperation.create || context.loginAccount.get.isAdmin) {
      LockUtil.lock(s"${form.owner}/${form.name}") {
-        if (getRepository(form.owner, form.name).isEmpty) {
+        if (getRepository(form.owner, form.name).isDefined) {
+          // redirect to the repository if repository already exists
+          redirect(s"/${form.owner}/${form.name}")
+        } else if (!canCreateRepository(form.owner, context.loginAccount.get)) {
+          // Permission error
+          Forbidden()
+        } else {
+          // create repository asynchronously
          createRepository(
            context.loginAccount.get,
            form.owner,
@@ -735,10 +742,10 @@ trait AccountControllerBase extends AccountManagementControllerBase {
            form.initOption,
            form.sourceUrl
          )
-        }
-      }
          // redirect to the repository
          redirect(s"/${form.owner}/${form.name}")
+        }
+      }
    } else Forbidden()
  })

@@ -773,10 +780,12 @@ trait AccountControllerBase extends AccountManagementControllerBase {
      val loginUserName = loginAccount.userName
      val accountName = form.accountName

-      if (getRepository(accountName, repository.name).isDefined ||
-          (accountName != loginUserName && !getGroupsByUserName(loginUserName).contains(accountName))) {
+      if (getRepository(accountName, repository.name).isDefined) {
        // redirect to the repository if repository already exists
        redirect(s"/${accountName}/${repository.name}")
+      } else if (!canCreateRepository(accountName, loginAccount)) {
+        // Permission error
+        Forbidden()
      } else {
        // fork repository asynchronously
        forkRepository(accountName, repository, loginUserName)
--- a/src/main/scala/gitbucket/core/controller/api/ApiRepositoryControllerBase.scala
+++ b/src/main/scala/gitbucket/core/controller/api/ApiRepositoryControllerBase.scala
@@ -8,6 +8,7 @@ import gitbucket.core.util._
 import gitbucket.core.util.Implicits._
 import gitbucket.core.model.Profile.profile.blockingApi._
 import org.eclipse.jgit.api.Git
+import org.scalatra.Forbidden

 import scala.concurrent.Await
 import scala.concurrent.duration.Duration
@@ -26,7 +27,7 @@ trait ApiRepositoryControllerBase extends ControllerBase {

  /**
   * i. List your repositories
-   * https://developer.github.com/v3/repos/#list-your-repositories
+   * https://docs.github.com/en/rest/reference/repos#list-repositories-for-the-authenticated-user
   */
  get("/api/v3/user/repos")(usersOnly {
    JsonFormat(getVisibleRepositories(context.loginAccount, Option(context.loginAccount.get.userName)).map { r =>
@@ -36,7 +37,7 @@ trait ApiRepositoryControllerBase extends ControllerBase {

  /**
   * ii. List user repositories
-   * https://developer.github.com/v3/repos/#list-user-repositories
+   * https://docs.github.com/en/rest/reference/repos#list-repositories-for-a-user
   */
  get("/api/v3/users/:userName/repos") {
    JsonFormat(getVisibleRepositories(context.loginAccount, Some(params("userName"))).map { r =>
@@ -46,7 +47,7 @@ trait ApiRepositoryControllerBase extends ControllerBase {

  /**
   * iii. List organization repositories
-   * https://developer.github.com/v3/repos/#list-organization-repositories
+   * https://docs.github.com/en/rest/reference/repos#list-organization-repositories
   */
  get("/api/v3/orgs/:orgName/repos") {
    JsonFormat(getVisibleRepositories(context.loginAccount, Some(params("orgName"))).map { r =>
@@ -56,7 +57,7 @@ trait ApiRepositoryControllerBase extends ControllerBase {

  /**
   * iv. List all public repositories
-   * https://developer.github.com/v3/repos/#list-public-repositories
+   * https://docs.github.com/en/rest/reference/repos#list-public-repositories
   */
  get("/api/v3/repositories") {
    JsonFormat(getPublicRepositories().map { r =>
@@ -66,13 +67,12 @@ trait ApiRepositoryControllerBase extends ControllerBase {

  /*
   * v. Create
-   * https://developer.github.com/v3/repos/#create
   * Implemented with two methods (user/orgs)
   */

  /**
   * Create user repository
-   * https://developer.github.com/v3/repos/#create
+   * https://docs.github.com/en/rest/reference/repos#create-a-repository-for-the-authenticated-user
   */
  post("/api/v3/user/repos")(usersOnly {
    val owner = context.loginAccount.get.userName
@@ -80,7 +80,12 @@ trait ApiRepositoryControllerBase extends ControllerBase {
      data <- extractFromJsonBody[CreateARepository] if data.isValid
    } yield {
      LockUtil.lock(s"${owner}/${data.name}") {
-        if (getRepository(owner, data.name).isEmpty) {
+        if (getRepository(owner, data.name).isDefined) {
+          ApiError(
+            "A repository with this name already exists on this account",
+            Some("https://developer.github.com/v3/repos/#create")
+          )
+        } else {
          val f = createRepository(
            context.loginAccount.get,
            owner,
@@ -95,11 +100,6 @@ trait ApiRepositoryControllerBase extends ControllerBase {
            getRepository(owner, data.name)(session).get
          }
          JsonFormat(ApiRepository(repository, ApiUser(getAccountByUserName(owner).get)))
-        } else {
-          ApiError(
-            "A repository with this name already exists on this account",
-            Some("https://developer.github.com/v3/repos/#create")
-          )
        }
      }
    }) getOrElse NotFound()
@@ -107,15 +107,22 @@ trait ApiRepositoryControllerBase extends ControllerBase {

  /**
   * Create group repository
-   * https://developer.github.com/v3/repos/#create
+   * https://docs.github.com/en/rest/reference/repos#create-an-organization-repository
   */
-  post("/api/v3/orgs/:org/repos")(managersOnly {
+  post("/api/v3/orgs/:org/repos")(usersOnly {
    val groupName = params("org")
    (for {
      data <- extractFromJsonBody[CreateARepository] if data.isValid
    } yield {
      LockUtil.lock(s"${groupName}/${data.name}") {
-        if (getRepository(groupName, data.name).isEmpty) {
+        if (getRepository(groupName, data.name).isDefined) {
+          ApiError(
+            "A repository with this name already exists for this group",
+            Some("https://developer.github.com/v3/repos/#create")
+          )
+        } else if (!canCreateRepository(groupName, context.loginAccount.get)) {
+          Forbidden()
+        } else {
          val f = createRepository(
            context.loginAccount.get,
            groupName,
@@ -129,11 +136,6 @@ trait ApiRepositoryControllerBase extends ControllerBase {
            getRepository(groupName, data.name).get
          }
          JsonFormat(ApiRepository(repository, ApiUser(getAccountByUserName(groupName).get)))
-        } else {
-          ApiError(
-            "A repository with this name already exists for this group",
-            Some("https://developer.github.com/v3/repos/#create")
-          )
        }
      }
    }) getOrElse NotFound()
@@ -141,7 +143,7 @@ trait ApiRepositoryControllerBase extends ControllerBase {

  /*
   * vi. Get
-   * https://developer.github.com/v3/repos/#get
+   * https://docs.github.com/en/rest/reference/repos#get-a-repository
   */
  get("/api/v3/repos/:owner/:repository")(referrersOnly { repository =>
    JsonFormat(ApiRepository(repository, ApiUser(getAccountByUserName(repository.owner).get)))
@@ -149,32 +151,32 @@ trait ApiRepositoryControllerBase extends ControllerBase {

  /*
   * vii. Edit
-   * https://developer.github.com/v3/repos/#edit
+   * https://docs.github.com/en/rest/reference/repos#update-a-repository
   */

  /*
   * viii. List all topics for a repository
-   * https://developer.github.com/v3/repos/#list-all-topics-for-a-repository
+   * https://docs.github.com/en/rest/reference/repos#get-all-repository-topics
   */

  /*
   * ix. Replace all topics for a repository
-   * https://developer.github.com/v3/repos/#replace-all-topics-for-a-repository
+   * https://docs.github.com/en/rest/reference/repos#replace-all-repository-topics
   */

  /*
   * x. List contributors
-   * https://developer.github.com/v3/repos/#list-contributors
+   * https://docs.github.com/en/rest/reference/repos#list-repository-contributors
   */

  /*
   * xi. List languages
-   * https://developer.github.com/v3/repos/#list-languages
+   * https://docs.github.com/en/rest/reference/repos#list-repository-languages
   */

  /*
   * xii. List teams
-   * https://developer.github.com/v3/repos/#list-teams
+   * https://docs.github.com/en/rest/reference/repos#list-repository-teams
   */

  /*
@@ -189,12 +191,12 @@ trait ApiRepositoryControllerBase extends ControllerBase {

  /*
   * xiv. Delete a repository
-   * https://developer.github.com/v3/repos/#delete-a-repository
+   * https://docs.github.com/en/rest/reference/repos#delete-a-repository
   */

  /*
   * xv. Transfer a repository
-   * https://developer.github.com/v3/repos/#transfer-a-repository
+   * https://docs.github.com/en/rest/reference/repos#transfer-a-repository
   */

  /**
--- a/src/main/scala/gitbucket/core/service/RepositoryCreationService.scala
+++ b/src/main/scala/gitbucket/core/service/RepositoryCreationService.scala
@@ -53,6 +53,11 @@ trait RepositoryCreationService {
    with ActivityService
    with PrioritiesService =>

+  def canCreateRepository(repositoryOwner: String, loginAccount: Account)(implicit session: Session): Boolean = {
+    repositoryOwner == loginAccount.userName || getGroupsByUserName(loginAccount.userName)
+      .contains(repositoryOwner) || loginAccount.isAdmin
+  }
+
  def createRepository(
    loginAccount: Account,
    owner: String,
@@ -76,7 +81,7 @@ trait RepositoryCreationService {
    RepositoryCreationService.startCreation(owner, name)
    try {
      Database() withTransaction { implicit session =>
-        val ownerAccount = getAccountByUserName(owner).get
+        //val ownerAccount = getAccountByUserName(owner).get
        val loginUserName = loginAccount.userName

        val copyRepositoryDir = if (initOption == "COPY") {
--- a/src/main/scala/gitbucket/core/util/Authenticator.scala
+++ b/src/main/scala/gitbucket/core/util/Authenticator.scala
@@ -113,13 +113,10 @@ trait ReadableUsersAuthenticator { self: ControllerBase with RepositoryService w
    val userName = params("owner")
    val repoName = params("repository")
    getRepository(userName, repoName).map { repository =>
-      context.loginAccount match {
-        case Some(x) if (x.isAdmin)                                                          => action(repository)
-        case Some(x) if (!repository.repository.isPrivate)                                   => action(repository)
-        case Some(x) if (userName == x.userName)                                             => action(repository)
-        case Some(x) if (getGroupMembers(repository.owner).exists(_.userName == x.userName)) => action(repository)
-        case Some(x) if (getCollaboratorUserNames(userName, repoName).contains(x.userName))  => action(repository)
-        case _                                                                               => Unauthorized()
+      if (isReadable(repository.repository, context.loginAccount) || !repository.repository.isPrivate) {
+        action(repository)
+      } else {
+        Unauthorized()
      }
    } getOrElse NotFound()
  }