🐛 (collaboration): Fix dev login scopes and comment-delete visibility
Changes
9 files changed, +188 -10
MODIFY
docs/maintainers/comments.md
+4 -1
@@ -33,7 +33,10 @@
33
33
34
34
- `IssueResource`: `POST {number}/comments` and
35
35
`POST {number}/comments/{commentId}/delete`; the detail view passes the comment
36
- list plus `loggedIn` / `currentUserId` to `issue.html`.
36
+ list plus `loggedIn` / `currentUserId` / `canModerate` to `issue.html`. The
37
+ delete control renders when the viewer is the comment author or `canModerate`
38
+ (`AccessPolicy.canWrite` — owner, collaborator or org member), matching what the
39
+ service enforces; `owner` (admin) alone would hide it from collaborators.
37
40
- `MergeRequestResource`: `POST {number}/discussion` for general comments;
38
41
deletion reuses the existing `{number}/comments/{commentId}/delete`. `detail`
39
42
splits `commentService.list(mr)` into line comments (`filePath != null`) and
MODIFY
src/main/java/de/workaround/web/IssueResource.java
+4 -2
@@ -47,7 +47,7 @@
47
47
48
48
static native TemplateInstance issue(Repository repo, RepoNav nav, boolean owner, Issue issue,
49
49
String descriptionHtml, List<Issue.Status> statuses, List<User> assignees, boolean loggedIn,
50
- UUID currentUserId, List<IssueComment> comments);
50
+ UUID currentUserId, boolean canModerate, List<IssueComment> comments);
51
51
}
52
52
53
53
@Inject
@@ -125,9 +125,11 @@
125
125
boolean isOwner = accessPolicy.canAdmin(user, repo);
126
126
boolean loggedIn = user != null;
127
127
UUID currentUserId = user == null ? null : user.id;
128
+ // comment moderation (delete any comment) follows write access: owner, collaborator or org member
129
+ boolean canModerate = accessPolicy.canWrite(user, repo);
128
130
String descriptionHtml = issue.description == null ? null : Markdown.render(issue.description);
129
131
return Templates.issue(repo, repoNav.build(repo, uriInfo), isOwner, issue, descriptionHtml,
130
- List.of(Issue.Status.values()), assignableUsers(repo), loggedIn, currentUserId,
132
+ List.of(Issue.Status.values()), assignableUsers(repo), loggedIn, currentUserId, canModerate,
131
133
issueCommentService.list(issue));
132
134
}
133
135
MODIFY
src/main/java/de/workaround/web/MergeRequestResource.java
+6 -4
@@ -49,8 +49,8 @@
49
49
String defaultBranch);
50
50
51
51
static native TemplateInstance mergeRequest(Repository repo, RepoNav nav, boolean owner, boolean loggedIn,
52
- UUID currentUserId, MergeRequest mr, List<FileDiffView> files, int additions, int deletions,
53
- List<User> assignees, List<MergeRequestComment> discussion);
52
+ UUID currentUserId, boolean canModerate, MergeRequest mr, List<FileDiffView> files, int additions,
53
+ int deletions, List<User> assignees, List<MergeRequestComment> discussion);
54
54
}
55
55
56
56
/**
@@ -149,6 +149,8 @@
149
149
User user = currentUser.get();
150
150
boolean loggedIn = user != null;
151
151
UUID currentUserId = user == null ? null : user.id;
152
+ // comment moderation (delete any comment) follows write access: owner, collaborator or org member
153
+ boolean canModerate = accessPolicy.canWrite(user, repo);
152
154
153
155
List<FileDiffView> files = new ArrayList<>();
154
156
int additions = 0;
@@ -179,8 +181,8 @@
179
181
fileIndex++;
180
182
}
181
183
}
182
- return Templates.mergeRequest(repo, repoNav.build(repo, uriInfo), isOwner(repo), loggedIn, currentUserId, mr,
183
- files, additions, deletions, assignableUsers(repo), discussion);
184
+ return Templates.mergeRequest(repo, repoNav.build(repo, uriInfo), isOwner(repo), loggedIn, currentUserId,
185
+ canModerate, mr, files, additions, deletions, assignableUsers(repo), discussion);
184
186
}
185
187
186
188
/**
ADD
src/main/resources/quarkus-realm.json
+56 -0
@@ -0,0 +1,56 @@
1
+{
2
+ "realm": "quarkus",
3
+ "enabled": true,
4
+ "sslRequired": "none",
5
+ "registrationAllowed": false,
6
+ "loginWithEmailAllowed": true,
7
+ "duplicateEmailsAllowed": false,
8
+ "roles": {
9
+ "realm": [
10
+ { "name": "user", "description": "User privileges" },
11
+ { "name": "admin", "description": "Administrator privileges" },
12
+ { "name": "confidential", "description": "Confidential privileges" }
13
+ ]
14
+ },
15
+ "users": [
16
+ {
17
+ "username": "alice",
18
+ "enabled": true,
19
+ "email": "alice@example.com",
20
+ "emailVerified": true,
21
+ "firstName": "Alice",
22
+ "lastName": "Demo",
23
+ "credentials": [
24
+ { "type": "password", "value": "alice" }
25
+ ],
26
+ "realmRoles": [ "user", "admin" ]
27
+ },
28
+ {
29
+ "username": "bob",
30
+ "enabled": true,
31
+ "email": "bob@example.com",
32
+ "emailVerified": true,
33
+ "firstName": "Bob",
34
+ "lastName": "Demo",
35
+ "credentials": [
36
+ { "type": "password", "value": "bob" }
37
+ ],
38
+ "realmRoles": [ "user" ]
39
+ }
40
+ ],
41
+ "clients": [
42
+ {
43
+ "clientId": "quarkus-app",
44
+ "enabled": true,
45
+ "publicClient": false,
46
+ "secret": "secret",
47
+ "clientAuthenticatorType": "client-secret",
48
+ "standardFlowEnabled": true,
49
+ "directAccessGrantsEnabled": true,
50
+ "redirectUris": [ "*" ],
51
+ "webOrigins": [ "*" ],
52
+ "defaultClientScopes": [ "profile", "email", "roles", "web-origins", "acr", "basic" ],
53
+ "optionalClientScopes": [ "address", "phone", "offline_access", "microprofile-jwt" ]
54
+ }
55
+ ]
56
+}
MODIFY
src/main/resources/templates/IssueResource/issue.html
+1 -1
@@ -44,7 +44,7 @@
44
44
</div>
45
45
<div class="comment-body">{c.body}</div>
46
46
</div>
47
- {#if owner || c.author.id == currentUserId}
47
+ {#if canModerate || c.author.id == currentUserId}
48
48
<form class="comment-del-form" method="post"
49
49
action="/repos/{repo.ownerHandle}/{repo.name}/issues/{issue.number}/comments/{c.id}/delete">
50
50
<button type="submit" class="comment-del" title="Delete comment" aria-label="Delete comment">
MODIFY
src/main/resources/templates/MergeRequestResource/mergeRequest.html
+2 -2
@@ -121,7 +121,7 @@
121
121
</div>
122
122
<div class="comment-body">{c.body}</div>
123
123
</div>
124
- {#if owner || c.author.id == currentUserId}
124
+ {#if canModerate || c.author.id == currentUserId}
125
125
<form class="comment-del-form" method="post"
126
126
action="/repos/{repo.ownerHandle}/{repo.name}/merge-requests/{mr.number}/comments/{c.id}/delete">
127
127
<button type="submit" class="comment-del" title="Delete comment" aria-label="Delete comment">
@@ -196,7 +196,7 @@
196
196
</div>
197
197
<div class="comment-body">{c.body}</div>
198
198
</div>
199
- {#if owner || c.author.id == currentUserId}
199
+ {#if canModerate || c.author.id == currentUserId}
200
200
<form class="comment-del-form" method="post"
201
201
action="/repos/{repo.ownerHandle}/{repo.name}/merge-requests/{mr.number}/comments/{c.id}/delete">
202
202
<button type="submit" class="comment-del" title="Delete comment" aria-label="Delete comment">
ADD
src/test/java/de/workaround/dev/DevRealmFileTest.java
+47 -0
@@ -0,0 +1,47 @@
1
+package de.workaround.dev;
2
+
3
+import java.io.InputStream;
4
+import java.util.List;
5
+
6
+import org.junit.jupiter.api.Test;
7
+
8
+import com.fasterxml.jackson.databind.JsonNode;
9
+import com.fasterxml.jackson.databind.ObjectMapper;
10
+
11
+import static org.junit.jupiter.api.Assertions.assertNotNull;
12
+import static org.junit.jupiter.api.Assertions.assertTrue;
13
+
14
+/**
15
+ * Guards the dev/test Keycloak realm shipped for {@code quarkus.keycloak.devservices.realm-path}. Without this
16
+ * file the auto-provisioned realm's {@code quarkus-app} client lacks the {@code profile}/{@code email} client
17
+ * scopes the app requests via {@code quarkus.oidc.authentication.scopes}, so every browser login fails with
18
+ * {@code invalid_scope}. The file must therefore exist and grant both scopes to the client by default.
19
+ */
20
+class DevRealmFileTest
21
+{
22
+ @Test
23
+ void quarkusAppClientGrantsProfileAndEmailScopesByDefault() throws Exception
24
+ {
25
+ try (InputStream in = Thread.currentThread().getContextClassLoader().getResourceAsStream("quarkus-realm.json"))
26
+ {
27
+ assertNotNull(in, "quarkus-realm.json must be on the classpath (referenced by devservices.realm-path)");
28
+ JsonNode realm = new ObjectMapper().readTree(in);
29
+
30
+ JsonNode client = null;
31
+ for (JsonNode c : realm.path("clients"))
32
+ {
33
+ if ("quarkus-app".equals(c.path("clientId").asText()))
34
+ {
35
+ client = c;
36
+ break;
37
+ }
38
+ }
39
+ assertNotNull(client, "realm must define the quarkus-app client");
40
+
41
+ List<String> defaultScopes = new ObjectMapper()
42
+ .convertValue(client.path("defaultClientScopes"), List.class);
43
+ assertTrue(defaultScopes.contains("profile"), "quarkus-app must grant the 'profile' scope by default");
44
+ assertTrue(defaultScopes.contains("email"), "quarkus-app must grant the 'email' scope by default");
45
+ }
46
+ }
47
+}
MODIFY
src/test/java/de/workaround/web/IssueCommentUiTest.java
+29 -0
@@ -4,6 +4,7 @@
4
4
5
5
import org.junit.jupiter.api.Test;
6
6
7
+import de.workaround.git.CollaboratorService;
7
8
import de.workaround.git.GitRepositoryService;
8
9
import de.workaround.git.IssueCommentService;
9
10
import de.workaround.git.IssueService;
@@ -35,6 +36,28 @@
35
36
@Inject
36
37
User.Repo userRepo;
37
38
39
+ @Inject
40
+ CollaboratorService collaboratorService;
41
+
42
+ @Test
43
+ @TestSecurity(user = "ic-collab")
44
+ void aCollaboratorSeesTheDeleteControlOnAnotherUsersComment()
45
+ {
46
+ User collaborator = persistUser("ic-collab");
47
+ User owner = persistUser("ic-owner-collab-" + UUID.randomUUID().toString().substring(0, 8));
48
+ Repository repo = service.create(owner, "board", Repository.Visibility.PUBLIC, null);
49
+ addCollaborator(owner, repo, collaborator);
50
+ Issue issue = issueService.create(owner, repo, "Team issue", null);
51
+ IssueComment comment = comments.add(owner, issue, "owner wrote this");
52
+ String detail = "/repos/" + owner.username + "/board/issues/" + issue.number;
53
+
54
+ // a collaborator (write access, not the owner and not the author) may delete comments,
55
+ // so the delete control must be rendered for them
56
+ given().when().get(detail)
57
+ .then().statusCode(200)
58
+ .body(containsString(detail + "/comments/" + comment.id + "/delete"));
59
+ }
60
+
38
61
@Test
39
62
@TestSecurity(user = "ic-owner")
40
63
void ownerCanCommentOnAnIssueAndSeeItInline()
@@ -126,6 +149,12 @@
126
149
}
127
150
128
151
@Transactional
152
+ void addCollaborator(User owner, Repository repo, User member)
153
+ {
154
+ collaboratorService.add(owner, repo, member.username);
155
+ }
156
+
157
+ @Transactional
129
158
User persistUser(String name)
130
159
{
131
160
User existing = userRepo.findByOidcSubOptional(name).orElse(null);
MODIFY
src/test/java/de/workaround/web/MergeRequestDiscussionUiTest.java
+39 -0
@@ -10,10 +10,13 @@
10
10
import org.eclipse.jgit.transport.RefSpec;
11
11
import org.junit.jupiter.api.Test;
12
12
13
+import de.workaround.git.CollaboratorService;
13
14
import de.workaround.git.GitRepositoryService;
14
15
import de.workaround.git.GitTestSeeder;
16
+import de.workaround.git.MergeRequestCommentService;
15
17
import de.workaround.git.MergeRequestService;
16
18
import de.workaround.model.MergeRequest;
19
+import de.workaround.model.MergeRequestComment;
17
20
import de.workaround.model.Repository;
18
21
import de.workaround.model.User;
19
22
import io.quarkus.test.junit.QuarkusTest;
@@ -36,6 +39,30 @@
36
39
@Inject
37
40
User.Repo userRepo;
38
41
42
+ @Inject
43
+ CollaboratorService collaboratorService;
44
+
45
+ @Inject
46
+ MergeRequestCommentService commentService;
47
+
48
+ @Test
49
+ @TestSecurity(user = "mrd-collab")
50
+ void aCollaboratorSeesTheDeleteControlOnAnotherUsersDiscussionComment()
51
+ {
52
+ User collaborator = persistUser("mrd-collab");
53
+ User owner = persistUser("mrd-owner-collab-" + UUID.randomUUID().toString().substring(0, 8));
54
+ MergeRequest mr = seededMr(owner, "board");
55
+ addCollaborator(owner, mr.repository, collaborator);
56
+ MergeRequestComment comment = addDiscussion(owner, mr, "owner note on the MR");
57
+ String detail = "/repos/" + owner.username + "/board/merge-requests/" + mr.number;
58
+
59
+ // a collaborator (write access, not the owner and not the author) may delete comments,
60
+ // so the delete control must be rendered for them
61
+ given().when().get(detail)
62
+ .then().statusCode(200)
63
+ .body(containsString(detail + "/comments/" + comment.id + "/delete"));
64
+ }
65
+
39
66
@Test
40
67
@TestSecurity(user = "mrd-owner")
41
68
void ownerCanPostAGeneralDiscussionCommentAndSeeIt()
@@ -99,6 +126,18 @@
99
126
}
100
127
101
128
@Transactional
129
+ void addCollaborator(User owner, Repository repo, User member)
130
+ {
131
+ collaboratorService.add(owner, repo, member.username);
132
+ }
133
+
134
+ @Transactional
135
+ MergeRequestComment addDiscussion(User author, MergeRequest mr, String body)
136
+ {
137
+ return commentService.addGeneral(author, mr, body);
138
+ }
139
+
140
+ @Transactional
102
141
User persistUser(String name)
103
142
{
104
143
User existing = userRepo.findByOidcSubOptional(name).orElse(null);