From 7ca05374db6f6af9de06665c9d2d08acfe85aa4f Mon Sep 17 00:00:00 2001 From: James Moger Date: Wed, 5 Mar 2014 12:26:14 -0500 Subject: Centralized ticket editing permission controls --- src/main/java/com/gitblit/models/UserModel.java | 17 ++- .../com/gitblit/wicket/pages/EditTicketPage.html | 7 +- .../com/gitblit/wicket/pages/EditTicketPage.java | 152 ++++++++++----------- .../java/com/gitblit/wicket/pages/TicketPage.java | 3 +- 4 files changed, 92 insertions(+), 87 deletions(-) (limited to 'src') diff --git a/src/main/java/com/gitblit/models/UserModel.java b/src/main/java/com/gitblit/models/UserModel.java index fbf311a5..fee9c172 100644 --- a/src/main/java/com/gitblit/models/UserModel.java +++ b/src/main/java/com/gitblit/models/UserModel.java @@ -447,16 +447,23 @@ public class UserModel implements Principal, Serializable, Comparable 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 @@ public class UserModel implements Principal, Serializable, Comparable 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 b3c102d9..b5fe0ae5 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 @@ - * - * + @@ -56,6 +55,10 @@ + + * + + diff --git a/src/main/java/com/gitblit/wicket/pages/EditTicketPage.java b/src/main/java/com/gitblit/wicket/pages/EditTicketPage.java index ac7ff97e..5fa31975 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 @@ public class EditTicketPage extends RepositoryPage { 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 @@ public class EditTicketPage extends RepositoryPage { } 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 @@ public class EditTicketPage extends RepositoryPage { } form.add(new DropDownChoice("type", typeModel, typeChoices)); - List 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("status", statusModel, statusChoices)); - form.add(new TextField("title", titleModel)); form.add(new TextField("topic", topicModel)); @@ -249,78 +234,85 @@ public class EditTicketPage extends RepositoryPage { descriptionEditor.setText(ticket.body); form.add(descriptionEditor); - if (currentUser != null && currentUser.isAuthenticated && currentUser.canPush(getRepositoryModel())) { - // responsible - Set userlist = new TreeSet(ticket.getParticipants()); + // status + List 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); + } + Fragment status = new Fragment("status", "statusFragment", this); + status.add(new DropDownChoice("status", statusModel, statusChoices)); + form.add(status); - for (RegistrantAccessPermission rp : app().repositories().getUserAccessPermissions(getRepositoryModel())) { - if (rp.permission.atLeast(AccessPermission.PUSH) && !rp.isTeam()) { - userlist.add(rp.registrant); - } - } + // responsible + Set userlist = new TreeSet(ticket.getParticipants()); - List responsibles = new ArrayList(); - 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); - } - } + for (RegistrantAccessPermission rp : app().repositories().getUserAccessPermissions(getRepositoryModel())) { + if (rp.permission.atLeast(AccessPermission.PUSH) && !rp.isTeam()) { + userlist.add(rp.registrant); } - Collections.sort(responsibles); - responsibles.add(new TicketResponsible(NIL, "", "")); - Fragment responsible = new Fragment("responsible", "responsibleFragment", this); - responsible.add(new DropDownChoice("responsible", responsibleModel, responsibles)); - form.add(responsible.setVisible(!responsibles.isEmpty())); - - // milestone - List milestones = app().tickets().getMilestones(getRepositoryModel(), Status.Open); - for (TicketMilestone milestone : milestones) { - if (milestone.name.equals(ticket.milestone)) { - milestoneModel.setObject(milestone); - break; + } + + List responsibles = new ArrayList(); + 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); } } - if (milestoneModel.getObject() == null && !StringUtils.isEmpty(ticket.milestone)) { - // ensure that this unrecognized milestone is listed - // so that we get the selection. - TicketMilestone tms = new TicketMilestone(ticket.milestone); - milestones.add(tms); - milestoneModel.setObject(tms); - } - if (!milestones.isEmpty()) { - milestones.add(new TicketMilestone(NIL)); + } + Collections.sort(responsibles); + responsibles.add(new TicketResponsible(NIL, "", "")); + Fragment responsible = new Fragment("responsible", "responsibleFragment", this); + responsible.add(new DropDownChoice("responsible", responsibleModel, responsibles)); + form.add(responsible.setVisible(!responsibles.isEmpty())); + + // milestone + List 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 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); + Fragment milestone = new Fragment("milestone", "milestoneFragment", this); - milestone.add(new DropDownChoice("milestone", milestoneModel, milestones)); - form.add(milestone.setVisible(!milestones.isEmpty())); + milestone.add(new DropDownChoice("milestone", milestoneModel, milestones)); + form.add(milestone.setVisible(!milestones.isEmpty())); - // mergeTo (integration branch) - List branches = new ArrayList(); - for (String branch : getRepositoryModel().getLocalBranches()) { - // exclude ticket branches - if (!branch.startsWith(Constants.R_TICKET)) { - branches.add(Repository.shortenRefName(branch)); - } + // mergeTo (integration branch) + List branches = new ArrayList(); + 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("mergeto", mergeToModel, branches)); - form.add(mergeto.setVisible(!branches.isEmpty())); - - } 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)); } + branches.remove(Repository.shortenRefName(getRepositoryModel().HEAD)); + branches.add(0, Repository.shortenRefName(getRepositoryModel().HEAD)); + + Fragment mergeto = new Fragment("mergeto", "mergeToFragment", this); + mergeto.add(new DropDownChoice("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 3f92eaa0..5e0e6820 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 @@ public class TicketPage extends TicketBasePage { 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 @@ public class TicketPage extends TicketBasePage { /* * UPDATE FORM (DISCUSSION TAB) */ - if (isAuthenticated && app().tickets().isAcceptingTicketUpdates(repository)) { + if (user.canEdit(ticket, repository) && app().tickets().isAcceptingTicketUpdates(repository)) { if (ticket.isOpen()) { /* * OPEN TICKET -- cgit v1.2.3