From 60ff046823996736698afc3c84634854ee7d3747 Mon Sep 17 00:00:00 2001 From: Naoki Takezoe Date: Mon, 31 Oct 2016 18:18:24 +0900 Subject: [PATCH 01/14] (refs #1286) Prototyping of new permission system --- .../resources/update/gitbucket-core_4.7.sql | 2 + .../resources/update/gitbucket-core_4.7.xml | 9 ++++ .../gitbucket/core/GitBucketCoreModule.scala | 4 ++ .../core/controller/IndexController.scala | 6 ++- .../RepositorySettingsController.scala | 12 +++--- .../core/service/RepositoryService.scala | 42 ++++++++++--------- .../gitbucket/core/util/Authenticator.scala | 29 +++++++------ .../gitbucket/core/helper/account.scala.html | 4 +- .../core/settings/collaborators.scala.html | 12 +----- 9 files changed, 69 insertions(+), 51 deletions(-) create mode 100644 src/main/resources/update/gitbucket-core_4.7.sql create mode 100644 src/main/resources/update/gitbucket-core_4.7.xml diff --git a/src/main/resources/update/gitbucket-core_4.7.sql b/src/main/resources/update/gitbucket-core_4.7.sql new file mode 100644 index 000000000..ef13c706b --- /dev/null +++ b/src/main/resources/update/gitbucket-core_4.7.sql @@ -0,0 +1,2 @@ +-- DELETE COLLABORATORS IN GROUP REPOSITORIES +DELETE FROM COLLABORATOR WHERE USER_NAME IN (SELECT USER_NAME FROM ACCOUNT WHERE GROUP_ACCOUNT = TRUE) diff --git a/src/main/resources/update/gitbucket-core_4.7.xml b/src/main/resources/update/gitbucket-core_4.7.xml new file mode 100644 index 000000000..c46a28ef9 --- /dev/null +++ b/src/main/resources/update/gitbucket-core_4.7.xml @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/src/main/scala/gitbucket/core/GitBucketCoreModule.scala b/src/main/scala/gitbucket/core/GitBucketCoreModule.scala index 6830d2d67..c04a68787 100644 --- a/src/main/scala/gitbucket/core/GitBucketCoreModule.scala +++ b/src/main/scala/gitbucket/core/GitBucketCoreModule.scala @@ -18,5 +18,9 @@ object GitBucketCoreModule extends Module("gitbucket-core", new Version("4.5.0"), new Version("4.6.0", new LiquibaseMigration("update/gitbucket-core_4.6.xml") + ), + new Version("4.7.0", + new LiquibaseMigration("update/gitbucket-core_4.7.xml"), + new SqlMigration("update/gitbucket-core_4.7.sql") ) ) diff --git a/src/main/scala/gitbucket/core/controller/IndexController.scala b/src/main/scala/gitbucket/core/controller/IndexController.scala index 6bc27515a..c5a17bc2a 100644 --- a/src/main/scala/gitbucket/core/controller/IndexController.scala +++ b/src/main/scala/gitbucket/core/controller/IndexController.scala @@ -109,7 +109,11 @@ trait IndexControllerBase extends ControllerBase { get("/_user/proposals")(usersOnly { contentType = formats("json") org.json4s.jackson.Serialization.write( - Map("options" -> getAllUsers(false).filter(!_.isGroupAccount).map(_.userName).toArray) + Map("options" -> (if(params.get("userOnly").isDefined) { + getAllUsers(false).filter(!_.isGroupAccount).map(_.userName).toArray + } else { + getAllUsers(false).map(_.userName).toArray + })) ) }) diff --git a/src/main/scala/gitbucket/core/controller/RepositorySettingsController.scala b/src/main/scala/gitbucket/core/controller/RepositorySettingsController.scala index 41455cc08..cceb9a603 100644 --- a/src/main/scala/gitbucket/core/controller/RepositorySettingsController.scala +++ b/src/main/scala/gitbucket/core/controller/RepositorySettingsController.scala @@ -182,7 +182,7 @@ trait RepositorySettingsControllerBase extends ControllerBase { * Add the collaborator. */ post("/:owner/:repository/settings/collaborators/add", collaboratorForm)(ownerOnly { (form, repository) => - if(!getAccountByUserName(repository.owner).get.isGroupAccount){ + getAccountByUserName(repository.owner).foreach { _ => addCollaborator(repository.owner, repository.name, form.userName) } redirect(s"/${repository.owner}/${repository.name}/settings/collaborators") @@ -192,9 +192,7 @@ trait RepositorySettingsControllerBase extends ControllerBase { * Add the collaborator. */ get("/:owner/:repository/settings/collaborators/remove")(ownerOnly { repository => - if(!getAccountByUserName(repository.owner).get.isGroupAccount){ - removeCollaborator(repository.owner, repository.name, params("name")) - } + removeCollaborator(repository.owner, repository.name, params("name")) redirect(s"/${repository.owner}/${repository.name}/settings/collaborators") }) @@ -404,10 +402,10 @@ trait RepositorySettingsControllerBase extends ControllerBase { override def validate(name: String, value: String, messages: Messages): Option[String] = getAccountByUserName(value) match { case None => Some("User does not exist.") - case Some(x) if(x.isGroupAccount) - => Some("User does not exist.") +// case Some(x) if(x.isGroupAccount) +// => Some("User does not exist.") case Some(x) if(x.userName == params("owner") || getCollaborators(params("owner"), params("repository")).contains(x.userName)) - => Some("User can access this repository already.") + => Some(value + " is repository owner.") // TODO also group members? case _ => None } } diff --git a/src/main/scala/gitbucket/core/service/RepositoryService.scala b/src/main/scala/gitbucket/core/service/RepositoryService.scala index 3541c0786..fef989767 100644 --- a/src/main/scala/gitbucket/core/service/RepositoryService.scala +++ b/src/main/scala/gitbucket/core/service/RepositoryService.scala @@ -337,49 +337,53 @@ trait RepositoryService { self: AccountService => .update (defaultBranch) /** - * Add collaborator to the repository. - * - * @param userName the user name of the repository owner - * @param repositoryName the repository name - * @param collaboratorName the collaborator name + * Add collaborator (user or group) to the repository. */ def addCollaborator(userName: String, repositoryName: String, collaboratorName: String)(implicit s: Session): Unit = Collaborators insert Collaborator(userName, repositoryName, collaboratorName) /** - * Remove collaborator from the repository. - * - * @param userName the user name of the repository owner - * @param repositoryName the repository name - * @param collaboratorName the collaborator name + * Remove collaborator (user or group) from the repository. */ def removeCollaborator(userName: String, repositoryName: String, collaboratorName: String)(implicit s: Session): Unit = Collaborators.filter(_.byPrimaryKey(userName, repositoryName, collaboratorName)).delete /** * Remove all collaborators from the repository. - * - * @param userName the user name of the repository owner - * @param repositoryName the repository name */ def removeCollaborators(userName: String, repositoryName: String)(implicit s: Session): Unit = Collaborators.filter(_.byRepository(userName, repositoryName)).delete /** - * Returns the list of collaborators name which is sorted with ascending order. - * - * @param userName the user name of the repository owner - * @param repositoryName the repository name - * @return the list of collaborators name + * Returns the list of collaborators name (user name or group name) which is sorted with ascending order. */ def getCollaborators(userName: String, repositoryName: String)(implicit s: Session): List[String] = Collaborators.filter(_.byRepository(userName, repositoryName)).sortBy(_.collaboratorName).map(_.collaboratorName).list + /** + * Returns the list of all collaborator name and permission which is sorted with ascending order. + * If a group is added as a collaborator, this method returns users who are belong to that group. + */ + def getCollaboratorUserNames(userName: String, repositoryName: String, filter: Seq[String] = Nil)(implicit s: Session): List[(String, String)] = { + val q1 = Collaborators.filter(_.byRepository(userName, repositoryName)) + .innerJoin(Accounts).on { case (t1, t2) => (t1.collaboratorName === t2.userName) && (t2.groupAccount === false.bind) } + .map { case (t1, t2) => (t1.collaboratorName, "ADMIN") } + + val q2 = Collaborators.filter(_.byRepository(userName, repositoryName)) + .innerJoin(Accounts).on { case (t1, t2) => (t1.collaboratorName === t2.userName) && (t2.groupAccount === true.bind) } + .innerJoin(GroupMembers).on { case ((t1, t2), t3) => t2.userName === t3.groupName } + .map { case ((t1, t2), t3) => (t3.userName, "ADMIN") } + + q1.union(q2).list + } + + def hasWritePermission(owner: String, repository: String, loginAccount: Option[Account])(implicit s: Session): Boolean = { loginAccount match { case Some(a) if(a.isAdmin) => true case Some(a) if(a.userName == owner) => true - case Some(a) if(getCollaborators(owner, repository).contains(a.userName)) => true + case Some(a) if(getGroupMembers(owner).exists(_.userName == a.userName)) => true + case Some(a) if(getCollaboratorUserNames(owner, repository).contains((a.userName, "ADMIN"))) => true // TODO ADMIN|WRITE case _ => false } } diff --git a/src/main/scala/gitbucket/core/util/Authenticator.scala b/src/main/scala/gitbucket/core/util/Authenticator.scala index 00354941b..ca1c39492 100644 --- a/src/main/scala/gitbucket/core/util/Authenticator.scala +++ b/src/main/scala/gitbucket/core/util/Authenticator.scala @@ -1,11 +1,13 @@ package gitbucket.core.util import gitbucket.core.controller.ControllerBase -import gitbucket.core.service.{RepositoryService, AccountService} +import gitbucket.core.service.{AccountService, RepositoryService} import RepositoryService.RepositoryInfo import Implicits._ import ControlUtil._ +import scala.collection.Searching.search + /** * Allows only oneself and administrators. */ @@ -40,9 +42,9 @@ trait OwnerAuthenticator { self: ControllerBase with RepositoryService with Acco context.loginAccount match { case Some(x) if(x.isAdmin) => action(repository) case Some(x) if(repository.owner == x.userName) => action(repository) - case Some(x) if(getGroupMembers(repository.owner).exists { member => - member.userName == x.userName && member.isManager == true - }) => action(repository) + // TODO Repository management is allowed for only group managers? + case Some(x) if(getGroupMembers(repository.owner).exists { m => m.userName == x.userName && m.isManager == true }) => action(repository) + case Some(x) if(getCollaboratorUserNames(paths(0), paths(1), Seq("ADMIN")).exists(_._1 == x.userName)) => action(repository) case _ => Unauthorized() } } getOrElse NotFound() @@ -88,7 +90,7 @@ trait AdminAuthenticator { self: ControllerBase => /** * Allows only collaborators and administrators. */ -trait CollaboratorsAuthenticator { self: ControllerBase with RepositoryService => +trait CollaboratorsAuthenticator { self: ControllerBase with RepositoryService with AccountService => protected def collaboratorsOnly(action: (RepositoryInfo) => Any) = { authenticate(action) } protected def collaboratorsOnly[T](action: (T, RepositoryInfo) => Any) = (form: T) => { authenticate(action(form, _)) } @@ -99,7 +101,8 @@ trait CollaboratorsAuthenticator { self: ControllerBase with RepositoryService = context.loginAccount match { case Some(x) if(x.isAdmin) => action(repository) case Some(x) if(paths(0) == x.userName) => action(repository) - case Some(x) if(getCollaborators(paths(0), paths(1)).contains(x.userName)) => action(repository) + case Some(x) if(getGroupMembers(repository.owner).exists(_.userName == x.userName)) => action(repository) + case Some(x) if(getCollaboratorUserNames(paths(0), paths(1), Seq("ADMIN", "WRITE")).exists(_._1 == x.userName)) => action(repository) case _ => Unauthorized() } } getOrElse NotFound() @@ -109,9 +112,9 @@ trait CollaboratorsAuthenticator { self: ControllerBase with RepositoryService = } /** - * Allows only the repository owner (or manager for group repository) and administrators. + * Allows only guests and signed in users who can access the repository. */ -trait ReferrerAuthenticator { self: ControllerBase with RepositoryService => +trait ReferrerAuthenticator { self: ControllerBase with RepositoryService with AccountService => protected def referrersOnly(action: (RepositoryInfo) => Any) = { authenticate(action) } protected def referrersOnly[T](action: (T, RepositoryInfo) => Any) = (form: T) => { authenticate(action(form, _)) } @@ -125,7 +128,8 @@ trait ReferrerAuthenticator { self: ControllerBase with RepositoryService => context.loginAccount match { case Some(x) if(x.isAdmin) => action(repository) case Some(x) if(paths(0) == x.userName) => action(repository) - case Some(x) if(getCollaborators(paths(0), paths(1)).contains(x.userName)) => action(repository) + case Some(x) if(getGroupMembers(repository.owner).exists(_.userName == x.userName)) => action(repository) + case Some(x) if(getCollaboratorUserNames(paths(0), paths(1)).exists(_._1 == x.userName)) => action(repository) case _ => Unauthorized() } } @@ -136,9 +140,9 @@ trait ReferrerAuthenticator { self: ControllerBase with RepositoryService => } /** - * Allows only signed in users which can access the repository. + * Allows only signed in users who can access the repository. */ -trait ReadableUsersAuthenticator { self: ControllerBase with RepositoryService => +trait ReadableUsersAuthenticator { self: ControllerBase with RepositoryService with AccountService => protected def readableUsersOnly(action: (RepositoryInfo) => Any) = { authenticate(action) } protected def readableUsersOnly[T](action: (T, RepositoryInfo) => Any) = (form: T) => { authenticate(action(form, _)) } @@ -150,7 +154,8 @@ trait ReadableUsersAuthenticator { self: ControllerBase with RepositoryService = case Some(x) if(x.isAdmin) => action(repository) case Some(x) if(!repository.repository.isPrivate) => action(repository) case Some(x) if(paths(0) == x.userName) => action(repository) - case Some(x) if(getCollaborators(paths(0), paths(1)).contains(x.userName)) => action(repository) + case Some(x) if(getGroupMembers(repository.owner).exists(_.userName == x.userName)) => action(repository) + case Some(x) if(getCollaboratorUserNames(paths(0), paths(1)).exists(_._1 == x.userName)) => action(repository) case _ => Unauthorized() } } getOrElse NotFound() diff --git a/src/main/twirl/gitbucket/core/helper/account.scala.html b/src/main/twirl/gitbucket/core/helper/account.scala.html index e3dc42c31..8fa8907ad 100644 --- a/src/main/twirl/gitbucket/core/helper/account.scala.html +++ b/src/main/twirl/gitbucket/core/helper/account.scala.html @@ -1,4 +1,4 @@ -@(id: String, width: Int)(implicit context: gitbucket.core.controller.Context) +@(id: String, width: Int, userOnly: Boolean = true)(implicit context: gitbucket.core.controller.Context) @@ -6,7 +6,7 @@ $(function(){ $('#@id').typeahead({ source: function (query, process) { - return $.get('@context.path/_user/proposals', { query: query }, + return $.get('@context.path/_user/proposals@if(userOnly){?userOnly}', { query: query }, function (data) { return process(data.options); }); diff --git a/src/main/twirl/gitbucket/core/settings/collaborators.scala.html b/src/main/twirl/gitbucket/core/settings/collaborators.scala.html index 21c84238f..83116e805 100644 --- a/src/main/twirl/gitbucket/core/settings/collaborators.scala.html +++ b/src/main/twirl/gitbucket/core/settings/collaborators.scala.html @@ -10,25 +10,17 @@ @collaborators.map { collaboratorName =>
  • @collaboratorName - @if(!isGroupRepository){ - (remove) - } else { - @if(repository.managers.contains(collaboratorName)){ - (Manager) - } - } + (remove)
    • } - @if(!isGroupRepository){
    - @gitbucket.core.helper.html.account("userName", 300) + @gitbucket.core.helper.html.account("userName", 300, false)
    - } } } } From ce916a7d4b4b178fea007e28b9c218692edffde4 Mon Sep 17 00:00:00 2001 From: Naoki Takezoe Date: Tue, 1 Nov 2016 06:51:30 +0900 Subject: [PATCH 02/14] (refs #1286) Fix models --- .../scala/gitbucket/core/model/Collaborator.scala | 6 ++++-- .../scala/gitbucket/core/model/Repository.scala | 6 ++++-- .../gitbucket/core/service/RepositoryService.scala | 14 ++++++-------- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/main/scala/gitbucket/core/model/Collaborator.scala b/src/main/scala/gitbucket/core/model/Collaborator.scala index 55ae80f14..f25e4b293 100644 --- a/src/main/scala/gitbucket/core/model/Collaborator.scala +++ b/src/main/scala/gitbucket/core/model/Collaborator.scala @@ -7,7 +7,8 @@ trait CollaboratorComponent extends TemplateComponent { self: Profile => class Collaborators(tag: Tag) extends Table[Collaborator](tag, "COLLABORATOR") with BasicTemplate { val collaboratorName = column[String]("COLLABORATOR_NAME") - def * = (userName, repositoryName, collaboratorName) <> (Collaborator.tupled, Collaborator.unapply) + val permission = column[String]("PERMISSION") + def * = (userName, repositoryName, collaboratorName, permission) <> (Collaborator.tupled, Collaborator.unapply) def byPrimaryKey(owner: String, repository: String, collaborator: String) = byRepository(owner, repository) && (collaboratorName === collaborator.bind) @@ -17,5 +18,6 @@ trait CollaboratorComponent extends TemplateComponent { self: Profile => case class Collaborator( userName: String, repositoryName: String, - collaboratorName: String + collaboratorName: String, + permission: String ) diff --git a/src/main/scala/gitbucket/core/model/Repository.scala b/src/main/scala/gitbucket/core/model/Repository.scala index c5c61bb39..feca31d81 100644 --- a/src/main/scala/gitbucket/core/model/Repository.scala +++ b/src/main/scala/gitbucket/core/model/Repository.scala @@ -23,11 +23,12 @@ trait RepositoryComponent extends TemplateComponent { self: Profile => val allowWikiEditing = column[Boolean]("ALLOW_WIKI_EDITING") val externalWikiUrl = column[String]("EXTERNAL_WIKI_URL") val allowFork = column[Boolean]("ALLOW_FORK") + val allowCreateIssue = column[Boolean]("ALLOW_CREATE_ISSUE") def * = ( (userName, repositoryName, isPrivate, description.?, defaultBranch, registeredDate, updatedDate, lastActivityDate, originUserName.?, originRepositoryName.?, parentUserName.?, parentRepositoryName.?), - (enableIssues, externalIssuesUrl.?, enableWiki, allowWikiEditing, externalWikiUrl.?, allowFork) + (enableIssues, externalIssuesUrl.?, enableWiki, allowWikiEditing, externalWikiUrl.?, allowFork, allowCreateIssue) ).shaped <> ( { case (repository, options) => Repository( @@ -90,5 +91,6 @@ case class RepositoryOptions( enableWiki: Boolean, allowWikiEditing: Boolean, externalWikiUrl: Option[String], - allowFork: Boolean + allowFork: Boolean, + allowCreateIssue: Boolean ) diff --git a/src/main/scala/gitbucket/core/service/RepositoryService.scala b/src/main/scala/gitbucket/core/service/RepositoryService.scala index fef989767..093e87621 100644 --- a/src/main/scala/gitbucket/core/service/RepositoryService.scala +++ b/src/main/scala/gitbucket/core/service/RepositoryService.scala @@ -43,7 +43,8 @@ trait RepositoryService { self: AccountService => enableWiki = true, allowWikiEditing = true, externalWikiUrl = None, - allowFork = true + allowFork = true, + allowCreateIssue = false ) ) @@ -124,11 +125,8 @@ trait RepositoryService { self: AccountService => repositoryName = newRepositoryName )) :_*) - if(account.isGroupAccount){ - Collaborators.insertAll(getGroupMembers(newUserName).map(m => Collaborator(newUserName, newRepositoryName, m.userName)) :_*) - } else { - Collaborators.insertAll(collaborators.map(_.copy(userName = newUserName, repositoryName = newRepositoryName)) :_*) - } + // TODO Drop transfered owner from collaborators? + Collaborators.insertAll(collaborators.map(_.copy(userName = newUserName, repositoryName = newRepositoryName)) :_*) // Update activity messages Activities.filter { t => @@ -340,7 +338,7 @@ trait RepositoryService { self: AccountService => * Add collaborator (user or group) to the repository. */ def addCollaborator(userName: String, repositoryName: String, collaboratorName: String)(implicit s: Session): Unit = - Collaborators insert Collaborator(userName, repositoryName, collaboratorName) + Collaborators insert Collaborator(userName, repositoryName, collaboratorName, "ADMIN") // TODO /** * Remove collaborator (user or group) from the repository. @@ -358,7 +356,7 @@ trait RepositoryService { self: AccountService => * Returns the list of collaborators name (user name or group name) which is sorted with ascending order. */ def getCollaborators(userName: String, repositoryName: String)(implicit s: Session): List[String] = - Collaborators.filter(_.byRepository(userName, repositoryName)).sortBy(_.collaboratorName).map(_.collaboratorName).list + Collaborators.filter(_.byRepository(userName, repositoryName)).sortBy(_.collaboratorName).map(_.collaboratorName).list // TODO return with permission /** * Returns the list of all collaborator name and permission which is sorted with ascending order. From 368052bd8f9ca330c5348f305c8f9db302df1290 Mon Sep 17 00:00:00 2001 From: Naoki Takezoe Date: Tue, 1 Nov 2016 07:24:51 +0900 Subject: [PATCH 03/14] (refs #1286) Fix services and beat compilation errors --- .../scala/gitbucket/core/api/ApiUser.scala | 2 +- .../core/controller/AccountController.scala | 28 +++++++++---------- .../core/controller/ApiController.scala | 3 +- .../core/controller/IssuesController.scala | 24 ++++++++-------- .../controller/PullRequestsController.scala | 18 ++++++------ .../RepositorySettingsController.scala | 2 +- .../controller/SystemSettingsController.scala | 14 +++++----- .../service/RepositoryCreationService.scala | 12 ++++---- .../core/service/RepositoryService.scala | 8 +++--- .../scala/gitbucket/core/util/Notifier.scala | 4 ++- .../core/settings/collaborators.scala.html | 8 +++--- 11 files changed, 63 insertions(+), 60 deletions(-) diff --git a/src/main/scala/gitbucket/core/api/ApiUser.scala b/src/main/scala/gitbucket/core/api/ApiUser.scala index 7259c12e4..9b3dc9dcc 100644 --- a/src/main/scala/gitbucket/core/api/ApiUser.scala +++ b/src/main/scala/gitbucket/core/api/ApiUser.scala @@ -30,7 +30,7 @@ object ApiUser{ def apply(user: Account): ApiUser = ApiUser( login = user.userName, email = user.mailAddress, - `type` = if(user.isGroupAccount){ "Organization" }else{ "User" }, + `type` = if(user.isGroupAccount){ "Organization" } else { "User" }, site_admin = user.isAdmin, created_at = user.registeredDate ) diff --git a/src/main/scala/gitbucket/core/controller/AccountController.scala b/src/main/scala/gitbucket/core/controller/AccountController.scala index fa46b7810..bfb9ccd08 100644 --- a/src/main/scala/gitbucket/core/controller/AccountController.scala +++ b/src/main/scala/gitbucket/core/controller/AccountController.scala @@ -319,13 +319,13 @@ trait AccountControllerBase extends AccountManagementControllerBase { // Update GROUP_MEMBER updateGroupMembers(form.groupName, members) - // Update COLLABORATOR for group repositories - getRepositoryNamesOfUser(form.groupName).foreach { repositoryName => - removeCollaborators(form.groupName, repositoryName) - members.foreach { case (userName, isManager) => - addCollaborator(form.groupName, repositoryName, userName) - } - } +// // Update COLLABORATOR for group repositories +// getRepositoryNamesOfUser(form.groupName).foreach { repositoryName => +// removeCollaborators(form.groupName, repositoryName) +// members.foreach { case (userName, isManager) => +// addCollaborator(form.groupName, repositoryName, userName) +// } +// } updateImage(form.groupName, form.fileId, form.clearImage) redirect(s"/${form.groupName}") @@ -402,13 +402,13 @@ trait AccountControllerBase extends AccountManagementControllerBase { parentUserName = Some(repository.owner) ) - // Add collaborators for group repository - val ownerAccount = getAccountByUserName(accountName).get - if(ownerAccount.isGroupAccount){ - getGroupMembers(accountName).foreach { member => - addCollaborator(accountName, repository.name, member.userName) - } - } +// // Add collaborators for group repository +// val ownerAccount = getAccountByUserName(accountName).get +// if(ownerAccount.isGroupAccount){ +// getGroupMembers(accountName).foreach { member => +// addCollaborator(accountName, repository.name, member.userName) +// } +// } // Insert default labels insertDefaultLabels(accountName, repository.name) diff --git a/src/main/scala/gitbucket/core/controller/ApiController.scala b/src/main/scala/gitbucket/core/controller/ApiController.scala index 692a1f992..e3175add5 100644 --- a/src/main/scala/gitbucket/core/controller/ApiController.scala +++ b/src/main/scala/gitbucket/core/controller/ApiController.scala @@ -177,7 +177,8 @@ trait ApiControllerBase extends ControllerBase { * https://developer.github.com/v3/repos/collaborators/#list-collaborators */ get("/api/v3/repos/:owner/:repo/collaborators") (referrersOnly { repository => - JsonFormat(getCollaborators(params("owner"), params("repo")).map(u => ApiUser(getAccountByUserName(u).get))) + // TODO Should ApiUser take permission? getCollaboratorUserNames does not return owner group members. + JsonFormat(getCollaboratorUserNames(params("owner"), params("repo")).map(u => ApiUser(getAccountByUserName(u._1).get))) }) /** diff --git a/src/main/scala/gitbucket/core/controller/IssuesController.scala b/src/main/scala/gitbucket/core/controller/IssuesController.scala index 6ddcd40e8..d97e31ea4 100644 --- a/src/main/scala/gitbucket/core/controller/IssuesController.scala +++ b/src/main/scala/gitbucket/core/controller/IssuesController.scala @@ -67,7 +67,7 @@ trait IssuesControllerBase extends ControllerBase { _, getComments(owner, name, issueId.toInt), getIssueLabels(owner, name, issueId.toInt), - (getCollaborators(owner, name) ::: (if(getAccountByUserName(owner).exists(_.isGroupAccount)) Nil else List(owner))).sorted, + getAssignableUserNames(owner, name), getMilestonesWithIssueCount(owner, name), getLabels(owner, name), hasWritePermission(owner, name, context.loginAccount), @@ -79,11 +79,11 @@ trait IssuesControllerBase extends ControllerBase { get("/:owner/:repository/issues/new")(readableUsersOnly { repository => defining(repository.owner, repository.name){ case (owner, name) => html.create( - (getCollaborators(owner, name) ::: (if(getAccountByUserName(owner).exists(_.isGroupAccount)) Nil else List(owner))).sorted, - getMilestones(owner, name), - getLabels(owner, name), - hasWritePermission(owner, name, context.loginAccount), - repository) + getAssignableUserNames(owner, name), + getMilestones(owner, name), + getLabels(owner, name), + hasWritePermission(owner, name, context.loginAccount), + repository) } }) @@ -369,11 +369,7 @@ trait IssuesControllerBase extends ControllerBase { "issues", searchIssue(condition, false, (page - 1) * IssueLimit, IssueLimit, owner -> repoName), page, - if(!getAccountByUserName(owner).exists(_.isGroupAccount)){ - (getCollaborators(owner, repoName) :+ owner).sorted - } else { - getCollaborators(owner, repoName) - }, + getAssignableUserNames(owner, repoName), getMilestones(owner, repoName), getLabels(owner, repoName), countIssue(condition.copy(state = "open" ), false, owner -> repoName), @@ -383,4 +379,10 @@ trait IssuesControllerBase extends ControllerBase { hasWritePermission(owner, repoName, context.loginAccount)) } } + + // TODO Move to IssuesService? + private def getAssignableUserNames(owner: String, repository: String): List[String] = + (getCollaboratorUserNames(owner, repository, Seq("ADMIN", "WRITE")).map(_._1) ::: + (if(getAccountByUserName(owner).get.isGroupAccount) getGroupMembers(owner).map(_.userName) else List(owner))).sorted + } diff --git a/src/main/scala/gitbucket/core/controller/PullRequestsController.scala b/src/main/scala/gitbucket/core/controller/PullRequestsController.scala index f3bd241ab..6d8fd7c19 100644 --- a/src/main/scala/gitbucket/core/controller/PullRequestsController.scala +++ b/src/main/scala/gitbucket/core/controller/PullRequestsController.scala @@ -18,7 +18,6 @@ import gitbucket.core.view.helpers import io.github.gitbucket.scalatra.forms._ import org.eclipse.jgit.api.Git import org.eclipse.jgit.lib.PersonIdent -import org.slf4j.LoggerFactory import scala.collection.JavaConverters._ @@ -34,8 +33,6 @@ trait PullRequestsControllerBase extends ControllerBase { with CommitsService with ActivityService with PullRequestService with WebHookPullRequestService with ReferrerAuthenticator with CollaboratorsAuthenticator with CommitStatusService with MergeService with ProtectedBranchService => - private val logger = LoggerFactory.getLogger(classOf[PullRequestsControllerBase]) - val pullRequestForm = mapping( "title" -> trim(label("Title" , text(required, maxlength(100)))), "content" -> trim(label("Content", optional(text()))), @@ -94,7 +91,7 @@ trait PullRequestsControllerBase extends ControllerBase { (commits.flatten.map(commit => getCommitComments(owner, name, commit.id, true)).flatten.toList ::: getComments(owner, name, issueId)) .sortWith((a, b) => a.registeredDate before b.registeredDate), getIssueLabels(owner, name, issueId), - (getCollaborators(owner, name) ::: (if(getAccountByUserName(owner).get.isGroupAccount) Nil else List(owner))).sorted, + getAssignableUserNames(owner, name), getMilestonesWithIssueCount(owner, name), getLabels(owner, name), commits, @@ -375,7 +372,7 @@ trait PullRequestsControllerBase extends ControllerBase { originRepository, forkedRepository, hasWritePermission(originRepository.owner, originRepository.name, context.loginAccount), - (getCollaborators(originRepository.owner, originRepository.name) ::: (if(getAccountByUserName(originRepository.owner).get.isGroupAccount) Nil else List(originRepository.owner))).sorted, + getAssignableUserNames(originRepository.owner, originRepository.name), getMilestones(originRepository.owner, originRepository.name), getLabels(originRepository.owner, originRepository.name) ) @@ -526,11 +523,7 @@ trait PullRequestsControllerBase extends ControllerBase { "pulls", searchIssue(condition, true, (page - 1) * PullRequestLimit, PullRequestLimit, owner -> repoName), page, - if(!getAccountByUserName(owner).exists(_.isGroupAccount)){ - (getCollaborators(owner, repoName) :+ owner).sorted - } else { - getCollaborators(owner, repoName) - }, + getAssignableUserNames(owner, repoName), getMilestones(owner, repoName), getLabels(owner, repoName), countIssue(condition.copy(state = "open" ), true, owner -> repoName), @@ -540,4 +533,9 @@ trait PullRequestsControllerBase extends ControllerBase { hasWritePermission(owner, repoName, context.loginAccount)) } + // TODO Move to IssuesService? + private def getAssignableUserNames(owner: String, repository: String): List[String] = + (getCollaboratorUserNames(owner, repository, Seq("ADMIN", "WRITE")).map(_._1) ::: + (if(getAccountByUserName(owner).get.isGroupAccount) getGroupMembers(owner).map(_.userName) else List(owner))).sorted + } diff --git a/src/main/scala/gitbucket/core/controller/RepositorySettingsController.scala b/src/main/scala/gitbucket/core/controller/RepositorySettingsController.scala index cceb9a603..d6726eefa 100644 --- a/src/main/scala/gitbucket/core/controller/RepositorySettingsController.scala +++ b/src/main/scala/gitbucket/core/controller/RepositorySettingsController.scala @@ -183,7 +183,7 @@ trait RepositorySettingsControllerBase extends ControllerBase { */ post("/:owner/:repository/settings/collaborators/add", collaboratorForm)(ownerOnly { (form, repository) => getAccountByUserName(repository.owner).foreach { _ => - addCollaborator(repository.owner, repository.name, form.userName) + addCollaborator(repository.owner, repository.name, form.userName, "ADMIN") // TODO } redirect(s"/${repository.owner}/${repository.name}/settings/collaborators") }) diff --git a/src/main/scala/gitbucket/core/controller/SystemSettingsController.scala b/src/main/scala/gitbucket/core/controller/SystemSettingsController.scala index 7a6874612..bfca8582d 100644 --- a/src/main/scala/gitbucket/core/controller/SystemSettingsController.scala +++ b/src/main/scala/gitbucket/core/controller/SystemSettingsController.scala @@ -279,13 +279,13 @@ trait SystemSettingsControllerBase extends AccountManagementControllerBase { } else { // Update GROUP_MEMBER updateGroupMembers(form.groupName, members) - // Update COLLABORATOR for group repositories - getRepositoryNamesOfUser(form.groupName).foreach { repositoryName => - removeCollaborators(form.groupName, repositoryName) - members.foreach { case (userName, isManager) => - addCollaborator(form.groupName, repositoryName, userName) - } - } +// // Update COLLABORATOR for group repositories +// getRepositoryNamesOfUser(form.groupName).foreach { repositoryName => +// removeCollaborators(form.groupName, repositoryName) +// members.foreach { case (userName, isManager) => +// addCollaborator(form.groupName, repositoryName, userName) +// } +// } } updateImage(form.groupName, form.fileId, form.clearImage) diff --git a/src/main/scala/gitbucket/core/service/RepositoryCreationService.scala b/src/main/scala/gitbucket/core/service/RepositoryCreationService.scala index 90e0afd63..04aef5099 100644 --- a/src/main/scala/gitbucket/core/service/RepositoryCreationService.scala +++ b/src/main/scala/gitbucket/core/service/RepositoryCreationService.scala @@ -21,12 +21,12 @@ trait RepositoryCreationService { // Insert to the database at first insertRepository(name, owner, description, isPrivate) - // Add collaborators for group repository - if(ownerAccount.isGroupAccount){ - getGroupMembers(owner).foreach { member => - addCollaborator(owner, name, member.userName) - } - } +// // Add collaborators for group repository +// if(ownerAccount.isGroupAccount){ +// getGroupMembers(owner).foreach { member => +// addCollaborator(owner, name, member.userName) +// } +// } // Insert default labels insertDefaultLabels(owner, name) diff --git a/src/main/scala/gitbucket/core/service/RepositoryService.scala b/src/main/scala/gitbucket/core/service/RepositoryService.scala index 093e87621..5391507f3 100644 --- a/src/main/scala/gitbucket/core/service/RepositoryService.scala +++ b/src/main/scala/gitbucket/core/service/RepositoryService.scala @@ -337,8 +337,8 @@ trait RepositoryService { self: AccountService => /** * Add collaborator (user or group) to the repository. */ - def addCollaborator(userName: String, repositoryName: String, collaboratorName: String)(implicit s: Session): Unit = - Collaborators insert Collaborator(userName, repositoryName, collaboratorName, "ADMIN") // TODO + def addCollaborator(userName: String, repositoryName: String, collaboratorName: String, permission: String)(implicit s: Session): Unit = + Collaborators insert Collaborator(userName, repositoryName, collaboratorName, permission) /** * Remove collaborator (user or group) from the repository. @@ -355,8 +355,8 @@ trait RepositoryService { self: AccountService => /** * Returns the list of collaborators name (user name or group name) which is sorted with ascending order. */ - def getCollaborators(userName: String, repositoryName: String)(implicit s: Session): List[String] = - Collaborators.filter(_.byRepository(userName, repositoryName)).sortBy(_.collaboratorName).map(_.collaboratorName).list // TODO return with permission + def getCollaborators(userName: String, repositoryName: String)(implicit s: Session): List[Collaborator] = + Collaborators.filter(_.byRepository(userName, repositoryName)).sortBy(_.collaboratorName).list /** * Returns the list of all collaborator name and permission which is sorted with ascending order. diff --git a/src/main/scala/gitbucket/core/util/Notifier.scala b/src/main/scala/gitbucket/core/util/Notifier.scala index 36ec89d9d..de424835c 100644 --- a/src/main/scala/gitbucket/core/util/Notifier.scala +++ b/src/main/scala/gitbucket/core/util/Notifier.scala @@ -22,8 +22,10 @@ trait Notifier extends RepositoryService with AccountService with IssuesService ( // individual repository's owner issue.userName :: + // group members of group repository + getGroupMembers(issue.userName).map(_.userName) ::: // collaborators - getCollaborators(issue.userName, issue.repositoryName) ::: + getCollaboratorUserNames(issue.userName, issue.repositoryName).map(_._1) ::: // participants issue.openedUserName :: getComments(issue.userName, issue.repositoryName, issue.issueId).map(_.commentedUserName) diff --git a/src/main/twirl/gitbucket/core/settings/collaborators.scala.html b/src/main/twirl/gitbucket/core/settings/collaborators.scala.html index 83116e805..715ea1c83 100644 --- a/src/main/twirl/gitbucket/core/settings/collaborators.scala.html +++ b/src/main/twirl/gitbucket/core/settings/collaborators.scala.html @@ -1,4 +1,4 @@ -@(collaborators: List[String], +@(collaborators: List[gitbucket.core.model.Collaborator], isGroupRepository: Boolean, repository: gitbucket.core.service.RepositoryService.RepositoryInfo)(implicit context: gitbucket.core.controller.Context) @import gitbucket.core.view.helpers @@ -7,10 +7,10 @@ @gitbucket.core.settings.html.menu("collaborators", repository){

    Manage Collaborators

    From 04567391180c3a7a91a970692ff0ec2299077777 Mon Sep 17 00:00:00 2001 From: Naoki Takezoe Date: Tue, 1 Nov 2016 09:10:40 +0900 Subject: [PATCH 04/14] (refs #1286) Update collaborators setting form --- .../RepositorySettingsController.scala | 37 ++++-- .../core/service/RepositoryService.scala | 10 +- .../core/settings/collaborators.scala.html | 112 +++++++++++++++--- 3 files changed, 126 insertions(+), 33 deletions(-) diff --git a/src/main/scala/gitbucket/core/controller/RepositorySettingsController.scala b/src/main/scala/gitbucket/core/controller/RepositorySettingsController.scala index d6726eefa..31ebeb9ca 100644 --- a/src/main/scala/gitbucket/core/controller/RepositorySettingsController.scala +++ b/src/main/scala/gitbucket/core/controller/RepositorySettingsController.scala @@ -178,23 +178,34 @@ trait RepositorySettingsControllerBase extends ControllerBase { repository) }) - /** - * Add the collaborator. - */ - post("/:owner/:repository/settings/collaborators/add", collaboratorForm)(ownerOnly { (form, repository) => - getAccountByUserName(repository.owner).foreach { _ => - addCollaborator(repository.owner, repository.name, form.userName, "ADMIN") // TODO + post("/:owner/:repository/settings/collaborators")(ownerOnly { repository => + val collaborators = params("collaborators") + removeCollaborators(repository.owner, repository.name) + collaborators.split(",").map { collaborator => + val userName :: permission :: Nil = collaborator.split(":").toList + addCollaborator(repository.owner, repository.name, userName, permission) } redirect(s"/${repository.owner}/${repository.name}/settings/collaborators") }) - /** - * Add the collaborator. - */ - get("/:owner/:repository/settings/collaborators/remove")(ownerOnly { repository => - removeCollaborator(repository.owner, repository.name, params("name")) - redirect(s"/${repository.owner}/${repository.name}/settings/collaborators") - }) + +// /** +// * Add the collaborator. +// */ +// post("/:owner/:repository/settings/collaborators/add", collaboratorForm)(ownerOnly { (form, repository) => +// getAccountByUserName(repository.owner).foreach { _ => +// addCollaborator(repository.owner, repository.name, form.userName, "ADMIN") // TODO +// } +// redirect(s"/${repository.owner}/${repository.name}/settings/collaborators") +// }) +// +// /** +// * Add the collaborator. +// */ +// get("/:owner/:repository/settings/collaborators/remove")(ownerOnly { repository => +// removeCollaborator(repository.owner, repository.name, params("name")) +// redirect(s"/${repository.owner}/${repository.name}/settings/collaborators") +// }) /** * Display the web hook page. diff --git a/src/main/scala/gitbucket/core/service/RepositoryService.scala b/src/main/scala/gitbucket/core/service/RepositoryService.scala index 5391507f3..6fe62e2ca 100644 --- a/src/main/scala/gitbucket/core/service/RepositoryService.scala +++ b/src/main/scala/gitbucket/core/service/RepositoryService.scala @@ -340,11 +340,11 @@ trait RepositoryService { self: AccountService => def addCollaborator(userName: String, repositoryName: String, collaboratorName: String, permission: String)(implicit s: Session): Unit = Collaborators insert Collaborator(userName, repositoryName, collaboratorName, permission) - /** - * Remove collaborator (user or group) from the repository. - */ - def removeCollaborator(userName: String, repositoryName: String, collaboratorName: String)(implicit s: Session): Unit = - Collaborators.filter(_.byPrimaryKey(userName, repositoryName, collaboratorName)).delete +// /** +// * Remove collaborator (user or group) from the repository. +// */ +// def removeCollaborator(userName: String, repositoryName: String, collaboratorName: String)(implicit s: Session): Unit = +// Collaborators.filter(_.byPrimaryKey(userName, repositoryName, collaboratorName)).delete /** * Remove all collaborators from the repository. diff --git a/src/main/twirl/gitbucket/core/settings/collaborators.scala.html b/src/main/twirl/gitbucket/core/settings/collaborators.scala.html index 715ea1c83..3d2d60cc7 100644 --- a/src/main/twirl/gitbucket/core/settings/collaborators.scala.html +++ b/src/main/twirl/gitbucket/core/settings/collaborators.scala.html @@ -6,21 +6,103 @@ @gitbucket.core.html.menu("settings", repository){ @gitbucket.core.settings.html.menu("collaborators", repository){

    Manage Collaborators

    - -
    -
    - -
    - @gitbucket.core.helper.html.account("userName", 300, false) - -
    +
    +
      +
    + @gitbucket.core.helper.html.account("userName", 200, false) + + +
    + +
    +
    + +
    +
    } } } + \ No newline at end of file From 0c3c6ea15a9f90762ec6c6d91dbc5ea42290638f Mon Sep 17 00:00:00 2001 From: Naoki Takezoe Date: Tue, 1 Nov 2016 16:03:02 +0900 Subject: [PATCH 05/14] (refs #1286) Show whether group account on the collaborators proposal --- .../core/controller/IndexController.scala | 6 ++--- .../core/service/RepositoryService.scala | 13 +++++++---- .../gitbucket/core/helper/account.scala.html | 7 ++++++ .../core/settings/collaborators.scala.html | 22 +++++++++---------- 4 files changed, 30 insertions(+), 18 deletions(-) diff --git a/src/main/scala/gitbucket/core/controller/IndexController.scala b/src/main/scala/gitbucket/core/controller/IndexController.scala index c5a17bc2a..8d15b2e38 100644 --- a/src/main/scala/gitbucket/core/controller/IndexController.scala +++ b/src/main/scala/gitbucket/core/controller/IndexController.scala @@ -110,10 +110,10 @@ trait IndexControllerBase extends ControllerBase { contentType = formats("json") org.json4s.jackson.Serialization.write( Map("options" -> (if(params.get("userOnly").isDefined) { - getAllUsers(false).filter(!_.isGroupAccount).map(_.userName).toArray + getAllUsers(false).filter(!_.isGroupAccount).map { t => (t.userName, t.isGroupAccount) }.toArray } else { - getAllUsers(false).map(_.userName).toArray - })) + getAllUsers(false).map { t => (t.userName, t.isGroupAccount) }.toArray + }).map { case (userName, groupAccount) => userName + ":" + groupAccount }) ) }) diff --git a/src/main/scala/gitbucket/core/service/RepositoryService.scala b/src/main/scala/gitbucket/core/service/RepositoryService.scala index 6fe62e2ca..41580b63d 100644 --- a/src/main/scala/gitbucket/core/service/RepositoryService.scala +++ b/src/main/scala/gitbucket/core/service/RepositoryService.scala @@ -355,8 +355,13 @@ trait RepositoryService { self: AccountService => /** * Returns the list of collaborators name (user name or group name) which is sorted with ascending order. */ - def getCollaborators(userName: String, repositoryName: String)(implicit s: Session): List[Collaborator] = - Collaborators.filter(_.byRepository(userName, repositoryName)).sortBy(_.collaboratorName).list + def getCollaborators(userName: String, repositoryName: String)(implicit s: Session): List[(Collaborator, Boolean)] = + Collaborators + .innerJoin(Accounts).on(_.collaboratorName === _.userName) + .filter { case (t1, t2) => t1.byRepository(userName, repositoryName) } + .map { case (t1, t2) => (t1, t2.groupAccount) } + .sortBy { case (t1, t2) => t1.collaboratorName } + .list /** * Returns the list of all collaborator name and permission which is sorted with ascending order. @@ -364,8 +369,8 @@ trait RepositoryService { self: AccountService => */ def getCollaboratorUserNames(userName: String, repositoryName: String, filter: Seq[String] = Nil)(implicit s: Session): List[(String, String)] = { val q1 = Collaborators.filter(_.byRepository(userName, repositoryName)) - .innerJoin(Accounts).on { case (t1, t2) => (t1.collaboratorName === t2.userName) && (t2.groupAccount === false.bind) } - .map { case (t1, t2) => (t1.collaboratorName, "ADMIN") } + .innerJoin(Accounts).on { case (t1, t2) => (t1.collaboratorName === t2.userName) && (t2.groupAccount === false.bind) } + .map { case (t1, t2) => (t1.collaboratorName, "ADMIN") } val q2 = Collaborators.filter(_.byRepository(userName, repositoryName)) .innerJoin(Accounts).on { case (t1, t2) => (t1.collaboratorName === t2.userName) && (t2.groupAccount === true.bind) } diff --git a/src/main/twirl/gitbucket/core/helper/account.scala.html b/src/main/twirl/gitbucket/core/helper/account.scala.html index 8fa8907ad..06964dc0c 100644 --- a/src/main/twirl/gitbucket/core/helper/account.scala.html +++ b/src/main/twirl/gitbucket/core/helper/account.scala.html @@ -5,6 +5,13 @@ diff --git a/src/main/twirl/gitbucket/core/wiki/compare.scala.html b/src/main/twirl/gitbucket/core/wiki/compare.scala.html index 3745cb731..e2fe16317 100644 --- a/src/main/twirl/gitbucket/core/wiki/compare.scala.html +++ b/src/main/twirl/gitbucket/core/wiki/compare.scala.html @@ -3,7 +3,7 @@ to: String, diffs: Seq[gitbucket.core.util.JGitUtil.DiffInfo], repository: gitbucket.core.service.RepositoryService.RepositoryInfo, - hasWritePermission: Boolean, + isEditable: Boolean, info: Option[Any])(implicit context: gitbucket.core.controller.Context) @import gitbucket.core.view.helpers @gitbucket.core.html.main(s"Compare Revisions - ${repository.owner}/${repository.name}", Some(repository)){ @@ -27,7 +27,7 @@
    @gitbucket.core.helper.html.diff(diffs, repository, None, None, false, None, false, false)
    - @if(hasWritePermission){ + @if(isEditable){
    @if(pageName.isDefined){ Revert Changes diff --git a/src/main/twirl/gitbucket/core/wiki/page.scala.html b/src/main/twirl/gitbucket/core/wiki/page.scala.html index 611714051..ac201e575 100644 --- a/src/main/twirl/gitbucket/core/wiki/page.scala.html +++ b/src/main/twirl/gitbucket/core/wiki/page.scala.html @@ -2,7 +2,7 @@ page: gitbucket.core.service.WikiService.WikiPageInfo, pages: List[String], repository: gitbucket.core.service.RepositoryService.RepositoryInfo, - hasWritePermission: Boolean, + isEditable: Boolean, sidebar: Option[gitbucket.core.service.WikiService.WikiPageInfo], footer: Option[gitbucket.core.service.WikiService.WikiPageInfo])(implicit context: gitbucket.core.controller.Context) @import gitbucket.core.view.helpers @@ -12,7 +12,7 @@
    Page History - @if(hasWritePermission){ + @if(isEditable){ Edit Page New Page } @@ -49,13 +49,13 @@ } @sidebar.map { sidebarPage =>
    - @if(hasWritePermission){ + @if(isEditable){ } @helpers.markdown(sidebarPage.content, repository, true, false, false, false, pages)
    }.getOrElse{ - @if(hasWritePermission){ + @if(isEditable){
    Add a custom sidebar
     @@ -88,13 +88,13 @@
    @footer.map { footerPage =>
    - @if(hasWritePermission){ + @if(isEditable){ } @helpers.markdown(footerPage.content, repository, true, false, false, false, pages)
    }.getOrElse{ - @if(hasWritePermission){ + @if(isEditable){
    Add a custom footer
     diff --git a/src/main/twirl/gitbucket/core/wiki/pages.scala.html b/src/main/twirl/gitbucket/core/wiki/pages.scala.html index cf6fd5926..e947f2112 100644 --- a/src/main/twirl/gitbucket/core/wiki/pages.scala.html +++ b/src/main/twirl/gitbucket/core/wiki/pages.scala.html @@ -1,6 +1,6 @@ @(pages: List[String], repository: gitbucket.core.service.RepositoryService.RepositoryInfo, - hasWritePermission: Boolean)(implicit context: gitbucket.core.controller.Context) + isEditable: Boolean)(implicit context: gitbucket.core.controller.Context) @import gitbucket.core.view.helpers @gitbucket.core.html.main(s"Pages - ${repository.owner}/${repository.name}", Some(repository)){ @gitbucket.core.html.menu("wiki", repository){ @@ -10,7 +10,7 @@
  • - @if(hasWritePermission){ + @if(isEditable){ New Page }
    From 132bb6bee4996471a400dbf03485a5719af1f285 Mon Sep 17 00:00:00 2001 From: Naoki Takezoe Date: Fri, 4 Nov 2016 13:57:39 +0900 Subject: [PATCH 10/14] (refs #1286) Update controllers --- .../core/controller/IssuesController.scala | 114 ++++++++++------ .../controller/PullRequestsController.scala | 127 +++++++++++------- .../core/controller/WikiController.scala | 26 ++-- .../core/service/HandleCommentService.scala | 20 +-- .../core/service/RepositoryService.scala | 6 +- .../core/issues/commentform.scala.html | 9 +- .../core/issues/commentlist.scala.html | 12 +- .../gitbucket/core/issues/create.scala.html | 6 +- .../gitbucket/core/issues/issue.scala.html | 11 +- .../core/issues/issueinfo.scala.html | 8 +- .../gitbucket/core/issues/list.scala.html | 11 +- .../core/issues/listparts.scala.html | 6 +- .../core/pulls/conversation.scala.html | 13 +- .../gitbucket/core/pulls/pullreq.scala.html | 9 +- .../core/settings/options.scala.html | 10 -- .../twirl/gitbucket/core/wiki/edit.scala.html | 2 +- .../gitbucket/core/wiki/history.scala.html | 23 ++-- 17 files changed, 230 insertions(+), 183 deletions(-) diff --git a/src/main/scala/gitbucket/core/controller/IssuesController.scala b/src/main/scala/gitbucket/core/controller/IssuesController.scala index 6fbe8e6d4..703dbefbb 100644 --- a/src/main/scala/gitbucket/core/controller/IssuesController.scala +++ b/src/main/scala/gitbucket/core/controller/IssuesController.scala @@ -2,13 +2,13 @@ package gitbucket.core.controller import gitbucket.core.issues.html import gitbucket.core.service.IssuesService._ +import gitbucket.core.service.RepositoryService.RepositoryInfo import gitbucket.core.service._ import gitbucket.core.util.ControlUtil._ import gitbucket.core.util.Implicits._ import gitbucket.core.util._ import gitbucket.core.view import gitbucket.core.view.Markdown - import io.github.gitbucket.scalatra.forms._ import org.scalatra.Ok @@ -70,7 +70,8 @@ trait IssuesControllerBase extends ControllerBase { getAssignableUserNames(owner, name), getMilestonesWithIssueCount(owner, name), getLabels(owner, name), - hasWritePermission(owner, name, context.loginAccount), + isEditable(repository), + isManageable(repository), repository) } getOrElse NotFound() } @@ -89,50 +90,53 @@ trait IssuesControllerBase extends ControllerBase { post("/:owner/:repository/issues/new", issueCreateForm)(readableUsersOnly { (form, repository) => defining(repository.owner, repository.name){ case (owner, name) => - val writable = hasWritePermission(owner, name, context.loginAccount) - val userName = context.loginAccount.get.userName + val manageable = isManageable(repository) + val editable = isEditable(repository) + if(editable) { + val userName = context.loginAccount.get.userName - // insert issue - val issueId = createIssue(owner, name, userName, form.title, form.content, - if(writable) form.assignedUserName else None, - if(writable) form.milestoneId else None) + // insert issue + val issueId = createIssue(owner, name, userName, form.title, form.content, + if (manageable) form.assignedUserName else None, + if (manageable) form.milestoneId else None) - // insert labels - if(writable){ - form.labelNames.map { value => - val labels = getLabels(owner, name) - value.split(",").foreach { labelName => - labels.find(_.labelName == labelName).map { label => - registerIssueLabel(owner, name, issueId, label.labelId) + // insert labels + if (manageable) { + form.labelNames.map { value => + val labels = getLabels(owner, name) + value.split(",").foreach { labelName => + labels.find(_.labelName == labelName).map { label => + registerIssueLabel(owner, name, issueId, label.labelId) + } } } } - } - // record activity - recordCreateIssueActivity(owner, name, userName, issueId, form.title) + // record activity + recordCreateIssueActivity(owner, name, userName, issueId, form.title) - getIssue(owner, name, issueId.toString).foreach { issue => - // extract references and create refer comment - createReferComment(owner, name, issue, form.title + " " + form.content.getOrElse(""), context.loginAccount.get) + getIssue(owner, name, issueId.toString).foreach { issue => + // extract references and create refer comment + createReferComment(owner, name, issue, form.title + " " + form.content.getOrElse(""), context.loginAccount.get) - // call web hooks - callIssuesWebHook("opened", repository, issue, context.baseUrl, context.loginAccount.get) + // call web hooks + callIssuesWebHook("opened", repository, issue, context.baseUrl, context.loginAccount.get) - // notifications - Notifier().toNotify(repository, issue, form.content.getOrElse("")){ - Notifier.msgIssue(s"${context.baseUrl}/${owner}/${name}/issues/${issueId}") + // notifications + Notifier().toNotify(repository, issue, form.content.getOrElse("")) { + Notifier.msgIssue(s"${context.baseUrl}/${owner}/${name}/issues/${issueId}") + } } - } - redirect(s"/${owner}/${name}/issues/${issueId}") + redirect(s"/${owner}/${name}/issues/${issueId}") + } else Unauthorized() } }) ajaxPost("/:owner/:repository/issues/edit_title/:id", issueTitleEditForm)(readableUsersOnly { (title, repository) => defining(repository.owner, repository.name){ case (owner, name) => getIssue(owner, name, params("id")).map { issue => - if(isEditable(owner, name, issue.openedUserName)){ + if(isEditableContent(owner, name, issue.openedUserName)){ // update issue updateIssue(owner, name, issue.issueId, title, issue.content) // extract references and create refer comment @@ -147,7 +151,7 @@ trait IssuesControllerBase extends ControllerBase { ajaxPost("/:owner/:repository/issues/edit/:id", issueEditForm)(readableUsersOnly { (content, repository) => defining(repository.owner, repository.name){ case (owner, name) => getIssue(owner, name, params("id")).map { issue => - if(isEditable(owner, name, issue.openedUserName)){ + if(isEditableContent(owner, name, issue.openedUserName)){ // update issue updateIssue(owner, name, issue.issueId, issue.title, content) // extract references and create refer comment @@ -161,7 +165,7 @@ trait IssuesControllerBase extends ControllerBase { post("/:owner/:repository/issue_comments/new", commentForm)(readableUsersOnly { (form, repository) => getIssue(repository.owner, repository.name, form.issueId.toString).flatMap { issue => - val actionOpt = params.get("action").filter(_ => isEditable(issue.userName, issue.repositoryName, issue.openedUserName)) + val actionOpt = params.get("action").filter(_ => isEditableContent(issue.userName, issue.repositoryName, issue.openedUserName)) handleComment(issue, Some(form.content), repository, actionOpt) map { case (issue, id) => redirect(s"/${repository.owner}/${repository.name}/${ if(issue.isPullRequest) "pull" else "issues"}/${form.issueId}#comment-${id}") @@ -171,7 +175,7 @@ trait IssuesControllerBase extends ControllerBase { post("/:owner/:repository/issue_comments/state", issueStateForm)(readableUsersOnly { (form, repository) => getIssue(repository.owner, repository.name, form.issueId.toString).flatMap { issue => - val actionOpt = params.get("action").filter(_ => isEditable(issue.userName, issue.repositoryName, issue.openedUserName)) + val actionOpt = params.get("action").filter(_ => isEditableContent(issue.userName, issue.repositoryName, issue.openedUserName)) handleComment(issue, form.content, repository, actionOpt) map { case (issue, id) => redirect(s"/${repository.owner}/${repository.name}/${ if(issue.isPullRequest) "pull" else "issues"}/${form.issueId}#comment-${id}") @@ -182,7 +186,7 @@ trait IssuesControllerBase extends ControllerBase { ajaxPost("/:owner/:repository/issue_comments/edit/:id", commentForm)(readableUsersOnly { (form, repository) => defining(repository.owner, repository.name){ case (owner, name) => getComment(owner, name, params("id")).map { comment => - if(isEditable(owner, name, comment.commentedUserName)){ + if(isEditableContent(owner, name, comment.commentedUserName)){ updateComment(comment.commentId, form.content) redirect(s"/${owner}/${name}/issue_comments/_data/${comment.commentId}") } else Unauthorized() @@ -193,7 +197,7 @@ trait IssuesControllerBase extends ControllerBase { ajaxPost("/:owner/:repository/issue_comments/delete/:id")(readableUsersOnly { repository => defining(repository.owner, repository.name){ case (owner, name) => getComment(owner, name, params("id")).map { comment => - if(isEditable(owner, name, comment.commentedUserName)){ + if(isEditableContent(owner, name, comment.commentedUserName)){ Ok(deleteComment(comment.commentId)) } else Unauthorized() } getOrElse NotFound() @@ -202,7 +206,7 @@ trait IssuesControllerBase extends ControllerBase { ajaxGet("/:owner/:repository/issues/_data/:id")(readableUsersOnly { repository => getIssue(repository.owner, repository.name, params("id")) map { x => - if(isEditable(x.userName, x.repositoryName, x.openedUserName)){ + if(isEditableContent(x.userName, x.repositoryName, x.openedUserName)){ params.get("dataType") collect { case t if t == "html" => html.editissue(x.content, x.issueId, repository) } getOrElse { @@ -218,7 +222,7 @@ trait IssuesControllerBase extends ControllerBase { enableAnchor = true, enableLineBreaks = true, enableTaskList = true, - hasWritePermission = isEditable(x.userName, x.repositoryName, x.openedUserName) + hasWritePermission = true ) ) ) @@ -229,7 +233,7 @@ trait IssuesControllerBase extends ControllerBase { ajaxGet("/:owner/:repository/issue_comments/_data/:id")(readableUsersOnly { repository => getComment(repository.owner, repository.name, params("id")) map { x => - if(isEditable(x.userName, x.repositoryName, x.commentedUserName)){ + if(isEditableContent(x.userName, x.repositoryName, x.commentedUserName)){ params.get("dataType") collect { case t if t == "html" => html.editcomment(x.content, x.commentId, repository) } getOrElse { @@ -244,7 +248,7 @@ trait IssuesControllerBase extends ControllerBase { enableAnchor = true, enableLineBreaks = true, enableTaskList = true, - hasWritePermission = isEditable(x.userName, x.repositoryName, x.commentedUserName) + hasWritePermission = true ) ) ) @@ -346,9 +350,6 @@ trait IssuesControllerBase extends ControllerBase { val assignedUserName = (key: String) => params.get(key) filter (_.trim != "") val milestoneId: String => Option[Int] = (key: String) => params.get(key).flatMap(_.toIntOpt) - private def isEditable(owner: String, repository: String, author: String)(implicit context: Context): Boolean = - hasWritePermission(owner, repository, context.loginAccount) || author == context.loginAccount.get.userName - private def executeBatch(repository: RepositoryService.RepositoryInfo)(execute: Int => Unit) = { params("checked").split(',') map(_.toInt) foreach execute params("from") match { @@ -359,8 +360,7 @@ trait IssuesControllerBase extends ControllerBase { private def searchIssues(repository: RepositoryService.RepositoryInfo) = { defining(repository.owner, repository.name){ case (owner, repoName) => - val page = IssueSearchCondition.page(request) - val sessionKey = Keys.Session.Issues(owner, repoName) + val page = IssueSearchCondition.page(request) // retrieve search condition val condition = IssueSearchCondition(request) @@ -376,8 +376,34 @@ trait IssuesControllerBase extends ControllerBase { countIssue(condition.copy(state = "closed"), false, owner -> repoName), condition, repository, - hasWritePermission(owner, repoName, context.loginAccount)) + isEditable(repository), + isManageable(repository)) } } + /** + * Tests whether an logged-in user can manage issues. + */ + private def isManageable(repository: RepositoryInfo)(implicit context: Context): Boolean = { + hasWritePermission(repository.owner, repository.name, context.loginAccount) + } + + /** + * Tests whether an logged-in user can post issues. + */ + private def isEditable(repository: RepositoryInfo)(implicit context: Context): Boolean = { + repository.repository.options.issuesOption match { + case "PUBLIC" => hasReadPermission(repository.owner, repository.name, context.loginAccount) + case "PRIVATE" => hasWritePermission(repository.owner, repository.name, context.loginAccount) + case "DISABLE" => false + } + } + + /** + * Tests whether an issue or a comment is editable by a logged-in user. + */ + private def isEditableContent(owner: String, repository: String, author: String)(implicit context: Context): Boolean = { + hasWritePermission(owner, repository, context.loginAccount) || author == context.loginAccount.get.userName + } + } diff --git a/src/main/scala/gitbucket/core/controller/PullRequestsController.scala b/src/main/scala/gitbucket/core/controller/PullRequestsController.scala index 4a97ae0ce..ae26bb009 100644 --- a/src/main/scala/gitbucket/core/controller/PullRequestsController.scala +++ b/src/main/scala/gitbucket/core/controller/PullRequestsController.scala @@ -6,6 +6,7 @@ import gitbucket.core.service.CommitStatusService import gitbucket.core.service.MergeService import gitbucket.core.service.IssuesService._ import gitbucket.core.service.PullRequestService._ +import gitbucket.core.service.RepositoryService.RepositoryInfo import gitbucket.core.service._ import gitbucket.core.util.ControlUtil._ import gitbucket.core.util.Directory._ @@ -14,7 +15,6 @@ import gitbucket.core.util.JGitUtil._ import gitbucket.core.util._ import gitbucket.core.view import gitbucket.core.view.helpers - import io.github.gitbucket.scalatra.forms._ import org.eclipse.jgit.api.Git import org.eclipse.jgit.lib.PersonIdent @@ -24,13 +24,15 @@ import scala.collection.JavaConverters._ class PullRequestsController extends PullRequestsControllerBase with RepositoryService with AccountService with IssuesService with PullRequestService with MilestonesService with LabelsService - with CommitsService with ActivityService with WebHookPullRequestService with ReferrerAuthenticator with CollaboratorsAuthenticator + with CommitsService with ActivityService with WebHookPullRequestService + with ReadableUsersAuthenticator with ReferrerAuthenticator with CollaboratorsAuthenticator with CommitStatusService with MergeService with ProtectedBranchService trait PullRequestsControllerBase extends ControllerBase { self: RepositoryService with AccountService with IssuesService with MilestonesService with LabelsService - with CommitsService with ActivityService with PullRequestService with WebHookPullRequestService with ReferrerAuthenticator with CollaboratorsAuthenticator + with CommitsService with ActivityService with PullRequestService with WebHookPullRequestService + with ReadableUsersAuthenticator with ReferrerAuthenticator with CollaboratorsAuthenticator with CommitStatusService with MergeService with ProtectedBranchService => val pullRequestForm = mapping( @@ -96,7 +98,8 @@ trait PullRequestsControllerBase extends ControllerBase { getLabels(owner, name), commits, diffs, - hasWritePermission(owner, name, context.loginAccount), + isEditable(repository), + isManageable(repository), repository, flash.toMap.map(f => f._1 -> f._2.toString)) } @@ -416,64 +419,68 @@ trait PullRequestsControllerBase extends ControllerBase { }) getOrElse NotFound() }) - post("/:owner/:repository/pulls/new", pullRequestForm)(referrersOnly { (form, repository) => + post("/:owner/:repository/pulls/new", pullRequestForm)(readableUsersOnly { (form, repository) => defining(repository.owner, repository.name){ case (owner, name) => - val writable = hasWritePermission(owner, name, context.loginAccount) - val loginUserName = context.loginAccount.get.userName + val manageable = isManageable(repository) + val editable = isEditable(repository) - val issueId = createIssue( - owner = repository.owner, - repository = repository.name, - loginUser = loginUserName, - title = form.title, - content = form.content, - assignedUserName = if(writable) form.assignedUserName else None, - milestoneId = if(writable) form.milestoneId else None, - isPullRequest = true) + if(editable) { + val loginUserName = context.loginAccount.get.userName - createPullRequest( - originUserName = repository.owner, - originRepositoryName = repository.name, - issueId = issueId, - originBranch = form.targetBranch, - requestUserName = form.requestUserName, - requestRepositoryName = form.requestRepositoryName, - requestBranch = form.requestBranch, - commitIdFrom = form.commitIdFrom, - commitIdTo = form.commitIdTo) + val issueId = createIssue( + owner = repository.owner, + repository = repository.name, + loginUser = loginUserName, + title = form.title, + content = form.content, + assignedUserName = if (manageable) form.assignedUserName else None, + milestoneId = if (manageable) form.milestoneId else None, + isPullRequest = true) - // insert labels - if(writable){ - form.labelNames.map { value => - val labels = getLabels(owner, name) - value.split(",").foreach { labelName => - labels.find(_.labelName == labelName).map { label => - registerIssueLabel(repository.owner, repository.name, issueId, label.labelId) + createPullRequest( + originUserName = repository.owner, + originRepositoryName = repository.name, + issueId = issueId, + originBranch = form.targetBranch, + requestUserName = form.requestUserName, + requestRepositoryName = form.requestRepositoryName, + requestBranch = form.requestBranch, + commitIdFrom = form.commitIdFrom, + commitIdTo = form.commitIdTo) + + // insert labels + if (manageable) { + form.labelNames.map { value => + val labels = getLabels(owner, name) + value.split(",").foreach { labelName => + labels.find(_.labelName == labelName).map { label => + registerIssueLabel(repository.owner, repository.name, issueId, label.labelId) + } } } } - } - // fetch requested branch - fetchAsPullRequest(owner, name, form.requestUserName, form.requestRepositoryName, form.requestBranch, issueId) + // fetch requested branch + fetchAsPullRequest(owner, name, form.requestUserName, form.requestRepositoryName, form.requestBranch, issueId) - // record activity - recordPullRequestActivity(owner, name, loginUserName, issueId, form.title) + // record activity + recordPullRequestActivity(owner, name, loginUserName, issueId, form.title) - // call web hook - callPullRequestWebHook("opened", repository, issueId, context.baseUrl, context.loginAccount.get) + // call web hook + callPullRequestWebHook("opened", repository, issueId, context.baseUrl, context.loginAccount.get) - getIssue(owner, name, issueId.toString) foreach { issue => - // extract references and create refer comment - createReferComment(owner, name, issue, form.title + " " + form.content.getOrElse(""), context.loginAccount.get) + getIssue(owner, name, issueId.toString) foreach { issue => + // extract references and create refer comment + createReferComment(owner, name, issue, form.title + " " + form.content.getOrElse(""), context.loginAccount.get) - // notifications - Notifier().toNotify(repository, issue, form.content.getOrElse("")){ - Notifier.msgPullRequest(s"${context.baseUrl}/${owner}/${name}/pull/${issueId}") + // notifications + Notifier().toNotify(repository, issue, form.content.getOrElse("")) { + Notifier.msgPullRequest(s"${context.baseUrl}/${owner}/${name}/pull/${issueId}") + } } - } - redirect(s"/${owner}/${name}/pull/${issueId}") + redirect(s"/${owner}/${name}/pull/${issueId}") + } else Unauthorized() } }) @@ -513,8 +520,7 @@ trait PullRequestsControllerBase extends ControllerBase { private def searchPullRequests(userName: Option[String], repository: RepositoryService.RepositoryInfo) = defining(repository.owner, repository.name){ case (owner, repoName) => - val page = IssueSearchCondition.page(request) - val sessionKey = Keys.Session.Pulls(owner, repoName) + val page = IssueSearchCondition.page(request) // retrieve search condition val condition = IssueSearchCondition(request) @@ -530,7 +536,26 @@ trait PullRequestsControllerBase extends ControllerBase { countIssue(condition.copy(state = "closed"), true, owner -> repoName), condition, repository, - hasWritePermission(owner, repoName, context.loginAccount)) + isEditable(repository), + isManageable(repository)) } + /** + * Tests whether an logged-in user can manage pull requests. + */ + private def isManageable(repository: RepositoryInfo)(implicit context: Context): Boolean = { + hasWritePermission(repository.owner, repository.name, context.loginAccount) + } + + /** + * Tests whether an logged-in user can post pull requests. + */ + private def isEditable(repository: RepositoryInfo)(implicit context: Context): Boolean = { + repository.repository.options.issuesOption match { + case "PUBLIC" => hasReadPermission(repository.owner, repository.name, context.loginAccount) + case "PRIVATE" => hasWritePermission(repository.owner, repository.name, context.loginAccount) + case "DISABLE" => false + } + } + } diff --git a/src/main/scala/gitbucket/core/controller/WikiController.scala b/src/main/scala/gitbucket/core/controller/WikiController.scala index 07153c1d7..beebfc28a 100644 --- a/src/main/scala/gitbucket/core/controller/WikiController.scala +++ b/src/main/scala/gitbucket/core/controller/WikiController.scala @@ -14,10 +14,10 @@ import org.scalatra.i18n.Messages class WikiController extends WikiControllerBase with WikiService with RepositoryService with AccountService with ActivityService - with CollaboratorsAuthenticator with ReferrerAuthenticator + with ReadableUsersAuthenticator with CollaboratorsAuthenticator with ReferrerAuthenticator trait WikiControllerBase extends ControllerBase { - self: WikiService with RepositoryService with ActivityService with CollaboratorsAuthenticator with ReferrerAuthenticator => + self: WikiService with RepositoryService with ActivityService with ReadableUsersAuthenticator with CollaboratorsAuthenticator with ReferrerAuthenticator => case class WikiPageEditForm(pageName: String, content: String, message: Option[String], currentPageName: String, id: String) @@ -62,7 +62,7 @@ trait WikiControllerBase extends ControllerBase { using(Git.open(getWikiRepositoryDir(repository.owner, repository.name))){ git => JGitUtil.getCommitLog(git, "master", path = pageName + ".md") match { - case Right((logs, hasNext)) => html.history(Some(pageName), logs, repository) + case Right((logs, hasNext)) => html.history(Some(pageName), logs, repository, isEditable(repository)) case Left(_) => NotFound() } } @@ -87,7 +87,7 @@ trait WikiControllerBase extends ControllerBase { } }) - get("/:owner/:repository/wiki/:page/_revert/:commitId")(referrersOnly { repository => + get("/:owner/:repository/wiki/:page/_revert/:commitId")(readableUsersOnly { repository => if(isEditable(repository)){ val pageName = StringUtil.urlDecode(params("page")) val Array(from, to) = params("commitId").split("\\.\\.\\.") @@ -101,7 +101,7 @@ trait WikiControllerBase extends ControllerBase { } else Unauthorized() }) - get("/:owner/:repository/wiki/_revert/:commitId")(referrersOnly { repository => + get("/:owner/:repository/wiki/_revert/:commitId")(readableUsersOnly { repository => if(isEditable(repository)){ val Array(from, to) = params("commitId").split("\\.\\.\\.") @@ -114,14 +114,14 @@ trait WikiControllerBase extends ControllerBase { } else Unauthorized() }) - get("/:owner/:repository/wiki/:page/_edit")(referrersOnly { repository => + get("/:owner/:repository/wiki/:page/_edit")(readableUsersOnly { repository => if(isEditable(repository)){ val pageName = StringUtil.urlDecode(params("page")) html.edit(pageName, getWikiPage(repository.owner, repository.name, pageName), repository) } else Unauthorized() }) - post("/:owner/:repository/wiki/_edit", editForm)(referrersOnly { (form, repository) => + post("/:owner/:repository/wiki/_edit", editForm)(readableUsersOnly { (form, repository) => if(isEditable(repository)){ defining(context.loginAccount.get){ loginAccount => saveWikiPage( @@ -146,13 +146,13 @@ trait WikiControllerBase extends ControllerBase { } else Unauthorized() }) - get("/:owner/:repository/wiki/_new")(referrersOnly { repository => + get("/:owner/:repository/wiki/_new")(readableUsersOnly { repository => if(isEditable(repository)){ html.edit("", None, repository) } else Unauthorized() }) - post("/:owner/:repository/wiki/_new", newForm)(referrersOnly { (form, repository) => + post("/:owner/:repository/wiki/_new", newForm)(readableUsersOnly { (form, repository) => if(isEditable(repository)){ defining(context.loginAccount.get){ loginAccount => saveWikiPage(repository.owner, repository.name, form.currentPageName, form.pageName, @@ -170,7 +170,7 @@ trait WikiControllerBase extends ControllerBase { } else Unauthorized() }) - get("/:owner/:repository/wiki/:page/_delete")(referrersOnly { repository => + get("/:owner/:repository/wiki/:page/_delete")(readableUsersOnly { repository => if(isEditable(repository)){ val pageName = StringUtil.urlDecode(params("page")) @@ -182,7 +182,7 @@ trait WikiControllerBase extends ControllerBase { } } else Unauthorized() }) - + get("/:owner/:repository/wiki/_pages")(referrersOnly { repository => html.pages(getWikiPageList(repository.owner, repository.name), repository, isEditable(repository)) }) @@ -190,7 +190,7 @@ trait WikiControllerBase extends ControllerBase { get("/:owner/:repository/wiki/_history")(referrersOnly { repository => using(Git.open(getWikiRepositoryDir(repository.owner, repository.name))){ git => JGitUtil.getCommitLog(git, "master") match { - case Right((logs, hasNext)) => html.history(None, logs, repository) + case Right((logs, hasNext)) => html.history(None, logs, repository, isEditable(repository)) case Left(_) => NotFound() } } @@ -242,7 +242,7 @@ trait WikiControllerBase extends ControllerBase { private def isEditable(repository: RepositoryInfo)(implicit context: Context): Boolean = { repository.repository.options.wikiOption match { - case "ALL" => repository.repository.isPrivate == false || hasReadPermission(repository.owner, repository.name, context.loginAccount) +// case "ALL" => repository.repository.isPrivate == false || hasReadPermission(repository.owner, repository.name, context.loginAccount) case "PUBLIC" => hasReadPermission(repository.owner, repository.name, context.loginAccount) case "PRIVATE" => hasWritePermission(repository.owner, repository.name, context.loginAccount) case "DISABLE" => false diff --git a/src/main/scala/gitbucket/core/service/HandleCommentService.scala b/src/main/scala/gitbucket/core/service/HandleCommentService.scala index 6a1b06869..22cfc8801 100644 --- a/src/main/scala/gitbucket/core/service/HandleCommentService.scala +++ b/src/main/scala/gitbucket/core/service/HandleCommentService.scala @@ -13,7 +13,7 @@ trait HandleCommentService { with WebHookService with WebHookIssueCommentService with WebHookPullRequestService => /** - * @see [[https://github.com/takezoe/gitbucket/wiki/CommentAction]] + * @see [[https://github.com/gitbucket/gitbucket/wiki/CommentAction]] */ def handleComment(issue: Issue, content: Option[String], repository: RepositoryService.RepositoryInfo, actionOpt: Option[String]) (implicit context: Context, s: Session) = { @@ -54,18 +54,20 @@ trait HandleCommentService { // call web hooks action match { - case None => commentId.map{ commentIdSome => callIssueCommentWebHook(repository, issue, commentIdSome, context.loginAccount.get) } - case Some(act) => val webHookAction = act match { - case "open" => "opened" - case "reopen" => "reopened" - case "close" => "closed" - case _ => act - } - if(issue.isPullRequest){ + case None => commentId.map { commentIdSome => callIssueCommentWebHook(repository, issue, commentIdSome, context.loginAccount.get) } + case Some(act) => { + val webHookAction = act match { + case "open" => "opened" + case "reopen" => "reopened" + case "close" => "closed" + case _ => act + } + if (issue.isPullRequest) { callPullRequestWebHook(webHookAction, repository, issue.issueId, context.baseUrl, context.loginAccount.get) } else { callIssuesWebHook(webHookAction, repository, issue, context.baseUrl, context.loginAccount.get) } + } } // notifications diff --git a/src/main/scala/gitbucket/core/service/RepositoryService.scala b/src/main/scala/gitbucket/core/service/RepositoryService.scala index 96bbe8106..9fa8552bb 100644 --- a/src/main/scala/gitbucket/core/service/RepositoryService.scala +++ b/src/main/scala/gitbucket/core/service/RepositoryService.scala @@ -363,15 +363,15 @@ trait RepositoryService { self: AccountService => val q1 = Collaborators .innerJoin(Accounts).on { case (t1, t2) => (t1.collaboratorName === t2.userName) && (t2.groupAccount === false.bind) } .filter { case (t1, t2) => t1.byRepository(userName, repositoryName) } - .map { case (t1, t2) => t1.collaboratorName } + .map { case (t1, t2) => (t1.collaboratorName, t1.permission) } val q2 = Collaborators .innerJoin(Accounts).on { case (t1, t2) => (t1.collaboratorName === t2.userName) && (t2.groupAccount === true.bind) } .innerJoin(GroupMembers).on { case ((t1, t2), t3) => t2.userName === t3.groupName } .filter { case ((t1, t2), t3) => t1.byRepository(userName, repositoryName) } - .map { case ((t1, t2), t3) => t3.userName } + .map { case ((t1, t2), t3) => (t3.userName, t1.permission) } - q1.union(q2).list.filter { x => filter.isEmpty || filter.exists(_.name == x) } + q1.union(q2).list.filter { x => filter.isEmpty || filter.exists(_.name == x._2) }.map(_._1) } diff --git a/src/main/twirl/gitbucket/core/issues/commentform.scala.html b/src/main/twirl/gitbucket/core/issues/commentform.scala.html index 9f11990b0..32316c781 100644 --- a/src/main/twirl/gitbucket/core/issues/commentform.scala.html +++ b/src/main/twirl/gitbucket/core/issues/commentform.scala.html @@ -1,9 +1,10 @@ @(issue: gitbucket.core.model.Issue, reopenable: Boolean, - hasWritePermission: Boolean, + isEditable: Boolean, + isManageable: Boolean, repository: gitbucket.core.service.RepositoryService.RepositoryInfo)(implicit context: gitbucket.core.controller.Context) @import gitbucket.core.view.helpers -@if(context.loginAccount.isDefined){ +@if(isEditable){

    @helpers.avatarLink(context.loginAccount.get.userName, 48)
    @@ -16,7 +17,7 @@ enableRefsLink = true, enableLineBreaks = true, enableTaskList = true, - hasWritePermission = hasWritePermission, + hasWritePermission = isEditable, completionContext = "issues", style = "", elastic = true, @@ -24,7 +25,7 @@ )
    - @if((reopenable || !issue.closed) && (hasWritePermission || issue.openedUserName == context.loginAccount.get.userName)){ + @if((reopenable || !issue.closed) && (isManageable || issue.openedUserName == context.loginAccount.get.userName)){ } diff --git a/src/main/twirl/gitbucket/core/issues/commentlist.scala.html b/src/main/twirl/gitbucket/core/issues/commentlist.scala.html index 268c7a58d..a2172c95f 100644 --- a/src/main/twirl/gitbucket/core/issues/commentlist.scala.html +++ b/src/main/twirl/gitbucket/core/issues/commentlist.scala.html @@ -1,6 +1,6 @@ @(issue: Option[gitbucket.core.model.Issue], comments: List[gitbucket.core.model.Comment], - hasWritePermission: Boolean, + isManageable: Boolean, repository: gitbucket.core.service.RepositoryService.RepositoryInfo, pullreq: Option[gitbucket.core.model.PullRequest] = None)(implicit context: gitbucket.core.controller.Context) @import gitbucket.core.view.helpers @@ -11,7 +11,7 @@
    @helpers.user(issue.get.openedUserName, styleClass="username strong") commented @gitbucket.core.helper.html.datetimeago(issue.get.registeredDate) - @if(hasWritePermission || context.loginAccount.map(_.userName == issue.get.openedUserName).getOrElse(false)){ + @if(isManageable || context.loginAccount.map(_.userName == issue.get.openedUserName).getOrElse(false)){ } @@ -24,7 +24,7 @@ enableRefsLink = true, enableLineBreaks = true, enableTaskList = true, - hasWritePermission = hasWritePermission + hasWritePermission = isManageable )
    @@ -48,7 +48,7 @@ @gitbucket.core.helper.html.datetimeago(comment.registeredDate) @if(comment.action != "commit" && comment.action != "merge" && comment.action != "refer" - && (hasWritePermission || context.loginAccount.map(_.userName == comment.commentedUserName).getOrElse(false))){ + && (isManageable || context.loginAccount.map(_.userName == comment.commentedUserName).getOrElse(false))){   @@ -63,7 +63,7 @@ enableRefsLink = true, enableLineBreaks = true, enableTaskList = true, - hasWritePermission = hasWritePermission + hasWritePermission = isManageable )
    • @@ -166,7 +166,7 @@ } } case comment: CommitComment => { - @gitbucket.core.helper.html.commitcomment(comment, hasWritePermission, repository, pullreq.map(_.commitIdTo)) + @gitbucket.core.helper.html.commitcomment(comment, isManageable, repository, pullreq.map(_.commitIdTo)) } } \ No newline at end of file diff --git a/src/main/twirl/gitbucket/core/settings/danger.scala.html b/src/main/twirl/gitbucket/core/settings/danger.scala.html index 60b400758..7df117034 100644 --- a/src/main/twirl/gitbucket/core/settings/danger.scala.html +++ b/src/main/twirl/gitbucket/core/settings/danger.scala.html @@ -13,7 +13,7 @@
    Transfer this repo to another user or to group.
    - @gitbucket.core.helper.html.account("newOwner", 200) + @gitbucket.core.helper.html.account("newOwner", 200, true, true)
    From d70c6cece7a6fe8bc59decb926e036d9ed7a4819 Mon Sep 17 00:00:00 2001 From: Naoki Takezoe Date: Sun, 6 Nov 2016 16:37:44 +0900 Subject: [PATCH 12/14] (refs #1286) Fix testcase --- .../scala/gitbucket/core/service/PullRequestServiceSpec.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/scala/gitbucket/core/service/PullRequestServiceSpec.scala b/src/test/scala/gitbucket/core/service/PullRequestServiceSpec.scala index ca55903e5..d6ddff9e7 100644 --- a/src/test/scala/gitbucket/core/service/PullRequestServiceSpec.scala +++ b/src/test/scala/gitbucket/core/service/PullRequestServiceSpec.scala @@ -3,7 +3,8 @@ package gitbucket.core.service import gitbucket.core.model._ import org.scalatest.FunSpec -class PullRequestServiceSpec extends FunSpec with ServiceSpecBase with PullRequestService with IssuesService with AccountService { +class PullRequestServiceSpec extends FunSpec with ServiceSpecBase + with PullRequestService with IssuesService with AccountService with RepositoryService { def swap(r: (Issue, PullRequest)) = (r._2 -> r._1) From 9eb9fc666c05685f9b20638661f76fb89403a44d Mon Sep 17 00:00:00 2001 From: Naoki Takezoe Date: Tue, 8 Nov 2016 02:28:50 +0900 Subject: [PATCH 13/14] (refs #1286) Bugfix --- .../core/controller/IssuesController.scala | 29 ++++++++++--------- .../core/service/RepositoryService.scala | 2 +- .../gitbucket/core/util/Authenticator.scala | 2 ++ .../gitbucket/core/issues/issue.scala.html | 6 ++-- 4 files changed, 22 insertions(+), 17 deletions(-) diff --git a/src/main/scala/gitbucket/core/controller/IssuesController.scala b/src/main/scala/gitbucket/core/controller/IssuesController.scala index 703dbefbb..a4b01f293 100644 --- a/src/main/scala/gitbucket/core/controller/IssuesController.scala +++ b/src/main/scala/gitbucket/core/controller/IssuesController.scala @@ -78,21 +78,22 @@ trait IssuesControllerBase extends ControllerBase { }) get("/:owner/:repository/issues/new")(readableUsersOnly { repository => - defining(repository.owner, repository.name){ case (owner, name) => - html.create( - getAssignableUserNames(owner, name), - getMilestones(owner, name), - getLabels(owner, name), - hasWritePermission(owner, name, context.loginAccount), - repository) - } + if(isEditable(repository)){ // TODO Should this check is provided by authenticator? + defining(repository.owner, repository.name){ case (owner, name) => + html.create( + getAssignableUserNames(owner, name), + getMilestones(owner, name), + getLabels(owner, name), + hasWritePermission(owner, name, context.loginAccount), + repository) + } + } else Unauthorized() }) post("/:owner/:repository/issues/new", issueCreateForm)(readableUsersOnly { (form, repository) => - defining(repository.owner, repository.name){ case (owner, name) => - val manageable = isManageable(repository) - val editable = isEditable(repository) - if(editable) { + if(isEditable(repository)){ // TODO Should this check is provided by authenticator? + defining(repository.owner, repository.name){ case (owner, name) => + val manageable = isManageable(repository) val userName = context.loginAccount.get.userName // insert issue @@ -129,8 +130,8 @@ trait IssuesControllerBase extends ControllerBase { } redirect(s"/${owner}/${name}/issues/${issueId}") - } else Unauthorized() - } + } + } else Unauthorized() }) ajaxPost("/:owner/:repository/issues/edit_title/:id", issueTitleEditForm)(readableUsersOnly { (title, repository) => diff --git a/src/main/scala/gitbucket/core/service/RepositoryService.scala b/src/main/scala/gitbucket/core/service/RepositoryService.scala index 9fa8552bb..26fa1599a 100644 --- a/src/main/scala/gitbucket/core/service/RepositoryService.scala +++ b/src/main/scala/gitbucket/core/service/RepositoryService.scala @@ -38,7 +38,7 @@ trait RepositoryService { self: AccountService => parentUserName = parentUserName, parentRepositoryName = parentRepositoryName, options = RepositoryOptions( - issuesOption = "PRIVATE", // TODO DISABLE for the forked repository? + issuesOption = "PUBLIC", // TODO DISABLE for the forked repository? externalIssuesUrl = None, wikiOption = "PUBLIC", // TODO DISABLE for the forked repository? externalWikiUrl = None, diff --git a/src/main/scala/gitbucket/core/util/Authenticator.scala b/src/main/scala/gitbucket/core/util/Authenticator.scala index 57f39421b..6d82508b2 100644 --- a/src/main/scala/gitbucket/core/util/Authenticator.scala +++ b/src/main/scala/gitbucket/core/util/Authenticator.scala @@ -90,6 +90,8 @@ trait AdminAuthenticator { self: ControllerBase => /** * Allows only collaborators and administrators. + * + * TODO This authenticator should be renamed. */ trait CollaboratorsAuthenticator { self: ControllerBase with RepositoryService with AccountService => protected def collaboratorsOnly(action: (RepositoryInfo) => Any) = { authenticate(action) } diff --git a/src/main/twirl/gitbucket/core/issues/issue.scala.html b/src/main/twirl/gitbucket/core/issues/issue.scala.html index 4691b83a6..4b494adc4 100644 --- a/src/main/twirl/gitbucket/core/issues/issue.scala.html +++ b/src/main/twirl/gitbucket/core/issues/issue.scala.html @@ -15,7 +15,9 @@ @if(isManageable || context.loginAccount.map(_.userName == issue.openedUserName).getOrElse(false)){ Edit } - New issue + @if(isEditable){ + New issue + }
    } } -