From 7ca05374db6f6af9de06665c9d2d08acfe85aa4f Mon Sep 17 00:00:00 2001
From: James Moger <james.moger@gitblit.com>
Date: Wed, 05 Mar 2014 12:26:14 -0500
Subject: [PATCH] Centralized ticket editing permission controls
---
src/main/java/com/gitblit/models/UserModel.java | 17 +++
src/main/java/com/gitblit/wicket/pages/EditTicketPage.java | 170 ++++++++++++++++++++----------------------
src/main/java/com/gitblit/wicket/pages/EditTicketPage.html | 7 +
src/main/java/com/gitblit/wicket/pages/TicketPage.java | 3
4 files changed, 101 insertions(+), 96 deletions(-)
diff --git a/src/main/java/com/gitblit/models/UserModel.java b/src/main/java/com/gitblit/models/UserModel.java
index fbf311a..fee9c17 100644
--- a/src/main/java/com/gitblit/models/UserModel.java
+++ b/src/main/java/com/gitblit/models/UserModel.java
@@ -447,16 +447,23 @@
return canAdmin() || model.isUsersPersonalRepository(username) || model.isOwner(username);
}
+ public boolean canEdit(TicketModel ticket, RepositoryModel repository) {
+ return isAuthenticated() &&
+ (username.equals(ticket.createdBy)
+ || username.equals(ticket.responsible)
+ || canPush(repository));
+ }
+
public boolean canReviewPatchset(RepositoryModel model) {
- return isAuthenticated && canClone(model);
+ return isAuthenticated() && canClone(model);
}
public boolean canApprovePatchset(RepositoryModel model) {
- return isAuthenticated && canPush(model);
+ return isAuthenticated() && canPush(model);
}
public boolean canVetoPatchset(RepositoryModel model) {
- return isAuthenticated && canPush(model);
+ return isAuthenticated() && canPush(model);
}
/**
@@ -540,6 +547,10 @@
return false;
}
+ public boolean isAuthenticated() {
+ return !UserModel.ANONYMOUS.equals(this) && isAuthenticated;
+ }
+
public boolean isTeamMember(String teamname) {
for (TeamModel team : teams) {
if (team.name.equalsIgnoreCase(teamname)) {
diff --git a/src/main/java/com/gitblit/wicket/pages/EditTicketPage.html b/src/main/java/com/gitblit/wicket/pages/EditTicketPage.html
index b3c102d..b5fe0ae 100644
--- a/src/main/java/com/gitblit/wicket/pages/EditTicketPage.html
+++ b/src/main/java/com/gitblit/wicket/pages/EditTicketPage.html
@@ -38,8 +38,7 @@
</div>
</div>
</td></tr>
- <tr><th><wicket:message key="gb.type"></wicket:message><span style="color:red;">*</span></th><td class="edit"><select class="input-large" wicket:id="type"></select></td></tr>
- <tr><th><wicket:message key="gb.status"></wicket:message><span style="color:red;">*</span></th><td class="edit"><select class="input-large" wicket:id="status"></select></td></tr>
+ <tr wicket:id="status"></tr>
<tr wicket:id="responsible"></tr>
<tr wicket:id="milestone"></tr>
<tr wicket:id="mergeto"></tr>
@@ -56,6 +55,10 @@
</div>
</body>
+<wicket:fragment wicket:id="statusFragment">
+ <tr><th><wicket:message key="gb.status"></wicket:message><span style="color:red;">*</span></th><td class="edit"><select class="input-large" wicket:id="status"></select></td></tr>
+</wicket:fragment>
+
<wicket:fragment wicket:id="responsibleFragment">
<th><wicket:message key="gb.responsible"></wicket:message></th><td class="edit"><select class="input-large" wicket:id="responsible"></select></td>
</wicket:fragment>
diff --git a/src/main/java/com/gitblit/wicket/pages/EditTicketPage.java b/src/main/java/com/gitblit/wicket/pages/EditTicketPage.java
index ac7ff97..5fa3197 100644
--- a/src/main/java/com/gitblit/wicket/pages/EditTicketPage.java
+++ b/src/main/java/com/gitblit/wicket/pages/EditTicketPage.java
@@ -88,11 +88,6 @@
currentUser = UserModel.ANONYMOUS;
}
- if (!currentUser.isAuthenticated || !app().tickets().isAcceptingTicketUpdates(getRepositoryModel())) {
- // tickets prohibited
- setResponsePage(TicketsPage.class, WicketUtils.newRepositoryParameter(repositoryName));
- }
-
long ticketId = 0L;
try {
String h = WicketUtils.getObject(params);
@@ -102,8 +97,10 @@
}
TicketModel ticket = app().tickets().getTicket(getRepositoryModel(), ticketId);
- if (ticket == null) {
- setResponsePage(TicketsPage.class, WicketUtils.newRepositoryParameter(repositoryName));
+ if (ticket == null
+ || !currentUser.canEdit(ticket, getRepositoryModel())
+ || !app().tickets().isAcceptingTicketUpdates(getRepositoryModel())) {
+ setResponsePage(TicketsPage.class, WicketUtils.newObjectParameter(repositoryName, "" + ticketId));
}
typeModel = Model.of(ticket.type);
@@ -223,18 +220,6 @@
}
form.add(new DropDownChoice<TicketModel.Type>("type", typeModel, typeChoices));
- List<Status> statusChoices;
- if (ticket.isClosed()) {
- statusChoices = Arrays.asList(ticket.status, Status.Open);
- } else if (ticket.isProposal()) {
- statusChoices = Arrays.asList(TicketModel.Status.proposalWorkflow);
- } else if (ticket.isBug()) {
- statusChoices = Arrays.asList(TicketModel.Status.bugWorkflow);
- } else {
- statusChoices = Arrays.asList(TicketModel.Status.requestWorkflow);
- }
- form.add(new DropDownChoice<TicketModel.Status>("status", statusModel, statusChoices));
-
form.add(new TextField<String>("title", titleModel));
form.add(new TextField<String>("topic", topicModel));
@@ -249,78 +234,85 @@
descriptionEditor.setText(ticket.body);
form.add(descriptionEditor);
- if (currentUser != null && currentUser.isAuthenticated && currentUser.canPush(getRepositoryModel())) {
- // responsible
- Set<String> userlist = new TreeSet<String>(ticket.getParticipants());
-
- for (RegistrantAccessPermission rp : app().repositories().getUserAccessPermissions(getRepositoryModel())) {
- if (rp.permission.atLeast(AccessPermission.PUSH) && !rp.isTeam()) {
- userlist.add(rp.registrant);
- }
- }
-
- List<TicketResponsible> responsibles = new ArrayList<TicketResponsible>();
- for (String username : userlist) {
- UserModel user = app().users().getUserModel(username);
- if (user != null) {
- TicketResponsible responsible = new TicketResponsible(user);
- responsibles.add(responsible);
- if (user.username.equals(ticket.responsible)) {
- responsibleModel.setObject(responsible);
- }
- }
- }
- Collections.sort(responsibles);
- responsibles.add(new TicketResponsible(NIL, "", ""));
- Fragment responsible = new Fragment("responsible", "responsibleFragment", this);
- responsible.add(new DropDownChoice<TicketResponsible>("responsible", responsibleModel, responsibles));
- form.add(responsible.setVisible(!responsibles.isEmpty()));
-
- // milestone
- List<TicketMilestone> milestones = app().tickets().getMilestones(getRepositoryModel(), Status.Open);
- for (TicketMilestone milestone : milestones) {
- if (milestone.name.equals(ticket.milestone)) {
- milestoneModel.setObject(milestone);
- break;
- }
- }
- if (milestoneModel.getObject() == null && !StringUtils.isEmpty(ticket.milestone)) {
- // ensure that this unrecognized milestone is listed
- // so that we get the <nil> selection.
- TicketMilestone tms = new TicketMilestone(ticket.milestone);
- milestones.add(tms);
- milestoneModel.setObject(tms);
- }
- if (!milestones.isEmpty()) {
- milestones.add(new TicketMilestone(NIL));
- }
-
- Fragment milestone = new Fragment("milestone", "milestoneFragment", this);
-
- milestone.add(new DropDownChoice<TicketMilestone>("milestone", milestoneModel, milestones));
- form.add(milestone.setVisible(!milestones.isEmpty()));
-
- // mergeTo (integration branch)
- List<String> branches = new ArrayList<String>();
- for (String branch : getRepositoryModel().getLocalBranches()) {
- // exclude ticket branches
- if (!branch.startsWith(Constants.R_TICKET)) {
- branches.add(Repository.shortenRefName(branch));
- }
- }
- branches.remove(Repository.shortenRefName(getRepositoryModel().HEAD));
- branches.add(0, Repository.shortenRefName(getRepositoryModel().HEAD));
-
- Fragment mergeto = new Fragment("mergeto", "mergeToFragment", this);
- mergeto.add(new DropDownChoice<String>("mergeto", mergeToModel, branches));
- form.add(mergeto.setVisible(!branches.isEmpty()));
-
+ // status
+ List<Status> statusChoices;
+ if (ticket.isClosed()) {
+ statusChoices = Arrays.asList(ticket.status, Status.Open);
+ } else if (ticket.isProposal()) {
+ statusChoices = Arrays.asList(TicketModel.Status.proposalWorkflow);
+ } else if (ticket.isBug()) {
+ statusChoices = Arrays.asList(TicketModel.Status.bugWorkflow);
} else {
- // user does not have permission to assign milestone or responsible
- form.add(new Label("responsible").setVisible(false));
- form.add(new Label("milestone").setVisible(false));
- form.add(new Label("mergeto").setVisible(false));
+ statusChoices = Arrays.asList(TicketModel.Status.requestWorkflow);
}
+ Fragment status = new Fragment("status", "statusFragment", this);
+ status.add(new DropDownChoice<TicketModel.Status>("status", statusModel, statusChoices));
+ form.add(status);
+
+ // responsible
+ Set<String> userlist = new TreeSet<String>(ticket.getParticipants());
+
+ for (RegistrantAccessPermission rp : app().repositories().getUserAccessPermissions(getRepositoryModel())) {
+ if (rp.permission.atLeast(AccessPermission.PUSH) && !rp.isTeam()) {
+ userlist.add(rp.registrant);
+ }
+ }
+
+ List<TicketResponsible> responsibles = new ArrayList<TicketResponsible>();
+ for (String username : userlist) {
+ UserModel user = app().users().getUserModel(username);
+ if (user != null) {
+ TicketResponsible responsible = new TicketResponsible(user);
+ responsibles.add(responsible);
+ if (user.username.equals(ticket.responsible)) {
+ responsibleModel.setObject(responsible);
+ }
+ }
+ }
+ Collections.sort(responsibles);
+ responsibles.add(new TicketResponsible(NIL, "", ""));
+ Fragment responsible = new Fragment("responsible", "responsibleFragment", this);
+ responsible.add(new DropDownChoice<TicketResponsible>("responsible", responsibleModel, responsibles));
+ form.add(responsible.setVisible(!responsibles.isEmpty()));
+
+ // milestone
+ List<TicketMilestone> milestones = app().tickets().getMilestones(getRepositoryModel(), Status.Open);
+ for (TicketMilestone milestone : milestones) {
+ if (milestone.name.equals(ticket.milestone)) {
+ milestoneModel.setObject(milestone);
+ break;
+ }
+ }
+ if (milestoneModel.getObject() == null && !StringUtils.isEmpty(ticket.milestone)) {
+ // ensure that this unrecognized milestone is listed
+ // so that we get the <nil> selection.
+ TicketMilestone tms = new TicketMilestone(ticket.milestone);
+ milestones.add(tms);
+ milestoneModel.setObject(tms);
+ }
+ if (!milestones.isEmpty()) {
+ milestones.add(new TicketMilestone(NIL));
+ }
+
+ Fragment milestone = new Fragment("milestone", "milestoneFragment", this);
+
+ milestone.add(new DropDownChoice<TicketMilestone>("milestone", milestoneModel, milestones));
+ form.add(milestone.setVisible(!milestones.isEmpty()));
+
+ // mergeTo (integration branch)
+ List<String> branches = new ArrayList<String>();
+ for (String branch : getRepositoryModel().getLocalBranches()) {
+ // exclude ticket branches
+ if (!branch.startsWith(Constants.R_TICKET)) {
+ branches.add(Repository.shortenRefName(branch));
+ }
+ }
+ branches.remove(Repository.shortenRefName(getRepositoryModel().HEAD));
+ branches.add(0, Repository.shortenRefName(getRepositoryModel().HEAD));
+
+ Fragment mergeto = new Fragment("mergeto", "mergeToFragment", this);
+ mergeto.add(new DropDownChoice<String>("mergeto", mergeToModel, branches));
+ form.add(mergeto.setVisible(!branches.isEmpty()));
form.add(new Button("update"));
Button cancel = new Button("cancel") {
diff --git a/src/main/java/com/gitblit/wicket/pages/TicketPage.java b/src/main/java/com/gitblit/wicket/pages/TicketPage.java
index 3f92eaa..5e0e682 100644
--- a/src/main/java/com/gitblit/wicket/pages/TicketPage.java
+++ b/src/main/java/com/gitblit/wicket/pages/TicketPage.java
@@ -115,7 +115,6 @@
super(params);
final UserModel user = GitBlitWebSession.get().getUser() == null ? UserModel.ANONYMOUS : GitBlitWebSession.get().getUser();
- final boolean isAuthenticated = !UserModel.ANONYMOUS.equals(user) && user.isAuthenticated;
final RepositoryModel repository = getRepositoryModel();
final String id = WicketUtils.getObject(params);
long ticketId = Long.parseLong(id);
@@ -327,7 +326,7 @@
/*
* UPDATE FORM (DISCUSSION TAB)
*/
- if (isAuthenticated && app().tickets().isAcceptingTicketUpdates(repository)) {
+ if (user.canEdit(ticket, repository) && app().tickets().isAcceptingTicketUpdates(repository)) {
if (ticket.isOpen()) {
/*
* OPEN TICKET
--
Gitblit v1.9.1