THIS IS A TEST INSTANCE ONLY! REPOSITORIES CAN BE DELETED AT ANY TIME!

Browse Source

checklist: Fix authorization related bug

Previously @IsAllowed annotation was used.
But I found there is a bug.  Becuase most of annotation based
authorization is depends on PathParser. But PathParser
is rather unstable because of its url parsing method.

So, for the reason, a user who has authorization can not change state of checklist,
except issue author.

This patch fix that problem.
tags/v1.11.0
Suwon Chae 1 year ago
parent
commit
0757f21501
2 changed files with 16 additions and 2 deletions
  1. +8
    -1
      app/controllers/api/BoardApi.java
  2. +8
    -1
      app/controllers/api/IssueApi.java

+ 8
- 1
app/controllers/api/BoardApi.java View File

@@ -122,7 +122,6 @@ public class BoardApi extends AbstractPostingApp {
}

@Transactional
@IsAllowed(Operation.UPDATE)
public static Result updatePostingContent(String owner, String projectName, Long number) {
User user = UserApp.currentUser();
if (user.isAnonymous()) {
@@ -137,6 +136,10 @@ public class BoardApi extends AbstractPostingApp {
Project project = Project.findByOwnerAndProjectName(owner, projectName);
final Posting posting = Posting.findByNumber(project, number);

if (!AccessControl.isAllowed(user, posting.asResource(), Operation.UPDATE)) {
return forbidden(Json.newObject().put("message", "Forbidden request"));
}

String content = json.findValue("content").asText();
String rememberedChecksum = json.findValue("sha1").asText();

@@ -208,6 +211,10 @@ public class BoardApi extends AbstractPostingApp {
final Posting posting = Posting.findByNumber(project, number);
PostingComment postingComment = posting.findCommentByCommentId(commentId);

if (!AccessControl.isAllowed(user, postingComment.asResource(), Operation.UPDATE)) {
return forbidden(Json.newObject().put("message", "Forbidden request"));
}

if (isModifiedByOthers(postingComment.contents, rememberedChecksum)) {
return conflicted(postingComment.contents);
}


+ 8
- 1
app/controllers/api/IssueApi.java View File

@@ -196,7 +196,6 @@ public class IssueApi extends AbstractPostingApp {
}

@Transactional
@IsAllowed(Operation.UPDATE)
public static Result updateIssueContent(String owner, String projectName, Long number) {

User user = UserApp.currentUser();
@@ -212,6 +211,10 @@ public class IssueApi extends AbstractPostingApp {
Project project = Project.findByOwnerAndProjectName(owner, projectName);
final Issue issue = Issue.findByNumber(project, number);

if (!AccessControl.isAllowed(user, issue.asResource(), Operation.UPDATE)) {
return forbidden(Json.newObject().put("message", "Forbidden request"));
}

String content = json.findValue("content").asText();
String rememberedChecksum = json.findValue("sha1").asText();

@@ -415,6 +418,10 @@ public class IssueApi extends AbstractPostingApp {
return conflicted(issueComment.contents);
}

if (!AccessControl.isAllowed(user, issueComment.asResource(), Operation.UPDATE)) {
return forbidden(Json.newObject().put("message", "Forbidden request"));
}

issueComment.contents = comment;
issueComment.save();



Loading…
Cancel
Save